Skip to content

CNV-93096: Fix wrong condition for the CPUManager feature gate - #321

Open
nunnatsa wants to merge 1 commit into
openshift-virtualization:mainfrom
nunnatsa:fix-CNV-93096
Open

CNV-93096: Fix wrong condition for the CPUManager feature gate#321
nunnatsa wants to merge 1 commit into
openshift-virtualization:mainfrom
nunnatsa:fix-CNV-93096

Conversation

@nunnatsa

@nunnatsa nunnatsa commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

The CPUManager is not HCO's feature gate, but KubeVirt's.

Added new condition type for KV FGs, and use it to replace the existing
condition for kubelet-cpu-manager.

@github-actions

Copy link
Copy Markdown
Contributor

Generated Files Verification Failed

One or more generated files in this PR are out of sync:

  • CRDs: Run make update-crds if CRD verification failed
  • RBAC: Run make generate-rbac if RBAC verification failed

Please regenerate the files locally and commit the changes.

@openshift-ci

openshift-ci Bot commented Aug 19, 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 machadovilaca 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

@nunnatsa nunnatsa changed the title WIP: add new condition type for KV FGs CNV-93096: Fix wrong condition for the CPUManager feature gate Aug 20, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Generated Files Verification Failed

One or more generated files in this PR are out of sync:

  • CRDs: Run make update-crds if CRD verification failed
  • RBAC: Run make generate-rbac if RBAC verification failed

Please regenerate the files locally and commit the changes.

@nunnatsa
nunnatsa force-pushed the fix-CNV-93096 branch 2 times, most recently from 73e631d to 7b98f64 Compare August 20, 2026 10:02
@github-actions

Copy link
Copy Markdown
Contributor

Generated Files Verification Failed

One or more generated files in this PR are out of sync:

  • CRDs: Run make update-crds if CRD verification failed
  • RBAC: Run make generate-rbac if RBAC verification failed

Please regenerate the files locally and commit the changes.

@github-actions

Copy link
Copy Markdown
Contributor

Generated Files Verification Failed

One or more generated files in this PR are out of sync:

  • CRDs: Run make update-crds if CRD verification failed
  • RBAC: Run make generate-rbac if RBAC verification failed

Please regenerate the files locally and commit the changes.

Comment thread docs/ARCHITECTURE.md Outdated
| `pci-passthrough` | | MachineConfig | Opt-in: hardware + annotation condition |
| `kubelet-perf-settings` | | KubeletConfig | Always-on baseline |
| `kubelet-cpu-manager` | | KubeletConfig | Opt-in: CPUManager feature gate |
| `kubelet-cpu-manager` | | KubeletConfig | Always-on baseline |

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.

Opt-in: CPUManager KubeVirt feature gate (on by default) ?

@@ -0,0 +1,42 @@
package resources

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.

please add copyright header

@@ -0,0 +1,235 @@
package resources_test

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.

same

Comment thread pkg/rbac/rbac_test.go Outdated
rules := StaticRules()
if len(rules) != 8 {
if len(rules) != 9 {
t.Errorf("expected 8 static rules, got %d", len(rules))

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.

now 9 also in the error message (or make it dynamic for the future).

Comment thread pkg/context/render_context.go Outdated
Topology *TopologyContext // Cluster topology info (HCP, compact, node counts)
Images map[string]string // Container images from RELATED_IMAGE_* env vars
Params map[string]string // Per-asset parameters from metadata template_params
KubeVirtFeatureGates []string // Full KubeVirt object, templates access directly

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.

now just the FG list

Comment thread pkg/assets/testdata/real Outdated
@@ -0,0 +1 @@
../../../assets No newline at end of file

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.

I don't really like the symlink approach, I think it could become fragile. Can we avoid it if it's just for loader := NewLoaderFromFS(real.EmbeddedFS) and directly use the production embedded ones ?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I'll drop the whole thing

Comment thread pkg/controller/hco_context.go Outdated

// Build constructs a RenderContext from the current HCO state
func (b *RenderContextBuilder) Build(ctx context.Context, hco *unstructured.Unstructured) (*pkgcontext.RenderContext, error) {
func (b *RenderContextBuilder) Build(ctx context.Context, hco *unstructured.Unstructured, kvFG []string) (*pkgcontext.RenderContext, error) {

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.

I find the kvFG []string parameter on RenderContextBuilder.Build() a bit awkward. Build() already collects cluster state (nodes, topology, infra CRs), so KubeVirt feature gates fit better there than as a controller-owned input. Could we move getKubeVirtFGs into RenderContextBuilder (soft-fail to nil on error) and have updateConditionEvaluator read renderCtx.KubeVirtFeatureGates (so just r.updateConditionEvaluator(hco, renderCtx) instead of r.updateConditionEvaluator(hco, kvFGs, renderCtx)) ?

@nunnatsa
nunnatsa force-pushed the fix-CNV-93096 branch 4 times, most recently from 842852e to 1875258 Compare August 23, 2026 11:14
The CPUManager is not HCO's feature gate, but KubeVirt's.

Added new condition type for KV FGs, and use it to replace the existing
condition for kubelet-cpu-manager.

Signed-off-by: Nahshon Unna Tsameret <nunnatsa@redhat.com>
@nunnatsa

Copy link
Copy Markdown
Collaborator Author

/cherry-pick release-5.0

@openshift-cherrypick-robot

Copy link
Copy Markdown

@nunnatsa: once the present PR merges, I will cherry-pick it on top of release-5.0 in a new PR and assign it to you.

Details

In response to this:

/cherry-pick release-5.0

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.

@nunnatsa

Copy link
Copy Markdown
Collaborator Author

/cherry-pick release-4.23

@openshift-cherrypick-robot

Copy link
Copy Markdown

@nunnatsa: once the present PR merges, I will cherry-pick it on top of release-4.23 in a new PR and assign it to you.

Details

In response to this:

/cherry-pick release-4.23

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.

@nunnatsa

Copy link
Copy Markdown
Collaborator Author

/hold

trying to watch the KubeVirt CRD

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.

3 participants