OCPBUGS-77848: [CI] Fix the VolumeSnapshot CRUD flaky test failures#16110
Conversation
openshift-ci
bot
added
kind/cypress
cajieh
changed the title
openshift-ci
bot
added
the
do-not-merge/work-in-progress
openshift-ci-robot
added
jira/valid-reference
|
@cajieh: This pull request references Jira Issue OCPBUGS-77848, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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 openshift-eng/jira-lifecycle-plugin repository. |
📝 WalkthroughWalkthroughThis change refactors the Kubernetes resource CRUD test suite to conditionally include VolumeSnapshot-related resources based on the BRIDGE_AWS environment variable. Previously, VolumeSnapshot objects were unconditionally added to the test dataset. The update creates a separate 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/jira refresh |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/packages/integration-tests-cypress/tests/crud/resource-crud.cy.ts`:
- Around line 63-81: The BRIDGE_AWS env check is using Cypress.env('BRIDGE_AWS')
directly, which treats the string "false" as truthy and enables snapshot tests;
update the check used to build snapshotObjs to explicitly normalize the env
value (e.g., treat only the string 'true' as enabled or parse to a boolean) so
snapshotObjs is only populated when BRIDGE_AWS is truly enabled, and ensure the
same normalization is applied where BRIDGE_AWS is read in snapshot.cy.ts and
clone.cy.ts; adjust the conditional that creates snapshotObjs and any merges
into k8sObjsWithSnapshots accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 42aab9e8-0a8f-4c2e-9961-4aa855c28917
📒 Files selected for processing (1)
frontend/packages/integration-tests-cypress/tests/crud/resource-crud.cy.ts
| // VolumeSnapshot tests require CSI-based storage classes which are only reliably available on AWS. | ||
| // These resources are thoroughly tested in tests/storage/snapshot.cy.ts on AWS platforms. | ||
| // Skip them here to avoid flaky failures on GCP and other platforms. | ||
| const snapshotObjs = Cypress.env('BRIDGE_AWS') | ||
| ? OrderedMap<string, TestDefinition>() | ||
| .set('snapshot.storage.k8s.io~v1~VolumeSnapshot', { | ||
| kind: 'snapshot.storage.k8s.io~v1~VolumeSnapshot', | ||
| }) | ||
| .set('snapshot.storage.k8s.io~v1~VolumeSnapshotClass', { | ||
| kind: 'snapshot.storage.k8s.io~v1~VolumeSnapshotClass', | ||
| namespaced: false, | ||
| }) | ||
| .set('snapshot.storage.k8s.io~v1~VolumeSnapshotContent', { | ||
| kind: 'snapshot.storage.k8s.io~v1~VolumeSnapshotContent', | ||
| namespaced: false, | ||
| }) | ||
| : OrderedMap<string, TestDefinition>(); | ||
|
|
||
| const k8sObjsWithSnapshots = k8sObjs.merge(snapshotObjs); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for BRIDGE_AWS definition and usage across the entire codebase
rg -n "BRIDGE_AWS" --type ts --type js --type json --type yamlRepository: openshift/console
Length of output: 402
🏁 Script executed:
# Also search for Cypress config and environment setup files
fd -e "cypress" -e "conf" -o "cypress.config" -o ".env" | head -20
git ls-files | grep -i cypress | grep -i configRepository: openshift/console
Length of output: 1239
🏁 Script executed:
# Check how Cypress.env is used with boolean-like values in tests
rg -n "Cypress\.env\(" --type ts --type js -A2 -B2 | head -100Repository: openshift/console
Length of output: 12209
🏁 Script executed:
# Check the main cypress config file for how BRIDGE_AWS is configured
cat -n frontend/packages/integration-tests-cypress/cypress.config.jsRepository: openshift/console
Length of output: 1745
🏁 Script executed:
# Also check if there are any CI pipeline configs that set BRIDGE_AWS
git ls-files | grep -E "\.(yml|yaml|groovy|json)$" | xargs rg "BRIDGE_AWS" -lRepository: openshift/console
Length of output: 45
🏁 Script executed:
# Check the snapshot.cy.ts and clone.cy.ts files to see context around BRIDGE_AWS checks
cat -n frontend/packages/integration-tests-cypress/tests/storage/snapshot.cy.ts | head -30
cat -n frontend/packages/integration-tests-cypress/tests/storage/clone.cy.ts | head -30Repository: openshift/console
Length of output: 3073
🏁 Script executed:
# Check the plugins/index.js file to see how BRIDGE_AWS is parsed
cat -n frontend/packages/integration-tests-cypress/plugins/index.js | head -100Repository: openshift/console
Length of output: 2964
🏁 Script executed:
# Also search for any CI configuration files that might set BRIDGE_AWS
git ls-files | xargs rg -l "BRIDGE_AWS" | grep -E "(\.yml|\.yaml|\.groovy|makefile|Makefile|\.sh|script)"Repository: openshift/console
Length of output: 43
🏁 Script executed:
# Search for where BRIDGE_AWS is actually set in the plugins or config
cat -n frontend/packages/integration-tests-cypress/plugins/index.js | tail -20Repository: openshift/console
Length of output: 1084
🏁 Script executed:
# Search more broadly for BRIDGE_AWS assignment or setup
rg -n "BRIDGE_AWS\s*=" --type js --type tsRepository: openshift/console
Length of output: 43
🏁 Script executed:
# Look at package.json scripts to see how tests are run and env vars passed
cat -n frontend/packages/integration-tests-cypress/package.json | grep -A5 -B5 "BRIDGE_AWS\|cypress"Repository: openshift/console
Length of output: 284
🏁 Script executed:
# Search across all files for where BRIDGE_AWS is configured
rg -r "config\.env\.BRIDGE_AWS\|process\.env\.BRIDGE_AWS" --type js --type tsRepository: openshift/console
Length of output: 457
🏁 Script executed:
# Check if BRIDGE_AWS is assigned in the plugin more carefully
cat -n frontend/packages/integration-tests-cypress/plugins/index.jsRepository: openshift/console
Length of output: 2964
🏁 Script executed:
# Search for any reference to BRIDGE_AWS in CI or setup scripts
git ls-files | xargs grep -l "BRIDGE_AWS" 2>/dev/nullRepository: openshift/console
Length of output: 280
🏁 Script executed:
# Double-check by examining how boolean env vars are typically handled in Cypress
rg -n "process\.env\." frontend/packages/integration-tests-cypress/plugins/index.js -A1Repository: openshift/console
Length of output: 682
🏁 Script executed:
# Verify Cypress behavior with string environment variables
cd frontend/packages/integration-tests-cypress && cat -n support/index.ts | head -50Repository: openshift/console
Length of output: 1704
🏁 Script executed:
# Verify if BRIDGE_AWS is actually missing from the config assignment
grep -n "config.env.BRIDGE_AWS" frontend/packages/integration-tests-cypress/plugins/index.jsRepository: openshift/console
Length of output: 43
🏁 Script executed:
# Check the resource-crud.cy.ts file around line 66 to see the full context
sed -n '60,85p' frontend/packages/integration-tests-cypress/tests/crud/resource-crud.cy.tsRepository: openshift/console
Length of output: 1261
🏁 Script executed:
# Check how Cypress.env works - does it fall back to process.env if not in config?
rg -n "Cypress\.env\(" frontend/packages/integration-tests-cypress/support/index.ts -A2 -B2Repository: openshift/console
Length of output: 43
🏁 Script executed:
# Look for where BRIDGE_AWS might be passed - check for any env loading logic
find frontend/packages/integration-tests-cypress -name "*.ts" -o -name "*.js" | xargs grep -l "BRIDGE_AWS" | xargs cat | grep -B5 -A5 "BRIDGE_AWS"Repository: openshift/console
Length of output: 1431
🏁 Script executed:
# Check if there are any other plugin files or setup that might handle BRIDGE_AWS
find frontend/packages/integration-tests-cypress -type f \( -name "*.js" -o -name "*.ts" \) ! -path "*/node_modules/*" | xargs grep -l "BRIDGE_AWS\|config\.env" | head -10Repository: openshift/console
Length of output: 339
🏁 Script executed:
# Look for CI configuration or test runner setup that might export BRIDGE_AWS
git ls-files | xargs grep -l "BRIDGE_AWS" | xargs cat | head -100Repository: openshift/console
Length of output: 3909
🏁 Script executed:
# Search for how BRIDGE_AWS is actually passed or configured - maybe it's in environment directly
rg -n "env\|ENV" frontend/packages/integration-tests-cypress/cypress.config.jsRepository: openshift/console
Length of output: 43
🏁 Script executed:
# Check if there's any Makefile or test runner that sets environment variables
git ls-files | grep -iE "(makefile|makefile\..*|test\.sh|run\.sh|\.github|\.gitlab)" | head -20Repository: openshift/console
Length of output: 1021
🏁 Script executed:
# Look for any GitHub Actions or CI config that might set BRIDGE_AWS
git ls-files | grep -E "\.github|\.gitlab|\.circleci" | head -20Repository: openshift/console
Length of output: 345
🏁 Script executed:
# Check what Cypress.env actually returns - look for any type handling
rg -n "Cypress\.env" frontend/packages/integration-tests-cypress --type ts --type js -B3 -A3 | grep -A3 -B3 "BRIDGE_AWS\|OPENSHIFT_CI\|typeof"Repository: openshift/console
Length of output: 3384
🏁 Script executed:
# Look at the test.sh to see how BRIDGE_AWS might be passed
cat -n test.sh | grep -i "bridge\|aws\|cypress" -A2 -B2Repository: openshift/console
Length of output: 43
🏁 Script executed:
# Check if Cypress.env falls back to process.env for variables not in config
rg -n "typeof.*Cypress\.env\|String.*Cypress\.env" frontend/packages/integration-tests-cypress --type ts --type jsRepository: openshift/console
Length of output: 43
🏁 Script executed:
# Look for documentation or comments on how BRIDGE_AWS is supposed to be set
rg -n "BRIDGE_AWS" frontend/packages/integration-tests-cypress -B5 -A5 | head -60Repository: openshift/console
Length of output: 3926
🏁 Script executed:
# Verify Cypress behavior with string env variables - check if Cypress.env() falls back to process.env
rg -n "config\.env\[|config\.env\." frontend/packages/integration-tests-cypress/plugins/index.js | wc -lRepository: openshift/console
Length of output: 62
🏁 Script executed:
# Count how many env vars are explicitly assigned vs which are missing
cat -n frontend/packages/integration-tests-cypress/plugins/index.js | sed -n '65,71p'Repository: openshift/console
Length of output: 550
🏁 Script executed:
# Create a simple test to understand: does BRIDGE_AWS appear in any CI/documentation?
rg -r "BRIDGE_AWS" --type md --type txt 2>/dev/null | head -20Repository: openshift/console
Length of output: 43
Normalize BRIDGE_AWS env check to prevent snapshot tests on non-AWS platforms.
Truthy checks on line 66 and similar locations treat the string "false" as enabled, which reintroduces flaky snapshot tests on GCP/non-AWS runs. Environment variables from CI are always strings, so Cypress.env('BRIDGE_AWS') evaluates to true even when set to "false".
Suggested fix
- const snapshotObjs = Cypress.env('BRIDGE_AWS')
+ const isAws = String(Cypress.env('BRIDGE_AWS')).toLowerCase() === 'true';
+ const snapshotObjs = isAws
? OrderedMap<string, TestDefinition>()
.set('snapshot.storage.k8s.io~v1~VolumeSnapshot', {
kind: 'snapshot.storage.k8s.io~v1~VolumeSnapshot',
})
.set('snapshot.storage.k8s.io~v1~VolumeSnapshotClass', {
kind: 'snapshot.storage.k8s.io~v1~VolumeSnapshotClass',
namespaced: false,
})
.set('snapshot.storage.k8s.io~v1~VolumeSnapshotContent', {
kind: 'snapshot.storage.k8s.io~v1~VolumeSnapshotContent',
namespaced: false,
})
: OrderedMap<string, TestDefinition>();Also apply the same normalization to snapshot.cy.ts line 13 and clone.cy.ts line 22.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/packages/integration-tests-cypress/tests/crud/resource-crud.cy.ts`
around lines 63 - 81, The BRIDGE_AWS env check is using
Cypress.env('BRIDGE_AWS') directly, which treats the string "false" as truthy
and enables snapshot tests; update the check used to build snapshotObjs to
explicitly normalize the env value (e.g., treat only the string 'true' as
enabled or parse to a boolean) so snapshotObjs is only populated when BRIDGE_AWS
is truly enabled, and ensure the same normalization is applied where BRIDGE_AWS
is read in snapshot.cy.ts and clone.cy.ts; adjust the conditional that creates
snapshotObjs and any merges into k8sObjsWithSnapshots accordingly.
|
@cajieh: This pull request references Jira Issue OCPBUGS-77848, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: 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 openshift-eng/jira-lifecycle-plugin repository. |
cajieh
force-pushed
the
fix-ci-flaky-tests
branch
from
db676e2 to
12741df
Compare
| }; | ||
|
|
||
| if (Cypress.env('BRIDGE_AWS')) { | ||
| // Normalize env check: CI env vars are strings, so "false" would be truthy without explicit comparison. |
|
Looks like we will need to backport this one |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cajieh, jhadvig The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
cajieh
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
cajieh
changed the title
|
/retest |
|
/retest e2e-gcp-console |
|
/test e2e-gcp-console |
1 similar comment
|
/test e2e-gcp-console |
|
c'mon |
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. |
|
Overriding to unblock the merge queue |
|
/verified later @yapei |
openshift-ci-robot
added
verified-later
verified
|
@jhadvig: This PR has been marked to be verified later by 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/retest |
2 similar comments
|
/retest |
|
/retest |
|
/test analyze backend images okd-scos-images frontend |
|
@cajieh: all tests passed! 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. |
|
@cajieh: Jira Issue OCPBUGS-77848: All pull requests linked via external trackers have merged: This pull request has the 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/cherry-pick release-4.21 |
|
@cajieh: new pull request created: #16118 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. |
5 participants
This change aims to resolve the VolumeSnapshot CRUD flaky test failures by addressing the following:
🤞
Summary by CodeRabbit