Repository navigation
test(rhdh): assert mirrored images and StorageClass in rendered manifests - #749
Open
gustavolira wants to merge 3 commits into
Open
gustavolira wants to merge 3 commits into
gustavolira wants to merge 3 commits into
Conversation
…ests Constrained clusters (mirrored registry only, no default StorageClass) depend on the chart applying global.imageRegistry to every image and on the dynamic plugins volume honouring the StorageClass settings. Nothing asserted either: CI only checked that helm template succeeds. Add hack/assert-rendered-values.sh, run by the test-charts action next to the helm template sanity step, before the kind cluster is created: - with global.imageRegistry set, every image (containers, init containers, helm test pod, SonataFlow CR images, CATALOG_INDEX_IMAGE and EXTRA_CATALOG_INDEX_IMAGES) is under the mirror, for the default values, every ci/*-values.yaml and a scenario enabling the optional images (OKP, orchestrator data index/job service, extra indexes); - the helm test pod image follows test.image.* (RHDHBUGS-559); - the ephemeral volumeClaimTemplate carries ephemeral.storageClassName or global.defaultStorageClass, and no storageClassName when neither is set (RHDHBUGS-781); - statefulSetPVC honours its storageClassName; - type=emptyDir renders no PVC or ephemeral claim. Add ci/with-custom-storage-class-values.yaml so ct install exercises the dynamic plugins volume on the custom-sc StorageClass the action already creates on kind. Bump the chart version for the new ci file. Refs: RHIDP-16230 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The mirror check matched the registry prefix as a regex, so a lookalike host such as mirrorXlocal/ passed; escape the dots. Compare the emptyDir volume's sorted keys so the check does not depend on the order the template renders fields in. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- The "nothing set" StorageClass check used has() on the claim spec, which yq answers false for a missing spec too, so it passed when the plugins volume was no longer ephemeral. Compare the spec's keys instead; a missing spec now fails the check. - An empty image list made grep exit 1 under pipefail and aborted the script before the "no image references" failure could be reported. - Assert that ephemeral.storageClassName takes precedence over global.defaultStorageClass when both are set. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Description of the change
A constrained cluster (mirrored registry only, no default StorageClass) depends on two things in the
rhdhchart:global.imageRegistryreaching every image, and the dynamic plugins volume honouring the StorageClass settings. Today CI only checks thathelm templatesucceeds, so a template that hardcodes a registry or dropsstorageClassNamewould still pass.This PR adds two checks:
hack/assert-rendered-values.shis a bash +yqscript with no new tool. Thetest-chartsaction runs it right after the existinghelm templatesanity step, before the kind cluster is created, so it adds about 2s. It renders the chart and asserts:--set global.imageRegistry=mirror.local, every image reference is undermirror.local/. That coversimagefields in containers, init containers, thehelm testpod and SonataFlow CRs, plus theCATALOG_INDEX_IMAGEandEXTRA_CATALOG_INDEX_IMAGESenv values. The script renders the default values, everycharts/rhdh/ci/*-values.yaml, and one extra scenario that turns on the optional images no ci file uses (OKP, orchestrator data index and job service, extra catalog indexes).helm testpod image followstest.image.*(RHDHBUGS-559).volumeClaimTemplatecarriesdynamicPlugins.volume.ephemeral.storageClassNameorglobal.defaultStorageClasswhen either is set, and the explicit value wins when both are. When neither is set it has nostorageClassName, so the cluster default applies (RHDHBUGS-781).statefulSetPVChonours itsstorageClassName.dynamicPlugins.volume.type=emptyDirrenders no PVC and no ephemeral claim.charts/rhdh/ci/with-custom-storage-class-values.yamlsetsdynamicPlugins.volume.ephemeral.storageClassName: custom-sc, soct install(andhelm test/--upgrade) runs the plugins volume on thecustom-scStorageClass that the action already creates on kind. Norhdhci file used that StorageClass before.The chart version goes from 3.5.17 to 3.5.18 because
ct lintneeds a bump when a file undercharts/rhdh/changes (here, the new ci file). Templates and values are unchanged.Relation to the coverage work: this complements #698 and ADR-010. Those measure template coverage, while this PR asserts rendered values. ADR-010 chose chart coverage over helm-unittest for coverage; this PR adds no helm-unittest and does not touch
test.yamlornightly.yaml. Armel Soro and Niall Twomey own the coverage work.Dropped from the ticket: "fail fast instead of hanging when there is no default StorageClass". The chart could only check this with
lookupon cluster-scoped StorageClasses.lookupreturns nothing underhelm template,--dry-runand ArgoCD, and it needs cluster-scope RBAC. A check like that would fail installs that are fine, so this PR leaves it out.Which issue(s) does this PR fix or relate to
How to test changes / Special notes to the reviewer
Each assertion was checked against a mutated template and went red, then green again after the template was restored:
quay.io/curl/curl:8.23.0storageClassNameinclude from the ephemeral templateephemeral.storageClassNameandglobal.defaultStorageClassstorageClassName: ""(the RHDHBUGS-781 regression)emptyDirbranch never matchestype=emptyDirCATALOG_INDEX_IMAGEignoresglobalrhdh.image.renderignoresglobalEXTRA_CATALOG_INDEX_IMAGESignoresglobalstatefulSetPVCignoresstorageClassNamestatefulSetPVC.storageClassNamemirrorXlocal/dynamicPlugins.volume.typechanged toemptyDirephemeral.storageClassName,global.defaultStorageClassglobal.defaultStorageClasstakes precedence over the explicit valueThe test workflow's
discover-chartsjob only runs whencharts/or the test CI files change, so a PR that touches onlyhack/assert-rendered-values.shgets ShellCheck but not this script. Fixing that means changingtest.yaml, which #698 is also editing, so I left it for a follow-up.Checklist
Chart.yamlaccording to Semantic Versioning.values.yamland added to the corresponding README.md. The pre-commit utility can be used to generate the necessary content. Runpre-commit run --all-filesto run the hooks and then push any resulting changes. The pre-commit Workflow will enforce this and warn you if needed.pre-commithook. (No schema change.)ct lintcommand.🤖 Generated with Claude Code