Skip to content

ci: add Helm chart validation#3019

Merged
hubcio merged 22 commits intoapache:masterfrom
avirajkhare00:issue-3007-helm-ci
Mar 27, 2026
Merged

ci: add Helm chart validation#3019
hubcio merged 22 commits intoapache:masterfrom
avirajkhare00:issue-3007-helm-ci

Conversation

@avirajkhare00
Copy link
Copy Markdown
Contributor

Summary

  • add a helm component to change detection so helm/** routes through the existing other_matrix
  • add pinned Helm install, helm lint --strict, and the 6 requested render scenarios in _test.yml
  • make the legacy ingress and HPA API selection deterministic by Kubernetes version so the 1.18 CI case really covers networking.k8s.io/v1beta1 and autoscaling/v2beta2

Closes #3007

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.74%. Comparing base (9ee1f6d) to head (5f99cfd).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3019      +/-   ##
============================================
- Coverage     71.76%   71.74%   -0.02%     
  Complexity      943      943              
============================================
  Files          1121     1121              
  Lines         93800    93800              
  Branches      71125    71135      +10     
============================================
- Hits          67315    67298      -17     
- Misses        23851    23852       +1     
- Partials       2634     2650      +16     
Components Coverage Δ
Rust Core 72.35% <ø> (+<0.01%) ⬆️
Java SDK 62.30% <ø> (ø)
C# SDK 67.43% <ø> (-0.21%) ⬇️
Python SDK 81.43% <ø> (ø)
Node SDK 91.44% <ø> (-0.10%) ⬇️
Go SDK 38.68% <ø> (ø)
see 22 files with indirect coverage changes
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@avirajkhare00 avirajkhare00 changed the title ci: add Helm chart validation ci: add Helm chart validation[WIP] Mar 24, 2026
@avirajkhare00 avirajkhare00 changed the title ci: add Helm chart validation[WIP] ci: add Helm chart validation Mar 24, 2026
@avirajkhare00
Copy link
Copy Markdown
Contributor Author

I dug through the hardcoded bits in this PR. The main problem is not the number of scenarios, it is that _test.yml is now carrying some chart-owned values that already exist under helm/charts/iggy/.

There are two buckets here:

  1. Reasonable CI-owned hardcoding
  • pinned Helm/kind versions and checksums
  • kind cluster config
  • ingress-nginx install
  • smoke-test hosts / ingress class / namespace / release names

Those are properties of the CI environment and smoke path, so keeping them in the workflow is fine.

  1. Chart-owned values that should ideally be reused from helm/charts/iggy/
  • chart version / appVersion from helm/charts/iggy/Chart.yaml
  • default image tags from helm/charts/iggy/values.yaml
  • render scenario inputs that are really chart fixtures rather than CI wiring

That second bucket is where the PR feels too hardcoded, because it creates a second source of truth in CI.

If you want to keep this structure, I would suggest a cleaner split:

  • leave CI/runtime setup in _test.yml
  • move chart-specific test fixtures into the helm/charts/iggy/ tree (for example dedicated values files under a small ci/ or tests/ subdir), or derive the asserted values from chart files directly

That keeps the Helm chart as the owner of chart data and makes version/tag bumps less noisy.

Copy link
Copy Markdown
Contributor

@hubcio hubcio left a comment

Choose a reason for hiding this comment

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

overall looks nice, but please don't put bash scripts in _test.yml - please move the actual scenario (not setup) to scripts/ci/test_helm.sh and call it from action context. we want to allow the devs to run this locally. it has to work this way too.

@avirajkhare00 avirajkhare00 requested a review from hubcio March 24, 2026 17:56
@avirajkhare00
Copy link
Copy Markdown
Contributor Author

avirajkhare00 commented Mar 24, 2026

@hubcio pretty sure c# issues are not happening from helm tests.

Edit, added a small fix here: #3024

Also, I have ran all the smoke tests of helm manually. so both ci and humans can run it

Copy link
Copy Markdown
Contributor

@hubcio hubcio left a comment

Choose a reason for hiding this comment

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

also please rename test_helm.sh to test-helm.sh for consistency with the other ci scripts (setup-helm-tools.sh, setup-helm-smoke-cluster.sh all use hyphens).

hubcio
hubcio previously approved these changes Mar 26, 2026
@hubcio
Copy link
Copy Markdown
Contributor

hubcio commented Mar 26, 2026

@avirajkhare00 looks like your newly added smoke test failed

@avirajkhare00 avirajkhare00 requested a review from hubcio March 26, 2026 12:56
@avirajkhare00
Copy link
Copy Markdown
Contributor Author

@hubcio race condition it was: https://stackoverflow.com/questions/68576225/failed-calling-webhook-validate-nginx-ingress-kubernetes-io-error-while-apply

thankfullly its fixed

@hubcio hubcio merged commit 411a697 into apache:master Mar 27, 2026
81 checks passed
slbotbm pushed a commit to slbotbm/iggy that referenced this pull request Mar 27, 2026
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.

Add Helm chart CI validation

4 participants