CNTRLPLANE-3515: test(e2e): add day-2 label/taint no-rollout verification#8595
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@mgencur: This pull request references CNTRLPLANE-3515 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "5.0.0" version, but no target version was set. 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. |
openshift-ci
Bot
added
do-not-merge/needs-area
area/testing
|
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)
📝 WalkthroughWalkthroughThis PR updates the nodepool_day2_tags e2e test: it adds the k8s sets import, captures the MachineDeployment.Generation before applying changes, applies EC2 resource tag updates plus node label and taint changes, polls NodePool status while tracking updating-related condition types (using a set and a rollingUpdateTriggered flag), asserts rollingUpdateTriggered is false, and re-fetches the MachineDeployment to verify its Generation is unchanged. Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (13 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8595 +/- ##
=======================================
Coverage 41.49% 41.49%
=======================================
Files 756 756
Lines 93648 93648
=======================================
Hits 38855 38855
Misses 52057 52057
Partials 2736 2736
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@test/e2e/nodepool_day2_tags_test.go`:
- Around line 79-88: The test currently replaces NodePool.Spec.NodeLabels and
NodePool.Spec.Taints outright; instead modify the test to merge the day-2 values
into existing fields: ensure NodePool.Spec.NodeLabels is non-nil (create if nil)
and set the "e2e.day2.validation" key to "true" without dropping other keys, and
for NodePool.Spec.Taints append the new hyperv1.Taint (Key:"e2e-day2",
Value:"test", Effect:corev1.TaintEffectPreferNoSchedule) only if an equivalent
taint (same Key, Value, Effect) does not already exist, so existing taints are
preserved rather than overwritten.
- Around line 153-154: The MachineDeployment generation assertion is hardcoded
to 1; instead capture the pre-update generation (e.g., store md.Generation in a
variable like initialGeneration or preUpdateGen before performing the day-2
tag/label/taint updates) and then assert that md.Generation is still equal to
that stored value after the update; update the test in
test/e2e/nodepool_day2_tags_test.go to use the stored pre-update generation for
the final assertion against md.Generation.
🪄 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: 9ed09c00-4205-40e5-b9d2-1c5734b44f66
📒 Files selected for processing (1)
test/e2e/nodepool_day2_tags_test.go
mgencur
force-pushed
the
node_labels_and_taints
branch
from
9ce5563 to
4f8b5f2
Compare
mgencur
force-pushed
the
node_labels_and_taints
branch
from
4f8b5f2 to
b5d3554
Compare
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mgencur, muraee 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 |
openshift-ci
Bot
added
the
approved
|
/lgtm |
|
Scheduling tests matching the |
Test Resultse2e-aws
e2e-aks
|
|
/retest |
5 similar comments
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
/retest |
Extend NodePoolDay2TagsTest to patch nodeLabels and taints day-2 and verify no rolling upgrade is triggered via NodePool conditions and MachineDeployment generation. Ref: CNTRLPLANE-3515 Co-Authored-By: Claude Opus 4.6 <[email protected]>
mgencur
force-pushed
the
node_labels_and_taints
branch
from
b5d3554 to
26c6325
Compare
|
Rebasing because Konflux enterprise contract checks were constantly failing. |
|
/lgtm |
|
Scheduling tests matching the |
|
/retest |
4 similar comments
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
I now have complete evidence. All 4 distinct failures occurred within a narrow time window (~10:01–10:04 UTC) when the management cluster's API server became unstable. The PR only modifies Test Failure Analysis CompleteJob Information
Test Failure AnalysisErrorSummaryAll 4 test failures are caused by a transient management cluster API server outage that occurred around 10:01–10:04 UTC, impacting every test that was communicating with the management cluster at that moment. The management cluster kube-apiserver (ELB Root CauseTransient management cluster API server outage (infrastructure flake, unrelated to PR #8595). The management cluster's kube-apiserver became unreachable starting around 10:01 UTC. The evidence shows a progression of connectivity failures:
The The PR's test ( Recommendations
Evidence
|
|
/verified by E2E This PR improves an E2E test and it passes |
openshift-ci-robot
added
the
verified
|
@mgencur: 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. |
openshift-merge-bot
Bot
merged commit cfa8fc6
into
openshift:main
5 participants
What this PR does / why we need it:
Extend NodePoolDay2TagsTest to patch nodeLabels and taints day-2 and verify no rolling upgrade is triggered via NodePool conditions and MachineDeployment generation.
Which issue(s) this PR fixes:
Fixes https://redhat.atlassian.net/browse/CNTRLPLANE-3515
Special notes for your reviewer:
The original test also scaled up the NodePool and verified that the new nodes actually have the labels/taints. But I think it's not necessary to test it. The existing NodePool upgrade tests already set labels/taints at the beginning and verify these are properly applied to nodes after initial start. These tests together with the new one from this PR cover the whole original test
Checklist:
Summary by CodeRabbit