Read workload image from WORKLOAD_IMAGE env var (fix disconnected CI)

[ugreener]

Summary

Fix disconnected CI failures caused by openshift/tools:latest not existing on air-gapped clusters (Cluster Samples Operator is Removed). Replace the hardcoded image constant with WORKLOAD_IMAGE env var, set by the medik8s-lib Prow step as the single source of truth for all environments.

Changes

• tests/internal/medik8sparams/const.go: Replace hardcoded DefaultWorkloadImage constant with os.Getenv("WORKLOAD_IMAGE") (no fallback)
• tests/far-operator/internal/farparams/const.go: Change WorkloadTestImage from const to var (required because DefaultWorkloadImage is now a var)
• tests/snr-operator/internal/snrparams/const.go: Same const to var change
• README.md: Add WORKLOAD_IMAGE to mandatory env vars with local-run example

The image is defined in ONE place: medik8s-lib-commands.sh in openshift/release. Both connected and disconnected CI read it from SHARED_DIR/workload_image. No hardcoded image string in this repo.

Companion PR: openshift/release#82523

Summary by CodeRabbit

• New Features

  • Workload images can now be configured through the WORKLOAD_IMAGE environment variable.
  • Test runs use the configured image consistently across supported operators.

• Bug Fixes

  • Runs now fail immediately with a clear error when WORKLOAD_IMAGE is missing or empty, instead of proceeding with an invalid image value.

• Documentation

  • Added setup instructions and local-run examples showing how to define WORKLOAD_IMAGE.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 21a7da68-611c-48de-b8b5-29e1c34a93e2

📥 Commits

Reviewing files that changed from the base of the PR and between 2eba054 and 665404b.

📒 Files selected for processing (4)

• README.md
• tests/far-operator/internal/farparams/const.go
• tests/internal/medik8sparams/const.go
• tests/snr-operator/internal/snrparams/const.go

🚧 Files skipped from review as they are similar to previous changes (3)

• README.md
• tests/far-operator/internal/farparams/const.go
• tests/snr-operator/internal/snrparams/const.go

---

📝 Walkthrough

Walkthrough

The workload image is now read from WORKLOAD_IMAGE. FAR and SNR workload image parameters are changed from constants to runtime-initialized variables referencing the shared value, and the required environment variable is documented.

Changes

Workload image configuration

Layer / File(s)	Summary
Configure workload image source tests/internal/medik8sparams/const.go, README.md	Initializes DefaultWorkloadImage from WORKLOAD_IMAGE, fails if it is empty, and documents the required variable for local runs.
Propagate runtime image configuration tests/far-operator/internal/farparams/const.go, tests/snr-operator/internal/snrparams/const.go	Declares FAR and SNR workload image values as variables referencing medik8sparams.DefaultWorkloadImage, while retaining MinReadyMasterNodes as a constant.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• medik8s/system-tests#58: Updates shared workload image usage and downstream FAR and SNR test image parameters.

Suggested reviewers: jmontleon, rbartal

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: sourcing the workload image from WORKLOAD_IMAGE to fix disconnected CI.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[qodo-2-for-medik8s]

PR Summary by Qodo

Use UBI9 ubi-minimal as default workload image for disconnected CI

🐞 Bug fix ⚙️ Configuration changes 🕐 Less than 10 minutes

AI Description

• Replace test workload image to avoid ImagePullBackOff on disconnected clusters.
• Use public UBI9 ubi-minimal and rely on ImageTagMirrorSet for disconnected mirroring.

Diagram

