Skip to content

fix(tests): keep two unit tests green after the release image restamp - #5179

Merged
Aleksei Sviridkin (lexfrei) merged 1 commit into
cozystack:release-1.7from
themoriarti:fix/release-1.7-stamped-image-tests
Nov 9, 2026
Merged

Aleksei Sviridkin (lexfrei) merged 1 commit into
cozystack:release-1.7from
themoriarti:fix/release-1.7-stamped-image-tests

Conversation

@themoriarti

@themoriarti Marian Koreniuk (themoriarti) commented Nov 1, 2026 •

Copy link
Copy Markdown
Collaborator

What this PR does

Every PR against release-1.7 has had a red Unit & controller tests check since v1.7.0-rc.3 (seen on #5172, job). The cause is the release build itself: the Prepare release commit (0402103) rewrites packages/*/values.yaml, and two unit tests read exactly the fields it rewrites.

test reads before the release build after
hack/cni-plugins-staging-contract.bats "the kube-ovn release the CNI plugin pin was checked against is the one vendored" .global.images.kubeovn.tag in packages/system/kubeovn/values.yaml v1.15.26@sha256:… v1.7.0@sha256:…
packages/system/monitoring/tests/vtraces_collector_test.yaml "pins the collector image through the cozy-lib.image helper" tracingCollector.image docker.io/otel/opentelemetry-collector-contrib:0.136.0@sha256:… ghcr.io/cozystack/cozystack/otelcol-tracing:v0.136.0-cozystack.0@sha256:…

Changes:

  • The bats test now takes the vendored kube-ovn release from appVersion in packages/system/kubeovn/charts/kube-ovn/Chart.yaml. That is the value packages/system/kubeovn/Makefile builds the image from, and a release build leaves it alone. The hint printed on a mismatch names the new file.
  • The helm unittest case no longer names the image at all. It sets _cluster.images-registry and asserts that the registry lands in front of the rendered image and that the image stays digest-pinned (the grafana_upstream_image_plugins_test.yaml pattern). Removing the cozy-lib.image call from the template makes it fail; the old regex did not.

Verified locally on this branch: bats hack/cni-plugins-staging-contract.bats 10/10, helm unittest packages/system/monitoring -f tests/vtraces_collector_test.yaml 28/28.

Not a backport: main still carries the pre-release pins, so both tests pass there today. The same patch is open against main as #5180, so release-1.8 does not meet the same surprise.

Fixes #5178.
Refs #5172 (blocked on this check).

Downstream repositories

  • No downstream repository is affected by this change

Release note

NONE

@coderabbitai

coderabbitai Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Marian Koreniuk (@themoriarti) The code is right, but the commit history needs one more pass before merge.

The bats test now reads the kube-ovn release from the place the build itself reads it. The kube-ovn Makefile builds the image from appVersion in the vendored chart, and its image target is what writes IMAGE_TAG@digest into .global.images.kubeovn.tag. So the old test was reading build output. Neither Prepare release commit on release-1.7 (rc.3 and v1.7.0) touches a Chart.yaml, and nothing under hack/ or .github/workflows/ writes appVersion.

I ran both tests on this head and on the release-1.7 tip. The bats file is 10/10 here and 9/10 on the base, where test 4 fails exactly as #5178 describes. The vtraces case is 28/28 here and 27/28 on the base. Unit & controller tests is green on this head, so these two were the only tests the restamp broke.

Both tests still go red when they should. Moving the multus marker to v1.15.25, bumping appVersion to 1.15.27, or deleting appVersion each fail test 4, while restamping the values.yaml tag keeps it green. On the helm side, replacing the cozy-lib.image include with the raw value fails the case, and so does stripping the digest from values.yaml. The old regex on main stays green with the helper removed, so the claim in the body holds.

What blocks is the history. The repo merges with merge commits, so both commit bodies land as written. The first one says the vtraces test "now accepts the first-party image name too". The second drops that regex and opens with "The previous assertion listed the image names...", which describes an earlier state of this PR, not the code. contributing.md treats review-iteration wording in a commit message as a blocker. Please fold the second commit into the first, or make it one commit per test, so each message describes what lands.

Two small things, not blocking:

  • The bats comment pins "(v1.7.0)". "to the Cozystack version" says the same and won't go stale.
  • The body still says "I can open it if wanted" about main, but #5180 exists now, and the body becomes the merge commit message.

@themoriarti
Marian Koreniuk (themoriarti) force-pushed the fix/release-1.7-stamped-image-tests branch from e05c6af to 661c5a2 Compare November 5, 2026 21:53
Marian Koreniuk (themoriarti) added a commit to themoriarti/cozystack that referenced this pull request Nov 5, 2026
The release build rewrites packages/*/values.yaml: the kube-ovn image tag
becomes the Cozystack version and the traces collector becomes the
first-party otelcol-tracing build. Two unit tests read exactly those
fields, so every PR against release-1.7 has failed Unit & controller
tests since the first rc, and release-1.8 would repeat it:

- hack/cni-plugins-staging-contract.bats compared the multus marker with
  the values.yaml tag. It now reads appVersion from the vendored kube-ovn
  chart, which is what packages/system/kubeovn/Makefile builds from and
  what a release build leaves alone.
- vtraces_collector_test.yaml matched the upstream collector image name.
  It now sets the cluster images-registry and checks that it lands in
  front of the rendered image and that the image stays digest-pinned:
  that is the cozy-lib.image helper's behaviour, and it holds on main
  and on a release branch alike. Removing the helper from the template
  makes the case fail; a name regex did not notice.

Same change as cozystack#5179 on release-1.7.

Assisted-by: LLM
Signed-off-by: Marian Koreniuk <[email protected]>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Marian Koreniuk (@themoriarti) NOT LGTM: please keep the release-restamp explanation and remove the implementation walkthrough from the commit body. The earlier request to fold the history is fixed; no source change is needed.

The remaining blocker is the combined commit body. In 661c5a205510, the failure explanation is followed by two bullets describing how the tests work:

The release build rewrites packages/*/values.yaml: the kube-ovn image tag
becomes the Cozystack version and the traces collector becomes the
first-party otelcol-tracing build. Two unit tests read exactly those
fields, so every PR against release-1.7 has failed Unit & controller
tests since the first rc:

- hack/cni-plugins-staging-contract.bats compared the multus marker with
  the values.yaml tag. It now reads appVersion from the vendored kube-ovn
  chart, which is what packages/system/kubeovn/Makefile builds from and
  what a release build leaves alone.
- vtraces_collector_test.yaml matched the upstream collector image name.
  It now sets the cluster images-registry and checks that it lands in
  front of the rendered image and that the image stays digest-pinned:
  that is the cozy-lib.image helper's behaviour, and it holds on main
  and on a release branch alike. Removing the helper from the template
  makes the case fail; a name regex did not notice.

The commit rules require the body to explain why the change is needed. The Review Blockers section explicitly includes bodies that walk through the diff. Please amend this commit to keep the rationale without the implementation bullets.

I reran the tests on a merge with current release-1.7: BATS 10/10 and collector 28/28; the base fails exactly those two cases. The full monitoring and multus Helm suites pass too. The version-mismatch, helper-bypass and digest controls fail the intended assertions.

The release build rewrites packages/*/values.yaml: the kube-ovn image tag
becomes the Cozystack version and the traces collector becomes the
first-party otelcol-tracing build. Two unit tests read exactly those
fields, so every PR against release-1.7 has failed Unit & controller
tests since the first rc.

The same patch applies to main as cozystack#5180.

Assisted-by: LLM
Signed-off-by: Marian Koreniuk <[email protected]>
@themoriarti
Marian Koreniuk (themoriarti) force-pushed the fix/release-1.7-stamped-image-tests branch from 661c5a2 to fcc1ada Compare November 8, 2026 18:29
Marian Koreniuk (themoriarti) added a commit to themoriarti/cozystack that referenced this pull request Nov 8, 2026
The release build rewrites packages/*/values.yaml: the kube-ovn image tag
becomes the Cozystack version and the traces collector becomes the
first-party otelcol-tracing build. Two unit tests read exactly those
fields, so every PR against release-1.7 has failed Unit & controller
tests since the first rc, and release-1.8 would repeat it.

Same change as cozystack#5179 on release-1.7.

Assisted-by: LLM
Signed-off-by: Marian Koreniuk <[email protected]>
@themoriarti

Copy link
Copy Markdown
Collaborator Author

Aleksei Sviridkin (@lexfrei) Fixed in fcc1ada: the commit body keeps the release-restamp explanation and drops the two implementation bullets. It is a message-only amend, the tree is identical to 661c5a2, and CI is green on the new head. #5180 got the same amend (e92bc86).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Marian Koreniuk (@themoriarti) LGTM. The commit-message request is fixed. The release-1.7 base and the complete tested tree are unchanged, so the earlier test and control results still apply. Current unit and fork E2E checks pass.

@lexfrei
Aleksei Sviridkin (lexfrei) merged commit ec30722 into cozystack:release-1.7 Nov 9, 2026
19 checks passed
Aleksei Sviridkin (lexfrei) added a commit that referenced this pull request Nov 9, 2026
…#5180)

## What this PR does

Same change as #5179, on `main`. Both tests pass here today, because
`main` still carries the pre-release image pins; they fail on every
release line as soon as the first rc is cut, since the `Prepare release`
commit rewrites `packages/*/values.yaml` and two unit tests read exactly
the fields it rewrites. On `release-1.7` this has kept `Unit &
controller tests` red on every PR since v1.7.0-rc.3
([job](https://github.com/cozystack/cozystack/actions/runs/41162009892/job/124421449910)
on #5172), and `release-1.8` would repeat it.

| test | reads | before the release build | after (release-1.7 today) |
|---|---|---|---|
| `hack/cni-plugins-staging-contract.bats` "the kube-ovn release the CNI
plugin pin was checked against is the one vendored" |
`.global.images.kubeovn.tag` in `packages/system/kubeovn/values.yaml` |
`v1.15.26@sha256:…` | `v1.7.0@sha256:…` |
| `packages/system/monitoring/tests/vtraces_collector_test.yaml` "pins
the collector image through the cozy-lib.image helper" |
`tracingCollector.image` |
`docker.io/otel/opentelemetry-collector-contrib:0.136.0@sha256:…` |
`ghcr.io/cozystack/cozystack/otelcol-tracing:v0.136.0-cozystack.0@sha256:…`
|

Changes:

- The bats test takes the vendored kube-ovn release from `appVersion` in
`packages/system/kubeovn/charts/kube-ovn/Chart.yaml`. That is the value
`packages/system/kubeovn/Makefile` builds the image from, and a release
build leaves it alone. The hint printed on a mismatch names the new
file.
- The helm unittest case no longer names the image at all. It sets
`_cluster.images-registry` and asserts that the registry lands in front
of the rendered image and that the image stays digest-pinned (the
`grafana_upstream_image_plugins_test.yaml` pattern). Removing the
`cozy-lib.image` call from the template makes it fail; the old regex did
not.

Same single commit as #5179; the only difference is the unrelated `DOC=`
line next to the edit, kept as `main` has it.

Verified locally on this branch: `bats
hack/cni-plugins-staging-contract.bats` 10/10, `helm unittest
packages/system/monitoring -f tests/vtraces_collector_test.yaml` 28/28.

Refs #5178 (the release-1.7 report), #5179, #5172.

### Downstream repositories

- [x] No downstream repository is affected by this change

### Release note

```release-note
NONE
```


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
<!-- coderabbit-ai-generated:v1 -->

## Summary by CodeRabbit

* **Tests**
* Version validation now compares the checked version with the chart’s
application version and reports missing or mismatched versions with the
chart path.
* Monitoring image checks now verify use of the configured mirror
registry and require a 64-character lowercase SHA-256 digest, without
requiring a specific image name.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/testing Issues or PRs related to testing (e2e, bats, unit tests) kind/bug Categorizes issue or PR as related to a bug size/S This PR changes 10-29 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants