Filter capacity work items by storage class allowed topologies - #1538
Filter capacity work items by storage class allowed topologies#1538DPS0340 wants to merge 1 commit into
Conversation
|
|
|
Welcome @DPS0340! |
|
Hi @DPS0340. Thanks for your PR. I'm waiting for a kubernetes-csi member to verify that this patch is reasonable to test. If it is, they should reply with Regular contributors should join the org to skip this step. Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: DPS0340 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 central capacity controller used to create a work item (and with it a CSIStorageCapacity object) for every combination of topology segment and storage class. Combinations which are ruled out by the storage class's allowed topologies are now skipped, which reduces the work queue depth and the number of CSIStorageCapacity objects in clusters where storage classes restrict provisioning to a subset of the topology segments. The check is conservative: a combination is only filtered out when a topology selector term requirement uses one of the segment's keys with a value that is not allowed. Requirements with label keys that are not part of the segment are assumed to match, because the CSI driver's topology keys are not necessarily the same as the node label keys used in the allowed topologies. When the allowed topologies of an existing storage class change, work items which have become impossible are removed so that their CSIStorageCapacity objects get deleted. Fixes: kubernetes-csi#1162
a8120f5 to
a078361
Compare
|
/ok-to-test |
|
@jsafrane thanks for the So the only thing left is a review. @pohly, would you be able to take a look, or point me at whoever owns I also confirmed it is still current: no conflicts against today's The one decision worth a reviewer's attentionEverything else in the diff follows from this: an unknown label key counts as a match, not as a mismatch. A segment is skipped only when a topology selector term has a requirement for one of that segment's keys with a value the segment does not have. If That is deliberately the conservative direction. CSI driver topology keys are not necessarily the same as the node label keys used in
One asymmetry that is intentional and easy to read as an oversight: |
What type of PR is this?
/kind feature
What this PR does / why we need it:
The central capacity controller creates a work item (and with it a
CSIStorageCapacityobject) for every combination of topology segment and storage class, even when the storage class'sallowedTopologiesrule out that segment. This PR filters out those impossible combinations when a storage class is added/updated and when new topology segments appear, reducing work queue depth and the number ofCSIStorageCapacityobjects.The check is intentionally conservative: a combination is only skipped when a topology selector term contains a requirement for one of the segment's keys with a value that is not allowed. Requirements using label keys which are not part of the segment are assumed to match, because the CSI driver's topology keys are not necessarily the same as the node label keys used in
allowedTopologies— this avoids regressions for such setups.When the
allowedTopologiesof an existing storage class are changed, work items that have become impossible are removed so that theirCSIStorageCapacityobjects get deleted.Which issue(s) this PR fixes:
Fixes #1162
Special notes for your reviewer:
onSCDeleteintentionally stays unfiltered: removing a work item that was never created is a cheap no-op, and this keeps deletion robust.TestSegmentCompatibleWithStorageClasscovers the matching semantics (OR of terms, AND of requirements, unknown label keys), andTestCapacityControllergained cases for initial filtering, filtering of newly added segments, non-segment label keys, and narrowingallowedTopologieson an update.Does this PR introduce a user-facing change?: