From 2728144520bdbd6fae29f8b07355305dcda76c26 Mon Sep 17 00:00:00 2001 From: Rahul Sharma Date: Thu, 21 Mar 2024 19:34:38 +0000 Subject: [PATCH 1/2] revert PR 184, PR 190 and PR 182 --- README.md | 11 - cloud/linode/client/client.go | 5 - cloud/linode/client/mock_client_test.go | 45 ----- cloud/linode/cloud.go | 23 +-- cloud/linode/instances.go | 135 ++----------- cloud/linode/instances_test.go | 91 +-------- cloud/linode/node_controller.go | 26 +-- cloud/linode/route_controller.go | 254 ------------------------ cloud/linode/vpc.go | 31 --- deploy/chart/templates/daemonset.yaml | 12 -- deploy/chart/values.yaml | 6 - main.go | 2 - 12 files changed, 27 insertions(+), 614 deletions(-) delete mode 100644 cloud/linode/route_controller.go delete mode 100644 cloud/linode/vpc.go diff --git a/README.md b/README.md index 1c91de39..b3e24a34 100644 --- a/README.md +++ b/README.md @@ -126,17 +126,6 @@ Users can create CloudFirewall instances, supply their own rules and attach them **Note**
If the user supplies a firewall-id, and later switches to using an ACL, the CCM will take over the CloudFirewall Instance. To avoid this, delete the service, and re-create it so the original CloudFirewall is left undisturbed. -#### Routes -When running k8s clusters within VPC, node specific podCIDRs need to be allowed on the VPC interface. Linode CCM comes with route-controller functionality which can be enabled for automatically adding/deleting routes on VPC interfaces. When installing CCM with helm, make sure to specify routeController settings. - -##### Example usage in values.yaml -```yaml -routeController: - vpcName: - clusterCIDR: 10.0.0.0/8 - configureCloudRoutes: true -``` - ### Nodes Kubernetes Nodes can be configured with the following annotations. diff --git a/cloud/linode/client/client.go b/cloud/linode/client/client.go index e9d4b15c..3b9bb74d 100644 --- a/cloud/linode/client/client.go +++ b/cloud/linode/client/client.go @@ -16,11 +16,6 @@ type Client interface { ListInstances(context.Context, *linodego.ListOptions) ([]linodego.Instance, error) GetInstanceIPAddresses(context.Context, int) (*linodego.InstanceIPAddressResponse, error) - ListInstanceConfigs(context.Context, int, *linodego.ListOptions) ([]linodego.InstanceConfig, error) - UpdateInstanceConfigInterface(context.Context, int, int, int, linodego.InstanceConfigInterfaceUpdateOptions) (*linodego.InstanceConfigInterface, error) - - ListVPCs(context.Context, *linodego.ListOptions) ([]linodego.VPC, error) - CreateNodeBalancer(context.Context, linodego.NodeBalancerCreateOptions) (*linodego.NodeBalancer, error) GetNodeBalancer(context.Context, int) (*linodego.NodeBalancer, error) UpdateNodeBalancer(context.Context, int, linodego.NodeBalancerUpdateOptions) (*linodego.NodeBalancer, error) diff --git a/cloud/linode/client/mock_client_test.go b/cloud/linode/client/mock_client_test.go index b39dca03..c7535897 100644 --- a/cloud/linode/client/mock_client_test.go +++ b/cloud/linode/client/mock_client_test.go @@ -226,21 +226,6 @@ func (mr *MockClientMockRecorder) ListFirewallDevices(arg0, arg1, arg2 interface return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListFirewallDevices", reflect.TypeOf((*MockClient)(nil).ListFirewallDevices), arg0, arg1, arg2) } -// ListInstanceConfigs mocks base method. -func (m *MockClient) ListInstanceConfigs(arg0 context.Context, arg1 int, arg2 *linodego.ListOptions) ([]linodego.InstanceConfig, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ListInstanceConfigs", arg0, arg1, arg2) - ret0, _ := ret[0].([]linodego.InstanceConfig) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// ListInstanceConfigs indicates an expected call of ListInstanceConfigs. -func (mr *MockClientMockRecorder) ListInstanceConfigs(arg0, arg1, arg2 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListInstanceConfigs", reflect.TypeOf((*MockClient)(nil).ListInstanceConfigs), arg0, arg1, arg2) -} - // ListInstances mocks base method. func (m *MockClient) ListInstances(arg0 context.Context, arg1 *linodego.ListOptions) ([]linodego.Instance, error) { m.ctrl.T.Helper() @@ -301,21 +286,6 @@ func (mr *MockClientMockRecorder) ListNodeBalancers(arg0, arg1 interface{}) *gom return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListNodeBalancers", reflect.TypeOf((*MockClient)(nil).ListNodeBalancers), arg0, arg1) } -// ListVPCs mocks base method. -func (m *MockClient) ListVPCs(arg0 context.Context, arg1 *linodego.ListOptions) ([]linodego.VPC, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ListVPCs", arg0, arg1) - ret0, _ := ret[0].([]linodego.VPC) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// ListVPCs indicates an expected call of ListVPCs. -func (mr *MockClientMockRecorder) ListVPCs(arg0, arg1 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListVPCs", reflect.TypeOf((*MockClient)(nil).ListVPCs), arg0, arg1) -} - // RebuildNodeBalancerConfig mocks base method. func (m *MockClient) RebuildNodeBalancerConfig(arg0 context.Context, arg1, arg2 int, arg3 linodego.NodeBalancerConfigRebuildOptions) (*linodego.NodeBalancerConfig, error) { m.ctrl.T.Helper() @@ -346,21 +316,6 @@ func (mr *MockClientMockRecorder) UpdateFirewallRules(arg0, arg1, arg2 interface return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateFirewallRules", reflect.TypeOf((*MockClient)(nil).UpdateFirewallRules), arg0, arg1, arg2) } -// UpdateInstanceConfigInterface mocks base method. -func (m *MockClient) UpdateInstanceConfigInterface(arg0 context.Context, arg1, arg2, arg3 int, arg4 linodego.InstanceConfigInterfaceUpdateOptions) (*linodego.InstanceConfigInterface, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "UpdateInstanceConfigInterface", arg0, arg1, arg2, arg3, arg4) - ret0, _ := ret[0].(*linodego.InstanceConfigInterface) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// UpdateInstanceConfigInterface indicates an expected call of UpdateInstanceConfigInterface. -func (mr *MockClientMockRecorder) UpdateInstanceConfigInterface(arg0, arg1, arg2, arg3, arg4 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateInstanceConfigInterface", reflect.TypeOf((*MockClient)(nil).UpdateInstanceConfigInterface), arg0, arg1, arg2, arg3, arg4) -} - // UpdateNodeBalancer mocks base method. func (m *MockClient) UpdateNodeBalancer(arg0 context.Context, arg1 int, arg2 linodego.NodeBalancerUpdateOptions) (*linodego.NodeBalancer, error) { m.ctrl.T.Helper() diff --git a/cloud/linode/cloud.go b/cloud/linode/cloud.go index 5e9b0436..123c2039 100644 --- a/cloud/linode/cloud.go +++ b/cloud/linode/cloud.go @@ -25,17 +25,14 @@ const ( // We expect it to be initialized with flags external to this package, likely in // main.go var Options struct { - KubeconfigFlag *pflag.Flag - LinodeGoDebug bool - EnableRouteController bool - VPCName string + KubeconfigFlag *pflag.Flag + LinodeGoDebug bool } type linodeCloud struct { client client.Client instances cloudprovider.InstancesV2 loadbalancers cloudprovider.LoadBalancer - routes cloudprovider.Routes } func init() { @@ -70,19 +67,12 @@ func newCloud() (cloudprovider.Interface, error) { linodeClient.SetDebug(true) } - routes, err := newRoutes(linodeClient) - if err != nil { - return nil, fmt.Errorf("routes client was not created successfully: %w", err) - } - - // create struct that satisfies cloudprovider.Interface - lcloud := &linodeCloud{ + // Return struct that satisfies cloudprovider.Interface + return &linodeCloud{ client: linodeClient, instances: newInstances(linodeClient), loadbalancers: newLoadbalancers(linodeClient, region), - routes: routes, - } - return lcloud, nil + }, nil } func (c *linodeCloud) Initialize(clientBuilder cloudprovider.ControllerClientBuilder, stopCh <-chan struct{}) { @@ -119,9 +109,6 @@ func (c *linodeCloud) Clusters() (cloudprovider.Clusters, bool) { } func (c *linodeCloud) Routes() (cloudprovider.Routes, bool) { - if Options.EnableRouteController { - return c.routes, true - } return nil, false } diff --git a/cloud/linode/instances.go b/cloud/linode/instances.go index ee027425..55362439 100644 --- a/cloud/linode/instances.go +++ b/cloud/linode/instances.go @@ -9,72 +9,21 @@ import ( "time" "github.com/linode/linodego" - "golang.org/x/sync/errgroup" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" cloudprovider "k8s.io/cloud-provider" - "k8s.io/klog/v2" "github.com/linode/linode-cloud-controller-manager/cloud/linode/client" "github.com/linode/linode-cloud-controller-manager/sentry" ) -type nodeIP struct { - ip string - ipType v1.NodeAddressType -} - -type linodeInstance struct { - instance *linodego.Instance - ips []nodeIP -} - type nodeCache struct { sync.RWMutex - nodes map[int]linodeInstance + nodes map[int]*linodego.Instance lastUpdate time.Time ttl time.Duration } -// getInstanceIPv4Addresses returns all ipv4 addresses configured on a linode. -func (nc *nodeCache) getInstanceIPv4Addresses(ctx context.Context, id int, client client.Client) ([]nodeIP, error) { - // Retrieve ipaddresses for the linode - addresses, err := client.GetInstanceIPAddresses(ctx, id) - if err != nil { - return nil, err - } - - var ips []nodeIP - if len(addresses.IPv4.Public) != 0 { - for _, ip := range addresses.IPv4.Public { - ips = append(ips, nodeIP{ip: ip.Address, ipType: v1.NodeExternalIP}) - } - } - - // Retrieve instance configs for the linode - configs, err := client.ListInstanceConfigs(ctx, id, &linodego.ListOptions{}) - if err != nil || len(configs) == 0 { - return nil, err - } - - // Iterate over interfaces in config and find VPC specific ips - for _, iface := range configs[0].Interfaces { - if iface.VPCID != nil && iface.IPv4.VPC != "" { - ips = append(ips, nodeIP{ip: iface.IPv4.VPC, ipType: v1.NodeInternalIP}) - } - } - - // NOTE: We specifically store VPC ips first so that if they exist, they are - // used as internal ip for the nodes than the private ip - if len(addresses.IPv4.Private) != 0 { - for _, ip := range addresses.IPv4.Private { - ips = append(ips, nodeIP{ip: ip.Address, ipType: v1.NodeInternalIP}) - } - } - - return ips, nil -} - // refreshInstances conditionally loads all instances from the Linode API and caches them. // It does not refresh if the last update happened less than `nodeCache.ttl` ago. func (nc *nodeCache) refreshInstances(ctx context.Context, client client.Client) error { @@ -90,29 +39,11 @@ func (nc *nodeCache) refreshInstances(ctx context.Context, client client.Client) return err } - nc.nodes = make(map[int]linodeInstance, len(instances)) + nc.nodes = make(map[int]*linodego.Instance) - mtx := sync.Mutex{} - g := new(errgroup.Group) for _, instance := range instances { instance := instance - g.Go(func() error { - addresses, err := nc.getInstanceIPv4Addresses(ctx, instance.ID, client) - if err != nil { - klog.Errorf("Failed fetching ip addresses for instance id %d. Error: %s", instance.ID, err.Error()) - return err - } - // take lock on map so that concurrent writes are safe - mtx.Lock() - defer mtx.Unlock() - node := linodeInstance{instance: &instance, ips: addresses} - nc.nodes[instance.ID] = node - return nil - }) - } - - if err := g.Wait(); err != nil { - return err + nc.nodes[instance.ID] = &instance } nc.lastUpdate = time.Now() @@ -133,10 +64,9 @@ func newInstances(client client.Client) *instances { timeout = t } } - klog.V(3).Infof("TTL for nodeCache set to %d", timeout) return &instances{client, &nodeCache{ - nodes: make(map[int]linodeInstance, 0), + nodes: make(map[int]*linodego.Instance), ttl: time.Duration(timeout) * time.Second, }} } @@ -153,8 +83,8 @@ func (i *instances) linodeByName(nodeName types.NodeName) (*linodego.Instance, e i.nodeCache.RLock() defer i.nodeCache.RUnlock() for _, node := range i.nodeCache.nodes { - if node.instance.Label == string(nodeName) { - return node.instance, nil + if node.Label == string(nodeName) { + return node, nil } } @@ -164,24 +94,11 @@ func (i *instances) linodeByName(nodeName types.NodeName) (*linodego.Instance, e func (i *instances) linodeByID(id int) (*linodego.Instance, error) { i.nodeCache.RLock() defer i.nodeCache.RUnlock() - linodeInstance, ok := i.nodeCache.nodes[id] + instance, ok := i.nodeCache.nodes[id] if !ok { return nil, cloudprovider.InstanceNotFound } - return linodeInstance.instance, nil -} - -// listAllInstances returns all instances in nodeCache -func (i *instances) listAllInstances(ctx context.Context) ([]linodego.Instance, error) { - if err := i.nodeCache.refreshInstances(ctx, i.client); err != nil { - return nil, err - } - - instances := []linodego.Instance{} - for _, linodeInstance := range i.nodeCache.nodes { - instances = append(instances, *linodeInstance.instance) - } - return instances, nil + return instance, nil } func (i *instances) lookupLinode(ctx context.Context, node *v1.Node) (*linodego.Instance, error) { @@ -248,13 +165,7 @@ func (i *instances) InstanceMetadata(ctx context.Context, node *v1.Node) (*cloud return nil, err } - ips, err := i.getLinodeIPv4Addresses(ctx, node) - if err != nil { - sentry.CaptureError(ctx, err) - return nil, err - } - - if len(ips) == 0 { + if len(linode.IPv4) == 0 { err := instanceNoIPAddressesError{linode.ID} sentry.CaptureError(ctx, err) return nil, err @@ -262,8 +173,12 @@ func (i *instances) InstanceMetadata(ctx context.Context, node *v1.Node) (*cloud addresses := []v1.NodeAddress{{Type: v1.NodeHostName, Address: linode.Label}} - for _, ip := range ips { - addresses = append(addresses, v1.NodeAddress{Type: ip.ipType, Address: ip.ip}) + for _, ip := range linode.IPv4 { + ipType := v1.NodeExternalIP + if ip.IsPrivate() { + ipType = v1.NodeInternalIP + } + addresses = append(addresses, v1.NodeAddress{Type: ipType, Address: ip.String()}) } // note that Zone is omitted as it's not a thing in Linode @@ -276,23 +191,3 @@ func (i *instances) InstanceMetadata(ctx context.Context, node *v1.Node) (*cloud return meta, nil } - -func (i *instances) getLinodeIPv4Addresses(ctx context.Context, node *v1.Node) ([]nodeIP, error) { - ctx = sentry.SetHubOnContext(ctx) - instance, err := i.lookupLinode(ctx, node) - if err != nil { - sentry.CaptureError(ctx, err) - return nil, err - } - - i.nodeCache.RLock() - defer i.nodeCache.RUnlock() - linodeInstance, ok := i.nodeCache.nodes[instance.ID] - if !ok || len(linodeInstance.ips) == 0 { - err := instanceNoIPAddressesError{instance.ID} - sentry.CaptureError(ctx, err) - return nil, err - } - - return linodeInstance.ips, nil -} diff --git a/cloud/linode/instances_test.go b/cloud/linode/instances_test.go index 2eadd4e4..a93711a1 100644 --- a/cloud/linode/instances_test.go +++ b/cloud/linode/instances_test.go @@ -64,18 +64,6 @@ func TestInstanceExists(t *testing.T) { Type: "g6-standard-2", }, }, nil) - client.EXPECT().GetInstanceIPAddresses(gomock.Any(), 123).Times(1).Return(&linodego.InstanceIPAddressResponse{ - IPv4: &linodego.InstanceIPv4Response{ - Public: []*linodego.InstanceIP{ - {Address: "45.76.101.25"}, - }, - Private: []*linodego.InstanceIP{ - {Address: "192.168.133.65"}, - }, - }, - IPv6: nil, - }, nil) - client.EXPECT().ListInstanceConfigs(gomock.Any(), 123, gomock.Any()).Times(1).Return([]linodego.InstanceConfig{}, nil) exists, err := instances.InstanceExists(ctx, node) assert.NoError(t, err) @@ -90,18 +78,6 @@ func TestInstanceExists(t *testing.T) { client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{ {ID: 123, Label: name}, }, nil) - client.EXPECT().GetInstanceIPAddresses(gomock.Any(), 123).Times(1).Return(&linodego.InstanceIPAddressResponse{ - IPv4: &linodego.InstanceIPv4Response{ - Public: []*linodego.InstanceIP{ - {Address: "45.76.101.25"}, - }, - Private: []*linodego.InstanceIP{ - {Address: "192.168.133.65"}, - }, - }, - IPv6: nil, - }, nil) - client.EXPECT().ListInstanceConfigs(gomock.Any(), 123, gomock.Any()).Times(1).Return([]linodego.InstanceConfig{}, nil) exists, err := instances.InstanceExists(ctx, node) assert.NoError(t, err) @@ -151,27 +127,13 @@ func TestMetadataRetrieval(t *testing.T) { client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{ {ID: id, Label: name, Type: linodeType, Region: region, IPv4: []*net.IP{&publicIPv4, &privateIPv4}}, }, nil) - client.EXPECT().GetInstanceIPAddresses(gomock.Any(), id).Times(1).Return(&linodego.InstanceIPAddressResponse{ - IPv4: &linodego.InstanceIPv4Response{ - Public: []*linodego.InstanceIP{ - {Address: "45.76.101.25"}, - }, - Private: []*linodego.InstanceIP{ - {Address: "192.168.133.65"}, - }, - }, - IPv6: nil, - }, nil) - client.EXPECT().ListInstanceConfigs(gomock.Any(), 123, gomock.Any()).Times(1).Return([]linodego.InstanceConfig{ - {ID: 123456}, - }, nil) meta, err := instances.InstanceMetadata(ctx, node) assert.NoError(t, err) assert.Equal(t, providerIDPrefix+strconv.Itoa(id), meta.ProviderID) assert.Equal(t, region, meta.Region) assert.Equal(t, linodeType, meta.InstanceType) - assert.Equal(t, []v1.NodeAddress{ + assert.Equal(t, meta.NodeAddresses, []v1.NodeAddress{ { Type: v1.NodeHostName, Address: name, @@ -184,7 +146,7 @@ func TestMetadataRetrieval(t *testing.T) { Type: v1.NodeInternalIP, Address: privateIPv4.String(), }, - }, meta.NodeAddresses) + }) }) ipTests := []struct { @@ -235,24 +197,12 @@ func TestMetadataRetrieval(t *testing.T) { node := nodeWithProviderID(providerID) ips := make([]*net.IP, 0, len(test.inputIPs)) - pubIPs := make([]*linodego.InstanceIP, 0) - privIPs := make([]*linodego.InstanceIP, 0) for _, ip := range test.inputIPs { parsed := net.ParseIP(ip) if parsed == nil { t.Fatalf("cannot parse %v as an ipv4", ip) } ips = append(ips, &parsed) - if parsed.IsPrivate() { - privIPs = append(privIPs, &linodego.InstanceIP{Address: ip}) - } else { - pubIPs = append(pubIPs, &linodego.InstanceIP{Address: ip}) - } - } - - ipv4s := &linodego.InstanceIPv4Response{ - Public: pubIPs, - Private: privIPs, } linodeType := "g6-standard-1" @@ -260,13 +210,6 @@ func TestMetadataRetrieval(t *testing.T) { client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{ {ID: id, Label: name, Type: linodeType, Region: region, IPv4: ips}, }, nil) - client.EXPECT().GetInstanceIPAddresses(gomock.Any(), id).Times(1).Return(&linodego.InstanceIPAddressResponse{ - IPv4: ipv4s, - IPv6: nil, - }, nil) - client.EXPECT().ListInstanceConfigs(gomock.Any(), id, gomock.Any()).Times(1).Return([]linodego.InstanceConfig{ - {ID: 123456}, - }, nil) meta, err := instances.InstanceMetadata(ctx, node) @@ -338,16 +281,6 @@ func TestInstanceShutdown(t *testing.T) { client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{ {ID: id, Label: "offline-linode", Status: linodego.InstanceOffline}, }, nil) - client.EXPECT().GetInstanceIPAddresses(gomock.Any(), id).Times(1).Return(&linodego.InstanceIPAddressResponse{ - IPv4: &linodego.InstanceIPv4Response{ - Public: []*linodego.InstanceIP{}, - Private: []*linodego.InstanceIP{}, - }, - IPv6: nil, - }, nil) - client.EXPECT().ListInstanceConfigs(gomock.Any(), id, gomock.Any()).Times(1).Return([]linodego.InstanceConfig{ - {ID: 123456}, - }, nil) shutdown, err := instances.InstanceShutdown(ctx, node) assert.NoError(t, err) @@ -361,16 +294,6 @@ func TestInstanceShutdown(t *testing.T) { client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{ {ID: id, Label: "shutting-down-linode", Status: linodego.InstanceShuttingDown}, }, nil) - client.EXPECT().GetInstanceIPAddresses(gomock.Any(), id).Times(1).Return(&linodego.InstanceIPAddressResponse{ - IPv4: &linodego.InstanceIPv4Response{ - Public: []*linodego.InstanceIP{}, - Private: []*linodego.InstanceIP{}, - }, - IPv6: nil, - }, nil) - client.EXPECT().ListInstanceConfigs(gomock.Any(), id, gomock.Any()).Times(1).Return([]linodego.InstanceConfig{ - {ID: 123456}, - }, nil) shutdown, err := instances.InstanceShutdown(ctx, node) assert.NoError(t, err) @@ -384,16 +307,6 @@ func TestInstanceShutdown(t *testing.T) { client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{ {ID: id, Label: "running-linode", Status: linodego.InstanceRunning}, }, nil) - client.EXPECT().GetInstanceIPAddresses(gomock.Any(), id).Times(1).Return(&linodego.InstanceIPAddressResponse{ - IPv4: &linodego.InstanceIPv4Response{ - Public: []*linodego.InstanceIP{}, - Private: []*linodego.InstanceIP{}, - }, - IPv6: nil, - }, nil) - client.EXPECT().ListInstanceConfigs(gomock.Any(), id, gomock.Any()).Times(1).Return([]linodego.InstanceConfig{ - {ID: 123456}, - }, nil) shutdown, err := instances.InstanceShutdown(ctx, node) assert.NoError(t, err) diff --git a/cloud/linode/node_controller.go b/cloud/linode/node_controller.go index 461407c9..e77eb20f 100644 --- a/cloud/linode/node_controller.go +++ b/cloud/linode/node_controller.go @@ -127,9 +127,8 @@ func (s *nodeController) handleNode(ctx context.Context, node *v1.Node) error { lastUpdate := s.LastMetadataUpdate(node.Name) - uuid, foundLabel := node.Labels[annotations.AnnLinodeHostUUID] - configuredPrivateIP, foundAnnotation := node.Annotations[annotations.AnnLinodeNodePrivateIP] - if foundLabel && foundAnnotation && time.Since(lastUpdate) < s.ttl { + uuid, ok := node.Labels[annotations.AnnLinodeHostUUID] + if ok && time.Since(lastUpdate) < s.ttl { return nil } @@ -139,19 +138,7 @@ func (s *nodeController) handleNode(ctx context.Context, node *v1.Node) error { return err } - expectedPrivateIP := "" - // linode API response for linode will contain only one private ip - // if any private ip is configured. If it changes in future or linode - // supports other subnets with nodebalancer, this logic needs to be updated. - // https://www.linode.com/docs/api/linode-instances/#linode-view - for _, addr := range linode.IPv4 { - if addr.IsPrivate() { - expectedPrivateIP = addr.String() - break - } - } - - if uuid == linode.HostUUID && configuredPrivateIP == expectedPrivateIP { + if uuid == linode.HostUUID { s.SetLastMetadataUpdate(node.Name) return nil } @@ -163,16 +150,13 @@ func (s *nodeController) handleNode(ctx context.Context, node *v1.Node) error { return err } - // It may be that the UUID label and private ip annotation has been set - if n.Labels[annotations.AnnLinodeHostUUID] == linode.HostUUID && n.Annotations[annotations.AnnLinodeNodePrivateIP] == expectedPrivateIP { + // It may be that the UUID has been set + if n.Labels[annotations.AnnLinodeHostUUID] == linode.HostUUID { return nil } // Try to update the node n.Labels[annotations.AnnLinodeHostUUID] = linode.HostUUID - if expectedPrivateIP != "" { - n.Annotations[annotations.AnnLinodeNodePrivateIP] = expectedPrivateIP - } _, err = s.kubeclient.CoreV1().Nodes().Update(ctx, n, metav1.UpdateOptions{}) return err }); err != nil { diff --git a/cloud/linode/route_controller.go b/cloud/linode/route_controller.go deleted file mode 100644 index dd634c29..00000000 --- a/cloud/linode/route_controller.go +++ /dev/null @@ -1,254 +0,0 @@ -package linode - -import ( - "context" - "fmt" - "os" - "strconv" - "sync" - "time" - - "github.com/linode/linodego" - "golang.org/x/sync/errgroup" - "golang.org/x/exp/slices" - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" - cloudprovider "k8s.io/cloud-provider" - "k8s.io/klog/v2" - - "github.com/linode/linode-cloud-controller-manager/cloud/linode/client" -) - -type routeCache struct { - sync.RWMutex - routes map[int][]linodego.InstanceConfig - lastUpdate time.Time - ttl time.Duration -} - -func (rc *routeCache) refreshRoutes(ctx context.Context, client client.Client) error { - rc.Lock() - defer rc.Unlock() - - if time.Since(rc.lastUpdate) < rc.ttl { - return nil - } - - instances, err := client.ListInstances(ctx, nil) - if err != nil { - return err - } - - rc.routes = make(map[int][]linodego.InstanceConfig, len(instances)) - - mtx := sync.Mutex{} - g := new(errgroup.Group) - for _, instance := range instances { - id := instance.ID - g.Go(func() error { - configs, err := client.ListInstanceConfigs(ctx, id, &linodego.ListOptions{}) - if err != nil { - klog.Errorf("Failed fetching instance configs for instance id %d. Error: %s", id, err.Error()) - return err - } - // take lock on map so that concurrent writes are safe - mtx.Lock() - defer mtx.Unlock() - rc.routes[id] = configs - return nil - }) - } - - if err := g.Wait(); err != nil { - return err - } - - rc.lastUpdate = time.Now() - return nil -} - -type routes struct { - vpcid int - client client.Client - instances *instances - routeCache *routeCache -} - -func newRoutes(client client.Client) (cloudprovider.Routes, error) { - timeout := 60 - if raw, ok := os.LookupEnv("LINODE_ROUTES_CACHE_TTL"); ok { - if t, _ := strconv.Atoi(raw); t > 0 { - timeout = t - } - } - klog.V(3).Infof("TTL for routeCache set to %d", timeout) - - vpcid := 0 - if Options.EnableRouteController { - id, err := getVPCID(client, Options.VPCName) - if err != nil { - return nil, err - } - vpcid = id - } - - return &routes{ - vpcid: vpcid, - client: client, - instances: newInstances(client), - routeCache: &routeCache{ - routes: make(map[int][]linodego.InstanceConfig, 0), - ttl: time.Duration(timeout) * time.Second, - }, - }, nil -} - -// instanceConfigsByID returns InstanceConfigs for given instance id -func (r *routes) instanceConfigsByID(id int) ([]linodego.InstanceConfig, error) { - r.routeCache.RLock() - defer r.routeCache.RUnlock() - instanceConfigs, ok := r.routeCache.routes[id] - if !ok { - return nil, fmt.Errorf("no configs found for instance %d", id) - } - return instanceConfigs, nil -} - -// getInstanceConfigs returns InstanceConfigs for given instance id -// It refreshes routeCache if it has expired -func (r *routes) getInstanceConfigs(ctx context.Context, id int) ([]linodego.InstanceConfig, error) { - if err := r.routeCache.refreshRoutes(ctx, r.client); err != nil { - return nil, err - } - - return r.instanceConfigsByID(id) -} - -// getInstanceFromName returns linode instance with given name if it exists -func (r *routes) getInstanceFromName(ctx context.Context, name string) (*linodego.Instance, error) { - // create node object - node := &v1.Node{ - ObjectMeta: metav1.ObjectMeta{ - Name: name, - }, - } - - // fetch instance with specified node name - instance, err := r.instances.lookupLinode(ctx, node) - if err != nil { - klog.Errorf("failed getting linode %s", name) - return nil, err - } - return instance, nil -} - -// CreateRoute adds route's subnet to ip_ranges of target node's VPC interface -func (r *routes) CreateRoute(ctx context.Context, clusterName string, nameHint string, route *cloudprovider.Route) error { - instance, err := r.getInstanceFromName(ctx, string(route.TargetNode)) - if err != nil { - return err - } - - // fetch instance configs - configs, err := r.getInstanceConfigs(ctx, instance.ID) - if err != nil { - return err - } - - // find VPC interface and add route to it - for _, iface := range configs[0].Interfaces { - if iface.VPCID == nil || r.vpcid != *iface.VPCID || iface.IPv4.VPC == "" { - continue - } - - if slices.Contains(iface.IPRanges, route.DestinationCIDR) { - klog.V(4).Infof("Route already exists for node %s", route.TargetNode) - return nil - } - - ipRanges := append(iface.IPRanges, route.DestinationCIDR) - interfaceUpdateOptions := linodego.InstanceConfigInterfaceUpdateOptions{ - IPRanges: ipRanges, - } - resp, err := r.client.UpdateInstanceConfigInterface(ctx, instance.ID, configs[0].ID, iface.ID, interfaceUpdateOptions) - if err != nil { - return err - } - klog.V(4).Infof("Added routes for node %s. Current routes: %v", route.TargetNode, resp.IPRanges) - return nil - } - - return fmt.Errorf("unable to add route %s for node %s. no valid interface found", route.DestinationCIDR, route.TargetNode) -} - -// DeleteRoute removes route's subnet from ip_ranges of target node's VPC interface -func (r *routes) DeleteRoute(ctx context.Context, clusterName string, route *cloudprovider.Route) error { - instance, err := r.getInstanceFromName(ctx, string(route.TargetNode)) - if err != nil { - return err - } - - configs, err := r.getInstanceConfigs(ctx, instance.ID) - if err != nil { - return err - } - - for _, iface := range configs[0].Interfaces { - if iface.VPCID == nil || r.vpcid != *iface.VPCID || iface.IPv4.VPC == "" { - continue - } - - ipRanges := []string{} - for _, configured_route := range iface.IPRanges { - if configured_route != route.DestinationCIDR { - ipRanges = append(ipRanges, configured_route) - } - } - - interfaceUpdateOptions := linodego.InstanceConfigInterfaceUpdateOptions{ - IPRanges: ipRanges, - } - resp, err := r.client.UpdateInstanceConfigInterface(ctx, instance.ID, configs[0].ID, iface.ID, interfaceUpdateOptions) - if err != nil { - return err - } - klog.V(4).Infof("Deleted route for node %s. Current routes: %v", route.TargetNode, resp.IPRanges) - return nil - } - return fmt.Errorf("unable to remove route %s for node %s", route.DestinationCIDR, route.TargetNode) -} - -// ListRoutes fetches routes configured on all instances which have VPC interfaces -func (r *routes) ListRoutes(ctx context.Context, clusterName string) ([]*cloudprovider.Route, error) { - klog.V(4).Infof("Fetching routes configured on the cluster") - instances, err := r.instances.listAllInstances(ctx) - if err != nil { - return nil, err - } - - var routes []*cloudprovider.Route - for _, instance := range instances { - configs, err := r.getInstanceConfigs(ctx, instance.ID) - if err != nil { - klog.Errorf("Failed finding routes for instance id %d. Error: %v", instance.ID, err) - continue - } - - for _, iface := range configs[0].Interfaces { - if iface.VPCID == nil || r.vpcid != *iface.VPCID || iface.IPv4.VPC == "" { - continue - } - - for _, ipsubnet := range iface.IPRanges { - route := &cloudprovider.Route{ - TargetNode: types.NodeName(instance.Label), - DestinationCIDR: ipsubnet, - } - klog.V(4).Infof("Found route: node %s, route %s", instance.Label, route.DestinationCIDR) - routes = append(routes, route) - } - } - } - return routes, nil -} diff --git a/cloud/linode/vpc.go b/cloud/linode/vpc.go deleted file mode 100644 index fd40e731..00000000 --- a/cloud/linode/vpc.go +++ /dev/null @@ -1,31 +0,0 @@ -package linode - -import ( - "context" - "fmt" - - "github.com/linode/linode-cloud-controller-manager/cloud/linode/client" - "github.com/linode/linodego" -) - -type vpcLookupError struct { - value string -} - -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) { - vpcs, err := client.ListVPCs(context.TODO(), &linodego.ListOptions{}) - if err != nil { - return 0, err - } - for _, vpc := range vpcs { - if vpc.Label == vpcName { - return vpc.ID, nil - } - } - return 0, vpcLookupError{vpcName} -} diff --git a/deploy/chart/templates/daemonset.yaml b/deploy/chart/templates/daemonset.yaml index 4b2b3d41..86d45dd5 100644 --- a/deploy/chart/templates/daemonset.yaml +++ b/deploy/chart/templates/daemonset.yaml @@ -33,18 +33,6 @@ spec: - --v=3 - --port=0 - --secure-port=10253 - {{- if .Values.linodegoDebug }} - - --linodego-debug={{ .Values.linodegoDebug }} - {{- end }} - {{- if .Values.routeController }} - - --enable-route-controller=true - - --vpc-name={{ .Values.routeController.vpcName }} - - --configure-cloud-routes={{ .Values.routeController.configureCloudRoutes }} - - --cluster-cidr={{ .Values.routeController.clusterCIDR }} - {{- if .Values.routeController.routeReconciliationPeriod }} - - --route-reconciliation-period={{ .Values.routeController.routeReconciliationPeriod }} - {{- end }} - {{- end }} volumeMounts: - mountPath: /etc/kubernetes name: k8s diff --git a/deploy/chart/values.yaml b/deploy/chart/values.yaml index 2df40a14..d8bf72ee 100644 --- a/deploy/chart/values.yaml +++ b/deploy/chart/values.yaml @@ -44,12 +44,6 @@ tolerations: operator: Exists effect: NoSchedule -# This section adds ability to enable route-controller for ccm -# routeController: -# vpcName: -# clusterCIDR: 10.0.0.0/8 -# configureCloudRoutes: true - # This section adds the ability to pass environment variables to adjust CCM defaults # https://github.com/linode/linode-cloud-controller-manager/blob/master/cloud/linode/loadbalancers.go # LINODE_HOSTNAME_ONLY_INGRESS type bool is supported diff --git a/main.go b/main.go index dfcf10bd..f82c9e18 100644 --- a/main.go +++ b/main.go @@ -76,8 +76,6 @@ 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") // Set static flags command.Flags().VisitAll(func(fl *pflag.Flag) { From 1b6fb3baf6aa72f54a820e04c77e1b06fc414486 Mon Sep 17 00:00:00 2001 From: Rahul Sharma Date: Thu, 21 Mar 2024 20:18:52 +0000 Subject: [PATCH 2/2] allow linodego-debug to be set by helm --- deploy/chart/templates/daemonset.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/deploy/chart/templates/daemonset.yaml b/deploy/chart/templates/daemonset.yaml index 86d45dd5..6a38e6e0 100644 --- a/deploy/chart/templates/daemonset.yaml +++ b/deploy/chart/templates/daemonset.yaml @@ -33,6 +33,9 @@ spec: - --v=3 - --port=0 - --secure-port=10253 + {{- if .Values.linodegoDebug }} + - --linodego-debug={{ .Values.linodegoDebug }} + {{- end }} volumeMounts: - mountPath: /etc/kubernetes name: k8s