From f512c956b65e667f71a7348878bbde1b3ec92e15 Mon Sep 17 00:00:00 2001 From: Tarun Chinmai Sekar Date: Mon, 6 Nov 2023 08:53:38 -0800 Subject: [PATCH] Support adding tags to nodebalancers --- README.md | 1 + cloud/linode/fake_linode_test.go | 4 ++ cloud/linode/loadbalancers.go | 23 +++++++++ cloud/linode/loadbalancers_test.go | 83 +++++++++++++++++++++++++++++- 4 files changed, 110 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 5c296277..118d75f3 100644 --- a/README.md +++ b/README.md @@ -56,6 +56,7 @@ Annotation (Suffix) | Values | Default | Description `preserve` | [bool](#annotation-bool-values) | `false` | When `true`, deleting a `LoadBalancer` service does not delete the underlying NodeBalancer. This will also prevent deletion of the former LoadBalancer when another one is specified with the `nodebalancer-id` annotation. `nodebalancer-id` | string | | The ID of the NodeBalancer to front the service. When not specified, a new NodeBalancer will be created. This can be configured on service creation or patching `hostname-only-ingress` | [bool](#annotation-bool-values) | `false` | When `true`, the LoadBalancerStatus for the service will only contain the Hostname. This is useful for bypassing kube-proxy's rerouting of in-cluster requests originally intended for the external LoadBalancer to the service's constituent pod IPs. +`tags` | string | | A comma seperated list of tags to be applied to the createad NodeBalancer instance #### Deprecated Annotations diff --git a/cloud/linode/fake_linode_test.go b/cloud/linode/fake_linode_test.go index cf3e7d4c..59b326e8 100644 --- a/cloud/linode/fake_linode_test.go +++ b/cloud/linode/fake_linode_test.go @@ -251,6 +251,7 @@ func (f *fakeAPI) ServeHTTP(w http.ResponseWriter, r *http.Request) { Region: nbco.Region, IPv4: &ip, Hostname: &hostname, + Tags: nbco.Tags, } if nbco.ClientConnThrottle != nil { @@ -561,6 +562,9 @@ func (f *fakeAPI) ServeHTTP(w http.ResponseWriter, r *http.Request) { if nbuo.Label != nil { nb.Label = nbuo.Label } + if nbuo.Tags != nil { + nb.Tags = *nbuo.Tags + } f.nb[strconv.Itoa(nb.ID)] = nb resp, err := json.Marshal(nb) diff --git a/cloud/linode/loadbalancers.go b/cloud/linode/loadbalancers.go index b3e4cda2..6248796e 100644 --- a/cloud/linode/loadbalancers.go +++ b/cloud/linode/loadbalancers.go @@ -6,6 +6,7 @@ import ( "fmt" "net/http" "os" + "reflect" "strconv" "strings" "time" @@ -47,6 +48,7 @@ const ( annLinodeNodeBalancerID = "service.beta.kubernetes.io/linode-loadbalancer-nodebalancer-id" annLinodeHostnameOnlyIngress = "service.beta.kubernetes.io/linode-loadbalancer-hostname-only-ingress" + annLinodeLoadBalancerTags = "service.beta.kubernetes.io/linode-loadbalancer-tags" ) type lbNotFoundError struct { @@ -266,6 +268,17 @@ func (l *loadbalancers) updateNodeBalancer(ctx context.Context, service *v1.Serv } } + tags := l.getLoadbalancerTags(ctx, service) + if !reflect.DeepEqual(nb.Tags, tags) { + update := nb.GetUpdateOptions() + update.Tags = &tags + nb, err = l.client.UpdateNodeBalancer(ctx, nb.ID, update) + if err != nil { + sentry.CaptureError(ctx, err) + return err + } + } + // Get all of the NodeBalancer's configs nbCfgs, err := l.client.ListNodeBalancerConfigs(ctx, nb.ID, nil) if err != nil { @@ -480,15 +493,25 @@ func (l *loadbalancers) getNodeBalancerByID(ctx context.Context, service *v1.Ser return nb, nil } +func (l *loadbalancers) getLoadbalancerTags(ctx context.Context, service *v1.Service) []string { + tagStr, ok := getServiceAnnotation(service, annLinodeLoadBalancerTags) + if ok { + return strings.Split(tagStr, ",") + } + return []string{} +} + func (l *loadbalancers) createNodeBalancer(ctx context.Context, clusterName string, service *v1.Service, configs []*linodego.NodeBalancerConfigCreateOptions) (lb *linodego.NodeBalancer, err error) { connThrottle := getConnectionThrottle(service) label := l.GetLoadBalancerName(ctx, clusterName, service) + tags := l.getLoadbalancerTags(ctx, service) createOpts := linodego.NodeBalancerCreateOptions{ Label: &label, Region: l.zone, ClientConnThrottle: &connThrottle, Configs: configs, + Tags: tags, } return l.client.CreateNodeBalancer(ctx, createOpts) } diff --git a/cloud/linode/loadbalancers_test.go b/cloud/linode/loadbalancers_test.go index f4ad2c6f..a15b9160 100644 --- a/cloud/linode/loadbalancers_test.go +++ b/cloud/linode/loadbalancers_test.go @@ -8,6 +8,7 @@ import ( "os" "reflect" "strconv" + "strings" "testing" "github.com/linode/linodego" @@ -127,6 +128,10 @@ func TestCCMLoadBalancers(t *testing.T) { name: "Update Load Balancer - Add TLS Port", f: testUpdateLoadBalancerAddTLSPort, }, + { + name: "Update Load Balancer - Add Tags", + f: testUpdateLoadBalancerAddTags, + }, { name: "Update Load Balancer - Specify NodeBalancerID", f: testUpdateLoadBalancerAddNodeBalancerID, @@ -201,7 +206,8 @@ func testCreateNodeBalancer(t *testing.T, client *linodego.Client, _ *fakeAPI) { Name: randString(10), UID: "foobar123", Annotations: map[string]string{ - annLinodeThrottle: "15", + annLinodeThrottle: "15", + annLinodeLoadBalancerTags: "fake,test,yolo", }, }, Spec: v1.ServiceSpec{ @@ -264,6 +270,13 @@ func testCreateNodeBalancer(t *testing.T, client *linodego.Client, _ *fakeAPI) { t.Logf("actual: %v", nb.ClientConnThrottle) } + expectedTags := []string{"fake", "test", "yolo"} + if !reflect.DeepEqual(nb.Tags, expectedTags) { + t.Error("unexpected Tags") + t.Logf("expected: %v", expectedTags) + t.Logf("actual: %v", nb.Tags) + } + defer func() { _ = lb.EnsureLoadBalancerDeleted(context.TODO(), "linodelb", svc) }() } @@ -415,6 +428,74 @@ func testUpdateLoadBalancerAddPortAnnotation(t *testing.T, client *linodego.Clie } } +func testUpdateLoadBalancerAddTags(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: "TCP", + Port: int32(80), + NodePort: int32(30000), + }, + }, + }, + } + + nodes := []*v1.Node{ + &v1.Node{ + Status: v1.NodeStatus{ + Addresses: []v1.NodeAddress{ + { + Type: v1.NodeInternalIP, + Address: "127.0.0.1", + }, + }, + }, + }, + } + + lb := &loadbalancers{client, "us-west", nil} + fakeClientset := fake.NewSimpleClientset() + lb.kubeClient = fakeClientset + + defer lb.EnsureLoadBalancerDeleted(context.TODO(), "linodelb", svc) + + lbStatus, err := lb.EnsureLoadBalancer(context.TODO(), "linodelb", svc, nodes) + if err != nil { + t.Errorf("EnsureLoadBalancer returned an error: %s", err) + } + svc.Status.LoadBalancer = *lbStatus + stubService(fakeClientset, svc) + + testTags := "test,new,tags" + svc.ObjectMeta.SetAnnotations(map[string]string{ + annLinodeLoadBalancerTags: testTags, + }) + + err = lb.UpdateLoadBalancer(context.TODO(), "linodelb", svc, nodes) + if err != nil { + t.Fatalf("UpdateLoadBalancer returned an error while updated annotations: %s", err) + } + + nb, err := lb.getNodeBalancerByStatus(context.TODO(), svc) + if err != nil { + t.Fatalf("failed to get NodeBalancer by status: %v", err) + } + + expectedTags := strings.Split(testTags, ",") + observedTags := nb.Tags + + if !reflect.DeepEqual(expectedTags, observedTags) { + t.Errorf("NodeBalancer tags mismatch: expected %v, got %v", expectedTags, observedTags) + } +} + func testUpdateLoadBalancerAddTLSPort(t *testing.T, client *linodego.Client, _ *fakeAPI) { svc := &v1.Service{ ObjectMeta: metav1.ObjectMeta{