Fixing the nodebalancer config rebuilds to include ids of preexisting nodebalancer nodes (#192)

Fixing the nodebalancer config rebuilds to include ids of preexisting nodebalancer nodes to avoid rebuilds.

+ tests
+ Bumping k8s deps and updating CCM to reflect new API
+ Fixing node_controller and service_controller due to changes in k8s api.
+ upgrading toolchain
+ Bumping CI's go version
+ Adding build deps/tools to separate file
+ Refactored client mocks to generate just one file and dropped the two copies which existed due to _test.go in name, that prevents the code to be imported

Author: akaokunc

cloud/linode/client/client.go

@@ -1,6 +1,6 @@
 package client
 
-//go:generate go run github.com/golang/mock/mockgen -destination mock_client_test.go -package client github.com/linode/linode-cloud-controller-manager/cloud/linode/client Client
+//go:generate go run github.com/golang/mock/mockgen -destination mocks/mock_client.go -package mocks github.com/linode/linode-cloud-controller-manager/cloud/linode/client Client
 
 import (
 	"context"
@@ -21,6 +21,7 @@ type Client interface {
 	UpdateNodeBalancer(context.Context, int, linodego.NodeBalancerUpdateOptions) (*linodego.NodeBalancer, error)
 	DeleteNodeBalancer(context.Context, int) error
 	ListNodeBalancers(context.Context, *linodego.ListOptions) ([]linodego.NodeBalancer, error)
+	ListNodeBalancerNodes(context.Context, int, int, *linodego.ListOptions) ([]linodego.NodeBalancerNode, error)
 
 	CreateNodeBalancerConfig(context.Context, int, linodego.NodeBalancerConfigCreateOptions) (*linodego.NodeBalancerConfig, error)
 	DeleteNodeBalancerConfig(context.Context, int, int) error

cloud/linode/client/mock_client_test.go cloud/linode/client/mocks/mock_client.gocloud/linode/client/mock_client_test.go renamed to cloud/linode/client/mocks/mock_client.go

@@ -8,6 +8,7 @@ import (
 	"testing"
 
 	"github.com/golang/mock/gomock"
+	"github.com/linode/linode-cloud-controller-manager/cloud/linode/client/mocks"
 	"github.com/linode/linodego"
 	"github.com/stretchr/testify/assert"
 	v1 "k8s.io/api/core/v1"
@@ -30,7 +31,7 @@ func TestInstanceExists(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()
 
-	client := NewMockClient(ctrl)
+	client := mocks.NewMockClient(ctrl)
 
 	t.Run("should return false if linode does not exist (by providerID)", func(t *testing.T) {
 		instances := newInstances(client)
@@ -90,7 +91,7 @@ func TestMetadataRetrieval(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()
 
-	client := NewMockClient(ctrl)
+	client := mocks.NewMockClient(ctrl)
 
 	t.Run("errors when linode does not exist (by name)", func(t *testing.T) {
 		instances := newInstances(client)
@@ -231,7 +232,7 @@ func TestMalformedProviders(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()
 
-	client := NewMockClient(ctrl)
+	client := mocks.NewMockClient(ctrl)
 
 	t.Run("fails on non-numeric providerID", func(t *testing.T) {
 		instances := newInstances(client)
@@ -250,7 +251,7 @@ func TestInstanceShutdown(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()
 
-	client := NewMockClient(ctrl)
+	client := mocks.NewMockClient(ctrl)
 
 	t.Run("fails when instance not found (by provider)", func(t *testing.T) {
 		instances := newInstances(client)

cloud/linode/instances_test.go

@@ -296,12 +296,6 @@ func (l *loadbalancers) updateNodeBalancer(
 			return err
 		}
 
-		// Add all of the Nodes to the config
-		var newNBNodes []linodego.NodeBalancerNodeCreateOptions
-		for _, node := range nodes {
-			newNBNodes = append(newNBNodes, l.buildNodeBalancerNodeCreateOptions(node, port.NodePort))
-		}
-
 		// Look for an existing config for this port
 		var currentNBCfg *linodego.NodeBalancerConfig
 		for i := range nbCfgs {
@@ -311,6 +305,34 @@ func (l *loadbalancers) updateNodeBalancer(
 				break
 			}
 		}
+		oldNBNodeIDs := make(map[string]int)
+		if currentNBCfg != nil {
+			// Obtain list of current NB nodes and convert it to map of node IDs
+			currentNBNodes, err := l.client.ListNodeBalancerNodes(ctx, nb.ID, currentNBCfg.ID, nil)
+			if err != nil {
+				// This error can be ignored, because if we fail to get nodes we can anyway rebuild the config from scratch,
+				// it would just cause the NB to reload config even if the node list did not change, so we prefer to send IDs when it is posible.
+				klog.Warningf("Unable to list existing nodebalancer nodes for NB %d config %d, error: %s", nb.ID, newNBCfg.ID, err)
+			}
+			for _, node := range currentNBNodes {
+				oldNBNodeIDs[node.Address] = node.ID
+			}
+			klog.Infof("Nodebalancer %d had nodes %v", nb.ID, oldNBNodeIDs)
+		} else {
+			klog.Infof("No preexisting nodebalancer for port %v found.", port.Port)
+		}
+		// Add all of the Nodes to the config
+		newNBNodes := make([]linodego.NodeBalancerConfigRebuildNodeOptions, 0, len(nodes))
+		for _, node := range nodes {
+			newNodeOpts := l.buildNodeBalancerNodeConfigRebuildOptions(node, port.NodePort)
+			oldNodeID, ok := oldNBNodeIDs[newNodeOpts.Address]
+			if ok {
+				newNodeOpts.ID = oldNodeID
+			} else {
+				klog.Infof("No preexisting node id for %v found.", newNodeOpts.Address)
+			}
+			newNBNodes = append(newNBNodes, newNodeOpts)
+		}
 
 		// If there's no existing config, create it
 		var rebuildOpts linodego.NodeBalancerConfigRebuildOptions
@@ -652,7 +674,7 @@ func (l *loadbalancers) buildLoadBalancerRequest(ctx context.Context, clusterNam
 		createOpt := config.GetCreateOptions()
 
 		for _, n := range nodes {
-			createOpt.Nodes = append(createOpt.Nodes, l.buildNodeBalancerNodeCreateOptions(n, port.NodePort))
+			createOpt.Nodes = append(createOpt.Nodes, l.buildNodeBalancerNodeConfigRebuildOptions(n, port.NodePort).NodeBalancerNodeCreateOptions)
 		}
 
 		configs = append(configs, &createOpt)
@@ -672,14 +694,16 @@ func coerceString(s string, minLen, maxLen int, padding string) string {
 	return s
 }
 
-func (l *loadbalancers) buildNodeBalancerNodeCreateOptions(node *v1.Node, nodePort int32) linodego.NodeBalancerNodeCreateOptions {
-	return linodego.NodeBalancerNodeCreateOptions{
-		Address: fmt.Sprintf("%v:%v", getNodePrivateIP(node), nodePort),
-		// NodeBalancer backends must be 3-32 chars in length
-		// If < 3 chars, pad node name with "node-" prefix
-		Label:  coerceString(node.Name, 3, 32, "node-"),
-		Mode:   "accept",
-		Weight: 100,
+func (l *loadbalancers) buildNodeBalancerNodeConfigRebuildOptions(node *v1.Node, nodePort int32) linodego.NodeBalancerConfigRebuildNodeOptions {
+	return linodego.NodeBalancerConfigRebuildNodeOptions{
+		NodeBalancerNodeCreateOptions: linodego.NodeBalancerNodeCreateOptions{
+			Address: fmt.Sprintf("%v:%v", getNodePrivateIP(node), nodePort),
+			// NodeBalancer backends must be 3-32 chars in length
+			// If < 3 chars, pad node name with "node-" prefix
+			Label:  coerceString(node.Name, 3, 32, "node-"),
+			Mode:   "accept",
+			Weight: 100,
+		},
 	}
 }