From 0f8f8f1743a3083358672ae807d7694a7a6b7b76 Mon Sep 17 00:00:00 2001 From: Shyam Radhakrishnan Date: Thu, 14 Mar 2024 20:02:57 +0530 Subject: [PATCH] Add support for multiple NSGs in machine spec (#356) --- api/v1beta1/types.go | 4 ++ api/v1beta1/zz_generated.conversion.go | 2 + api/v1beta1/zz_generated.deepcopy.go | 5 ++ api/v1beta2/types.go | 4 ++ api/v1beta2/zz_generated.deepcopy.go | 5 ++ cloud/scope/machine.go | 5 +- cloud/scope/machine_test.go | 70 +++++++++++++++++++ ...ture.cluster.x-k8s.io_ocimachinepools.yaml | 18 ++++- ...tructure.cluster.x-k8s.io_ocimachines.yaml | 16 ++++- ....cluster.x-k8s.io_ocimachinetemplates.yaml | 18 ++++- 10 files changed, 140 insertions(+), 7 deletions(-) diff --git a/api/v1beta1/types.go b/api/v1beta1/types.go index 7d6d609f..a6e8a83d 100644 --- a/api/v1beta1/types.go +++ b/api/v1beta1/types.go @@ -44,8 +44,12 @@ type NetworkDetails struct { SubnetName string `json:"subnetName,omitempty"` // NSGId defines the ID of the NSG to use. This parameter takes priority over NsgNames. + // Deprecated, please use NetworkDetails.NSGIds NSGId *string `json:"nsgId,omitempty"` + // NSGIds defines the list of NSG IDs to use. This parameter takes priority over NsgNames. + NSGIds []string `json:"nsgIds,omitempty"` + // SkipSourceDestCheck defines whether the source/destination check is disabled on the VNIC. SkipSourceDestCheck *bool `json:"skipSourceDestCheck,omitempty"` diff --git a/api/v1beta1/zz_generated.conversion.go b/api/v1beta1/zz_generated.conversion.go index 2956d670..9ff820b5 100644 --- a/api/v1beta1/zz_generated.conversion.go +++ b/api/v1beta1/zz_generated.conversion.go @@ -1674,6 +1674,7 @@ func autoConvert_v1beta1_NetworkDetails_To_v1beta2_NetworkDetails(in *NetworkDet out.AssignPublicIp = in.AssignPublicIp out.SubnetName = in.SubnetName out.NSGId = (*string)(unsafe.Pointer(in.NSGId)) + out.NSGIds = *(*[]string)(unsafe.Pointer(&in.NSGIds)) out.SkipSourceDestCheck = (*bool)(unsafe.Pointer(in.SkipSourceDestCheck)) out.NsgNames = *(*[]string)(unsafe.Pointer(&in.NsgNames)) out.HostnameLabel = (*string)(unsafe.Pointer(in.HostnameLabel)) @@ -1688,6 +1689,7 @@ func autoConvert_v1beta2_NetworkDetails_To_v1beta1_NetworkDetails(in *v1beta2.Ne out.SubnetName = in.SubnetName out.SkipSourceDestCheck = (*bool)(unsafe.Pointer(in.SkipSourceDestCheck)) out.NSGId = (*string)(unsafe.Pointer(in.NSGId)) + out.NSGIds = *(*[]string)(unsafe.Pointer(&in.NSGIds)) out.NsgNames = *(*[]string)(unsafe.Pointer(&in.NsgNames)) out.HostnameLabel = (*string)(unsafe.Pointer(in.HostnameLabel)) out.DisplayName = (*string)(unsafe.Pointer(in.DisplayName)) diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 247c3d78..c2ef267f 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -947,6 +947,11 @@ func (in *NetworkDetails) DeepCopyInto(out *NetworkDetails) { *out = new(string) **out = **in } + if in.NSGIds != nil { + in, out := &in.NSGIds, &out.NSGIds + *out = make([]string, len(*in)) + copy(*out, *in) + } if in.SkipSourceDestCheck != nil { in, out := &in.SkipSourceDestCheck, &out.SkipSourceDestCheck *out = new(bool) diff --git a/api/v1beta2/types.go b/api/v1beta2/types.go index f7be929b..6ce40b5f 100644 --- a/api/v1beta2/types.go +++ b/api/v1beta2/types.go @@ -48,8 +48,12 @@ type NetworkDetails struct { SkipSourceDestCheck *bool `json:"skipSourceDestCheck,omitempty"` // NSGId defines the ID of the NSG to use. This parameter takes priority over NsgNames. + // Deprecated, please use NetworkDetails.NSGIds NSGId *string `json:"nsgId,omitempty"` + // NSGIds defines the list of NSG IDs to use. This parameter takes priority over NsgNames. + NSGIds []string `json:"nsgIds,omitempty"` + // NsgNames defines a list of the nsg names of the network security groups (NSGs) to add the VNIC to. NsgNames []string `json:"nsgNames,omitempty"` diff --git a/api/v1beta2/zz_generated.deepcopy.go b/api/v1beta2/zz_generated.deepcopy.go index 3d1c7f4b..1f51bdc5 100644 --- a/api/v1beta2/zz_generated.deepcopy.go +++ b/api/v1beta2/zz_generated.deepcopy.go @@ -1154,6 +1154,11 @@ func (in *NetworkDetails) DeepCopyInto(out *NetworkDetails) { *out = new(string) **out = **in } + if in.NSGIds != nil { + in, out := &in.NSGIds, &out.NSGIds + *out = make([]string, len(*in)) + copy(*out, *in) + } if in.NsgNames != nil { in, out := &in.NsgNames, &out.NsgNames *out = make([]string, len(*in)) diff --git a/cloud/scope/machine.go b/cloud/scope/machine.go index bfa713c0..08d5d521 100644 --- a/cloud/scope/machine.go +++ b/cloud/scope/machine.go @@ -188,8 +188,11 @@ func (m *MachineScope) GetOrCreateMachine(ctx context.Context) (*core.Instance, } var nsgIds []string + machineNsgIds := m.OCIMachine.Spec.NetworkDetails.NSGIds nsgId := m.OCIMachine.Spec.NetworkDetails.NSGId - if nsgId != nil { + if machineNsgIds != nil && len(machineNsgIds) > 0 { + nsgIds = machineNsgIds + } else if nsgId != nil { nsgIds = []string{*nsgId} } else { if m.IsControlPlane() { diff --git a/cloud/scope/machine_test.go b/cloud/scope/machine_test.go index 7fe3a80b..1f94adb5 100644 --- a/cloud/scope/machine_test.go +++ b/cloud/scope/machine_test.go @@ -455,6 +455,76 @@ func TestInstanceReconciliation(t *testing.T) { OpcRetryToken: ociutil.GetOPCRetryToken("machineuid")})).Return(core.LaunchInstanceResponse{}, nil) }, }, + { + name: "check all params together, with subnet id set, nsg id list", + errorExpected: false, + testSpecificSetup: func(machineScope *MachineScope, computeClient *mock_compute.MockComputeClient) { + setupAllParams(ms) + ms.OCIMachine.Spec.CapacityReservationId = common.String("cap-id") + ms.OCIMachine.Spec.DedicatedVmHostId = common.String("dedicated-host-id") + ms.OCIMachine.Spec.NetworkDetails.HostnameLabel = common.String("hostname-label") + ms.OCIMachine.Spec.NetworkDetails.SubnetId = common.String("subnet-machine-id") + ms.OCIMachine.Spec.NetworkDetails.NSGIds = []string{"nsg-machine-id-1", "nsg-machine-id-2"} + // above array should take precedence + ms.OCIMachine.Spec.NetworkDetails.NSGId = common.String("nsg-machine-id") + ms.OCIMachine.Spec.NetworkDetails.SkipSourceDestCheck = common.Bool(true) + ms.OCIMachine.Spec.NetworkDetails.AssignPrivateDnsRecord = common.Bool(true) + ms.OCIMachine.Spec.NetworkDetails.DisplayName = common.String("display-name") + ms.OCIMachine.Spec.InstanceSourceViaImageDetails = &infrastructurev1beta2.InstanceSourceViaImageConfig{ + KmsKeyId: common.String("kms-key-id"), + BootVolumeVpusPerGB: common.Int64(32), + } + computeClient.EXPECT().ListInstances(gomock.Any(), gomock.Eq(core.ListInstancesRequest{ + DisplayName: common.String("name"), + CompartmentId: common.String("test"), + })).Return(core.ListInstancesResponse{}, nil) + + launchDetails := core.LaunchInstanceDetails{DisplayName: common.String("name"), + CapacityReservationId: common.String("cap-id"), + DedicatedVmHostId: common.String("dedicated-host-id"), + SourceDetails: core.InstanceSourceViaImageDetails{ + ImageId: common.String("image"), + BootVolumeSizeInGBs: common.Int64(120), + KmsKeyId: common.String("kms-key-id"), + BootVolumeVpusPerGB: common.Int64(32), + }, + CreateVnicDetails: &core.CreateVnicDetails{ + SubnetId: common.String("subnet-machine-id"), + AssignPublicIp: common.Bool(false), + DefinedTags: map[string]map[string]interface{}{}, + FreeformTags: map[string]string{ + ociutil.CreatedBy: ociutil.OCIClusterAPIProvider, + ociutil.ClusterResourceIdentifier: "resource_uid", + }, + NsgIds: []string{"nsg-machine-id-1", "nsg-machine-id-2"}, + HostnameLabel: common.String("hostname-label"), + SkipSourceDestCheck: common.Bool(true), + AssignPrivateDnsRecord: common.Bool(true), + DisplayName: common.String("display-name"), + }, + Metadata: map[string]string{ + "user_data": base64.StdEncoding.EncodeToString([]byte("test")), + }, + Shape: common.String("shape"), + ShapeConfig: &core.LaunchInstanceShapeConfigDetails{ + Ocpus: common.Float32(2), + MemoryInGBs: common.Float32(100), + BaselineOcpuUtilization: core.LaunchInstanceShapeConfigDetailsBaselineOcpuUtilization8, + }, + AvailabilityDomain: common.String("ad2"), + CompartmentId: common.String("test"), + IsPvEncryptionInTransitEnabled: common.Bool(true), + DefinedTags: map[string]map[string]interface{}{}, + FreeformTags: map[string]string{ + ociutil.CreatedBy: ociutil.OCIClusterAPIProvider, + ociutil.ClusterResourceIdentifier: "resource_uid", + }, + } + computeClient.EXPECT().LaunchInstance(gomock.Any(), gomock.Eq(core.LaunchInstanceRequest{ + LaunchInstanceDetails: launchDetails, + OpcRetryToken: ociutil.GetOPCRetryToken("machineuid")})).Return(core.LaunchInstanceResponse{}, nil) + }, + }, { name: "shape config is empty", errorExpected: false, diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_ocimachinepools.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_ocimachinepools.yaml index a2c49c73..c3c1d317 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_ocimachinepools.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_ocimachinepools.yaml @@ -205,8 +205,15 @@ spec: type: string nsgId: description: NSGId defines the ID of the NSG to use. This - parameter takes priority over NsgNames. + parameter takes priority over NsgNames. Deprecated, please + use NetworkDetails.NSGIds type: string + nsgIds: + description: NSGIds defines the list of NSG IDs to use. This + parameter takes priority over NsgNames. + items: + type: string + type: array nsgNames: description: NsgNames defines a list of the nsg names of the network security groups (NSGs) to add the VNIC to. @@ -926,8 +933,15 @@ spec: type: string nsgId: description: NSGId defines the ID of the NSG to use. This - parameter takes priority over NsgNames. + parameter takes priority over NsgNames. Deprecated, please + use NetworkDetails.NSGIds type: string + nsgIds: + description: NSGIds defines the list of NSG IDs to use. This + parameter takes priority over NsgNames. + items: + type: string + type: array nsgNames: description: NsgNames defines a list of the nsg names of the network security groups (NSGs) to add the VNIC to. diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_ocimachines.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_ocimachines.yaml index a34096cf..e76cf247 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_ocimachines.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_ocimachines.yaml @@ -278,8 +278,14 @@ spec: type: string nsgId: description: NSGId defines the ID of the NSG to use. This parameter - takes priority over NsgNames. + takes priority over NsgNames. Deprecated, please use NetworkDetails.NSGIds type: string + nsgIds: + description: NSGIds defines the list of NSG IDs to use. This parameter + takes priority over NsgNames. + items: + type: string + type: array nsgNames: description: NsgNames defines a list of the nsg names of the network security groups (NSGs) to add the VNIC to. @@ -1046,8 +1052,14 @@ spec: type: string nsgId: description: NSGId defines the ID of the NSG to use. This parameter - takes priority over NsgNames. + takes priority over NsgNames. Deprecated, please use NetworkDetails.NSGIds type: string + nsgIds: + description: NSGIds defines the list of NSG IDs to use. This parameter + takes priority over NsgNames. + items: + type: string + type: array nsgNames: description: NsgNames defines a list of the nsg names of the network security groups (NSGs) to add the VNIC to. diff --git a/config/crd/bases/infrastructure.cluster.x-k8s.io_ocimachinetemplates.yaml b/config/crd/bases/infrastructure.cluster.x-k8s.io_ocimachinetemplates.yaml index 1aed9e43..6bfb313e 100644 --- a/config/crd/bases/infrastructure.cluster.x-k8s.io_ocimachinetemplates.yaml +++ b/config/crd/bases/infrastructure.cluster.x-k8s.io_ocimachinetemplates.yaml @@ -304,8 +304,15 @@ spec: type: string nsgId: description: NSGId defines the ID of the NSG to use. This - parameter takes priority over NsgNames. + parameter takes priority over NsgNames. Deprecated, + please use NetworkDetails.NSGIds type: string + nsgIds: + description: NSGIds defines the list of NSG IDs to use. + This parameter takes priority over NsgNames. + items: + type: string + type: array nsgNames: description: NsgNames defines a list of the nsg names of the network security groups (NSGs) to add the VNIC @@ -1052,8 +1059,15 @@ spec: type: string nsgId: description: NSGId defines the ID of the NSG to use. This - parameter takes priority over NsgNames. + parameter takes priority over NsgNames. Deprecated, + please use NetworkDetails.NSGIds type: string + nsgIds: + description: NSGIds defines the list of NSG IDs to use. + This parameter takes priority over NsgNames. + items: + type: string + type: array nsgNames: description: NsgNames defines a list of the nsg names of the network security groups (NSGs) to add the VNIC