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

feat(log-level): add option to set logLevel in chart #687

Merged
merged 4 commits into from
Oct 22, 2024

Conversation

dabcoder
Copy link
Contributor

Implements #681.

Copy link

@dabcoder Images are available for testing. docker pull ghcr.io/stakater/reloader:SNAPSHOT-PR-687-9b21bf36\ndocker pull ghcr.io/stakater/reloader:SNAPSHOT-PR-687-UBI-9b21bf36

@dabcoder dabcoder marked this pull request as ready for review June 13, 2024 16:24
@dabcoder
Copy link
Contributor Author

dabcoder commented Jun 13, 2024

Tested with:

spec:
  containers:
    - args:
        - --log-level=error

Yet in the actual container logs I can see:

time="2024-06-13T12:21:17Z" level=info msg="Environment: Kubernetes"

Feedback welcome.

@dabcoder
Copy link
Contributor Author

cc @MuneebAijaz

@MuneebAijaz
Copy link
Contributor

hi @dabcoder thanks for the PR, if the actual flag doesn't work for log levels, we might need to fix that first.

@dabcoder
Copy link
Contributor Author

Yes that's what I was trying to check and figure out but I don't immediately see any issues with the flag in the codebase. I can take another look anyway.

@martin-gutwin
Copy link

martin-gutwin commented Jul 30, 2024

Tested with:

spec:
  containers:
    - args:
        - --log-level=error

Yet in the actual container logs I can see:

time="2024-06-13T12:21:17Z" level=info msg="Environment: Kubernetes"

Feedback welcome.

Hi,

For me this is the only Log-Message that doesn't consider the options.
I think the problem is, that it is written during initialization of the kube client (client.go), while the log format and log level options for logrus are only set afterwards in the initialisation of the application (app.go).
For all following messages, log level and format are appropriately considered, at least for me.

@dabcoder
Copy link
Contributor Author

@martin-gutwin thanks for the insights, didn't think about that. So I can update this PR to resolve the conflicts then.

Copy link

@dabcoder Images are available for testing. docker pull ghcr.io/stakater/reloader:SNAPSHOT-PR-687-08f16d13\ndocker pull ghcr.io/stakater/reloader:SNAPSHOT-PR-687-UBI-08f16d13

@dabcoder
Copy link
Contributor Author

dabcoder commented Aug 9, 2024

Can this PR be re-reviewed and merged if it satisfy requirements?

@MuneebAijaz
Copy link
Contributor

hi @dabcoder we have to do some changes to chart workflows, till then this sadly cant be merged. I will merge this when workflows are fixed.

@dabcoder
Copy link
Contributor Author

hi @dabcoder we have to do some changes to chart workflows, till then this sadly cant be merged. I will merge this when workflows are fixed.

Any updates on the above? Thanks.

@dabcoder
Copy link
Contributor Author

dabcoder commented Oct 9, 2024

cc @MuneebAijaz

@MuneebAijaz MuneebAijaz merged commit 325515f into stakater:master Oct 22, 2024
4 of 5 checks passed
@dabcoder dabcoder deleted the db/-/log-level branch October 23, 2024 07:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants