Repository navigation
Fix PartitionSet recreating Partitions when dimension and selector labels overlap - #4550
officialasishkumar wants to merge 1 commit into
Conversation
When a label is both a PartitionSet dimension and a key of spec.shardSelector.matchLabels, partition() appended it twice to the list of labels used to build the partition key, producing keys such as "+region=eu+region=eu". The key computed for an existing Partition is derived from its matchLabels map and can therefore only ever be "+region=eu", so it never matched. As a result every reconciliation deleted the existing Partition and created a new one with a new generated name, and those events requeued the PartitionSet, resulting in an endless delete/create loop. Build the label list from a set so that each label is only considered once. sets.List keeps the labels sorted, preserving the consistent key ordering the comparison relies on. Add a TestPartition case asserting the keys produced for overlapping labels, and a TestReconcile case asserting that an existing Partition is kept (neither deleted nor recreated) when dimensions and selector matchLabels overlap. Fixes kcp-dev#4549 Signed-off-by: Asish Kumar <officialasishkumar@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
|
Hi @officialasishkumar. Thanks for your PR. I'm waiting for a kcp-dev member to verify that this patch is reasonable to test. If it is, they should reply with 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. |
Summary
When a label is both a
PartitionSetdimension and a key ofspec.shardSelector.matchLabels,partition()appended it twice to the list of labels used to build the partition key (e.g.+region=eu+region=eu). The key for an existingPartitionis computed from itsmatchLabelsmap and can only be+region=eu, so it never matched. Every reconciliation therefore deleted the existingPartitionand created a new one. The resulting events requeued thePartitionSet, and the delete/create loop never stopped.This PR builds the label list from a set, so each label is only considered once.
sets.Listreturns the labels sorted, which keeps the key ordering the comparison depends on.Tests:
TestPartitionnow checks the exact keys produced when dimensions and selectormatchLabelsoverlap.TestReconcilegainsExisting partition kept with selector and dimension overlapping. It checks that an existing matchingPartitionis neither deleted nor recreated. The existing overlap case only covered the first reconcile, when noPartitionexists yet. Without the fix, both new assertions fail: the existing partition is deleted and two partitions are created instead of one.Manual check with a local
kcp start -v=2: I created a shard labelledregion: euand aPartitionSetwithdimensions: [region]andshardSelector.matchLabels: {region: eu}.creating partition/deleting partitionlog lines appeared within 45s.Partition(regions-eu-…) was created and stayed unchanged, with a singlecreating partitionlog line.What Type of PR Is This?
/kind bug
Related Issue(s)
Fixes #4549
Release Notes