From 0ecf56c7437ee1d0c7f71bd61e92b8993fc9528b Mon Sep 17 00:00:00 2001 From: Wenqi Qiu Date: Tue, 2 Jun 2026 11:50:28 +0800 Subject: [PATCH 1/2] Support auto-allocate VLAN for SubnetConnectionBindingMap When spec.vlanTrafficTag is not set, the subnetbinding controller allocates a VLAN from [0, 4094], writes it to spec, and realizes the NSX binding map. Used VLANs are discovered via NSX search on parent Subnet paths so bindings created outside the Supervisor are included. VLAN extension child Subnets prefer spec/status vlanId when it is still free on the target. Signed-off-by: Wenqi Qiu --- ...mware.com_subnetconnectionbindingmaps.yaml | 3 +- ...x_v1alpha1_subnetconnectionbindingmap.yaml | 10 ++ .../subnetconnectionbindingmap_types.go | 16 ++- .../vpc/v1alpha1/zz_generated.deepcopy.go | 5 + .../common/dependency_watcher_test.go | 4 +- .../subnet/subnetbinding_handler_test.go | 14 +-- .../subnetbinding/subnetbinding_controller.go | 75 ++++++++++++ .../subnetbinding_controller_test.go | 34 +++--- .../subnetbinding/subnets_handler_test.go | 4 +- .../subnetset/subnetbinding_handler_test.go | 12 +- pkg/nsx/services/subnetbinding/builder.go | 6 +- pkg/nsx/services/subnetbinding/store_test.go | 4 +- .../services/subnetbinding/vlan_allocation.go | 112 ++++++++++++++++++ pkg/nsx/services/vlanpool/vlanpool.go | 60 ++++++++++ pkg/nsx/services/vlanpool/vlanpool_test.go | 51 ++++++++ 15 files changed, 371 insertions(+), 39 deletions(-) create mode 100644 pkg/nsx/services/subnetbinding/vlan_allocation.go create mode 100644 pkg/nsx/services/vlanpool/vlanpool.go create mode 100644 pkg/nsx/services/vlanpool/vlanpool_test.go diff --git a/build/yaml/crd/vpc/crd.nsx.vmware.com_subnetconnectionbindingmaps.yaml b/build/yaml/crd/vpc/crd.nsx.vmware.com_subnetconnectionbindingmaps.yaml index 9e74b3664..4026dee2b 100644 --- a/build/yaml/crd/vpc/crd.nsx.vmware.com_subnetconnectionbindingmaps.yaml +++ b/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 diff --git a/build/yaml/samples/nsx_v1alpha1_subnetconnectionbindingmap.yaml b/build/yaml/samples/nsx_v1alpha1_subnetconnectionbindingmap.yaml index 073a1f1cc..746595a92 100644 --- a/build/yaml/samples/nsx_v1alpha1_subnetconnectionbindingmap.yaml +++ b/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 diff --git a/pkg/apis/vpc/v1alpha1/subnetconnectionbindingmap_types.go b/pkg/apis/vpc/v1alpha1/subnetconnectionbindingmap_types.go index b9f1db1a4..7611cc9f8 100644 --- a/pkg/apis/vpc/v1alpha1/subnetconnectionbindingmap_types.go +++ b/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{}) } diff --git a/pkg/apis/vpc/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/vpc/v1alpha1/zz_generated.deepcopy.go index 19d668ac2..c8bfd2af8 100644 --- a/pkg/apis/vpc/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/vpc/v1alpha1/zz_generated.deepcopy.go @@ -1034,6 +1034,11 @@ func (in *SubnetConnectionBindingMapList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *SubnetConnectionBindingMapSpec) DeepCopyInto(out *SubnetConnectionBindingMapSpec) { *out = *in + if in.VLANTrafficTag != nil { + in, out := &in.VLANTrafficTag, &out.VLANTrafficTag + *out = new(int64) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SubnetConnectionBindingMapSpec. diff --git a/pkg/controllers/common/dependency_watcher_test.go b/pkg/controllers/common/dependency_watcher_test.go index 9bf1fda49..748be4e8a 100644 --- a/pkg/controllers/common/dependency_watcher_test.go +++ b/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{ diff --git a/pkg/controllers/subnet/subnetbinding_handler_test.go b/pkg/controllers/subnet/subnetbinding_handler_test.go index deec1d674..2cf794621 100644 --- a/pkg/controllers/subnet/subnetbinding_handler_test.go +++ b/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() diff --git a/pkg/controllers/subnetbinding/subnetbinding_controller.go b/pkg/controllers/subnetbinding/subnetbinding_controller.go index 88b2c969b..73947b935 100644 --- a/pkg/controllers/subnetbinding/subnetbinding_controller.go +++ b/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} diff --git a/pkg/controllers/subnetbinding/subnetbinding_controller_test.go b/pkg/controllers/subnetbinding/subnetbinding_controller_test.go index 28fc34210..8a5c875c0 100644 --- a/pkg/controllers/subnetbinding/subnetbinding_controller_test.go +++ b/pkg/controllers/subnetbinding/subnetbinding_controller_test.go @@ -6,7 +6,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" @@ -102,7 +102,7 @@ func TestReconcile(t *testing.T) { Spec: v1alpha1.SubnetConnectionBindingMapSpec{ SubnetName: "child", TargetSubnetSetName: "parentSubnetSet", - VLANTrafficTag: 101, + VLANTrafficTag: v1alpha1.VLANTrafficTagPtr(101), }, } for _, tc := range []struct { @@ -184,6 +184,9 @@ func TestReconcile(t *testing.T) { patches := gomonkey.ApplyPrivateMethod(reflect.TypeOf(r), "validateDependency", func(_ *Reconciler, ctx context.Context, bindingMap *v1alpha1.SubnetConnectionBindingMap) (string, []string, *errorWithRetry) { return "/subnet-child", []string{"/subnet-parent"}, nil }) + patches.ApplyPrivateMethod(reflect.TypeOf(r), "reconcileVlanTrafficTag", func(_ *Reconciler, ctx context.Context, bindingMap *v1alpha1.SubnetConnectionBindingMap, parentSubnetPaths []string) *errorWithRetry { + return nil + }) patches.ApplyMethod(reflect.TypeOf(r.SubnetBindingService), "CreateOrUpdateSubnetConnectionBindingMap", func(_ *subnetbinding.BindingService, subnetBinding *v1alpha1.SubnetConnectionBindingMap, childSubnetPath string, parentSubnetPaths []string) error { return fmt.Errorf("failed to configure NSX") @@ -198,6 +201,9 @@ func TestReconcile(t *testing.T) { patches := gomonkey.ApplyPrivateMethod(reflect.TypeOf(r), "validateDependency", func(_ *Reconciler, ctx context.Context, bindingMap *v1alpha1.SubnetConnectionBindingMap) (string, []string, *errorWithRetry) { return "/subnet-child", []string{"/subnet-parent"}, nil }) + patches.ApplyPrivateMethod(reflect.TypeOf(r), "reconcileVlanTrafficTag", func(_ *Reconciler, ctx context.Context, bindingMap *v1alpha1.SubnetConnectionBindingMap, parentSubnetPaths []string) *errorWithRetry { + return nil + }) patches.ApplyMethod(reflect.TypeOf(r.SubnetBindingService), "CreateOrUpdateSubnetConnectionBindingMap", func(_ *subnetbinding.BindingService, subnetBinding *v1alpha1.SubnetConnectionBindingMap, childSubnetPath string, parentSubnetPaths []string) error { return nil @@ -287,7 +293,7 @@ func TestValidateDependency(t *testing.T) { Spec: v1alpha1.SubnetConnectionBindingMapSpec{ SubnetName: childSubnet, TargetSubnetName: targetSubnet, - VLANTrafficTag: 101, + VLANTrafficTag: v1alpha1.VLANTrafficTagPtr(101), }, } bindingCR2 := &v1alpha1.SubnetConnectionBindingMap{ @@ -298,7 +304,7 @@ func TestValidateDependency(t *testing.T) { Spec: v1alpha1.SubnetConnectionBindingMapSpec{ SubnetName: childSubnet, TargetSubnetSetName: targetSubnetSet, - VLANTrafficTag: 101, + VLANTrafficTag: v1alpha1.VLANTrafficTagPtr(101), }, } @@ -740,7 +746,7 @@ func TestUpdateBindingMapStatusWithConditions(t *testing.T) { Spec: v1alpha1.SubnetConnectionBindingMapSpec{ SubnetName: "child", TargetSubnetSetName: "parent", - VLANTrafficTag: 101, + VLANTrafficTag: v1alpha1.VLANTrafficTagPtr(101), }, } bindingMap2 := &v1alpha1.SubnetConnectionBindingMap{ @@ -751,7 +757,7 @@ func TestUpdateBindingMapStatusWithConditions(t *testing.T) { Spec: v1alpha1.SubnetConnectionBindingMapSpec{ SubnetName: "child", TargetSubnetSetName: "parent", - VLANTrafficTag: 101, + VLANTrafficTag: v1alpha1.VLANTrafficTagPtr(101), }, Status: v1alpha1.SubnetConnectionBindingMapStatus{ Conditions: []v1alpha1.Condition{ @@ -770,7 +776,7 @@ func TestUpdateBindingMapStatusWithConditions(t *testing.T) { Spec: v1alpha1.SubnetConnectionBindingMapSpec{ SubnetName: "child", TargetSubnetSetName: "parent", - VLANTrafficTag: 101, + VLANTrafficTag: v1alpha1.VLANTrafficTagPtr(101), }, Status: v1alpha1.SubnetConnectionBindingMapStatus{ Conditions: []v1alpha1.Condition{ @@ -792,7 +798,7 @@ func TestUpdateBindingMapStatusWithConditions(t *testing.T) { Spec: v1alpha1.SubnetConnectionBindingMapSpec{ SubnetName: "child", TargetSubnetSetName: "parent", - VLANTrafficTag: 101, + VLANTrafficTag: v1alpha1.VLANTrafficTagPtr(101), }, Status: v1alpha1.SubnetConnectionBindingMapStatus{ Conditions: []v1alpha1.Condition{ @@ -880,7 +886,7 @@ func TestUpdateBindingMapConditionWithRetry(t *testing.T) { Spec: v1alpha1.SubnetConnectionBindingMapSpec{ SubnetName: "child", TargetSubnetSetName: "parent", - VLANTrafficTag: 101, + VLANTrafficTag: v1alpha1.VLANTrafficTagPtr(101), }, }, condition: v1alpha1.Condition{ @@ -903,7 +909,7 @@ func TestUpdateBindingMapConditionWithRetry(t *testing.T) { Spec: v1alpha1.SubnetConnectionBindingMapSpec{ SubnetName: "child", TargetSubnetSetName: "parent", - VLANTrafficTag: 101, + VLANTrafficTag: v1alpha1.VLANTrafficTagPtr(101), }, Status: v1alpha1.SubnetConnectionBindingMapStatus{ Conditions: []v1alpha1.Condition{ @@ -936,7 +942,7 @@ func TestUpdateBindingMapConditionWithRetry(t *testing.T) { Spec: v1alpha1.SubnetConnectionBindingMapSpec{ SubnetName: "child", TargetSubnetSetName: "parent", - VLANTrafficTag: 101, + VLANTrafficTag: v1alpha1.VLANTrafficTagPtr(101), }, Status: v1alpha1.SubnetConnectionBindingMapStatus{ Conditions: []v1alpha1.Condition{ @@ -1082,7 +1088,7 @@ func TestPredicateFuncsBindingMaps(t *testing.T) { Spec: v1alpha1.SubnetConnectionBindingMapSpec{ SubnetName: "child", TargetSubnetSetName: "parent", - VLANTrafficTag: 101, + VLANTrafficTag: v1alpha1.VLANTrafficTagPtr(101), }, Status: v1alpha1.SubnetConnectionBindingMapStatus{ Conditions: []v1alpha1.Condition{ @@ -1101,7 +1107,7 @@ func TestPredicateFuncsBindingMaps(t *testing.T) { Spec: v1alpha1.SubnetConnectionBindingMapSpec{ SubnetName: "child", TargetSubnetSetName: "parent", - VLANTrafficTag: 102, + VLANTrafficTag: v1alpha1.VLANTrafficTagPtr(102), }, Status: v1alpha1.SubnetConnectionBindingMapStatus{ Conditions: []v1alpha1.Condition{ @@ -1120,7 +1126,7 @@ func TestPredicateFuncsBindingMaps(t *testing.T) { Spec: v1alpha1.SubnetConnectionBindingMapSpec{ SubnetName: "child", TargetSubnetSetName: "parent", - VLANTrafficTag: 101, + VLANTrafficTag: v1alpha1.VLANTrafficTagPtr(101), }, Status: v1alpha1.SubnetConnectionBindingMapStatus{ Conditions: []v1alpha1.Condition{ diff --git a/pkg/controllers/subnetbinding/subnets_handler_test.go b/pkg/controllers/subnetbinding/subnets_handler_test.go index 8264c917b..9b60fb106 100644 --- a/pkg/controllers/subnetbinding/subnets_handler_test.go +++ b/pkg/controllers/subnetbinding/subnets_handler_test.go @@ -163,7 +163,7 @@ func TestRequeueSubnetConnectionBindingMapsBySubnet(t *testing.T) { Spec: v1alpha1.SubnetConnectionBindingMapSpec{ SubnetName: "child", TargetSubnetName: "parent", - VLANTrafficTag: 101, + VLANTrafficTag: v1alpha1.VLANTrafficTagPtr(101), }, } req := reconcile.Request{ @@ -210,7 +210,7 @@ func TestRequeueSubnetConnectionBindingMapsBySubnetSet(t *testing.T) { Spec: v1alpha1.SubnetConnectionBindingMapSpec{ SubnetName: "child", TargetSubnetSetName: "parent", - VLANTrafficTag: 101, + VLANTrafficTag: v1alpha1.VLANTrafficTagPtr(101), }, } newScheme := runtime.NewScheme() diff --git a/pkg/controllers/subnetset/subnetbinding_handler_test.go b/pkg/controllers/subnetset/subnetbinding_handler_test.go index d5dbaa302..d4228c0ea 100644 --- a/pkg/controllers/subnetset/subnetbinding_handler_test.go +++ b/pkg/controllers/subnetset/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: "child", TargetSubnetName: "parent", - VLANTrafficTag: 101, + VLANTrafficTag: v1alpha1.VLANTrafficTagPtr(101), }, } @@ -45,7 +45,7 @@ var ( Spec: v1alpha1.SubnetConnectionBindingMapSpec{ SubnetName: "child1", TargetSubnetSetName: "parentSet", - VLANTrafficTag: 102, + VLANTrafficTag: v1alpha1.VLANTrafficTagPtr(102), }, } @@ -57,7 +57,7 @@ var ( Spec: v1alpha1.SubnetConnectionBindingMapSpec{ SubnetName: "child1", TargetSubnetSetName: "parentSet2", - VLANTrafficTag: 102, + VLANTrafficTag: v1alpha1.VLANTrafficTagPtr(102), }, } @@ -148,7 +148,7 @@ func TestGetSubnetBindingCRsBySubnet(t *testing.T) { Spec: v1alpha1.SubnetConnectionBindingMapSpec{ SubnetName: "subnet1", TargetSubnetSetName: "subnetset1", - VLANTrafficTag: 201, + VLANTrafficTag: v1alpha1.VLANTrafficTagPtr(201), }, } binding2 := &v1alpha1.SubnetConnectionBindingMap{ @@ -159,7 +159,7 @@ func TestGetSubnetBindingCRsBySubnet(t *testing.T) { Spec: v1alpha1.SubnetConnectionBindingMapSpec{ SubnetName: "subnet2", TargetSubnetName: "subnet1", - VLANTrafficTag: 201, + VLANTrafficTag: v1alpha1.VLANTrafficTagPtr(201), }, } newScheme := runtime.NewScheme() diff --git a/pkg/nsx/services/subnetbinding/builder.go b/pkg/nsx/services/subnetbinding/builder.go index 78b6e4250..fe2921e37 100644 --- a/pkg/nsx/services/subnetbinding/builder.go +++ b/pkg/nsx/services/subnetbinding/builder.go @@ -28,10 +28,14 @@ func (s *BindingService) buildSubnetBindings(binding *v1alpha1.SubnetConnectionB log.Error(err, "failed to parse parent Subnet path, ignore it") continue } + if !binding.Spec.HasVlanTrafficTag() { + log.Error(nil, "SubnetConnectionBindingMap has no vlanTrafficTag", "SubnetConnectionBindingMap", binding.Namespace+"/"+binding.Name) + continue + } bindingMaps[i] = &model.SubnetConnectionBindingMap{ Id: String(s.buildSubnetBindingID(binding, vpcSubnetInfo.ID)), DisplayName: String(binding.Name), - VlanTrafficTag: Int64(binding.Spec.VLANTrafficTag), + VlanTrafficTag: Int64(*binding.Spec.VLANTrafficTag), SubnetPath: &path, Tags: tags, } diff --git a/pkg/nsx/services/subnetbinding/store_test.go b/pkg/nsx/services/subnetbinding/store_test.go index ed4281d69..8a7904871 100644 --- a/pkg/nsx/services/subnetbinding/store_test.go +++ b/pkg/nsx/services/subnetbinding/store_test.go @@ -25,7 +25,7 @@ var ( }, Spec: v1alpha1.SubnetConnectionBindingMapSpec{ SubnetName: "child", - VLANTrafficTag: 201, + VLANTrafficTag: v1alpha1.VLANTrafficTagPtr(201), TargetSubnetSetName: "parent", }, } @@ -37,7 +37,7 @@ var ( }, Spec: v1alpha1.SubnetConnectionBindingMapSpec{ SubnetName: "child2", - VLANTrafficTag: 202, + VLANTrafficTag: v1alpha1.VLANTrafficTagPtr(202), TargetSubnetSetName: "parent2", }, } diff --git a/pkg/nsx/services/subnetbinding/vlan_allocation.go b/pkg/nsx/services/subnetbinding/vlan_allocation.go new file mode 100644 index 000000000..d07e83091 --- /dev/null +++ b/pkg/nsx/services/subnetbinding/vlan_allocation.go @@ -0,0 +1,112 @@ +package subnetbinding + +import ( + "fmt" + "strconv" + "strings" + + "github.com/vmware/vsphere-automation-sdk-go/services/nsxt/model" + "k8s.io/apimachinery/pkg/util/sets" + + servicecommon "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/common" + nsxutil "github.com/vmware-tanzu/nsx-operator/pkg/nsx/util" +) + +const searchPageSize = int64(1000) + +// CollectUsedVlansOnParentSubnets returns VLAN tags already used on the given parent Subnet paths by +// querying NSX (includes bindings not created by this Supervisor). +func (s *BindingService) CollectUsedVlansOnParentSubnets(parentSubnetPaths []string, excludeCRUID string) (sets.Set[int], error) { + used := sets.New[int]() + for _, parentPath := range parentSubnetPaths { + bindings, err := s.listBindingMapsByParentSubnetPath(parentPath) + if err != nil { + return nil, err + } + for _, bm := range bindings { + if excludeCRUID != "" && bindingMapCRUID(bm) == excludeCRUID { + continue + } + if bm.VlanTrafficTag != nil { + used.Insert(int(*bm.VlanTrafficTag)) + } + } + } + return used, nil +} + +func bindingMapCRUID(bm *model.SubnetConnectionBindingMap) string { + for _, tag := range bm.Tags { + if tag.Scope != nil && *tag.Scope == servicecommon.TagScopeSubnetBindingCRUID && tag.Tag != nil { + return *tag.Tag + } + } + return "" +} + +func (s *BindingService) listBindingMapsByParentSubnetPath(parentSubnetPath string) ([]*model.SubnetConnectionBindingMap, error) { + if s == nil || s.NSXClient == nil || s.NSXClient.QueryClient == nil { + return nil, fmt.Errorf("NSX query client is not initialized") + } + + pathEscaped := strings.ReplaceAll(parentSubnetPath, "/", "\\/") + queryParam := fmt.Sprintf("%s:%s AND marked_for_delete:false AND subnet_path:%s", + servicecommon.ResourceType, ResourceTypeSubnetConnectionBindingMap, pathEscaped) + + converter := servicecommon.NewConverter() + bindings := make([]*model.SubnetConnectionBindingMap, 0) + var cursor *string + pageSize := searchPageSize + + for { + response, err := s.NSXClient.QueryClient.List(queryParam, cursor, nil, &pageSize, nil, nil) + if err != nil { + err = servicecommon.TransError(err) + if _, ok := err.(nsxutil.PageMaxError); ok { + servicecommon.DecrementPageSize(&pageSize) + continue + } + return nil, err + } + + for _, entity := range response.Results { + obj, convErrs := converter.ConvertToGolang(entity, model.SubnetConnectionBindingMapBindingType()) + if len(convErrs) > 0 { + log.Error(convErrs[0], "Failed to convert NSX SubnetConnectionBindingMap from search result") + continue + } + bm, ok := obj.(*model.SubnetConnectionBindingMap) + if !ok || bm == nil { + log.Info("Skipping unexpected search result type for SubnetConnectionBindingMap") + continue + } + bindings = append(bindings, bm) + } + + if response.Cursor == nil { + break + } + c, err := strconv.Atoi(*response.Cursor) + if err != nil { + break + } + if response.ResultCount != nil && int64(c) >= *response.ResultCount { + break + } + cursor = response.Cursor + } + + return bindings, nil +} + +// ValidateVlanOnParentSubnets checks that vlan is not already used on any of the parent Subnet paths. +func (s *BindingService) ValidateVlanOnParentSubnets(parentSubnetPaths []string, vlan int64, excludeCRUID string) error { + used, err := s.CollectUsedVlansOnParentSubnets(parentSubnetPaths, excludeCRUID) + if err != nil { + return err + } + if used.Has(int(vlan)) { + return fmt.Errorf("vlanTrafficTag %d is already used on target Subnet or SubnetSet", vlan) + } + return nil +} diff --git a/pkg/nsx/services/vlanpool/vlanpool.go b/pkg/nsx/services/vlanpool/vlanpool.go new file mode 100644 index 000000000..c3b2e9d88 --- /dev/null +++ b/pkg/nsx/services/vlanpool/vlanpool.go @@ -0,0 +1,60 @@ +package vlanpool + +import ( + "fmt" + + "k8s.io/apimachinery/pkg/util/sets" + + "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/subnetbinding" +) + +const ( + poolStart = int64(0) + poolEnd = int64(4094) +) + +// VlanCollector collects VLAN IDs in use on parent Subnets from NSX. +type VlanCollector interface { + CollectUsedVlansOnParentSubnets(parentSubnetPaths []string, excludeCRUID string) (sets.Set[int], error) +} + +// Service allocates VLAN IDs for SubnetConnectionBindingMap from the range [0, 4094]. +type Service struct { + collector VlanCollector +} + +func NewService(collector *subnetbinding.BindingService) *Service { + return &Service{collector: collector} +} + +// Allocate picks an available VLAN. When preferred is valid and unused on the target, it is returned; +// otherwise the smallest free ID from [0, 4094] is used. +func (s *Service) Allocate(parentSubnetPaths []string, excludeCRUID string, preferred int64) (int64, error) { + used, err := s.collector.CollectUsedVlansOnParentSubnets(parentSubnetPaths, excludeCRUID) + if err != nil { + return 0, err + } + + if preferred >= poolStart && preferred <= poolEnd && !used.Has(int(preferred)) { + return preferred, nil + } + + for id := poolStart; id <= poolEnd; id++ { + if !used.Has(int(id)) { + return id, nil + } + } + return 0, fmt.Errorf("no available VLAN in pool [%d, %d] for target Subnet or SubnetSet", poolStart, poolEnd) +} + +// ValidateManualVlan checks that vlan is not already used on the parent Subnet paths. +func (s *Service) ValidateManualVlan(parentSubnetPaths []string, vlan int64, excludeCRUID string) error { + used, err := s.collector.CollectUsedVlansOnParentSubnets(parentSubnetPaths, excludeCRUID) + if err != nil { + return err + } + if used.Has(int(vlan)) { + return fmt.Errorf("vlanTrafficTag %d is already used on target Subnet or SubnetSet", vlan) + } + return nil +} diff --git a/pkg/nsx/services/vlanpool/vlanpool_test.go b/pkg/nsx/services/vlanpool/vlanpool_test.go new file mode 100644 index 000000000..387ef8c1d --- /dev/null +++ b/pkg/nsx/services/vlanpool/vlanpool_test.go @@ -0,0 +1,51 @@ +package vlanpool + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/util/sets" +) + +type fakeVlanCollector struct { + used sets.Set[int] + err error +} + +func (f *fakeVlanCollector) CollectUsedVlansOnParentSubnets(_ []string, _ string) (sets.Set[int], error) { + if f.err != nil { + return nil, f.err + } + return f.used, nil +} + +func TestAllocate_PrefersSubnetVlan(t *testing.T) { + svc := &Service{collector: &fakeVlanCollector{used: sets.New(100)}} + vlan, err := svc.Allocate([]string{"/parent"}, "", 201) + require.NoError(t, err) + assert.Equal(t, int64(201), vlan) +} + +func TestAllocate_FallbackOnConflict(t *testing.T) { + svc := &Service{collector: &fakeVlanCollector{used: sets.New(201)}} + vlan, err := svc.Allocate([]string{"/parent"}, "", 201) + require.NoError(t, err) + assert.Equal(t, int64(0), vlan) +} + +func TestAllocate_PoolExhausted(t *testing.T) { + used := sets.New[int]() + for i := 0; i <= 4094; i++ { + used.Insert(i) + } + svc := &Service{collector: &fakeVlanCollector{used: used}} + _, err := svc.Allocate([]string{"/parent"}, "", -1) + require.Error(t, err) +} + +func TestValidateManualVlan_Conflict(t *testing.T) { + svc := &Service{collector: &fakeVlanCollector{used: sets.New(50)}} + err := svc.ValidateManualVlan([]string{"/parent"}, 50, "") + require.Error(t, err) +} From 6ffa2b2a167686d72a0d410a9f1d3e7d66a942ba Mon Sep 17 00:00:00 2001 From: Wenqi Qiu Date: Tue, 2 Jun 2026 21:10:12 +0800 Subject: [PATCH 2/2] add ut Signed-off-by: Wenqi Qiu --- .../subnetbinding_controller_test.go | 163 +++++++++++++ .../services/subnetbinding/vlan_allocation.go | 6 +- .../subnetbinding/vlan_allocation_test.go | 219 ++++++++++++++++++ 3 files changed, 385 insertions(+), 3 deletions(-) create mode 100644 pkg/nsx/services/subnetbinding/vlan_allocation_test.go diff --git a/pkg/controllers/subnetbinding/subnetbinding_controller_test.go b/pkg/controllers/subnetbinding/subnetbinding_controller_test.go index 8a5c875c0..c4821a891 100644 --- a/pkg/controllers/subnetbinding/subnetbinding_controller_test.go +++ b/pkg/controllers/subnetbinding/subnetbinding_controller_test.go @@ -32,6 +32,7 @@ import ( "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" ) type fakeRecorder struct{} @@ -1271,3 +1272,165 @@ func createFakeReconciler(objs ...client.Object) *Reconciler { return NewReconciler(mgr, subnetService, bindingService) } + +func TestReconcileVlanTrafficTag(t *testing.T) { + bm1 := &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "bm-1", + UID: "uuid-1", + }, + Spec: v1alpha1.SubnetConnectionBindingMapSpec{ + SubnetName: "subnet-child-1", + TargetSubnetName: "subnet-parent-1", + VLANTrafficTag: v1alpha1.VLANTrafficTagPtr(201), + }, + } + bm2 := &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "bm-2", + UID: "uuid-2", + }, + Spec: v1alpha1.SubnetConnectionBindingMapSpec{ + SubnetName: "subnet-child-2", + TargetSubnetName: "subnet-parent-2", + }, + } + childSubnet := &v1alpha1.Subnet{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "subnet-child-2", + }, + Status: v1alpha1.SubnetStatus{ + VLANExtension: v1alpha1.VLANExtension{ + VLANID: 300, + }, + }, + } + + for _, tc := range []struct { + name string + bindingMap *v1alpha1.SubnetConnectionBindingMap + objects []client.Object + patches func(t *testing.T, r *Reconciler) *gomonkey.Patches + expErr bool + expVlan *int64 + }{ + { + name: "Manual VLAN successfully validated", + bindingMap: bm1, + patches: func(t *testing.T, r *Reconciler) *gomonkey.Patches { + return gomonkey.ApplyMethod(reflect.TypeOf(r.VlanPoolService), "ValidateManualVlan", func(_ *vlanpool.Service, parentSubnetPaths []string, vlan int64, excludeCRUID string) error { + return nil + }) + }, + expErr: false, + expVlan: v1alpha1.VLANTrafficTagPtr(201), + }, + { + name: "Manual VLAN validation failed", + bindingMap: bm1, + patches: func(t *testing.T, r *Reconciler) *gomonkey.Patches { + return gomonkey.ApplyMethod(reflect.TypeOf(r.VlanPoolService), "ValidateManualVlan", func(_ *vlanpool.Service, parentSubnetPaths []string, vlan int64, excludeCRUID string) error { + return fmt.Errorf("conflict") + }) + }, + expErr: true, + }, + { + name: "Auto allocate VLAN fails on getting child Subnet", + bindingMap: bm2, + patches: func(t *testing.T, r *Reconciler) *gomonkey.Patches { + return gomonkey.ApplyMethod(reflect.TypeOf(r.Client), "Get", func(_ client.Client, ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + return fmt.Errorf("cr not found") + }) + }, + expErr: true, + }, + { + name: "Auto allocate VLAN successfully", + bindingMap: bm2, + objects: []client.Object{childSubnet, bm2}, + patches: func(t *testing.T, r *Reconciler) *gomonkey.Patches { + patches := gomonkey.ApplyMethod(reflect.TypeOf(r.VlanPoolService), "Allocate", func(_ *vlanpool.Service, parentSubnetPaths []string, excludeCRUID string, preferred int64) (int64, error) { + return 300, nil + }) + patches.ApplyPrivateMethod(reflect.TypeOf(r), "patchSpecVlanTrafficTag", func(_ *Reconciler, ctx context.Context, bindingMap *v1alpha1.SubnetConnectionBindingMap, vlan int64) error { + return nil + }) + return patches + }, + expErr: false, + expVlan: v1alpha1.VLANTrafficTagPtr(300), + }, + { + name: "Auto allocate VLAN fails on patch spec", + bindingMap: bm2, + objects: []client.Object{childSubnet, bm2}, + patches: func(t *testing.T, r *Reconciler) *gomonkey.Patches { + patches := gomonkey.ApplyMethod(reflect.TypeOf(r.VlanPoolService), "Allocate", func(_ *vlanpool.Service, parentSubnetPaths []string, excludeCRUID string, preferred int64) (int64, error) { + return 300, nil + }) + patches.ApplyPrivateMethod(reflect.TypeOf(r), "patchSpecVlanTrafficTag", func(_ *Reconciler, ctx context.Context, bindingMap *v1alpha1.SubnetConnectionBindingMap, vlan int64) error { + return fmt.Errorf("patch error") + }) + return patches + }, + expErr: true, + }, + } { + t.Run(tc.name, func(t *testing.T) { + ctx := context.TODO() + r := createFakeReconciler(tc.objects...) + if tc.patches != nil { + patches := tc.patches(t, r) + defer patches.Reset() + } + err := r.reconcileVlanTrafficTag(ctx, tc.bindingMap, []string{"/parent"}) + if tc.expErr { + assert.NotNil(t, err) + } else { + assert.Nil(t, err) + if tc.expVlan != nil { + assert.Equal(t, *tc.expVlan, *tc.bindingMap.Spec.VLANTrafficTag) + } + } + }) + } +} + +func TestPatchSpecVlanTrafficTag(t *testing.T) { + bm1 := &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "bm-1", + }, + Spec: v1alpha1.SubnetConnectionBindingMapSpec{ + SubnetName: "subnet-child-1", + TargetSubnetName: "subnet-parent-1", + }, + } + r := createFakeReconciler(bm1) + newScheme := runtime.NewScheme() + utilruntime.Must(clientgoscheme.AddToScheme(newScheme)) + utilruntime.Must(v1alpha1.AddToScheme(newScheme)) + r.Client = fake.NewClientBuilder(). + WithScheme(newScheme). + WithObjects(bm1). + Build() + + ctx := context.TODO() + // Patch new VLAN + err := r.patchSpecVlanTrafficTag(ctx, bm1, 101) + assert.NoError(t, err) + + updatedBM := &v1alpha1.SubnetConnectionBindingMap{} + err = r.Client.Get(ctx, types.NamespacedName{Namespace: "default", Name: "bm-1"}, updatedBM) + assert.NoError(t, err) + assert.Equal(t, int64(101), *updatedBM.Spec.VLANTrafficTag) + + // Patch same VLAN should succeed immediately + err = r.patchSpecVlanTrafficTag(ctx, bm1, 101) + assert.NoError(t, err) +} diff --git a/pkg/nsx/services/subnetbinding/vlan_allocation.go b/pkg/nsx/services/subnetbinding/vlan_allocation.go index d07e83091..4e562c08a 100644 --- a/pkg/nsx/services/subnetbinding/vlan_allocation.go +++ b/pkg/nsx/services/subnetbinding/vlan_allocation.go @@ -75,12 +75,12 @@ func (s *BindingService) listBindingMapsByParentSubnetPath(parentSubnetPath stri log.Error(convErrs[0], "Failed to convert NSX SubnetConnectionBindingMap from search result") continue } - bm, ok := obj.(*model.SubnetConnectionBindingMap) - if !ok || bm == nil { + bmV, ok := obj.(model.SubnetConnectionBindingMap) + if !ok { log.Info("Skipping unexpected search result type for SubnetConnectionBindingMap") continue } - bindings = append(bindings, bm) + bindings = append(bindings, &bmV) } if response.Cursor == nil { diff --git a/pkg/nsx/services/subnetbinding/vlan_allocation_test.go b/pkg/nsx/services/subnetbinding/vlan_allocation_test.go new file mode 100644 index 000000000..eaf4ab6eb --- /dev/null +++ b/pkg/nsx/services/subnetbinding/vlan_allocation_test.go @@ -0,0 +1,219 @@ +package subnetbinding + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/vmware/vsphere-automation-sdk-go/runtime/data" + "github.com/vmware/vsphere-automation-sdk-go/services/nsxt/model" + "go.uber.org/mock/gomock" + + search_mocks "github.com/vmware-tanzu/nsx-operator/pkg/mock/searchclient" + "github.com/vmware-tanzu/nsx-operator/pkg/nsx" + "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/common" + nsxutil "github.com/vmware-tanzu/nsx-operator/pkg/nsx/util" +) + +func TestCollectUsedVlansOnParentSubnets(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + fakeQueryClient := search_mocks.NewMockQueryClient(ctrl) + svc := &BindingService{ + Service: common.Service{ + NSXClient: &nsx.Client{ + QueryClient: fakeQueryClient, + }, + }, + } + + bm1 := &model.SubnetConnectionBindingMap{ + Id: String("id-1"), + DisplayName: String("binding1"), + SubnetPath: String("/parent1"), + ParentPath: String("/child1"), + Path: String("/child1/subnet-connection-binding-maps/id-1"), + VlanTrafficTag: Int64(101), + ResourceType: String(ResourceTypeSubnetConnectionBindingMap), + Tags: []model.Tag{ + {Scope: String(common.TagScopeSubnetBindingCRUID), Tag: String("uuid-1")}, + }, + } + bm2 := &model.SubnetConnectionBindingMap{ + Id: String("id-2"), + DisplayName: String("binding2"), + SubnetPath: String("/parent1"), + ParentPath: String("/child1"), + Path: String("/child1/subnet-connection-binding-maps/id-2"), + VlanTrafficTag: Int64(102), + ResourceType: String(ResourceTypeSubnetConnectionBindingMap), + Tags: []model.Tag{ + {Scope: String(common.TagScopeSubnetBindingCRUID), Tag: String("uuid-2")}, + }, + } + bm3 := &model.SubnetConnectionBindingMap{ + Id: String("id-3"), + DisplayName: String("binding3"), + SubnetPath: String("/parent1"), + ParentPath: String("/child1"), + Path: String("/child1/subnet-connection-binding-maps/id-3"), + VlanTrafficTag: nil, // no tag + ResourceType: String(ResourceTypeSubnetConnectionBindingMap), + } + + converter := common.NewConverter() + dv1, _ := converter.ConvertToVapi(bm1, model.SubnetConnectionBindingMapBindingType()) + dv2, _ := converter.ConvertToVapi(bm2, model.SubnetConnectionBindingMapBindingType()) + dv3, _ := converter.ConvertToVapi(bm3, model.SubnetConnectionBindingMapBindingType()) + + resultCount := int64(3) + fakeQueryClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(model.SearchResponse{ + ResultCount: &resultCount, + Results: []*data.StructValue{ + dv1.(*data.StructValue), + dv2.(*data.StructValue), + dv3.(*data.StructValue), + }, + }, nil).Times(1) + + used, err := svc.CollectUsedVlansOnParentSubnets([]string{"/parent1"}, "uuid-1") // Exclude uuid-1 + require.NoError(t, err) + + assert.False(t, used.Has(101)) // Excluded + assert.True(t, used.Has(102)) // Included + assert.Equal(t, 1, used.Len()) +} + +func TestListBindingMapsByParentSubnetPath(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + t.Run("Query client not initialized", func(t *testing.T) { + svc := &BindingService{} + _, err := svc.listBindingMapsByParentSubnetPath("/parent") + require.Error(t, err) + assert.Contains(t, err.Error(), "NSX query client is not initialized") + }) + + t.Run("Success single page", func(t *testing.T) { + fakeQueryClient := search_mocks.NewMockQueryClient(ctrl) + svc := &BindingService{ + Service: common.Service{ + NSXClient: &nsx.Client{ + QueryClient: fakeQueryClient, + }, + }, + } + bm := &model.SubnetConnectionBindingMap{ + Id: String("id-1"), + DisplayName: String("binding1"), + SubnetPath: String("/parent1"), + ParentPath: String("/child1"), + Path: String("/child1/subnet-connection-binding-maps/id-1"), + ResourceType: String(ResourceTypeSubnetConnectionBindingMap), + } + dv, _ := common.NewConverter().ConvertToVapi(bm, model.SubnetConnectionBindingMapBindingType()) + fakeQueryClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(model.SearchResponse{ + Results: []*data.StructValue{dv.(*data.StructValue)}, + }, nil).Times(1) + + bindings, err := svc.listBindingMapsByParentSubnetPath("/parent") + require.NoError(t, err) + assert.Len(t, bindings, 1) + assert.Equal(t, "id-1", *bindings[0].Id) + }) + + t.Run("Page max error retry", func(t *testing.T) { + fakeQueryClient := search_mocks.NewMockQueryClient(ctrl) + svc := &BindingService{ + Service: common.Service{ + NSXClient: &nsx.Client{ + QueryClient: fakeQueryClient, + }, + }, + } + + fakeQueryClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(model.SearchResponse{}, nsxutil.PageMaxError{}).Times(1) + fakeQueryClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(model.SearchResponse{ + Results: []*data.StructValue{}, + }, nil).Times(1) + + bindings, err := svc.listBindingMapsByParentSubnetPath("/parent") + require.NoError(t, err) + assert.Len(t, bindings, 0) + }) + + t.Run("Error listing", func(t *testing.T) { + fakeQueryClient := search_mocks.NewMockQueryClient(ctrl) + svc := &BindingService{ + Service: common.Service{ + NSXClient: &nsx.Client{ + QueryClient: fakeQueryClient, + }, + }, + } + fakeQueryClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(model.SearchResponse{}, fmt.Errorf("search error")).Times(1) + + _, err := svc.listBindingMapsByParentSubnetPath("/parent") + require.Error(t, err) + }) +} + +func TestValidateVlanOnParentSubnets(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + fakeQueryClient := search_mocks.NewMockQueryClient(ctrl) + svc := &BindingService{ + Service: common.Service{ + NSXClient: &nsx.Client{ + QueryClient: fakeQueryClient, + }, + }, + } + + bm := &model.SubnetConnectionBindingMap{ + Id: String("id-1"), + DisplayName: String("binding1"), + SubnetPath: String("/parent1"), + ParentPath: String("/child1"), + Path: String("/child1/subnet-connection-binding-maps/id-1"), + VlanTrafficTag: Int64(201), + ResourceType: String(ResourceTypeSubnetConnectionBindingMap), + } + dv, _ := common.NewConverter().ConvertToVapi(bm, model.SubnetConnectionBindingMapBindingType()) + + // Test conflict + fakeQueryClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(model.SearchResponse{ + Results: []*data.StructValue{dv.(*data.StructValue)}, + }, nil).Times(1) + + err := svc.ValidateVlanOnParentSubnets([]string{"/parent"}, 201, "") + require.Error(t, err) + assert.Contains(t, err.Error(), "already used") + + // Test success + fakeQueryClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(model.SearchResponse{ + Results: []*data.StructValue{dv.(*data.StructValue)}, + }, nil).Times(1) + + err = svc.ValidateVlanOnParentSubnets([]string{"/parent"}, 202, "") + require.NoError(t, err) +} + +func TestBindingMapCRUID(t *testing.T) { + bm := &model.SubnetConnectionBindingMap{ + Tags: []model.Tag{ + {Scope: String(common.TagScopeSubnetBindingCRUID), Tag: String("test-uuid")}, + {Scope: String(common.TagScopeNamespace), Tag: String("default")}, + }, + } + uid := bindingMapCRUID(bm) + assert.Equal(t, "test-uuid", uid) + + bm2 := &model.SubnetConnectionBindingMap{} + uid2 := bindingMapCRUID(bm2) + assert.Equal(t, "", uid2) +}