Skip to content

[WIP] hco, net: Disable HCO network-resources-injector when SR-IOV deploys its own - #369

Draft
nirdothan wants to merge 1 commit into
openshift-virtualization:mainfrom
nirdothan:net-res-inj-dedup
Draft

[WIP] hco, net: Disable HCO network-resources-injector when SR-IOV deploys its own#369
nirdothan wants to merge 1 commit into
openshift-virtualization:mainfrom
nirdothan:net-res-inj-dedup

Conversation

@nirdothan

Copy link
Copy Markdown
Contributor

Both the SR-IOV Network Operator and HCO can deploy a network-resources-injector mutating webhook. Having both active does not cause conflicts, but is wasteful. In such cases, it is better to keep the SR-IOV webhook and disbles the HCO one, as the former has a wider scope of operation whereas the later only intercepts virt-launcher pods.

Set HCO spec.deployment.deployNetworkResourcesInjector to false whenever SriovOperatorConfig/default has spec.enableInjector=true. When SR-IOV's injector is disabled (or its CRD is absent) the field is left unmanaged so HCO reverts to its own default via SSA ownership release.

Reading a boolean from an arbitrary live object had no template helper, so add objectFieldBool and the accompanying static RBAC read on sriovoperatorconfigs.

Both the SR-IOV Network Operator and HCO can deploy
a network-resources-injector mutating webhook.
Having both active does not cause conflicts, but
is wasteful.
In such cases, it is better to keep the SR-IOV
webhook and disble the HCO one,
as the former has a wider scope of operation
whereas the later only intercepts
virt-launcher pods.

Set HCO spec.deployment.deployNetworkResourcesInjector
to false whenever SriovOperatorConfig/default has
spec.enableInjector=true.
When SR-IOV's injector is disabled (or its CRD is absent)
the field is left unmanaged so HCO reverts to its own
default via SSA ownership release.

Reading a boolean from an arbitrary live object had no
template helper, so add objectFieldBool and the
accompanying static RBAC read on sriovoperatorconfigs.

Signed-off-by: Nir Dothan <ndothan@redhat.com>
Assisted by: Claude Sonnet 4.5 <noreply@anthropic.com>
@openshift-ci

openshift-ci Bot commented Aug 31, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign fabiand for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci

openshift-ci Bot commented Aug 31, 2026

Copy link
Copy Markdown

Hi @nirdothan. Thanks for your PR.

I'm waiting for a openshift-virtualization member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@nirdothan
nirdothan marked this pull request as draft August 31, 2026 17:26
@nirdothan nirdothan changed the title hco, net: Disable HCO network-resources-injector when SR-IOV deploys its own [WIP] hco, net: Disable HCO network-resources-injector when SR-IOV deploys its own Aug 31, 2026
@nirdothan

Copy link
Copy Markdown
Contributor Author

/cc @tiraboschi

Checking the direction, looking for feedback.
Still missing:

  • Listen to sriovoperatorconfigs events, as we can't wait 5 minutes for periodic sync in case sriov operator is uninstalled.
  • Integration test

@openshift-ci
openshift-ci Bot requested a review from tiraboschi August 31, 2026 17:30
@nirdothan

Copy link
Copy Markdown
Contributor Author

/cc @orelmisan

@openshift-ci

openshift-ci Bot commented Aug 31, 2026

Copy link
Copy Markdown

@nirdothan: GitHub didn't allow me to request PR reviews from the following users: orelmisan.

Note that only openshift-virtualization members and repo collaborators can review this PR, and authors cannot review their own PRs.

Details

In response to this:

/cc @orelmisan

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@tiraboschi

Copy link
Copy Markdown
Member

/ok-to-test

Comment on lines +9 to +15
{{- if objectFieldBool "sriovnetwork.openshift.io/v1" "SriovOperatorConfig" "sriov-network-operator" "default" "spec.enableInjector" }}
spec:
deployment:
deployNetworkResourcesInjector: false
{{- else }}
spec: {}
{{- end }}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HCO CR is ideally the entry-point for autopilot, not one of the managed assets. Can't we do the opposite and control SRIOV Operator setting spec.enableInjector: false there when we have the CNV one?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No we can't. Especially because if any bugs or issues occur in sriov-operator, we don't want us to be responsible for switching off their injector.
But also, because they are not identical: we only target virt-launcher pods and they target all pods. This leaves out sriov pods that are not CNV.

@openshift-ci

openshift-ci Bot commented Sep 4, 2026

Copy link
Copy Markdown

PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants