diff --git a/cloud/linode/cloud.go b/cloud/linode/cloud.go index 46230bbf..8d2b6cf2 100644 --- a/cloud/linode/cloud.go +++ b/cloud/linode/cloud.go @@ -6,13 +6,13 @@ import ( "net" "os" "strconv" - "sync" "time" "github.com/spf13/pflag" "golang.org/x/exp/slices" "k8s.io/client-go/informers" cloudprovider "k8s.io/cloud-provider" + "k8s.io/klog/v2" "github.com/linode/linode-cloud-controller-manager/cloud/linode/client" ) @@ -35,41 +35,14 @@ var Options struct { KubeconfigFlag *pflag.Flag LinodeGoDebug bool EnableRouteController bool + // deprecated: use VPCNames instead VPCName string + VPCNames string LoadBalancerType string BGPNodeSelector string LinodeExternalNetwork *net.IPNet } -// vpcDetails is set when VPCName options flag is set. -// We use it to list instances running within the VPC if set -type vpcDetails struct { - mu sync.RWMutex - id int - name string -} - -func (v *vpcDetails) setDetails(client client.Client, name string) error { - v.mu.Lock() - defer v.mu.Unlock() - - id, err := getVPCID(client, Options.VPCName) - if err != nil { - return fmt.Errorf("failed finding VPC ID: %w", err) - } - v.id = id - v.name = name - return nil -} - -func (v *vpcDetails) getID() int { - v.mu.Lock() - defer v.mu.Unlock() - return v.id -} - -var vpcInfo vpcDetails = vpcDetails{id: 0, name: ""} - type linodeCloud struct { client client.Client instances cloudprovider.InstancesV2 @@ -114,11 +87,13 @@ func newCloud() (cloudprovider.Interface, error) { linodeClient.SetDebug(true) } + if Options.VPCName != "" && Options.VPCNames != "" { + return nil, fmt.Errorf("cannot have both vpc-name and vpc-names set") + } + if Options.VPCName != "" { - err := vpcInfo.setDetails(linodeClient, Options.VPCName) - if err != nil { - return nil, fmt.Errorf("failed finding VPC ID: %w", err) - } + klog.Warningf("vpc-name flag is deprecated. Use vpc-names instead") + Options.VPCNames = Options.VPCName } routes, err := newRoutes(linodeClient) diff --git a/cloud/linode/instances.go b/cloud/linode/instances.go index 61e48b6d..fd344f98 100644 --- a/cloud/linode/instances.go +++ b/cloud/linode/instances.go @@ -79,17 +79,28 @@ func (nc *nodeCache) refreshInstances(ctx context.Context, client client.Client) // If running within VPC, find instances and store their ips vpcNodes := map[int][]string{} - vpcID := vpcInfo.getID() - if vpcID != 0 { - resp, err := client.ListVPCIPAddresses(ctx, vpcID, linodego.NewListOptions(0, "")) + vpcNames := strings.Split(Options.VPCNames, ",") + for _, v := range vpcNames { + vpcName := strings.TrimSpace(v) + if vpcName == "" { + continue + } + vpcID, err := GetVPCID(client, strings.TrimSpace(vpcName)) if err != nil { - return err + klog.Errorf("failed updating instances cache for VPC %s. Error: %s", vpcName, err.Error()) + continue } - for _, r := range resp { - if r.Address == nil { - continue + if vpcID != 0 { + resp, err := client.ListVPCIPAddresses(ctx, vpcID, linodego.NewListOptions(0, "")) + if err != nil { + return err + } + for _, r := range resp { + if r.Address == nil { + continue + } + vpcNodes[r.LinodeID] = append(vpcNodes[r.LinodeID], *r.Address) } - vpcNodes[r.LinodeID] = append(vpcNodes[r.LinodeID], *r.Address) } } @@ -97,7 +108,7 @@ func (nc *nodeCache) refreshInstances(ctx context.Context, client client.Client) for i, instance := range instances { // if running within VPC, only store instances in cache which are part of VPC - if vpcID != 0 && len(vpcNodes[instance.ID]) == 0 { + if Options.VPCNames != "" && len(vpcNodes[instance.ID]) == 0 { continue } node := linodeInstance{ diff --git a/cloud/linode/route_controller.go b/cloud/linode/route_controller.go index 909ea76f..fbfe9bcb 100644 --- a/cloud/linode/route_controller.go +++ b/cloud/linode/route_controller.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "strconv" + "strings" "sync" "time" @@ -19,28 +20,40 @@ import ( ) type routeCache struct { - sync.RWMutex + Mu sync.RWMutex routes map[int][]linodego.VPCIP lastUpdate time.Time ttl time.Duration } +// RefreshCache checks if cache has expired and updates it accordingly func (rc *routeCache) refreshRoutes(ctx context.Context, client client.Client) error { - rc.Lock() - defer rc.Unlock() + rc.Mu.Lock() + defer rc.Mu.Unlock() if time.Since(rc.lastUpdate) < rc.ttl { return nil } vpcNodes := map[int][]linodego.VPCIP{} - vpcID := vpcInfo.getID() - resp, err := client.ListVPCIPAddresses(ctx, vpcID, linodego.NewListOptions(0, "")) - if err != nil { - return err - } - for _, r := range resp { - vpcNodes[r.LinodeID] = append(vpcNodes[r.LinodeID], r) + vpcNames := strings.Split(Options.VPCNames, ",") + for _, v := range vpcNames { + vpcName := strings.TrimSpace(v) + if vpcName == "" { + continue + } + vpcID, err := GetVPCID(client, strings.TrimSpace(vpcName)) + if err != nil { + klog.Errorf("failed updating cache for VPC %s. Error: %s", vpcName, err.Error()) + continue + } + resp, err := client.ListVPCIPAddresses(ctx, vpcID, linodego.NewListOptions(0, "")) + if err != nil { + return err + } + for _, r := range resp { + vpcNodes[r.LinodeID] = append(vpcNodes[r.LinodeID], r) + } } rc.routes = vpcNodes @@ -49,7 +62,6 @@ func (rc *routeCache) refreshRoutes(ctx context.Context, client client.Client) e } type routes struct { - vpcid int client client.Client instances *instances routeCache *routeCache @@ -64,13 +76,11 @@ func newRoutes(client client.Client) (cloudprovider.Routes, error) { } klog.V(3).Infof("TTL for routeCache set to %d seconds", timeout) - vpcid := vpcInfo.getID() - if Options.EnableRouteController && vpcid == 0 { - return nil, fmt.Errorf("cannot enable route controller as vpc [%s] not found", Options.VPCName) + if Options.EnableRouteController && Options.VPCNames == "" { + return nil, fmt.Errorf("cannot enable route controller as vpc-names is empty") } return &routes{ - vpcid: vpcid, client: client, instances: newInstances(client), routeCache: &routeCache{ @@ -82,8 +92,8 @@ func newRoutes(client client.Client) (cloudprovider.Routes, error) { // instanceRoutesByID returns routes for given instance id func (r *routes) instanceRoutesByID(id int) ([]linodego.VPCIP, error) { - r.routeCache.RLock() - defer r.routeCache.RUnlock() + r.routeCache.Mu.RLock() + defer r.routeCache.Mu.RUnlock() instanceRoutes, ok := r.routeCache.routes[id] if !ok { return nil, fmt.Errorf("no routes found for instance %d", id) @@ -135,22 +145,25 @@ func (r *routes) CreateRoute(ctx context.Context, clusterName string, nameHint s // check already configured routes intfRoutes := []string{} intfVPCIP := linodego.VPCIP{} - for _, ir := range instanceRoutes { - if ir.VPCID != r.vpcid { - continue - } - if ir.Address != nil { - intfVPCIP = ir - continue - } + for _, vpcid := range GetAllVPCIDs() { + for _, ir := range instanceRoutes { + if ir.VPCID != vpcid { + continue + } - if ir.AddressRange != nil && *ir.AddressRange == route.DestinationCIDR { - klog.V(4).Infof("Route already exists for node %s", route.TargetNode) - return nil - } + if ir.Address != nil { + intfVPCIP = ir + continue + } - intfRoutes = append(intfRoutes, *ir.AddressRange) + if ir.AddressRange != nil && *ir.AddressRange == route.DestinationCIDR { + klog.V(4).Infof("Route already exists for node %s", route.TargetNode) + return nil + } + + intfRoutes = append(intfRoutes, *ir.AddressRange) + } } if intfVPCIP.Address == nil { @@ -185,21 +198,24 @@ func (r *routes) DeleteRoute(ctx context.Context, clusterName string, route *clo // check already configured routes intfRoutes := []string{} intfVPCIP := linodego.VPCIP{} - for _, ir := range instanceRoutes { - if ir.VPCID != r.vpcid { - continue - } - if ir.Address != nil { - intfVPCIP = ir - continue - } + for _, vpcid := range GetAllVPCIDs() { + for _, ir := range instanceRoutes { + if ir.VPCID != vpcid { + continue + } - if ir.AddressRange != nil && *ir.AddressRange == route.DestinationCIDR { - continue - } + if ir.Address != nil { + intfVPCIP = ir + continue + } + + if ir.AddressRange != nil && *ir.AddressRange == route.DestinationCIDR { + continue + } - intfRoutes = append(intfRoutes, *ir.AddressRange) + intfRoutes = append(intfRoutes, *ir.AddressRange) + } } if intfVPCIP.Address == nil { @@ -234,17 +250,19 @@ func (r *routes) ListRoutes(ctx context.Context, clusterName string) ([]*cloudpr } // check for configured routes - for _, ir := range instanceRoutes { - if ir.Address != nil || ir.VPCID != r.vpcid { - continue - } + for _, vpcid := range GetAllVPCIDs() { + for _, ir := range instanceRoutes { + if ir.Address != nil || ir.VPCID != vpcid { + continue + } - if ir.AddressRange != nil { - route := &cloudprovider.Route{ - TargetNode: types.NodeName(instance.Label), - DestinationCIDR: *ir.AddressRange, + if ir.AddressRange != nil { + route := &cloudprovider.Route{ + TargetNode: types.NodeName(instance.Label), + DestinationCIDR: *ir.AddressRange, + } + configuredRoutes = append(configuredRoutes, route) } - configuredRoutes = append(configuredRoutes, route) } } } diff --git a/cloud/linode/route_controller_test.go b/cloud/linode/route_controller_test.go index 2235b18c..cf862340 100644 --- a/cloud/linode/route_controller_test.go +++ b/cloud/linode/route_controller_test.go @@ -10,17 +10,16 @@ import ( "github.com/stretchr/testify/assert" "k8s.io/apimachinery/pkg/types" cloudprovider "k8s.io/cloud-provider" + "k8s.io/utils/ptr" "github.com/linode/linode-cloud-controller-manager/cloud/linode/client/mocks" ) func TestListRoutes(t *testing.T) { - Options.VPCName = "test" + Options.VPCNames = "test" + vpcIDs["test"] = 1 Options.EnableRouteController = true - vpcInfo.id = 1 - vpcid := vpcInfo.getID() - nodeID := 123 name := "mock-instance" publicIPv4 := net.ParseIP("45.76.101.25") @@ -38,7 +37,7 @@ func TestListRoutes(t *testing.T) { client.EXPECT().ListInstances(gomock.Any(), gomock.Any()).Times(1).Return([]linodego.Instance{}, nil) client.EXPECT().ListVPCIPAddresses(gomock.Any(), gomock.Any(), gomock.Any()).Times(1).Return([]linodego.VPCIP{}, nil) - routes, err := routeController.ListRoutes(ctx, "abc") + routes, err := routeController.ListRoutes(ctx, "test") assert.NoError(t, err) assert.Empty(t, routes) }) @@ -61,7 +60,7 @@ func TestListRoutes(t *testing.T) { client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{validInstance}, nil) client.EXPECT().ListVPCIPAddresses(gomock.Any(), gomock.Any(), gomock.Any()).Times(1).Return([]linodego.VPCIP{}, nil) - routes, err := routeController.ListRoutes(ctx, "abc") + routes, err := routeController.ListRoutes(ctx, "test") assert.NoError(t, err) assert.Empty(t, routes) }) @@ -71,7 +70,7 @@ func TestListRoutes(t *testing.T) { { Address: &vpcIP, AddressRange: nil, - VPCID: vpcid, + VPCID: vpcIDs["test"], NAT1To1: nil, LinodeID: nodeID, }, @@ -87,7 +86,7 @@ func TestListRoutes(t *testing.T) { client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{validInstance}, nil) client.EXPECT().ListVPCIPAddresses(gomock.Any(), gomock.Any(), gomock.Any()).Times(2).Return(noRoutesInVPC, nil) - routes, err := routeController.ListRoutes(ctx, "abc") + routes, err := routeController.ListRoutes(ctx, "test") assert.NoError(t, err) assert.Empty(t, routes) }) @@ -98,21 +97,21 @@ func TestListRoutes(t *testing.T) { { Address: &vpcIP, AddressRange: nil, - VPCID: vpcid, + VPCID: vpcIDs["test"], NAT1To1: nil, LinodeID: nodeID, }, { Address: nil, AddressRange: &addressRange1, - VPCID: vpcid, + VPCID: vpcIDs["test"], NAT1To1: nil, LinodeID: nodeID, }, { Address: nil, AddressRange: &addressRange2, - VPCID: vpcid, + VPCID: vpcIDs["test"], NAT1To1: nil, LinodeID: nodeID, }, @@ -128,7 +127,7 @@ func TestListRoutes(t *testing.T) { client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{validInstance}, nil) client.EXPECT().ListVPCIPAddresses(gomock.Any(), gomock.Any(), gomock.Any()).Times(2).Return(routesInVPC, nil) - routes, err := routeController.ListRoutes(ctx, "abc") + routes, err := routeController.ListRoutes(ctx, "test") assert.NoError(t, err) assert.NotEmpty(t, routes) assert.Equal(t, addressRange1, routes[0].DestinationCIDR) @@ -169,7 +168,7 @@ func TestListRoutes(t *testing.T) { client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{validInstance}, nil) client.EXPECT().ListVPCIPAddresses(gomock.Any(), gomock.Any(), gomock.Any()).Times(2).Return(routesInDifferentVPC, nil) - routes, err := routeController.ListRoutes(ctx, "abc") + routes, err := routeController.ListRoutes(ctx, "test") assert.NoError(t, err) assert.Empty(t, routes) }) @@ -177,12 +176,10 @@ func TestListRoutes(t *testing.T) { func TestCreateRoute(t *testing.T) { ctx := context.Background() - Options.VPCName = "test" + Options.VPCNames = "dummy" + vpcIDs["dummy"] = 1 Options.EnableRouteController = true - vpcInfo.id = 1 - vpcid := vpcInfo.getID() - nodeID := 123 name := "mock-instance" publicIPv4 := net.ParseIP("45.76.101.25") @@ -202,14 +199,14 @@ func TestCreateRoute(t *testing.T) { { Address: &vpcIP, AddressRange: nil, - VPCID: vpcid, + VPCID: vpcIDs["dummy"], NAT1To1: nil, LinodeID: nodeID, }, } instanceConfigIntfWithVPCAndRoute := linodego.InstanceConfigInterface{ - VPCID: &vpcid, + VPCID: ptr.To(vpcIDs["dummy"]), IPv4: &linodego.VPCIPv4{VPC: vpcIP}, IPRanges: []string{"10.10.10.0/24"}, } @@ -238,14 +235,14 @@ func TestCreateRoute(t *testing.T) { { Address: &vpcIP, AddressRange: nil, - VPCID: vpcid, + VPCID: vpcIDs["dummy"], NAT1To1: nil, LinodeID: nodeID, }, { Address: nil, AddressRange: &addressRange1, - VPCID: vpcid, + VPCID: vpcIDs["dummy"], NAT1To1: nil, LinodeID: nodeID, }, @@ -279,14 +276,12 @@ func TestCreateRoute(t *testing.T) { } func TestDeleteRoute(t *testing.T) { - Options.VPCName = "test" + Options.VPCNames = "dummy" + vpcIDs["dummy"] = 1 Options.EnableRouteController = true ctx := context.Background() - vpcInfo.id = 1 - vpcid := vpcInfo.getID() - nodeID := 123 name := "mock-instance" publicIPv4 := net.ParseIP("45.76.101.25") @@ -326,14 +321,14 @@ func TestDeleteRoute(t *testing.T) { { Address: &vpcIP, AddressRange: nil, - VPCID: vpcid, + VPCID: vpcIDs["dummy"], NAT1To1: nil, LinodeID: nodeID, }, } instanceConfigIntfWithVPCAndNoRoute := linodego.InstanceConfigInterface{ - VPCID: &vpcid, + VPCID: ptr.To(vpcIDs["dummy"]), IPv4: &linodego.VPCIPv4{VPC: vpcIP}, IPRanges: []string{}, } @@ -356,14 +351,14 @@ func TestDeleteRoute(t *testing.T) { { Address: &vpcIP, AddressRange: nil, - VPCID: vpcid, + VPCID: vpcIDs["dummy"], NAT1To1: nil, LinodeID: nodeID, }, { Address: nil, AddressRange: &addressRange1, - VPCID: vpcid, + VPCID: vpcIDs["dummy"], NAT1To1: nil, LinodeID: nodeID, }, diff --git a/cloud/linode/vpc.go b/cloud/linode/vpc.go index fd40e731..fa505523 100644 --- a/cloud/linode/vpc.go +++ b/cloud/linode/vpc.go @@ -3,11 +3,18 @@ package linode import ( "context" "fmt" + "sync" "github.com/linode/linode-cloud-controller-manager/cloud/linode/client" "github.com/linode/linodego" ) +var ( + Mu sync.RWMutex + // vpcIDs map stores vpc id's for given vpc labels + vpcIDs = make(map[string]int, 0) +) + type vpcLookupError struct { value string } @@ -16,14 +23,33 @@ func (e vpcLookupError) Error() string { return fmt.Sprintf("failed to find VPC: %q", e.value) } -// getVPCID returns the VPC id using the VPC label -func getVPCID(client client.Client, vpcName string) (int, error) { +// GetAllVPCIDs returns vpc ids stored in map +func GetAllVPCIDs() []int { + Mu.Lock() + defer Mu.Unlock() + values := make([]int, 0, len(vpcIDs)) + for _, v := range vpcIDs { + values = append(values, v) + } + return values +} + +// GetVPCID returns the VPC id of given VPC label +func GetVPCID(client client.Client, vpcName string) (int, error) { + Mu.Lock() + defer Mu.Unlock() + + // check if map contains vpc id for given label + if vpcid, ok := vpcIDs[vpcName]; ok { + return vpcid, nil + } vpcs, err := client.ListVPCs(context.TODO(), &linodego.ListOptions{}) if err != nil { return 0, err } for _, vpc := range vpcs { if vpc.Label == vpcName { + vpcIDs[vpcName] = vpc.ID return vpc.ID, nil } } diff --git a/deploy/chart/templates/daemonset.yaml b/deploy/chart/templates/daemonset.yaml index f8ed0397..53d98b6a 100644 --- a/deploy/chart/templates/daemonset.yaml +++ b/deploy/chart/templates/daemonset.yaml @@ -38,7 +38,8 @@ spec: {{- end }} {{- if .Values.routeController }} - --enable-route-controller=true - - --vpc-name={{ required "A valid .Values.routeController.vpcName is required" .Values.routeController.vpcName }} + - --vpc-name={{ "A valid .Values.routeController.vpcName is required" .Values.routeController.vpcName }} + - --vpc-names={{ "A valid .Values.routeController.vpcNames is required" .Values.routeController.vpcNames }} - --configure-cloud-routes={{ default true .Values.routeController.configureCloudRoutes }} - --cluster-cidr={{ required "A valid .Values.routeController.clusterCIDR is required" .Values.routeController.clusterCIDR }} {{- if .Values.routeController.routeReconciliationPeriod }} diff --git a/deploy/chart/values.yaml b/deploy/chart/values.yaml index d360beef..5bd4546c 100644 --- a/deploy/chart/values.yaml +++ b/deploy/chart/values.yaml @@ -51,7 +51,8 @@ tolerations: # This section adds ability to enable route-controller for ccm # routeController: -# vpcName: +# vpcName: [Deprecated: use vpcNames instead] +# vpcNames: # clusterCIDR: 10.0.0.0/8 # configureCloudRoutes: true diff --git a/main.go b/main.go index f8b3c1ee..455c9475 100644 --- a/main.go +++ b/main.go @@ -81,7 +81,8 @@ func main() { // Add Linode-specific flags 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().StringVar(&linode.Options.VPCName, "vpc-name", "", "vpc name whose routes will be managed by route-controller") + command.Flags().StringVar(&linode.Options.VPCName, "vpc-name", "", "[deprecated] vpc name whose routes will be managed by route-controller") + 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.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")