Support auto-allocate VLAN for SubnetConnectionBindingMap

build/yaml/crd/vpc/crd.nsx.vmware.com_subnetconnectionbindingmaps.yaml

@@ -79,14 +79,13 @@ spec:
               vlanTrafficTag:
                 description: |-
                   VLANTrafficTag is the VLAN tag configured in the binding. Note, the value of VLANTrafficTag should be
-                  unique on the target Subnet or SubnetSet.
+                  unique on the target Subnet or SubnetSet. When omitted, the operator auto-allocates a VLAN ID.
                 format: int64
                 maximum: 4094
                 minimum: 0
                 type: integer
             required:
             - subnetName
-            - vlanTrafficTag
             type: object
             x-kubernetes-validations:
             - message: Only one of targetSubnetSetName or targetSubnetName can be

build/yaml/samples/nsx_v1alpha1_subnetconnectionbindingmap.yaml

@@ -17,3 +17,13 @@ spec:
   subnetName: subnet-sample
   targetSubnetName: subnet1
   vlanTrafficTag: 202
+---
+apiVersion: crd.nsx.vmware.com/v1alpha1
+kind: SubnetConnectionBindingMap
+metadata:
+  name: binding-auto
+  namespace: default
+spec:
+  subnetName: vlan-ext-subnet
+  targetSubnetSetName: vm-default
+  # vlanTrafficTag omitted: operator auto-allocates and patches spec.vlanTrafficTag

pkg/apis/vpc/v1alpha1/subnetconnectionbindingmap_types.go

@@ -18,11 +18,11 @@ type SubnetConnectionBindingMapSpec struct {
 	// +kubebuilder:validation:Optional
 	TargetSubnetName string `json:"targetSubnetName,omitempty"`
 	// VLANTrafficTag is the VLAN tag configured in the binding. Note, the value of VLANTrafficTag should be
-	// unique on the target Subnet or SubnetSet.
+	// unique on the target Subnet or SubnetSet. When omitted, the operator auto-allocates a VLAN ID.
 	// +kubebuilder:validation:Maximum:=4094
 	// +kubebuilder:validation:Minimum:=0
-	// +kubebuilder:validation:Required
-	VLANTrafficTag int64 `json:"vlanTrafficTag"`
+	// +kubebuilder:validation:Optional
+	VLANTrafficTag *int64 `json:"vlanTrafficTag,omitempty"`
 }
 
 // SubnetConnectionBindingMapStatus defines the observed state of SubnetConnectionBindingMap.
@@ -60,6 +60,16 @@ type SubnetConnectionBindingMapList struct {
 	Items           []SubnetConnectionBindingMap `json:"items,omitempty"`
 }
 
+// VLANTrafficTagPtr returns a pointer to the given VLAN traffic tag value.
+func VLANTrafficTagPtr(v int64) *int64 {
+	return &v
+}
+
+// HasVlanTrafficTag reports whether spec.vlanTrafficTag is set.
+func (s *SubnetConnectionBindingMapSpec) HasVlanTrafficTag() bool {
+	return s.VLANTrafficTag != nil
+}
+
 func init() {
 	SchemeBuilder.Register(&SubnetConnectionBindingMap{}, &SubnetConnectionBindingMapList{})
 }

pkg/controllers/common/dependency_watcher_test.go

