*DO NOT MERGE*: Test GHA builds suppression#15351
Conversation
|
Skipping CI for Draft Pull Request. |
|
Images are ready for the commit at a34e5a5. To use with deploy scripts, first |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #15351 +/- ##
==========================================
+ Coverage 49.21% 49.23% +0.02%
==========================================
Files 2573 2573
Lines 188848 188854 +6
==========================================
+ Hits 92937 92982 +45
+ Misses 88583 88551 -32
+ Partials 7328 7321 -7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
d53b86d to
c10654b
Compare
9cac587 to
cd3ed76
Compare
|
/cancel |
2bce533 to
3ffd510
Compare
cd3ed76 to
94fda96
Compare
|
/cancel |
6fd3af6 to
14c2a59
Compare
94fda96 to
b0141e1
Compare
|
/test ? |
|
@msugakov: No presubmit jobs available for stackrox/stackrox@misha/ROX-27716-take-konflux-on-release DetailsIn response to this:
Instructions 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. |
|
/test ? |
This comment was marked as outdated.
This comment was marked as outdated.
|
/test gke-nongroovy-e2e-tests gke-qa-e2e-tests |
|
Expectedly, the e2e logs are full of because Konflux images have |
build in favor of Konflux build.
to get Konflux image tag suffix suppression
When there's only one matrix item (RHACS_BRANDING) and when it gets excluded (with `exclude`), the corresponding job still runs but in no-matrix mode. It's weird but that's the way it works. `build-and-push-operator` seems to be the only job like this and for it the default branding STACKROX_BRANDING gets assumed which results in building images tagged for quay.io/stackrox-io. Subsequently, any job steps that try to work with quay.io/rhacs-eng images fail and so I disable them with an additional clause in the `if` condition.
because the branch name should do the magic.
b0141e1 to
3ca3d21
Compare
There was a problem hiding this comment.
Hey @msugakov - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| if grep -qE '^((refs/heads/)?release-[0-9a-z]+\.[0-9a-z]+|refs/tags/[0-9]+\.[0-9]+\.[0-9]+(-rc\.[0-9]+)?)$' <<< "${the_ref}"; then | ||
| log "This looks like a release branch or tag push. GHA quay.io/rhacs-eng/* builds must be suppressed in favor of the Konflux ones." | ||
| if [[ -f "${holdfile}" ]]; then |
There was a problem hiding this comment.
suggestion: Temporary holdfile logic should be tracked for removal.
Define a removal plan for TODO ROX-29357 and eliminate the holdfile logic once testing completes to prevent confusion or unexpected behavior.
| assert grep -F "$1" <<< "${stderr_lines[@]}" | ||
| } | ||
|
|
||
| @test "should fail when required values are not set" { |
There was a problem hiding this comment.
issue (complexity): Consider refactoring the repetitive test cases into table-driven loops with a helper function to reduce duplication.
You can collapse all of these almost-identical @test blocks into just a few loops + a small helper. For example, pull your per-test env setup + “which checker” into a table, then iterate:
#!/usr/bin/env bats
load "../test_helpers.bats"
# common setup / runner / checkers...
setup() {
unset SOURCE_BRANCH TARGET_BRANCH GITHUB_REF GITHUB_BASE_REF GITHUB_HEAD_REF
bats_require_minimum_version 1.5.0
}
run_cmd() {
cp -a "${BATS_TEST_DIRNAME}/should-konflux-replace-gha-build.sh" \
"${BATS_TEST_TMPDIR}/our-script.sh"
run --separate-stderr "${BATS_TEST_TMPDIR}/our-script.sh"
}
check_both_go() {
run_cmd; assert_success; assert_output "BUILD_AND_PUSH_BOTH"
assert_stderr_contains "does not look like" "release" "branch or tag"
}
check_gha_suppressed() {
run_cmd; assert_success; assert_output "BUILD_AND_PUSH_ONLY_KONFLUX"
assert_stderr_contains "looks like" "release" "branch or tag"
}
check_gha_suppressed_for_pr() {
run_cmd; assert_success; assert_output "BUILD_AND_PUSH_ONLY_KONFLUX"
assert_stderr_contains "magic"
}
# generic table‐driven runner
run_scenario() {
local envs="$1"; local fn="$2"
for e in $envs; do export $e; done
$fn
}
#–– Konflux tests ––
declare -a KONFLUX_SCENARIOS=(
"rc tag pushed|SOURCE_BRANCH=refs/tags/4.10.56-rc.172 TARGET_BRANCH=refs/tags/4.10.56-rc.172|check_gha_suppressed"
"nightly tag|SOURCE_BRANCH=refs/tags/4.10.56-nightly.20250515 TARGET_BRANCH=refs/tags/4.10.56-nightly.20250515|check_both_go"
"release-like branch|SOURCE_BRANCH=release-4.8 TARGET_BRANCH=release-4.8|check_gha_suppressed"
"feature branch|SOURCE_BRANCH=author/ROX-27716-useful-feature TARGET_BRANCH=author/ROX-27716-useful-feature|check_both_go"
"magic‐PR|SOURCE_BRANCH=author/konflux-release-like TARGET_BRANCH=master|check_gha_suppressed_for_pr"
"normal‐PR|SOURCE_BRANCH=author/my-useful-feature TARGET_BRANCH=master|check_both_go"
"PR→release target|SOURCE_BRANCH=author/my-useful-feature TARGET_BRANCH=release-4.8|check_gha_suppressed"
)
for scenario in "${KONFLUX_SCENARIOS[@]}"; do
IFS='|' read -r desc envs fn <<<"$scenario"
@test "Konflux: $desc" {
run_scenario "$envs" "$fn"
}
done
#–– GHA tests ––
declare -a GHA_SCENARIOS=(
"release tag|GITHUB_REF=refs/tags/24.58.60|check_gha_suppressed"
"other tag|GITHUB_REF=refs/tags/0.0.0-author-testing|check_both_go"
"release-like branch|GITHUB_REF=refs/heads/release-x.y|check_gha_suppressed"
"feature branch|GITHUB_REF=refs/heads/many-funky/...|check_both_go"
)
for scenario in "${GHA_SCENARIOS[@]}"; do
IFS='|' read -r desc envs fn <<<"$scenario"
@test "GHA: $desc" {
run_scenario "$envs" "$fn"
}
done
# a couple of special‐case/tests can stay explicit (failure‐modes, holdfile)
@test "should fail when required values are not set" {
run_cmd; assert_failure 2
}
@test "GHA: fail on PR without BASE/HEAD" {
export GITHUB_REF=refs/pull/1005006/merge
run_cmd; assert_failure 3
}
@test "should respect holdfile when release push" {
export SOURCE_BRANCH=release-4.8 TARGET_BRANCH=release-4.8
touch "${BATS_TEST_TMPDIR}/should-konflux-replace-gha-build.hold"
run_cmd; assert_success; assert_output "BUILD_AND_PUSH_BOTH"
assert_stderr_contains "holdfile"
}
@test "should ignore holdfile when PR with magic branch" {
export SOURCE_BRANCH=author/konflux-release-like TARGET_BRANCH=master
touch "${BATS_TEST_TMPDIR}/should-konflux-replace-gha-build.hold"
check_gha_suppressed_for_pr
}This preserves every single assertion, but collapses ~20 nearly-identical @test blocks down to 2 arrays + a tiny runner.
|
/retest operator-bundle-on-push |
|
/cancel |
|
/retest central-db-on-push |
|
/retest operator-on-push |
|
/retest operator-bundle-on-push |
|
/retest operator-on-push |
|
/test gke-scanner-v4-install-tests gke-nongroovy-e2e-tests gke-upgrade-tests gke-qa-e2e-tests gke-operator-e2e-tests ocp-4-12-scanner-v4-install-tests ocp-4-12-operator-e2e-tests ocp-4-12-qa-e2e-tests ocp-4-12-nongroovy-e2e-tests ocp-4-18-operator-e2e-tests ocp-4-18-scanner-v4-install-tests ocp-4-18-nongroovy-e2e-tests ocp-4-18-qa-e2e-tests |
|
@msugakov: 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. |
|
I need to close this because I'm losing track of things. |
On top of #15309 / testing for stackrox/konflux-tasks#55
The PR is marked as "Ready for review" but it isn't. I just want OSCI to start automatically without me having to type commands.