From 89d4cf94eb16143bb014bede9c590056e9094e3f Mon Sep 17 00:00:00 2001 From: Asish Kumar Date: Thu, 5 Nov 2026 18:11:18 +0530 Subject: [PATCH] Fix PartitionSet recreating Partitions when labels overlap 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 #4549 Signed-off-by: Asish Kumar --- .../topology/partitionset/partitioning_test.go | 6 ++++++ .../partitionset/partitionset_controller_test.go | 7 +++++++ .../topology/partitionset/partitionset_reconcile.go | 11 +++++------ 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/pkg/reconciler/topology/partitionset/partitioning_test.go b/pkg/reconciler/topology/partitionset/partitioning_test.go index 6f0335e7630..40231603302 100644 --- a/pkg/reconciler/topology/partitionset/partitioning_test.go +++ b/pkg/reconciler/topology/partitionset/partitioning_test.go @@ -135,6 +135,12 @@ func TestPartition(t *testing.T) { for _, v := range matchLabelsMap { require.Equal(t, "prod", v["environment"], "Expected that all partitions have a label selector for environment = prod") } + + matchLabelsMap = partition(shards, []string{"region", "environment"}, map[string]string{"environment": "prod"}) + require.Equal(t, map[string]map[string]string{ + "+environment=prod+region=Asia": {"environment": "prod", "region": "Asia"}, + "+environment=prod+region=Europe": {"environment": "prod", "region": "Europe"}, + }, matchLabelsMap, "Labels being both a dimension and in the shard selector should only be considered once") } func TestGeneratePartitionName(t *testing.T) { diff --git a/pkg/reconciler/topology/partitionset/partitionset_controller_test.go b/pkg/reconciler/topology/partitionset/partitionset_controller_test.go index 2efb014228c..ff65e748198 100644 --- a/pkg/reconciler/topology/partitionset/partitionset_controller_test.go +++ b/pkg/reconciler/topology/partitionset/partitionset_controller_test.go @@ -92,6 +92,13 @@ func TestReconcile(t *testing.T) { wantPartitionCount: 2, wantCountCreated: 2, }, + "Existing partition kept with selector and dimension overlapping": { + withMatchLabelOverlap: true, + existingValidPartition: true, + wantPartitionsReady: true, + wantPartitionCount: 2, + wantCountCreated: 1, + }, } for name, tc := range tests { diff --git a/pkg/reconciler/topology/partitionset/partitionset_reconcile.go b/pkg/reconciler/topology/partitionset/partitionset_reconcile.go index 1e9b89c5326..3dd1fd52e72 100644 --- a/pkg/reconciler/topology/partitionset/partitionset_reconcile.go +++ b/pkg/reconciler/topology/partitionset/partitionset_reconcile.go @@ -196,12 +196,11 @@ func (c *controller) reconcile(ctx context.Context, partitionSet *topologyv1alph // so that Partitions not referring to any Shard would not get created. func partition(shards []*corev1alpha1.Shard, dimensions []string, shardSelectorLabels map[string]string) (matchLabelsMap map[string]map[string]string) { matchLabelsMap = make(map[string]map[string]string) - l := make([]string, len(dimensions), len(dimensions)+len(shardSelectorLabels)) - copy(l, dimensions) - for label := range shardSelectorLabels { - l = append(l, label) - } - sort.Strings(l) // Sorting for consistent comparison. + // A label can be both a dimension and part of the shard selector matchLabels. + // It must only be considered once, otherwise the generated key would not + // match the one computed from the matchLabels of existing Partitions. + // sets.List returns the labels sorted for consistent comparison. + l := sets.List(sets.KeySet(shardSelectorLabels).Insert(dimensions...)) for _, shard := range shards { key := "" selector := make(map[string]string)