search

kubernetes / cloud-provider

cloud-provider defines the shared interfaces which Kubernetes cloud providers implement. These interfaces allow various controllers to integrate with any cloud provider in a pluggable fashion. Also serves as an issue tracker for SIG Cloud Provider.
Apache License 2.0
244 stars 110 forks source link

app.NewCloudControllerManagerCommand additionalFlags not working as expected #62

Closed snebel29 closed 1 year ago

snebel29 commented 2 years ago

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 https://github.com/kubernetes/cloud-provider/blob/9c22fcdc5eab5b9e62f9bdf5dbca07bcbdf6fab5/app/controllermanager.go#L123

The problem seems to be in the the use namedFlagSets which does not contain the additional flags to print he usage and help sections https://github.com/kubernetes/cloud-provider/blob/9c22fcdc5eab5b9e62f9bdf5dbca07bcbdf6fab5/app/controllermanager.go#L130

https://github.com/kubernetes/cloud-provider/blob/9c22fcdc5eab5b9e62f9bdf5dbca07bcbdf6fab5/app/controllermanager.go#L135

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 commented 2 years ago

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 commented 2 years ago

I have created https://github.com/kubernetes/kubernetes/pull/112405 with a fix for this which is awaiting for review, feedback is welcomed.

k8s-triage-robot commented 1 year ago

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:

You can:

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

/lifecycle stale

k8s-triage-robot commented 1 year ago

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:

You can:

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

/lifecycle rotten

k8s-triage-robot commented 1 year ago

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:

You can:

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

/close not-planned

k8s-ci-robot commented 1 year ago

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

In response to [this](https://github.com/kubernetes/cloud-provider/issues/62#issuecomment-1426343131): >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][1] > >Please send feedback to sig-contributor-experience at [kubernetes/community](https://github.com/kubernetes/community). > >/close not-planned > >[1]: https://www.kubernetes.dev/docs/guide/issue-triage/ Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.