From 8b84f4d197e58ec6ea2f0b837df3c10e3325ba99 Mon Sep 17 00:00:00 2001 From: LochanRn Date: Tue, 31 Jan 2023 16:54:33 +0530 Subject: [PATCH] add subnetName support to ammp --- .../azuremanagedmachinepool_conversion.go | 1 + api/v1alpha3/zz_generated.conversion.go | 1 + .../azuremanagedmachinepool_conversion.go | 1 + api/v1alpha4/zz_generated.conversion.go | 1 + api/v1beta1/azuremanagedmachinepool_types.go | 3 + .../azuremanagedmachinepool_webhook.go | 19 +++ .../azuremanagedmachinepool_webhook_test.go | 47 ++++++ azure/scope/managedmachinepool.go | 7 +- azure/scope/managedmachinepool_test.go | 147 ++++++++++++++++++ ...ter.x-k8s.io_azuremanagedmachinepools.yaml | 4 + 10 files changed, 230 insertions(+), 1 deletion(-) diff --git a/api/v1alpha3/azuremanagedmachinepool_conversion.go b/api/v1alpha3/azuremanagedmachinepool_conversion.go index 3958c37aee4..e3b3b8a91e6 100644 --- a/api/v1alpha3/azuremanagedmachinepool_conversion.go +++ b/api/v1alpha3/azuremanagedmachinepool_conversion.go @@ -53,6 +53,7 @@ func (src *AzureManagedMachinePool) ConvertTo(dstRaw conversion.Hub) error { if restored.Spec.KubeletConfig != nil { dst.Spec.KubeletConfig = restored.Spec.KubeletConfig } + dst.Spec.SubnetName = restored.Spec.SubnetName dst.Status.LongRunningOperationStates = restored.Status.LongRunningOperationStates dst.Status.Conditions = restored.Status.Conditions diff --git a/api/v1alpha3/zz_generated.conversion.go b/api/v1alpha3/zz_generated.conversion.go index 76b8ac86ac1..e5c585b7cba 100644 --- a/api/v1alpha3/zz_generated.conversion.go +++ b/api/v1alpha3/zz_generated.conversion.go @@ -1628,6 +1628,7 @@ func autoConvert_v1beta1_AzureManagedMachinePoolSpec_To_v1alpha3_AzureManagedMac // WARNING: in.ScaleSetPriority requires manual conversion: does not exist in peer-type // WARNING: in.KubeletConfig requires manual conversion: does not exist in peer-type // WARNING: in.KubeletDiskType requires manual conversion: does not exist in peer-type + // WARNING: in.SubnetName requires manual conversion: does not exist in peer-type return nil } diff --git a/api/v1alpha4/azuremanagedmachinepool_conversion.go b/api/v1alpha4/azuremanagedmachinepool_conversion.go index 0765a8fbe9d..09cd68db7ef 100644 --- a/api/v1alpha4/azuremanagedmachinepool_conversion.go +++ b/api/v1alpha4/azuremanagedmachinepool_conversion.go @@ -53,6 +53,7 @@ func (src *AzureManagedMachinePool) ConvertTo(dstRaw conversion.Hub) error { if restored.Spec.KubeletConfig != nil { dst.Spec.KubeletConfig = restored.Spec.KubeletConfig } + dst.Spec.SubnetName = restored.Spec.SubnetName dst.Status.LongRunningOperationStates = restored.Status.LongRunningOperationStates dst.Status.Conditions = restored.Status.Conditions diff --git a/api/v1alpha4/zz_generated.conversion.go b/api/v1alpha4/zz_generated.conversion.go index aa031337403..5b103d6417e 100644 --- a/api/v1alpha4/zz_generated.conversion.go +++ b/api/v1alpha4/zz_generated.conversion.go @@ -1868,6 +1868,7 @@ func autoConvert_v1beta1_AzureManagedMachinePoolSpec_To_v1alpha4_AzureManagedMac // WARNING: in.ScaleSetPriority requires manual conversion: does not exist in peer-type // WARNING: in.KubeletConfig requires manual conversion: does not exist in peer-type // WARNING: in.KubeletDiskType requires manual conversion: does not exist in peer-type + // WARNING: in.SubnetName requires manual conversion: does not exist in peer-type return nil } diff --git a/api/v1beta1/azuremanagedmachinepool_types.go b/api/v1beta1/azuremanagedmachinepool_types.go index 85e5d3851f4..3e680c4bef7 100644 --- a/api/v1beta1/azuremanagedmachinepool_types.go +++ b/api/v1beta1/azuremanagedmachinepool_types.go @@ -203,6 +203,9 @@ type AzureManagedMachinePoolSpec struct { // +kubebuilder:validation:Enum=OS;Temporary // +optional KubeletDiskType *KubeletDiskType `json:"kubeletDiskType,omitempty"` + // SubnetName specifies the Subnet where the MachinePool will be placed + // +optional + SubnetName string `json:"subnetName,omitempty"` } // ManagedMachinePoolScaling specifies scaling options. diff --git a/api/v1beta1/azuremanagedmachinepool_webhook.go b/api/v1beta1/azuremanagedmachinepool_webhook.go index bc558213792..f3ef2026db0 100644 --- a/api/v1beta1/azuremanagedmachinepool_webhook.go +++ b/api/v1beta1/azuremanagedmachinepool_webhook.go @@ -78,6 +78,7 @@ func (m *AzureManagedMachinePool) ValidateCreate(client client.Client) error { m.validateNodePublicIPPrefixID, m.validateEnableNodePublicIP, m.validateKubeletConfig, + m.validateSubnetName, } var errs []error @@ -131,6 +132,13 @@ func (m *AzureManagedMachinePool) ValidateUpdate(oldRaw runtime.Object, client c allErrs = append(allErrs, err) } + if err := webhookutils.ValidateImmutable( + field.NewPath("Spec", "SubnetName"), + old.Spec.SubnetName, + m.Spec.SubnetName); err != nil { + allErrs = append(allErrs, err) + } + // custom headers are immutable oldCustomHeaders := maps.FilterByKeyPrefix(old.ObjectMeta.Annotations, CustomHeaderPrefix) newCustomHeaders := maps.FilterByKeyPrefix(m.ObjectMeta.Annotations, CustomHeaderPrefix) @@ -343,6 +351,17 @@ func (m *AzureManagedMachinePool) validateEnableNodePublicIP() error { return nil } +func (m *AzureManagedMachinePool) validateSubnetName() error { + if m.Spec.SubnetName != "" { + subnetRegex := `^[-\w\._]+$` + if success, _ := regexp.Match(subnetRegex, []byte(m.Spec.SubnetName)); !success { + return field.Invalid(field.NewPath("Spec", "SubnetName"), m.Spec.SubnetName, + fmt.Sprintf("name of subnet doesn't match regex %s", subnetRegex)) + } + } + return nil +} + // validateKubeletConfig enforces the AKS API configuration for KubeletConfig. // See: https://learn.microsoft.com/en-us/azure/aks/custom-node-configuration. func (m *AzureManagedMachinePool) validateKubeletConfig() error { diff --git a/api/v1beta1/azuremanagedmachinepool_webhook_test.go b/api/v1beta1/azuremanagedmachinepool_webhook_test.go index 6622018c256..596ee9ea790 100644 --- a/api/v1beta1/azuremanagedmachinepool_webhook_test.go +++ b/api/v1beta1/azuremanagedmachinepool_webhook_test.go @@ -514,6 +514,34 @@ func TestAzureManagedMachinePoolUpdatingWebhook(t *testing.T) { }, wantErr: true, }, + { + name: "Can't update SubnetName with error", + new: &AzureManagedMachinePool{ + Spec: AzureManagedMachinePoolSpec{ + SubnetName: "my-subnet", + }, + }, + old: &AzureManagedMachinePool{ + Spec: AzureManagedMachinePoolSpec{ + SubnetName: "", + }, + }, + wantErr: true, + }, + { + name: "Can't update SubnetName without error", + new: &AzureManagedMachinePool{ + Spec: AzureManagedMachinePoolSpec{ + SubnetName: "my-subnet", + }, + }, + old: &AzureManagedMachinePool{ + Spec: AzureManagedMachinePoolSpec{ + SubnetName: "my-subnet", + }, + }, + wantErr: false, + }, } var client client.Client for _, tc := range tests { @@ -572,6 +600,25 @@ func TestAzureManagedMachinePool_ValidateCreate(t *testing.T) { wantErr: true, errorLen: 1, }, + { + name: "invalid subnetname", + ammp: &AzureManagedMachinePool{ + Spec: AzureManagedMachinePoolSpec{ + SubnetName: "1+subnet", + }, + }, + wantErr: true, + errorLen: 1, + }, + { + name: "valid subnetname", + ammp: &AzureManagedMachinePool{ + Spec: AzureManagedMachinePoolSpec{ + SubnetName: "my-subnet", + }, + }, + wantErr: false, + }, { name: "too few MaxPods", ammp: &AzureManagedMachinePool{ diff --git a/azure/scope/managedmachinepool.go b/azure/scope/managedmachinepool.go index 38aa26fbd81..7c3cd47d94e 100644 --- a/azure/scope/managedmachinepool.go +++ b/azure/scope/managedmachinepool.go @@ -154,6 +154,11 @@ func buildAgentPoolSpec(managedControlPlane *infrav1.AzureManagedControlPlane, replicas = *machinePool.Spec.Replicas } + subnetName := managedControlPlane.Spec.VirtualNetwork.Subnet.Name + if managedMachinePool.Spec.SubnetName != "" { + subnetName = managedMachinePool.Spec.SubnetName + } + agentPoolSpec := &agentpools.AgentPoolSpec{ Name: to.String(managedMachinePool.Spec.Name), ResourceGroup: managedControlPlane.Spec.ResourceGroupName, @@ -166,7 +171,7 @@ func buildAgentPoolSpec(managedControlPlane *infrav1.AzureManagedControlPlane, managedControlPlane.Spec.SubscriptionID, managedControlPlane.Spec.VirtualNetwork.ResourceGroup, managedControlPlane.Spec.VirtualNetwork.Name, - managedControlPlane.Spec.VirtualNetwork.Subnet.Name, + subnetName, ), Mode: managedMachinePool.Spec.Mode, MaxPods: managedMachinePool.Spec.MaxPods, diff --git a/azure/scope/managedmachinepool_test.go b/azure/scope/managedmachinepool_test.go index c30dd51d7b6..a03adbed153 100644 --- a/azure/scope/managedmachinepool_test.go +++ b/azure/scope/managedmachinepool_test.go @@ -623,6 +623,147 @@ func TestManagedMachinePoolScope_OSDiskType(t *testing.T) { } } +func TestManagedMachinePoolScope_SubnetName(t *testing.T) { + scheme := runtime.NewScheme() + _ = expv1.AddToScheme(scheme) + _ = infrav1.AddToScheme(scheme) + + cases := []struct { + Name string + Input ManagedMachinePoolScopeParams + Expected azure.ResourceSpecGetter + }{ + { + Name: "Without Vnet and SubnetName", + Input: ManagedMachinePoolScopeParams{ + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + }, + ControlPlane: &infrav1.AzureManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + Spec: infrav1.AzureManagedControlPlaneSpec{ + SubscriptionID: "00000000-0000-0000-0000-000000000000", + }, + }, + ManagedMachinePool: ManagedMachinePool{ + MachinePool: getMachinePool("pool0"), + InfraMachinePool: getAzureMachinePool("pool0", infrav1.NodePoolModeSystem), + }, + }, + Expected: &agentpools.AgentPoolSpec{ + Name: "pool0", + SKU: "Standard_D2s_v3", + Replicas: 1, + Mode: "System", + Cluster: "cluster1", + VnetSubnetID: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups//providers/Microsoft.Network/virtualNetworks//subnets/", + Headers: map[string]string{}, + }, + }, + { + Name: "With Vnet and Without SubnetName", + Input: ManagedMachinePoolScopeParams{ + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + }, + ControlPlane: &infrav1.AzureManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + Spec: infrav1.AzureManagedControlPlaneSpec{ + SubscriptionID: "00000000-0000-0000-0000-000000000000", + VirtualNetwork: infrav1.ManagedControlPlaneVirtualNetwork{ + Name: "my-vnet", + Subnet: infrav1.ManagedControlPlaneSubnet{ + Name: "my-vnet-subnet", + }, + ResourceGroup: "my-resource-group", + }, + }, + }, + ManagedMachinePool: ManagedMachinePool{ + MachinePool: getMachinePool("pool1"), + InfraMachinePool: getAzureMachinePool("pool1", infrav1.NodePoolModeUser), + }, + }, + Expected: &agentpools.AgentPoolSpec{ + Name: "pool1", + SKU: "Standard_D2s_v3", + Mode: "User", + Cluster: "cluster1", + Replicas: 1, + VnetSubnetID: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/my-resource-group/providers/Microsoft.Network/virtualNetworks/my-vnet/subnets/my-vnet-subnet", + Headers: map[string]string{}, + }, + }, + { + Name: "With Vnet and With SubnetName", + Input: ManagedMachinePoolScopeParams{ + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + }, + ControlPlane: &infrav1.AzureManagedControlPlane{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster1", + Namespace: "default", + }, + Spec: infrav1.AzureManagedControlPlaneSpec{ + SubscriptionID: "00000000-0000-0000-0000-000000000000", + VirtualNetwork: infrav1.ManagedControlPlaneVirtualNetwork{ + Name: "my-vnet", + Subnet: infrav1.ManagedControlPlaneSubnet{ + Name: "my-vnet-subnet", + }, + ResourceGroup: "my-resource-group", + }, + }, + }, + ManagedMachinePool: ManagedMachinePool{ + MachinePool: getMachinePool("pool1"), + InfraMachinePool: getAzureMachinePoolWithSubnetName("pool1", "my-subnet"), + }, + }, + Expected: &agentpools.AgentPoolSpec{ + Name: "pool1", + SKU: "Standard_D2s_v3", + Mode: "User", + Cluster: "cluster1", + Replicas: 1, + VnetSubnetID: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/my-resource-group/providers/Microsoft.Network/virtualNetworks/my-vnet/subnets/my-subnet", + Headers: map[string]string{}, + }, + }, + } + + for _, c := range cases { + c := c + t.Run(c.Name, func(t *testing.T) { + g := NewWithT(t) + fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(c.Input.MachinePool, c.Input.InfraMachinePool, c.Input.ControlPlane).Build() + c.Input.Client = fakeClient + s, err := NewManagedMachinePoolScope(context.TODO(), c.Input) + g.Expect(err).To(Succeed()) + agentPool := s.AgentPoolSpec() + if !reflect.DeepEqual(c.Expected, agentPool) { + t.Errorf("Got difference between expected result and result:\n%s", cmp.Diff(c.Expected, agentPool)) + } + }) + } +} + func TestManagedMachinePoolScope_KubeletDiskType(t *testing.T) { scheme := runtime.NewScheme() _ = expv1.AddToScheme(scheme) @@ -763,6 +904,12 @@ func getAzureMachinePoolWithTaints(name string, taints infrav1.Taints) *infrav1. return managedPool } +func getAzureMachinePoolWithSubnetName(name, subnetName string) *infrav1.AzureManagedMachinePool { + managedPool := getAzureMachinePool(name, infrav1.NodePoolModeUser) + managedPool.Spec.SubnetName = subnetName + return managedPool +} + func getAzureMachinePoolWithOsDiskType(name string, osDiskType string) *infrav1.AzureManagedMachinePool { managedPool := getAzureMachinePool(name, infrav1.NodePoolModeUser) managedPool.Spec.OsDiskType = to.StringPtr(osDiskType) diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedmachinepools.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedmachinepools.yaml index a1ed626bd12..d0d1999fb8d 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedmachinepools.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedmachinepools.yaml @@ -371,6 +371,10 @@ spec: sku: description: SKU is the size of the VMs in the node pool. type: string + subnetName: + description: SubnetName specifies the Subnet where the MachinePool + will be placed + type: string taints: description: Taints specifies the taints for nodes present in this agent pool.