Merge pull request #2774 from pkbhowmick/fix-2656

Skip system pool validation while cluster is paused/moved
kubernetes-sigs · Feb 1, 2023 · d253509 · d253509
2 parents 9259e5b + e7a1c38
commit d253509
Show file tree

Hide file tree

Showing 2 changed files with 100 additions and 0 deletions.
diff --git a/api/v1beta1/azuremanagedmachinepool_webhook.go b/api/v1beta1/azuremanagedmachinepool_webhook.go
@@ -254,6 +254,10 @@ func (m *AzureManagedMachinePool) validateLastSystemNodePool(cli client.Client)
 		return nil
 	}
 
+	if ownerCluster.Spec.Paused {
+		return nil
+	}
+
 	opt1 := client.InNamespace(m.Namespace)
 	opt2 := client.MatchingLabels(map[string]string{
 		clusterv1.ClusterLabelName: clusterName,

diff --git a/api/v1beta1/azuremanagedmachinepool_webhook_test.go b/api/v1beta1/azuremanagedmachinepool_webhook_test.go
@@ -22,11 +22,14 @@ import (
 	"github.com/Azure/azure-sdk-for-go/services/containerservice/mgmt/2022-03-01/containerservice"
 	. "github.com/onsi/gomega"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	utilfeature "k8s.io/component-base/featuregate/testing"
 	"k8s.io/utils/pointer"
 	"sigs.k8s.io/cluster-api-provider-azure/feature"
+	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	capifeature "sigs.k8s.io/cluster-api/feature"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 )
 
 func TestAzureManagedMachinePoolDefaultingWebhook(t *testing.T) {
@@ -882,6 +885,87 @@ func TestAzureManagedMachinePool_ValidateCreateFailure(t *testing.T) {
 	}
 }
 
+func TestAzureManagedMachinePool_validateLastSystemNodePool(t *testing.T) {
+	deletionTime := metav1.Now()
+	systemMachinePool := getManagedMachinePoolWithSystemMode()
+	tests := []struct {
+		name    string
+		ammp    *AzureManagedMachinePool
+		cluster *clusterv1.Cluster
+		wantErr bool
+	}{
+		{
+			name: "Test with paused cluster without deletion timestamp having one system pool node(valid delete:move operation)",
+			ammp: systemMachinePool,
+			cluster: &clusterv1.Cluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:              systemMachinePool.GetLabels()[clusterv1.ClusterLabelName],
+					Namespace:         systemMachinePool.Namespace,
+					DeletionTimestamp: &deletionTime,
+				},
+				Spec: clusterv1.ClusterSpec{
+					Paused: true,
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "Test with paused cluster with deletion timestamp having one system pool node(valid delete)",
+			ammp: systemMachinePool,
+			cluster: &clusterv1.Cluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:              systemMachinePool.GetLabels()[clusterv1.ClusterLabelName],
+					Namespace:         systemMachinePool.Namespace,
+					DeletionTimestamp: &deletionTime,
+				},
+				Spec: clusterv1.ClusterSpec{
+					Paused: true,
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "Test with running cluster without deletion timestamp having one system pool node(invalid delete)",
+			ammp: systemMachinePool,
+			cluster: &clusterv1.Cluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      systemMachinePool.GetLabels()[clusterv1.ClusterLabelName],
+					Namespace: systemMachinePool.Namespace,
+				},
+			},
+			wantErr: true,
+		},
+		{
+			name: "Test with running cluster with deletion timestamp having one system pool node(valid delete)",
+			ammp: systemMachinePool,
+			cluster: &clusterv1.Cluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:              systemMachinePool.GetLabels()[clusterv1.ClusterLabelName],
+					Namespace:         systemMachinePool.Namespace,
+					DeletionTimestamp: &deletionTime,
+				},
+			},
+			wantErr: false,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			g := NewWithT(t)
+			scheme := runtime.NewScheme()
+			_ = AddToScheme(scheme)
+			_ = clusterv1.AddToScheme(scheme)
+			fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(tc.cluster, tc.ammp).Build()
+			err := tc.ammp.validateLastSystemNodePool(fakeClient)
+			if tc.wantErr {
+				g.Expect(err).To(HaveOccurred())
+			} else {
+				g.Expect(err).NotTo(HaveOccurred())
+			}
+		})
+	}
+}
+
 func getKnownValidAzureManagedMachinePool() *AzureManagedMachinePool {
 	return &AzureManagedMachinePool{
 		Spec: AzureManagedMachinePoolSpec{
@@ -890,3 +974,15 @@ func getKnownValidAzureManagedMachinePool() *AzureManagedMachinePool {
 		},
 	}
 }
+
+func getManagedMachinePoolWithSystemMode() *AzureManagedMachinePool {
+	return &AzureManagedMachinePool{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: metav1.NamespaceDefault,
+			Labels: map[string]string{
+				clusterv1.ClusterLabelName: "test-cluster",
+				LabelAgentPoolMode:         string(NodePoolModeSystem),
+			},
+		},
+	}
+}