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: Add ability to configure loglevel for each ArgoCD component #371

Merged
merged 17 commits into from
Jul 28, 2021

Conversation

tylerauerbeck
Copy link
Contributor

What type of PR is this?

/kind enhancement

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?

  • Documentation update is required by this PR.
  • Documentation has been updated.

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:

apiVersion: argoproj.io/v1alpha1
kind: ArgoCD
metadata:
  name: example-argocd
  labels:
    example: basic
spec: 
  server:
    logLevel: "warn"
  repo:
    logLevel: "error"
  controller:
    logLevel: "info"

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

@CLAassistant
Copy link

CLAassistant commented Jul 15, 2021

CLA assistant check
All committers have signed the CLA.

@tylerauerbeck tylerauerbeck changed the title Add ability to configure loglevel for each ArgoCD component feat: Add ability to configure loglevel for each ArgoCD component Jul 15, 2021
Copy link
Collaborator

@wtam2018 wtam2018 left a 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.

docs/reference/argocd.md Show resolved Hide resolved
pkg/apis/argoproj/v1alpha1/argocd_types.go Outdated Show resolved Hide resolved
pkg/controller/argocd/deployment.go Outdated Show resolved Hide resolved
pkg/controller/argocd/util.go Outdated Show resolved Hide resolved
@tylerauerbeck
Copy link
Contributor Author

@wtam2018 Updated based on your requests. But I do see that it appears that the controller and server pods are entering CrashLoopBackOff. Going to take a look at this now.

@wtam2018
Copy link
Collaborator

Any chance we can add this set loglevel feature to applicationset-controller as well? :-)

@tylerauerbeck tylerauerbeck changed the title feat: Add ability to configure loglevel for each ArgoCD component WIP: feat: Add ability to configure loglevel for each ArgoCD component Jul 16, 2021
@tylerauerbeck tylerauerbeck changed the title WIP: feat: Add ability to configure loglevel for each ArgoCD component feat: Add ability to configure loglevel for each ArgoCD component Jul 16, 2021
@tylerauerbeck
Copy link
Contributor Author

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

@tylerauerbeck
Copy link
Contributor Author

@wtam2018 Looks like all is good here. Are there any other changes you wanted to see?

Copy link
Collaborator

@wtam2018 wtam2018 left a 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)

docs/reference/argocd.md Outdated Show resolved Hide resolved
docs/reference/argocd.md Outdated Show resolved Hide resolved
docs/reference/argocd.md Outdated Show resolved Hide resolved
pkg/apis/argoproj/v1alpha1/argocd_types.go Outdated Show resolved Hide resolved
pkg/controller/argocd/util.go Outdated Show resolved Hide resolved
tylerauerbeck and others added 5 commits July 27, 2021 11:09
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>
@tylerauerbeck
Copy link
Contributor Author

@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:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please re-generate CRD.

Copy link
Contributor Author

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>
Copy link
Collaborator

@wtam2018 wtam2018 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
thanks @tylerauerbeck !

@wtam2018 wtam2018 merged commit c5a3e0d into argoproj-labs:master Jul 28, 2021
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.

Setting loglevel for components
3 participants