Skip to content

Commit f1b64ce

Browse files
rahulaitrahulait
authored
[improvement] : allow passing vpcids and subnetids to ccm (#423)
* allow passing vpcids and subnetids to ccm * fix linting failures * add unittests * handle default value for subnet-names, add docs * resolve comments * update docs to avoid showing slice in example
1 parent 65152d7 commit f1b64ce

File tree

17 files changed

+479
-79
lines changed

17 files changed

+479
-79
lines changed

‎cloud/linode/client/client.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ type Client interface {
3434

3535
UpdateInstanceConfigInterface(context.Context, int, int, int, linodego.InstanceConfigInterfaceUpdateOptions) (*linodego.InstanceConfigInterface, error)
3636

37+
GetVPC(context.Context, int) (*linodego.VPC, error)
38+
GetVPCSubnet(context.Context, int, int) (*linodego.VPCSubnet, error)
3739
ListVPCs(context.Context, *linodego.ListOptions) ([]linodego.VPC, error)
3840
ListVPCIPAddresses(context.Context, int, *linodego.ListOptions) ([]linodego.VPCIP, error)
3941
ListVPCSubnets(context.Context, int, *linodego.ListOptions) ([]linodego.VPCSubnet, error)

‎cloud/linode/client/client_with_metrics.go‎

Lines changed: 26 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎cloud/linode/client/mocks/mock_client.go‎

Lines changed: 30 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎cloud/linode/cloud.go‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@ import (
66
"io"
77
"net"
88
"os"
9+
"regexp"
910
"strconv"
1011
"time"
11-
"regexp"
1212

1313
"github.com/spf13/pflag"
1414
"golang.org/x/exp/slices"
@@ -39,8 +39,10 @@ var Options struct {
3939
LinodeGoDebug bool
4040
EnableRouteController bool
4141
EnableTokenHealthChecker bool
42-
VPCNames string
43-
SubnetNames string
42+
VPCNames []string
43+
VPCIDs []int
44+
SubnetNames []string
45+
SubnetIDs []int
4446
LoadBalancerType string
4547
BGPNodeSelector string
4648
IpHolderSuffix string
@@ -139,10 +141,9 @@ func newCloud() (cloudprovider.Interface, error) {
139141
healthChecker = newHealthChecker(linodeClient, tokenHealthCheckPeriod, Options.GlobalStopChannel)
140142
}
141143

142-
// SubnetNames can't be used without VPCNames also being set
143-
if Options.SubnetNames != "" && Options.VPCNames == "" {
144-
klog.Warningf("failed to set flag subnet-names: vpc-names must be set to a non-empty value")
145-
Options.SubnetNames = ""
144+
err = validateAndSetVPCSubnetFlags(linodeClient)
145+
if err != nil {
146+
return nil, fmt.Errorf("failed to validate VPC and subnet flags: %w", err)
146147
}
147148

148149
if Options.NodeBalancerBackendIPv4SubnetID != 0 && Options.NodeBalancerBackendIPv4SubnetName != "" {

‎cloud/linode/cloud_test.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,14 @@ func TestNewCloudRouteControllerDisabled(t *testing.T) {
2323
Options.NodeBalancerPrefix = "ccm"
2424

2525
t.Run("should not fail if vpc is empty and routecontroller is disabled", func(t *testing.T) {
26-
Options.VPCNames = ""
26+
Options.VPCNames = []string{}
2727
Options.EnableRouteController = false
2828
_, err := newCloud()
2929
assert.NoError(t, err)
3030
})
3131

3232
t.Run("fail if vpcname is empty and routecontroller is enabled", func(t *testing.T) {
33-
Options.VPCNames = ""
33+
Options.VPCNames = []string{}
3434
Options.EnableRouteController = true
3535
_, err := newCloud()
3636
assert.Error(t, err)
@@ -61,11 +61,11 @@ func TestNewCloud(t *testing.T) {
6161
})
6262

6363
t.Run("should fail if both nodeBalancerBackendIPv4SubnetID and nodeBalancerBackendIPv4SubnetName are set", func(t *testing.T) {
64-
Options.VPCNames = "tt"
64+
Options.VPCNames = []string{"tt"}
6565
Options.NodeBalancerBackendIPv4SubnetID = 12345
6666
Options.NodeBalancerBackendIPv4SubnetName = "test-subnet"
6767
defer func() {
68-
Options.VPCNames = ""
68+
Options.VPCNames = []string{}
6969
Options.NodeBalancerBackendIPv4SubnetID = 0
7070
Options.NodeBalancerBackendIPv4SubnetName = ""
7171
}()
@@ -77,14 +77,14 @@ func TestNewCloud(t *testing.T) {
7777
rtEnabled := Options.EnableRouteController
7878
Options.EnableRouteController = false
7979
Options.LoadBalancerType = "test"
80-
Options.VPCNames = "vpc-test1,vpc-test2"
80+
Options.VPCNames = []string{"vpc-test1", "vpc-test2"}
8181
Options.NodeBalancerBackendIPv4SubnetName = "t1"
8282
vpcIDs = map[string]int{"vpc-test1": 1, "vpc-test2": 2, "vpc-test3": 3}
8383
subnetIDs = map[string]int{"t1": 1, "t2": 2, "t3": 3}
8484
defer func() {
8585
Options.LoadBalancerType = ""
8686
Options.EnableRouteController = rtEnabled
87-
Options.VPCNames = ""
87+
Options.VPCNames = []string{}
8888
Options.NodeBalancerBackendIPv4SubnetID = 0
8989
Options.NodeBalancerBackendIPv4SubnetName = ""
9090
vpcIDs = map[string]int{}

‎cloud/linode/instances.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,8 @@ func (nc *nodeCache) refreshInstances(ctx context.Context, client client.Client)
8080

8181
// If running within VPC, find instances and store their ips
8282
vpcNodes := map[int][]string{}
83-
vpcNames := strings.Split(Options.VPCNames, ",")
84-
for _, v := range vpcNames {
85-
vpcName := strings.TrimSpace(v)
83+
for _, name := range Options.VPCNames {
84+
vpcName := strings.TrimSpace(name)
8685
if vpcName == "" {
8786
continue
8887
}
@@ -102,7 +101,7 @@ func (nc *nodeCache) refreshInstances(ctx context.Context, client client.Client)
102101
newNodes := make(map[int]linodeInstance, len(instances))
103102
for index, instance := range instances {
104103
// if running within VPC, only store instances in cache which are part of VPC
105-
if Options.VPCNames != "" && len(vpcNodes[instance.ID]) == 0 {
104+
if len(Options.VPCNames) > 0 && len(vpcNodes[instance.ID]) == 0 {
106105
continue
107106
}
108107
node := linodeInstance{

‎cloud/linode/instances_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ func TestMetadataRetrieval(t *testing.T) {
159159
ipv6Addr := "2001::8a2e:370:7348"
160160
linodeType := typeG6
161161

162-
Options.VPCNames = "test"
162+
Options.VPCNames = []string{"test"}
163163
vpcIDs["test"] = 1
164164
Options.EnableRouteController = true
165165

@@ -229,7 +229,7 @@ func TestMetadataRetrieval(t *testing.T) {
229229
},
230230
}, meta.NodeAddresses)
231231

232-
Options.VPCNames = ""
232+
Options.VPCNames = []string{}
233233
})
234234

235235
ipTests := []struct {

‎cloud/linode/loadbalancers.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -460,7 +460,7 @@ func (l *loadbalancers) updateNodeBalancer(
460460
return err
461461
}
462462
}
463-
if Options.VPCNames != "" && !Options.DisableNodeBalancerVPCBackends {
463+
if len(Options.VPCNames) > 0 && !Options.DisableNodeBalancerVPCBackends {
464464
var id int
465465
id, err = l.getSubnetIDForSVC(ctx, service)
466466
if err != nil {
@@ -831,7 +831,7 @@ func (l *loadbalancers) createNodeBalancer(ctx context.Context, clusterName stri
831831
Type: nbType,
832832
}
833833

834-
if Options.VPCNames != "" && !Options.DisableNodeBalancerVPCBackends {
834+
if len(Options.VPCNames) > 0 && !Options.DisableNodeBalancerVPCBackends {
835835
createOpts.VPCs, err = l.getVPCCreateOptions(ctx, service)
836836
if err != nil {
837837
return nil, err
@@ -971,7 +971,7 @@ func (l *loadbalancers) addTLSCert(ctx context.Context, service *v1.Service, nbC
971971
// 3. If CCM is configured with --nodebalancer-backend-ipv4-subnet-id, it will be used as the subnet ID.
972972
// 4. Else, use first VPCName and SubnetName to calculate subnet id for the service.
973973
func (l *loadbalancers) getSubnetIDForSVC(ctx context.Context, service *v1.Service) (int, error) {
974-
if Options.VPCNames == "" {
974+
if len(Options.VPCNames) == 0 {
975975
return 0, fmt.Errorf("CCM not configured with VPC, cannot create NodeBalancer with specified annotation")
976976
}
977977
// Check if the service has an annotation for NodeBalancerBackendSubnetID
@@ -992,7 +992,7 @@ func (l *loadbalancers) getSubnetIDForSVC(ctx context.Context, service *v1.Servi
992992
return Options.NodeBalancerBackendIPv4SubnetID, nil
993993
}
994994

995-
vpcName := strings.Split(Options.VPCNames, ",")[0]
995+
vpcName := Options.VPCNames[0]
996996
if vpcOk {
997997
vpcName = specifiedVPCName
998998
}
@@ -1001,7 +1001,7 @@ func (l *loadbalancers) getSubnetIDForSVC(ctx context.Context, service *v1.Servi
10011001
return 0, err
10021002
}
10031003

1004-
subnetName := strings.Split(Options.SubnetNames, ",")[0]
1004+
subnetName := Options.SubnetNames[0]
10051005
if subnetOk {
10061006
subnetName = specifiedSubnetName
10071007
}
@@ -1030,7 +1030,7 @@ func (l *loadbalancers) buildLoadBalancerRequest(ctx context.Context, clusterNam
10301030
return nil, err
10311031
}
10321032
}
1033-
if Options.VPCNames != "" && !Options.DisableNodeBalancerVPCBackends {
1033+
if len(Options.VPCNames) > 0 && !Options.DisableNodeBalancerVPCBackends {
10341034
id, err := l.getSubnetIDForSVC(ctx, service)
10351035
if err != nil {
10361036
return nil, err

‎cloud/linode/loadbalancers_test.go‎

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -627,8 +627,8 @@ func testCreateNodeBalancerWithVPCBackend(t *testing.T, client *linodego.Client,
627627
Options.VPCNames = vpcNames
628628
Options.SubnetNames = subnetNames
629629
}()
630-
Options.VPCNames = "test1"
631-
Options.SubnetNames = defaultSubnet
630+
Options.VPCNames = []string{"test1"}
631+
Options.SubnetNames = []string{defaultSubnet}
632632
_, _ = client.CreateVPC(t.Context(), linodego.VPCCreateOptions{
633633
Label: "test1",
634634
Description: "",
@@ -669,8 +669,8 @@ func testUpdateNodeBalancerWithVPCBackend(t *testing.T, client *linodego.Client,
669669
Options.VPCNames = vpcNames
670670
Options.SubnetNames = subnetNames
671671
}()
672-
Options.VPCNames = "test1"
673-
Options.SubnetNames = defaultSubnet
672+
Options.VPCNames = []string{"test1"}
673+
Options.SubnetNames = []string{defaultSubnet}
674674
_, _ = client.CreateVPC(t.Context(), linodego.VPCCreateOptions{
675675
Label: "test1",
676676
Description: "",
@@ -757,8 +757,8 @@ func testCreateNodeBalancerWithVPCOnlySubnetFlag(t *testing.T, client *linodego.
757757
Options.SubnetNames = subnetNames
758758
Options.NodeBalancerBackendIPv4Subnet = nbBackendSubnet
759759
}()
760-
Options.VPCNames = "test-subflag"
761-
Options.SubnetNames = defaultSubnet
760+
Options.VPCNames = []string{"test-subflag"}
761+
Options.SubnetNames = []string{defaultSubnet}
762762
Options.NodeBalancerBackendIPv4Subnet = "10.254.0.0/24"
763763
_, _ = client.CreateVPC(t.Context(), linodego.VPCCreateOptions{
764764
Label: "test-subflag",
@@ -851,8 +851,8 @@ func testCreateNodeBalancerWithVPCNoFlagOrAnnotation(t *testing.T, client *linod
851851
Options.VPCNames = vpcNames
852852
Options.SubnetNames = subnetNames
853853
}()
854-
Options.VPCNames = "test-noflags"
855-
Options.SubnetNames = defaultSubnet
854+
Options.VPCNames = []string{"test-noflags"}
855+
Options.SubnetNames = []string{defaultSubnet}
856856
_, _ = client.CreateVPC(t.Context(), linodego.VPCCreateOptions{
857857
Label: "test-noflags",
858858
Description: "",
@@ -934,8 +934,8 @@ func testCreateNodeBalancerWithVPCAnnotationOnly(t *testing.T, client *linodego.
934934
Options.VPCNames = vpcNames
935935
Options.SubnetNames = subnetNames
936936
}()
937-
Options.VPCNames = "test-onlyannotation"
938-
Options.SubnetNames = defaultSubnet
937+
Options.VPCNames = []string{"test-onlyannotation"}
938+
Options.SubnetNames = []string{defaultSubnet}
939939
_, _ = client.CreateVPC(t.Context(), linodego.VPCCreateOptions{
940940
Label: "test-onlyannotation",
941941
Description: "",
@@ -1024,8 +1024,8 @@ func testCreateNodeBalancerWithVPCOnlySubnetIDFlag(t *testing.T, client *linodeg
10241024
Options.SubnetNames = subnetNames
10251025
Options.NodeBalancerBackendIPv4SubnetID = nbBackendSubnetID
10261026
}()
1027-
Options.VPCNames = "test1"
1028-
Options.SubnetNames = defaultSubnet
1027+
Options.VPCNames = []string{"test1"}
1028+
Options.SubnetNames = []string{defaultSubnet}
10291029
Options.NodeBalancerBackendIPv4SubnetID = 1111
10301030
_, _ = client.CreateVPC(t.Context(), linodego.VPCCreateOptions{
10311031
Label: "test-subid-flag",
@@ -1105,12 +1105,15 @@ func testCreateNodeBalancerWithVPCAnnotationOverwrite(t *testing.T, client *lino
11051105

11061106
// provision multiple vpcs
11071107
vpcNames := Options.VPCNames
1108+
subnetNames := Options.SubnetNames
11081109
nodebalancerBackendIPv4Subnet := Options.NodeBalancerBackendIPv4Subnet
11091110
defer func() {
11101111
Options.VPCNames = vpcNames
1112+
Options.SubnetNames = subnetNames
11111113
Options.NodeBalancerBackendIPv4Subnet = nodebalancerBackendIPv4Subnet
11121114
}()
1113-
Options.VPCNames = "test1"
1115+
Options.VPCNames = []string{"test1"}
1116+
Options.SubnetNames = []string{defaultSubnet}
11141117
Options.NodeBalancerBackendIPv4Subnet = "10.100.0.0/24"
11151118

11161119
_, _ = client.CreateVPC(t.Context(), linodego.VPCCreateOptions{

‎cloud/linode/route_controller.go‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,7 @@ func (rc *routeCache) refreshRoutes(ctx context.Context, client client.Client) {
3939
}
4040

4141
vpcNodes := map[int][]linodego.VPCIP{}
42-
vpcNames := strings.Split(Options.VPCNames, ",")
43-
for _, v := range vpcNames {
42+
for _, v := range Options.VPCNames {
4443
vpcName := strings.TrimSpace(v)
4544
if vpcName == "" {
4645
continue
@@ -74,7 +73,7 @@ func newRoutes(client client.Client, instanceCache *instances) (cloudprovider.Ro
7473
}
7574
klog.V(3).Infof("TTL for routeCache set to %d seconds", timeout)
7675

77-
if Options.EnableRouteController && Options.VPCNames == "" {
76+
if Options.EnableRouteController && len(Options.VPCNames) == 0 {
7877
return nil, fmt.Errorf("cannot enable route controller as vpc-names is empty")
7978
}
8079

0 commit comments

Comments
 (0)