OCPBUGS-78617: Fix PinnedImages test should respect node taints#30913
OCPBUGS-78617: Fix PinnedImages test should respect node taints#30913bshaw7 wants to merge 2 commits intoopenshift:mainfrom
Conversation
The PinnedImages conformance test was selecting worker nodes by label only, ignoring node taints. This caused issues in environments with dedicated/tainted nodes (e.g., OPCT test infrastructure, edge zones). Problem: - Test created custom MachineConfigPool targeting any worker node - Could select nodes with NoSchedule/NoExecute taints - In OPCT, selecting the dedicated node caused pod eviction and test failure Solution: - Use e2enode.GetReadySchedulableNodes() to filter nodes - This function excludes nodes with NoSchedule/NoExecute taints - Follows the same pattern used by other OpenShift conformance tests Changes: - Added import: e2enode "k8s.io/kubernetes/test/e2e/framework/node" - Modified addWorkerNodesToCustomPool() to: 1. Get schedulable nodes using e2enode.GetReadySchedulableNodes() 2. Filter for worker nodes only 3. Select from schedulable workers (excluding tainted nodes) Testing: - Verified node filtering logic correctly excludes tainted nodes - Tested on cluster with dedicated OPCT node (node-role.kubernetes.io/tests:NoSchedule) - Dedicated node correctly filtered out, test selects from 3 schedulable workers Fixes: https://github.com/redhat-openshift-ecosystem/opct/issues/[TBD] Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
|
Important Review skippedAuto reviews are limited based on label configuration. 🚫 Excluded labels (none allowed) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes ✨ 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 |
| // Filter for worker nodes only | ||
| var workerNodes []corev1.Node | ||
| for _, node := range nodes.Items { | ||
| if _, hasWorkerLabel := node.Labels["node-role.kubernetes.io/worker"]; hasWorkerLabel { | ||
| workerNodes = append(workerNodes, node) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
@bshaw7 I think we could create a exported function in the package test/extended/node, maybe in node_utils.go, to wrap this logic with e2enode.GetReadySchedulableNodes so that other tests can reuse that logic (retrieve all scheduable worker nodes).
WDYT?
There was a problem hiding this comment.
@mtulio That's good idea to make it re-usable. So far i updated OCPBUGS-78617 with test result without exported function. Next I'll create exporter function and add test result of that too.
|
@bshaw7: This pull request references Jira Issue OCPBUGS-78617, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
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. |
|
/payload-job periodic-ci-openshift-release-main-nightly-4.19-opct-external-aws-ccm |
|
@mtulio: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/cb1fbc90-247e-11f1-8e80-2eb60f5762cd-0 |
…function As suggested in code review, created a shared utility function GetReadySchedulableWorkerNodes() in test/extended/node/node_utils.go to encapsulate the logic of retrieving schedulable worker nodes (excluding nodes with NoSchedule/NoExecute taints). This makes the node selection logic reusable across multiple tests and improves code maintainability. Changes: - Added GetReadySchedulableWorkerNodes() in test/extended/node/node_utils.go - Refactored PinnedImages test to use the new utility function - Simplified addWorkerNodesToCustomPool() by removing manual filtering logic
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: bshaw7 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 |
The PinnedImages conformance test was selecting worker nodes by label only, ignoring node taints. This caused issues in environments with dedicated/tainted nodes (e.g., OPCT test infrastructure, edge zones).
Problem:
Solution:
Changes:
Testing:
Fixes: https://redhat.atlassian.net/browse/OCPBUGS-78617