Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions pkg/controller/machinepool/machinepool_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -1394,9 +1394,25 @@ func baseMachinePool(pool *hivev1.MachinePool) *installertypes.MachinePool {
}

func isControlledByMachinePool(cd *hivev1.ClusterDeployment, pool *hivev1.MachinePool, obj metav1.Object) bool {
// Primary check: MachineSets with the machinePoolNameLabel matching the pool name are controlled.
labelValue := obj.GetLabels()[machinePoolNameLabel]
if labelValue != "" {
// If label exists, it must match exactly. If it doesn't match, this MachineSet belongs
// to a different pool. Return false immediately to avoid false positives from prefix
// matching when one pool's name is a prefix of another (e.g., "worker" and "worker2").
return labelValue == pool.Spec.Name
}

// Secondary check: For prefix-based matching, we require the HiveManagedLabel to be present.
// This prevents false positives where MachineSets match the naming prefix pattern but aren't
// actually managed by Hive. MachineSets managed by Hive (either created by Hive or adopted by Hive)
// will have the HiveManagedLabel.
prefix := strings.Join([]string{cd.Spec.ClusterName, pool.Spec.Name, ""}, "-")
return strings.HasPrefix(obj.GetName(), prefix) ||
obj.GetLabels()[machinePoolNameLabel] == pool.Spec.Name
if strings.HasPrefix(obj.GetName(), prefix) {
return obj.GetLabels()[constants.HiveManagedLabel] == "true"
}

return false
}

func (r *ReconcileMachinePool) removeFinalizer(pool *hivev1.MachinePool, logger log.FieldLogger) (reconcile.Result, error) {
Expand Down
123 changes: 123 additions & 0 deletions pkg/controller/machinepool/machinepool_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2138,3 +2138,126 @@ func TestEnsureEnoughReplicas_ConditionClearing(t *testing.T) {
})
}
}

func TestIsControlledByMachinePool(t *testing.T) {
cd := &hivev1.ClusterDeployment{
ObjectMeta: metav1.ObjectMeta{
Name: "test-cluster",
Namespace: testNamespace,
},
Spec: hivev1.ClusterDeploymentSpec{
ClusterName: "test-cluster",
},
}

pool := &hivev1.MachinePool{
ObjectMeta: metav1.ObjectMeta{
Name: "test-cluster-worker",
Namespace: testNamespace,
},
Spec: hivev1.MachinePoolSpec{
Name: "worker",
},
}

tests := []struct {
name string
machineSetName string
labels map[string]string
expected bool
description string
}{
{
name: "MachineSet with correct machinePoolNameLabel",
machineSetName: "any-name",
labels: map[string]string{
machinePoolNameLabel: "worker",
},
expected: true,
description: "MachineSet with matching machinePoolNameLabel should be controlled",
},
{
name: "MachineSet with prefix and HiveManagedLabel",
machineSetName: "test-cluster-worker-us-east-1a",
labels: map[string]string{
constants.HiveManagedLabel: "true",
},
expected: true,
description: "MachineSet with matching prefix and HiveManagedLabel should be controlled",
},
{
name: "MachineSet with prefix but no HiveManagedLabel",
machineSetName: "test-cluster-worker-copied-by-user",
labels: map[string]string{},
expected: false,
description: "MachineSet with matching prefix but no HiveManagedLabel should NOT be controlled (prevents false positives)",
},
{
name: "MachineSet with prefix and wrong HiveManagedLabel value",
machineSetName: "test-cluster-worker-us-east-1a",
labels: map[string]string{
constants.HiveManagedLabel: "false",
},
expected: false,
description: "MachineSet with matching prefix but wrong HiveManagedLabel value should NOT be controlled",
},
{
name: "MachineSet with neither label nor matching prefix",
machineSetName: "some-other-machineset",
labels: map[string]string{},
expected: false,
description: "MachineSet with no matching label or prefix should NOT be controlled",
},
{
name: "MachineSet with wrong machinePoolNameLabel value",
machineSetName: "any-name",
labels: map[string]string{
machinePoolNameLabel: "different-pool",
},
expected: false,
description: "MachineSet with non-matching machinePoolNameLabel should NOT be controlled",
},
{
name: "MachineSet with both label and prefix",
machineSetName: "test-cluster-worker-us-east-1a",
labels: map[string]string{
machinePoolNameLabel: "worker",
constants.HiveManagedLabel: "true",
},
expected: true,
description: "MachineSet with both matching label and prefix should be controlled",
},
{
name: "MachineSet with prefix matching but different pool name in prefix",
machineSetName: "test-cluster-different-pool-us-east-1a",
labels: map[string]string{
constants.HiveManagedLabel: "true",
},
expected: false,
description: "MachineSet with prefix that doesn't match the pool name should NOT be controlled",
},
{
name: "MachineSet with non-matching label but prefix matches (worker vs worker2)",
machineSetName: "test-cluster-worker2-us-east-1a",
labels: map[string]string{
machinePoolNameLabel: "worker2",
},
expected: false,
description: "MachineSet with non-matching machinePoolNameLabel should NOT be controlled even if prefix matches (prevents false positives when one pool name is prefix of another)",
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
obj := &machineapi.MachineSet{
ObjectMeta: metav1.ObjectMeta{
Name: test.machineSetName,
Labels: test.labels,
},
}

result := isControlledByMachinePool(cd, pool, obj)
assert.Equal(t, test.expected, result, test.description)
})
}
}