Merge pull request #139 from okokes-akamai/empty_nodes

Do not ensure/update LBs if nodes are empty

Author: luthermonson

cloud/linode/loadbalancers.go

@@ -3,6 +3,7 @@ package linode
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"net/http"
 	"os"
@@ -51,6 +52,10 @@ const (
 	annLinodeLoadBalancerTags    = "service.beta.kubernetes.io/linode-loadbalancer-tags"
 )
 
+var (
+	errNoNodesAvailable = errors.New("no nodes available for nodebalancer")
+)
+
 type lbNotFoundError struct {
 	serviceNn      string
 	nodeBalancerID int
@@ -256,6 +261,10 @@ func (l *loadbalancers) EnsureLoadBalancer(ctx context.Context, clusterName stri
 
 //nolint:funlen
 func (l *loadbalancers) updateNodeBalancer(ctx context.Context, service *v1.Service, nodes []*v1.Node, nb *linodego.NodeBalancer) (err error) {
+	if len(nodes) == 0 {
+		return fmt.Errorf("%w: service %s", errNoNodesAvailable, getServiceNn(service))
+	}
+
 	connThrottle := getConnectionThrottle(service)
 	if connThrottle != nb.ClientConnThrottle {
 		update := nb.GetUpdateOptions()
@@ -607,6 +616,9 @@ func (l *loadbalancers) addTLSCert(ctx context.Context, service *v1.Service, nbC
 // buildLoadBalancerRequest returns a linodego.NodeBalancer
 // requests for service across nodes.
 func (l *loadbalancers) buildLoadBalancerRequest(ctx context.Context, clusterName string, service *v1.Service, nodes []*v1.Node) (*linodego.NodeBalancer, error) {
+	if len(nodes) == 0 {
+		return nil, fmt.Errorf("%w: cluster %s, service %s", errNoNodesAvailable, clusterName, getServiceNn(service))
+	}
 	ports := service.Spec.Ports
 	configs := make([]*linodego.NodeBalancerConfigCreateOptions, 0, len(ports))
 

cloud/linode/loadbalancers_test.go

@@ -2,6 +2,7 @@ package linode
 
 import (
 	"context"
+	stderrors "errors"
 	"fmt"
 	"net/http"
 	"net/http/httptest"
@@ -180,6 +181,10 @@ func TestCCMLoadBalancers(t *testing.T) {
 			name: "Cleanup does not call the API unless Service annotated",
 			f:    testCleanupDoesntCall,
 		},
+		{
+			name: "Update Load Balancer - No Nodes",
+			f:    testUpdateLoadBalancerNoNodes,
+		},
 	}
 
 	for _, tc := range testCases {
@@ -229,7 +234,9 @@ func testCreateNodeBalancer(t *testing.T, client *linodego.Client, _ *fakeAPI) {
 	}
 
 	lb := &loadbalancers{client, "us-west", nil}
-	var nodes []*v1.Node
+	nodes := []*v1.Node{
+		{ObjectMeta: metav1.ObjectMeta{Name: "node-1"}},
+	}
 	nb, err := lb.buildLoadBalancerRequest(context.TODO(), "linodelb", svc, nodes)
 	if err != nil {
 		t.Fatal(err)
@@ -1631,6 +1638,55 @@ func testCleanupDoesntCall(t *testing.T, client *linodego.Client, fakeAPI *fakeA
 	})
 }
 
+func testUpdateLoadBalancerNoNodes(t *testing.T, client *linodego.Client, _ *fakeAPI) {
+	svc := &v1.Service{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:        randString(10),
+			UID:         "foobar123",
+			Annotations: map[string]string{},
+		},
+		Spec: v1.ServiceSpec{
+			Ports: []v1.ServicePort{
+				{
+					Name:     randString(10),
+					Protocol: "http",
+					Port:     int32(80),
+					NodePort: int32(8080),
+				},
+			},
+		},
+	}
+
+	lb := &loadbalancers{client, "us-west", nil}
+	defer lb.EnsureLoadBalancerDeleted(context.TODO(), "linodelb", svc)
+
+	fakeClientset := fake.NewSimpleClientset()
+	lb.kubeClient = fakeClientset
+
+	nodeBalancer, err := client.CreateNodeBalancer(context.TODO(), linodego.NodeBalancerCreateOptions{
+		Region: lb.zone,
+	})
+	if err != nil {
+		t.Fatalf("failed to create NodeBalancer: %s", err)
+	}
+	svc.Status.LoadBalancer = *makeLoadBalancerStatus(svc, nodeBalancer)
+	stubService(fakeClientset, svc)
+	svc.ObjectMeta.SetAnnotations(map[string]string{
+		annLinodeNodeBalancerID: strconv.Itoa(nodeBalancer.ID),
+	})
+
+	// setup done, test ensure/update
+	nodes := []*v1.Node{}
+
+	if _, err = lb.EnsureLoadBalancer(context.TODO(), "linodelb", svc, nodes); !stderrors.Is(err, errNoNodesAvailable) {
+		t.Errorf("EnsureLoadBalancer should return %v, got %v", errNoNodesAvailable, err)
+	}
+
+	if err := lb.UpdateLoadBalancer(context.TODO(), "linodelb", svc, nodes); !stderrors.Is(err, errNoNodesAvailable) {
+		t.Errorf("UpdateLoadBalancer should return %v, got %v", errNoNodesAvailable, err)
+	}
+}
+
 func testGetNodeBalancerForServiceIDDoesNotExist(t *testing.T, client *linodego.Client, _ *fakeAPI) {
 	lb := &loadbalancers{client, "us-west", nil}
 	bogusNodeBalancerID := "123456"