Skip to content

SPLAT-2717: Migrate vSphere sync of kube-cloud-config to 3CMO#481

Open
vr4manta wants to merge 2 commits intoopenshift:mainfrom
vr4manta:SPLAT-2717
Open

SPLAT-2717: Migrate vSphere sync of kube-cloud-config to 3CMO#481
vr4manta wants to merge 2 commits intoopenshift:mainfrom
vr4manta:SPLAT-2717

Conversation

@vr4manta
Copy link
Copy Markdown
Contributor

@vr4manta vr4manta commented Apr 8, 2026 •
edited by coderabbitai Bot
Loading

SPLAT-2717

Summary

This PR adds logic to the KubeCloudConfigController to conditionally skip managing the kube-cloud-config ConfigMap based on platform-specific feature gates.
Specifically, for vSphere platforms, when the VSphereMultiVCenterDay2 feature gate is enabled, ownership of the ConfigMap transfers to the
cluster-cloud-controller-manager-operator.

Changes

Controller Updates (pkg/operator/kube_cloud_config/controller.go)

  1. Added Feature Gate Support
    - Added featureGateLister field to KubeCloudConfigController struct
    - Updated NewController to accept and wire up the feature gate lister and informer
    - Controller now watches feature gate changes via the informer
  2. Platform-Specific Management Logic
    - Created shouldManageCloudConfig() method with a switch statement for platform-specific logic
    - For vSphere: Checks if VSphereMultiVCenterDay2 feature gate is enabled; skips management if enabled
    - For all other platforms (AWS, Azure, GCP, etc.): Always manages the ConfigMap
    - Extensible design allows easy addition of feature gate checks for other platforms in the future
  3. Feature Gate Checking Helper
    - Created isFeatureGateEnabled() method that encapsulates all feature gate lister logic
    - Handles missing lister, NotFound errors, and feature gate status checking
    - Returns (false, nil) when feature gate is not enabled or unavailable
    - Returns error only for actual retrieval issues
    - Reusable across different platforms and feature gates
  4. Use of Constants
    - Uses features.FeatureGateVSphereMultiVCenterDay2 constant instead of hardcoded strings
    - Provides type safety and prevents typos

Starter Updates (pkg/operator/starter.go)

  • Updated kubeCloudConfigController initialization to pass feature gate lister and informer

Testing (pkg/operator/kube_cloud_config/controller_test.go)

Added comprehensive test coverage with Test_sync_withVSphereMultiVCenterDay2FeatureGate:

  • ✅ vSphere with feature gate enabled → ConfigMap management skipped (0 actions)
  • ✅ vSphere with feature gate disabled → ConfigMap managed normally (3 actions)
  • ✅ AWS with feature gate enabled → ConfigMap managed (feature gate ignored)
  • ✅ Azure with feature gate enabled → ConfigMap managed (feature gate ignored)
  • ✅ GCP with feature gate enabled → ConfigMap managed (feature gate ignored)

All existing tests continue to pass.

Design Decisions

  1. Switch Statement Pattern: Used a switch statement in shouldManageCloudConfig() to make platform-specific logic explicit and easy to extend
  2. Error Handling: Distinguished between "feature gate not enabled" (returns false, nil) and actual errors (returns false, err)
  3. Graceful Degradation: When feature gate lister is unavailable or feature gate resource is not found, the controller defaults to managing the ConfigMap
    (backwards compatible)
  4. Reusability: Encapsulated feature gate checking logic in a dedicated method for future use across different platforms

Testing

  • All existing unit tests pass
  • New platform-specific feature gate tests cover both enabled and disabled scenarios
  • Verified behavior across multiple platforms (vSphere, AWS, Azure, GCP)
  • Build succeeds without errors

Summary by CodeRabbit

  • Chores

    • Bumped OpenShift API, client, library, and related dependency versions

  • Improvements

    • Cloud configuration management now respects a vSphere feature gate to alter platform-specific behavior
    • Operator can be scoped via a new namespace flag and initializes a feature-gates accessor at startup
    • New utility exposes the operator release version from the environment

  • Tests

    • Added tests covering feature-gated platform behaviors in cloud configuration management

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Apr 8, 2026
@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 8, 2026
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Apr 8, 2026

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci-robot
Copy link
Copy Markdown

openshift-ci-robot commented Apr 8, 2026 •
edited by openshift-ci Bot
Loading

@vr4manta: This pull request references SPLAT-2717 which is a valid jira issue.

Details

In response to this:

SPLAT-2717

Summary

This PR adds logic to the KubeCloudConfigController to conditionally skip managing the kube-cloud-config ConfigMap based on platform-specific feature gates.
Specifically, for vSphere platforms, when the VSphereMultiVCenterDay2 feature gate is enabled, ownership of the ConfigMap transfers to the
cluster-cloud-controller-manager-operator.

Changes

Controller Updates (pkg/operator/kube_cloud_config/controller.go)

  1. Added Feature Gate Support
  • Added featureGateLister field to KubeCloudConfigController struct
  • Updated NewController to accept and wire up the feature gate lister and informer
  • Controller now watches feature gate changes via the informer
  1. Platform-Specific Management Logic
  • Created shouldManageCloudConfig() method with a switch statement for platform-specific logic
  • For vSphere: Checks if VSphereMultiVCenterDay2 feature gate is enabled; skips management if enabled
  • For all other platforms (AWS, Azure, GCP, etc.): Always manages the ConfigMap
  • Extensible design allows easy addition of feature gate checks for other platforms in the future
  1. Feature Gate Checking Helper
  • Created isFeatureGateEnabled() method that encapsulates all feature gate lister logic
  • Handles missing lister, NotFound errors, and feature gate status checking
  • Returns (false, nil) when feature gate is not enabled or unavailable
  • Returns error only for actual retrieval issues
  • Reusable across different platforms and feature gates
  1. Use of Constants
  • Uses features.FeatureGateVSphereMultiVCenterDay2 constant instead of hardcoded strings
  • Provides type safety and prevents typos

