8000 kube-controller-manager: Extend --controllers usage string to contain required feature gates by tchap · Pull Request #132684 · kubernetes/kubernetes · GitHub
[go: up one dir, main page]
Skip to content

kube-controller-manager: Extend --controllers usage string to contain required feature gates #132684

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

kube-controller-manager: Extend --controllers usage string to contain required feature gates #132684

wants to merge 1 commit into from
+284 −31

Conversation

tchap
Copy link
Contributor
@tchap tchap commented Jul 2, 2025 •
edited
Loading

What type of PR is this?

/kind feature

What this PR does / why we need it:

Print the feature gates required for each controller.
This makes it easy for the user to see what feature gates need to be enabled for particular controllers.

The updated usage output:

     --controllers strings                                                                                                                                       
                A list of controllers to enable. '*' enables all on-by-default controllers, 'foo' enables the controller named 'foo', '-foo' disables the controller
                named 'foo'.
                
                All controllers: 
                bootstrap-signer-controller
                certificatesigningrequest-approving-controller
                certificatesigningrequest-cleaner-controller
                certificatesigningrequest-signing-controller
                cloud-node-lifecycle-controller
                clusterrole-aggregation-controller
                cronjob-controller
                daemonset-controller
                deployment-controller
                device-taint-eviction-controller [feature gates required: DynamicResourceAllocation, DRADeviceTaints]
                disruption-controller
                endpoints-controller
                endpointslice-controller
                endpointslice-mirroring-controller
                ephemeral-volume-controller
                garbage-collector-controller
                horizontal-pod-autoscaler-controller
                job-controller
                kube-apiserver-serving-clustertrustbundle-publisher-controller [feature gates required: ClusterTrustBundle]
                legacy-serviceaccount-token-cleaner-controller
                namespace-controller
                node-ipam-controller
                node-lifecycle-controller
                node-route-controller
                persistentvolume-attach-detach-controller
                persistentvolume-binder-controller
                persistentvolume-expander-controller
                persistentvolume-protection-controller
                persistentvolumeclaim-protection-controller
                pod-garbage-collector-controller
                replicaset-controller
                replicationcontroller-controller
                resourceclaim-controller [feature gates required: DynamicResourceAllocation]
                resourcequota-controller
                root-ca-certificate-publisher-controller
                selinux-warning-controller [feature gates required: SELinuxChangePolicy]
                service-cidr-controller [feature gates required: MultiCIDRServiceAllocator]
                service-lb-controller
                serviceaccount-controller
                serviceaccount-token-controller
                statefulset-controller
                storage-version-migrator-controller [feature gates required: StorageVersionMigrator, InformerResourceVersion]
                storageversion-garbage-collector-controller [feature gates required: APIServerIdentity, StorageVersionAPI]
                taint-eviction-controller [feature gates required: SeparateTaintEvictionController]
                token-cleaner-controller
                ttl-after-finished-controller
                ttl-controller
                validatingadmissionpolicy-status-controller
                volumeattributesclass-protection-controller [feature gates required: VolumeAttributesClass]
                
                Disabled-by-default controllers: 
                bootstrap-signer-controller
                selinux-warning-controller [feature gates required: SELinuxChangePolicy]
                token-cleaner-controller
                 (default [*])

Which issue(s) this PR is related to:

N/A

Special notes for your reviewer:

  1. Feel free to propose a nicer formatting.
  2. To make sure the output is correct for KCM, I made sure all controller descriptors contain correct information. I had to fix a few controllers, which were checking gates manually instead of using the descriptor.

Does this PR introduce a user-facing change?

Extended `kube-controller-manager` usage help for the `--controllers` command line option, to print the names of associated feature gates.

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

N/A

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jul 2, 2025
@k8s-ci-robot
Copy link
Contributor

Hi @tchap. Thanks for your PR.

I'm waiting for a kubernetes 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.

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.

