argo-helm icon indicating copy to clipboard operation
argo-helm copied to clipboard

feat(argo-cd): Add argocd-cmd-params-cm

Open pdrastil opened this issue 2 years ago • 5 comments

This PR should start discussion regarding https://github.com/argoproj/argo-helm/issues/1210

Changes:

  • Added argocd-cmd-params-cm configmap
  • Exposed all known config options via environment variables
  • Rewire internal parameters for Redis / Dex
  • Added deprecation notice for arguments that were superseded by new config

Resolves:

  • https://github.com/argoproj/argo-helm/issues/1210

Checklist:

  • [x] I have bumped the chart version according to versioning
  • [x] I have updated the documentation according to documentation
  • [x] I have updated the chart changelog with all the changes that come with this pull request according to changelog.
  • [x] Any new values are backwards compatible and/or have sensible default.
  • [x] I have signed off all my commits as required by DCO.
  • [x] My build is green (troubleshooting builds).

Changes are automatically published when merged to master. They are not published on branches.

pdrastil avatar May 07 '22 13:05 pdrastil

@mkilchhofer Please take a look on another iteration and tell me WDYT. When you wanted to go with breaking change I've tried to consolidate parameters such as tlsCerts and tlsCertsAnnotations under one config section. Things I haven't visited yet:

  • credentials / clusters / templates - Do we want to have special config section for these? What to do with legacy configuration option that is deprecated? remove this one?
  • notifications - it have configuration in argocd-notifications-cm in upstream repo - maybe it should be also refactored and added to configs
  • configs.cm.url - could we use this for notification context and maybe ingress host as a default?

pdrastil avatar May 21 '22 20:05 pdrastil

Asked the community in the slack channel for their thoughts about this refactoring: https://cloud-native.slack.com/archives/C02097VUKUM/p1654880501143779

grafik

mkilchhofer avatar Jun 10 '22 17:06 mkilchhofer

Hi everyone, is someone taking a look at adding this config argocd-cmd-params-cm.yaml to repo-server? We are getting ready to leverage argocd in production and want to ensure we can change the reposerver.parallelism.limit value.

zestrells avatar Aug 03 '22 16:08 zestrells

Hi everyone, is someone taking a look at adding this config argocd-cmd-params-cm.yaml to repo-server? We are getting ready to leverage argocd in production and want to ensure we can change the reposerver.parallelism.limit value.

@zestrells I'm currently pulling backward comapatible changes from this PR and trying to push it in smaller pieces.

pdrastil avatar Aug 14 '22 07:08 pdrastil

@pdrastil #1320 has been merged. controller.args.appHardResyncPeriod can be added as deprecated to this PR. Thank you.

jmeridth avatar Aug 26 '22 16:08 jmeridth

@mkilchhofer any update on this?

pdrastil avatar Sep 20 '22 17:09 pdrastil