-
Notifications
You must be signed in to change notification settings - Fork 763
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: Add ability to configure loglevel for each ArgoCD component #371
feat: Add ability to configure loglevel for each ArgoCD component #371
Conversation
b293b70
to
4a8b9c3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks good. Just a few questions. Thanks.
@wtam2018 Updated based on your requests. But I do see that it appears that the controller and server pods are entering |
Any chance we can add this set loglevel feature to applicationset-controller as well? :-) |
@wtam2018 Think I've got everything fixed up now. I've also included the loglevel for applicationset. Just waiting on the tests to pass now. |
@wtam2018 Looks like all is good here. Are there any other changes you wanted to see? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for updating. It looks good. Have a few minor suggestions. (note: indent is a bit off)
Co-authored-by: William Tam <wtam@redhat.com>
Co-authored-by: William Tam <wtam@redhat.com>
Co-authored-by: William Tam <wtam@redhat.com>
Co-authored-by: William Tam <wtam@redhat.com>
Co-authored-by: William Tam <wtam@redhat.com>
@wtam2018 Thanks for the recommendations. Got all of those merged in and looks like tests have passed. Let me know if this is good to go. |
@@ -42,6 +42,11 @@ spec: | |||
image: | |||
description: Image is the Argo CD ApplicationSet image (optional) | |||
type: string | |||
logLevel: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please re-generate CRD.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Co-authored-by: William Tam <wtam@redhat.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
thanks @tylerauerbeck !
What type of PR is this?
What does this PR do / why we need it:
This PR adds the ability to adjust the loglevel of each of the ArgoCD components (controller, repo, server)
Have you updated the necessary documentation?
Which issue(s) this PR fixes:
Fixes #329
How to test changes / Special notes to the reviewer:
After building the operator from this PR branch, you can deploy it with the following to confirm the loglevel functionality:
If you don't set any of the above, it will default the loglevel to info (which is the current behavior of each of the components).