feat: add support for load balancer, listener, and pool tags annotations

docs/openstack-cloud-controller-manager/expose-applications-using-loadbalancer-type-service.md

@@ -239,6 +239,18 @@ Request Body:
 
   If this annotation is specified, the other annotations which define the load balancer features will be ignored.
 
+- `loadbalancer.openstack.org/load-balancer-tags`
+
+  A comma-separated list of tags to add to the load balancer resource in addition to the tags managed by OCCM. Example: `env=prod,team=network`.
+
+- `loadbalancer.openstack.org/listener-tags`
+
+  A comma-separated list of tags to add to the load balancer listener resources in addition to the tags managed by OCCM. Example: `env=prod,team=network`.
+
+- `loadbalancer.openstack.org/pool-tags`
+
+  A comma-separated list of tags to add to the load balancer pool resources. Example: `env=prod,team=network`.
+
 - `loadbalancer.openstack.org/hostname`
 
   This annotations explicitly sets a hostname in the status of the load balancer service.

pkg/openstack/loadbalancer.go

@@ -96,6 +96,13 @@ const (
 	defaultProxyHostnameSuffix      = "nip.io"
 	ServiceAnnotationLoadBalancerID = "loadbalancer.openstack.org/load-balancer-id"
 
+	// ServiceAnnotationLoadBalancerTags is used to set additional tags on the loadbalancer resource itself (comma-separated list).
+	ServiceAnnotationLoadBalancerTags = "loadbalancer.openstack.org/load-balancer-tags"
+	// ServiceAnnotationListenerTags is used to set additional tags on the loadbalancer listener resources (comma-separated list).
+	ServiceAnnotationListenerTags = "loadbalancer.openstack.org/listener-tags"
+	// ServiceAnnotationPoolTags is used to set additional tags on the loadbalancer pool resources (comma-separated list).
+	ServiceAnnotationPoolTags = "loadbalancer.openstack.org/pool-tags"
+
 	// Octavia resources name formats
 	servicePrefix  = "kube_service_"
 	lbFormat       = "%s%s_%s_%s"
@@ -146,6 +153,9 @@ type serviceConfig struct {
 	healthMonitorMaxRetries     int
 	healthMonitorMaxRetriesDown int
 	preferredIPFamily           corev1.IPFamily // preferred (the first) IP family indicated in service's `spec.ipFamilies`
+	lbTags                      string
+	listenerTags                string
+	poolTags                    string
 }
 
 type listenerKey struct {
@@ -197,6 +207,22 @@ func getLoadbalancerByName(ctx context.Context, client *gophercloud.ServiceClien
 	return &validLBs[0], nil
 }
 
+// withLBNameTag returns the LB name (OCCM's ownership tag) followed by the
+// comma-separated tags from the given Service annotation value.
+func withLBNameTag(lbName, annotation string) []string {
+	return append([]string{lbName}, cpoutil.SplitTrim(annotation, ',')...)
+}
+
+// mergeTags merges existedTags and newTags, returns true if all newTags are already in existedTags.
+func mergeTags(existedTags []string, newTags []string) (bool, []string) {
+	tagSet := sets.NewString(existedTags...)
+	if tagSet.HasAll(newTags...) {
+		return true, nil
+	}
+
+	return false, tagSet.Union(sets.NewString(newTags...)).List()
+}
+
 func popListener(existingListeners []listeners.Listener, id string) []listeners.Listener {
 	newListeners := []listeners.Listener{}
 	for _, existingListener := range existingListeners {
@@ -235,7 +261,7 @@ func (lbaas *LbaasV2) createOctaviaLoadBalancer(ctx context.Context, name, clust
 	}
 
 	if svcConf.supportLBTags {
-		createOpts.Tags = []string{svcConf.lbName}
+		createOpts.Tags = withLBNameTag(svcConf.lbName, svcConf.lbTags)
 	}
 
 	if svcConf.flavorID != "" {
@@ -910,24 +936,39 @@ func (lbaas *LbaasV2) ensureOctaviaPool(ctx context.Context, lbID string, name s
 		// if LBMethod is not defined, fallback on default OCCM's default method
 		poolLbMethod = lbaas.opts.LBMethod
 	}
-	if pool != nil && pool.LBMethod != poolLbMethod {
-		klog.InfoS("Updating LoadBalancer LBMethod", "poolID", pool.ID, "listenerID", listener.ID, "lbID", lbID)
-		err = openstackutil.UpdatePool(ctx, lbaas.lb, lbID, pool.ID, v2pools.UpdateOpts{LBMethod: v2pools.LBMethod(poolLbMethod)})
-		if err != nil {
-			err = PreserveGopherError(err)
-			msg := fmt.Sprintf("Error updating LB method for LoadBalancer: %v", err)
-			klog.Errorf(msg, "poolID", pool.ID, "listenerID", listener.ID, "lbID", lbID)
-			lbaas.eventRecorder.Event(service, corev1.EventTypeWarning, eventLBLbMethodUnknown, msg)
-		} else {
-			pool.LBMethod = poolLbMethod
+	poolTags := cpoutil.SplitTrim(svcConf.poolTags, ',')
+
+	if pool != nil {
+		updateOpts := v2pools.UpdateOpts{}
+		if pool.LBMethod != poolLbMethod {
+			updateOpts.LBMethod = v2pools.LBMethod(poolLbMethod)
+		}
+		if svcConf.supportLBTags && len(poolTags) > 0 {
+			klog.V(4).Infof("Desired pool tags: %+v from service annotation key: %s", poolTags, ServiceAnnotationPoolTags)
+			if ok, tags := mergeTags(pool.Tags, poolTags); !ok {
+				updateOpts.Tags = &tags
+			}
+		}
+		if updateOpts != (v2pools.UpdateOpts{}) {
+			klog.InfoS("Updating pool", "poolID", pool.ID, "listenerID", listener.ID, "lbID", lbID, "lbMethod", updateOpts.LBMethod, "tags", updateOpts.Tags)
+			err = openstackutil.UpdatePool(ctx, lbaas.lb, lbID, pool.ID, updateOpts)
+			if err != nil {
+				err = PreserveGopherError(err)
+				msg := fmt.Sprintf("Error updating pool for LoadBalancer: %v", err)
+				klog.Errorf(msg, "poolID", pool.ID, "listenerID", listener.ID, "lbID", lbID)
+				lbaas.eventRecorder.Event(service, corev1.EventTypeWarning, eventLBLbMethodUnknown, msg)
+			}
 		}
 	}
 
 	if pool == nil {
 		createOpt := lbaas.buildPoolCreateOpt(listener.Protocol, service, svcConf, name)
 		createOpt.ListenerID = listener.ID
-
+		if svcConf.supportLBTags {
+			createOpt.Tags = poolTags
+		}
 		klog.InfoS("Creating pool", "listenerID", listener.ID, "protocol", createOpt.Protocol)
+		klog.V(4).Infof("Pool create options: %+v", createOpt)
 		pool, err = openstackutil.CreatePool(ctx, lbaas.lb, createOpt, lbID)
 		if err != nil {
 			return nil, err
@@ -1099,11 +1140,11 @@ func (lbaas *LbaasV2) ensureOctaviaListener(ctx context.Context, lbID string, na
 		updateOpts := listeners.UpdateOpts{}
 
 		if svcConf.supportLBTags {
-			if !slices.Contains(listener.Tags, svcConf.lbName) {
-				var newTags []string
-				copy(newTags, listener.Tags)
-				newTags = append(newTags, svcConf.lbName)
-				updateOpts.Tags = &newTags
+			// Ensure the LB name tag plus any desired tags from the Service annotation.
+			listenerTags := withLBNameTag(svcConf.lbName, svcConf.listenerTags)
+			if ok, tags := mergeTags(listener.Tags, listenerTags); !ok {
+				klog.V(4).Infof("Will update listener tags, current listener tags: %+v, desired tags: %+v", listener.Tags, tags)
+				updateOpts.Tags = &tags
 				listenerChanged = true
 			}
 		}
@@ -1177,7 +1218,8 @@ func (lbaas *LbaasV2) buildListenerCreateOpt(ctx context.Context, port corev1.Se
 	}
 
 	if svcConf.supportLBTags {
-		listenerCreateOpt.Tags = []string{svcConf.lbName}
+		// The LB name is always tagged, plus any desired tags from the Service annotation.
+		listenerCreateOpt.Tags = withLBNameTag(svcConf.lbName, svcConf.listenerTags)
 	}
 
 	if openstackutil.IsOctaviaFeatureSupported(ctx, lbaas.lb, openstackutil.OctaviaFeatureTimeout, lbaas.opts.LBProvider) {
@@ -1365,6 +1407,11 @@ func (lbaas *LbaasV2) checkService(ctx context.Context, service *corev1.Service,
 		return fmt.Errorf("no service ports provided")
 	}
 
+	annotations := service.GetAnnotations()
+	svcConf.lbTags = annotations[ServiceAnnotationLoadBalancerTags]
+	svcConf.listenerTags = annotations[ServiceAnnotationListenerTags]
+	svcConf.poolTags = annotations[ServiceAnnotationPoolTags]
+
 	if len(service.Spec.IPFamilies) > 0 {
 		// Since OCCM does not support multiple load-balancers per service yet,
 		// the first IP family will determine the IP family of the load-balancer
@@ -1826,15 +1873,16 @@ func (lbaas *LbaasV2) ensureOctaviaLoadBalancer(ctx context.Context, clusterName
 	// save address into the annotation
 	lbaas.updateServiceAnnotation(service, ServiceAnnotationLoadBalancerAddress, addr)
 
-	// add LB name to load balancer tags.
+	// Ensure the LB name tag plus any tags from the Service annotation in a single update.
 	if svcConf.supportLBTags {
-		lbTags := loadbalancer.Tags
-		if !slices.Contains(lbTags, lbName) {
-			lbTags = append(lbTags, lbName)
-			klog.InfoS("Updating load balancer tags", "lbID", loadbalancer.ID, "tags", lbTags)
-			if err := openstackutil.UpdateLoadBalancerTags(ctx, lbaas.lb, loadbalancer.ID, lbTags); err != nil {
-				return nil, err
+		lbTags := withLBNameTag(lbName, svcConf.lbTags)
+		klog.V(4).Infof("Desired load balancer tags: %v (LB name plus annotation %s)", lbTags, ServiceAnnotationLoadBalancerTags)
+		if ok, tags := mergeTags(loadbalancer.Tags, lbTags); !ok {
+			klog.InfoS("Updating load balancer tags", "lbID", loadbalancer.ID, "tags", tags)
+			if err := openstackutil.UpdateLoadBalancerTags(ctx, lbaas.lb, loadbalancer.ID, tags); err != nil {
+				return nil, fmt.Errorf("failed to update load balancer %s tags: %w", loadbalancer.ID, err)
 			}
+			loadbalancer.Tags = tags
 		}
 	}
 

pkg/openstack/loadbalancer_test.go

@@ -152,6 +152,67 @@ type testGetRulesToCreateAndDelete struct {
 	toDelete      []rules.SecGroupRule
 }
 
+func TestMergeTags(t *testing.T) {
+	testCases := []struct {
+		name         string
+		existedTags  []string
+		newTags      []string
+		expectedOK   bool
+		expectedTags []string
+	}{
+		{
+			name:         "nil existing tags returns desired tags",
+			existedTags:  nil,
+			newTags:      []string{"a", "b"},
+			expectedOK:   false,
+			expectedTags: []string{"a", "b"},
+		},
+		{
+			name:         "empty existing tags returns desired tags",
+			existedTags:  []string{},
+			newTags:      []string{"a"},
+			expectedOK:   false,
+			expectedTags: []string{"a"},
+		},
+		{
+			name:         "all desired tags already present",
+			existedTags:  []string{"a", "b", "c"},
+			newTags:      []string{"a", "b"},
+			expectedOK:   true,
+			expectedTags: nil,
+		},
+		{
+			name:         "some desired tags missing are merged and sorted",
+			existedTags:  []string{"b", "d"},
+			newTags:      []string{"a", "c"},
+			expectedOK:   false,
+			expectedTags: []string{"a", "b", "c", "d"},
+		},
+		{
+			name:         "empty desired tags with existing tags is a no-op",
+			existedTags:  []string{"a"},
+			newTags:      []string{},
+			expectedOK:   true,
+			expectedTags: nil,
+		},
+		{
+			name:         "empty existing and empty desired tags is a no-op",
+			existedTags:  []string{},
+			newTags:      []string{},
+			expectedOK:   true,
+			expectedTags: nil,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			ok, tags := mergeTags(tc.existedTags, tc.newTags)
+			assert.Equal(t, tc.expectedOK, ok)
+			assert.Equal(t, tc.expectedTags, tags)
+		})
+	}
+}
+
 func TestGetRulesToCreateAndDelete(t *testing.T) {
 	tests := []testGetRulesToCreateAndDelete{
 		{
@@ -2485,6 +2546,45 @@ func TestBuildListenerCreateOpt(t *testing.T) {
 				Tags:          nil,
 			},
 		},
+		{
+			name: "Test with LB tags support and no listener-tags annotation",
+			port: corev1.ServicePort{
+				Protocol: "TCP",
+				Port:     80,
+			},
+			svcConf: &serviceConfig{
+				connLimit:     100,
+				lbName:        "my-lb",
+				supportLBTags: true,
+			},
+			expectedCreateOpt: listeners.CreateOpts{
+				Name:         "Test with LB tags support and no listener-tags annotation",
+				Protocol:     listeners.ProtocolTCP,
+				ProtocolPort: 80,
+				ConnLimit:    &svcConf.connLimit,
+				Tags:         []string{"my-lb"},
+			},
+		},
+		{
+			name: "Test with LB tags support and listener-tags annotation",
+			port: corev1.ServicePort{
+				Protocol: "TCP",
+				Port:     80,
+			},
+			svcConf: &serviceConfig{
+				connLimit:     100,
+				lbName:        "my-lb",
+				supportLBTags: true,
+				listenerTags:  "foo, bar",
+			},
+			expectedCreateOpt: listeners.CreateOpts{
+				Name:         "Test with LB tags support and listener-tags annotation",
+				Protocol:     listeners.ProtocolTCP,
+				ProtocolPort: 80,
+				ConnLimit:    &svcConf.connLimit,
+				Tags:         []string{"my-lb", "foo", "bar"},
+			},
+		},
 	}
 
 	for _, tc := range testCases {