OCPBUGS-104543: fix test isolation bug in EnsureDefaultSecurityGroupTagsTest - #9228
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@ironcladlou: This pull request references Jira Issue OCPBUGS-104543, 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. |
📝 WalkthroughWalkthroughThe AWS hosted cluster end-to-end test now validates tag cleanup. After restoring the original AWS resource tags, it retrieves the HostedCluster Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (10 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9228 +/- ##
==========================================
+ Coverage 44.95% 44.96% +0.01%
==========================================
Files 778 778
Lines 97434 97452 +18
==========================================
+ Hits 43797 43820 +23
+ Misses 50615 50607 -8
- Partials 3022 3025 +3 see 2 files with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
/jira refresh |
|
@ironcladlou: This pull request references Jira Issue OCPBUGS-104543, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
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. |
|
/pipeline required |
|
Scheduling tests matching the |
|
/test e2e-v2-azure-self-managed |
|
Pretty sure the issue is not limited to the cleanup, it's also an issue with the test reading stale hostedcluster state, which I'm addressing systemically in #9229 |
Before this commit, `EnsureDefaultSecurityGroupTagsTest()` mutated the cluster but used a fire-and-forget deferred cleanup, causing downstream tests which observe the same state to sometimes see the polluted state, leading to failed assertions (e.g. `EnsureInfrastructureResourceTagsTest()`). The test also read cached hostedcluster state from the test context which is mutated by other tests. That issue is systemic. This commit updates the deferred cleanup method to await convergence of the mutation rollback to make the test more fully isolated. It also uses a targeted fix for the cached hostedcluster lookup by fetching a fresh instance for assertions.
76e3c25 to
ca12889
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
@ironcladlou: This pull request references Jira Issue OCPBUGS-104543, which is valid. 3 validation(s) were run on this bug
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. |
|
/test e2e-v2-aws |
|
With the latest commit I was unable to reproduce the issue with 10 consecutive e2e v2 aws runs locally /pipeline required |
|
Scheduling tests matching the |
|
Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bryan-cox, ironcladlou 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 |
Test Resultse2e-aws
e2e-aks
|
|
/retest |
|
/verified by e2e regression analysis and manual testing executed full v2 e2e aws workflow (create-guests / run-tests) 10x against with no errors |
|
@ironcladlou: This PR has been marked as verified 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. |
|
/test e2e-kubevirt-aws-ovn-reduced |
|
@ironcladlou: 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. |
|
@ironcladlou: Jira Issue Verification Checks: Jira Issue OCPBUGS-104543 Jira Issue OCPBUGS-104543 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓 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. |
|
Fix included in release 5.0.0-0.nightly-2026-08-07-105601 |
Before this commit,
EnsureDefaultSecurityGroupTagsTest()mutated the cluster but used a fire-and-forget deferred cleanup, causing downstream tests which observe the same state to sometimes see the polluted state, leading to failed assertions (e.g.EnsureInfrastructureResourceTagsTest()).The test also read cached hostedcluster state from the test context which is mutated by other tests. That issue is systemic.
This commit updates the deferred cleanup method to await convergence of the mutation rollback to make the test more fully isolated. It also uses a targeted fix for the cached hostedcluster lookup by fetching a fresh instance for assertions.
Summary by CodeRabbit