Starter Updates (pkg/operator/starter.go)

  • Updated kubeCloudConfigController initialization to pass feature gate lister and informer

Testing (pkg/operator/kube_cloud_config/controller_test.go)

Added comprehensive test coverage with Test_sync_withVSphereMultiVCenterDay2FeatureGate:

  • ✅ vSphere with feature gate enabled → ConfigMap management skipped (0 actions)
  • ✅ vSphere with feature gate disabled → ConfigMap managed normally (3 actions)
  • ✅ AWS with feature gate enabled → ConfigMap managed (feature gate ignored)
  • ✅ Azure with feature gate enabled → ConfigMap managed (feature gate ignored)
  • ✅ GCP with feature gate enabled → ConfigMap managed (feature gate ignored)

All existing tests continue to pass.

Design Decisions

  1. Switch Statement Pattern: Used a switch statement in shouldManageCloudConfig() to make platform-specific logic explicit and easy to extend
  2. Error Handling: Distinguished between "feature gate not enabled" (returns false, nil) and actual errors (returns false, err)
  3. Graceful Degradation: When feature gate lister is unavailable or feature gate resource is not found, the controller defaults to managing the ConfigMap
    (backwards compatible)
  4. Reusability: Encapsulated feature gate checking logic in a dedicated method for future use across different platforms

Testing

  • All existing unit tests pass
  • New platform-specific feature gate tests cover both enabled and disabled scenarios
  • Verified behavior across multiple platforms (vSphere, AWS, Azure, GCP)
  • Build succeeds without errors

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 openshift-eng/jira-lifecycle-plugin repository.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 8, 2026 •
edited
Loading

Walkthrough

Adds feature-gated, platform-aware control to the kube-cloud-config controller (skips managing the ConfigMap for vSphere when VSphereMultiVCenterDay2 is enabled), wires a FeatureGate accessor at startup, updates tests, bumps go.mod dependencies, and adds a util to read the operator release version.

Changes

Cohort / File(s) Summary
Dependency Updates
go.mod		 Bumped versions for github.com/openshift/api, github.com/openshift/client-go, github.com/openshift/library-go, gopkg.in/gcfg.v1; indirect gopkg.in/warnings.v0 bumped. No other module directives changed.
Kube-cloud-config controller
pkg/operator/kube_cloud_config/controller.go		 Controller now accepts featuregates.FeatureGateAccess, consults it during sync to decide whether to manage kube-cloud-config (special-case: vSphere + VSphereMultiVCenterDay2 enabled → skip), and standardized NotFound handling via apierrors.
Controller tests
pkg/operator/kube_cloud_config/controller_test.go		 Injects a deterministic FeatureGateAccess into tests and adds table-driven tests verifying VSphere behavior when VSphereMultiVCenterDay2 is enabled vs disabled; other platforms unchanged.
Startup wiring
pkg/operator/starter.go		 Adds ManagedNamespace option and --namespace flag, creates and starts a FeatureGate accessor (using informers and an events-backed recorder scoped to the managed namespace/pod), and passes it into the kube-cloud-config controller.
Utility
pkg/util/feature_gate.go		 New GetReleaseVersion() reads OPERATOR_IMAGE_VERSION env var, returning it or "unknown" and logging when unset.

Sequence Diagram(s)

