Merge pull request #195 from linode/revert-route-controller

revert PR 184, PR 190 and PR 182

Author: tchinmai7

README.md

@@ -126,17 +126,6 @@ Users can create CloudFirewall instances, supply their own rules and attach them
 **Note**<br/>
 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: <name of VPC>
-  clusterCIDR: 10.0.0.0/8
-  configureCloudRoutes: true
-```
-
 ### Nodes
 Kubernetes Nodes can be configured with the following annotations.
 

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)

cloud/linode/client/mock_client_test.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
 }
 

cloud/linode/cloud.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,22 +165,20 @@ 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
 	}
 
 	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
-}