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

[improvement] : store subnet id in linodeVPC and remove GetVPC call #548

Merged
merged 3 commits into from
Oct 24, 2024
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
4 changes: 4 additions & 0 deletions api/v1alpha1/conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,3 +96,7 @@ func Convert_v1alpha2_LinodeClusterSpec_To_v1alpha1_LinodeClusterSpec(in *infras
// VLAN is not supported in v1alpha1
return autoConvert_v1alpha2_LinodeClusterSpec_To_v1alpha1_LinodeClusterSpec(in, out, scope)
}

func Convert_v1alpha2_VPCSubnetCreateOptions_To_v1alpha1_VPCSubnetCreateOptions(in *infrastructurev1alpha2.VPCSubnetCreateOptions, out *VPCSubnetCreateOptions, scope conversion.Scope) error {
return autoConvert_v1alpha2_VPCSubnetCreateOptions_To_v1alpha1_VPCSubnetCreateOptions(in, out, scope)
}
64 changes: 50 additions & 14 deletions api/v1alpha1/zz_generated.conversion.go

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

4 changes: 3 additions & 1 deletion api/v1alpha2/linodevpc_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ type LinodeVPCSpec struct {
Description string `json:"description,omitempty"`
// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
Region string `json:"region"`
// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
// +optional
Subnets []VPCSubnetCreateOptions `json:"subnets,omitempty"`

Expand All @@ -54,6 +53,9 @@ type VPCSubnetCreateOptions struct {
Label string `json:"label,omitempty"`
// +optional
IPv4 string `json:"ipv4,omitempty"`
// SubnetID is subnet id for the subnet
// +optional
SubnetID int `json:"subnetID,omitempty"`
}

// LinodeVPCStatus defines the observed state of LinodeVPC
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,11 +254,11 @@ spec:
maxLength: 63
minLength: 3
type: string
subnetID:
description: SubnetID is subnet id for the subnet
type: integer
type: object
type: array
x-kubernetes-validations:
- message: Value is immutable
rule: self == oldSelf
vpcID:
type: integer
required:
Expand Down
25 changes: 6 additions & 19 deletions controller/linodemachine_controller_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
"net/http"
"net/netip"
"slices"
"sort"
"time"

"github.com/go-logr/logr"
Expand Down Expand Up @@ -403,29 +402,17 @@
return nil, errors.New("vpc is not available")
}

var subnetID int
vpc, err := machineScope.LinodeClient.GetVPC(ctx, *linodeVPC.Spec.VPCID)
if err != nil {
logger.Error(err, "Failed to fetch LinodeVPC")

return nil, err
}
if vpc == nil {
logger.Error(nil, "Failed to fetch VPC")

return nil, errors.New("failed to fetch VPC")
}
if len(vpc.Subnets) == 0 {
if len(linodeVPC.Spec.Subnets) == 0 {
logger.Error(nil, "Failed to find subnet")

return nil, errors.New("failed to find subnet")
}
// Place node into the least busy subnet
sort.Slice(vpc.Subnets, func(i, j int) bool {
return len(vpc.Subnets[i].Linodes) > len(vpc.Subnets[j].Linodes)
})
komer3 marked this conversation as resolved.
Show resolved Hide resolved

subnetID = vpc.Subnets[0].ID
subnetID := linodeVPC.Spec.Subnets[0].SubnetID
if subnetID == 0 {
return nil, errors.New("failed to find subnet as subnet id set is 0")
}

Check warning on line 414 in controller/linodemachine_controller_helpers.go

View check run for this annotation

Codecov / codecov/patch

controller/linodemachine_controller_helpers.go#L413-L414

Added lines #L413 - L414 were not covered by tests

for i, netInterface := range interfaces {
if netInterface.Purpose == linodego.InterfacePurposeVPC {
interfaces[i].SubnetID = &subnetID
Expand Down
18 changes: 9 additions & 9 deletions controller/linodemachine_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1783,9 +1783,15 @@ var _ = Describe("machine in VPC", Label("machine", "VPC"), Ordered, func() {
UID: "5123122",
},
Spec: infrav1alpha2.LinodeVPCSpec{
VPCID: ptr.To(1),
Region: "us-ord",
Subnets: []infrav1alpha2.VPCSubnetCreateOptions{},
VPCID: ptr.To(1),
Region: "us-ord",
Subnets: []infrav1alpha2.VPCSubnetCreateOptions{
{
IPv4: "10.0.0.0/8",
SubnetID: 1,
Label: "test",
},
},
},
Status: infrav1alpha2.LinodeVPCStatus{
Ready: true,
Expand Down Expand Up @@ -1849,9 +1855,6 @@ var _ = Describe("machine in VPC", Label("machine", "VPC"), Ordered, func() {
Return([]linodego.VPC{}, nil)
mockLinodeClient.EXPECT().
CreateVPC(ctx, gomock.Any()).
Return(&linodego.VPC{ID: 1}, nil)
mockLinodeClient.EXPECT().
GetVPC(ctx, gomock.Any()).
Return(&linodego.VPC{ID: 1, Subnets: []linodego.VPCSubnet{{
ID: 1,
Label: "test",
Expand Down Expand Up @@ -1927,9 +1930,6 @@ var _ = Describe("machine in VPC", Label("machine", "VPC"), Ordered, func() {
Return([]linodego.VPC{}, nil)
mockLinodeClient.EXPECT().
CreateVPC(ctx, gomock.Any()).
Return(&linodego.VPC{ID: 1}, nil)
mockLinodeClient.EXPECT().
GetVPC(ctx, gomock.Any()).
Return(&linodego.VPC{ID: 1, Subnets: []linodego.VPCSubnet{{
ID: 1,
Label: "test",
Expand Down
14 changes: 14 additions & 0 deletions controller/linodevpc_controller_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ func reconcileVPC(ctx context.Context, vpcScope *scope.VPCScope, logger logr.Log
} else if len(vpcs) != 0 {
// Labels are unique
vpcScope.LinodeVPC.Spec.VPCID = &vpcs[0].ID
updateVPCSpecSubnets(vpcScope, &vpcs[0])

return nil
}
Expand All @@ -75,10 +76,23 @@ func reconcileVPC(ctx context.Context, vpcScope *scope.VPCScope, logger logr.Log
}

vpcScope.LinodeVPC.Spec.VPCID = &vpc.ID
updateVPCSpecSubnets(vpcScope, vpc)

return nil
}

// updateVPCSpecSubnets updates Subnets in linodeVPC spec and adds linode specific ID to them
func updateVPCSpecSubnets(vpcScope *scope.VPCScope, vpc *linodego.VPC) {
for i, specSubnet := range vpcScope.LinodeVPC.Spec.Subnets {
for _, vpcSubnet := range vpc.Subnets {
if specSubnet.Label == vpcSubnet.Label {
vpcScope.LinodeVPC.Spec.Subnets[i].SubnetID = vpcSubnet.ID
break
}
}
}
}

func linodeVPCSpecToVPCCreateConfig(vpcSpec infrav1alpha2.LinodeVPCSpec) *linodego.VPCCreateOptions {
var buf bytes.Buffer
enc := gob.NewEncoder(&buf)
Expand Down
Loading