sequenceDiagram
    participant Starter
    participant FGAccessor
    participant Controller
    participant API
    participant Recorder

    Starter->>FGAccessor: create(informers, recorder, releaseVersion)
    Starter->>FGAccessor: start() (async)
    Starter->>Controller: NewController(..., featureGateAccess)
    loop controller sync
        Controller->>API: Get ClusterInfrastructure (platform)
        Controller->>FGAccessor: CurrentFeatureGates() / isEnabled(VSphereMultiVCenterDay2)
        alt platform == vSphere and feature enabled
            Controller->>Recorder: emit event("not managing kube-cloud-config")
            Controller-->>API: skip ConfigMap update
        else
            Controller->>API: get/create/update `kube-cloud-config` ConfigMap
            Controller->>Recorder: emit event(update/create)
        end
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 10 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Test Structure And Quality ⚠️ Warning Tests lack meaningful assertion failure messages and use deprecated configfakeclient.NewSimpleClientset() API on lines 218 and 369. Add context messages to Expect() calls and replace deprecated NewSimpleClientset() with NewClientset() to resolve SA1019 lint warnings.
✅ Passed checks (10 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically identifies the main change: migrating vSphere kube-cloud-config sync to 3CMO via feature gate awareness, which aligns with the core objective of conditionally skipping ConfigMap management based on platform-specific feature gates.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed All test names in the pull request are stable and deterministic with no dynamic content such as timestamps, UUIDs, or generated identifiers.
Microshift Test Compatibility ✅ Passed The PR adds unit tests using Go testing framework, not Ginkgo e2e tests. No Ginkgo patterns or imports are present.
Single Node Openshift (Sno) Test Compatibility ✅ Passed The pull request does not add new Ginkgo e2e tests. All test additions are standard Go unit tests using testing package without Ginkgo imports.
Topology-Aware Scheduling Compatibility ✅ Passed This pull request introduces no scheduling constraints or topology-related configurations. Changes are exclusively in controller business logic with no deployment manifest modifications.
Ote Binary Stdout Contract ✅ Passed Logging is initialized first via library-go controllercmd framework, then GetReleaseVersion() is safely called from RunOperator() with only klog output to stderr.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed The custom check is not applicable to this pull request. The test additions use standard Go testing package unit tests, not Ginkgo e2e tests.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Review rate limit: 8/10 reviews remaining, refill in 6 minutes and 55 seconds.

Comment @coderabbitai help to get the list of available commands and usage tips.

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Apr 8, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: vr4manta
Once this PR has been reviewed and has the lgtm label, please assign joelspeed 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 added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 10, 2026
JoelSpeed
JoelSpeed reviewed Apr 21, 2026
View reviewed changes
Comment thread pkg/operator/kube_cloud_config/controller.go Outdated
Comment on lines +236 to +243
// Check all versions in the status
for _, featureGateDetails := range featureGate.Status.FeatureGates {
for _, enabledGate := range featureGateDetails.Enabled {
if enabledGate.Name == gateName {
return true, nil
}
}
}
Copy link
Copy Markdown
Contributor

@JoelSpeed JoelSpeed Apr 21, 2026

Choose a reason for hiding this comment

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

This is not the correct behaviour. Please use the standard feature gate accessor pattern that exists in library go. You should be able to see this demonstrated in the main of many operators, I think CCMO and CPMSO both have it.

We may have to fettle this slightly since the same manager is responsible for feature gates in the cluster, but since it's a different controller, we should be able to have this controller consume a feature gate accessor

At a minimum we should be doing

cluster-config-operator/pkg/operator/featuregates/featuregate_controller.go

Line 94 in e0c2428

currentDetails, err := FeaturesGateDetailsFromFeatureSets(c.featureSetMap, featureGates, c.processVersion)

Copy link
Copy Markdown
Contributor Author

@vr4manta vr4manta Apr 30, 2026

Choose a reason for hiding this comment

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

Makes sense. I'll update this.

Copy link
Copy Markdown
Contributor Author

@vr4manta vr4manta Apr 30, 2026

Choose a reason for hiding this comment

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

ok, i took a stab at using accessor instead of informer. take a look and let me know what you think.

@openshift-ci openshift-ci Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 30, 2026
coderabbitai[bot]
coderabbitai Bot reviewed Apr 30, 2026
View reviewed changes
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
pkg/operator/kube_cloud_config/controller_test.go (1)

258-307: ⚡ Quick win

Add the vSphere / nil-lister fallback case.

This table covers enabled/disabled gate states, but it still misses the new vSphere path where featureGateLister is unavailable and the controller should keep managing kube-cloud-config. That fallback is part of the PR’s intended behavior, so it’s worth pinning down here.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@pkg/operator/kube_cloud_config/controller_test.go` around lines 258 - 307,
Add a test case in Test_sync_withVSphereMultiVCenterDay2FeatureGate covering the
vSphere fallback when the featureGate lister is nil: create a case with
platformType configv1.VSpherePlatformType, inputData like `[Global]\ntest =
value`, set featureGateEnabled true but arrange the test harness to simulate a
nil featureGateLister (i.e., the lister used by the controller is absent), and
assert expectedActions == 3 (Get source, Get target, Update target); include a
description like "vSphere with featureGate lister nil should still update
kube-cloud-config (fallback)".
🤖 Prompt for all review comments with AI agents 
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@pkg/operator/kube_cloud_config/controller_test.go`:
- Around line 258-307: Add a test case in
Test_sync_withVSphereMultiVCenterDay2FeatureGate covering the vSphere fallback
when the featureGate lister is nil: create a case with platformType
configv1.VSpherePlatformType, inputData like `[Global]\ntest = value`, set
featureGateEnabled true but arrange the test harness to simulate a nil
featureGateLister (i.e., the lister used by the controller is absent), and
assert expectedActions == 3 (Get source, Get target, Update target); include a
description like "vSphere with featureGate lister nil should still update
kube-cloud-config (fallback)".
ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 421e7cfd-c156-48f9-8a01-fefc6560f3ec

📥 Commits

Reviewing files that changed from the base of the PR and between cd73cef and 9f574e6.

⛔ Files ignored due to path filters (88)
  • go.sum is excluded by !**/*.sum
  • vendor/github.com/openshift/api/config/v1/types_apiserver.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/api/config/v1/types_authentication.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/api/config/v1/types_kmsencryption.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/api/config/v1/zz_generated.deepcopy.go is excluded by !**/vendor/**, !vendor/**, !**/zz_generated*
  • vendor/github.com/openshift/api/config/v1/zz_generated.featuregated-crd-manifests.yaml is excluded by !**/vendor/**, !vendor/**, !**/zz_generated*
  • vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.go is excluded by !**/vendor/**, !vendor/**, !**/zz_generated*
  • vendor/github.com/openshift/api/config/v1alpha1/types_cluster_monitoring.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/api/config/v1alpha1/zz_generated.deepcopy.go is excluded by !**/vendor/**, !vendor/**, !**/zz_generated*
  • vendor/github.com/openshift/api/config/v1alpha1/zz_generated.swagger_doc_generated.go is excluded by !**/vendor/**, !vendor/**, !**/zz_generated*
  • vendor/github.com/openshift/api/features.md is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/api/features/features.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/api/machine/v1beta1/types_machineset.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/api/machine/v1beta1/zz_generated.swagger_doc_generated.go is excluded by !**/vendor/**, !vendor/**, !**/zz_generated*
  • vendor/github.com/openshift/api/operator/v1alpha1/types_clusterapi.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.deepcopy.go is excluded by !**/vendor/**, !vendor/**, !**/zz_generated*
  • vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.swagger_doc_generated.go is excluded by !**/vendor/**, !vendor/**, !**/zz_generated*
  • vendor/github.com/openshift/api/security/v1/generated.proto is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/api/security/v1/types.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/api/security/v1/zz_generated.featuregated-crd-manifests.yaml is excluded by !**/vendor/**, !vendor/**, !**/zz_generated*
  • vendor/github.com/openshift/api/security/v1/zz_generated.swagger_doc_generated.go is excluded by !**/vendor/**, !vendor/**, !**/zz_generated*
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/awsdnsspec.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/awskmsconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/infrastructurestatus.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/kmsconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/prefixedclaimmapping.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/usernameclaimmapping.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/vaultapproleauthentication.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/vaultauthentication.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/vaultconfigmapreference.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/vaultkmsconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/vaultsecretreference.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/vaulttlsconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/alertmanagercustomconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/clustermonitoringspec.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/containerresource.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/metricsserverconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/monitoringpluginconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/nodeexportercollectorbuddyinfoconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/nodeexportercollectorconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/nodeexportercollectorcpufreqconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/nodeexportercollectorethtoolconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/nodeexportercollectorksmdconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/nodeexportercollectormountstatsconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/nodeexportercollectornetclasscollectconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/nodeexportercollectornetclassconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/nodeexportercollectornetdevconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/nodeexportercollectorprocessesconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/nodeexportercollectorsystemdcollectconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/nodeexportercollectorsystemdconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/nodeexportercollectortcpstatconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/nodeexporterconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/openshiftstatemetricsconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/prometheusconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/prometheusoperatoradmissionwebhookconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/prometheusoperatorconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/telemeterclientconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/thanosquerierconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/internal/internal.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/utils.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/operator/applyconfigurations/internal/internal.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1/awscsidriverconfigspec.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1/bgpmanagedconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1/ingresscontrollertuningoptions.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1/nooverlayconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1/ovnkubernetesconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/library-go/pkg/config/serving/server.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/library-go/pkg/controller/controllercmd/builder.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/library-go/pkg/controller/controllercmd/cmd.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/library-go/pkg/crypto/cert_config.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/library-go/pkg/crypto/keygen.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/library-go/pkg/crypto/options.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/library-go/pkg/operator/certrotation/client_cert_rotation_controller.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/library-go/pkg/operator/certrotation/signer.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/library-go/pkg/operator/certrotation/target.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/library-go/pkg/pki/profile.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/library-go/pkg/pki/provider.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/library-go/pkg/pki/resolve.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/library-go/pkg/pki/types.go is excluded by !**/vendor/**, !vendor/**
  • vendor/gopkg.in/gcfg.v1/errors.go is excluded by !**/vendor/**, !vendor/**
  • vendor/gopkg.in/gcfg.v1/go1_0.go is excluded by !**/vendor/**, !vendor/**
  • vendor/gopkg.in/gcfg.v1/go1_2.go is excluded by !**/vendor/**, !vendor/**
  • vendor/gopkg.in/gcfg.v1/read.go is excluded by !**/vendor/**, !vendor/**
  • vendor/gopkg.in/gcfg.v1/scanner/scanner.go is excluded by !**/vendor/**, !vendor/**
  • vendor/gopkg.in/gcfg.v1/set.go is excluded by !**/vendor/**, !vendor/**
  • vendor/gopkg.in/warnings.v0/README is excluded by !**/vendor/**, !vendor/**
  • vendor/gopkg.in/warnings.v0/warnings.go is excluded by !**/vendor/**, !vendor/**
  • vendor/modules.txt is excluded by !**/vendor/**, !vendor/**
📒 Files selected for processing (4)
  • go.mod
  • pkg/operator/kube_cloud_config/controller.go
  • pkg/operator/kube_cloud_config/controller_test.go
  • pkg/operator/starter.go

@openshift-ci-robot
Copy link
Copy Markdown

openshift-ci-robot commented Apr 30, 2026 •
edited by openshift-ci Bot
Loading

@vr4manta: This pull request references SPLAT-2717 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target either version "5.0." or "openshift-5.0.", but it targets "openshift-4.22" instead.

Details

In response to this:

SPLAT-2717

Summary

This PR adds logic to the KubeCloudConfigController to conditionally skip managing the kube-cloud-config ConfigMap based on platform-specific feature gates.
Specifically, for vSphere platforms, when the VSphereMultiVCenterDay2 feature gate is enabled, ownership of the ConfigMap transfers to the
cluster-cloud-controller-manager-operator.

Changes

Controller Updates (pkg/operator/kube_cloud_config/controller.go)

  1. Added Feature Gate Support
  • Added featureGateLister field to KubeCloudConfigController struct
  • Updated NewController to accept and wire up the feature gate lister and informer
  • Controller now watches feature gate changes via the informer
  1. Platform-Specific Management Logic
  • Created shouldManageCloudConfig() method with a switch statement for platform-specific logic
  • For vSphere: Checks if VSphereMultiVCenterDay2 feature gate is enabled; skips management if enabled
  • For all other platforms (AWS, Azure, GCP, etc.): Always manages the ConfigMap
  • Extensible design allows easy addition of feature gate checks for other platforms in the future
  1. Feature Gate Checking Helper
  • Created isFeatureGateEnabled() method that encapsulates all feature gate lister logic
  • Handles missing lister, NotFound errors, and feature gate status checking
  • Returns (false, nil) when feature gate is not enabled or unavailable
  • Returns error only for actual retrieval issues
  • Reusable across different platforms and feature gates
  1. Use of Constants
  • Uses features.FeatureGateVSphereMultiVCenterDay2 constant instead of hardcoded strings
  • Provides type safety and prevents typos

Starter Updates (pkg/operator/starter.go)

  • Updated kubeCloudConfigController initialization to pass feature gate lister and informer

Testing (pkg/operator/kube_cloud_config/controller_test.go)

Added comprehensive test coverage with Test_sync_withVSphereMultiVCenterDay2FeatureGate:

  • ✅ vSphere with feature gate enabled → ConfigMap management skipped (0 actions)
  • ✅ vSphere with feature gate disabled → ConfigMap managed normally (3 actions)
  • ✅ AWS with feature gate enabled → ConfigMap managed (feature gate ignored)
  • ✅ Azure with feature gate enabled → ConfigMap managed (feature gate ignored)
  • ✅ GCP with feature gate enabled → ConfigMap managed (feature gate ignored)

All existing tests continue to pass.

Design Decisions

  1. Switch Statement Pattern: Used a switch statement in shouldManageCloudConfig() to make platform-specific logic explicit and easy to extend
  2. Error Handling: Distinguished between "feature gate not enabled" (returns false, nil) and actual errors (returns false, err)
  3. Graceful Degradation: When feature gate lister is unavailable or feature gate resource is not found, the controller defaults to managing the ConfigMap
    (backwards compatible)
  4. Reusability: Encapsulated feature gate checking logic in a dedicated method for future use across different platforms

Testing

  • All existing unit tests pass
  • New platform-specific feature gate tests cover both enabled and disabled scenarios
  • Verified behavior across multiple platforms (vSphere, AWS, Azure, GCP)
  • Build succeeds without errors

Summary by CodeRabbit

  • Chores

  • Updated OpenShift API, client, and library dependencies to newer versions

  • Improvements

  • Cloud configuration now respects platform-specific behavior driven by a vSphere feature gate

  • Operator can be scoped via a new namespace flag and initializes a feature-gates accessor at startup

  • Added utility to expose the operator release version from environment

  • Tests

  • Added tests covering feature-gated platform behaviors for cloud configuration management

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 openshift-eng/jira-lifecycle-plugin repository.

coderabbitai[bot]
coderabbitai Bot reviewed Apr 30, 2026
View reviewed changes
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
pkg/operator/kube_cloud_config/controller.go (1)

54-69: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

FeatureGate changes are not wired into reconcile triggers.

featureGateAccess is injected, but there is no FeatureGate informer in WithInformers, so a gate flip won’t trigger an immediate sync (only the 1-minute resync catches it). That can delay ownership handoff and cause short-lived controller contention.

Suggested wiring change 
 func NewController(operatorClient operatorv1helpers.OperatorClient,
 	infraClient configv1client.InfrastructuresGetter, infraLister configv1listers.InfrastructureLister, infraInformer cache.SharedIndexInformer,
 	configMapClient corev1client.ConfigMapsGetter,
 	openshiftConfigConfigMapInformer cache.SharedIndexInformer, openshiftConfigManagedConfigMapInformer cache.SharedIndexInformer,
+	featureGateInformer cache.SharedIndexInformer,
 	featureGateAccess featuregates.FeatureGateAccess,
 	recorder events.Recorder) factory.Controller {
@@
 	return factory.New().
 		WithInformers(
 			operatorClient.Informer(),
 			infraInformer,
 			openshiftConfigConfigMapInformer,
 			openshiftConfigManagedConfigMapInformer,
+			featureGateInformer,
 		).
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@pkg/operator/kube_cloud_config/controller.go` around lines 54 - 69,
KubeCloudConfigController receives featureGateAccess but doesn't include a
FeatureGate informer in the factory.WithInformers call, so feature gate flips
won't trigger an immediate reconcile; update the controller wiring to pass the
FeatureGate informer (obtained from featureGateAccess) into WithInformers (e.g.,
add featureGateAccess's informer—use the appropriate accessor such as
FeatureGateInformer() or Informer() provided by featuregates.FeatureGateAccess)
alongside operatorClient.Informer(), infraInformer,
openshiftConfigConfigMapInformer, and openshiftConfigManagedConfigMapInformer so
that the controller reacts immediately to gate changes.
🤖 Prompt for all review comments with AI agents 
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pkg/operator/kube_cloud_config/controller.go`:
- Around line 39-44: The struct KubeCloudConfigController was renamed from
featureGateLister to featureGateAccess causing tests to fail; update the test
struct literal(s) that construct KubeCloudConfigController (in the test that
reported the error) to use the new field name featureGateAccess and provide the
appropriate FeatureGateAccess implementation/mocking instance instead of
featureGateLister so the types match (search for KubeCloudConfigController and
featureGateLister in tests and replace the field name and injected mock
accordingly).

---

Outside diff comments:
In `@pkg/operator/kube_cloud_config/controller.go`:
- Around line 54-69: KubeCloudConfigController receives featureGateAccess but
doesn't include a FeatureGate informer in the factory.WithInformers call, so
feature gate flips won't trigger an immediate reconcile; update the controller
wiring to pass the FeatureGate informer (obtained from featureGateAccess) into
WithInformers (e.g., add featureGateAccess's informer—use the appropriate
accessor such as FeatureGateInformer() or Informer() provided by
featuregates.FeatureGateAccess) alongside operatorClient.Informer(),
infraInformer, openshiftConfigConfigMapInformer, and
openshiftConfigManagedConfigMapInformer so that the controller reacts
immediately to gate changes.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 382e707c-8e28-4a30-abf5-025d20d0d3ec

📥 Commits

Reviewing files that changed from the base of the PR and between 9f574e6 and 70839fb.

📒 Files selected for processing (3)
  • pkg/operator/kube_cloud_config/controller.go
  • pkg/operator/starter.go
  • pkg/util/feature_gate.go
✅ Files skipped from review due to trivial changes (1)
  • pkg/util/feature_gate.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • pkg/operator/starter.go

Comment thread pkg/operator/kube_cloud_config/controller.go
@vr4manta vr4manta marked this pull request as ready for review April 30, 2026 17:14
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 30, 2026
@openshift-ci openshift-ci Bot requested review from JoelSpeed and jstuever April 30, 2026 17:15
coderabbitai[bot]
coderabbitai Bot reviewed Apr 30, 2026
View reviewed changes
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents 
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pkg/operator/kube_cloud_config/controller_test.go`:
- Around line 351-358: Replace the deprecated fake.NewSimpleClientset() with the
non-deprecated fake.NewClientset() and check the errors returned by
fake.Tracker().Add calls; update the test to use fake.NewClientset() (or the
package's recommended constructor) and capture the error values from both
Tracker().Add(...) invocations and fail the test/assert on non-nil errors (e.g.,
t.Fatalf or require.NoError) so staticcheck SA1019 and errcheck violations are
resolved while keeping the rest of the setup in controller_test.go unchanged.

In `@pkg/operator/kube_cloud_config/controller.go`:
- Around line 218-227: In isFeatureGateEnabled, avoid hard-failing when feature
gates aren't initialized by first calling
c.featureGateAccess.AreInitialFeatureGatesObserved(); if it returns false,
return (false, nil) as a safe fallback; otherwise proceed to call
c.featureGateAccess.CurrentFeatureGates() and continue using
currentFeatures.Enabled(gateName) and propagate real errors from
CurrentFeatureGates() as before.

In `@pkg/operator/starter.go`:
- Around line 65-71: Move the namespace flag registration out of RunOperator and
into OperatorOptions by adding a ManagedNamespace string field to the
OperatorOptions struct (default "openshift-config-operator"), register it in
OperatorOptions.AddFlags using pflag (e.g., AddFlags should call
fs.StringVar(&o.ManagedNamespace, "namespace", o.ManagedNamespace, "...")),
remove the runtime call to flag.String in RunOperator, and update references in
RunOperator (previously using *managedNamespace at the locations referenced
around the former lines 112 and 122) to use o.ManagedNamespace directly; ensure
no stdlib flag usage remains to avoid duplicate registration and to keep
pflag-based CLI parsing consistent.
- Line 124: The goroutine is started with context.Background() which detaches it
from operator shutdown; replace the call to
featureGateAccessor.Run(context.Background()) so it uses the operator
cancellation context (ctx) instead by calling featureGateAccessor.Run(ctx) so
the Run method is cancelled when the operator context is cancelled.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: eb7e4cc7-2e35-4bc2-87b2-6d36cd650964

📥 Commits

Reviewing files that changed from the base of the PR and between 70839fb and 878a434.

📒 Files selected for processing (4)
  • pkg/operator/kube_cloud_config/controller.go
  • pkg/operator/kube_cloud_config/controller_test.go
  • pkg/operator/starter.go
  • pkg/util/feature_gate.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • pkg/util/feature_gate.go

Comment thread pkg/operator/kube_cloud_config/controller_test.go Outdated
Comment thread pkg/operator/kube_cloud_config/controller.go
Comment on lines +218 to +227
// isFeatureGateEnabled checks if the specified feature gate is enabled in the cluster.
// It returns an error only if there's an issue retrieving the feature gate (other than NotFound).
func (c KubeCloudConfigController) isFeatureGateEnabled(gateName configv1.FeatureGateName) (bool, error) {
currentFeatures, err := c.featureGateAccess.CurrentFeatureGates()
if err != nil {
klog.Errorf("unable to get feature gates: %v", err)
return false, err
}

return currentFeatures.Enabled(gateName), nil
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 30, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's look at the file to see the context around lines 218-227
cat -n pkg/operator/kube_cloud_config/controller.go | sed -n '210,235p'

Repository: openshift/cluster-config-operator

Length of output: 818

🏁 Script executed:

# Find the type definition for featureGateAccess to understand what methods are available
fd -t f "controller.go" pkg/operator/kube_cloud_config/ -x cat -n {} | head -100

Repository: openshift/cluster-config-operator

Length of output: 5181

🏁 Script executed:

# Search for the featureGateAccess field definition in the controller struct
rg "featureGateAccess" pkg/operator/kube_cloud_config/controller.go -A 2 -B 2

Repository: openshift/cluster-config-operator

Length of output: 1130

🏁 Script executed:

# Search for AreInitialFeatureGatesObserved method in the entire codebase
rg "AreInitialFeatureGatesObserved" --type go

Repository: openshift/cluster-config-operator

Length of output: 1419

🏁 Script executed:

# Search for the interface definition that featureGateAccess might implement
rg "FeatureGateAccess|featureGateAccess" pkg/operator/kube_cloud_config/ -A 3 -B 3

Repository: openshift/cluster-config-operator

Length of output: 6228

🏁 Script executed:

# Find the FeatureGateAccess interface definition
rg "type FeatureGateAccess interface" --type go -A 20

Repository: openshift/cluster-config-operator

Length of output: 3755

🏁 Script executed:

# Look for how CurrentFeatureGates() is used in the library-go vendored code
rg "CurrentFeatureGates\(\)" vendor/github.com/openshift/library-go/pkg/operator/configobserver/featuregates/ -B 3 -A 3

Repository: openshift/cluster-config-operator

Length of output: 5032

🏁 Script executed:

# Check if there are any error checks for uninitialized feature gates
rg "AreInitialFeatureGatesObserved" vendor/github.com/openshift/library-go/pkg/operator/configobserver/featuregates/ -B 5 -A 5

Repository: openshift/cluster-config-operator

Length of output: 10680

🏁 Script executed:

# Look at how isFeatureGateEnabled is called in the codebase to understand the impact
rg "isFeatureGateEnabled" pkg/operator/kube_cloud_config/controller.go -B 2 -A 2

Repository: openshift/cluster-config-operator

Length of output: 745

🏁 Script executed:

# Check if there's any context where this would fail at startup
rg "sync.*defer" pkg/operator/kube_cloud_config/controller.go -A 10 | head -40

Repository: openshift/cluster-config-operator

Length of output: 59

Handle uninitialized feature gates gracefully during startup.

Currently, any error from CurrentFeatureGates() is returned, including "featureGates not yet observed" errors that occur during startup. This hard-fails the sync when feature gates haven't been initialized yet, degrading controller state on vSphere. Check AreInitialFeatureGatesObserved() before calling CurrentFeatureGates() and return (false, nil) as a safe fallback for the not-yet-observed case.

Suggested fix 
 func (c KubeCloudConfigController) isFeatureGateEnabled(gateName configv1.FeatureGateName) (bool, error) {
+	if c.featureGateAccess == nil {
+		return false, nil
+	}
+	if !c.featureGateAccess.AreInitialFeatureGatesObserved() {
+		return false, nil
+	}
+
 	currentFeatures, err := c.featureGateAccess.CurrentFeatureGates()
 	if err != nil {
 		klog.Errorf("unable to get feature gates: %v", err)
-		return false, err
+		return false, err
 	}
 
 	return currentFeatures.Enabled(gateName), nil
 }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@pkg/operator/kube_cloud_config/controller.go` around lines 218 - 227, In
isFeatureGateEnabled, avoid hard-failing when feature gates aren't initialized
by first calling c.featureGateAccess.AreInitialFeatureGatesObserved(); if it
returns false, return (false, nil) as a safe fallback; otherwise proceed to call
c.featureGateAccess.CurrentFeatureGates() and continue using
currentFeatures.Enabled(gateName) and propagate real errors from
CurrentFeatureGates() as before.

Comment thread pkg/operator/starter.go Outdated
Comment thread pkg/operator/starter.go Outdated
JoelSpeed
JoelSpeed reviewed Apr 30, 2026
View reviewed changes
Comment thread pkg/operator/kube_cloud_config/controller.go
Comment on lines +221 to +225
currentFeatures, err := c.featureGateAccess.CurrentFeatureGates()
if err != nil {
klog.Errorf("unable to get feature gates: %v", err)
return false, err
}
Copy link
Copy Markdown
Contributor

@JoelSpeed JoelSpeed Apr 30, 2026

Choose a reason for hiding this comment

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

This shouldn't ever change result, so you can do this in the init of c and then you don't need any error for this path

Comment thread pkg/operator/kube_cloud_config/controller_test.go Outdated
Comment thread pkg/operator/starter.go Outdated
Comment thread pkg/operator/starter.go Outdated
coderabbitai[bot]
coderabbitai Bot reviewed May 1, 2026
View reviewed changes
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents 
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pkg/operator/kube_cloud_config/controller_test.go`:
- Line 369: Replace uses of the deprecated constructor
configfakeclient.NewSimpleClientset with configfakeclient.NewClientset to avoid
SA1019; update both occurrences (the fakeConfig assignment creating fakeConfig
and the earlier usage around line 218) so they call NewClientset(inputInfra)
with the same arguments, keeping the fakeConfig variable and test setup
unchanged except for the constructor name.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 493451cc-4573-4e1d-a483-be5cfd021049

📥 Commits

Reviewing files that changed from the base of the PR and between 878a434 and b52acc2.

⛔ Files ignored due to path filters (88)
  • go.sum is excluded by !**/*.sum
  • vendor/github.com/openshift/api/config/v1/types_apiserver.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/api/config/v1/types_authentication.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/api/config/v1/types_kmsencryption.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/api/config/v1/zz_generated.deepcopy.go is excluded by !**/vendor/**, !vendor/**, !**/zz_generated*
  • vendor/github.com/openshift/api/config/v1/zz_generated.featuregated-crd-manifests.yaml is excluded by !**/vendor/**, !vendor/**, !**/zz_generated*
  • vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.go is excluded by !**/vendor/**, !vendor/**, !**/zz_generated*
  • vendor/github.com/openshift/api/config/v1alpha1/types_cluster_monitoring.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/api/config/v1alpha1/zz_generated.deepcopy.go is excluded by !**/vendor/**, !vendor/**, !**/zz_generated*
  • vendor/github.com/openshift/api/config/v1alpha1/zz_generated.swagger_doc_generated.go is excluded by !**/vendor/**, !vendor/**, !**/zz_generated*
  • vendor/github.com/openshift/api/features.md is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/api/features/features.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/api/machine/v1beta1/types_machineset.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/api/machine/v1beta1/zz_generated.swagger_doc_generated.go is excluded by !**/vendor/**, !vendor/**, !**/zz_generated*
  • vendor/github.com/openshift/api/operator/v1alpha1/types_clusterapi.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.deepcopy.go is excluded by !**/vendor/**, !vendor/**, !**/zz_generated*
  • vendor/github.com/openshift/api/operator/v1alpha1/zz_generated.swagger_doc_generated.go is excluded by !**/vendor/**, !vendor/**, !**/zz_generated*
  • vendor/github.com/openshift/api/security/v1/generated.proto is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/api/security/v1/types.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/api/security/v1/zz_generated.featuregated-crd-manifests.yaml is excluded by !**/vendor/**, !vendor/**, !**/zz_generated*
  • vendor/github.com/openshift/api/security/v1/zz_generated.swagger_doc_generated.go is excluded by !**/vendor/**, !vendor/**, !**/zz_generated*
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/awsdnsspec.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/awskmsconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/infrastructurestatus.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/kmsconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/prefixedclaimmapping.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/usernameclaimmapping.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/vaultapproleauthentication.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/vaultauthentication.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/vaultconfigmapreference.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/vaultkmsconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/vaultsecretreference.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/vaulttlsconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/alertmanagercustomconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/clustermonitoringspec.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/containerresource.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/metricsserverconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/monitoringpluginconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/nodeexportercollectorbuddyinfoconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/nodeexportercollectorconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/nodeexportercollectorcpufreqconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/nodeexportercollectorethtoolconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/nodeexportercollectorksmdconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/nodeexportercollectormountstatsconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/nodeexportercollectornetclasscollectconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/nodeexportercollectornetclassconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/nodeexportercollectornetdevconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/nodeexportercollectorprocessesconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/nodeexportercollectorsystemdcollectconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/nodeexportercollectorsystemdconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/nodeexportercollectortcpstatconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/nodeexporterconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/openshiftstatemetricsconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/prometheusconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/prometheusoperatoradmissionwebhookconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/prometheusoperatorconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/telemeterclientconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1alpha1/thanosquerierconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/internal/internal.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/config/applyconfigurations/utils.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/operator/applyconfigurations/internal/internal.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1/awscsidriverconfigspec.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1/bgpmanagedconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1/ingresscontrollertuningoptions.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1/nooverlayconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1/ovnkubernetesconfig.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/library-go/pkg/config/serving/server.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/library-go/pkg/controller/controllercmd/builder.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/library-go/pkg/controller/controllercmd/cmd.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/library-go/pkg/crypto/cert_config.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/library-go/pkg/crypto/keygen.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/library-go/pkg/crypto/options.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/library-go/pkg/operator/certrotation/client_cert_rotation_controller.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/library-go/pkg/operator/certrotation/signer.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/library-go/pkg/operator/certrotation/target.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/library-go/pkg/pki/profile.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/library-go/pkg/pki/provider.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/library-go/pkg/pki/resolve.go is excluded by !**/vendor/**, !vendor/**
  • vendor/github.com/openshift/library-go/pkg/pki/types.go is excluded by !**/vendor/**, !vendor/**
  • vendor/gopkg.in/gcfg.v1/errors.go is excluded by !**/vendor/**, !vendor/**
  • vendor/gopkg.in/gcfg.v1/go1_0.go is excluded by !**/vendor/**, !vendor/**
  • vendor/gopkg.in/gcfg.v1/go1_2.go is excluded by !**/vendor/**, !vendor/**
  • vendor/gopkg.in/gcfg.v1/read.go is excluded by !**/vendor/**, !vendor/**
  • vendor/gopkg.in/gcfg.v1/scanner/scanner.go is excluded by !**/vendor/**, !vendor/**
  • vendor/gopkg.in/gcfg.v1/set.go is excluded by !**/vendor/**, !vendor/**
  • vendor/gopkg.in/warnings.v0/README is excluded by !**/vendor/**, !vendor/**
  • vendor/gopkg.in/warnings.v0/warnings.go is excluded by !**/vendor/**, !vendor/**
  • vendor/modules.txt is excluded by !**/vendor/**, !vendor/**
📒 Files selected for processing (5)
  • go.mod
  • pkg/operator/kube_cloud_config/controller.go
  • pkg/operator/kube_cloud_config/controller_test.go
  • pkg/operator/starter.go
  • pkg/util/feature_gate.go
✅ Files skipped from review due to trivial changes (2)
  • pkg/util/feature_gate.go
  • go.mod

Comment thread pkg/operator/kube_cloud_config/controller_test.go
t.Fatal(err.Error())
}

fakeConfig := configfakeclient.NewSimpleClientset(inputInfra)
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 1, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify constructor availability/deprecation in vendored openshift config fake client.
rg -n "func NewClientset|func NewSimpleClientset|deprecated" \
  vendor/github.com/openshift/client-go/config/clientset/versioned/fake -C2

# Verify current usage in this test file.
rg -n "configfakeclient.NewSimpleClientset|configfakeclient.NewClientset" \
  pkg/operator/kube_cloud_config/controller_test.go -C2

Repository: openshift/cluster-config-operator

Length of output: 1930

🏁 Script executed:

# Check the full function definitions and any deprecation markers
cat -n vendor/github.com/openshift/client-go/config/clientset/versioned/fake/clientset_generated.go | sed -n '25,100p'

Repository: openshift/cluster-config-operator

Length of output: 3205

🏁 Script executed:

# Find all usages of NewSimpleClientset in the repository
rg -n "NewSimpleClientset|NewClientset" pkg/operator/kube_cloud_config/controller_test.go

Repository: openshift/cluster-config-operator

Length of output: 296

🏁 Script executed:

# Check the context around line 369 in controller_test.go
sed -n '365,375p' pkg/operator/kube_cloud_config/controller_test.go

Repository: openshift/cluster-config-operator

Length of output: 516

🏁 Script executed:

# Check context around line 218
sed -n '214,224p' pkg/operator/kube_cloud_config/controller_test.go

Repository: openshift/cluster-config-operator

Length of output: 576

Replace deprecated configfakeclient.NewSimpleClientset to avoid SA1019 lint failure.

The NewSimpleClientset constructor is explicitly deprecated in the vendored openshift/client-go fake config client. Use NewClientset(inputInfra) instead, which provides improved support for field management and server-side apply testing. Both functions have compatible signatures.

Note: Line 218 in the same file also uses the deprecated constructor and should be updated for consistency.

Suggested change 
-			fakeConfig := configfakeclient.NewSimpleClientset(inputInfra)
+			fakeConfig := configfakeclient.NewClientset(inputInfra)
🧰 Tools
🪛 golangci-lint (2.11.4)

[error] 369-369: SA1019: configfakeclient.NewSimpleClientset is deprecated: NewClientset replaces this with support for field management, which significantly improves server side apply testing. NewClientset is only available when apply configurations are generated (e.g. via --with-applyconfig).

(staticcheck)

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@pkg/operator/kube_cloud_config/controller_test.go` at line 369, Replace uses
of the deprecated constructor configfakeclient.NewSimpleClientset with
configfakeclient.NewClientset to avoid SA1019; update both occurrences (the
fakeConfig assignment creating fakeConfig and the earlier usage around line 218)
so they call NewClientset(inputInfra) with the same arguments, keeping the
fakeConfig variable and test setup unchanged except for the constructor name.

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented May 1, 2026

@vr4manta: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-upgrade b52acc2 link true /test e2e-upgrade
ci/prow/e2e-aws-ovn b52acc2 link true /test e2e-aws-ovn
ci/prow/e2e-aws-ovn-techpreview b52acc2 link true /test e2e-aws-ovn-techpreview
ci/prow/e2e-aws-serial-1of2 b52acc2 link true /test e2e-aws-serial-1of2
ci/prow/e2e-aws-serial-techpreview-1of2 b52acc2 link true /test e2e-aws-serial-techpreview-1of2
ci/prow/e2e-aws-serial-techpreview-2of2 b52acc2 link true /test e2e-aws-serial-techpreview-2of2
ci/prow/e2e-aws-serial-2of2 b52acc2 link true /test e2e-aws-serial-2of2

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

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

Reviewers

@JoelSpeed JoelSpeed JoelSpeed left review comments

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

@jstuever jstuever Awaiting requested review from jstuever

Assignees

No one assigned

Labels

jira/valid-reference Indicates that this PR references a valid Jira ticket of any type.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@vr4manta @openshift-ci-robot @JoelSpeed