Skip to content

Commit

Permalink
[improvement] : simplify addr calculation and fetch instance config o…
Browse files Browse the repository at this point in the history 
…nly for vlans (#551)

* simplify addr calculation and fetch instance config only when using vlans

* add test for instance in vlan

* address review comment
  • Loading branch information
@rahulait
rahulait authored Oct 25, 2024
1 parent 4e8e3ce commit 6696734
Show file tree
Hide file tree
Showing 2 changed files with 193 additions and 65 deletions.
37 changes: 21 additions & 16 deletions controller/linodemachine_controller_helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,12 +170,6 @@ func buildInstanceAddrs(ctx context.Context, machineScope *scope.MachineScope, i
return nil, fmt.Errorf("get instance ips: %w", err)
}

// get the default instance config
configs, err := machineScope.LinodeClient.ListInstanceConfigs(ctx, instanceID, &linodego.ListOptions{})
if err != nil || len(configs) == 0 {
return nil, fmt.Errorf("list instance configs: %w", err)
}

ips := []clusterv1.MachineAddress{}
// check if a node has public ipv4 ip and store it
if len(addresses.IPv4.Public) == 0 {
Expand All @@ -198,21 +192,32 @@ func buildInstanceAddrs(ctx context.Context, machineScope *scope.MachineScope, i
Type: clusterv1.MachineExternalIP,
})

// Iterate over interfaces in config and find VPC or VLAN specific ips
for _, iface := range configs[0].Interfaces {
if iface.VPCID != nil && iface.IPv4.VPC != "" {
// check if a node has vpc specific ip and store it
for _, vpcIP := range addresses.IPv4.VPC {
if *vpcIP.Address != "" {
ips = append(ips, clusterv1.MachineAddress{
Address: iface.IPv4.VPC,
Address: *vpcIP.Address,
Type: clusterv1.MachineInternalIP,
})
}
}

if iface.Purpose == linodego.InterfacePurposeVLAN {
// vlan addresses have a /11 appended to them - we should strip it out.
ips = append(ips, clusterv1.MachineAddress{
Address: netip.MustParsePrefix(iface.IPAMAddress).Addr().String(),
Type: clusterv1.MachineInternalIP,
})
if machineScope.LinodeCluster.Spec.Network.UseVlan {
// get the default instance config
configs, err := machineScope.LinodeClient.ListInstanceConfigs(ctx, instanceID, &linodego.ListOptions{})
if err != nil || len(configs) == 0 {
return nil, fmt.Errorf("list instance configs: %w", err)
}

// Iterate over interfaces in config and find VLAN specific ips
for _, iface := range configs[0].Interfaces {
if iface.Purpose == linodego.InterfacePurposeVLAN {
// vlan addresses have a /11 appended to them - we should strip it out.
ips = append(ips, clusterv1.MachineAddress{
Address: netip.MustParsePrefix(iface.IPAMAddress).Addr().String(),
Type: clusterv1.MachineInternalIP,
})
}
}
}

Expand Down
221 changes: 172 additions & 49 deletions controller/linodemachine_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ var _ = Describe("create", Label("machine", "create"), func() {
BootInstance(ctx, 123, 0).
After(createInst).
Return(nil)
getAddrs := mockLinodeClient.EXPECT().
mockLinodeClient.EXPECT().
GetInstanceIPAddresses(ctx, 123).
After(bootInst).
Return(&linodego.InstanceIPAddressResponse{
Expand All @@ -243,14 +243,6 @@ var _ = Describe("create", Label("machine", "create"), func() {
},
},
}, nil)
mockLinodeClient.EXPECT().
ListInstanceConfigs(ctx, 123, gomock.Any()).
After(getAddrs).
Return([]linodego.InstanceConfig{{
Devices: &linodego.InstanceConfigDeviceMap{
SDA: &linodego.InstanceConfigDevice{DiskID: 100},
},
}}, nil)
linodeMachine.Spec.FirewallRef = &corev1.ObjectReference{Name: "fw2", Namespace: defaultNamespace, Kind: "LinodeFirewall", APIVersion: "infrastructure.cluster.x-k8s.io/v1alpha2"}
mScope := scope.MachineScope{
Client: k8sClient,
Expand Down Expand Up @@ -295,7 +287,7 @@ var _ = Describe("create", Label("machine", "create"), func() {
BootInstance(ctx, 123, 0).
After(createInst).
Return(nil)
getAddrs := mockLinodeClient.EXPECT().
mockLinodeClient.EXPECT().
GetInstanceIPAddresses(ctx, 123).
After(bootInst).
Return(&linodego.InstanceIPAddressResponse{
Expand All @@ -309,14 +301,6 @@ var _ = Describe("create", Label("machine", "create"), func() {
},
},
}, nil)
mockLinodeClient.EXPECT().
ListInstanceConfigs(ctx, 123, gomock.Any()).
After(getAddrs).
Return([]linodego.InstanceConfig{{
Devices: &linodego.InstanceConfigDeviceMap{
SDA: &linodego.InstanceConfigDevice{DiskID: 100},
},
}}, nil)

mScope := scope.MachineScope{
Client: k8sClient,
Expand Down Expand Up @@ -536,7 +520,7 @@ var _ = Describe("create", Label("machine", "create"), func() {
}).
After(getAddrs).MaxTimes(2).
Return(nil, nil)
getAddrs = mockLinodeClient.EXPECT().
mockLinodeClient.EXPECT().
GetInstanceIPAddresses(ctx, 123).
After(createNB).
Return(&linodego.InstanceIPAddressResponse{
Expand All @@ -550,14 +534,6 @@ var _ = Describe("create", Label("machine", "create"), func() {
},
},
}, nil).MaxTimes(2)
mockLinodeClient.EXPECT().
ListInstanceConfigs(ctx, 123, gomock.Any()).
After(getAddrs).
Return([]linodego.InstanceConfig{{
Devices: &linodego.InstanceConfigDeviceMap{
SDA: &linodego.InstanceConfigDevice{DiskID: 100},
},
}}, nil)

mScope := scope.MachineScope{
Client: k8sClient,
Expand Down Expand Up @@ -716,6 +692,7 @@ var _ = Describe("create", Label("machine", "create"), func() {
IPv4: &linodego.InstanceIPv4Response{
Private: []*linodego.InstanceIP{{Address: "192.168.0.2"}},
Public: []*linodego.InstanceIP{{Address: "172.0.0.2"}},
VPC: []*linodego.VPCIP{{Address: ptr.To("10.0.0.2")}},
},
IPv6: &linodego.InstanceIPv6Response{
SLAAC: &linodego.InstanceIP{
Expand All @@ -731,32 +708,21 @@ var _ = Describe("create", Label("machine", "create"), func() {
}).
After(getAddrs).
Return(nil, nil).MaxTimes(2)
getAddrs = mockLinodeClient.EXPECT().
mockLinodeClient.EXPECT().
GetInstanceIPAddresses(ctx, 123).
After(createNB).
Return(&linodego.InstanceIPAddressResponse{
IPv4: &linodego.InstanceIPv4Response{
Private: []*linodego.InstanceIP{{Address: "192.168.0.2"}},
Public: []*linodego.InstanceIP{{Address: "172.0.0.2"}},
VPC: []*linodego.VPCIP{{Address: ptr.To("10.0.0.2")}},
},
IPv6: &linodego.InstanceIPv6Response{
SLAAC: &linodego.InstanceIP{
Address: "fd00::",
},
},
}, nil).MaxTimes(2)
mockLinodeClient.EXPECT().
ListInstanceConfigs(ctx, 123, gomock.Any()).
After(getAddrs).
Return([]linodego.InstanceConfig{{
Devices: &linodego.InstanceConfigDeviceMap{
SDA: &linodego.InstanceConfigDevice{DiskID: 100},
},
Interfaces: []linodego.InstanceConfigInterface{{
VPCID: ptr.To(1),
IPv4: &linodego.VPCIPv4{VPC: "10.0.0.2"},
}},
}}, nil)

_, err = reconciler.reconcileCreate(ctx, logger, &mScope)
Expect(err).NotTo(HaveOccurred())
Expand Down Expand Up @@ -889,7 +855,7 @@ var _ = Describe("createDNS", Label("machine", "createDNS"), func() {
BootInstance(ctx, 123, 0).
After(createInst).
Return(nil)
getAddrs := mockLinodeClient.EXPECT().
mockLinodeClient.EXPECT().
GetInstanceIPAddresses(ctx, 123).
After(bootInst).
Return(&linodego.InstanceIPAddressResponse{
Expand All @@ -903,14 +869,6 @@ var _ = Describe("createDNS", Label("machine", "createDNS"), func() {
},
},
}, nil)
mockLinodeClient.EXPECT().
ListInstanceConfigs(ctx, 123, gomock.Any()).
After(getAddrs).
Return([]linodego.InstanceConfig{{
Devices: &linodego.InstanceConfigDeviceMap{
SDA: &linodego.InstanceConfigDevice{DiskID: 100},
},
}}, nil)

mScope := scope.MachineScope{
Client: k8sClient,
Expand Down Expand Up @@ -1975,3 +1933,168 @@ var _ = Describe("machine in VPC", Label("machine", "VPC"), Ordered, func() {
}}))
})
})

var _ = Describe("machine in vlan", Label("machine", "vlan"), Ordered, func() {
var machine clusterv1.Machine
var secret corev1.Secret

var mockCtrl *gomock.Controller
var testLogs *bytes.Buffer
var logger logr.Logger

var reconciler *LinodeMachineReconciler
var linodeMachine infrav1alpha2.LinodeMachine

cluster := clusterv1.Cluster{
ObjectMeta: metav1.ObjectMeta{
Name: "mock",
Namespace: defaultNamespace,
},
}

linodeCluster := infrav1alpha2.LinodeCluster{
Spec: infrav1alpha2.LinodeClusterSpec{
Region: "us-ord",
Network: infrav1alpha2.NetworkSpec{
UseVlan: true,
},
},
}

recorder := record.NewFakeRecorder(10)

BeforeEach(func(ctx SpecContext) {
secret = corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: "bootstrap-secret",
Namespace: defaultNamespace,
},
Data: map[string][]byte{
"value": []byte("userdata"),
},
}
Expect(k8sClient.Create(ctx, &secret)).To(Succeed())

machine = clusterv1.Machine{
ObjectMeta: metav1.ObjectMeta{
Namespace: defaultNamespace,
Labels: make(map[string]string),
},
Spec: clusterv1.MachineSpec{
Bootstrap: clusterv1.Bootstrap{
DataSecretName: ptr.To("bootstrap-secret"),
},
},
}

linodeMachine = infrav1alpha2.LinodeMachine{
ObjectMeta: metav1.ObjectMeta{
Name: "mock",
Namespace: defaultNamespace,
UID: "12345",
},
Spec: infrav1alpha2.LinodeMachineSpec{
Type: "g6-nanode-1",
Image: rutil.DefaultMachineControllerLinodeImage,
DiskEncryption: string(linodego.InstanceDiskEncryptionEnabled),
},
}

mockCtrl = gomock.NewController(GinkgoT())
testLogs = &bytes.Buffer{}
logger = zap.New(
zap.WriteTo(GinkgoWriter),
zap.WriteTo(testLogs),
zap.UseDevMode(true),
)
reconciler = &LinodeMachineReconciler{
Recorder: recorder,
}
})

AfterEach(func(ctx SpecContext) {
Expect(k8sClient.Delete(ctx, &secret)).To(Succeed())

mockCtrl.Finish()
for len(recorder.Events) > 0 {
<-recorder.Events
}
})

It("creates an instance with vlan", func(ctx SpecContext) {
mockLinodeClient := mock.NewMockLinodeClient(mockCtrl)
getRegion := mockLinodeClient.EXPECT().
GetRegion(ctx, gomock.Any()).
Return(&linodego.Region{Capabilities: []string{linodego.CapabilityMetadata, linodego.CapabilityDiskEncryption}}, nil)
getImage := mockLinodeClient.EXPECT().
GetImage(ctx, gomock.Any()).
After(getRegion).
Return(&linodego.Image{Capabilities: []string{"cloud-init"}}, nil)
createInst := mockLinodeClient.EXPECT().
CreateInstance(ctx, gomock.Any()).
After(getImage).
Return(&linodego.Instance{
ID: 123,
IPv4: []*net.IP{ptr.To(net.IPv4(192, 168, 0, 2))},
IPv6: "fd00::",
Status: linodego.InstanceOffline,
}, nil)
mockLinodeClient.EXPECT().
OnAfterResponse(gomock.Any()).
Return()
bootInst := mockLinodeClient.EXPECT().
BootInstance(ctx, 123, 0).
After(createInst).
Return(nil)
getAddrs := mockLinodeClient.EXPECT().
GetInstanceIPAddresses(ctx, 123).
After(bootInst).
Return(&linodego.InstanceIPAddressResponse{
IPv4: &linodego.InstanceIPv4Response{
Private: []*linodego.InstanceIP{{Address: "192.168.0.2"}},
Public: []*linodego.InstanceIP{{Address: "172.0.0.2"}},
VPC: []*linodego.VPCIP{},
},
IPv6: &linodego.InstanceIPv6Response{
SLAAC: &linodego.InstanceIP{
Address: "fd00::",
},
},
}, nil)
mockLinodeClient.EXPECT().
ListInstanceConfigs(ctx, 123, gomock.Any()).
After(getAddrs).
Return([]linodego.InstanceConfig{{
Interfaces: []linodego.InstanceConfigInterface{
{
Purpose: linodego.InterfacePurposeVLAN,
IPAMAddress: "10.0.0.2/11",
},
},
}}, nil)

mScope := scope.MachineScope{
Client: k8sClient,
LinodeClient: mockLinodeClient,
Cluster: &cluster,
Machine: &machine,
LinodeCluster: &linodeCluster,
LinodeMachine: &linodeMachine,
}

patchHelper, err := patch.NewHelper(mScope.LinodeMachine, k8sClient)
Expect(err).NotTo(HaveOccurred())
mScope.PatchHelper = patchHelper

_, err = reconciler.reconcileCreate(ctx, logger, &mScope)
Expect(err).NotTo(HaveOccurred())
_, err = reconciler.reconcileCreate(ctx, logger, &mScope)
Expect(err).NotTo(HaveOccurred())

Expect(rutil.ConditionTrue(&linodeMachine, ConditionPreflightMetadataSupportConfigured)).To(BeTrue())
Expect(rutil.ConditionTrue(&linodeMachine, ConditionPreflightCreated)).To(BeTrue())
Expect(rutil.ConditionTrue(&linodeMachine, ConditionPreflightConfigured)).To(BeTrue())
Expect(rutil.ConditionTrue(&linodeMachine, ConditionPreflightBootTriggered)).To(BeTrue())
Expect(rutil.ConditionTrue(&linodeMachine, ConditionPreflightReady)).To(BeTrue())
})
})

0 comments on commit 6696734

Please sign in to comment.