diff --git a/api/v1alpha3/azuremanagedmachinepool_conversion.go b/api/v1alpha3/azuremanagedmachinepool_conversion.go index 0719e5a8c8db..3b1922620913 100644 --- a/api/v1alpha3/azuremanagedmachinepool_conversion.go +++ b/api/v1alpha3/azuremanagedmachinepool_conversion.go @@ -55,6 +55,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 85993b3076b7..90dfaff119fb 100644 --- a/api/v1alpha3/zz_generated.conversion.go +++ b/api/v1alpha3/zz_generated.conversion.go @@ -1630,6 +1630,7 @@ func autoConvert_v1beta1_AzureManagedMachinePoolSpec_To_v1alpha3_AzureManagedMac // 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.LinuxOSConfig 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 ecd45c152c52..847783c0933c 100644 --- a/api/v1alpha4/azuremanagedmachinepool_conversion.go +++ b/api/v1alpha4/azuremanagedmachinepool_conversion.go @@ -55,6 +55,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 a42ae25ebe98..917e31cad196 100644 --- a/api/v1alpha4/zz_generated.conversion.go +++ b/api/v1alpha4/zz_generated.conversion.go @@ -1867,6 +1867,7 @@ func autoConvert_v1beta1_AzureManagedMachinePoolSpec_To_v1alpha4_AzureManagedMac // 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.LinuxOSConfig 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 36e5258f1915..4af113de91a8 100644 --- a/api/v1beta1/azuremanagedmachinepool_types.go +++ b/api/v1beta1/azuremanagedmachinepool_types.go @@ -458,6 +458,9 @@ type AzureManagedMachinePoolSpec struct { // LinuxOSConfig specifies the custom Linux OS settings and configurations. // +optional LinuxOSConfig *LinuxOSConfig `json:"linuxOSConfig,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 7eee9517b2eb..2c6547c358a4 100644 --- a/api/v1beta1/azuremanagedmachinepool_webhook.go +++ b/api/v1beta1/azuremanagedmachinepool_webhook.go @@ -80,6 +80,7 @@ func (m *AzureManagedMachinePool) ValidateCreate(client client.Client) error { m.validateEnableNodePublicIP, m.validateKubeletConfig, m.validateLinuxOSConfig, + m.validateSubnetName, } var errs []error @@ -133,6 +134,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) @@ -356,6 +364,17 @@ func (m *AzureManagedMachinePool) validateEnableNodePublicIP() error { return nil } +func (m *AzureManagedMachinePool) validateSubnetName() error { + if m.Spec.SubnetName != nil { + 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 5970397a4ca0..8c593f6741e7 100644 --- a/api/v1beta1/azuremanagedmachinepool_webhook_test.go +++ b/api/v1beta1/azuremanagedmachinepool_webhook_test.go @@ -20,6 +20,7 @@ import ( "testing" "github.com/Azure/azure-sdk-for-go/services/containerservice/mgmt/2022-03-01/containerservice" + "github.com/Azure/go-autorest/autorest/to" . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -535,6 +536,34 @@ func TestAzureManagedMachinePoolUpdatingWebhook(t *testing.T) { }, wantErr: true, }, + { + name: "Can't update SubnetName with error", + new: &AzureManagedMachinePool{ + Spec: AzureManagedMachinePoolSpec{ + SubnetName: to.StringPtr("my-subnet"), + }, + }, + old: &AzureManagedMachinePool{ + Spec: AzureManagedMachinePoolSpec{ + SubnetName: to.StringPtr(""), + }, + }, + wantErr: true, + }, + { + name: "Can't update SubnetName without error", + new: &AzureManagedMachinePool{ + Spec: AzureManagedMachinePoolSpec{ + SubnetName: to.StringPtr("my-subnet"), + }, + }, + old: &AzureManagedMachinePool{ + Spec: AzureManagedMachinePoolSpec{ + SubnetName: to.StringPtr("my-subnet"), + }, + }, + wantErr: false, + }, } var client client.Client for _, tc := range tests { @@ -593,6 +622,25 @@ func TestAzureManagedMachinePool_ValidateCreate(t *testing.T) { wantErr: true, errorLen: 1, }, + { + name: "invalid subnetname", + ammp: &AzureManagedMachinePool{ + Spec: AzureManagedMachinePoolSpec{ + SubnetName: to.StringPtr("1+subnet"), + }, + }, + wantErr: true, + errorLen: 1, + }, + { + name: "valid subnetname", + ammp: &AzureManagedMachinePool{ + Spec: AzureManagedMachinePoolSpec{ + SubnetName: to.StringPtr("my-subnet"), + }, + }, + wantErr: false, + }, { name: "too few MaxPods", ammp: &AzureManagedMachinePool{ diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 8d24ee954fc2..8978a44dc662 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -1417,6 +1417,11 @@ func (in *AzureManagedMachinePoolSpec) DeepCopyInto(out *AzureManagedMachinePool *out = new(LinuxOSConfig) (*in).DeepCopyInto(*out) } + if in.SubnetName != nil { + in, out := &in.SubnetName, &out.SubnetName + *out = new(string) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AzureManagedMachinePoolSpec. diff --git a/azure/scope/managedmachinepool.go b/azure/scope/managedmachinepool.go index eb23772a45e9..5d170727d886 100644 --- a/azure/scope/managedmachinepool.go +++ b/azure/scope/managedmachinepool.go @@ -21,6 +21,7 @@ import ( "fmt" "strings" + "github.com/Azure/go-autorest/autorest/to" "github.com/pkg/errors" "k8s.io/utils/pointer" infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" @@ -134,6 +135,13 @@ func (s *ManagedMachinePoolScope) Name() string { return s.InfraMachinePool.Name } +// SetSubnetName updates AzureManagedMachinePool.SubnetName if AzureManagedMachinePool.SubnetName is empty with s.ControlPlane.Spec.VirtualNetwork.Subnet.Name. +func (s *ManagedMachinePoolScope) SetSubnetName() { + if s.ControlPlane.Spec.VirtualNetwork.Name != "" && s.InfraMachinePool.Spec.SubnetName == nil { + s.InfraMachinePool.Spec.SubnetName = to.StringPtr(s.ControlPlane.Spec.VirtualNetwork.Subnet.Name) + } +} + // AgentPoolSpec returns an azure.ResourceSpecGetter for currently reconciled AzureManagedMachinePool. func (s *ManagedMachinePoolScope) AgentPoolSpec() azure.ResourceSpecGetter { return buildAgentPoolSpec(s.ControlPlane, s.MachinePool, s.InfraMachinePool, s.AgentPoolAnnotations()) @@ -166,7 +174,7 @@ func buildAgentPoolSpec(managedControlPlane *infrav1.AzureManagedControlPlane, managedControlPlane.Spec.SubscriptionID, managedControlPlane.Spec.VirtualNetwork.ResourceGroup, managedControlPlane.Spec.VirtualNetwork.Name, - managedControlPlane.Spec.VirtualNetwork.Subnet.Name, + to.String(managedMachinePool.Spec.SubnetName), ), Mode: managedMachinePool.Spec.Mode, MaxPods: managedMachinePool.Spec.MaxPods, diff --git a/azure/scope/managedmachinepool_test.go b/azure/scope/managedmachinepool_test.go index 6723e51d1411..4939da9a3d8f 100644 --- a/azure/scope/managedmachinepool_test.go +++ b/azure/scope/managedmachinepool_test.go @@ -22,6 +22,7 @@ import ( "testing" "github.com/Azure/azure-sdk-for-go/services/containerservice/mgmt/2022-03-01/containerservice" + "github.com/Azure/go-autorest/autorest/to" "github.com/google/go-cmp/cmp" . "github.com/onsi/gomega" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -623,6 +624,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", to.StringPtr("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 +905,12 @@ func getAzureMachinePoolWithTaints(name string, taints infrav1.Taints) *infrav1. return managedPool } +func getAzureMachinePoolWithSubnetName(name string, 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 = pointer.String(osDiskType) diff --git a/azure/services/agentpools/agentpools.go b/azure/services/agentpools/agentpools.go index 9361c29cff2e..f4b1bba6359b 100644 --- a/azure/services/agentpools/agentpools.go +++ b/azure/services/agentpools/agentpools.go @@ -46,6 +46,7 @@ type AgentPoolScope interface { SetCAPIMachinePoolReplicas(replicas *int32) SetCAPIMachinePoolAnnotation(key, value string) RemoveCAPIMachinePoolAnnotation(key string) + SetSubnetName() } // Service provides operations on Azure resources. diff --git a/azure/services/agentpools/mock_agentpools/agentpools_mock.go b/azure/services/agentpools/mock_agentpools/agentpools_mock.go index 91eefee9fa95..c9e0d6f30611 100644 --- a/azure/services/agentpools/mock_agentpools/agentpools_mock.go +++ b/azure/services/agentpools/mock_agentpools/agentpools_mock.go @@ -443,6 +443,18 @@ func (mr *MockAgentPoolScopeMockRecorder) SetLongRunningOperationState(arg0 inte return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetLongRunningOperationState", reflect.TypeOf((*MockAgentPoolScope)(nil).SetLongRunningOperationState), arg0) } +// SetSubnetName mocks base method. +func (m *MockAgentPoolScope) SetSubnetName() { + m.ctrl.T.Helper() + m.ctrl.Call(m, "SetSubnetName") +} + +// SetSubnetName indicates an expected call of SetSubnetName. +func (mr *MockAgentPoolScopeMockRecorder) SetSubnetName() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetSubnetName", reflect.TypeOf((*MockAgentPoolScope)(nil).SetSubnetName)) +} + // SubscriptionID mocks base method. func (m *MockAgentPoolScope) SubscriptionID() string { m.ctrl.T.Helper() 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 04f9e824c48c..4e11fd326993 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedmachinepools.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedmachinepools.yaml @@ -635,6 +635,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. diff --git a/controllers/azuremanagedmachinepool_reconciler.go b/controllers/azuremanagedmachinepool_reconciler.go index dd928ea83684..0558c332f924 100644 --- a/controllers/azuremanagedmachinepool_reconciler.go +++ b/controllers/azuremanagedmachinepool_reconciler.go @@ -95,6 +95,8 @@ func (s *azureManagedMachinePoolService) Reconcile(ctx context.Context) error { ctx, log, done := tele.StartSpanWithLogger(ctx, "controllers.azureManagedMachinePoolService.Reconcile") defer done() + s.scope.SetSubnetName() + log.Info("reconciling managed machine pool") agentPoolName := s.scope.AgentPoolSpec().ResourceName()