fix: convert one-time jobs to Helm hooks to prevent ArgoCD OutOfSync - #899
fix: convert one-time jobs to Helm hooks to prevent ArgoCD OutOfSync#899pinkneyj wants to merge 5 commits into
Conversation
Greptile SummaryThis PR converts four one-time Kubernetes Jobs to stable-name Helm pre-install and pre-upgrade hooks to avoid ArgoCD tracking completed Jobs.
Confidence Score: 4/5This PR should not merge until the hook ordering is changed so fresh installations have the required resources and upgrades process the new revision's configuration. The new pre-install hooks depend on ordinary resources that do not yet exist, while the pre-upgrade config-loader consumes ordinary ConfigMaps before their new revision is applied. Files Needing Attention: helm/templates/config-loader-job.yaml, helm/templates/backfill-job.yaml, helm/templates/helmless-job.yaml, helm/templates/init-cert-job.yaml Important Files Changed
Prompt To Fix All With AI### Issue 1
helm/templates/config-loader-job.yaml:12-13
**Pre-install hooks lack dependencies**
On a fresh installation, Helm runs these pre-install Jobs before creating their ordinary ConfigMaps, service accounts, Secrets, and RBAC resources. The config-loader and helmless hooks are always rendered, so their pods cannot start and the installation blocks before the required release resources are created.
### Issue 2
helm/templates/config-loader-job.yaml:12-13
**Pre-upgrade hook reads stale configuration**
When an upgrade changes values represented in the agent, validator, helmless, webhook, or aggregator ConfigMaps, this pre-upgrade hook runs before Helm applies the new ordinary manifests. It therefore loads and validates the previous revision's configuration, causing the deployed revision and the configuration sent to CloudZero to disagree until a later successful run.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "remove checksum naming" | Re-trigger Greptile |
| (dict "helm.sh/hook" "pre-install,pre-upgrade") | ||
| (dict "helm.sh/hook-delete-policy" "before-hook-creation") |
There was a problem hiding this comment.
Pre-install hooks lack dependencies
On a fresh installation, Helm runs these pre-install Jobs before creating their ordinary ConfigMaps, service accounts, Secrets, and RBAC resources. The config-loader and helmless hooks are always rendered, so their pods cannot start and the installation blocks before the required release resources are created.
Prompt To Fix With AI
This is a comment left during a code review.
Path: helm/templates/config-loader-job.yaml
Line: 12-13
Comment:
**Pre-install hooks lack dependencies**
On a fresh installation, Helm runs these pre-install Jobs before creating their ordinary ConfigMaps, service accounts, Secrets, and RBAC resources. The config-loader and helmless hooks are always rendered, so their pods cannot start and the installation blocks before the required release resources are created.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| (dict "helm.sh/hook" "pre-install,pre-upgrade") | ||
| (dict "helm.sh/hook-delete-policy" "before-hook-creation") |
There was a problem hiding this comment.
Pre-upgrade hook reads stale configuration
When an upgrade changes values represented in the agent, validator, helmless, webhook, or aggregator ConfigMaps, this pre-upgrade hook runs before Helm applies the new ordinary manifests. It therefore loads and validates the previous revision's configuration, causing the deployed revision and the configuration sent to CloudZero to disagree until a later successful run.
Prompt To Fix With AI
This is a comment left during a code review.
Path: helm/templates/config-loader-job.yaml
Line: 12-13
Comment:
**Pre-upgrade hook reads stale configuration**
When an upgrade changes values represented in the agent, validator, helmless, webhook, or aggregator ConfigMaps, this pre-upgrade hook runs before Helm applies the new ordinary manifests. It therefore loads and validates the previous revision's configuration, causing the deployed revision and the configuration sent to CloudZero to disagree until a later successful run.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.|
Hi @pinkneyj , thank you so much for the contribution! We really appreciate you taking the time to do so, and it goes a long way in improving the usability and function of the chart. Regarding your change; this is certainly solving a legitimate problem. However, while we want this chart to work well with standard deployment tools, we try not to design the basic functions of the chart around features from a particular deployment tool. Even though ArgoCD handles helm hooks by mapping them to their own lifecycle annotations, we cannot be sure that all users will have a deployment strategy that does so. ArgoCD itself ignored helm hooks in previous versions. Some other thoughts/questions:
Thank you again for contributing. This is a helpful change, and I want to make sure we get the benefits while also keeping the chart flexible. We might be able to get to the same place with more instructive documentation. |
Why
When the following Jobs complete and are deleted, ArgoCD marks the application as
OutOfSyncbecause it expects them to exist as tracked resources:cloudzero-backfillcloudzero-confload(config-loader-job.yaml)cloudzero-helmlesscloudzero-init-certAdditionally, the current approach of embedding a
checksum/configin job names to trigger re-runs on config changes creates resource churn and leaves stale Job objects in the cluster.What
Convert the one-time Jobs to Helm hooks by adding the following annotations:
This means:
before-hook-creationdeletes the previous job run before creating a new one, replacing the need for achecksum/config-based nameThe
CronJobinbackfill-job.yamlis left unchanged as it manages its own lifecycle.Note on previous hook rejection
A comment in
backfill-job.yamlnotes that Helm hooks were previously rejected due to incompatibility withhelm template, ArgoCD, and Flux. This is addressed as follows:--enable-helm-hookshelm template: Hooks are excluded fromhelm templateoutput by design. Users relying onhelm template | kubectl applywill need to handle these jobs separately or usehelm install/upgradedirectlyFiles changed
charts/cloudzero-agent/templates/backfill-job.yaml— Job only (not CronJob)charts/cloudzero-agent/templates/config-loader-job.yamlcharts/cloudzero-agent/templates/helmless-job.yamlcharts/cloudzero-agent/templates/init-cert-job.yamlHow Tested
Validated the following against a live cluster:
Syncedwith no OutOfSync errorshelm upgradeand confirmed hook jobs were recreated as expected via thebefore-hook-creationdelete policycloudzero-backfillCronJob was untouched and continued to run on its configured schedule