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

fix: Changing log level/format triggers reconciliation #453

Merged
merged 2 commits into from
Oct 14, 2021
Merged

fix: Changing log level/format triggers reconciliation #453

merged 2 commits into from
Oct 14, 2021

Conversation

reginapizza
Copy link
Collaborator

What type of PR is this?
/kind bug

What does this PR do / why we need it:
When changing the log level or log format, the operator should change the deployment spec even if the argocd-repo-server deployment resource already exists, currently that is not happening.

Have you updated the necessary documentation?

N/A

Which issue(s) this PR fixes:

Fixes #448

How to test changes:

  1. Inspect argocd-repo-server deployment's container command line: kubectl get deployment argocd-repo-server -o jsonpath='{.spec.template.spec.containers[0].command}'. Observe --loglevel info and --logformat text command line parameters.
  2. Patch the ArgoCD Operand to change log level and format for the repository server: kubectl patch argocd argocd --type=merge --patch '{"spec": {"repo": {"logFormat": "json", "logLevel": "debug"}}}'
  3. Inspect argocd-repo-server deployment's container command line again: kubectl get deployment argocd-repo-server -o jsonpath='{.spec.template.spec.containers[0].command}'. Observe command line parameters have been changed from step 1.

cc: @jannfis @jaideepr97

@CLAassistant
Copy link

CLAassistant commented Oct 1, 2021

CLA assistant check
All committers have signed the CLA.

@wtam2018
Copy link
Collaborator

wtam2018 commented Oct 1, 2021

@reginapizza Thanks for the PR. Any chance you can add a unit test?

@jannfis
Copy link
Collaborator

jannfis commented Oct 1, 2021

I think this is hard to be done in a unit test. This should be an e2e test, imho.

@wtam2018
Copy link
Collaborator

wtam2018 commented Oct 5, 2021

@reginapizza the test won't compile.

# github.com/argoproj-labs/argocd-operator/controllers/argocd [github.com/argoproj-labs/argocd-operator/controllers/argocd.test]
controllers/argocd/deployment_test.go:273:18: undefined: repoServerDefaultCommand

@reginapizza
Copy link
Collaborator Author

@wtam2018 could you please re-review? Thanks!

@jaideepr97
Copy link
Collaborator

@reginapizza
Looks like you might need to rebase

@iam-veeramalla iam-veeramalla changed the title Changing log level/format triggers reconciliation fix: Changing log level/format triggers reconciliation Oct 14, 2021
@iam-veeramalla
Copy link
Collaborator

LGTM !! Thanks @reginapizza for your first contribution ☑️. Looking forward for many more.

@iam-veeramalla iam-veeramalla merged commit 711ee66 into argoproj-labs:master Oct 14, 2021
@iam-veeramalla iam-veeramalla self-assigned this Oct 14, 2021
jopit pushed a commit to jopit/argocd-operator that referenced this pull request Nov 8, 2021
…#453)

* Changing log level/format triggers reconciliation

* add test case for changing log format/level
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.

Changing log level or log format for repo server doesn't trigger reconciliation
6 participants