Merge pull request vmware-tanzu#482 from zhengxiexie/subnettags_main

Limit subnet/subnetset tags to 26

pkg/controllers/subnet/subnet_controller.go

@@ -68,7 +68,7 @@ func (r *SubnetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 			controllerutil.AddFinalizer(obj, servicecommon.SubnetFinalizerName)
 			if err := r.Client.Update(ctx, obj); err != nil {
 				log.Error(err, "add finalizer", "subnet", req.NamespacedName)
-				updateFail(r, &ctx, obj)
+				updateFail(r, &ctx, obj, "")
 				return ResultRequeue, err
 			}
 			log.V(1).Info("added finalizer on subnet CR", "subnet", req.NamespacedName)
@@ -78,7 +78,7 @@ func (r *SubnetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 			if vpcNetworkConfig == nil {
 				err := fmt.Errorf("operate failed: cannot get configuration for Subnet CR")
 				log.Error(nil, "failed to find VPCNetworkConfig for Subnet CR", "subnet", req.NamespacedName, "namespace %s", obj.Namespace)
-				updateFail(r, &ctx, obj)
+				updateFail(r, &ctx, obj, "")
 				return ResultRequeue, err
 			}
 			if obj.Spec.AccessMode == "" {
@@ -95,7 +95,7 @@ func (r *SubnetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 		}
 		if err := r.Client.Get(context.Background(), namespacedName, namespace); err != nil {
 			log.Error(err, "unable to fetch namespace of Subnet CR", "req", req.NamespacedName)
-			updateFail(r, &ctx, obj)
+			updateFail(r, &ctx, obj, "")
 			return ResultRequeue, err
 		}
 		tags := r.Service.GenerateSubnetNSTags(obj, string(nsObj.UID))
@@ -107,13 +107,18 @@ func (r *SubnetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 			return ResultRequeueAfter10sec, nil
 		}
 		if _, err := r.Service.CreateOrUpdateSubnet(obj, *vpcInfo, tags); err != nil {
+			if errors.As(err, &util.ExceedTagsError{}) {
+				log.Error(err, "exceed tags limit, would not retry", "subnet", req.NamespacedName)
+				updateFail(r, &ctx, obj, err.Error())
+				return ResultNormal, nil
+			}
 			log.Error(err, "operate failed, would retry exponentially", "subnet", req.NamespacedName)
-			updateFail(r, &ctx, obj)
+			updateFail(r, &ctx, obj, "")
 			return ResultRequeue, err
 		}
 		if err := r.updateSubnetStatus(obj); err != nil {
 			log.Error(err, "update subnet status failed, would retry exponentially", "subnet", req.NamespacedName)
-			updateFail(r, &ctx, obj)
+			updateFail(r, &ctx, obj, "")
 			return ResultRequeue, err
 		}
 		updateSuccess(r, &ctx, obj)
@@ -122,13 +127,13 @@ func (r *SubnetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
 			metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerDeleteTotal, MetricResTypeSubnet)
 			if err := r.DeleteSubnet(*obj); err != nil {
 				log.Error(err, "deletion failed, would retry exponentially", "subnet", req.NamespacedName)
-				deleteFail(r, &ctx, obj)
+				deleteFail(r, &ctx, obj, "")
 				return ResultRequeue, err
 			}
 			controllerutil.RemoveFinalizer(obj, servicecommon.SubnetFinalizerName)
 			if err := r.Client.Update(ctx, obj); err != nil {
 				log.Error(err, "deletion failed, would retry exponentially", "subnet", req.NamespacedName)
-				deleteFail(r, &ctx, obj)
+				deleteFail(r, &ctx, obj, "")
 				return ResultRequeue, err
 			}
 			log.V(1).Info("removed finalizer", "subnet", req.NamespacedName)
@@ -184,7 +189,7 @@ func (r *SubnetReconciler) setSubnetReadyStatusTrue(ctx *context.Context, subnet
 	r.updateSubnetStatusConditions(ctx, subnet, newConditions)
 }
 
-func (r *SubnetReconciler) setSubnetReadyStatusFalse(ctx *context.Context, subnet *v1alpha1.Subnet) {
+func (r *SubnetReconciler) setSubnetReadyStatusFalse(ctx *context.Context, subnet *v1alpha1.Subnet, msg string) {
 	newConditions := []v1alpha1.Condition{
 		{
 			Type:    v1alpha1.Ready,
@@ -193,6 +198,9 @@ func (r *SubnetReconciler) setSubnetReadyStatusFalse(ctx *context.Context, subne
 			Reason:  "SubnetNotReady",
 		},
 	}
+	if msg != "" {
+		newConditions[0].Message = msg
+	}
 	r.updateSubnetStatusConditions(ctx, subnet, newConditions)
 }
 
@@ -237,13 +245,13 @@ func getExistingConditionOfType(conditionType v1alpha1.ConditionType, existingCo
 	return nil
 }
 
-func updateFail(r *SubnetReconciler, c *context.Context, o *v1alpha1.Subnet) {
-	r.setSubnetReadyStatusFalse(c, o)
+func updateFail(r *SubnetReconciler, c *context.Context, o *v1alpha1.Subnet, m string) {
+	r.setSubnetReadyStatusFalse(c, o, m)
 	metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerUpdateFailTotal, MetricResTypeSubnet)
 }
 
-func deleteFail(r *SubnetReconciler, c *context.Context, o *v1alpha1.Subnet) {
-	r.setSubnetReadyStatusFalse(c, o)
+func deleteFail(r *SubnetReconciler, c *context.Context, o *v1alpha1.Subnet, m string) {
+	r.setSubnetReadyStatusFalse(c, o, m)
 	metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerDeleteFailTotal, MetricResTypeSubnet)
 }
 

pkg/controllers/subnetset/subnetset_controller.go

@@ -60,7 +60,7 @@ func (r *SubnetSetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 				if vpcNetworkConfig == nil {
 					err := fmt.Errorf("failed to find VPCNetworkConfig for namespace %s", obj.Namespace)
 					log.Error(err, "operate failed, would retry exponentially", "subnet", req.NamespacedName)
-					updateFail(r, &ctx, obj)
+					updateFail(r, &ctx, obj, "")
 					return ResultRequeue, err
 				}
 				if obj.Spec.AccessMode == "" {
@@ -72,7 +72,7 @@ func (r *SubnetSetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 			}
 			if err := r.Client.Update(ctx, obj); err != nil {
 				log.Error(err, "add finalizer", "subnetset", req.NamespacedName)
-				updateFail(r, &ctx, obj)
+				updateFail(r, &ctx, obj, "")
 				return ResultRequeue, err
 			}
 			log.V(1).Info("added finalizer on subnetset CR", "subnetset", req.NamespacedName)
@@ -91,6 +91,13 @@ func (r *SubnetSetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 			for k, v := range nsObj.Labels {
 				tags = append(tags, model.Tag{Scope: servicecommon.String(k), Tag: servicecommon.String(v)})
 			}
+			// tags cannot exceed maximum size 26
+			if len(tags) > servicecommon.TagsCountMax {
+				errorMsg := fmt.Sprintf("tags cannot exceed maximum size 26, tags length: %d", len(tags))
+				log.Error(nil, "exceed tags limit, would not retry", "subnet", req.NamespacedName)
+				updateFail(r, &ctx, obj, errorMsg)
+				return ResultNormal, nil
+			}
 			if err := r.Service.UpdateSubnetSetTags(obj.Namespace, nsxSubnets, tags); err != nil {
 				log.Error(err, "failed to update subnetset tags")
 			}
@@ -101,13 +108,13 @@ func (r *SubnetSetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 			metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerDeleteTotal, MetricResTypeSubnetSet)
 			if err := r.DeleteSubnetForSubnetSet(*obj, false); err != nil {
 				log.Error(err, "deletion failed, would retry exponentially", "subnetset", req.NamespacedName)
-				deleteFail(r, &ctx, obj)
+				deleteFail(r, &ctx, obj, "")
 				return ResultRequeue, err
 			}
 			controllerutil.RemoveFinalizer(obj, servicecommon.SubnetSetFinalizerName)
 			if err := r.Client.Update(ctx, obj); err != nil {
 				log.Error(err, "deletion failed, would retry exponentially", "subnetset", req.NamespacedName)
-				deleteFail(r, &ctx, obj)
+				deleteFail(r, &ctx, obj, "")
 				return ResultRequeue, err
 			}
 			log.V(1).Info("removed finalizer", "subnetset", req.NamespacedName)
@@ -119,13 +126,13 @@ func (r *SubnetSetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 	return ctrl.Result{}, nil
 }
 
-func updateFail(r *SubnetSetReconciler, c *context.Context, o *v1alpha1.SubnetSet) {
-	r.setSubnetSetReadyStatusFalse(c, o)
+func updateFail(r *SubnetSetReconciler, c *context.Context, o *v1alpha1.SubnetSet, m string) {
+	r.setSubnetSetReadyStatusFalse(c, o, m)
 	metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerUpdateFailTotal, MetricResTypeSubnetSet)
 }
 
-func deleteFail(r *SubnetSetReconciler, c *context.Context, o *v1alpha1.SubnetSet) {
-	r.setSubnetSetReadyStatusFalse(c, o)
+func deleteFail(r *SubnetSetReconciler, c *context.Context, o *v1alpha1.SubnetSet, m string) {
+	r.setSubnetSetReadyStatusFalse(c, o, m)
 	metrics.CounterInc(r.Service.NSXConfig, metrics.ControllerDeleteFailTotal, MetricResTypeSubnetSet)
 }
 
@@ -150,7 +157,7 @@ func (r *SubnetSetReconciler) setSubnetSetReadyStatusTrue(ctx *context.Context,
 	r.updateSubnetSetStatusConditions(ctx, subnetset, newConditions)
 }
 
-func (r *SubnetSetReconciler) setSubnetSetReadyStatusFalse(ctx *context.Context, subnetset *v1alpha1.SubnetSet) {
+func (r *SubnetSetReconciler) setSubnetSetReadyStatusFalse(ctx *context.Context, subnetset *v1alpha1.SubnetSet, m string) {
 	newConditions := []v1alpha1.Condition{
 		{
 			Type:    v1alpha1.Ready,
@@ -159,6 +166,9 @@ func (r *SubnetSetReconciler) setSubnetSetReadyStatusFalse(ctx *context.Context,
 			Reason:  "SubnetNotReady",
 		},
 	}
+	if m != "" {
+		newConditions[0].Message = m
+	}
 	r.updateSubnetSetStatusConditions(ctx, subnetset, newConditions)
 }
 

pkg/nsx/services/common/limit.go

@@ -0,0 +1,5 @@
+package common
+
+const (
+	TagsCountMax = 26
+)

pkg/nsx/services/subnet/builder.go

@@ -1,12 +1,15 @@
 package subnet
 
 import (
+	"fmt"
+
 	"github.com/google/uuid"
 	"github.com/vmware/vsphere-automation-sdk-go/services/nsxt/model"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/vmware-tanzu/nsx-operator/pkg/apis/v1alpha1"
 	"github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/common"
+	util2 "github.com/vmware-tanzu/nsx-operator/pkg/nsx/util"
 	"github.com/vmware-tanzu/nsx-operator/pkg/util"
 )
 
@@ -65,6 +68,11 @@ func (service *SubnetService) buildSubnet(obj client.Object, tags []model.Tag) (
 	default:
 		return nil, SubnetTypeError
 	}
+	// tags cannot exceed maximum size 26
+	if len(tags) > common.TagsCountMax {
+		errorMsg := fmt.Sprintf("tags cannot exceed maximum size 26, tags length: %d", len(tags))
+		return nil, util2.ExceedTagsError{Desc: errorMsg}
+	}
 	nsxSubnet.Tags = tags
 	nsxSubnet.AdvancedConfig = &model.SubnetAdvancedConfig{
 		StaticIpAllocation: &model.StaticIpAllocation{

pkg/nsx/util/errors.go

@@ -574,3 +574,9 @@ type IPBlockAllExhaustedError struct {
 func (err IPBlockAllExhaustedError) Error() string {
 	return err.Desc
 }
+
+type ExceedTagsError struct {
+	Desc string
+}
+
+func (err ExceedTagsError) Error() string { return err.Desc }