app.NewCloudControllerManagerCommand additionalFlags not working as expected

[snebel29]

Hi,
Poking around with app.NewCloudControllerManagerCommand I noticed additionalFlags argument does not seems to be working as I would expect, while the flag is parsed it is not show in the usage or help.

When an additional flagSet is passed it is parsed, and can be used however it is not shown in the usage or help of the command.

To reproduce, use the following snippet of code:

package main

import (
	"os"

	"k8s.io/apimachinery/pkg/util/wait"
	cloudprovider "k8s.io/cloud-provider"
	"k8s.io/cloud-provider/app"
	"k8s.io/cloud-provider/app/config"
	"k8s.io/cloud-provider/fake"
	"k8s.io/cloud-provider/options"
	kflag "k8s.io/component-base/cli/flag"
	klog "k8s.io/klog/v2"
)

var myFlag string

func main() {

	additionalFlags := kflag.NamedFlagSets{}
	additionalFlags.FlagSet("myFlagSet").StringVar(&myFlag, "abcd", "defaultValue", "n/a")

	opts, err := options.NewCloudControllerManagerOptions()
	if err != nil {
		klog.Fatalf("Failed to create new cloud controller manager options: %v", err)
	}
	opts.KubeCloudShared.CloudProvider.Name = "myProvider"
	home, _ := os.LookupEnv("HOME")
	opts.Kubeconfig = home + "/.kube/config"

	command := app.NewCloudControllerManagerCommand(
		opts,
		cloudProviderInitializer,
		app.DefaultInitFuncConstructors,
		additionalFlags,
		wait.NeverStop,
	)

	if err := command.Execute(); err != nil {
		klog.Fatalf("Failed to execute cloud controller manager command: %v", err)
	}
}

func cloudProviderInitializer(_ *config.CompletedConfig) cloudprovider.Interface {
	klog.Info("abcd flag value is: ", myFlag)
	return &fake.Cloud{}
}

The flag is not shown in the help with the existing cloud controller manager flags, the following command return no results.

$ go run cmd/test/main.go --help |grep abcd

While the flag has been really bound and parsed and can In fact be used.

$  go run cmd/test/main.go --abcd 123456             
I0602 19:23:40.055119   44145 serving.go:348] Generated self-signed cert in-memory
W0602 19:23:40.376430   44145 authentication.go:317] No authentication-kubeconfig provided in order to lookup client-ca-file in configmap/extension-apiserver-authentication in kube-system, so client certificate authentication won't work.
W0602 19:23:40.376466   44145 authentication.go:341] No authentication-kubeconfig provided in order to lookup requestheader-client-ca-file in configmap/extension-apiserver-authentication in kube-system, so request-header client certificate authentication won't work.
W0602 19:23:40.376521   44145 authorization.go:193] No authorization-kubeconfig provided, so SubjectAccessReview of authorization tokens won't work.
I0602 19:23:40.381690   44145 main.go:45] abcd flag value is: 123456

It does feel either a bug or an unimplemented feature, while the additional flags are successfully bound in

cloud-provider/app/controllermanager.go

Line 123 in 9c22fcd

fs.AddFlagSet(f)

The problem seems to be in the the use namedFlagSets which does not contain the additional flags to print he usage and help sections

cloud-provider/app/controllermanager.go

Line 130 in 9c22fcd

cliflag.PrintSections(cmd.OutOrStderr(), namedFlagSets, cols)

cloud-provider/app/controllermanager.go

Line 135 in 9c22fcd

cliflag.PrintSections(cmd.OutOrStdout(), namedFlagSets, cols)

I wouldn't mind having a closer look and propose a fix if one would be accepted, I just wanted to share and get feedback first to ensure this make sense.

Cheers.

[snebel29]

Hi, If I spend my time looking to propose a fix to this issue, and assuming the code is good, would it be reviewed and merged?

Thank in advance.

[snebel29]

I have created kubernetes/kubernetes#112405 with a fix for this which is awaiting for review, feedback is welcomed.

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Reopen this issue with /reopen
• Mark this issue as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

[k8s-ci-robot]

@k8s-triage-robot: Closing this issue, marking it as "Not Planned".

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Reopen this issue with /reopen
• Mark this issue as fresh with /remove-lifecycle rotten
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close not-planned

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/test-infra repository.