Add subnet id to machine spec(v0.9.0 branch) (#293)

* Add Subnet Id and NSG ID to machine spec

api/v1beta1/conversion.go

@@ -17,7 +17,6 @@
 package v1beta1
 
 import (
-	"errors"
 	"github.com/oracle/cluster-api-provider-oci/api/v1beta2"
 	"k8s.io/apimachinery/pkg/conversion"
 )
@@ -115,12 +114,6 @@ func Convert_v1beta1_OCIMachineSpec_To_v1beta2_OCIMachineSpec(in *OCIMachineSpec
 	if err != nil {
 		return err
 	}
-	if in.NetworkDetails.SubnetId != nil {
-		return errors.New("deprecated field NetworkDetails.SubnetId is present in OCIMachineSpec")
-	}
-	if in.NetworkDetails.NSGId != nil {
-		return errors.New("deprecated field NetworkDetails.NSGId is present in OCIMachineSpec")
-	}
 	if in.NSGName != "" && len(in.NetworkDetails.NsgNames) == 0 {
 		out.NetworkDetails.NsgNames = []string{in.NSGName}
 	}

api/v1beta1/conversion_test.go

@@ -40,8 +40,6 @@ func fuzzFuncs(_ runtimeserializer.CodecFactory) []interface{} {
 func OCIMachineFuzzer(obj *OCIMachine, c fuzz.Continue) {
 	c.FuzzNoCustom(obj)
 	// nil fields which have been removed so that tests dont fail
-	obj.Spec.NetworkDetails.NSGId = nil
-	obj.Spec.NetworkDetails.SubnetId = nil
 	obj.Spec.NSGName = ""
 }
 
@@ -92,8 +90,6 @@ func OCIClusterTemplateFuzzer(obj *OCIClusterTemplate, c fuzz.Continue) {
 func OCIMachineTemplateFuzzer(obj *OCIMachineTemplate, c fuzz.Continue) {
 	c.FuzzNoCustom(obj)
 	// nil fields which ave been removed so that tests dont fail
-	obj.Spec.Template.Spec.NetworkDetails.NSGId = nil
-	obj.Spec.Template.Spec.NetworkDetails.SubnetId = nil
 	obj.Spec.Template.Spec.NSGName = ""
 }
 

api/v1beta1/types.go

@@ -34,8 +34,7 @@ var OCIManagedClusterSubnetRoles = []Role{PodRole, ControlPlaneEndpointRole, Wor
 
 // NetworkDetails defines the configuration options for the network
 type NetworkDetails struct {
-	// SubnetId defines the ID of the subnet to use.
-	// Deprecated, use SubnetName parameter
+	// SubnetId defines the ID of the subnet to use. This parameter takes priority over SubnetName.
 	SubnetId *string `json:"subnetId,omitempty"`
 
 	// AssignPublicIp defines whether the instance should have a public IP address
@@ -44,7 +43,7 @@ type NetworkDetails struct {
 	// SubnetName defines the subnet name to use for the VNIC
 	SubnetName string `json:"subnetName,omitempty"`
 
-	// Deprecated, use 	NsgNames parameter to define the NSGs
+	// NSGId defines the ID of the NSG to use. This parameter takes priority over NsgNames.
 	NSGId *string `json:"nsgId,omitempty"`
 
 	// SkipSourceDestCheck defines whether the source/destination check is disabled on the VNIC.

api/v1beta2/types.go

@@ -34,6 +34,9 @@ var OCIManagedClusterSubnetRoles = []Role{PodRole, ControlPlaneEndpointRole, Wor
 
 // NetworkDetails defines the configuration options for the network
 type NetworkDetails struct {
+	// SubnetId defines the ID of the subnet to use. This parameter takes priority over SubnetName.
+	SubnetId *string `json:"subnetId,omitempty"`
+
 	// AssignPublicIp defines whether the instance should have a public IP address
 	AssignPublicIp bool `json:"assignPublicIp,omitempty"`
 
@@ -43,6 +46,9 @@ type NetworkDetails struct {
 	// SkipSourceDestCheck defines whether the source/destination check is disabled on the VNIC.
 	SkipSourceDestCheck *bool `json:"skipSourceDestCheck,omitempty"`
 
+	// NSGId defines the ID of the NSG to use. This parameter takes priority over NsgNames.
+	NSGId *string `json:"nsgId,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"`
 

cloud/scope/machine.go

@@ -178,18 +178,25 @@ func (m *MachineScope) GetOrCreateMachine(ctx context.Context) (*core.Instance,
 		sourceDetails.BootVolumeVpusPerGB = m.OCIMachine.Spec.InstanceSourceViaImageDetails.BootVolumeVpusPerGB
 	}
 
-	var subnetId *string
-	if m.IsControlPlane() {
-		subnetId = m.getGetControlPlaneMachineSubnet()
-	} else {
-		subnetId = m.getWorkerMachineSubnet()
+	subnetId := m.OCIMachine.Spec.NetworkDetails.SubnetId
+	if subnetId == nil {
+		if m.IsControlPlane() {
+			subnetId = m.getGetControlPlaneMachineSubnet()
+		} else {
+			subnetId = m.getWorkerMachineSubnet()
+		}
 	}
 
 	var nsgIds []string
-	if m.IsControlPlane() {
-		nsgIds = m.getGetControlPlaneMachineNSGs()
+	nsgId := m.OCIMachine.Spec.NetworkDetails.NSGId
+	if nsgId != nil {
+		nsgIds = []string{*nsgId}
 	} else {
-		nsgIds = m.getWorkerMachineNSGs()
+		if m.IsControlPlane() {
+			nsgIds = m.getGetControlPlaneMachineNSGs()
+		} else {
+			nsgIds = m.getWorkerMachineNSGs()
+		}
 	}
 
 	failureDomain := m.Machine.Spec.FailureDomain

cloud/scope/machine_test.go

@@ -381,6 +381,74 @@ func TestInstanceReconciliation(t *testing.T) {
 					OpcRetryToken:         ociutil.GetOPCRetryToken("machineuid")})).Return(core.LaunchInstanceResponse{}, nil)
 			},
 		},
+		{
+			name:          "check all params together, with subnet id set",
+			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.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"},
+						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,

config/crd/bases/infrastructure.cluster.x-k8s.io_ocimachinepools.yaml

@@ -204,8 +204,8 @@ spec:
                           primary private IP. Used for DNS.
                         type: string
                       nsgId:
-                        description: "Deprecated, use \tNsgNames parameter to define
-                          the NSGs"
+                        description: NSGId defines the ID of the NSG to use. This
+                          parameter takes priority over NsgNames.
                         type: string
                       nsgNames:
                         description: NsgNames defines a list of the nsg names of the
@@ -219,7 +219,7 @@ spec:
                         type: boolean
                       subnetId:
                         description: SubnetId defines the ID of the subnet to use.
-                          Deprecated, use SubnetName parameter
+                          This parameter takes priority over SubnetName.
                         type: string
                       subnetName:
                         description: SubnetName defines the subnet name to use for
@@ -920,6 +920,10 @@ spec:
                         description: HostnameLabel defines the hostname for the VNIC's
                           primary private IP. Used for DNS.
                         type: string
+                      nsgId:
+                        description: NSGId defines the ID of the NSG to use. This
+                          parameter takes priority over NsgNames.
+                        type: string
                       nsgNames:
                         description: NsgNames defines a list of the nsg names of the
                           network security groups (NSGs) to add the VNIC to.
@@ -930,6 +934,10 @@ spec:
                         description: SkipSourceDestCheck defines whether the source/destination
                           check is disabled on the VNIC.
                         type: boolean
+                      subnetId:
+                        description: SubnetId defines the ID of the subnet to use.
+                          This parameter takes priority over SubnetName.
+                        type: string
                       subnetName:
                         description: SubnetName defines the subnet name to use for
                           the VNIC

config/crd/bases/infrastructure.cluster.x-k8s.io_ocimachines.yaml

@@ -270,8 +270,8 @@ spec:
                       primary private IP. Used for DNS.
                     type: string
                   nsgId:
-                    description: "Deprecated, use \tNsgNames parameter to define the
-                      NSGs"
+                    description: NSGId defines the ID of the NSG to use. This parameter
+                      takes priority over NsgNames.
                     type: string
                   nsgNames:
                     description: NsgNames defines a list of the nsg names of the network
@@ -284,8 +284,8 @@ spec:
                       check is disabled on the VNIC.
                     type: boolean
                   subnetId:
-                    description: SubnetId defines the ID of the subnet to use. Deprecated,
-                      use SubnetName parameter
+                    description: SubnetId defines the ID of the subnet to use. This
+                      parameter takes priority over SubnetName.
                     type: string
                   subnetName:
                     description: SubnetName defines the subnet name to use for the
@@ -1030,6 +1030,10 @@ spec:
                     description: HostnameLabel defines the hostname for the VNIC's
                       primary private IP. Used for DNS.
                     type: string
+                  nsgId:
+                    description: NSGId defines the ID of the NSG to use. This parameter
+                      takes priority over NsgNames.
+                    type: string
                   nsgNames:
                     description: NsgNames defines a list of the nsg names of the network
                       security groups (NSGs) to add the VNIC to.
@@ -1040,6 +1044,10 @@ spec:
                     description: SkipSourceDestCheck defines whether the source/destination
                       check is disabled on the VNIC.
                     type: boolean
+                  subnetId:
+                    description: SubnetId defines the ID of the subnet to use. This
+                      parameter takes priority over SubnetName.
+                    type: string
                   subnetName:
                     description: SubnetName defines the subnet name to use for the
                       VNIC

config/crd/bases/infrastructure.cluster.x-k8s.io_ocimachinetemplates.yaml

@@ -295,8 +295,8 @@ spec:
                               VNIC's primary private IP. Used for DNS.
                             type: string
                           nsgId:
-                            description: "Deprecated, use \tNsgNames parameter to
-                              define the NSGs"
+                            description: NSGId defines the ID of the NSG to use. This
+                              parameter takes priority over NsgNames.
                             type: string
                           nsgNames:
                             description: NsgNames defines a list of the nsg names
@@ -311,7 +311,7 @@ spec:
                             type: boolean
                           subnetId:
                             description: SubnetId defines the ID of the subnet to
-                              use. Deprecated, use SubnetName parameter
+                              use. This parameter takes priority over SubnetName.
                             type: string
                           subnetName:
                             description: SubnetName defines the subnet name to use
@@ -1034,6 +1034,10 @@ spec:
                             description: HostnameLabel defines the hostname for the
                               VNIC's primary private IP. Used for DNS.
                             type: string
+                          nsgId:
+                            description: NSGId defines the ID of the NSG to use. This
+                              parameter takes priority over NsgNames.
+                            type: string
                           nsgNames:
                             description: NsgNames defines a list of the nsg names
                               of the network security groups (NSGs) to add the VNIC
@@ -1045,6 +1049,10 @@ spec:
                             description: SkipSourceDestCheck defines whether the source/destination
                               check is disabled on the VNIC.
                             type: boolean
+                          subnetId:
+                            description: SubnetId defines the ID of the subnet to
+                              use. This parameter takes priority over SubnetName.
+                            type: string
                           subnetName:
                             description: SubnetName defines the subnet name to use
                               for the VNIC