Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not ensure/update LBs if nodes are empty #139

Merged
merged 2 commits into from
Nov 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions cloud/linode/loadbalancers.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package linode
import (
"context"
"encoding/json"
"errors"
"fmt"
"net/http"
"os"
Expand Down Expand Up @@ -49,6 +50,10 @@ const (
annLinodeHostnameOnlyIngress = "service.beta.kubernetes.io/linode-loadbalancer-hostname-only-ingress"
)

var (
errNoNodesAvailable = errors.New("no nodes available for nodebalancer")
)

type lbNotFoundError struct {
serviceNn string
nodeBalancerID int
Expand Down Expand Up @@ -254,6 +259,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()
Expand Down Expand Up @@ -584,6 +593,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))

Expand Down
58 changes: 57 additions & 1 deletion 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"
luthermonson marked this conversation as resolved.
Show resolved Hide resolved
"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 @@ -223,7 +228,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)
Expand Down Expand Up @@ -1550,6 +1557,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
Loading