Skip to content
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.

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

flag redefined: kubeconfig: tf-operator.v1 and universal operator can not work together #1316

Closed
Jeffwan opened this issue Jul 31, 2021 · 4 comments
Assignees
Labels

Comments

@Jeffwan
Copy link
Member

Jeffwan commented Jul 31, 2021

This is an issue found in #1315 tests. part of #1299

/opt/tf-operator.v1 flag redefined: kubeconfig
panic: /opt/tf-operator.v1 flag redefined: kubeconfig  -> 

goroutine 1 [running]:
flag.(*FlagSet).Var(0xc0000ca120, 0x1bd0d40, 0xc0001ce540, 0x195df71, 0xa, 0x197271c, 0x1b)
	/usr/local/go/src/flag/flag.go:848 +0x4ae
flag.(*FlagSet).StringVar(...)
	/usr/local/go/src/flag/flag.go:751
github.com/kubeflow/tf-operator/cmd/tf-operator.v1/app/options.(*ServerOption).AddFlags(0xc0001ce540, 0xc0000ca120)
	/go/src/github.com/kubeflow/tf-operator/cmd/tf-operator.v1/app/options/options.go:54 +0x8b
main.main()
	/go/src/github.com/kubeflow/tf-operator/cmd/tf-operator.v1/main.go:54 +0x50

Our goal is to have original tf operator and new universal operator working together. However, Addflag is called twice and report the error. See more details in #1315 (comment)

Seem there's no good way to workaround it. We can temporarily comment that line and ask user to use KUBECONFIG env instead

Jeffwan added a commit to Jeffwan/tf-operator that referenced this issue Jul 31, 2021
CI reports `flag redefined: kubeconfig` issue and this is due to duplicate flag registration. See kubeflow#1316 for more details.

Signed-off-by: Jiaxin Shan <[email protected]>
@Jeffwan Jeffwan self-assigned this Jul 31, 2021
google-oss-robot pushed a commit that referenced this issue Aug 2, 2021
* Use low-level controller and handlers in SetupWithManager

Signed-off-by: Jiaxin Shan <[email protected]>

* Add job validation in reconcile loop

Signed-off-by: Jiaxin Shan <[email protected]>

* Set defaults in onOwnerCreateFunc

Signed-off-by: Jiaxin Shan <[email protected]>

* Correctly update job status in apiserver

Signed-off-by: Jiaxin Shan <[email protected]>

* Remove Flag for Kubeconfig to fix flag redefined

CI reports `flag redefined: kubeconfig` issue and this is due to duplicate flag registration. See #1316 for more details.

Signed-off-by: Jiaxin Shan <[email protected]>

* Fix tensorflow job missing default port issue

This is to fix original controller issue. It leverages init to register default to scheme. In our unversal operator project, we did clean up for register.go and break the case.

For more details, please check #1317 (comment)

Signed-off-by: Jiaxin Shan <[email protected]>
@Jeffwan
Copy link
Member Author

Jeffwan commented Aug 2, 2021

I comment out //fs.StringVar(&s.Kubeconfig, "kubeconfig", "", "The path of kubeconfig file") at this moment.
If user like to use external kubeconfig, they have to use KUEBCONFIG at this moment. This is not a big problem as I think (most user still use in cluster config)

I will leave this open if there's an elegant way to claim it.

Jeffwan added a commit that referenced this issue Aug 5, 2021
* Use low-level controller and handlers in SetupWithManager

Signed-off-by: Jiaxin Shan <[email protected]>

* Add job validation in reconcile loop

Signed-off-by: Jiaxin Shan <[email protected]>

* Set defaults in onOwnerCreateFunc

Signed-off-by: Jiaxin Shan <[email protected]>

* Correctly update job status in apiserver

Signed-off-by: Jiaxin Shan <[email protected]>

* Remove Flag for Kubeconfig to fix flag redefined

CI reports `flag redefined: kubeconfig` issue and this is due to duplicate flag registration. See #1316 for more details.

Signed-off-by: Jiaxin Shan <[email protected]>

* Fix tensorflow job missing default port issue

This is to fix original controller issue. It leverages init to register default to scheme. In our unversal operator project, we did clean up for register.go and break the case.

For more details, please check #1317 (comment)

Signed-off-by: Jiaxin Shan <[email protected]>
@Jeffwan
Copy link
Member Author

Jeffwan commented Aug 6, 2021

This is not something we are worried about any more. We plan to cut v1.1-branch for old tf-operator. We don't necessarily need to support compatibility between two version in same branch.

@Jeffwan
Copy link
Member Author

Jeffwan commented Aug 6, 2021

/close

@google-oss-robot
Copy link

@Jeffwan: Closing this issue.

In response to this:

/close

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants