OCPBUGS-74232: volume-data-source-validator should run on master nodes#659
OCPBUGS-74232: volume-data-source-validator should run on master nodes#659RomanBednar wants to merge 1 commit intoopenshift:mainfrom
Conversation
|
@RomanBednar: This pull request references Jira Issue OCPBUGS-74232, 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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: RomanBednar 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 |
|
/jira refresh |
|
@RomanBednar: once the present PR merges, I will cherry-pick it on top of 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. |
|
@RomanBednar: This pull request references Jira Issue OCPBUGS-74232, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
No GitHub users were found matching the public email listed for the QA contact in Jira (wduan@redhat.com), skipping review request. 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-required |
|
/verified by @duanwei33 |
|
/retest-required |
|
@duanwei33: 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. |
|
Wait, after checking the failed hypershift job, the volume-data-source-validator is deployed to the guest cluster and the guest cluster only has worker nodes — so the storage CO is degraded. I think we need to add this like the csidriveroperators. |
|
/verified cancel |
|
@duanwei33: 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. |
|
/verified remove |
|
@duanwei33: 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. |
|
/verified remove |
|
@duanwei33: 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. |
…hift scheduling Standalone: add nodeSelector and tolerations to the validator deployment so it schedules on master/control plane nodes, consistent with other components in openshift-cluster-storage-operator (e.g. vsphere-problem-detector). HyperShift: guest clusters have no master nodes, so the same nodeSelector left the pod Pending. Add WithControlPlaneTopologyHook so when Infrastructure.Status.ControlPlaneTopology is External the nodeSelector is cleared and the validator can schedule on guest worker nodes.
e7e962e to
d9b6091
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThe volume data source validator is updated to support control plane topology awareness by adding scheduling constraints to the deployment manifest and integrating infrastructure configuration informers and control plane topology hooks into the deployment controller. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip You can get early access to new features in CodeRabbit.Enable the |
|
@duanwei33 I think we need I'm assuming it is ok to run the validator in guest cluster (as before) because The validator could run in management cluster instead. However, that would require much more refactoring to a structure similar to CSI Operators, as you suggested. |
|
@RomanBednar Thanks for the explanation, I see the WithControlPlaneTopologyHook implementation now. This works on both standard and hypershift clusters, and that's fine from my side. |
|
/retest |
|
/verified by @duanwei33 Just re-triggered the hypershift-aws-e2e-external job (one test case looks like a flake). |
|
@duanwei33: 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. |
|
/retest-required |
|
@RomanBednar: 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. |
The volume-data-source-validator deployment was missing nodeSelector and tolerations required to ensure it runs on master/control plane nodes. This is inconsistent with other control plane components in the same namespace (openshift-cluster-storage-operator) such as vsphere-problem-detector and the main cluster-storage-operator.
Added:
This ensures the validator pod:
cc @openshift/storage