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

Bind log configuration flags by default in main.go #1517

Closed
bharathi-tenneti opened this issue May 14, 2020 · 10 comments
Closed

Bind log configuration flags by default in main.go #1517

bharathi-tenneti opened this issue May 14, 2020 · 10 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Milestone

Comments

@bharathi-tenneti
Copy link
Contributor

bharathi-tenneti commented May 14, 2020

Description:
Update the main.go scaffold code to configure log options from command line flags via zap.BindFlags()
Example:

opts := zap.Options{}
opts.BindFlags(flag.CommandLine)
flag.Parse()
logger := zap.New(zap.UseFlagOptions(&opts))
log.SetLogger(logger)

This way, the log options can be configured at runtime via flags in the manager’s deployment manifest e.g:

  spec:
    containers:
    - command:
      - /manager
      args:
      - --enable-leader-election
      - --zap-devel=true
      - --zap-log-level=debug

/kind feature

@bharathi-tenneti bharathi-tenneti added the kind/feature Categorizes issue or PR as related to a new feature. label May 14, 2020
@bharathi-tenneti
Copy link
Contributor Author

/cc @hasbro17 @mengqiy

@bharathi-tenneti
Copy link
Contributor Author

Can we have some inputs on this issue?

@camilamacedo86
Copy link
Member

It would be to apply the changes made in kubernetes-sigs/controller-runtime#915 and kubernetes-sigs/controller-runtime#1035

@camilamacedo86
Copy link
Member

It shows a follow up of : #1721

@georgettica would you like to help on this one? WDYT?

@georgettica
Copy link
Contributor

I can and I will,
so the issue as I see it @bharathi-tenneti is that you want a slightly more configurable template..

as of v3 and my PR, you can get these configurations but the only difference is that the debug level is forced through the code and not the config.

do you think moving all of the configs outside of the main.go is the better way to go? I have no clear agenda related to that.

if you want help changing the code inside the template I can gladly assist / create the PR to make it a reality

@camilamacedo86 camilamacedo86 added this to the v3+ plugin milestone Nov 17, 2020
@estroz
Copy link
Contributor

estroz commented Nov 17, 2020

Those flags aren't actually needed since Development: true is set by default. If anyone thinks scaffolding flags in manager.yaml is better and has a reasonable argument for doing so over the current default setup, please comment and we can re-consider this issue.

/close

@k8s-ci-robot
Copy link
Contributor

@estroz: Closing this issue.

In response to this:

Those flags aren't actually needed since Development: true is set by default. If anyone thinks scaffolding flags in manager.yaml is better and has a reasonable argument for doing so over the current default setup, please comment and we can re-consider this issue.

/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.

@camilamacedo86
Copy link
Member

+1 to close this one. However, if more community members think that would be better we scaffold the flags instead than, please feel free to raise an issue with this as RFE for we are able to address.

@nathanperkins
Copy link

Is there a way to set log levels via command-line flag with the scaffolded code?

It looks like we're having to change the code to support this.

@estroz
Copy link
Contributor

estroz commented Apr 30, 2021

You're looking for --zap-log-level.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

6 participants