Adopt NodeLifecycleControllerOptions for k8s.io/cloud-provider v0.37.0 - #10777
Adopt NodeLifecycleControllerOptions for k8s.io/cloud-provider v0.37.0#10777danbruno101 wants to merge 1 commit into
Conversation
kubernetes/kubernetes#137964 moved NodeMonitorPeriod from KubeCloudSharedConfiguration to a new NodeLifecycleControllerConfiguration and added a concurrentNodeLifecycleSyncs parameter to NewCloudNodeLifecycleController. Because this repo maintains its own CloudControllerManagerOptions, the new NodeLifecycleControllerOptions group must be wired into the struct, constructor, Flags, ApplyTo, and Validate; otherwise --node-monitor-period would no longer be registered and the controller would receive zero-valued config. No behavioral change at default flag values. Staged to land with the v0.37.0 dependency bump. Signed-off-by: Daniel Bruno <daniel.c.bruno@gmail.com>
|
Welcome @danbruno101! |
|
Hi @danbruno101. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: danbruno101 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/kind bug |
|
@danbruno101: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. DetailsInstructions 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. |
There was a problem hiding this comment.
Pull request overview
Prepares cloud-provider-azure’s cloud-controller-manager wiring for the upcoming k8s.io/cloud-provider v0.37.0 API changes by introducing NodeLifecycleControllerOptions usage and switching the cloud node lifecycle controller startup path to read NodeMonitorPeriod from the new NodeLifecycleController config (and pass the new concurrency parameter).
Changes:
- Add
NodeLifecycleControllerOptionsto CCM options, including flags/apply/validate wiring. - Update CCM node lifecycle controller startup to use
ComponentConfig.NodeLifecycleControllerand passConcurrentNodeLifecycleSyncs. - Update options tests to reflect the config move of
NodeMonitorPeriodout ofKubeCloudSharedConfiguration.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| cmd/cloud-controller-manager/app/options/options.go | Adds NodeLifecycleControllerOptions into CCM option construction, flag sets, apply, and validation. |
| cmd/cloud-controller-manager/app/options/options_test.go | Updates expected option structures for NodeMonitorPeriod relocation and new node lifecycle controller config fields. |
| cmd/cloud-controller-manager/app/core.go | Switches node lifecycle controller startup to the new component config location and passes the new concurrency argument. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
What type of PR is this?
/kind bug
What this PR does / why we need it:
Prepares this repo for the k8s.io/* v0.37.0 dependency bump. kubernetes/kubernetes#137964
moved NodeMonitorPeriod from KubeCloudSharedConfiguration to a new
NodeLifecycleControllerConfiguration and added a concurrentNodeLifecycleSyncs parameter
to NewCloudNodeLifecycleController. This updates both call sites: the standard CCM node
lifecycle controller starter and the GKE tenant controller manager. Compile fix only;
no behavioral change at default flag values.
Which issue(s) this PR fixes:
Fixes #1254
Special notes for your reviewer:
CI is expected to fail until the dependency bump to k8s.io/* v0.37.0 lands — this change
is source-compatible with v0.37.0 only. Note the staging pins live in the replace blocks
of both go.mod and go.work; this PR intentionally touches neither and contains only the
two source fixes, to compose with the bump PR. Verified against
k8s.io/cloud-provider@master (
go build ./cmd/...passes with the vendor tree synced).Opened per elmiko's suggestion so the provider team can sequence it with the bump.
Happy to rebase or fold into the bump PR as preferred.
Does this PR introduce a user-facing change?