graph TD
  A["E2E / destructive tests"] --> B["medik8sparams/const.go"] --> C["Workload pods"] --> D["Image pull"]
  D --> E["registry.access.redhat.com/ubi9/ubi-minimal"]
  D --> F["Mirror via ImageTagMirrorSet"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Make DefaultWorkloadImage configurable (env/flag) with a safe default

• ➕ Avoids future hard-coded image churn for CI/environment differences
• ➕ Lets downstream/partners pin to an approved mirrored image without code changes
• ➖ Adds configuration surface area and validation complexity
• ➖ Can increase test flakiness if misconfigured

2. Use an image guaranteed in release payload / internal registry

• ➕ Removes dependency on external registry availability
• ➕ Potentially simpler for fully disconnected environments
• ➖ Requires identifying and maintaining a stable payload-provided image contract
• ➖ May still fail if cluster operators providing it are removed/disabled (similar to samples operator issue)

Recommendation: The PR’s approach (switching to UBI9 ubi-minimal) is the simplest and most robust immediate fix because it works on connected clusters and has a clear mirroring story for disconnected clusters via ImageTagMirrorSet. If this value is expected to change across environments over time, consider a follow-up to allow overriding DefaultWorkloadImage via configuration while keeping UBI minimal as the default.

Files changed (1) +4 / -1

Bug fix (1) +4 / -1

const.goSwitch DefaultWorkloadImage to UBI9 ubi-minimal +4/-1

Switch DefaultWorkloadImage to UBI9 ubi-minimal

• Replaces the default test workload image from an internal OpenShift ImageStream reference to the public UBI9 ubi-minimal image. Adds inline documentation explaining connected vs disconnected behavior and the ImageTagMirrorSet-based mirroring expectation.

tests/internal/medik8sparams/const.go

[qodo-2-for-medik8s]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 13 rules

1. Workload/tools image coupling ✗ Dismissed 🐞 Bug ≡ Correctness

Description

Changing DefaultWorkloadImage also changes the default SBR WatchdogDebugImage, but SBR uses that
image for privileged host-namespace tooling (e.g., running /bin/bash and nsenter-based host
operations), not just a simple “sleep” workload container. This coupling means a change intended for
workload pods can inadvertently break SBR watchdog/debug pods unless the chosen image satisfies
those tooling requirements or CI always sets SBR_WATCHDOG_DEBUG_IMAGE.

Code

tests/internal/medik8sparams/const.go[R16-19]

+	// Uses a public UBI image that works on connected clusters directly and on
+	// disconnected clusters via ImageTagMirrorSet (mirrored by the
+	// medik8s-disconnected-catalogsource Prow step).
+	DefaultWorkloadImage = "registry.access.redhat.com/ubi9/ubi-minimal:latest"

Relevance

⭐⭐ Medium

Coupling workload and privileged debug/tooling images could break nsenter/iptables usage; no clear
precedent on decoupling defaults.

PR-#30

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

SBR’s WatchdogDebugImage defaults to DefaultWorkloadImage, and SBR tests run /bin/bash and
nsenter/iptables commands in pods built from that image. Therefore, changing
DefaultWorkloadImage changes a tooling image as well, not just a workload image.

tests/internal/medik8sparams/const.go[15-20]
tests/sbr-operator/internal/sbrparams/const.go[271-279]
tests/sbr-operator/tests/detect_only.go[166-176]
tests/sbr-operator/tests/sbr_helpers.go[38-52]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`medik8sparams.DefaultWorkloadImage` is used for two different roles:
1) FAR/SNR workload pods that just run `sleep infinity`.
2) SBR watchdog/debug pods that execute host-namespace tooling (e.g., `/bin/bash -c ... nsenter ...`, plus iptables operations).

Switching the default to `ubi-minimal` may be fine for (1) but is risky for (2), and the PR description suggests the intent was only to change the workload pod image.

### Issue Context
- `sbrparams.WatchdogDebugImage` defaults to `medik8sparams.DefaultWorkloadImage`.
- SBR creates privileged pods using that image and runs commands that require specific binaries.

### Fix Focus Areas
- tests/internal/medik8sparams/const.go[15-20]
- tests/sbr-operator/internal/sbrparams/const.go[271-279]
- tests/sbr-operator/tests/detect_only.go[166-176]
- tests/sbr-operator/tests/sbr_helpers.go[38-52]

### Suggested fix approach
- Introduce a separate default for SBR tooling, e.g. `medik8sparams.DefaultToolsImage` (or `DefaultWatchdogDebugImage`) that is explicitly meant for nsenter/iptables/bash use, and have `WatchdogDebugImage` default to that.
- Keep `DefaultWorkloadImage` optimized for simple sleep-based workload pods.
- If you want to keep a single image, tighten the contract in comments and add a CI guard (or a small runtime self-check pod) that fails fast with a clear message when required binaries are missing, and document setting `SBR_WATCHDOG_DEBUG_IMAGE` when needed.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Hardcoded workload image ✓ Resolved 🐞 Bug ☼ Reliability

