Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions api/v1beta1/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
package v1beta1

import (
"errors"
"github.com/oracle/cluster-api-provider-oci/api/v1beta2"
"k8s.io/apimachinery/pkg/conversion"
)
Expand Down Expand Up @@ -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}
}
Expand Down
4 changes: 0 additions & 4 deletions api/v1beta1/conversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ""
}

Expand Down Expand Up @@ -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 = ""
}

Expand Down
5 changes: 2 additions & 3 deletions api/v1beta1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down
6 changes: 4 additions & 2 deletions api/v1beta1/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions api/v1beta2/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`

Expand All @@ -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"`

Expand Down
10 changes: 10 additions & 0 deletions api/v1beta2/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 15 additions & 8 deletions cloud/scope/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
68 changes: 68 additions & 0 deletions cloud/scope/machine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down
16 changes: 12 additions & 4 deletions config/crd/bases/infrastructure.cluster.x-k8s.io_ocimachines.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down