flag redefined: kubeconfig: allow double vendoring this library but still register flags on behalf of users

[akoserwal]

Error:

 /var/folders/3d/_kgky9bj61g5144z004y51qr0000gp/T/go-build652443801/b001/e2e.test flag redefined: kubeconfig
panic: /var/folders/3d/_kgky9bj61g5144z004y51qr0000gp/T/go-build652443801/b001/e2e.test flag redefined: kubeconfig

goroutine 1 [running]:
flag.(*FlagSet).Var(0xc0000d2120, 0x244c6a0, 0xc00004e250, 0x224908b, 0xa, 0x225fced, 0x12)
    /usr/local/Cellar/go/1.13.8/libexec/src/flag/flag.go:848 +0x4ae
flag.(*FlagSet).StringVar(...)
    /usr/local/Cellar/go/1.13.8/libexec/src/flag/flag.go:751
github.com/operator-framework/operator-sdk/pkg/test.(*frameworkOpts).addToFlagSet(0xc00004e240, 0xc0000d2120)
    /Users/akoserwa/go/pkg/mod/github.com/operator-framework/operator-sdk@v0.15.1/pkg/test/framework.go:102 +0x1bf
github.com/operator-framework/operator-sdk/pkg/test.MainEntry(0xc00040d280)
    /Users/akoserwa/go/pkg/mod/github.com/operator-framework/operator-sdk@v0.15.1/pkg/test/main_entry.go:27 +0x50
github.com/atlasmap/atlasmap-operator/test/e2e.TestMain(...)
       _testmain.go:42 +0x136

Using :-

framework “github.com/operator-framework/operator-sdk/pkg/test” where it is setting
> flagset.StringVar(&opts.kubeconfigPath, KubeConfigFlag, “”, “path to kubeconfig”)

And in my test case, I am using a library which is also using controller-runtime/operator-sdk
github.com/RHsyseng/operator-utils/pkg/utils/openshift

 Method I am calling: openshift.IsOpenShift(framework.KubeConfig)

controller-runtime/pkg/client/config/config.go

// TODO: Fix this to allow double vendoring this library but still register flags on behalf of users
  flag.StringVar(&kubeconfig, “kubeconfig”, “”,
    “Paths to a kubeconfig. Only required if out-of-cluster.“)

I suspect fixing TODO will resolve it? If yes, I would like to work on it.

[mengqiy]

/cc @DirectXMan12

[DirectXMan12]

So, due to some legacy cruft, controller-runtime expects to be the thing that registers that flag. We probably shouldn't but that's a pretty serious breaking change that I don't think we want to make just yet.

cc @shawn-hurley @camilamacedo86 @joelanford @estroz any of you have insight into what the operatorsdk package is doing that it's re-registering that flag?

[DirectXMan12]

/assign @camilamacedo86

to answer or triage elsewhere.

[estroz]

The SDK's test framework uses some client-go APIs directly and requires the kubeconfig's namespace, so it needs the kubeconfig path. While the framework could/should be refactored to use pure controller-runtime, which would allow the framework to delegate registering this flag to controller-runtime, we can use a local flagset in the mean time retrieve the value using a lookup.

PR: operator-framework/operator-sdk#2731

@akoserwal can you test my branch out to make sure it fixes your issue?

[akoserwal]

@estroz: Getting error:

flag provided but not defined: -singleNamespace
Usage of /var/folders/3d/_kgky9bj61g5144z004y51qr0000gp/T/go-build683683410/b001/e2e.test:
  -globalMan string
        path to operator manifest
  -kubeconfig string
        Paths to a kubeconfig. Only required if out-of-cluster.
  -localOperator
        enable if operator is running locally (not in cluster)
  -localOperatorArgs string
        flags that the operator needs (while using --up-local). example: "--flag1 value1 --flag2=value2"
  -master --kubeconfig

private variable:
singleNamespaceMode bool

[camilamacedo86]

Hi @estroz,

Since you are working on it already. /assign @estroz

[estroz]

@akoserwal try out SDK's master.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[lrascao]

For future reference, this is the lookup workaround mentioned by @estroz :

kubeconfig := flag.Lookup("kubeconfig").Value.String()

[mvleandro]

For future reference, this is the lookup workaround mentioned by @estroz :

kubeconfig := flag.Lookup("kubeconfig").Value.String()

Thank you!

[stevekuznetsov]

/reopen

It's been a bit. This is still super annoying for basically anyone who happens to import any part of controller-runtime. Mutating global state in an init() in a library is fairly unpleasant - made worse by the fact that Go will only complain the second time the flag is registered, and because of init() ordering, controller-runtime is always the first one, so finding what just side-effect registered --kubeconfig is impossible.

I'd really suggest another look at this. If this were defined in some runtime code or in code that's unlikely to be imported unless by someone actually using controller-runtime, perhaps it would be less frustrating. In my case, I imported an API module that happened to use the controller-runtime Scheme builder. Really divorced from any runtime considerations.

A workaround for now is to re-set flag.CommandLine in my own init().

cc @joelanford @vincepri @alvaroaleman

[k8s-ci-robot]

@stevekuznetsov: Reopened this issue.

In response to this:

/reopen

It's been a bit. This is still super annoying for basically anyone who happens to import any part of controller-runtime. Mutating global state in an init() in a library is fairly unpleasant - made worse by the fact that Go will only complain the second time the flag is registered, and because of init() ordering, controller-runtime is always the first one, so finding what just side-effect registered --kubeconfig is impossible.

I'd really suggest another look at this. If this were defined in some runtime code or in code that's unlikely to be imported unless by someone actually using controller-runtime, perhaps it would be less frustrating. In my case, I imported an API module that happened to use the controller-runtime Scheme builder. Really divorced from any runtime considerations.

A workaround for now is to re-set flag.CommandLine in my own init().

cc @joelanford @vincepri @alvaroaleman

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.

[k8s-triage-robot]

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

This bot triages un-triaged 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:

• Mark this issue as fresh with /remove-lifecycle rotten
• Close this issue 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.

[joelanford]

/label lifecycle/frozen

[k8s-ci-robot]

@joelanford: The label(s) /label lifecycle/frozen cannot be applied. These labels are supported: api-review, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash, team/katacoda, refactor. Is this label configured under labels -> additional_labels or labels -> restricted_labels in plugin.yaml?

In response to this:

/label lifecycle/frozen

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.