From efa6079faeae2b910b42abb1f1141ccbed9f8f60 Mon Sep 17 00:00:00 2001 From: Dane Thorsen Date: Thu, 18 May 2023 18:51:47 +0000 Subject: [PATCH] Support preexisting network interfaces for AzureMachines --- api/v1beta1/types.go | 30 +++++ api/v1beta1/zz_generated.deepcopy.go | 30 +++++ azure/scope/machine.go | 18 ++- azure/scope/machine_test.go | 108 ++++++++++++++++ azure/scope/machinepool_test.go | 4 +- .../networkinterfaces/networkinterfaces.go | 50 +++++++- .../networkinterfaces_test.go | 115 ++++++++++++++++-- azure/services/scalesets/scalesets_test.go | 6 +- azure/types.go | 2 +- config/capz/manager_image_patch.yaml | 2 +- config/capz/manager_pull_policy.yaml | 2 +- ...re.cluster.x-k8s.io_azuremachinepools.yaml | 3 +- ...ucture.cluster.x-k8s.io_azuremachines.yaml | 15 +++ ...luster.x-k8s.io_azuremachinetemplates.yaml | 16 +++ .../src/topics/custom-network-interfaces.md | 57 +++++++++ exp/api/v1beta1/azuremachinepool_default.go | 2 +- .../v1beta1/azuremachinepool_default_test.go | 8 +- exp/api/v1beta1/azuremachinepool_types.go | 2 +- .../v1beta1/azuremachinepool_webhook_test.go | 20 +-- exp/api/v1beta1/zz_generated.deepcopy.go | 2 +- 20 files changed, 448 insertions(+), 44 deletions(-) create mode 100644 docs/book/src/topics/custom-network-interfaces.md diff --git a/api/v1beta1/types.go b/api/v1beta1/types.go index b1a96bb0915a..306d4d39a176 100644 --- a/api/v1beta1/types.go +++ b/api/v1beta1/types.go @@ -734,6 +734,36 @@ type NetworkInterface struct { // +kubebuilder:validation:nullable // +optional AcceleratedNetworking *bool `json:"acceleratedNetworking,omitempty"` + + // Name optionally allows you to override the Azure NetworkInterface name. If left unspecified, CAPZ will + // generate a NetworkInterface name based on the VM name. + // If specified, the specified networkInterface does not exist, CAPZ will create it when the VM is created and delete it when the VM is deleted. + // If you specify an existing Azure NetworkInterface, CAPZ will attach and detach the interface from the VM when the VM is created and deleted, + // but CAPZ will not delete the NetworkInterface if the VM is deleted. + // +optional + Name *string `json:"name,omitempty"` + + // ResourceGroup optionally specifies the resource group for the NetworkInterface resource. + // +optional + ResouceGroup *string `json:"resourceGroup,omitempty"` +} + +// MachinePoolNetworkInterface defines a VMSS network interface profile. +type MachinePoolNetworkInterface struct { + // SubnetName specifies the subnet in which the new network interface will be placed. + SubnetName string `json:"subnetName,omitempty"` + + // PrivateIPConfigs specifies the number of private IP addresses to attach to the interface. + // Defaults to 1 if not specified. + // +optional + PrivateIPConfigs int `json:"privateIPConfigs,omitempty"` + + // AcceleratedNetworking enables or disables Azure accelerated networking. If omitted, it will be set based on + // whether the requested VMSize supports accelerated networking. + // If AcceleratedNetworking is set to true with a VMSize that does not support it, Azure will return an error. + // +kubebuilder:validation:nullable + // +optional + AcceleratedNetworking *bool `json:"acceleratedNetworking,omitempty"` } // GetControlPlaneSubnet returns the cluster control plane subnet. diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 32fff5508c48..ceee815a8777 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -2091,6 +2091,26 @@ func (in *LoadBalancerSpec) DeepCopy() *LoadBalancerSpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *MachinePoolNetworkInterface) DeepCopyInto(out *MachinePoolNetworkInterface) { + *out = *in + if in.AcceleratedNetworking != nil { + in, out := &in.AcceleratedNetworking, &out.AcceleratedNetworking + *out = new(bool) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MachinePoolNetworkInterface. +func (in *MachinePoolNetworkInterface) DeepCopy() *MachinePoolNetworkInterface { + if in == nil { + return nil + } + out := new(MachinePoolNetworkInterface) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ManagedControlPlaneSubnet) DeepCopyInto(out *ManagedControlPlaneSubnet) { *out = *in @@ -2236,6 +2256,16 @@ func (in *NetworkInterface) DeepCopyInto(out *NetworkInterface) { *out = new(bool) **out = **in } + if in.Name != nil { + in, out := &in.Name, &out.Name + *out = new(string) + **out = **in + } + if in.ResouceGroup != nil { + in, out := &in.ResouceGroup, &out.ResouceGroup + *out = new(string) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new NetworkInterface. diff --git a/azure/scope/machine.go b/azure/scope/machine.go index bd6b318203ab..8d1e9c1a0213 100644 --- a/azure/scope/machine.go +++ b/azure/scope/machine.go @@ -237,15 +237,23 @@ func (m *MachineScope) NICSpecs() []azure.ResourceSpecGetter { isMultiNIC := len(m.AzureMachine.Spec.NetworkInterfaces) > 1 for i := 0; i < len(m.AzureMachine.Spec.NetworkInterfaces); i++ { + nic := &m.AzureMachine.Spec.NetworkInterfaces[i] + nicName := "" isPrimary := i == 0 - nicName := azure.GenerateNICName(m.Name(), isMultiNIC, i) - nicSpecs = append(nicSpecs, m.BuildNICSpec(nicName, m.AzureMachine.Spec.NetworkInterfaces[i], isPrimary)) + + if nic.Name != nil && *nic.Name != "" { + nicName = *nic.Name + } else { + nicName = azure.GenerateNICName(m.Name(), isMultiNIC, i) + } + + nicSpecs = append(nicSpecs, m.BuildNICSpec(nicName, nic, isPrimary)) } return nicSpecs } // BuildNICSpec takes a NetworkInterface from the AzureMachineSpec and returns a NICSpec for use by the networkinterfaces service. -func (m *MachineScope) BuildNICSpec(nicName string, infrav1NetworkInterface infrav1.NetworkInterface, primaryNetworkInterface bool) *networkinterfaces.NICSpec { +func (m *MachineScope) BuildNICSpec(nicName string, infrav1NetworkInterface *infrav1.NetworkInterface, primaryNetworkInterface bool) *networkinterfaces.NICSpec { spec := &networkinterfaces.NICSpec{ Name: nicName, ResourceGroup: m.ResourceGroup(), @@ -264,6 +272,10 @@ func (m *MachineScope) BuildNICSpec(nicName string, infrav1NetworkInterface infr IPConfigs: []networkinterfaces.IPConfig{}, } + if infrav1NetworkInterface.ResouceGroup != nil && *infrav1NetworkInterface.ResouceGroup != "" { + spec.ResourceGroup = *infrav1NetworkInterface.ResouceGroup + } + if m.cache != nil { spec.SKU = &m.cache.VMSKU } diff --git a/azure/scope/machine_test.go b/azure/scope/machine_test.go index a92ac9d5ca45..f680cb2eff5b 100644 --- a/azure/scope/machine_test.go +++ b/azure/scope/machine_test.go @@ -2802,6 +2802,114 @@ func TestMachineScope_NICSpecs(t *testing.T) { }, }, }, + { + name: "Node Machine with a custom network interface name", + machineScope: MachineScope{ + ClusterScoper: &ClusterScope{ + AzureClients: AzureClients{ + EnvironmentSettings: auth.EnvironmentSettings{ + Values: map[string]string{ + auth.SubscriptionID: "123", + }, + }, + }, + Cluster: &clusterv1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + Namespace: "default", + }, + }, + AzureCluster: &infrav1.AzureCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cluster", + Namespace: "default", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "cluster.x-k8s.io/v1beta1", + Kind: "Cluster", + Name: "cluster", + }, + }, + }, + Spec: infrav1.AzureClusterSpec{ + ResourceGroup: "my-rg", + AzureClusterClassSpec: infrav1.AzureClusterClassSpec{ + Location: "westus", + }, + NetworkSpec: infrav1.NetworkSpec{ + Vnet: infrav1.VnetSpec{ + Name: "vnet1", + ResourceGroup: "rg1", + }, + Subnets: []infrav1.SubnetSpec{ + { + SubnetClassSpec: infrav1.SubnetClassSpec{ + Role: infrav1.SubnetNode, + Name: "subnet1", + }, + }, + }, + APIServerLB: infrav1.LoadBalancerSpec{ + Name: "api-lb", + }, + NodeOutboundLB: &infrav1.LoadBalancerSpec{ + Name: "outbound-lb", + }, + }, + }, + }, + }, + AzureMachine: &infrav1.AzureMachine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "machine", + }, + Spec: infrav1.AzureMachineSpec{ + ProviderID: pointer.String("azure://compute/virtual-machines/machine-name"), + NetworkInterfaces: []infrav1.NetworkInterface{ + { + Name: pointer.String("custom-name-nic"), + SubnetName: "subnet1", + AcceleratedNetworking: pointer.Bool(true), + PrivateIPConfigs: 1, + }, + }, + }, + }, + Machine: &clusterv1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "machine", + Labels: map[string]string{}, + }, + }, + }, + want: []azure.ResourceSpecGetter{ + &networkinterfaces.NICSpec{ + Name: "custom-name-nic", + ResourceGroup: "my-rg", + Location: "westus", + SubscriptionID: "123", + MachineName: "machine-name", + SubnetName: "subnet1", + IPConfigs: []networkinterfaces.IPConfig{{}}, + VNetName: "vnet1", + VNetResourceGroup: "rg1", + PublicLBName: "outbound-lb", + PublicLBAddressPoolName: "outbound-lb-outboundBackendPool", + PublicLBNATRuleName: "", + InternalLBName: "", + InternalLBAddressPoolName: "", + PublicIPName: "", + AcceleratedNetworking: pointer.Bool(true), + IPv6Enabled: false, + EnableIPForwarding: false, + SKU: nil, + ClusterName: "cluster", + AdditionalTags: map[string]string{ + "kubernetes.io_cluster_cluster": "owned", + }, + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/azure/scope/machinepool_test.go b/azure/scope/machinepool_test.go index 96a5888aef95..857ce42fdd9a 100644 --- a/azure/scope/machinepool_test.go +++ b/azure/scope/machinepool_test.go @@ -133,7 +133,7 @@ func TestMachinePoolScope_NetworkInterfaces(t *testing.T) { }, Spec: infrav1exp.AzureMachinePoolSpec{ Template: infrav1exp.AzureMachinePoolMachineTemplate{ - NetworkInterfaces: []infrav1.NetworkInterface{ + NetworkInterfaces: []infrav1.MachinePoolNetworkInterface{ { SubnetName: "node-subnet", }, @@ -155,7 +155,7 @@ func TestMachinePoolScope_NetworkInterfaces(t *testing.T) { }, Spec: infrav1exp.AzureMachinePoolSpec{ Template: infrav1exp.AzureMachinePoolMachineTemplate{ - NetworkInterfaces: []infrav1.NetworkInterface{ + NetworkInterfaces: []infrav1.MachinePoolNetworkInterface{ { SubnetName: "control-plane-subnet", }, diff --git a/azure/services/networkinterfaces/networkinterfaces.go b/azure/services/networkinterfaces/networkinterfaces.go index 0926e8fdf761..59a93f67918a 100644 --- a/azure/services/networkinterfaces/networkinterfaces.go +++ b/azure/services/networkinterfaces/networkinterfaces.go @@ -18,11 +18,15 @@ package networkinterfaces import ( "context" + "fmt" + "github.com/pkg/errors" infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" "sigs.k8s.io/cluster-api-provider-azure/azure" + "sigs.k8s.io/cluster-api-provider-azure/azure/converters" "sigs.k8s.io/cluster-api-provider-azure/azure/services/async" "sigs.k8s.io/cluster-api-provider-azure/azure/services/resourceskus" + "sigs.k8s.io/cluster-api-provider-azure/azure/services/tags" "sigs.k8s.io/cluster-api-provider-azure/util/reconciler" "sigs.k8s.io/cluster-api-provider-azure/util/tele" ) @@ -40,15 +44,18 @@ type NICScope interface { type Service struct { Scope NICScope async.Reconciler + async.TagsGetter resourceSKUCache *resourceskus.Cache } // New creates a new service. func New(scope NICScope, skuCache *resourceskus.Cache) *Service { Client := NewClient(scope) + tagsClient := tags.NewClient(scope) return &Service{ Scope: scope, Reconciler: async.New(scope, Client, Client), + TagsGetter: tagsClient, resourceSKUCache: skuCache, } } @@ -89,7 +96,7 @@ func (s *Service) Reconcile(ctx context.Context) error { // Delete deletes the network interface with the provided name. func (s *Service) Delete(ctx context.Context) error { - ctx, _, done := tele.StartSpanWithLogger(ctx, "networkinterfaces.Service.Delete") + ctx, log, done := tele.StartSpanWithLogger(ctx, "networkinterfaces.Service.Delete") defer done() ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout) @@ -105,6 +112,12 @@ func (s *Service) Delete(ctx context.Context) error { // Order of precedence (highest -> lowest) is: error that is not an operationNotDoneError (i.e. error deleting) -> operationNotDoneError (i.e. deleting in progress) -> no error (i.e. deleted) var result error for _, nicSpec := range specs { + if managed, err := s.isManaged(ctx, nicSpec); err == nil && !managed { + log.V(4).Info("Skipping network interface deletion in custom network interface mode") + continue + } else if err != nil { + return errors.Wrap(err, "failed to check if network interface is managed") + } if err := s.DeleteResource(ctx, nicSpec, serviceName); err != nil { if !azure.IsOperationNotDoneError(err) || result == nil { result = err @@ -116,7 +129,40 @@ func (s *Service) Delete(ctx context.Context) error { return result } -// IsManaged returns always returns true as CAPZ does not support BYO network interfaces. +// isManaged returns true if the network interface has an owned tag with the cluster name as value, +// meaning that the network interface's lifecycle is managed. +func (s *Service) isManaged(ctx context.Context, spec azure.ResourceSpecGetter) (bool, error) { + _, _, done := tele.StartSpanWithLogger(ctx, "networkinterfaces.Service.IsManaged") + defer done() + + if spec == nil { + return false, errors.New("cannot get network interface to check if it is managed: spec is nil") + } + + scope := azure.NetworkInterfaceID(s.Scope.SubscriptionID(), spec.ResourceGroupName(), spec.ResourceName()) + if s.TagsGetter == nil { + return false, fmt.Errorf("tagsgetter is nil") + } + if ctx == nil { + return false, fmt.Errorf("ctx is nil") + } + result, err := s.TagsGetter.GetAtScope(ctx, scope) + if err != nil { + return false, err + } + + tagsMap := make(map[string]*string) + if result.Properties != nil && result.Properties.Tags != nil { + tagsMap = result.Properties.Tags + } + + tags := converters.MapToTags(tagsMap) + return tags.HasOwned(s.Scope.ClusterName()), nil +} + +// IsManaged returns true if the network interface has an owned tag with the cluster name as value, +// meaning that the network interface's lifecycle is managed. func (s *Service) IsManaged(ctx context.Context) (bool, error) { + // This method is unused, but it is required to satisfy the ServiceReconciler interface return true, nil } diff --git a/azure/services/networkinterfaces/networkinterfaces_test.go b/azure/services/networkinterfaces/networkinterfaces_test.go index 59af9f25bf3b..034067a6f168 100644 --- a/azure/services/networkinterfaces/networkinterfaces_test.go +++ b/azure/services/networkinterfaces/networkinterfaces_test.go @@ -22,10 +22,12 @@ import ( "net/http" "testing" + "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2019-10-01/resources" "github.com/Azure/go-autorest/autorest" "github.com/golang/mock/gomock" "github.com/google/go-cmp/cmp" . "github.com/onsi/gomega" + "k8s.io/utils/pointer" infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1" "sigs.k8s.io/cluster-api-provider-azure/azure" "sigs.k8s.io/cluster-api-provider-azure/azure/services/async/mock_async" @@ -160,20 +162,37 @@ func TestDeleteNetworkInterface(t *testing.T) { testcases := []struct { name string expectedError string - expect func(s *mock_networkinterfaces.MockNICScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) + expect func(s *mock_networkinterfaces.MockNICScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder, t *mock_async.MockTagsGetterMockRecorder) }{ { name: "noop if no network interface specs are found", expectedError: "", - expect: func(s *mock_networkinterfaces.MockNICScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { + expect: func(s *mock_networkinterfaces.MockNICScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder, t *mock_async.MockTagsGetterMockRecorder) { s.NICSpecs().Return([]azure.ResourceSpecGetter{}) }, }, { name: "successfully delete an existing network interface", expectedError: "", - expect: func(s *mock_networkinterfaces.MockNICScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { - s.NICSpecs().Return([]azure.ResourceSpecGetter{&fakeNICSpec1}) + expect: func(s *mock_networkinterfaces.MockNICScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder, t *mock_async.MockTagsGetterMockRecorder) { + mockSpecs := []azure.ResourceSpecGetter{&fakeNICSpec1} + s.NICSpecs().Return(mockSpecs) + s.SubscriptionID().Return(fakeNICSpec1.SubscriptionID) + t.GetAtScope( + gomockinternal.AContext(), + azure.NetworkInterfaceID( + fakeNICSpec1.SubscriptionID, + fakeNICSpec1.ResourceGroup, + fakeNICSpec1.Name, + ), + ).Return(resources.TagsResource{ + Properties: &resources.Tags{ + Tags: map[string]*string{ + infrav1.ClusterTagKey("my-cluster"): pointer.String(string(infrav1.ResourceLifecycleOwned)), + }, + }, + }, nil) + s.ClusterName().Return("my-cluster") r.DeleteResource(gomockinternal.AContext(), &fakeNICSpec1, serviceName).Return(nil) s.UpdateDeleteStatus(infrav1.NetworkInterfaceReadyCondition, serviceName, nil) }, @@ -181,23 +200,91 @@ func TestDeleteNetworkInterface(t *testing.T) { { name: "successfully delete multiple existing network interfaces", expectedError: "", - expect: func(s *mock_networkinterfaces.MockNICScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { - s.NICSpecs().Return([]azure.ResourceSpecGetter{&fakeNICSpec1, &fakeNICSpec2}) - r.DeleteResource(gomockinternal.AContext(), &fakeNICSpec1, serviceName).Return(nil) - r.DeleteResource(gomockinternal.AContext(), &fakeNICSpec2, serviceName).Return(nil) + expect: func(s *mock_networkinterfaces.MockNICScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder, t *mock_async.MockTagsGetterMockRecorder) { + nicSpecs := []*NICSpec{&fakeNICSpec1, &fakeNICSpec2} + mockSpecs := []azure.ResourceSpecGetter{&fakeNICSpec1, &fakeNICSpec2} + s.NICSpecs().Return(mockSpecs) + for _, nicSpec := range nicSpecs { + s.SubscriptionID().Return(nicSpec.SubscriptionID) + t.GetAtScope( + gomockinternal.AContext(), + azure.NetworkInterfaceID( + nicSpec.SubscriptionID, + nicSpec.ResourceGroup, + nicSpec.Name, + ), + ).Return(resources.TagsResource{ + Properties: &resources.Tags{ + Tags: map[string]*string{ + infrav1.ClusterTagKey("my-cluster"): pointer.String(string(infrav1.ResourceLifecycleOwned)), + }, + }, + }, nil) + s.ClusterName().Return("my-cluster") + r.DeleteResource(gomockinternal.AContext(), nicSpec, serviceName).Return(nil) + } s.UpdateDeleteStatus(infrav1.NetworkInterfaceReadyCondition, serviceName, nil) }, }, { name: "network interface deletion fails", expectedError: internalError.Error(), - expect: func(s *mock_networkinterfaces.MockNICScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) { - s.NICSpecs().Return([]azure.ResourceSpecGetter{&fakeNICSpec1, &fakeNICSpec2}) - r.DeleteResource(gomockinternal.AContext(), &fakeNICSpec1, serviceName).Return(nil) - r.DeleteResource(gomockinternal.AContext(), &fakeNICSpec2, serviceName).Return(internalError) + expect: func(s *mock_networkinterfaces.MockNICScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder, t *mock_async.MockTagsGetterMockRecorder) { + nicSpecs := []*NICSpec{&fakeNICSpec1, &fakeNICSpec2} + mockSpecs := []azure.ResourceSpecGetter{&fakeNICSpec1, &fakeNICSpec2} + s.NICSpecs().Return(mockSpecs) + returnErrors := []error{ + nil, + internalError, + } + for i, nicSpec := range nicSpecs { + s.SubscriptionID().Return(nicSpec.SubscriptionID) + t.GetAtScope( + gomockinternal.AContext(), + azure.NetworkInterfaceID( + nicSpec.SubscriptionID, + nicSpec.ResourceGroup, + nicSpec.Name, + ), + ).Return(resources.TagsResource{ + Properties: &resources.Tags{ + Tags: map[string]*string{ + infrav1.ClusterTagKey("my-cluster"): pointer.String(string(infrav1.ResourceLifecycleOwned)), + }, + }, + }, nil) + s.ClusterName().Return("my-cluster") + r.DeleteResource(gomockinternal.AContext(), nicSpec, serviceName).Return(returnErrors[i]) + } s.UpdateDeleteStatus(infrav1.NetworkInterfaceReadyCondition, serviceName, internalError) }, }, + { + name: "preexisting network interface should not be deleted", + expectedError: "", + expect: func(s *mock_networkinterfaces.MockNICScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder, t *mock_async.MockTagsGetterMockRecorder) { + nicSpecs := []*NICSpec{&fakeNICSpec1, &fakeNICSpec2} + mockSpecs := []azure.ResourceSpecGetter{&fakeNICSpec1, &fakeNICSpec2} + s.NICSpecs().Return(mockSpecs) + for _, nicSpec := range nicSpecs { + s.SubscriptionID().Return(nicSpec.SubscriptionID) + t.GetAtScope( + gomockinternal.AContext(), + azure.NetworkInterfaceID( + nicSpec.SubscriptionID, + nicSpec.ResourceGroup, + nicSpec.Name, + ), + ).Return(resources.TagsResource{ + Properties: &resources.Tags{ + Tags: map[string]*string{}, + }, + }, nil) + s.ClusterName().Return("my-cluster") + } + s.UpdateDeleteStatus(infrav1.NetworkInterfaceReadyCondition, serviceName, nil) + }, + }, } for _, tc := range testcases { @@ -209,12 +296,14 @@ func TestDeleteNetworkInterface(t *testing.T) { defer mockCtrl.Finish() scopeMock := mock_networkinterfaces.NewMockNICScope(mockCtrl) asyncMock := mock_async.NewMockReconciler(mockCtrl) + tagsMock := mock_async.NewMockTagsGetter(mockCtrl) - tc.expect(scopeMock.EXPECT(), asyncMock.EXPECT()) + tc.expect(scopeMock.EXPECT(), asyncMock.EXPECT(), tagsMock.EXPECT()) s := &Service{ Scope: scopeMock, Reconciler: asyncMock, + TagsGetter: tagsMock, } err := s.Delete(context.TODO()) diff --git a/azure/services/scalesets/scalesets_test.go b/azure/services/scalesets/scalesets_test.go index d966573cb531..654a970ec081 100644 --- a/azure/services/scalesets/scalesets_test.go +++ b/azure/services/scalesets/scalesets_test.go @@ -265,7 +265,7 @@ func TestReconcileVMSS(t *testing.T) { expect: func(g *WithT, s *mock_scalesets.MockScaleSetScopeMockRecorder, m *mock_scalesets.MockClientMockRecorder) { spec := newDefaultVMSSSpec() spec.Size = "VM_SIZE_AN" - spec.NetworkInterfaces = []infrav1.NetworkInterface{ + spec.NetworkInterfaces = []infrav1.MachinePoolNetworkInterface{ { SubnetName: "somesubnet", PrivateIPConfigs: 1, // defaulter sets this to one @@ -304,7 +304,7 @@ func TestReconcileVMSS(t *testing.T) { StorageAccountType: "UltraSSD_LRS", }, }) - spec.NetworkInterfaces = []infrav1.NetworkInterface{ + spec.NetworkInterfaces = []infrav1.MachinePoolNetworkInterface{ { SubnetName: "my-subnet", PrivateIPConfigs: 1, @@ -1281,7 +1281,7 @@ func newDefaultVMSSSpec() azure.ScaleSetSpec { AcceleratedNetworking: nil, TerminateNotificationTimeout: pointer.Int(7), FailureDomains: []string{"1", "3"}, - NetworkInterfaces: []infrav1.NetworkInterface{ + NetworkInterfaces: []infrav1.MachinePoolNetworkInterface{ { SubnetName: "my-subnet", PrivateIPConfigs: 1, diff --git a/azure/types.go b/azure/types.go index 873cf1cced4b..e3de030d84d4 100644 --- a/azure/types.go +++ b/azure/types.go @@ -67,7 +67,7 @@ type ScaleSetSpec struct { DiagnosticsProfile *infrav1.Diagnostics FailureDomains []string VMExtensions []infrav1.VMExtension - NetworkInterfaces []infrav1.NetworkInterface + NetworkInterfaces []infrav1.MachinePoolNetworkInterface IPv6Enabled bool OrchestrationMode infrav1.OrchestrationModeType } diff --git a/config/capz/manager_image_patch.yaml b/config/capz/manager_image_patch.yaml index 0876a1db40db..e3345cd4db44 100644 --- a/config/capz/manager_image_patch.yaml +++ b/config/capz/manager_image_patch.yaml @@ -8,5 +8,5 @@ spec: spec: containers: # Change the value of image field below to your controller image URL - - image: gcr.io/k8s-staging-cluster-api-azure/cluster-api-azure-controller:latest + - image: localhost:5000/ci-e2e/cluster-api-azure-controller-amd64:20230518170459 name: manager diff --git a/config/capz/manager_pull_policy.yaml b/config/capz/manager_pull_policy.yaml index 74a0879c604a..cd7ae12c01ea 100644 --- a/config/capz/manager_pull_policy.yaml +++ b/config/capz/manager_pull_policy.yaml @@ -8,4 +8,4 @@ spec: spec: containers: - name: manager - imagePullPolicy: Always + imagePullPolicy: IfNotPresent diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinepools.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinepools.yaml index 38491aa737f6..baf2e6c392b9 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinepools.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinepools.yaml @@ -512,7 +512,8 @@ spec: in the cluster's node subnet field. The primary interface will be the first networkInterface specified (index 0) in the list. items: - description: NetworkInterface defines a network interface. + description: MachinePoolNetworkInterface defines a VMSS network + interface profile. properties: acceleratedNetworking: description: AcceleratedNetworking enables or disables Azure diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachines.yaml index 74baeb5becc1..28acb1169bf1 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachines.yaml @@ -420,11 +420,26 @@ spec: If AcceleratedNetworking is set to true with a VMSize that does not support it, Azure will return an error. type: boolean + name: + description: Name optionally allows you to override the Azure + NetworkInterface name. If left unspecified, CAPZ will generate + a NetworkInterface name based on the VM name. If specified, + the specified networkInterface does not exist, CAPZ will create + it when the VM is created and delete it when the VM is deleted. + If you specify an existing Azure NetworkInterface, CAPZ will + attach and detach the interface from the VM when the VM is + created and deleted, but CAPZ will not delete the NetworkInterface + if the VM is deleted. + type: string privateIPConfigs: description: PrivateIPConfigs specifies the number of private IP addresses to attach to the interface. Defaults to 1 if not specified. type: integer + resourceGroup: + description: ResourceGroup optionally specifies the resource + group for the NetworkInterface resource. + type: string subnetName: description: SubnetName specifies the subnet in which the new network interface will be placed. diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinetemplates.yaml index c03e2cecb18f..827585365873 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_azuremachinetemplates.yaml @@ -451,11 +451,27 @@ spec: set to true with a VMSize that does not support it, Azure will return an error. type: boolean + name: + description: Name optionally allows you to override + the Azure NetworkInterface name. If left unspecified, + CAPZ will generate a NetworkInterface name based on + the VM name. If specified, the specified networkInterface + does not exist, CAPZ will create it when the VM is + created and delete it when the VM is deleted. If you + specify an existing Azure NetworkInterface, CAPZ will + attach and detach the interface from the VM when the + VM is created and deleted, but CAPZ will not delete + the NetworkInterface if the VM is deleted. + type: string privateIPConfigs: description: PrivateIPConfigs specifies the number of private IP addresses to attach to the interface. Defaults to 1 if not specified. type: integer + resourceGroup: + description: ResourceGroup optionally specifies the + resource group for the NetworkInterface resource. + type: string subnetName: description: SubnetName specifies the subnet in which the new network interface will be placed. diff --git a/docs/book/src/topics/custom-network-interfaces.md b/docs/book/src/topics/custom-network-interfaces.md new file mode 100644 index 000000000000..51a3ded62ced --- /dev/null +++ b/docs/book/src/topics/custom-network-interfaces.md @@ -0,0 +1,57 @@ +# Custom Network Interfaces for AzureMachines + +## Pre-existing Network Interfaces +To deploy an AzureMachine using a pre-existing network interface, set the `AzureMachine` or +`AzureMachineTemplate` spec to include the name and optionally the resource group of the existing network +interface(s) as follows: + +```yaml +apiVersion: infrastructure.cluster.x-k8s.io/v1beta1 +kind: AzureMachineTemplate +metadata: + name: template-byo-nic +spec: + template: + metadata: {} + spec: + networkInterfaces: + - name: byo-nic + resourceGroup: group-byo-nic + acceleratedNetworking: true + privateIPConfigs: 1 + subnetName: byo-nic-node-subnet + osDisk: + cachingType: None + diskSizeGB: 30 + osType: Linux + sshPublicKey: mykey + vmSize: Standard_B2s +``` + +The pre-existing network interface can be in the same resource group or a different resource group in the same +subscription as the target cluster. When deleting the `AzureMachine`, the network interface will only be +deleted if they are "managed" by capz, ie. they were created during `AzureMachine` deployment. Pre-existing +network interfaces will *not* be deleted. If a resource group is specified, it must already exist. CAPZ will +*not* create or delete a resource group that is specific to a network interface. If the resource group is +omitted, it will default to the resourceGroup of the `AzureCluster`. + +## Custom Network Interface +Alternatively, if you specify a network interface name and optionally a resource group, but the network +interface does not exist, CAPZ will create it and manage its lifecycle. In this case, CAPZ *will* delete the +network interface upon `AzureMachine` deletion. If a resource group is specified, it must already exist. CAPZ +will *not* create or delete a resource group that is specific to a network interface. If the resource group is +omitted, it will default to the resourceGroup of the `AzureCluster`. + + diff --git a/exp/api/v1beta1/azuremachinepool_default.go b/exp/api/v1beta1/azuremachinepool_default.go index d2baa14b1739..338ccca39f4d 100644 --- a/exp/api/v1beta1/azuremachinepool_default.go +++ b/exp/api/v1beta1/azuremachinepool_default.go @@ -139,7 +139,7 @@ func (amp *AzureMachinePool) SetNetworkInterfacesDefaults() { } if len(amp.Spec.Template.NetworkInterfaces) == 0 { - amp.Spec.Template.NetworkInterfaces = []infrav1.NetworkInterface{ + amp.Spec.Template.NetworkInterfaces = []infrav1.MachinePoolNetworkInterface{ { SubnetName: amp.Spec.Template.SubnetName, AcceleratedNetworking: amp.Spec.Template.AcceleratedNetworking, diff --git a/exp/api/v1beta1/azuremachinepool_default_test.go b/exp/api/v1beta1/azuremachinepool_default_test.go index d7e57f55ada2..13dbc7f1e775 100644 --- a/exp/api/v1beta1/azuremachinepool_default_test.go +++ b/exp/api/v1beta1/azuremachinepool_default_test.go @@ -262,7 +262,7 @@ func TestAzureMachinePool_SetNetworkInterfacesDefaults(t *testing.T) { Spec: AzureMachinePoolSpec{ Template: AzureMachinePoolMachineTemplate{ SubnetName: "", - NetworkInterfaces: []infrav1.NetworkInterface{ + NetworkInterfaces: []infrav1.MachinePoolNetworkInterface{ { SubnetName: "test-subnet", PrivateIPConfigs: 1, @@ -287,7 +287,7 @@ func TestAzureMachinePool_SetNetworkInterfacesDefaults(t *testing.T) { Template: AzureMachinePoolMachineTemplate{ SubnetName: "", AcceleratedNetworking: nil, - NetworkInterfaces: []infrav1.NetworkInterface{ + NetworkInterfaces: []infrav1.MachinePoolNetworkInterface{ { SubnetName: "test-subnet", PrivateIPConfigs: 1, @@ -304,7 +304,7 @@ func TestAzureMachinePool_SetNetworkInterfacesDefaults(t *testing.T) { Spec: AzureMachinePoolSpec{ Template: AzureMachinePoolMachineTemplate{ SubnetName: "test-subnet", - NetworkInterfaces: []infrav1.NetworkInterface{{ + NetworkInterfaces: []infrav1.MachinePoolNetworkInterface{{ SubnetName: "test-subnet", }}, }, @@ -315,7 +315,7 @@ func TestAzureMachinePool_SetNetworkInterfacesDefaults(t *testing.T) { Template: AzureMachinePoolMachineTemplate{ SubnetName: "test-subnet", AcceleratedNetworking: nil, - NetworkInterfaces: []infrav1.NetworkInterface{ + NetworkInterfaces: []infrav1.MachinePoolNetworkInterface{ { SubnetName: "test-subnet", }, diff --git a/exp/api/v1beta1/azuremachinepool_types.go b/exp/api/v1beta1/azuremachinepool_types.go index 0a91795426f0..7db1a19cba0a 100644 --- a/exp/api/v1beta1/azuremachinepool_types.go +++ b/exp/api/v1beta1/azuremachinepool_types.go @@ -102,7 +102,7 @@ type ( // single IPConfig in the subnet specified in the cluster's node subnet field. // The primary interface will be the first networkInterface specified (index 0) in the list. // +optional - NetworkInterfaces []infrav1.NetworkInterface `json:"networkInterfaces,omitempty"` + NetworkInterfaces []infrav1.MachinePoolNetworkInterface `json:"networkInterfaces,omitempty"` } // AzureMachinePoolSpec defines the desired state of AzureMachinePool. diff --git a/exp/api/v1beta1/azuremachinepool_webhook_test.go b/exp/api/v1beta1/azuremachinepool_webhook_test.go index 31661d07cc06..3d969dfe6bc3 100644 --- a/exp/api/v1beta1/azuremachinepool_webhook_test.go +++ b/exp/api/v1beta1/azuremachinepool_webhook_test.go @@ -204,17 +204,17 @@ func TestAzureMachinePool_ValidateCreate(t *testing.T) { }, { name: "azuremachinepool with valid legacy network configuration", - amp: createMachinePoolWithNetworkConfig("testSubnet", []infrav1.NetworkInterface{}), + amp: createMachinePoolWithNetworkConfig("testSubnet", []infrav1.MachinePoolNetworkInterface{}), wantErr: false, }, { name: "azuremachinepool with invalid legacy network configuration", - amp: createMachinePoolWithNetworkConfig("testSubnet", []infrav1.NetworkInterface{{SubnetName: "testSubnet"}}), + amp: createMachinePoolWithNetworkConfig("testSubnet", []infrav1.MachinePoolNetworkInterface{{SubnetName: "testSubnet"}}), wantErr: true, }, { name: "azuremachinepool with valid networkinterface configuration", - amp: createMachinePoolWithNetworkConfig("", []infrav1.NetworkInterface{{SubnetName: "testSubnet"}}), + amp: createMachinePoolWithNetworkConfig("", []infrav1.MachinePoolNetworkInterface{{SubnetName: "testSubnet"}}), wantErr: false, }, { @@ -365,20 +365,20 @@ func TestAzureMachinePool_ValidateUpdate(t *testing.T) { }, { name: "azuremachinepool with valid network interface config", - oldAMP: createMachinePoolWithNetworkConfig("", []infrav1.NetworkInterface{{SubnetName: "testSubnet"}}), - amp: createMachinePoolWithNetworkConfig("", []infrav1.NetworkInterface{{SubnetName: "testSubnet2"}}), + oldAMP: createMachinePoolWithNetworkConfig("", []infrav1.MachinePoolNetworkInterface{{SubnetName: "testSubnet"}}), + amp: createMachinePoolWithNetworkConfig("", []infrav1.MachinePoolNetworkInterface{{SubnetName: "testSubnet2"}}), wantErr: false, }, { name: "azuremachinepool with valid network interface config", - oldAMP: createMachinePoolWithNetworkConfig("", []infrav1.NetworkInterface{{SubnetName: "testSubnet"}}), - amp: createMachinePoolWithNetworkConfig("subnet", []infrav1.NetworkInterface{{SubnetName: "testSubnet2"}}), + oldAMP: createMachinePoolWithNetworkConfig("", []infrav1.MachinePoolNetworkInterface{{SubnetName: "testSubnet"}}), + amp: createMachinePoolWithNetworkConfig("subnet", []infrav1.MachinePoolNetworkInterface{{SubnetName: "testSubnet2"}}), wantErr: true, }, { name: "azuremachinepool with valid network interface config", - oldAMP: createMachinePoolWithNetworkConfig("subnet", []infrav1.NetworkInterface{}), - amp: createMachinePoolWithNetworkConfig("subnet", []infrav1.NetworkInterface{{SubnetName: "testSubnet2"}}), + oldAMP: createMachinePoolWithNetworkConfig("subnet", []infrav1.MachinePoolNetworkInterface{}), + amp: createMachinePoolWithNetworkConfig("subnet", []infrav1.MachinePoolNetworkInterface{{SubnetName: "testSubnet2"}}), wantErr: true, }, } @@ -530,7 +530,7 @@ func createMachinePoolWithSharedImage(subscriptionID, resourceGroup, name, galle } } -func createMachinePoolWithNetworkConfig(subnetName string, interfaces []infrav1.NetworkInterface) *AzureMachinePool { +func createMachinePoolWithNetworkConfig(subnetName string, interfaces []infrav1.MachinePoolNetworkInterface) *AzureMachinePool { return &AzureMachinePool{ Spec: AzureMachinePoolSpec{ Template: AzureMachinePoolMachineTemplate{ diff --git a/exp/api/v1beta1/zz_generated.deepcopy.go b/exp/api/v1beta1/zz_generated.deepcopy.go index 0e53ab982981..7a9de9bfcaa2 100644 --- a/exp/api/v1beta1/zz_generated.deepcopy.go +++ b/exp/api/v1beta1/zz_generated.deepcopy.go @@ -301,7 +301,7 @@ func (in *AzureMachinePoolMachineTemplate) DeepCopyInto(out *AzureMachinePoolMac } if in.NetworkInterfaces != nil { in, out := &in.NetworkInterfaces, &out.NetworkInterfaces - *out = make([]apiv1beta1.NetworkInterface, len(*in)) + *out = make([]apiv1beta1.MachinePoolNetworkInterface, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) }