NETOBSERV-2748 e2etests images and pre-merge tests#80004
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds an ChangesGinkgo E2E Test Step
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: memodi The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/pj-rehearse pull-ci-netobserv-netobserv-operator-main-images |
|
@memodi: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse |
|
@memodi: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
ci-operator/config/netobserv/netobserv-operator/netobserv-netobserv-operator-main.yaml (1)
184-184: ⚡ Quick winPin yamlfmt version for reproducible builds.
Installing
yamlfmtwith@latestcreates non-deterministic builds if the tool is updated between CI runs.📌 Proposed fix to pin the version
- go install github.com/google/yamlfmt/cmd/yamlfmt@latest + go install github.com/google/yamlfmt/cmd/yamlfmt@v0.13.0🤖 Prompt for 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. In `@ci-operator/config/netobserv/netobserv-operator/netobserv-netobserv-operator-main.yaml` at line 184, Replace the non-deterministic installer call "go install github.com/google/yamlfmt/cmd/yamlfmt@latest" with a pinned released version tag (e.g., use a specific semver tag like vX.Y.Z) so CI builds are reproducible; update the command in the same location and add a brief comment indicating the pinned yamlfmt version and date so future maintainers know why it was fixed.ci-operator/step-registry/netobserv/ginkgo-tests/netobserv-ginkgo-tests-commands.sh (1)
28-28: 💤 Low valueEscape special regex characters in filenames.
The focus-file regex is constructed from filenames without escaping special regex metacharacters. The
.in_test.gowill match any character, not a literal dot. While unlikely to cause false matches with typical Go test filenames, this is technically incorrect.For example, the regex
(foo_test.go)would also matchfoo_testXgo(if such a file existed).🔧 Proposed fix to escape special characters
basename=$(basename "${file}") + # Escape special regex characters + basename_escaped=$(printf '%s\n' "$basename" | sed 's/[.[\*^$()+?{|]/\\&/g') if [[ -n "${CHANGED_TEST_FILES}" ]]; then - CHANGED_TEST_FILES="${CHANGED_TEST_FILES}|${basename}" + CHANGED_TEST_FILES="${CHANGED_TEST_FILES}|${basename_escaped}" else - CHANGED_TEST_FILES="${basename}" + CHANGED_TEST_FILES="${basename_escaped}" fi🤖 Prompt for 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. In `@ci-operator/step-registry/netobserv/ginkgo-tests/netobserv-ginkgo-tests-commands.sh` at line 28, The FOCUS_FILE_ARG currently injects raw CHANGED_TEST_FILES into a regex which fails to escape regex metacharacters (e.g., the dot in _test.go); update the code that constructs FOCUS_FILE_ARG so it first escapes regex special characters in CHANGED_TEST_FILES (escape characters like .[]^$*+?(){}| and backslash) and then builds the focus-file expression from the escaped names; locate the place that sets FOCUS_FILE_ARG and replace the raw injection with a pipeline that escapes each filename (preserving the intended grouping) before wrapping with "--focus-file=(...)" so filenames are matched literally.
🤖 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.
Nitpick comments:
In
`@ci-operator/config/netobserv/netobserv-operator/netobserv-netobserv-operator-main.yaml`:
- Line 184: Replace the non-deterministic installer call "go install
github.com/google/yamlfmt/cmd/yamlfmt@latest" with a pinned released version tag
(e.g., use a specific semver tag like vX.Y.Z) so CI builds are reproducible;
update the command in the same location and add a brief comment indicating the
pinned yamlfmt version and date so future maintainers know why it was fixed.
In
`@ci-operator/step-registry/netobserv/ginkgo-tests/netobserv-ginkgo-tests-commands.sh`:
- Line 28: The FOCUS_FILE_ARG currently injects raw CHANGED_TEST_FILES into a
regex which fails to escape regex metacharacters (e.g., the dot in _test.go);
update the code that constructs FOCUS_FILE_ARG so it first escapes regex special
characters in CHANGED_TEST_FILES (escape characters like .[]^$*+?(){}| and
backslash) and then builds the focus-file expression from the escaped names;
locate the place that sets FOCUS_FILE_ARG and replace the raw injection with a
pipeline that escapes each filename (preserving the intended grouping) before
wrapping with "--focus-file=(...)" so filenames are matched literally.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 020d0cf1-9fdc-4d06-95af-17fad832ab76
⛔ Files ignored due to path filters (2)
ci-operator/jobs/netobserv/netobserv-operator/netobserv-netobserv-operator-main-postsubmits.yamlis excluded by!ci-operator/jobs/**ci-operator/jobs/netobserv/netobserv-operator/netobserv-netobserv-operator-main-presubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (5)
ci-operator/config/netobserv/netobserv-operator/netobserv-netobserv-operator-main.yamlci-operator/step-registry/netobserv/ginkgo-tests/OWNERSci-operator/step-registry/netobserv/ginkgo-tests/netobserv-ginkgo-tests-commands.shci-operator/step-registry/netobserv/ginkgo-tests/netobserv-ginkgo-tests-ref.metadata.jsonci-operator/step-registry/netobserv/ginkgo-tests/netobserv-ginkgo-tests-ref.yaml
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
ci-operator/config/netobserv/netobserv-operator/netobserv-netobserv-operator-main.yaml (2)
242-245: 💤 Low valueConsider adding a fallback for git diff on shallow clones.
The
git diff --name-only HEAD~1command assumes a parent commit exists. While ci-operator typically provides sufficient git history, edge cases (shallow clones, initial commits, or single-commit PRs) could cause this to fail silently or with an error.🛡️ Proposed defensive fallback
- as: compute-changed-tests commands: | - git diff --name-only HEAD~1 -- integration-tests/backend/ > "${SHARED_DIR}/changed_files" + git diff --name-only HEAD~1 -- integration-tests/backend/ > "${SHARED_DIR}/changed_files" 2>/dev/null || \ + find integration-tests/backend/ -type f \( -name '*.go' -o -name '*.yaml' \) > "${SHARED_DIR}/changed_files" echo "Changed files:" cat "${SHARED_DIR}/changed_files"This falls back to listing all backend test files if
git difffails.🤖 Prompt for 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. In `@ci-operator/config/netobserv/netobserv-operator/netobserv-netobserv-operator-main.yaml` around lines 242 - 245, The git diff command ("git diff --name-only HEAD~1 -- integration-tests/backend/") can fail on shallow or single-commit clones; modify the commands block to run the git diff into "${SHARED_DIR}/changed_files" and if it exits non-zero or produces no output then populate "${SHARED_DIR}/changed_files" with a safe fallback (e.g., run "git ls-files integration-tests/backend/" or "find integration-tests/backend -type f" to list all backend test files), and then continue to echo and cat "${SHARED_DIR}/changed_files"; ensure the fallback is used only when git diff fails or yields empty output.
211-211: ⚡ Quick winPin yamlfmt to a specific version for reproducibility.
Installing
yamlfmt@latestcan lead to non-reproducible builds when the tool is updated. Pin to a specific version to ensure consistent CI behavior across time.📌 Proposed fix to pin yamlfmt version
- go install github.com/google/yamlfmt/cmd/yamlfmt@latest + go install github.com/google/yamlfmt/cmd/yamlfmt@v0.13.0🤖 Prompt for 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. In `@ci-operator/config/netobserv/netobserv-operator/netobserv-netobserv-operator-main.yaml` at line 211, Replace the free-floating "go install github.com/google/yamlfmt/cmd/yamlfmt@latest" invocation with a pinned release by replacing `@latest` with a specific tag (for example `@v0.11.0`) so CI is reproducible; update the line that contains "go install github.com/google/yamlfmt/cmd/yamlfmt@latest" to use the chosen semver tag and, if applicable, add a short comment noting the pinned version and rationale so future maintainers know to update the tag intentionally.
🤖 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
`@ci-operator/config/netobserv/netobserv-operator/netobserv-netobserv-operator-main.yaml`:
- Around line 29-49: The Dockerfile builder stage currently installs Ginkgo
non-deterministically using "go install
github.com/onsi/ginkgo/v2/ginkgo@latest"; update that RUN install command in the
builder stage to pin a specific release (e.g., change to "go install
github.com/onsi/ginkgo/v2/ginkgo@v2.28.1") so the e2e build is reproducible and
consistent across runs.
---
Nitpick comments:
In
`@ci-operator/config/netobserv/netobserv-operator/netobserv-netobserv-operator-main.yaml`:
- Around line 242-245: The git diff command ("git diff --name-only HEAD~1 --
integration-tests/backend/") can fail on shallow or single-commit clones; modify
the commands block to run the git diff into "${SHARED_DIR}/changed_files" and if
it exits non-zero or produces no output then populate
"${SHARED_DIR}/changed_files" with a safe fallback (e.g., run "git ls-files
integration-tests/backend/" or "find integration-tests/backend -type f" to list
all backend test files), and then continue to echo and cat
"${SHARED_DIR}/changed_files"; ensure the fallback is used only when git diff
fails or yields empty output.
- Line 211: Replace the free-floating "go install
github.com/google/yamlfmt/cmd/yamlfmt@latest" invocation with a pinned release
by replacing `@latest` with a specific tag (for example `@v0.11.0`) so CI is
reproducible; update the line that contains "go install
github.com/google/yamlfmt/cmd/yamlfmt@latest" to use the chosen semver tag and,
if applicable, add a short comment noting the pinned version and rationale so
future maintainers know to update the tag intentionally.
🪄 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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 4c2a58ca-2fd1-41f7-974d-fde1396c4f03
⛔ Files ignored due to path filters (1)
ci-operator/jobs/netobserv/netobserv-operator/netobserv-netobserv-operator-main-presubmits.yamlis excluded by!ci-operator/jobs/**
📒 Files selected for processing (1)
ci-operator/config/netobserv/netobserv-operator/netobserv-netobserv-operator-main.yaml
| - dockerfile_literal: | | ||
| FROM docker.io/library/golang:1.25 AS builder | ||
| ARG TARGETARCH=amd64 | ||
| WORKDIR /workspace | ||
| COPY go.mod go.mod | ||
| COPY go.sum go.sum | ||
| RUN go mod download | ||
| RUN go install github.com/onsi/ginkgo/v2/ginkgo@latest | ||
| COPY *.go ./ | ||
| COPY testdata/ testdata/ | ||
| RUN GOGC=50 GOMAXPROCS=4 GOOS=linux GOARCH=$TARGETARCH \ | ||
| ginkgo build --ldflags="-s -w" -o e2e-tests.test | ||
| FROM registry.access.redhat.com/ubi9/ubi-minimal:9.8-1777460003 | ||
| WORKDIR /tests | ||
| COPY --from=builder /workspace/e2e-tests.test . | ||
| COPY --from=builder /workspace/testdata/ testdata/ | ||
| COPY --from=builder /go/bin/ginkgo /usr/local/bin/ginkgo | ||
| USER 65532:65532 | ||
| ENTRYPOINT ["ginkgo", "run", "./e2e-tests.test"] | ||
| from: ubi | ||
| to: e2etests |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check latest stable Go version from official registry
curl -s https://go.dev/dl/?mode=json | jq -r '.[0].version'
# Also check what golang images are available on Docker Hub
curl -s "https://hub.docker.com/v2/repositories/library/golang/tags?page_size=100" | jq -r '.results[].name' | grep -E '^1\.[0-9]+$' | sort -V | tail -5Repository: openshift/release
Length of output: 80
🌐 Web query:
What is the latest stable version of Go (golang) as of June 2026?
💡 Result:
As of June 3, 2026, the latest stable version of Go (golang) is 1.26.4, which was released on June 2, 2026 [1]. This release includes security fixes for the crypto/x509, mime, and net/textproto packages, as well as several bug fixes [1].
Citations:
🌐 Web query:
github.com/onsi/ginkgo/v2 latest release tag
💡 Result:
The latest release tag for the Ginkgo v2 repository (github.com/onsi/ginkgo) is v2.28.1 [1][2][3]. This version was released on January 30, 2026 [2].
Citations:
- 1: https://github.com/onsi/ginkgo/tree/refs/heads/master
- 2: https://github.com/onsi/ginkgo/releases/tag/v2.28.1
- 3: https://github.com/onsi/ginkgo/tree/ver2
Pin Ginkgo instead of using @latest in the e2etests build
golang:1.25is available on Docker Hub, so it won’t inherently break the build (current Go stable is 1.26.4); consider bumping for freshness but it’s not a validity failure.go install github.com/onsi/ginkgo/v2/ginkgo@latest(line ~36) is non-reproducible; pin togithub.com/onsi/ginkgo/v2/ginkgo@v2.28.1.
🤖 Prompt for 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.
In
`@ci-operator/config/netobserv/netobserv-operator/netobserv-netobserv-operator-main.yaml`
around lines 29 - 49, The Dockerfile builder stage currently installs Ginkgo
non-deterministically using "go install
github.com/onsi/ginkgo/v2/ginkgo@latest"; update that RUN install command in the
builder stage to pin a specific release (e.g., change to "go install
github.com/onsi/ginkgo/v2/ginkgo@v2.28.1") so the e2e build is reproducible and
consistent across runs.
|
/pj-rehearse pull-ci-netobserv-netobserv-operator-main-e2etest |
|
@memodi: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse pull-ci-netobserv-netobserv-operator-main-e2etests-lint-fmt |
|
@memodi: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse pull-ci-netobserv-netobserv-operator-main-e2etest |
|
@memodi: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse pull-ci-netobserv-netobserv-operator-main-e2etests-lint-fmt |
|
@memodi: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse pull-ci-netobserv-netobserv-operator-main-e2etests-lint-fmt |
|
@memodi: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
/pj-rehearse pull-ci-netobserv-netobserv-operator-main-e2etest |
|
@memodi: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
[REHEARSALNOTIFIER]
Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
@memodi: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
1 participant
NETOBSERV-2748 e2etests images and pre-merge tests
Summary by CodeRabbit
This PR adds targeted e2e test support and CI steps for netobserv-operator in OpenShift CI (refs NETOBSERV-2748).
What changed practically
Why this matters
Files of interest