@k8s-ci-robot k8s-ci-robot added needs-priority Indicates a PR lacks a `priority/foo` label and requires one. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jul 2, 2025
@k8s-ci-robot k8s-ci-robot requested review from humblec and jiahuif July 2, 2025 14:23
tchap
tchap commented Jul 2, 2025
View reviewed changes
cmd/kube-controller-manager/app/core.go
if !utilfeature.DefaultFeatureGate.Enabled(features.SELinuxChangePolicy) {
return nil, false, nil
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The gate is actually already present in the descriptor, so this is redundant.

@tchap
Copy link
Contributor Author
tchap commented Jul 2, 2025

/sig api-machinery

@lmktfy
Copy link
lmktfy commented Jul 2, 2025

Changelog suggestion

-Extended `kube-controller-manager` usage help for `--controllers` flag to print the associated feature gates.
+Extended `kube-controller-manager` usage help for the `--controllers` command line option, to print the names of associated feature gates.

@tchap
Copy link
Contributor Author
tchap commented Jul 2, 2025

Changelog suggestion

-Extended `kube-controller-manager` usage help for `--controllers` flag to print the associated feature gates.
+Extended `kube-controller-manager` usage help for the `--controllers` command line option, to print the names of associated feature gates.

@lmktfy Thanks, amended.

@tchap
Copy link
Contributor Author
tchap commented Jul 3, 2025

Looks like I failed to check some files for whether all the feature gates are in descriptors.

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 3, 2025
@tchap tchap changed the title kube-controller-manager: Extend --controllers usage string to contain required feature gates WIP: kube-controller-manager: Extend --controllers usage string to contain required feature gates Jul 3, 2025
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 3, 2025
@tchap tchap force-pushed the kcm-help-feature-gates branch 2 times, most recently from 79e24f5 to 4f49c45 Compare July 3, 2025 09:31
tchap
tchap commented Jul 3, 2025
View reviewed changes
cmd/kube-controller-manager/app/certificates.go
@@ -244,7 +243,7 @@ func newKubeAPIServerSignerClusterTrustBundledPublisherController(ctx context.Co
return nil, false, err
}

if len(rootCA) == 0 || !utilfeature.DefaultFeatureGate.Enabled(features.ClusterTrustBundle) {
if len(rootCA) == 0 {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The gate is actually already present in the descriptor, so this is redundant.

@tchap tchap changed the title WIP: kube-controller-manager: Extend --controllers usage string to contain required feature gates kube-controller-manager: Extend --controllers usage string to contain required feature gates Jul 3, 2025
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 3, 2025
@tchap
Copy link
Contributor Author
tchap commented Jul 3, 2025

Should be ok now...

/unhold

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 3, 2025
@ardaguclu
Copy link
Member

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jul 3, 2025
@tchap tchap force-pushed the kcm-help-feature-gates branch 3 times, most recently from 2ff28fa to d943c94 Compare July 4, 2025 07:09
@tchap tchap force-pushed the kcm-help-feature-gates branch 2 times, most recently from c1167bd to 5b89122 Compare July 4, 2025 07:50
@tchap
kcm: Extend --controllers flag usage string
dcf39a2 
Print the feature gates required for each controller.
@tchap tchap force-pushed the kcm-help-feature-gates branch from 5b89122 to dcf39a2 Compare July 4, 2025 08:22
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

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

@tchap
Copy link
Contributor Author
tchap commented Jul 4, 2025

/remove-area cloudprovider
/remove-sig cloud-provider

@k8s-ci-robot k8s-ci-robot removed area/cloudprovider sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. labels Jul 4, 2025
@tchap
Copy link
Contributor Author
tchap commented Jul 4, 2025

/retest-required

@jpbetz
Copy link
Contributor
jpbetz commented Jul 8, 2025

/assign @Jefftree
Would you review? Thanks!

/triage accepted

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jul 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@humblec humblec Awaiting requested review from humblec

@jiahuif jiahuif Awaiting requested review from jiahuif

Assignees

@Jefftree Jefftree

Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@tchap @k8s-ci-robot @lmktfy @ardaguclu @jpbetz @Jefftree
0