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

KIC 2.0: handle stderrthreshold flag #1297

Closed
rainest opened this issue May 10, 2021 · 2 comments · Fixed by #1512
Closed

KIC 2.0: handle stderrthreshold flag #1297

rainest opened this issue May 10, 2021 · 2 comments · Fixed by #1512
Assignees
Labels
bug Something isn't working priority/low size/small

Comments

@rainest
Copy link
Contributor

rainest commented May 10, 2021

Follow up from #1294 (review)

Although the controller's logger doesn't make use of the --stderrthreshold flag, its client-go instance uses klog and does. However, Cobra doesn't play nicely with it; we need to figure out how to make it handle the klog flags properly.

@rainest
Copy link
Contributor Author

rainest commented May 18, 2021

Hmm, so maybe not as easy as I thought. 852a8f5 is based off a combination of
https://flowerinthenight.com/blog/2019/02/05/golang-cobra-klog
https://pkg.go.dev/github.com/spf13/pflag#readme-supporting-go-flags-when-using-pflag
https://pkg.go.dev/k8s.io/klog#readme-how-to-use-klog

But this results in a conflict:

$ ./bin/manager --help
 flag redefined: kubeconfig
panic:  flag redefined: kubeconfig

Removing our kubeconfig definition still overwrites Cobra's generated help with the klog flag set's help, and I'm not sure how we pull out the kubeconfig flag for use with controller-runtime init :(

@shaneutt
Copy link
Member

shaneutt commented Jul 8, 2021

After reading through this and given the complexities and cost-benefit considering that we've moved away from the previous logging mechanisms that employed this, I have opted to deprecate the flag but make it available for compatibility reasons in 2.0 #1512

@rainest rainest closed this as completed Jul 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority/low size/small
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants