Skip to content

feat: fix and improve ConfigMap and CR deployment#59

Merged
mchmarny merged 3 commits intoNVIDIA:mainfrom
yuanchen8911:fix/helm-hook-for-crd-dependent-resources
Feb 5, 2026
Merged

feat: fix and improve ConfigMap and CR deployment#59
mchmarny merged 3 commits intoNVIDIA:mainfrom
yuanchen8911:fix/helm-hook-for-crd-dependent-resources

Conversation

@yuanchen8911
Copy link
Contributor

@yuanchen8911 yuanchen8911 commented Feb 5, 2026

Summary

This PR addresses three issues with the Helm umbrella chart generation for ConfigMap and CRD-dependent resources:

  1. Fix ConfigMaps incorrectly placed in post-install directory

    • ConfigMaps (dcgm-exporter.yaml, kernel-module-params.yaml) are standard K8s resources that should be deployed during helm install, not via separate kubectl apply
    • Added isCRDDependentResource() to distinguish between standard K8s resources (v1, apps/v1, etc.) and Custom Resources based on apiVersion
    • Standard resources now go to templates/, only CRs get Helm hooks
  2. Use Helm hooks for CRD-dependent resources instead of post-install

    • Replaced the separate post-install/ directory approach with Helm post-install,post-upgrade hooks
    • Added addHelmHookAnnotation() to inject hook annotations into CRs
    • This enables single-step deployment compatible with GitOps (ArgoCD/Flux), CI/CD pipelines, and standard Helm workflows
    • Removed processManifestForKubectl() as it's no longer needed
  3. Clean up old CRs before helm install/upgrade

    • Added helm.sh/hook-delete-policy: before-hook-creation annotation
    • Ensures old CRs are deleted before new ones are created during upgrades
    • Provides idempotent deployments with fresh CR state on each upgrade

Also fixes documentation in docs/architecture/component.md:

  • Removed kubectl apply -f post-install/ step
  • Fixed make dev -> make dev-env typo

Test plan

  • Run make test - all tests pass
  • Run make lint - no issues
  • Test bundle generation and verify ConfigMaps in templates/ and CRs have hook annotations
  • Deploy to Kind cluster and verify single-step helm install works

@yuanchen8911 yuanchen8911 requested a review from a team as a code owner February 5, 2026 01:02
@yuanchen8911 yuanchen8911 requested a review from mchmarny February 5, 2026 01:06
@github-actions
Copy link

github-actions bot commented Feb 5, 2026

Coverage Report ✅

Metric Value
Coverage 73.0%
Threshold 70%
Status Pass
Coverage Badge
![Coverage](https://img.shields.io/badge/coverage-73.0%25-green)

No Go source files changed in this PR.

@yuanchen8911 yuanchen8911 changed the title fix: use Helm hooks for CRD-dependent resources instead of post-install feat: use Helm hooks for CRD-dependent resources instead of post-install Feb 5, 2026
@yuanchen8911 yuanchen8911 changed the title feat: use Helm hooks for CRD-dependent resources instead of post-install feat: fix and improve ConfigMap and CR deployment Feb 5, 2026
This commit addresses three issues with the Helm umbrella chart generation:

1. Fix ConfigMaps incorrectly placed in post-install directory
   - ConfigMaps (dcgm-exporter.yaml, kernel-module-params.yaml) are standard
     K8s resources that should be deployed during helm install
   - Added isCRDDependentResource() to distinguish between standard K8s
     resources and Custom Resources based on apiVersion
   - Standard resources now go to templates/, CRs get Helm hooks

2. Use Helm hooks for CRD-dependent resources instead of post-install
   - Replaced the separate post-install/ directory approach with Helm
     post-install/post-upgrade hooks
   - Added addHelmHookAnnotation() to inject hook annotations into CRs
   - This enables single-step deployment compatible with GitOps (ArgoCD/Flux),
     CI/CD pipelines, and standard Helm workflows
   - Removed processManifestForKubectl() as it's no longer needed

3. Clean up old CRs before helm install/upgrade
   - Added helm.sh/hook-delete-policy: before-hook-creation annotation
   - Ensures old CRs are deleted before new ones are created during upgrades
   - Provides idempotent deployments with fresh CR state on each upgrade

Also fixes documentation:
- Updated docs/architecture/component.md to remove kubectl apply post-install
- Fixed make dev -> make dev-env typo
@yuanchen8911 yuanchen8911 force-pushed the fix/helm-hook-for-crd-dependent-resources branch from a8d07d1 to 7f8b013 Compare February 5, 2026 01:25
Copy link
Member

@mchmarny mchmarny left a comment

Choose a reason for hiding this comment

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

/lgtm

@mchmarny mchmarny merged commit 3d54db0 into NVIDIA:main Feb 5, 2026
6 checks passed
@mchmarny mchmarny deleted the fix/helm-hook-for-crd-dependent-resources branch February 5, 2026 11:36
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.

2 participants