Skip to content

Commit

Permalink
Do not ensure/update LBs if nodes are empty
Browse files Browse the repository at this point in the history
  • Loading branch information
okokes-akamai committed Nov 8, 2023
1 parent aec305f commit 7da7ae9
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 0 deletions.
10 changes: 10 additions & 0 deletions cloud/linode/loadbalancers.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ const (
annLinodeHostnameOnlyIngress = "service.beta.kubernetes.io/linode-loadbalancer-hostname-only-ingress"
)

var (
errNoNodesAvailable = fmt.Errorf("no nodes available for nodebalancer")
)

type lbNotFoundError struct {
serviceNn string
nodeBalancerID int
Expand Down Expand Up @@ -212,6 +216,9 @@ func (l *loadbalancers) GetLoadBalancer(ctx context.Context, clusterName string,
//
// EnsureLoadBalancer will not modify service or nodes.
func (l *loadbalancers) EnsureLoadBalancer(ctx context.Context, clusterName string, service *v1.Service, nodes []*v1.Node) (lbStatus *v1.LoadBalancerStatus, err error) {
if len(nodes) == 0 {
return nil, fmt.Errorf("%w: cluster %s, service %s", errNoNodesAvailable, clusterName, getServiceNn(service))
}
ctx = sentry.SetHubOnContext(ctx)
sentry.SetTag(ctx, "cluster_name", clusterName)
sentry.SetTag(ctx, "service", service.Name)
Expand Down Expand Up @@ -342,6 +349,9 @@ func (l *loadbalancers) updateNodeBalancer(ctx context.Context, service *v1.Serv

// UpdateLoadBalancer updates the NodeBalancer to have configs that match the Service's ports
func (l *loadbalancers) UpdateLoadBalancer(ctx context.Context, clusterName string, service *v1.Service, nodes []*v1.Node) (err error) {
if len(nodes) == 0 {
return fmt.Errorf("%w: cluster %s, service %s", errNoNodesAvailable, clusterName, getServiceNn(service))
}
ctx = sentry.SetHubOnContext(ctx)
sentry.SetTag(ctx, "cluster_name", clusterName)
sentry.SetTag(ctx, "service", service.Name)
Expand Down
54 changes: 54 additions & 0 deletions cloud/linode/loadbalancers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package linode

import (
"context"
stderrors "errors"
"fmt"
"net/http"
"net/http/httptest"
Expand Down Expand Up @@ -175,6 +176,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 {
Expand Down Expand Up @@ -1550,6 +1555,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"
Expand Down

0 comments on commit 7da7ae9

Please sign in to comment.