fix(controller): get commit server url from env #23536
Conversation
❌ Preview Environment deleted from BunnyshellAvailable commands (reply to this comment):
|
This commit updates the ArgoCD application controller to retrieve the commit server URL from an environment variable, allowing for more flexible configuration. Signed-off-by: Alexej Disterhoft <alexej.disterhoft@redcare-pharmacy.com>
bc26f04 to
4377e69
Compare
Signed-off-by: Alexej Disterhoft <alexej.disterhoft@redcare-pharmacy.com>
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the ArgoCD Application Controller deployment by sourcing the commit server URL from an environment variable backed by the argocd-cmd-params-cm ConfigMap, making the commit server endpoint configurable.
- Added
ARGOCD_APPLICATION_CONTROLLER_COMMIT_SERVERenv var (fromcommit.serverkey) to controller StatefulSet and Deployment manifests. - Marked the new env var as optional for backward compatibility.
- Applied this change across standard, HA, and hydrator-enabled install manifests.
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| manifests/namespace-install.yaml | Added commit server env var to controller container |
| manifests/namespace-install-with-hydrator.yaml | Added commit server env var to controller container |
| manifests/install.yaml | Added commit server env var to controller container |
| manifests/install-with-hydrator.yaml | Added commit server env var to controller container |
| manifests/ha/namespace-install.yaml | Added commit server env var to controller container |
| manifests/ha/namespace-install-with-hydrator.yaml | Added commit server env var to controller container |
| manifests/ha/install.yaml | Added commit server env var to controller container |
| manifests/ha/install-with-hydrator.yaml | Added commit server env var to controller container |
| manifests/core-install.yaml | Added commit server env var to controller container |
| manifests/core-install-with-hydrator.yaml | Added commit server env var to controller container |
| manifests/base/application-controller/argocd-application-controller-statefulset.yaml | Added commit server env var to controller container |
| manifests/base/application-controller-deployment/argocd-application-controller-deployment.yaml | Added commit server env var to controller container |
Comments suppressed due to low confidence (3)
manifests/namespace-install.yaml:2341
- [nitpick] This env var is added repeatedly across many manifests, which could be hard to maintain. Consider centralizing it with a common patch or using a templating tool (Helm/Kustomize) to reduce duplication.
- name: ARGOCD_APPLICATION_CONTROLLER_COMMIT_SERVER
manifests/namespace-install.yaml:2341
- [nitpick] There is no documentation update for users on how to configure
ARGOCD_APPLICATION_CONTROLLER_COMMIT_SERVER. Please update the relevant docs or README to describe this new parameter and its effect.
- name: ARGOCD_APPLICATION_CONTROLLER_COMMIT_SERVER
manifests/namespace-install.yaml:2341
- No tests have been added to verify that the controller picks up and uses this new env var. Please include unit or e2e tests to cover this behavior.
- name: ARGOCD_APPLICATION_CONTROLLER_COMMIT_SERVER
|
@nobbs thanks for the PR! Out of curiosity, what kind of setup are you going for? |
|
Hey @crenshaw-dev - I mostly was playing around with the hydrator (which I've somehow missed the last months). I've deployed ArgoCD with the Helm chart and noticed errors along the lines of "k8s internal dns can't resolve Eventually, I figured out that the services deployed by the Helm chart are all prefixed with the release name, so the name is something along the lines of I raised a PR on the chart argoproj/argo-helm#3367 and was told to first raise a PR on the upstream manifests :D |
|
/cherry-pick release-3.1 |
|
/cherry-pick release-3.0 |
|
/cherry-pick release-2.14 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #23536 +/- ##
==========================================
+ Coverage 60.22% 60.29% +0.07%
==========================================
Files 345 345
Lines 59084 59084
==========================================
+ Hits 35583 35625 +42
+ Misses 20626 20592 -34
+ Partials 2875 2867 -8 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Alexej Disterhoft <alexej.disterhoft@redcare-pharmacy.com>
Signed-off-by: Alexej Disterhoft <alexej.disterhoft@redcare-pharmacy.com>
Signed-off-by: Alexej Disterhoft <alexej.disterhoft@redcare-pharmacy.com>
Signed-off-by: Alexej Disterhoft <alexej.disterhoft@redcare-pharmacy.com>
Signed-off-by: Alexej Disterhoft <alexej.disterhoft@redcare-pharmacy.com> Signed-off-by: enneitex <etienne.divet@gmail.com>
This commit updates the ArgoCD application controller deployment to retrieve the commit server URL from an environment variable / the params configmap, allowing for more flexible configuration.
Checklist: