From e0eaa4105338e1ad1ce5c050cff75fefcf2acf81 Mon Sep 17 00:00:00 2001 From: Rahul Sharma Date: Tue, 22 Jul 2025 14:30:53 +0000 Subject: [PATCH 1/6] allow passing vpcids and subnetids to ccm --- cloud/linode/client/client.go | 2 + cloud/linode/client/client_with_metrics.go | 26 ++++++++ cloud/linode/client/mocks/mock_client.go | 30 +++++++++ cloud/linode/cloud.go | 15 +++-- cloud/linode/cloud_test.go | 12 ++-- cloud/linode/instances.go | 7 +- cloud/linode/instances_test.go | 4 +- cloud/linode/loadbalancers.go | 12 ++-- cloud/linode/loadbalancers_test.go | 29 +++++---- cloud/linode/route_controller.go | 5 +- cloud/linode/route_controller_test.go | 8 ++- cloud/linode/vpc.go | 75 ++++++++++++++++++++-- cloud/linode/vpc_test.go | 8 +-- main.go | 6 +- 14 files changed, 182 insertions(+), 57 deletions(-) diff --git a/cloud/linode/client/client.go b/cloud/linode/client/client.go index 609f88e1..4938a8fb 100644 --- a/cloud/linode/client/client.go +++ b/cloud/linode/client/client.go @@ -34,6 +34,8 @@ type Client interface { UpdateInstanceConfigInterface(context.Context, int, int, int, linodego.InstanceConfigInterfaceUpdateOptions) (*linodego.InstanceConfigInterface, error) + GetVPC(context.Context, int) (*linodego.VPC, error) + GetVPCSubnet(context.Context, int, int) (*linodego.VPCSubnet, error) ListVPCs(context.Context, *linodego.ListOptions) ([]linodego.VPC, error) ListVPCIPAddresses(context.Context, int, *linodego.ListOptions) ([]linodego.VPCIP, error) ListVPCSubnets(context.Context, int, *linodego.ListOptions) ([]linodego.VPCSubnet, error) diff --git a/cloud/linode/client/client_with_metrics.go b/cloud/linode/client/client_with_metrics.go index 4812e296..1034fbd7 100644 --- a/cloud/linode/client/client_with_metrics.go +++ b/cloud/linode/client/client_with_metrics.go @@ -241,6 +241,32 @@ func (_d ClientWithPrometheus) GetProfile(ctx context.Context) (pp1 *linodego.Pr return _d.base.GetProfile(ctx) } +// GetVPC implements Client +func (_d ClientWithPrometheus) GetVPC(ctx context.Context, i1 int) (vp1 *linodego.VPC, err error) { + defer func() { + result := "ok" + if err != nil { + result = "error" + } + + ClientMethodCounterVec.WithLabelValues("GetVPC", result).Inc() + }() + return _d.base.GetVPC(ctx, i1) +} + +// GetVPCSubnet implements Client +func (_d ClientWithPrometheus) GetVPCSubnet(ctx context.Context, i1 int, i2 int) (vp1 *linodego.VPCSubnet, err error) { + defer func() { + result := "ok" + if err != nil { + result = "error" + } + + ClientMethodCounterVec.WithLabelValues("GetVPCSubnet", result).Inc() + }() + return _d.base.GetVPCSubnet(ctx, i1, i2) +} + // ListFirewallDevices implements Client func (_d ClientWithPrometheus) ListFirewallDevices(ctx context.Context, firewallID int, opts *linodego.ListOptions) (fa1 []linodego.FirewallDevice, err error) { defer func() { diff --git a/cloud/linode/client/mocks/mock_client.go b/cloud/linode/client/mocks/mock_client.go index bea9ea02..038379d5 100644 --- a/cloud/linode/client/mocks/mock_client.go +++ b/cloud/linode/client/mocks/mock_client.go @@ -270,6 +270,36 @@ func (mr *MockClientMockRecorder) GetProfile(arg0 interface{}) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetProfile", reflect.TypeOf((*MockClient)(nil).GetProfile), arg0) } +// GetVPC mocks base method. +func (m *MockClient) GetVPC(arg0 context.Context, arg1 int) (*linodego.VPC, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetVPC", arg0, arg1) + ret0, _ := ret[0].(*linodego.VPC) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetVPC indicates an expected call of GetVPC. +func (mr *MockClientMockRecorder) GetVPC(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetVPC", reflect.TypeOf((*MockClient)(nil).GetVPC), arg0, arg1) +} + +// GetVPCSubnet mocks base method. +func (m *MockClient) GetVPCSubnet(arg0 context.Context, arg1, arg2 int) (*linodego.VPCSubnet, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetVPCSubnet", arg0, arg1, arg2) + ret0, _ := ret[0].(*linodego.VPCSubnet) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetVPCSubnet indicates an expected call of GetVPCSubnet. +func (mr *MockClientMockRecorder) GetVPCSubnet(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetVPCSubnet", reflect.TypeOf((*MockClient)(nil).GetVPCSubnet), arg0, arg1, arg2) +} + // ListFirewallDevices mocks base method. func (m *MockClient) ListFirewallDevices(arg0 context.Context, arg1 int, arg2 *linodego.ListOptions) ([]linodego.FirewallDevice, error) { m.ctrl.T.Helper() diff --git a/cloud/linode/cloud.go b/cloud/linode/cloud.go index c786971c..04bd6958 100644 --- a/cloud/linode/cloud.go +++ b/cloud/linode/cloud.go @@ -6,9 +6,9 @@ import ( "io" "net" "os" + "regexp" "strconv" "time" - "regexp" "github.com/spf13/pflag" "golang.org/x/exp/slices" @@ -39,8 +39,10 @@ var Options struct { LinodeGoDebug bool EnableRouteController bool EnableTokenHealthChecker bool - VPCNames string - SubnetNames string + VPCNames []string + VPCIDs []int + SubnetNames []string + SubnetIDs []int LoadBalancerType string BGPNodeSelector string IpHolderSuffix string @@ -139,10 +141,9 @@ func newCloud() (cloudprovider.Interface, error) { healthChecker = newHealthChecker(linodeClient, tokenHealthCheckPeriod, Options.GlobalStopChannel) } - // SubnetNames can't be used without VPCNames also being set - if Options.SubnetNames != "" && Options.VPCNames == "" { - klog.Warningf("failed to set flag subnet-names: vpc-names must be set to a non-empty value") - Options.SubnetNames = "" + err = validateAndSetVPCSubnetFlags(linodeClient) + if err != nil { + return nil, fmt.Errorf("failed to validate VPC and subnet flags: %w", err) } if Options.NodeBalancerBackendIPv4SubnetID != 0 && Options.NodeBalancerBackendIPv4SubnetName != "" { diff --git a/cloud/linode/cloud_test.go b/cloud/linode/cloud_test.go index b47c7fe0..a46aa218 100644 --- a/cloud/linode/cloud_test.go +++ b/cloud/linode/cloud_test.go @@ -23,14 +23,14 @@ func TestNewCloudRouteControllerDisabled(t *testing.T) { Options.NodeBalancerPrefix = "ccm" t.Run("should not fail if vpc is empty and routecontroller is disabled", func(t *testing.T) { - Options.VPCNames = "" + Options.VPCNames = []string{} Options.EnableRouteController = false _, err := newCloud() assert.NoError(t, err) }) t.Run("fail if vpcname is empty and routecontroller is enabled", func(t *testing.T) { - Options.VPCNames = "" + Options.VPCNames = []string{} Options.EnableRouteController = true _, err := newCloud() assert.Error(t, err) @@ -61,11 +61,11 @@ func TestNewCloud(t *testing.T) { }) t.Run("should fail if both nodeBalancerBackendIPv4SubnetID and nodeBalancerBackendIPv4SubnetName are set", func(t *testing.T) { - Options.VPCNames = "tt" + Options.VPCNames = []string{"tt"} Options.NodeBalancerBackendIPv4SubnetID = 12345 Options.NodeBalancerBackendIPv4SubnetName = "test-subnet" defer func() { - Options.VPCNames = "" + Options.VPCNames = []string{} Options.NodeBalancerBackendIPv4SubnetID = 0 Options.NodeBalancerBackendIPv4SubnetName = "" }() @@ -77,14 +77,14 @@ func TestNewCloud(t *testing.T) { rtEnabled := Options.EnableRouteController Options.EnableRouteController = false Options.LoadBalancerType = "test" - Options.VPCNames = "vpc-test1,vpc-test2" + Options.VPCNames = []string{"vpc-test1", "vpc-test2"} Options.NodeBalancerBackendIPv4SubnetName = "t1" vpcIDs = map[string]int{"vpc-test1": 1, "vpc-test2": 2, "vpc-test3": 3} subnetIDs = map[string]int{"t1": 1, "t2": 2, "t3": 3} defer func() { Options.LoadBalancerType = "" Options.EnableRouteController = rtEnabled - Options.VPCNames = "" + Options.VPCNames = []string{} Options.NodeBalancerBackendIPv4SubnetID = 0 Options.NodeBalancerBackendIPv4SubnetName = "" vpcIDs = map[string]int{} diff --git a/cloud/linode/instances.go b/cloud/linode/instances.go index 0cc5940b..926155b0 100644 --- a/cloud/linode/instances.go +++ b/cloud/linode/instances.go @@ -80,9 +80,8 @@ func (nc *nodeCache) refreshInstances(ctx context.Context, client client.Client) // If running within VPC, find instances and store their ips vpcNodes := map[int][]string{} - vpcNames := strings.Split(Options.VPCNames, ",") - for _, v := range vpcNames { - vpcName := strings.TrimSpace(v) + for _, vpcName := range Options.VPCNames { + vpcName := strings.TrimSpace(vpcName) if vpcName == "" { continue } @@ -102,7 +101,7 @@ func (nc *nodeCache) refreshInstances(ctx context.Context, client client.Client) newNodes := make(map[int]linodeInstance, len(instances)) for index, instance := range instances { // if running within VPC, only store instances in cache which are part of VPC - if Options.VPCNames != "" && len(vpcNodes[instance.ID]) == 0 { + if len(Options.VPCNames) > 0 && len(vpcNodes[instance.ID]) == 0 { continue } node := linodeInstance{ diff --git a/cloud/linode/instances_test.go b/cloud/linode/instances_test.go index e1574a7e..4cedfff5 100644 --- a/cloud/linode/instances_test.go +++ b/cloud/linode/instances_test.go @@ -159,7 +159,7 @@ func TestMetadataRetrieval(t *testing.T) { ipv6Addr := "2001::8a2e:370:7348" linodeType := typeG6 - Options.VPCNames = "test" + Options.VPCNames = []string{"test"} vpcIDs["test"] = 1 Options.EnableRouteController = true @@ -229,7 +229,7 @@ func TestMetadataRetrieval(t *testing.T) { }, }, meta.NodeAddresses) - Options.VPCNames = "" + Options.VPCNames = []string{} }) ipTests := []struct { diff --git a/cloud/linode/loadbalancers.go b/cloud/linode/loadbalancers.go index c37e0edb..1b8e9276 100644 --- a/cloud/linode/loadbalancers.go +++ b/cloud/linode/loadbalancers.go @@ -460,7 +460,7 @@ func (l *loadbalancers) updateNodeBalancer( return err } } - if Options.VPCNames != "" && !Options.DisableNodeBalancerVPCBackends { + if len(Options.VPCNames) > 0 && !Options.DisableNodeBalancerVPCBackends { var id int id, err = l.getSubnetIDForSVC(ctx, service) if err != nil { @@ -831,7 +831,7 @@ func (l *loadbalancers) createNodeBalancer(ctx context.Context, clusterName stri Type: nbType, } - if Options.VPCNames != "" && !Options.DisableNodeBalancerVPCBackends { + if len(Options.VPCNames) > 0 && !Options.DisableNodeBalancerVPCBackends { createOpts.VPCs, err = l.getVPCCreateOptions(ctx, service) if err != nil { return nil, err @@ -971,7 +971,7 @@ func (l *loadbalancers) addTLSCert(ctx context.Context, service *v1.Service, nbC // 3. If CCM is configured with --nodebalancer-backend-ipv4-subnet-id, it will be used as the subnet ID. // 4. Else, use first VPCName and SubnetName to calculate subnet id for the service. func (l *loadbalancers) getSubnetIDForSVC(ctx context.Context, service *v1.Service) (int, error) { - if Options.VPCNames == "" { + if len(Options.VPCNames) == 0 { return 0, fmt.Errorf("CCM not configured with VPC, cannot create NodeBalancer with specified annotation") } // Check if the service has an annotation for NodeBalancerBackendSubnetID @@ -992,7 +992,7 @@ func (l *loadbalancers) getSubnetIDForSVC(ctx context.Context, service *v1.Servi return Options.NodeBalancerBackendIPv4SubnetID, nil } - vpcName := strings.Split(Options.VPCNames, ",")[0] + vpcName := Options.VPCNames[0] if vpcOk { vpcName = specifiedVPCName } @@ -1001,7 +1001,7 @@ func (l *loadbalancers) getSubnetIDForSVC(ctx context.Context, service *v1.Servi return 0, err } - subnetName := strings.Split(Options.SubnetNames, ",")[0] + subnetName := Options.SubnetNames[0] if subnetOk { subnetName = specifiedSubnetName } @@ -1030,7 +1030,7 @@ func (l *loadbalancers) buildLoadBalancerRequest(ctx context.Context, clusterNam return nil, err } } - if Options.VPCNames != "" && !Options.DisableNodeBalancerVPCBackends { + if len(Options.VPCNames) > 0 && !Options.DisableNodeBalancerVPCBackends { id, err := l.getSubnetIDForSVC(ctx, service) if err != nil { return nil, err diff --git a/cloud/linode/loadbalancers_test.go b/cloud/linode/loadbalancers_test.go index db417fe7..848c4d12 100644 --- a/cloud/linode/loadbalancers_test.go +++ b/cloud/linode/loadbalancers_test.go @@ -627,8 +627,8 @@ func testCreateNodeBalancerWithVPCBackend(t *testing.T, client *linodego.Client, Options.VPCNames = vpcNames Options.SubnetNames = subnetNames }() - Options.VPCNames = "test1" - Options.SubnetNames = defaultSubnet + Options.VPCNames = []string{"test1"} + Options.SubnetNames = []string{defaultSubnet} _, _ = client.CreateVPC(t.Context(), linodego.VPCCreateOptions{ Label: "test1", Description: "", @@ -669,8 +669,8 @@ func testUpdateNodeBalancerWithVPCBackend(t *testing.T, client *linodego.Client, Options.VPCNames = vpcNames Options.SubnetNames = subnetNames }() - Options.VPCNames = "test1" - Options.SubnetNames = defaultSubnet + Options.VPCNames = []string{"test1"} + Options.SubnetNames = []string{defaultSubnet} _, _ = client.CreateVPC(t.Context(), linodego.VPCCreateOptions{ Label: "test1", Description: "", @@ -757,8 +757,8 @@ func testCreateNodeBalancerWithVPCOnlySubnetFlag(t *testing.T, client *linodego. Options.SubnetNames = subnetNames Options.NodeBalancerBackendIPv4Subnet = nbBackendSubnet }() - Options.VPCNames = "test-subflag" - Options.SubnetNames = defaultSubnet + Options.VPCNames = []string{"test-subflag"} + Options.SubnetNames = []string{defaultSubnet} Options.NodeBalancerBackendIPv4Subnet = "10.254.0.0/24" _, _ = client.CreateVPC(t.Context(), linodego.VPCCreateOptions{ Label: "test-subflag", @@ -851,8 +851,8 @@ func testCreateNodeBalancerWithVPCNoFlagOrAnnotation(t *testing.T, client *linod Options.VPCNames = vpcNames Options.SubnetNames = subnetNames }() - Options.VPCNames = "test-noflags" - Options.SubnetNames = defaultSubnet + Options.VPCNames = []string{"test-noflags"} + Options.SubnetNames = []string{defaultSubnet} _, _ = client.CreateVPC(t.Context(), linodego.VPCCreateOptions{ Label: "test-noflags", Description: "", @@ -934,8 +934,8 @@ func testCreateNodeBalancerWithVPCAnnotationOnly(t *testing.T, client *linodego. Options.VPCNames = vpcNames Options.SubnetNames = subnetNames }() - Options.VPCNames = "test-onlyannotation" - Options.SubnetNames = defaultSubnet + Options.VPCNames = []string{"test-onlyannotation"} + Options.SubnetNames = []string{defaultSubnet} _, _ = client.CreateVPC(t.Context(), linodego.VPCCreateOptions{ Label: "test-onlyannotation", Description: "", @@ -1024,8 +1024,8 @@ func testCreateNodeBalancerWithVPCOnlySubnetIDFlag(t *testing.T, client *linodeg Options.SubnetNames = subnetNames Options.NodeBalancerBackendIPv4SubnetID = nbBackendSubnetID }() - Options.VPCNames = "test1" - Options.SubnetNames = defaultSubnet + Options.VPCNames = []string{"test1"} + Options.SubnetNames = []string{defaultSubnet} Options.NodeBalancerBackendIPv4SubnetID = 1111 _, _ = client.CreateVPC(t.Context(), linodego.VPCCreateOptions{ Label: "test-subid-flag", @@ -1105,12 +1105,15 @@ func testCreateNodeBalancerWithVPCAnnotationOverwrite(t *testing.T, client *lino // provision multiple vpcs vpcNames := Options.VPCNames + subnetNames := Options.SubnetNames nodebalancerBackendIPv4Subnet := Options.NodeBalancerBackendIPv4Subnet defer func() { Options.VPCNames = vpcNames + Options.SubnetNames = subnetNames Options.NodeBalancerBackendIPv4Subnet = nodebalancerBackendIPv4Subnet }() - Options.VPCNames = "test1" + Options.VPCNames = []string{"test1"} + Options.SubnetNames = []string{defaultSubnet} Options.NodeBalancerBackendIPv4Subnet = "10.100.0.0/24" _, _ = client.CreateVPC(t.Context(), linodego.VPCCreateOptions{ diff --git a/cloud/linode/route_controller.go b/cloud/linode/route_controller.go index 6486b744..b02ac55b 100644 --- a/cloud/linode/route_controller.go +++ b/cloud/linode/route_controller.go @@ -39,8 +39,7 @@ func (rc *routeCache) refreshRoutes(ctx context.Context, client client.Client) { } vpcNodes := map[int][]linodego.VPCIP{} - vpcNames := strings.Split(Options.VPCNames, ",") - for _, v := range vpcNames { + for _, v := range Options.VPCNames { vpcName := strings.TrimSpace(v) if vpcName == "" { continue @@ -74,7 +73,7 @@ func newRoutes(client client.Client, instanceCache *instances) (cloudprovider.Ro } klog.V(3).Infof("TTL for routeCache set to %d seconds", timeout) - if Options.EnableRouteController && Options.VPCNames == "" { + if Options.EnableRouteController && len(Options.VPCNames) == 0 { return nil, fmt.Errorf("cannot enable route controller as vpc-names is empty") } diff --git a/cloud/linode/route_controller_test.go b/cloud/linode/route_controller_test.go index 9d4a725a..ef727b25 100644 --- a/cloud/linode/route_controller_test.go +++ b/cloud/linode/route_controller_test.go @@ -19,9 +19,11 @@ import ( ) func TestListRoutes(t *testing.T) { - Options.VPCNames = "test,abc" + Options.VPCNames = []string{"test", "abc"} vpcIDs["test"] = 1 vpcIDs["abc"] = 2 + Options.SubnetNames = []string{"default"} + subnetIDs["default"] = 1 Options.EnableRouteController = true nodeID := 123 @@ -312,7 +314,7 @@ func TestListRoutes(t *testing.T) { func TestCreateRoute(t *testing.T) { ctx := t.Context() - Options.VPCNames = "dummy" + Options.VPCNames = []string{"dummy"} vpcIDs["dummy"] = 1 Options.EnableRouteController = true @@ -463,7 +465,7 @@ func TestCreateRoute(t *testing.T) { } func TestDeleteRoute(t *testing.T) { - Options.VPCNames = "dummy" + Options.VPCNames = []string{"dummy"} vpcIDs["dummy"] = 1 Options.EnableRouteController = true diff --git a/cloud/linode/vpc.go b/cloud/linode/vpc.go index 716846ea..d0d2d195 100644 --- a/cloud/linode/vpc.go +++ b/cloud/linode/vpc.go @@ -110,13 +110,11 @@ func GetVPCIPAddresses(ctx context.Context, client client.Client, vpcName string resultFilter := "" // Get subnet ID(s) from name(s) if subnet-names is specified - if Options.SubnetNames != "" { - // Get the IDs and store them + if len(Options.SubnetNames) > 0 { // subnetIDList is a slice of strings for ease of use with resultFilter - subnetNames := strings.Split(Options.SubnetNames, ",") subnetIDList := []string{} - for _, name := range subnetNames { + for _, name := range Options.SubnetNames { // For caching var subnetID int subnetID, err = GetSubnetID(ctx, client, vpcID, name) @@ -156,11 +154,10 @@ func GetVPCIPAddresses(ctx context.Context, client client.Client, vpcName string // It uses the first VPC name from Options.VPCNames to find the VPC ID and then retrieves the subnet ID // for the NodeBalancer backend IPv4 subnet name specified in Options.NodeBalancerBackendIPv4SubnetName. func getNodeBalancerBackendIPv4SubnetID(client client.Client) (int, error) { - vpcName := strings.Split(Options.VPCNames, ",")[0] // Get the VPC ID from the name - vpcID, err := GetVPCID(context.TODO(), client, vpcName) + vpcID, err := GetVPCID(context.TODO(), client, Options.VPCNames[0]) if err != nil { - return 0, fmt.Errorf("failed to get vpc id for %s: %w", vpcName, err) + return 0, fmt.Errorf("failed to get vpc id for %s: %w", Options.VPCNames[0], err) } // Get the subnet ID from the name subnetID, err := GetSubnetID(context.TODO(), client, vpcID, Options.NodeBalancerBackendIPv4SubnetName) @@ -170,3 +167,67 @@ func getNodeBalancerBackendIPv4SubnetID(client client.Client) (int, error) { return subnetID, nil } + +func validateVPCSubnetFlags() error { + switch { + case len(Options.SubnetIDs) > 0 && len(Options.VPCIDs) == 0: + return fmt.Errorf("subnet-ids cannot be set without vpc-ids") + case len(Options.SubnetNames) > 0 && len(Options.VPCNames) == 0: + return fmt.Errorf("subnet-names cannot be set without vpc-names") + case len(Options.VPCIDs) > 0 && len(Options.VPCNames) > 0: + return fmt.Errorf("cannot have both vpc-ids and vpc-names set") + case len(Options.SubnetIDs) > 0 && len(Options.SubnetNames) > 0: + return fmt.Errorf("cannot have both subnet-ids and subnet-names set") + case len(Options.VPCIDs) == 0: + return nil + } + return nil +} + +func resolveSubnetNames(client client.Client, vpcID int) ([]string, error) { + var subnetNames []string + + for _, subnetID := range Options.SubnetIDs { + subnet, err := client.GetVPCSubnet(context.TODO(), vpcID, subnetID) + if err != nil { + return nil, fmt.Errorf("failed to get subnet %d for VPC %d: %w", subnetID, vpcID, err) + } + + subnetIDs[subnet.Label] = subnet.ID + subnetNames = append(subnetNames, subnet.Label) + } + + return subnetNames, nil +} + +func validateAndSetVPCSubnetFlags(client client.Client) error { + if err := validateVPCSubnetFlags(); err != nil { + return err + } + + Mu.Lock() + defer Mu.Unlock() + + var vpcNames []string + + for idx, vpcID := range Options.VPCIDs { + vpc, err := client.GetVPC(context.TODO(), vpcID) + if err != nil { + return fmt.Errorf("failed to get VPC %d: %w", vpcID, err) + } + + vpcIDs[vpc.Label] = vpcID + vpcNames = append(vpcNames, vpc.Label) + + if idx == 0 && len(Options.SubnetIDs) > 0 { + subnetNames, err := resolveSubnetNames(client, vpcID) + if err != nil { + return err + } + Options.SubnetNames = append(Options.SubnetNames, subnetNames...) + } + } + + Options.VPCNames = append(Options.VPCNames, vpcNames...) + return nil +} diff --git a/cloud/linode/vpc_test.go b/cloud/linode/vpc_test.go index a802df56..f3b2c059 100644 --- a/cloud/linode/vpc_test.go +++ b/cloud/linode/vpc_test.go @@ -154,7 +154,7 @@ func TestGetVPCIPAddresses(t *testing.T) { client := mocks.NewMockClient(ctrl) sn := Options.SubnetNames defer func() { Options.SubnetNames = sn }() - Options.SubnetNames = "subnet4" + Options.SubnetNames = []string{"subnet4"} vpcIDs = map[string]int{"test1": 1} subnetIDs = map[string]int{"subnet1": 1} client.EXPECT().ListVPCs(gomock.Any(), gomock.Any()).Times(1).Return([]linodego.VPC{{ID: 10, Label: "test10"}}, nil) @@ -238,7 +238,7 @@ func TestGetNodeBalancerBackendIPv4SubnetID(t *testing.T) { vpcIDs = currVPCIDs subnetIDs = currSubnetIDs }() - Options.VPCNames = "vpc-test1,vpc-test2,vpc-test3" + Options.VPCNames = []string{"vpc-test1", "vpc-test2", "vpc-test3"} vpcIDs = map[string]int{"vpc-test2": 2, "vpc-test3": 3} subnetIDs = map[string]int{"test1": 1, "test2": 2, "test3": 3} client.EXPECT().ListVPCs(gomock.Any(), gomock.Any()).Times(1).Return([]linodego.VPC{}, errors.New("error")) @@ -255,7 +255,7 @@ func TestGetNodeBalancerBackendIPv4SubnetID(t *testing.T) { client := mocks.NewMockClient(ctrl) currVPCNames := Options.VPCNames defer func() { Options.VPCNames = currVPCNames }() - Options.VPCNames = "vpc-test1,vpc-test2,vpc-test3" + Options.VPCNames = []string{"vpc-test1", "vpc-test2", "vpc-test3"} vpcIDs = map[string]int{"vpc-test1": 1, "vpc-test2": 2, "vpc-test3": 3} subnetIDs = map[string]int{"test1": 1, "test2": 2, "test3": 3} client.EXPECT().ListVPCSubnets(gomock.Any(), gomock.Any(), gomock.Any()).Times(1).Return([]linodego.VPCSubnet{}, errors.New("error")) @@ -276,7 +276,7 @@ func TestGetNodeBalancerBackendIPv4SubnetID(t *testing.T) { Options.VPCNames = currVPCNames Options.NodeBalancerBackendIPv4SubnetName = currNodeBalancerBackendIPv4SubnetName }() - Options.VPCNames = "vpc-test1,vpc-test2,vpc-test3" + Options.VPCNames = []string{"vpc-test1", "vpc-test2", "vpc-test3"} Options.NodeBalancerBackendIPv4SubnetName = "test4" vpcIDs = map[string]int{"vpc-test1": 1, "vpc-test2": 2, "vpc-test3": 3} subnetIDs = map[string]int{"test1": 1, "test2": 2, "test3": 3} diff --git a/main.go b/main.go index 886ecf4b..4b7d888b 100644 --- a/main.go +++ b/main.go @@ -83,8 +83,10 @@ func main() { command.Flags().BoolVar(&linode.Options.LinodeGoDebug, "linodego-debug", false, "enables debug output for the LinodeAPI wrapper") command.Flags().BoolVar(&linode.Options.EnableRouteController, "enable-route-controller", false, "enables route_controller for ccm") command.Flags().BoolVar(&linode.Options.EnableTokenHealthChecker, "enable-token-health-checker", false, "enables Linode API token health checker") - command.Flags().StringVar(&linode.Options.VPCNames, "vpc-names", "", "comma separated vpc names whose routes will be managed by route-controller") - command.Flags().StringVar(&linode.Options.SubnetNames, "subnet-names", "default", "comma separated subnet names whose routes will be managed by route-controller (requires vpc-names flag to also be set)") + command.Flags().StringSliceVar(&linode.Options.VPCNames, "vpc-names", nil, "comma separated vpc names whose routes will be managed by route-controller") + command.Flags().StringSliceVar(&linode.Options.SubnetNames, "subnet-names", []string{"default"}, "comma separated subnet names whose routes will be managed by route-controller (requires vpc-names flag to also be set)") + command.Flags().IntSliceVar(&linode.Options.VPCIDs, "vpc-ids", nil, "comma separated vpc ids whose routes will be managed by route-controller") + command.Flags().IntSliceVar(&linode.Options.SubnetIDs, "subnet-ids", nil, "comma separated subnet ids whose routes will be managed by route-controller (requires vpc-ids flag to also be set)") command.Flags().StringVar(&linode.Options.LoadBalancerType, "load-balancer-type", "nodebalancer", "configures which type of load-balancing to use for LoadBalancer Services (options: nodebalancer, cilium-bgp)") command.Flags().StringVar(&linode.Options.BGPNodeSelector, "bgp-node-selector", "", "node selector to use to perform shared IP fail-over with BGP (e.g. cilium-bgp-peering=true") command.Flags().StringVar(&linode.Options.IpHolderSuffix, "ip-holder-suffix", "", "suffix to append to the ip holder name when using shared IP fail-over with BGP (e.g. ip-holder-suffix=my-cluster-name") From b5bdc78ae9ad95a78327831294c7800911f18498 Mon Sep 17 00:00:00 2001 From: Rahul Sharma Date: Tue, 22 Jul 2025 15:46:48 +0000 Subject: [PATCH 2/6] fix linting failures --- cloud/linode/instances.go | 4 ++-- cloud/linode/vpc.go | 6 ++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/cloud/linode/instances.go b/cloud/linode/instances.go index 926155b0..1be51ccd 100644 --- a/cloud/linode/instances.go +++ b/cloud/linode/instances.go @@ -80,8 +80,8 @@ func (nc *nodeCache) refreshInstances(ctx context.Context, client client.Client) // If running within VPC, find instances and store their ips vpcNodes := map[int][]string{} - for _, vpcName := range Options.VPCNames { - vpcName := strings.TrimSpace(vpcName) + for _, name := range Options.VPCNames { + vpcName := strings.TrimSpace(name) if vpcName == "" { continue } diff --git a/cloud/linode/vpc.go b/cloud/linode/vpc.go index d0d2d195..608f0750 100644 --- a/cloud/linode/vpc.go +++ b/cloud/linode/vpc.go @@ -185,8 +185,7 @@ func validateVPCSubnetFlags() error { } func resolveSubnetNames(client client.Client, vpcID int) ([]string, error) { - var subnetNames []string - + subnetNames := []string{} for _, subnetID := range Options.SubnetIDs { subnet, err := client.GetVPCSubnet(context.TODO(), vpcID, subnetID) if err != nil { @@ -208,8 +207,7 @@ func validateAndSetVPCSubnetFlags(client client.Client) error { Mu.Lock() defer Mu.Unlock() - var vpcNames []string - + vpcNames := []string{} for idx, vpcID := range Options.VPCIDs { vpc, err := client.GetVPC(context.TODO(), vpcID) if err != nil { From f640b633d26406cde572f943075df996994da03e Mon Sep 17 00:00:00 2001 From: Rahul Sharma Date: Tue, 22 Jul 2025 18:51:37 +0000 Subject: [PATCH 3/6] add unittests --- cloud/linode/vpc.go | 6 +- cloud/linode/vpc_test.go | 209 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 214 insertions(+), 1 deletion(-) diff --git a/cloud/linode/vpc.go b/cloud/linode/vpc.go index 608f0750..8f5e595d 100644 --- a/cloud/linode/vpc.go +++ b/cloud/linode/vpc.go @@ -170,9 +170,13 @@ func getNodeBalancerBackendIPv4SubnetID(client client.Client) (int, error) { func validateVPCSubnetFlags() error { switch { + case len(Options.VPCIDs) > 0 && len(Options.SubnetIDs) == 0: + return fmt.Errorf("vpc-ids cannot be set without subnet-ids") case len(Options.SubnetIDs) > 0 && len(Options.VPCIDs) == 0: return fmt.Errorf("subnet-ids cannot be set without vpc-ids") - case len(Options.SubnetNames) > 0 && len(Options.VPCNames) == 0: + // since subnet-names defaults to "default", we also check if route-controller is enabled + // and if so, we require vpc-names to be set + case len(Options.SubnetNames) > 0 && len(Options.VPCNames) == 0 && Options.EnableRouteController: return fmt.Errorf("subnet-names cannot be set without vpc-names") case len(Options.VPCIDs) > 0 && len(Options.VPCNames) > 0: return fmt.Errorf("cannot have both vpc-ids and vpc-names set") diff --git a/cloud/linode/vpc_test.go b/cloud/linode/vpc_test.go index f3b2c059..916536fa 100644 --- a/cloud/linode/vpc_test.go +++ b/cloud/linode/vpc_test.go @@ -288,3 +288,212 @@ func TestGetNodeBalancerBackendIPv4SubnetID(t *testing.T) { } }) } + +func Test_validateVPCSubnetFlags(t *testing.T) { + tests := []struct { + name string + vpcIDs []int + vpcNames []string + subnetIDs []int + subnetNames []string + wantErr bool + }{ + { + name: "invalid flags with vpc-names and vpc-ids set", + vpcIDs: []int{1, 2}, + vpcNames: []string{"vpc1", "vpc2"}, + subnetIDs: []int{1}, + subnetNames: []string{}, + wantErr: true, + }, + { + name: "invalid flags with subnet-names and subnet-ids set", + vpcIDs: []int{}, + vpcNames: []string{"vpc1", "vpc2"}, + subnetIDs: []int{1, 2}, + subnetNames: []string{"subnet1", "subnet2"}, + wantErr: true, + }, + { + name: "invalid flags with subnet-names and no vpc-names", + vpcIDs: []int{}, + vpcNames: []string{}, + subnetIDs: []int{}, + subnetNames: []string{"subnet1", "subnet2"}, + wantErr: true, + }, + { + name: "invalid flags with subnet-ids and no vpc-ids", + vpcIDs: []int{}, + vpcNames: []string{}, + subnetIDs: []int{1, 2}, + subnetNames: []string{}, + wantErr: true, + }, + { + name: "invalid flags with vpc-ids and no subnet-ids", + vpcIDs: []int{1, 2}, + vpcNames: []string{}, + subnetIDs: []int{}, + subnetNames: []string{}, + wantErr: true, + }, + { + name: "valid flags with vpc-names and subnet-names", + vpcIDs: []int{}, + vpcNames: []string{"vpc1", "vpc2"}, + subnetIDs: []int{}, + subnetNames: []string{"subnet1", "subnet2"}, + wantErr: false, + }, + { + name: "valid flags with vpc-ids and subnet-ids", + vpcIDs: []int{1, 2}, + vpcNames: []string{}, + subnetIDs: []int{1, 2}, + subnetNames: []string{}, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + Options.VPCIDs = tt.vpcIDs + Options.VPCNames = tt.vpcNames + Options.SubnetIDs = tt.subnetIDs + Options.SubnetNames = tt.subnetNames + if err := validateVPCSubnetFlags(); (err != nil) != tt.wantErr { + t.Errorf("validateVPCSubnetFlags() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} + +func Test_resolveSubnetNames(t *testing.T) { + t.Run("empty subnet ids", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + client := mocks.NewMockClient(ctrl) + optionsSubnetIDs := Options.SubnetIDs + currSubnetIDs := subnetIDs + defer func() { + Options.SubnetIDs = optionsSubnetIDs + subnetIDs = currSubnetIDs + }() + Options.SubnetIDs = []int{} + // client.EXPECT().GetVPCSubnet(gomock.Any(), gomock.Any(), gomock.Any()).Times(1).Return(&linodego.VPCSubnet{}, errors.New("error")) + subnetNames, err := resolveSubnetNames(client, 10) + if err != nil { + t.Errorf("resolveSubnetNames() error = %v", err) + return + } + if len(subnetNames) != 0 { + t.Errorf("resolveSubnetNames() = %v, want %v", subnetNames, []string{}) + } + }) + + t.Run("fails getting subnet ids", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + client := mocks.NewMockClient(ctrl) + optionsSubnetIDs := Options.SubnetIDs + currSubnetIDs := subnetIDs + defer func() { + Options.SubnetIDs = optionsSubnetIDs + subnetIDs = currSubnetIDs + }() + Options.SubnetIDs = []int{1} + client.EXPECT().GetVPCSubnet(gomock.Any(), gomock.Any(), gomock.Any()).Times(1).Return(&linodego.VPCSubnet{}, errors.New("error")) + _, err := resolveSubnetNames(client, 10) + require.Error(t, err) + }) + + t.Run("correctly resolves subnet names", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + client := mocks.NewMockClient(ctrl) + optionsSubnetIDs := Options.SubnetIDs + currSubnetIDs := subnetIDs + defer func() { + Options.SubnetIDs = optionsSubnetIDs + subnetIDs = currSubnetIDs + }() + Options.SubnetIDs = []int{1} + client.EXPECT().GetVPCSubnet(gomock.Any(), gomock.Any(), gomock.Any()).Times(1).Return(&linodego.VPCSubnet{ID: 1, Label: "subnet1"}, nil) + subnet, err := resolveSubnetNames(client, 10) + require.NoError(t, err) + require.Equal(t, []string{"subnet1"}, subnet, "Expected subnet names to match") + }) +} + +func Test_validateAndSetVPCSubnetFlags(t *testing.T) { + t.Run("invalid flags", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + client := mocks.NewMockClient(ctrl) + Options.VPCIDs = []int{1, 2} + Options.SubnetIDs = []int{} + err := validateAndSetVPCSubnetFlags(client) + require.Error(t, err) + }) + + t.Run("valid flags with vpc-ids and subnet-ids", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + client := mocks.NewMockClient(ctrl) + currVPCIDs := Options.VPCIDs + currSubnetIDs := Options.SubnetIDs + defer func() { + Options.VPCIDs = currVPCIDs + Options.SubnetIDs = currSubnetIDs + vpcIDs = map[string]int{} + subnetIDs = map[string]int{} + }() + Options.VPCIDs = []int{1} + Options.SubnetIDs = []int{1, 2} + client.EXPECT().GetVPC(gomock.Any(), gomock.Any()).Times(1).Return(&linodego.VPC{ID: 1, Label: "test"}, nil) + client.EXPECT().GetVPCSubnet(gomock.Any(), gomock.Any(), gomock.Any()).Times(2).Return(&linodego.VPCSubnet{ID: 1, Label: "subnet1"}, nil) + err := validateAndSetVPCSubnetFlags(client) + require.NoError(t, err) + }) + + t.Run("error while making linode api call", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + client := mocks.NewMockClient(ctrl) + currVPCIDs := Options.VPCIDs + currSubnetIDs := Options.SubnetIDs + defer func() { + Options.VPCIDs = currVPCIDs + Options.SubnetIDs = currSubnetIDs + vpcIDs = map[string]int{} + subnetIDs = map[string]int{} + }() + Options.VPCIDs = []int{1} + Options.SubnetIDs = []int{1, 2} + Options.VPCNames = []string{} + Options.SubnetNames = []string{} + client.EXPECT().GetVPC(gomock.Any(), gomock.Any()).Times(1).Return(nil, errors.New("error")) + err := validateAndSetVPCSubnetFlags(client) + require.Error(t, err) + }) + + t.Run("valid flags with vpc-names and subnet-names", func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + client := mocks.NewMockClient(ctrl) + currVPCNames := Options.VPCNames + currSubnetNames := Options.SubnetNames + defer func() { + Options.VPCNames = currVPCNames + Options.SubnetNames = currSubnetNames + vpcIDs = map[string]int{} + subnetIDs = map[string]int{} + }() + Options.VPCNames = []string{"vpc1"} + Options.SubnetNames = []string{"subnet1", "subnet2"} + Options.VPCIDs = []int{} + Options.SubnetIDs = []int{} + err := validateAndSetVPCSubnetFlags(client) + require.NoError(t, err) + }) +} From 9aa8eb4c1e2165fd1b3e2b6b432a924f8c6d1da0 Mon Sep 17 00:00:00 2001 From: Rahul Sharma Date: Tue, 22 Jul 2025 20:25:52 +0000 Subject: [PATCH 4/6] handle default value for subnet-names, add docs --- cloud/linode/vpc.go | 13 +++++++++---- cloud/linode/vpc_test.go | 8 -------- deploy/chart/templates/daemonset.yaml | 27 +++++++++++++++++++++++++-- deploy/chart/values.yaml | 6 ++++++ docs/configuration/environment.md | 6 ++++-- 5 files changed, 44 insertions(+), 16 deletions(-) diff --git a/cloud/linode/vpc.go b/cloud/linode/vpc.go index 8f5e595d..f161d5fe 100644 --- a/cloud/linode/vpc.go +++ b/cloud/linode/vpc.go @@ -170,6 +170,8 @@ func getNodeBalancerBackendIPv4SubnetID(client client.Client) (int, error) { func validateVPCSubnetFlags() error { switch { + case len(Options.VPCIDs) > 0 && len(Options.VPCNames) > 0: + return fmt.Errorf("cannot have both vpc-ids and vpc-names set") case len(Options.VPCIDs) > 0 && len(Options.SubnetIDs) == 0: return fmt.Errorf("vpc-ids cannot be set without subnet-ids") case len(Options.SubnetIDs) > 0 && len(Options.VPCIDs) == 0: @@ -178,10 +180,6 @@ func validateVPCSubnetFlags() error { // and if so, we require vpc-names to be set case len(Options.SubnetNames) > 0 && len(Options.VPCNames) == 0 && Options.EnableRouteController: return fmt.Errorf("subnet-names cannot be set without vpc-names") - case len(Options.VPCIDs) > 0 && len(Options.VPCNames) > 0: - return fmt.Errorf("cannot have both vpc-ids and vpc-names set") - case len(Options.SubnetIDs) > 0 && len(Options.SubnetNames) > 0: - return fmt.Errorf("cannot have both subnet-ids and subnet-names set") case len(Options.VPCIDs) == 0: return nil } @@ -204,6 +202,11 @@ func resolveSubnetNames(client client.Client, vpcID int) ([]string, error) { } func validateAndSetVPCSubnetFlags(client client.Client) error { + // ignore default subnet-names if subnet-ids are set + if len(Options.SubnetIDs) > 0 { + Options.SubnetNames = []string{} + } + if err := validateVPCSubnetFlags(); err != nil { return err } @@ -231,5 +234,7 @@ func validateAndSetVPCSubnetFlags(client client.Client) error { } Options.VPCNames = append(Options.VPCNames, vpcNames...) + klog.V(3).Infof("VPC IDs: %v, VPC Names: %v, Subnet IDs: %v, Subnet Names: %v", + Options.VPCIDs, Options.VPCNames, Options.SubnetIDs, Options.SubnetNames) return nil } diff --git a/cloud/linode/vpc_test.go b/cloud/linode/vpc_test.go index 916536fa..736f14da 100644 --- a/cloud/linode/vpc_test.go +++ b/cloud/linode/vpc_test.go @@ -306,14 +306,6 @@ func Test_validateVPCSubnetFlags(t *testing.T) { subnetNames: []string{}, wantErr: true, }, - { - name: "invalid flags with subnet-names and subnet-ids set", - vpcIDs: []int{}, - vpcNames: []string{"vpc1", "vpc2"}, - subnetIDs: []int{1, 2}, - subnetNames: []string{"subnet1", "subnet2"}, - wantErr: true, - }, { name: "invalid flags with subnet-names and no vpc-names", vpcIDs: []int{}, diff --git a/deploy/chart/templates/daemonset.yaml b/deploy/chart/templates/daemonset.yaml index dc894f3f..6763add7 100644 --- a/deploy/chart/templates/daemonset.yaml +++ b/deploy/chart/templates/daemonset.yaml @@ -83,6 +83,20 @@ spec: {{- if and .Values.routeController .Values.routeController.subnetNames }} {{- $subnetNames = .Values.routeController.subnetNames }} {{- end }} + {{- $vpcIDs := .Values.vpcIDs }} + {{- if and .Values.routeController .Values.routeController.vpcIDs }} + {{- $vpcIDs = .Values.routeController.vpcIDs }} + {{- end }} + {{- $subnetIDs := .Values.subnetIDs }} + {{- if and .Values.routeController .Values.routeController.subnetIDs }} + {{- $subnetIDs = .Values.routeController.subnetIDs }} + {{- end }} + {{- if and $vpcIDs $vpcNames }} + {{- fail "Both vpcIDs and vpcNames are set. Please use only one." }} + {{- end }} + {{- if and $subnetIDs $subnetNames }} + {{- fail "Both subnetIDs and subnetNames are set. Please use only one." }} + {{- end }} {{- if .Values.disableNodeBalancerVPCBackends }} - --disable-nodebalancer-vpc-backends={{ .Values.disableNodeBalancerVPCBackends }} {{- end }} @@ -99,8 +113,11 @@ spec: {{- end }} {{- if .Values.routeController }} - --enable-route-controller=true - {{- if not $vpcNames }} - {{- fail "vpcNames not set. Please set it when enabling route_controller for VPCs." }} + {{- if not (or $vpcNames $vpcIDs) }} + {{- fail "vpcNames or vpcIDs not set. Please set it when enabling route_controller for VPCs." }} + {{- end }} + {{- if and $vpcIDs (not $subnetIDs) }} + {{- fail "subnetIDs must be set when vpcIDs are set for route-controller" }} {{- end }} {{- if not $clusterCIDR }} {{- fail "clusterCIDR is required if route-controller is enabled" }} @@ -116,6 +133,12 @@ spec: {{- with $subnetNames }} - --subnet-names={{ . }} {{- end }} + {{- with $vpcIDs }} + - --vpc-ids={{ . }} + {{- end }} + {{- with $subnetIDs }} + - --subnet-ids={{ . }} + {{- end }} {{- with $clusterCIDR }} - --cluster-cidr={{ . }} {{- end }} diff --git a/deploy/chart/values.yaml b/deploy/chart/values.yaml index 0f129255..48629d34 100644 --- a/deploy/chart/values.yaml +++ b/deploy/chart/values.yaml @@ -75,8 +75,11 @@ tolerations: # This section adds ability to enable route-controller for ccm # routeController: +# Use one of the two: either [vpcNames and subnetNames] or [vpcIDs and subnetIDs] # vpcNames: # subnetNames: +# vpcIDs: +# subnetIDs: # clusterCIDR: 10.192.0.0/10 # configureCloudRoutes: true @@ -87,8 +90,11 @@ tolerations: # nodeCIDRMaskSizeIPv6: 64 # vpcs and subnets that node internal IPs will be assigned from (not required if already specified in routeController) +# Use one of the two: either [vpcNames and subnetNames] or [vpcIDs and subnetIDs] # vpcNames: # subnetNames: +# vpcIDs: +# subnetIDs: # Enable Linode token health checker # tokenHealthChecker: true diff --git a/docs/configuration/environment.md b/docs/configuration/environment.md index 0ce99c30..c0edd305 100644 --- a/docs/configuration/environment.md +++ b/docs/configuration/environment.md @@ -39,8 +39,10 @@ The CCM supports the following flags: | `--linodego-debug` | `false` | Enables debug output for the LinodeAPI wrapper | | `--enable-route-controller` | `false` | Enables route_controller for CCM | | `--enable-token-health-checker` | `false` | Enables Linode API token health checker | -| `--vpc-names` | `""` | Comma separated VPC names whose routes will be managed by route-controller | -| `--subnet-names` | `""` | Comma separated subnet names whose routes will be managed by route-controller (requires vpc-names flag) | +| `--vpc-names` | `[]` | Comma separated VPC names whose routes will be managed by route-controller | +| `--subnet-names` | `["default"]` | Comma separated subnet names whose routes will be managed by route-controller (requires vpc-names flag) | +| `--vpc-ids` | `[]` | Comma separated VPC ids whose routes will be managed by route-controller | +| `--subnet-ids` | `[]` | Comma separated subnet ids whose routes will be managed by route-controller (requires vpc-ids flag) | | `--load-balancer-type` | `nodebalancer` | Configures which type of load-balancing to use (options: nodebalancer, cilium-bgp) | | `--bgp-node-selector` | `""` | Node selector to use to perform shared IP fail-over with BGP | | `--ip-holder-suffix` | `""` | Suffix to append to the IP holder name when using shared IP fail-over with BGP | From d515402527c8146b69065d312bd4ceeba0a203cc Mon Sep 17 00:00:00 2001 From: Rahul Sharma Date: Tue, 22 Jul 2025 20:48:25 +0000 Subject: [PATCH 5/6] resolve comments --- cloud/linode/vpc.go | 53 ++++++++++++++++++++++----- deploy/chart/templates/daemonset.yaml | 3 ++ 2 files changed, 46 insertions(+), 10 deletions(-) diff --git a/cloud/linode/vpc.go b/cloud/linode/vpc.go index f161d5fe..62d282f2 100644 --- a/cloud/linode/vpc.go +++ b/cloud/linode/vpc.go @@ -168,24 +168,51 @@ func getNodeBalancerBackendIPv4SubnetID(client client.Client) (int, error) { return subnetID, nil } -func validateVPCSubnetFlags() error { - switch { - case len(Options.VPCIDs) > 0 && len(Options.VPCNames) > 0: +// validateBothNameAndIDNotSet checks if both VPC IDs and names are set. +func validateBothNameAndIDNotSet() error { + if len(Options.VPCIDs) != 0 && len(Options.VPCNames) != 0 { return fmt.Errorf("cannot have both vpc-ids and vpc-names set") - case len(Options.VPCIDs) > 0 && len(Options.SubnetIDs) == 0: + } + return nil +} + +// validateVPCAndSubnetPairing checks if both VPC and subnet IDs or names are set correctly. +func validateVPCAndSubnetPairing() error { + if len(Options.VPCIDs) != 0 && len(Options.SubnetIDs) == 0 { return fmt.Errorf("vpc-ids cannot be set without subnet-ids") - case len(Options.SubnetIDs) > 0 && len(Options.VPCIDs) == 0: + } + if len(Options.SubnetIDs) != 0 && len(Options.VPCIDs) == 0 { return fmt.Errorf("subnet-ids cannot be set without vpc-ids") - // since subnet-names defaults to "default", we also check if route-controller is enabled - // and if so, we require vpc-names to be set - case len(Options.SubnetNames) > 0 && len(Options.VPCNames) == 0 && Options.EnableRouteController: + } + return nil +} + +// validateSubnetNamesWithRouteController checks if subnet-names are set without vpc-names +func validateSubnetNamesWithRouteController() error { + if len(Options.SubnetNames) != 0 && len(Options.VPCNames) == 0 && Options.EnableRouteController { return fmt.Errorf("subnet-names cannot be set without vpc-names") - case len(Options.VPCIDs) == 0: - return nil } return nil } +// validateVPCSubnetFlags validates the VPC and subnet flags for the route controller. +// It checks that both VPC and subnet IDs or names are set correctly, and that they are paired correctly. +// It also checks that if subnet names are set, VPC names must also be set, +// and that if subnet IDs are set, VPC IDs must also be set. +func validateVPCSubnetFlags() error { + if err := validateBothNameAndIDNotSet(); err != nil { + return err + } + if err := validateVPCAndSubnetPairing(); err != nil { + return err + } + if err := validateSubnetNamesWithRouteController(); err != nil { + return err + } + return nil +} + +// resolveSubnetNames resolves subnet ids to names for the given VPC ID. func resolveSubnetNames(client client.Client, vpcID int) ([]string, error) { subnetNames := []string{} for _, subnetID := range Options.SubnetIDs { @@ -201,6 +228,9 @@ func resolveSubnetNames(client client.Client, vpcID int) ([]string, error) { return subnetNames, nil } +// validateAndSetVPCSubnetFlags validates the VPC and subnet flags and sets the vpcNames and subnetNames options. +// It retrieves the VPC names and subnet names from the Linode API based on the provided flags. +// If subnet IDs are provided, it resolves the subnet names based on the first VPC ID. func validateAndSetVPCSubnetFlags(client client.Client) error { // ignore default subnet-names if subnet-ids are set if len(Options.SubnetIDs) > 0 { @@ -224,6 +254,9 @@ func validateAndSetVPCSubnetFlags(client client.Client) error { vpcIDs[vpc.Label] = vpcID vpcNames = append(vpcNames, vpc.Label) + // we resolve subnet names only for the first VPC ID + // as there is no vpc to subnet mapping in input flags + // and we assume all subnets are in the same VPC if idx == 0 && len(Options.SubnetIDs) > 0 { subnetNames, err := resolveSubnetNames(client, vpcID) if err != nil { diff --git a/deploy/chart/templates/daemonset.yaml b/deploy/chart/templates/daemonset.yaml index 6763add7..acf99bec 100644 --- a/deploy/chart/templates/daemonset.yaml +++ b/deploy/chart/templates/daemonset.yaml @@ -119,6 +119,9 @@ spec: {{- if and $vpcIDs (not $subnetIDs) }} {{- fail "subnetIDs must be set when vpcIDs are set for route-controller" }} {{- end }} + {{- if and $subnetIDs (not $vpcIDs) }} + {{- fail "vpcIDs must be set when subnetIDs are set for route-controller" }} + {{- end }} {{- if not $clusterCIDR }} {{- fail "clusterCIDR is required if route-controller is enabled" }} {{- end }} From 3747f45caa1987c919645c9ee503fbff505f4c1f Mon Sep 17 00:00:00 2001 From: Rahul Sharma Date: Wed, 23 Jul 2025 14:49:57 +0000 Subject: [PATCH 6/6] update docs to avoid showing slice in example --- docs/configuration/environment.md | 44 +++++++++++++++---------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/docs/configuration/environment.md b/docs/configuration/environment.md index c0edd305..b7565773 100644 --- a/docs/configuration/environment.md +++ b/docs/configuration/environment.md @@ -34,28 +34,28 @@ The CCM can be configured using environment variables and flags. Environment var The CCM supports the following flags: -| Flag | Default | Description | -|------|---------|-------------| -| `--linodego-debug` | `false` | Enables debug output for the LinodeAPI wrapper | -| `--enable-route-controller` | `false` | Enables route_controller for CCM | -| `--enable-token-health-checker` | `false` | Enables Linode API token health checker | -| `--vpc-names` | `[]` | Comma separated VPC names whose routes will be managed by route-controller | -| `--subnet-names` | `["default"]` | Comma separated subnet names whose routes will be managed by route-controller (requires vpc-names flag) | -| `--vpc-ids` | `[]` | Comma separated VPC ids whose routes will be managed by route-controller | -| `--subnet-ids` | `[]` | Comma separated subnet ids whose routes will be managed by route-controller (requires vpc-ids flag) | -| `--load-balancer-type` | `nodebalancer` | Configures which type of load-balancing to use (options: nodebalancer, cilium-bgp) | -| `--bgp-node-selector` | `""` | Node selector to use to perform shared IP fail-over with BGP | -| `--ip-holder-suffix` | `""` | Suffix to append to the IP holder name when using shared IP fail-over with BGP | -| `--default-nodebalancer-type` | `common` | Default type of NodeBalancer to create (options: common, premium) | -| `--nodebalancer-tags` | `[]` | Linode tags to apply to all NodeBalancers | -| `--nodebalancer-backend-ipv4-subnet` | `""` | ipv4 subnet to use for NodeBalancer backends | -| `--nodebalancer-backend-ipv4-subnet-id` | `""` | ipv4 subnet id to use for NodeBalancer backends | -| `--nodebalancer-backend-ipv4-subnet-name` | `""` | ipv4 subnet name to use for NodeBalancer backends | -| `--disable-nodebalancer-vpc-backends` | `false` | don't use VPC specific ip-addresses for nodebalancer backend ips when running in VPC (set to `true` for backward compatibility if needed) | -| `--enable-ipv6-for-loadbalancers` | `false` | Set both IPv4 and IPv6 addresses for all LoadBalancer services (when disabled, only IPv4 is used). This can also be configured per-service using the `service.beta.kubernetes.io/linode-loadbalancer-enable-ipv6-ingress` annotation. | -| `--node-cidr-mask-size-ipv4` | `24` | ipv4 cidr mask size for pod cidrs allocated to nodes | -| `--node-cidr-mask-size-ipv6` | `64` | ipv6 cidr mask size for pod cidrs allocated to nodes | -| `--nodebalancer-prefix` | `ccm` | Name prefix for NoadBalancers. | +| Flag | Type | Default | Description | +|------|------|---------|-------------| +| `--linodego-debug` | Boolean | `false` | Enables debug output for the LinodeAPI wrapper | +| `--enable-route-controller` | Boolean | `false` | Enables route_controller for CCM | +| `--enable-token-health-checker` | Boolean | `false` | Enables Linode API token health checker | +| `--vpc-names` | String (comma separated) | | Comma separated VPC names whose routes will be managed by route-controller | +| `--subnet-names` | String (comma separated) | `"default"` | Comma separated subnet names whose routes will be managed by route-controller (requires vpc-names flag) | +| `--vpc-ids` | Int (comma separated) | | Comma separated VPC ids whose routes will be managed by route-controller | +| `--subnet-ids` | Int (comma separated) | | Comma separated subnet ids whose routes will be managed by route-controller (requires vpc-ids flag) | +| `--load-balancer-type` | String | `nodebalancer` | Configures which type of load-balancing to use (options: nodebalancer, cilium-bgp) | +| `--bgp-node-selector` | String | `""` | Node selector to use to perform shared IP fail-over with BGP | +| `--ip-holder-suffix` | String | `""` | Suffix to append to the IP holder name when using shared IP fail-over with BGP | +| `--default-nodebalancer-type` | String | `common` | Default type of NodeBalancer to create (options: common, premium) | +| `--nodebalancer-tags` | String (comma separated) | | Linode tags to apply to all NodeBalancers | +| `--nodebalancer-backend-ipv4-subnet` | String | `""` | ipv4 subnet to use for NodeBalancer backends | +| `--nodebalancer-backend-ipv4-subnet-id` | Int | `""` | ipv4 subnet id to use for NodeBalancer backends | +| `--nodebalancer-backend-ipv4-subnet-name` | String | `""` | ipv4 subnet name to use for NodeBalancer backends | +| `--disable-nodebalancer-vpc-backends` | Boolean | `false` | don't use VPC specific ip-addresses for nodebalancer backend ips when running in VPC (set to `true` for backward compatibility if needed) | +| `--enable-ipv6-for-loadbalancers` | Boolean | `false` | Set both IPv4 and IPv6 addresses for all LoadBalancer services (when disabled, only IPv4 is used). This can also be configured per-service using the `service.beta.kubernetes.io/linode-loadbalancer-enable-ipv6-ingress` annotation. | +| `--node-cidr-mask-size-ipv4` | Int | `24` | ipv4 cidr mask size for pod cidrs allocated to nodes | +| `--node-cidr-mask-size-ipv6` | Int | `64` | ipv6 cidr mask size for pod cidrs allocated to nodes | +| `--nodebalancer-prefix` | String | `ccm` | Name prefix for NoadBalancers. | ## Configuration Methods