@@ -98,7 +98,7 @@ func TestPredicateFuncsBindingMap(t *testing.T) {
 		Spec: v1alpha1.SubnetConnectionBindingMapSpec{
 			SubnetName:       "child",
 			TargetSubnetName: "parent1",
-			VLANTrafficTag:   202,
+			VLANTrafficTag:   v1alpha1.VLANTrafficTagPtr(202),
 		},
 		Status: v1alpha1.SubnetConnectionBindingMapStatus{
 			Conditions: []v1alpha1.Condition{
@@ -117,7 +117,7 @@ func TestPredicateFuncsBindingMap(t *testing.T) {
 		Spec: v1alpha1.SubnetConnectionBindingMapSpec{
 			SubnetName:       "child",
 			TargetSubnetName: "parent1",
-			VLANTrafficTag:   201,
+			VLANTrafficTag:   v1alpha1.VLANTrafficTagPtr(201),
 		},
 		Status: v1alpha1.SubnetConnectionBindingMapStatus{
 			Conditions: []v1alpha1.Condition{

pkg/controllers/subnet/subnetbinding_handler_test.go

@@ -5,7 +5,7 @@ import (
 	"reflect"
 	"testing"
 
-	"github.com/agiledragon/gomonkey/v2"
+	gomonkey "github.com/agiledragon/gomonkey/v2"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/vmware/vsphere-automation-sdk-go/services/nsxt/model"
@@ -33,7 +33,7 @@ var (
 		Spec: v1alpha1.SubnetConnectionBindingMapSpec{
 			SubnetName:       "child1",
 			TargetSubnetName: "parent",
-			VLANTrafficTag:   101,
+			VLANTrafficTag:   v1alpha1.VLANTrafficTagPtr(101),
 		},
 	}
 
@@ -45,7 +45,7 @@ var (
 		Spec: v1alpha1.SubnetConnectionBindingMapSpec{
 			SubnetName:       "child1",
 			TargetSubnetName: "parent2",
-			VLANTrafficTag:   102,
+			VLANTrafficTag:   v1alpha1.VLANTrafficTagPtr(102),
 		},
 	}
 
@@ -57,7 +57,7 @@ var (
 		Spec: v1alpha1.SubnetConnectionBindingMapSpec{
 			SubnetName:          "child1",
 			TargetSubnetSetName: "parentSet2",
-			VLANTrafficTag:      101,
+			VLANTrafficTag:      v1alpha1.VLANTrafficTagPtr(101),
 		},
 	}
 
@@ -69,7 +69,7 @@ var (
 		Spec: v1alpha1.SubnetConnectionBindingMapSpec{
 			SubnetName:       "child2",
 			TargetSubnetName: "parent3",
-			VLANTrafficTag:   101,
+			VLANTrafficTag:   v1alpha1.VLANTrafficTagPtr(101),
 		},
 	}
 
@@ -172,7 +172,7 @@ func TestGetSubnetBindingCRsBySubnet(t *testing.T) {
 		Spec: v1alpha1.SubnetConnectionBindingMapSpec{
 			SubnetName:       "subnet1",
 			TargetSubnetName: "subnet2",
-			VLANTrafficTag:   201,
+			VLANTrafficTag:   v1alpha1.VLANTrafficTagPtr(201),
 		},
 	}
 	binding2 := &v1alpha1.SubnetConnectionBindingMap{
@@ -183,7 +183,7 @@ func TestGetSubnetBindingCRsBySubnet(t *testing.T) {
 		Spec: v1alpha1.SubnetConnectionBindingMapSpec{
 			SubnetName:       "subnet2",
 			TargetSubnetName: "subnet3",
-			VLANTrafficTag:   201,
+			VLANTrafficTag:   v1alpha1.VLANTrafficTagPtr(201),
 		},
 	}
 	newScheme := runtime.NewScheme()

pkg/controllers/subnetbinding/subnetbinding_controller.go

@@ -27,6 +27,7 @@ import (
 	servicecommon "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/common"
 	"github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/subnet"
 	"github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/subnetbinding"
+	"github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/vlanpool"
 )
 
 var (
@@ -45,6 +46,7 @@ type Reconciler struct {
 	Scheme               *runtime.Scheme
 	SubnetService        *subnet.SubnetService
 	SubnetBindingService *subnetbinding.BindingService
+	VlanPoolService      *vlanpool.Service
 	StatusUpdater        common.StatusUpdater
 }
 
@@ -76,6 +78,7 @@ func NewReconciler(mgr ctrl.Manager, subnetService *subnet.SubnetService, subnet
 		Scheme:               mgr.GetScheme(),
 		SubnetService:        subnetService,
 		SubnetBindingService: subnetBindingService,
+		VlanPoolService:      vlanpool.NewService(subnetBindingService),
 		StatusUpdater:        common.NewStatusUpdater(mgr.GetClient(), subnetBindingService.NSXConfig, recorder, common.MetricResTypeSubnetConnectionBindingMap, "SubnetConnectionBindingMap", "SubnetConnectionBindingMap"),
 	}
 }
@@ -118,6 +121,14 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		return common.ResultRequeueAfter60sec, nil
 	}
 
+	if vlanErr := r.reconcileVlanTrafficTag(ctx, bindingMapCR, parentSubnetPaths); vlanErr != nil {
+		r.StatusUpdater.UpdateFail(ctx, bindingMapCR, vlanErr, "failed to reconcile VLAN traffic tag", updateBindingMapStatusWithUnreadyCondition, "VlanAllocationFailed", vlanErr.message)
+		if !vlanErr.retry {
+			return common.ResultNormal, nil
+		}
+		return common.ResultRequeue, nil
+	}
+
 	if err := r.SubnetBindingService.CreateOrUpdateSubnetConnectionBindingMap(bindingMapCR, childSubnetPath, parentSubnetPaths); err != nil {
 		// Update SubnetConnectionBindingMap with not-ready condition
 		r.StatusUpdater.UpdateFail(ctx, bindingMapCR, err, "failure to configure SubnetConnectionBindingMaps on NSX", updateBindingMapStatusWithUnreadyCondition, "ConfigureFailed", fmt.Sprintf("Failed to realize SubnetConnectionBindingMap %s on NSX", req.Name))
@@ -276,6 +287,70 @@ func (r *Reconciler) validateDependency(ctx context.Context, bindingMap *v1alpha
 	return childSubnetPath, parentSubnetPaths, nil
 }
 
+func (r *Reconciler) reconcileVlanTrafficTag(ctx context.Context, bindingMap *v1alpha1.SubnetConnectionBindingMap, parentSubnetPaths []string) *errorWithRetry {
+	if bindingMap.Spec.HasVlanTrafficTag() {
+		vlan := *bindingMap.Spec.VLANTrafficTag
+		if err := r.VlanPoolService.ValidateManualVlan(parentSubnetPaths, vlan, string(bindingMap.UID)); err != nil {
+			return &errorWithRetry{
+				message: err.Error(),
+				error:   err,
+				retry:   true,
+			}
+		}
+		return nil
+	}
+
+	childSubnet := &v1alpha1.Subnet{}
+	childSubnetKey := types.NamespacedName{Namespace: bindingMap.Namespace, Name: bindingMap.Spec.SubnetName}
+	if err := r.Client.Get(ctx, childSubnetKey, childSubnet); err != nil {
+		log.Error(err, "Failed to get Subnet CR for VLAN auto allocation", "Subnet", childSubnetKey.String())
+		return &errorWithRetry{
+			message: fmt.Sprintf("Unable to get Subnet CR %s for VLAN auto allocation", bindingMap.Spec.SubnetName),
+			error:   fmt.Errorf("failed to get Subnet %s in Namespace %s: %w", bindingMap.Spec.SubnetName, bindingMap.Namespace, err),
+			retry:   false,
+		}
+	}
+
+	preferred := int64(-1)
+	if childSubnet.Status.VLANExtension.VLANID != 0 {
+		preferred = int64(childSubnet.Status.VLANExtension.VLANID)
+	}
+
+	vlan, err := r.VlanPoolService.Allocate(parentSubnetPaths, string(bindingMap.UID), preferred)
+	if err != nil {
+		return &errorWithRetry{
+			message: err.Error(),
+			error:   err,
+			retry:   true,
+		}
+	}
+
+	if err := r.patchSpecVlanTrafficTag(ctx, bindingMap, vlan); err != nil {
+		return &errorWithRetry{
+			message: fmt.Sprintf("Failed to update spec.vlanTrafficTag: %v", err),
+			error:   err,
+			retry:   true,
+		}
+	}
+	bindingMap.Spec.VLANTrafficTag = v1alpha1.VLANTrafficTagPtr(vlan)
+	return nil
+}
+
+func (r *Reconciler) patchSpecVlanTrafficTag(ctx context.Context, bindingMap *v1alpha1.SubnetConnectionBindingMap, vlan int64) error {
+	key := types.NamespacedName{Namespace: bindingMap.Namespace, Name: bindingMap.Name}
+	return retry.RetryOnConflict(retry.DefaultRetry, func() error {
+		latest := &v1alpha1.SubnetConnectionBindingMap{}
+		if err := r.Client.Get(ctx, key, latest); err != nil {
+			return err
+		}
+		if latest.Spec.HasVlanTrafficTag() && *latest.Spec.VLANTrafficTag == vlan {
+			return nil
+		}
+		latest.Spec.VLANTrafficTag = v1alpha1.VLANTrafficTagPtr(vlan)
+		return r.Client.Update(ctx, latest)
+	})
+}
+
 func (r *Reconciler) validateVpcSubnetsBySubnetCR(ctx context.Context, namespace, name string, isTarget bool) ([]string, *v1alpha1.Subnet, *errorWithRetry) {
 	subnetCR := &v1alpha1.Subnet{}
 	subnetKey := types.NamespacedName{Namespace: namespace, Name: name}