Description

FAR/SNR tests compile in the workload image via medik8sparams.DefaultWorkloadImage with no
env/config override, so clusters that cannot reach registry.access.redhat.com and do not have the
expected mirror configuration will hit ImagePullBackOff and fail the suites. This PR changes that
hardcoded value to an external registry reference, increasing the chance of test failure in
restricted/disconnected environments outside the specific Prow step setup.

Code

tests/internal/medik8sparams/const.go[R16-19]

+	// Uses a public UBI image that works on connected clusters directly and on
+	// disconnected clusters via ImageTagMirrorSet (mirrored by the
+	// medik8s-disconnected-catalogsource Prow step).
+	DefaultWorkloadImage = "registry.access.redhat.com/ubi9/ubi-minimal:latest"

Relevance

⭐⭐⭐ High

Team previously accepted making disconnected-breaking test images configurable/mirrorable; same
reliability concern applies here.

PR-#52

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

DefaultWorkloadImage is a hardcoded constant and is used as the workload image for FAR and SNR
pods; FAR/SNR provide no env var override, so the new external registry reference becomes mandatory
unless cluster-level mirroring is set up externally.

tests/internal/medik8sparams/const.go[15-20]
tests/far-operator/internal/farparams/const.go[118-120]
tests/snr-operator/internal/snrparams/const.go[126-128]
tests/far-operator/tests/far_destructive.go[346-358]
tests/snr-operator/tests/helpers.go[500-513]
PR-#52

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`DefaultWorkloadImage` is a compile-time constant and FAR/SNR bind their `WorkloadTestImage` constants directly to it, leaving no supported way to redirect the workload image to an environment-specific mirror/registry (common in disconnected/restricted clusters). With this PR switching the value to an external registry, that lack of override becomes an operational footgun.

### Issue Context
- FAR and SNR define `WorkloadTestImage` as a `const`, so it cannot reference a runtime value (like `os.Getenv`).
- SBR already uses an env-var override pattern for a related image (`SBR_WATCHDOG_DEBUG_IMAGE`).

### Fix Focus Areas
- tests/internal/medik8sparams/const.go[15-20]
- tests/far-operator/internal/farparams/const.go[111-120]
- tests/snr-operator/internal/snrparams/const.go[122-128]

### Suggested fix approach
- Introduce an env var (e.g., `MEDIK8S_WORKLOAD_IMAGE`) and wire it into FAR/SNR as a `var WorkloadTestImage = func() string { ... }()` fallbacking to `medik8sparams.DefaultWorkloadImage`.
- Alternatively, add a new shared helper in `medik8sparams` returning an overridable workload image, and switch FAR/SNR to use it (which will require converting their `WorkloadTestImage` from `const` to `var`).
- Document the env var next to the image constants/comments.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@README.md`:
- Around line 45-48: Update the later make run-tests example in README.md to
also export WORKLOAD_IMAGE, using the documented local image value, alongside
KUBECONFIG and the test selectors.

In `@tests/internal/medik8sparams/const.go`:
- Around line 17-23: Validate WORKLOAD_IMAGE during initialization of
DefaultWorkloadImage and fail immediately when it is unset or empty, rather than
propagating an invalid image value. Do not add a public-registry fallback;
preserve the explicitly configured mirror-specific image for disconnected
environments.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 19a35644-8d26-4dba-9a0e-c61b5b4586d5

📥 Commits

Reviewing files that changed from the base of the PR and between 2853a1a and 2eba054.

📒 Files selected for processing (4)

• README.md
• tests/far-operator/internal/farparams/const.go
• tests/internal/medik8sparams/const.go
• tests/snr-operator/internal/snrparams/const.go

🚧 Files skipped from review as they are similar to previous changes (2)

• tests/far-operator/internal/farparams/const.go
• tests/snr-operator/internal/snrparams/const.go

[ugreener]

/test 4.22-disconnected-e2e-far-aws-disconnected

[ugreener]

/test 4.22-disconnected-e2e-far-aws-disconnected

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: razo7, ugreener

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [razo7,ugreener]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment