From 346923e86e2b240563de521ca3302035a1f560ea Mon Sep 17 00:00:00 2001 From: Wenying Dong Date: Fri, 1 Nov 2024 11:16:07 +0800 Subject: [PATCH] Implement the reconciler for SubnetConnectionBindingMap 1. Implement the reconciler for SubnetConnectionBindingMap, it may update the SubnetConnectionBindingMap status with condition ready is false if its dependent Subnet or SubnetSet is not ready (or realized) or it hits errors when realizing NSX SubnetConnectionBindingMaps. It updates the status with ready condition as true if it is successfully realized on NSX. The reconciler also watches the Subnet/SubnetSet CR events to sync the connection binding maps. 2. The change also modifies the Subnet/SubnetSet reconciler to watch SubnetConnectionBindingMap CR events. If a Subnet/SubnetSet is used by a SubnetConnectionBindingMap, a finalizer is added on the corresponding Subnet/SubnetSet CR, and the finalizer is removed automatically if the CR is not used by any SubnetConnectionBindingMaps. --- cmd/main.go | 14 +- pkg/apis/vpc/v1alpha1/condition_types.go | 1 + pkg/clean/clean.go | 7 + pkg/clean/clean_test.go | 11 +- pkg/controllers/common/dependency_watcher.go | 93 ++ .../common/dependency_watcher_test.go | 272 ++++++ pkg/controllers/common/types.go | 33 +- pkg/controllers/subnet/subnet_controller.go | 69 +- .../subnet/subnet_controller_test.go | 39 +- .../subnet/subnetbinding_handler.go | 108 +++ .../subnet/subnetbinding_handler_test.go | 170 ++++ pkg/controllers/subnetbinding/controller.go | 333 +++++++ .../subnetbinding/controller_test.go | 900 ++++++++++++++++++ .../subnetbinding/subnets_handler.go | 110 +++ .../subnetbinding/subnets_handler_test.go | 258 +++++ .../subnetset/subnetbinding_handler.go | 101 ++ .../subnetset/subnetbinding_handler_test.go | 153 +++ .../subnetset/subnetset_controller.go | 141 ++- .../subnetset/subnetset_controller_test.go | 50 +- pkg/mock/orgrootclient/client.go | 64 ++ pkg/mock/searchclient/client.go | 50 + .../client.go | 108 +++ pkg/nsx/client.go | 114 +-- pkg/nsx/services/common/types.go | 5 + pkg/nsx/services/subnetbinding/builder.go | 66 ++ .../services/subnetbinding/builder_test.go | 135 +++ pkg/nsx/services/subnetbinding/compare.go | 36 + pkg/nsx/services/subnetbinding/store.go | 162 ++++ pkg/nsx/services/subnetbinding/store_test.go | 148 +++ .../services/subnetbinding/subnetbinding.go | 224 +++++ .../subnetbinding/subnetbinding_test.go | 516 ++++++++++ pkg/nsx/services/subnetbinding/tree.go | 221 +++++ pkg/nsx/services/subnetbinding/tree_test.go | 359 +++++++ pkg/util/utils.go | 4 + test/e2e/nsx_networkinfo_test.go | 2 +- 35 files changed, 4972 insertions(+), 105 deletions(-) create mode 100644 pkg/controllers/common/dependency_watcher.go create mode 100644 pkg/controllers/common/dependency_watcher_test.go create mode 100644 pkg/controllers/subnet/subnetbinding_handler.go create mode 100644 pkg/controllers/subnet/subnetbinding_handler_test.go create mode 100644 pkg/controllers/subnetbinding/controller.go create mode 100644 pkg/controllers/subnetbinding/controller_test.go create mode 100644 pkg/controllers/subnetbinding/subnets_handler.go create mode 100644 pkg/controllers/subnetbinding/subnets_handler_test.go create mode 100644 pkg/controllers/subnetset/subnetbinding_handler.go create mode 100644 pkg/controllers/subnetset/subnetbinding_handler_test.go create mode 100644 pkg/mock/orgrootclient/client.go create mode 100644 pkg/mock/searchclient/client.go create mode 100644 pkg/mock/subnetconnectionbindingmapclient/client.go create mode 100644 pkg/nsx/services/subnetbinding/builder.go create mode 100644 pkg/nsx/services/subnetbinding/builder_test.go create mode 100644 pkg/nsx/services/subnetbinding/compare.go create mode 100644 pkg/nsx/services/subnetbinding/store.go create mode 100644 pkg/nsx/services/subnetbinding/store_test.go create mode 100644 pkg/nsx/services/subnetbinding/subnetbinding.go create mode 100644 pkg/nsx/services/subnetbinding/subnetbinding_test.go create mode 100644 pkg/nsx/services/subnetbinding/tree.go create mode 100644 pkg/nsx/services/subnetbinding/tree_test.go diff --git a/cmd/main.go b/cmd/main.go index 6080fb9bc..a4e81081e 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -34,12 +34,14 @@ import ( "github.com/vmware-tanzu/nsx-operator/pkg/controllers/service" staticroutecontroller "github.com/vmware-tanzu/nsx-operator/pkg/controllers/staticroute" "github.com/vmware-tanzu/nsx-operator/pkg/controllers/subnet" + subnetbindingcontroller "github.com/vmware-tanzu/nsx-operator/pkg/controllers/subnetbinding" "github.com/vmware-tanzu/nsx-operator/pkg/controllers/subnetport" "github.com/vmware-tanzu/nsx-operator/pkg/controllers/subnetset" "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/ipblocksinfo" nodeservice "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/node" "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/staticroute" subnetservice "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/subnet" + subnetbindingservice "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/subnetbinding" subnetportservice "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/subnetport" commonctl "github.com/vmware-tanzu/nsx-operator/pkg/controllers/common" @@ -221,6 +223,13 @@ func startServiceController(mgr manager.Manager, nsxClient *nsx.Client) { os.Exit(1) } ipblocksInfoService := ipblocksinfo.InitializeIPBlocksInfoService(commonService) + + subnetBindingService, err := subnetbindingservice.InitializeService(commonService) + if err != nil { + log.Error(err, "Failed to initialize SubnetConnectionBindingMap commonService") + os.Exit(1) + } + // Start controllers which only supports VPC StartNetworkInfoController(mgr, vpcService, ipblocksInfoService) StartNamespaceController(mgr, cf, vpcService) @@ -239,10 +248,10 @@ func startServiceController(mgr manager.Manager, nsxClient *nsx.Client) { } } // Start Subnet/SubnetSet controller. - if err := subnet.StartSubnetController(mgr, subnetService, subnetPortService, vpcService, hookServer); err != nil { + if err := subnet.StartSubnetController(mgr, subnetService, subnetPortService, vpcService, subnetBindingService, hookServer); err != nil { os.Exit(1) } - if err := subnetset.StartSubnetSetController(mgr, subnetService, subnetPortService, vpcService, hookServer); err != nil { + if err := subnetset.StartSubnetSetController(mgr, subnetService, subnetPortService, vpcService, subnetBindingService, hookServer); err != nil { os.Exit(1) } @@ -253,6 +262,7 @@ func startServiceController(mgr manager.Manager, nsxClient *nsx.Client) { StartIPAddressAllocationController(mgr, ipAddressAllocationService, vpcService) networkpolicycontroller.StartNetworkPolicyController(mgr, commonService, vpcService) service.StartServiceLbController(mgr, commonService) + subnetbindingcontroller.StartSubnetBindingController(mgr, subnetService, subnetBindingService) } // Start controllers which can run in non-VPC mode securitypolicycontroller.StartSecurityPolicyController(mgr, commonService, vpcService) diff --git a/pkg/apis/vpc/v1alpha1/condition_types.go b/pkg/apis/vpc/v1alpha1/condition_types.go index a7d31d45a..2cfab6fa2 100644 --- a/pkg/apis/vpc/v1alpha1/condition_types.go +++ b/pkg/apis/vpc/v1alpha1/condition_types.go @@ -12,6 +12,7 @@ const ( GatewayConnectionReady ConditionType = "GatewayConnectionReady" AutoSnatEnabled ConditionType = "AutoSnatEnabled" ExternalIPBlocksConfigured ConditionType = "ExternalIPBlocksConfigured" + DeleteFailure ConditionType = "DeletionFailed" ) // Condition defines condition of custom resource. diff --git a/pkg/clean/clean.go b/pkg/clean/clean.go index d7300640f..bca702e7f 100644 --- a/pkg/clean/clean.go +++ b/pkg/clean/clean.go @@ -17,6 +17,7 @@ import ( "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/securitypolicy" sr "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/staticroute" "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/subnetport" "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/vpc" nsxutil "github.com/vmware-tanzu/nsx-operator/pkg/nsx/util" @@ -168,9 +169,15 @@ func InitializeCleanupService(cf *config.NSXOperatorConfig, nsxClient *nsx.Clien return ipaddressallocation.InitializeIPAddressAllocation(service, vpcService, true) } } + wrapInitializeSubnetBinding := func(service common.Service) cleanupFunc { + return func() (cleanup, error) { + return subnetbinding.InitializeService(service) + } + } // TODO: initialize other CR services cleanupService = cleanupService. AddCleanupService(wrapInitializeSubnetPort(commonService)). + AddCleanupService(wrapInitializeSubnetBinding(commonService)). AddCleanupService(wrapInitializeSubnetService(commonService)). AddCleanupService(wrapInitializeSecurityPolicy(commonService)). AddCleanupService(wrapInitializeStaticRoute(commonService)). diff --git a/pkg/clean/clean_test.go b/pkg/clean/clean_test.go index cc4ff8e65..064fc33e8 100644 --- a/pkg/clean/clean_test.go +++ b/pkg/clean/clean_test.go @@ -18,6 +18,7 @@ import ( "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/securitypolicy" sr "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/staticroute" "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/subnetport" "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/vpc" ) @@ -180,11 +181,14 @@ func TestInitializeCleanupService_Success(t *testing.T) { patches.ApplyFunc(ipaddressallocation.InitializeIPAddressAllocation, func(service common.Service, vpcService common.VPCServiceProvider, flag bool) (*ipaddressallocation.IPAddressAllocationService, error) { return &ipaddressallocation.IPAddressAllocationService{}, nil }) + patches.ApplyFunc(subnetbinding.InitializeService, func(service common.Service) (*subnetbinding.BindingService, error) { + return &subnetbinding.BindingService{}, nil + }) cleanupService, err := InitializeCleanupService(cf, nsxClient) assert.NoError(t, err) assert.NotNil(t, cleanupService) - assert.Len(t, cleanupService.cleans, 6) + assert.Len(t, cleanupService.cleans, 7) } func TestInitializeCleanupService_VPCError(t *testing.T) { @@ -214,10 +218,13 @@ func TestInitializeCleanupService_VPCError(t *testing.T) { patches.ApplyFunc(ipaddressallocation.InitializeIPAddressAllocation, func(service common.Service, vpcService common.VPCServiceProvider, flag bool) (*ipaddressallocation.IPAddressAllocationService, error) { return &ipaddressallocation.IPAddressAllocationService{}, nil }) + patches.ApplyFunc(subnetbinding.InitializeService, func(service common.Service) (*subnetbinding.BindingService, error) { + return &subnetbinding.BindingService{}, nil + }) cleanupService, err := InitializeCleanupService(cf, nsxClient) assert.NoError(t, err) assert.NotNil(t, cleanupService) - assert.Len(t, cleanupService.cleans, 4) + assert.Len(t, cleanupService.cleans, 5) assert.Equal(t, expectedError, cleanupService.err) } diff --git a/pkg/controllers/common/dependency_watcher.go b/pkg/controllers/common/dependency_watcher.go new file mode 100644 index 000000000..7175da374 --- /dev/null +++ b/pkg/controllers/common/dependency_watcher.go @@ -0,0 +1,93 @@ +package common + +import ( + "context" + "fmt" + + corev1 "k8s.io/api/core/v1" + "k8s.io/client-go/util/workqueue" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1" +) + +type RequeueObjectByEvent func(ctx context.Context, c client.Client, obj client.Object, q workqueue.TypedRateLimitingInterface[reconcile.Request]) +type RequeueObjectsByUpdate func(ctx context.Context, c client.Client, objOld client.Object, objNew client.Object, q workqueue.TypedRateLimitingInterface[reconcile.Request]) + +type EnqueueRequestForDependency struct { + Client client.Client + ResourceType string + RequeueByCreate RequeueObjectByEvent + RequeueByDelete RequeueObjectByEvent + RequeueByUpdate RequeueObjectsByUpdate +} + +func (e *EnqueueRequestForDependency) Create(ctx context.Context, ev event.CreateEvent, q workqueue.TypedRateLimitingInterface[reconcile.Request]) { + obj := ev.Object + log.V(1).Info(fmt.Sprintf("%s create event", e.ResourceType), "Namespace", obj.GetNamespace(), "Name", obj.GetName()) + if e.RequeueByCreate != nil { + e.RequeueByCreate(ctx, e.Client, obj, q) + } +} + +func (e *EnqueueRequestForDependency) Delete(ctx context.Context, ev event.DeleteEvent, q workqueue.TypedRateLimitingInterface[reconcile.Request]) { + obj := ev.Object + log.V(1).Info(fmt.Sprintf("%s delete event", e.ResourceType), "Namespace", obj.GetNamespace(), "Name", obj.GetName()) + if e.RequeueByDelete != nil { + e.RequeueByDelete(ctx, e.Client, obj, q) + } +} + +func (e *EnqueueRequestForDependency) Generic(_ context.Context, _ event.GenericEvent, _ workqueue.TypedRateLimitingInterface[reconcile.Request]) { + log.V(1).Info(fmt.Sprintf("%s generic event, do nothing", e.ResourceType)) +} + +func (e *EnqueueRequestForDependency) Update(ctx context.Context, ev event.UpdateEvent, q workqueue.TypedRateLimitingInterface[reconcile.Request]) { + objNew := ev.ObjectNew + log.V(1).Info(fmt.Sprintf("%s update event", e.ResourceType), "Namespace", objNew.GetNamespace(), "Name", objNew.GetName()) + if e.RequeueByUpdate != nil { + objOld := ev.ObjectOld + e.RequeueByUpdate(ctx, e.Client, objOld, objNew, q) + } +} + +func IsObjectUpdateToReady(oldConditions []v1alpha1.Condition, newConditions []v1alpha1.Condition) bool { + return !IsObjectReady(oldConditions) && IsObjectReady(newConditions) +} + +func IsObjectReady(conditions []v1alpha1.Condition) bool { + for _, con := range conditions { + if con.Type == v1alpha1.Ready && con.Status == corev1.ConditionTrue { + return true + } + } + return false +} + +var PredicateFuncsWithBindingMapUpdateDelete = predicate.Funcs{ + UpdateFunc: func(e event.UpdateEvent) bool { + oldBindingMap, _ := e.ObjectOld.(*v1alpha1.SubnetConnectionBindingMap) + newBindingMap, _ := e.ObjectNew.(*v1alpha1.SubnetConnectionBindingMap) + if IsObjectReady(oldBindingMap.Status.Conditions) != IsObjectReady(newBindingMap.Status.Conditions) { + return true + } + if !IsObjectReady(newBindingMap.Status.Conditions) { + return false + } + if oldBindingMap.Spec.TargetSubnetSetName != newBindingMap.Spec.TargetSubnetSetName || + oldBindingMap.Spec.TargetSubnetName != newBindingMap.Spec.TargetSubnetName { + return true + } + return false + }, + CreateFunc: func(e event.CreateEvent) bool { + return false + }, + DeleteFunc: func(e event.DeleteEvent) bool { return true }, + GenericFunc: func(e event.GenericEvent) bool { + return false + }, +} diff --git a/pkg/controllers/common/dependency_watcher_test.go b/pkg/controllers/common/dependency_watcher_test.go new file mode 100644 index 000000000..5f03226aa --- /dev/null +++ b/pkg/controllers/common/dependency_watcher_test.go @@ -0,0 +1,272 @@ +package common + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/util/workqueue" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1" +) + +func TestEnqueueRequestForBindingMap(t *testing.T) { + myQueue := workqueue.NewTypedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[reconcile.Request]()) + defer myQueue.ShutDown() + + requeueByCreate := func(ctx context.Context, _ client.Client, obj client.Object, q workqueue.TypedRateLimitingInterface[reconcile.Request]) { + q.Add(reconcile.Request{NamespacedName: types.NamespacedName{Name: "create", Namespace: "default"}}) + } + requeueByUpdate := func(ctx context.Context, _ client.Client, _, _ client.Object, q workqueue.TypedRateLimitingInterface[reconcile.Request]) { + q.Add(reconcile.Request{NamespacedName: types.NamespacedName{Name: "update", Namespace: "default"}}) + } + requeueByDelete := func(ctx context.Context, _ client.Client, obj client.Object, q workqueue.TypedRateLimitingInterface[reconcile.Request]) { + q.Add(reconcile.Request{NamespacedName: types.NamespacedName{Name: "delete", Namespace: "default"}}) + } + + obj := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "test", + }, + } + obj2 := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "default", + Name: "test", + Annotations: map[string]string{ + "update": "true", + }, + }, + } + enqueueRequest := &EnqueueRequestForDependency{ + ResourceType: "create", + RequeueByCreate: requeueByCreate, + RequeueByDelete: requeueByDelete, + RequeueByUpdate: requeueByUpdate, + } + createEvent := event.CreateEvent{ + Object: obj, + } + updateEvent := event.UpdateEvent{ + ObjectOld: obj, + ObjectNew: obj2, + } + deleteEvent := event.DeleteEvent{ + Object: obj, + } + genericEvent := event.GenericEvent{ + Object: obj, + } + + ctx := context.Background() + enqueueRequest.Create(ctx, createEvent, myQueue) + require.Equal(t, 1, myQueue.Len()) + item, _ := myQueue.Get() + assert.Equal(t, "create", item.Name) + myQueue.Done(item) + + enqueueRequest.Update(ctx, updateEvent, myQueue) + require.Equal(t, 1, myQueue.Len()) + item, _ = myQueue.Get() + assert.Equal(t, "update", item.Name) + myQueue.Done(item) + + enqueueRequest.Delete(ctx, deleteEvent, myQueue) + require.Equal(t, 1, myQueue.Len()) + item, _ = myQueue.Get() + assert.Equal(t, "delete", item.Name) + myQueue.Done(item) + + enqueueRequest.Generic(ctx, genericEvent, myQueue) + require.Equal(t, 0, myQueue.Len()) +} + +func TestPredicateFuncsBindingMap(t *testing.T) { + readyBM := &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bm1", + Namespace: "default", + }, + Spec: v1alpha1.SubnetConnectionBindingMapSpec{ + VLANTrafficTag: 201, + }, + Status: v1alpha1.SubnetConnectionBindingMapStatus{ + Conditions: []v1alpha1.Condition{ + { + Type: v1alpha1.Ready, + Status: corev1.ConditionTrue, + }, + }, + }, + } + readyBM2 := &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bm1", + Namespace: "default", + }, + Spec: v1alpha1.SubnetConnectionBindingMapSpec{ + VLANTrafficTag: 202, + }, + Status: v1alpha1.SubnetConnectionBindingMapStatus{ + Conditions: []v1alpha1.Condition{ + { + Type: v1alpha1.Ready, + Status: corev1.ConditionTrue, + }, + }, + }, + } + unreadyBM := &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bm1", + Namespace: "default", + }, + Spec: v1alpha1.SubnetConnectionBindingMapSpec{ + VLANTrafficTag: 201, + }, + Status: v1alpha1.SubnetConnectionBindingMapStatus{ + Conditions: []v1alpha1.Condition{ + { + Type: v1alpha1.Ready, + Status: corev1.ConditionFalse, + }, + }, + }, + } + bmWithSubnet1 := &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bm1", + Namespace: "default", + }, + Spec: v1alpha1.SubnetConnectionBindingMapSpec{ + SubnetName: "child", + TargetSubnetName: "parent1", + }, + Status: v1alpha1.SubnetConnectionBindingMapStatus{ + Conditions: []v1alpha1.Condition{ + { + Type: v1alpha1.Ready, + Status: corev1.ConditionTrue, + }, + }, + }, + } + bmWithSubnet2 := &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bm1", + Namespace: "default", + }, + Spec: v1alpha1.SubnetConnectionBindingMapSpec{ + SubnetName: "child", + TargetSubnetName: "parent2", + }, + Status: v1alpha1.SubnetConnectionBindingMapStatus{ + Conditions: []v1alpha1.Condition{ + { + Type: v1alpha1.Ready, + Status: corev1.ConditionTrue, + }, + }, + }, + } + bmWithSubnetSet1 := &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bm1", + Namespace: "default", + }, + Spec: v1alpha1.SubnetConnectionBindingMapSpec{ + SubnetName: "child", + TargetSubnetSetName: "parent1", + }, + Status: v1alpha1.SubnetConnectionBindingMapStatus{ + Conditions: []v1alpha1.Condition{ + { + Type: v1alpha1.Ready, + Status: corev1.ConditionTrue, + }, + }, + }, + } + bmWithSubnetSet2 := &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bm1", + Namespace: "default", + }, + Spec: v1alpha1.SubnetConnectionBindingMapSpec{ + SubnetName: "child", + TargetSubnetSetName: "parent2", + }, + Status: v1alpha1.SubnetConnectionBindingMapStatus{ + Conditions: []v1alpha1.Condition{ + { + Type: v1alpha1.Ready, + Status: corev1.ConditionTrue, + }, + }, + }, + } + createEvent := event.CreateEvent{ + Object: readyBM, + } + assert.False(t, PredicateFuncsWithBindingMapUpdateDelete.CreateFunc(createEvent)) + + updateEvent1 := event.UpdateEvent{ + ObjectOld: unreadyBM, + ObjectNew: readyBM, + } + assert.True(t, PredicateFuncsWithBindingMapUpdateDelete.Update(updateEvent1)) + updateEvent2 := event.UpdateEvent{ + ObjectOld: readyBM, + ObjectNew: unreadyBM, + } + assert.True(t, PredicateFuncsWithBindingMapUpdateDelete.Update(updateEvent2)) + updateEvent3 := event.UpdateEvent{ + ObjectOld: readyBM, + ObjectNew: readyBM2, + } + assert.False(t, PredicateFuncsWithBindingMapUpdateDelete.Update(updateEvent3)) + updateEvent4 := event.UpdateEvent{ + ObjectOld: bmWithSubnet1, + ObjectNew: bmWithSubnet2, + } + assert.True(t, PredicateFuncsWithBindingMapUpdateDelete.Update(updateEvent4)) + updateEvent5 := event.UpdateEvent{ + ObjectOld: bmWithSubnetSet1, + ObjectNew: bmWithSubnetSet2, + } + assert.True(t, PredicateFuncsWithBindingMapUpdateDelete.Update(updateEvent5)) + deleteEvent := event.DeleteEvent{ + Object: readyBM, + } + assert.True(t, PredicateFuncsWithBindingMapUpdateDelete.Delete(deleteEvent)) + genericEvent := event.GenericEvent{ + Object: readyBM, + } + assert.False(t, PredicateFuncsWithBindingMapUpdateDelete.GenericFunc(genericEvent)) +} + +func TestIsObjectUpdateToReady(t *testing.T) { + oldConditions := []v1alpha1.Condition{ + { + Status: corev1.ConditionFalse, + Type: v1alpha1.Ready, + }, + } + newConditions := []v1alpha1.Condition{ + { + Status: corev1.ConditionTrue, + Type: v1alpha1.Ready, + }, + } + assert.True(t, IsObjectUpdateToReady(oldConditions, newConditions)) + assert.False(t, IsObjectUpdateToReady(newConditions, newConditions)) + assert.False(t, IsObjectUpdateToReady(newConditions, oldConditions)) +} diff --git a/pkg/controllers/common/types.go b/pkg/controllers/common/types.go index e703096b1..57d47cc19 100644 --- a/pkg/controllers/common/types.go +++ b/pkg/controllers/common/types.go @@ -8,22 +8,23 @@ import ( ) const ( - MetricResTypeSecurityPolicy = "securitypolicy" - MetricResTypeNetworkPolicy = "networkpolicy" - MetricResTypeIPPool = "ippool" - MetricResTypeIPAddressAllocation = "ipaddressallocation" - MetricResTypeNSXServiceAccount = "nsxserviceaccount" - MetricResTypeSubnetPort = "subnetport" - MetricResTypeStaticRoute = "staticroute" - MetricResTypeSubnet = "subnet" - MetricResTypeSubnetSet = "subnetset" - MetricResTypeNetworkInfo = "networkinfo" - MetricResTypeNamespace = "namespace" - MetricResTypePod = "pod" - MetricResTypeNode = "node" - MetricResTypeServiceLb = "servicelb" - MaxConcurrentReconciles = 8 - NSXOperatorError = "nsx-op/error" + MetricResTypeSecurityPolicy = "securitypolicy" + MetricResTypeNetworkPolicy = "networkpolicy" + MetricResTypeIPPool = "ippool" + MetricResTypeIPAddressAllocation = "ipaddressallocation" + MetricResTypeNSXServiceAccount = "nsxserviceaccount" + MetricResTypeSubnetPort = "subnetport" + MetricResTypeStaticRoute = "staticroute" + MetricResTypeSubnet = "subnet" + MetricResTypeSubnetSet = "subnetset" + MetricResTypeSubnetConnectionBindingMap = "subnetconnectionbindingmap" + MetricResTypeNetworkInfo = "networkinfo" + MetricResTypeNamespace = "namespace" + MetricResTypePod = "pod" + MetricResTypeNode = "node" + MetricResTypeServiceLb = "servicelb" + MaxConcurrentReconciles = 8 + NSXOperatorError = "nsx-op/error" //sync the error with NCP side ErrorNoDFWLicense = "NO_DFW_LICENSE" diff --git a/pkg/controllers/subnet/subnet_controller.go b/pkg/controllers/subnet/subnet_controller.go index f37126550..eac21962c 100644 --- a/pkg/controllers/subnet/subnet_controller.go +++ b/pkg/controllers/subnet/subnet_controller.go @@ -18,6 +18,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" @@ -26,6 +27,7 @@ import ( "github.com/vmware-tanzu/nsx-operator/pkg/logger" 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" nsxutil "github.com/vmware-tanzu/nsx-operator/pkg/nsx/util" ) @@ -36,6 +38,11 @@ var ( ResultRequeueAfter5mins = common.ResultRequeueAfter5mins ResultRequeueAfter10sec = common.ResultRequeueAfter10sec MetricResTypeSubnet = common.MetricResTypeSubnet + + msgFailAddFinalizer = "Failed to add finalizer on Subnet for the dependency by SubnetConnectionBindingMap %s" + msgFailDelFinalizer = "Failed to remove finalizer on Subnet after no the dependency by SubnetConnectionBindingMaps" + msgDeleteInUse = "Subnet is used by SubnetConnectionBindingMap %s and not able to delete" + reasonDeleteInUse = "SubnetInUse" ) // SubnetReconciler reconciles a SubnetSet object @@ -45,6 +52,7 @@ type SubnetReconciler struct { SubnetService *subnet.SubnetService SubnetPortService servicecommon.SubnetPortServiceProvider VPCService servicecommon.VPCServiceProvider + BindingService *subnetbinding.BindingService Recorder record.EventRecorder StatusUpdater common.StatusUpdater } @@ -70,8 +78,37 @@ func (r *SubnetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr log.Error(err, "Unable to fetch Subnet CR", "req", req.NamespacedName) return ResultRequeue, err } + + hasBindings := r.subnetHasBindings(string(subnetCR.UID)) + if len(hasBindings) > 0 { + if !controllerutil.ContainsFinalizer(subnetCR, servicecommon.SubnetFinalizerName) { + controllerutil.AddFinalizer(subnetCR, servicecommon.SubnetFinalizerName) + if err := r.Client.Update(ctx, subnetCR); err != nil { + log.Error(err, "Failed to add finalizer", "subnet", req.NamespacedName) + r.StatusUpdater.UpdateFail(ctx, subnetCR, err, "Failed to add finalizer on Subnet used by SubnetConnectionBindingMaps", setSubnetReadyStatusFalse, fmt.Sprintf(msgFailAddFinalizer, hasBindings[0].Name)) + return ResultRequeue, err + } + } + } else { + if controllerutil.ContainsFinalizer(subnetCR, servicecommon.SubnetFinalizerName) { + controllerutil.RemoveFinalizer(subnetCR, servicecommon.SubnetFinalizerName) + if err := r.Client.Update(ctx, subnetCR); err != nil { + log.Error(err, "Failed to delete finalizer", "subnet", req.NamespacedName) + r.StatusUpdater.UpdateFail(ctx, subnetCR, err, "Failed to delete finalizer from Subnet", setSubnetReadyStatusFalse, fmt.Sprint(msgFailDelFinalizer)) + return ResultRequeue, err + } + } + } + if !subnetCR.DeletionTimestamp.IsZero() { r.StatusUpdater.IncreaseDeleteTotal() + if len(hasBindings) > 0 { + err := fmt.Errorf("subnet %s is used by SubnetConnectionBindingMaps %s", req.String(), hasBindings[0].GetName()) + log.Error(err, "Failed to delete Subnet CR because it is used by SubnetConnectionBindingMaps, retrying", "Subnet", req.NamespacedName) + r.setSubnetDeletionFailedStatus(ctx, subnetCR, metav1.Now(), fmt.Sprintf(msgDeleteInUse, hasBindings[0].GetName()), reasonDeleteInUse) + r.StatusUpdater.DeleteFail(req.NamespacedName, nil, err) + return ResultRequeue, err + } if err := r.deleteSubnetByID(string(subnetCR.GetUID())); err != nil { r.StatusUpdater.DeleteFail(req.NamespacedName, nil, err) return ResultRequeue, err @@ -248,6 +285,25 @@ func setSubnetReadyStatusFalse(client client.Client, ctx context.Context, obj cl updateSubnetStatusConditions(client, ctx, subnet, newConditions) } +func (r *SubnetReconciler) setSubnetDeletionFailedStatus(ctx context.Context, subnet *v1alpha1.Subnet, transitionTime metav1.Time, msg string, reason string) { + newConditions := []v1alpha1.Condition{ + { + Type: v1alpha1.DeleteFailure, + Status: v1.ConditionTrue, + Message: "Subnet could not be deleted", + Reason: "NSXOperationFailed", + LastTransitionTime: transitionTime, + }, + } + if msg != "" { + newConditions[0].Message = msg + } + if reason != "" { + newConditions[0].Reason = reason + } + updateSubnetStatusConditions(r.Client, ctx, subnet, newConditions) +} + func updateSubnetStatusConditions(client client.Client, ctx context.Context, subnet *v1alpha1.Subnet, newConditions []v1alpha1.Condition) { conditionsUpdated := false for i := range newConditions { @@ -291,7 +347,7 @@ func getExistingConditionOfType(conditionType v1alpha1.ConditionType, existingCo return nil } -func StartSubnetController(mgr ctrl.Manager, subnetService *subnet.SubnetService, subnetPortService servicecommon.SubnetPortServiceProvider, vpcService servicecommon.VPCServiceProvider, hookServer webhook.Server) error { +func StartSubnetController(mgr ctrl.Manager, subnetService *subnet.SubnetService, subnetPortService servicecommon.SubnetPortServiceProvider, vpcService servicecommon.VPCServiceProvider, bindingService *subnetbinding.BindingService, hookServer webhook.Server) error { // Create the Subnet Reconciler with the necessary services and configuration subnetReconciler := &SubnetReconciler{ Client: mgr.GetClient(), @@ -299,6 +355,7 @@ func StartSubnetController(mgr ctrl.Manager, subnetService *subnet.SubnetService SubnetService: subnetService, SubnetPortService: subnetPortService, VPCService: vpcService, + BindingService: bindingService, Recorder: mgr.GetEventRecorderFor("subnet-controller"), } subnetReconciler.StatusUpdater = common.NewStatusUpdater(subnetReconciler.Client, subnetReconciler.SubnetService.NSXConfig, subnetReconciler.Recorder, MetricResTypeSubnet, "Subnet", "Subnet") @@ -340,6 +397,16 @@ func (r *SubnetReconciler) setupWithManager(mgr ctrl.Manager) error { &EnqueueRequestForNamespace{Client: mgr.GetClient()}, builder.WithPredicates(PredicateFuncsNs), ). + Watches( + &v1alpha1.SubnetConnectionBindingMap{}, + &common.EnqueueRequestForDependency{ + Client: r.Client, + ResourceType: "SubnetConnectionBindingMap", + RequeueByUpdate: requeueSubnetByBindingMapUpdate, + RequeueByDelete: requeueSubnetByBindingMapDelete, + }, + builder.WithPredicates(common.PredicateFuncsWithBindingMapUpdateDelete), + ). // Set controller options, including max concurrent reconciles WithOptions( controller.Options{ diff --git a/pkg/controllers/subnet/subnet_controller_test.go b/pkg/controllers/subnet/subnet_controller_test.go index 53fb64db5..008026885 100644 --- a/pkg/controllers/subnet/subnet_controller_test.go +++ b/pkg/controllers/subnet/subnet_controller_test.go @@ -30,6 +30,7 @@ import ( "github.com/vmware-tanzu/nsx-operator/pkg/nsx" "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/subnetport" "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/vpc" ) @@ -370,6 +371,9 @@ func TestSubnetReconciler_Reconcile(t *testing.T) { patches := gomonkey.ApplyPrivateMethod(reflect.TypeOf(r), "deleteSubnetByID", func(_ *SubnetReconciler, _ string) error { return nil }) + patches.ApplyPrivateMethod(reflect.TypeOf(r), "subnetHasBindings", func(_ *SubnetReconciler, subnetSetCRUID string) []*v1alpha1.SubnetConnectionBindingMap { + return []*v1alpha1.SubnetConnectionBindingMap{} + }) return patches }, expectRes: ResultNormal, @@ -399,6 +403,9 @@ func TestSubnetReconciler_Reconcile(t *testing.T) { &vpcSubnetSkip, &vpcSubnetDelete, } }) + patches.ApplyPrivateMethod(reflect.TypeOf(r), "subnetHasBindings", func(_ *SubnetReconciler, subnetSetCRUID string) []*v1alpha1.SubnetConnectionBindingMap { + return []*v1alpha1.SubnetConnectionBindingMap{} + }) patches.ApplyMethod(reflect.TypeOf(r.SubnetPortService), "GetPortsOfSubnet", func(_ *subnetport.SubnetPortService, _ string) (ports []*model.VpcSubnetPort) { return nil }) @@ -434,6 +441,9 @@ func TestSubnetReconciler_Reconcile(t *testing.T) { &vpcSubnetSkip, &vpcSubnetDelete, } }) + patches.ApplyPrivateMethod(reflect.TypeOf(r), "subnetHasBindings", func(_ *SubnetReconciler, subnetSetCRUID string) []*v1alpha1.SubnetConnectionBindingMap { + return []*v1alpha1.SubnetConnectionBindingMap{} + }) patches.ApplyMethod(reflect.TypeOf(r.SubnetPortService), "GetPortsOfSubnet", func(_ *subnetport.SubnetPortService, _ string) (ports []*model.VpcSubnetPort) { return nil }) @@ -455,6 +465,10 @@ func TestSubnetReconciler_Reconcile(t *testing.T) { return vpcConfig }) + patches.ApplyPrivateMethod(reflect.TypeOf(r), "subnetHasBindings", func(_ *SubnetReconciler, subnetSetCRUID string) []*v1alpha1.SubnetConnectionBindingMap { + return []*v1alpha1.SubnetConnectionBindingMap{} + }) + tags := []model.Tag{{Scope: common.String(common.TagScopeSubnetCRUID), Tag: common.String("fake-tag")}} patches.ApplyMethod(reflect.TypeOf(r.SubnetService), "GenerateSubnetNSTags", func(_ *subnet.SubnetService, obj client.Object) []model.Tag { return tags @@ -483,6 +497,10 @@ func TestSubnetReconciler_Reconcile(t *testing.T) { return vpcConfig }) + patches.ApplyPrivateMethod(reflect.TypeOf(r), "subnetHasBindings", func(_ *SubnetReconciler, subnetSetCRUID string) []*v1alpha1.SubnetConnectionBindingMap { + return []*v1alpha1.SubnetConnectionBindingMap{} + }) + tags := []model.Tag{{Scope: common.String(common.TagScopeSubnetCRUID), Tag: common.String("fake-tag")}} patches.ApplyMethod(reflect.TypeOf(r.SubnetService), "GenerateSubnetNSTags", func(_ *subnet.SubnetService, obj client.Object) []model.Tag { return tags @@ -533,6 +551,10 @@ func TestSubnetReconciler_Reconcile(t *testing.T) { return vpcConfig }) + patches.ApplyPrivateMethod(reflect.TypeOf(r), "subnetHasBindings", func(_ *SubnetReconciler, subnetSetCRUID string) []*v1alpha1.SubnetConnectionBindingMap { + return []*v1alpha1.SubnetConnectionBindingMap{} + }) + tags := []model.Tag{{Scope: common.String(common.TagScopeSubnetCRUID), Tag: common.String("fake-tag")}} patches.ApplyMethod(reflect.TypeOf(r.SubnetService), "GenerateSubnetNSTags", func(_ *subnet.SubnetService, obj client.Object) []model.Tag { return tags @@ -568,6 +590,9 @@ func TestSubnetReconciler_Reconcile(t *testing.T) { patches := gomonkey.ApplyPrivateMethod(reflect.TypeOf(r.VPCService), "GetVPCNetworkConfigByNamespace", func(_ *vpc.VPCService, ns string) *common.VPCNetworkConfigInfo { return nil }) + patches.ApplyPrivateMethod(reflect.TypeOf(r), "subnetHasBindings", func(_ *SubnetReconciler, subnetSetCRUID string) []*v1alpha1.SubnetConnectionBindingMap { + return []*v1alpha1.SubnetConnectionBindingMap{} + }) return patches }, existingSubnetCR: createNewSubnet(), @@ -583,6 +608,10 @@ func TestSubnetReconciler_Reconcile(t *testing.T) { return vpcConfig }) + patches.ApplyPrivateMethod(reflect.TypeOf(r), "subnetHasBindings", func(_ *SubnetReconciler, subnetSetCRUID string) []*v1alpha1.SubnetConnectionBindingMap { + return []*v1alpha1.SubnetConnectionBindingMap{} + }) + patches.ApplyMethod(reflect.TypeOf(r.SubnetService), "GenerateSubnetNSTags", func(_ *subnet.SubnetService, obj client.Object) []model.Tag { return nil }) @@ -601,6 +630,10 @@ func TestSubnetReconciler_Reconcile(t *testing.T) { return vpcConfig }) + patches.ApplyPrivateMethod(reflect.TypeOf(r), "subnetHasBindings", func(_ *SubnetReconciler, subnetSetCRUID string) []*v1alpha1.SubnetConnectionBindingMap { + return []*v1alpha1.SubnetConnectionBindingMap{} + }) + tags := []model.Tag{{Scope: common.String(common.TagScopeSubnetCRUID), Tag: common.String("fake-tag")}} patches.ApplyMethod(reflect.TypeOf(r.SubnetService), "GenerateSubnetNSTags", func(_ *subnet.SubnetService, obj client.Object) []model.Tag { return tags @@ -688,6 +721,10 @@ func TestStartSubnetController(t *testing.T) { Service: common.Service{}, SubnetPortStore: nil, } + bindingService := &subnetbinding.BindingService{ + Service: common.Service{}, + BindingStore: subnetbinding.SetupStore(), + } mockMgr := &MockManager{scheme: runtime.NewScheme()} @@ -735,7 +772,7 @@ func TestStartSubnetController(t *testing.T) { patches := testCase.patches() defer patches.Reset() - err := StartSubnetController(mockMgr, subnetService, subnetPortService, vpcService, nil) + err := StartSubnetController(mockMgr, subnetService, subnetPortService, vpcService, bindingService, nil) if testCase.expectErrStr != "" { assert.ErrorContains(t, err, testCase.expectErrStr) diff --git a/pkg/controllers/subnet/subnetbinding_handler.go b/pkg/controllers/subnet/subnetbinding_handler.go new file mode 100644 index 000000000..c26131bcb --- /dev/null +++ b/pkg/controllers/subnet/subnetbinding_handler.go @@ -0,0 +1,108 @@ +package subnet + +import ( + "context" + "time" + + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/util/workqueue" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1" + "github.com/vmware-tanzu/nsx-operator/pkg/controllers/common" + servicecommon "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/common" +) + +var ( + removeFinalizerDelay = 1 * time.Second +) + +func requeueSubnetByBindingMapUpdate(ctx context.Context, c client.Client, objOld client.Object, objNew client.Object, q workqueue.TypedRateLimitingInterface[reconcile.Request]) { + bindingMap := objNew.(*v1alpha1.SubnetConnectionBindingMap) + needFinalizer := common.IsObjectReady(bindingMap.Status.Conditions) + enqueueSubnets(ctx, c, bindingMap, needFinalizer, q) + + oldBM := objOld.(*v1alpha1.SubnetConnectionBindingMap) + if common.IsObjectReady(oldBM.Status.Conditions) && oldBM.Spec.TargetSubnetName != "" && + oldBM.Spec.TargetSubnetName != bindingMap.Spec.TargetSubnetName { + // Remove the finalizer from old target Subnet if it is not used. + err := enqueue(ctx, c, bindingMap.Namespace, oldBM.Spec.TargetSubnetName, false, q) + if err != nil { + log.Error(err, "Failed to requeue Subnet", "Namespace", bindingMap.Namespace, "Name", oldBM.Spec.SubnetName) + return + } + } +} + +func enqueueSubnets(ctx context.Context, c client.Client, bindingMap *v1alpha1.SubnetConnectionBindingMap, needFinalizer bool, q workqueue.TypedRateLimitingInterface[reconcile.Request]) { + err := enqueue(ctx, c, bindingMap.Namespace, bindingMap.Spec.SubnetName, needFinalizer, q) + if err != nil { + log.Error(err, "Failed to requeue Subnet", "Namespace", bindingMap.Namespace, "Name", bindingMap.Spec.SubnetName) + return + } + + if bindingMap.Spec.TargetSubnetName == "" { + return + } + + err = enqueue(ctx, c, bindingMap.Namespace, bindingMap.Spec.TargetSubnetName, needFinalizer, q) + if err != nil { + log.Error(err, "Failed to requeue Subnet", "Namespace", bindingMap.Namespace, "Name", bindingMap.Spec.TargetSubnetName) + return + } +} + +func enqueue(ctx context.Context, c client.Client, namespace, name string, needFinalizer bool, q workqueue.TypedRateLimitingInterface[reconcile.Request]) error { + subnetCR := &v1alpha1.Subnet{} + subnetKey := types.NamespacedName{Namespace: namespace, Name: name} + err := c.Get(ctx, subnetKey, subnetCR) + if err != nil { + log.Error(err, "Failed to get Subnet CR", "key", subnetKey.String()) + return err + } + addedFinalizer := controllerutil.ContainsFinalizer(subnetCR, servicecommon.SubnetFinalizerName) + if addedFinalizer != needFinalizer { + log.V(1).Info("Requeue subnet", "key", subnetKey.String()) + req := reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: name, + Namespace: namespace, + }, + } + if needFinalizer { + q.Add(req) + } else { + // Enqueue the Subnet request with 1s delay with the SubnetConnectionBindingMap deletion event. + // This is because when Subnet controller watches the deletion event at the same time as + // SubnetConnectionBindingMap Controller, and it may cause a wrong result of the Subnet dependency + // calculation. So we leave 1s to ensure the Subnet Controller reconciles Subnet status after + // the binding calculation is completed. + q.AddAfter(req, removeFinalizerDelay) + } + } + return nil +} + +func requeueSubnetByBindingMapDelete(ctx context.Context, c client.Client, obj client.Object, q workqueue.TypedRateLimitingInterface[reconcile.Request]) { + bindingMap := obj.(*v1alpha1.SubnetConnectionBindingMap) + enqueueSubnets(ctx, c, bindingMap, false, q) +} + +func (r *SubnetReconciler) subnetHasBindings(subnetCRUID string) []*v1alpha1.SubnetConnectionBindingMap { + vpcSubnets := r.SubnetService.ListSubnetCreatedBySubnet(subnetCRUID) + if len(vpcSubnets) == 0 { + log.V(1).Info("No VpcSubnet found with Subnet", "SubnetID", subnetCRUID) + return nil + } + + bindingMaps := make([]*v1alpha1.SubnetConnectionBindingMap, 0) + for _, vpcSubnet := range vpcSubnets { + bindings := r.BindingService.GetSubnetConnectionBindingMapCRsBySubnet(vpcSubnet) + if len(bindings) > 0 { + bindingMaps = append(bindingMaps, bindings...) + } + } + return bindingMaps +} diff --git a/pkg/controllers/subnet/subnetbinding_handler_test.go b/pkg/controllers/subnet/subnetbinding_handler_test.go new file mode 100644 index 000000000..ee7f93c82 --- /dev/null +++ b/pkg/controllers/subnet/subnetbinding_handler_test.go @@ -0,0 +1,170 @@ +package subnet + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/util/workqueue" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1" + servicecommon "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/common" +) + +var ( + bm1 = &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "binding1", + Namespace: "default", + }, + Spec: v1alpha1.SubnetConnectionBindingMapSpec{ + SubnetName: "child", + TargetSubnetName: "parent", + VLANTrafficTag: 101, + }, + Status: v1alpha1.SubnetConnectionBindingMapStatus{ + Conditions: []v1alpha1.Condition{ + { + Type: v1alpha1.Ready, + Status: corev1.ConditionTrue, + }, + }, + }, + } + + bm2 = &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "binding1", + Namespace: "default", + }, + Spec: v1alpha1.SubnetConnectionBindingMapSpec{ + SubnetName: "child2", + TargetSubnetSetName: "parentSet", + VLANTrafficTag: 102, + }, + Status: v1alpha1.SubnetConnectionBindingMapStatus{ + Conditions: []v1alpha1.Condition{ + { + Type: v1alpha1.Ready, + Status: corev1.ConditionFalse, + }, + }, + }, + } + + bm3 = &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "binding1", + Namespace: "default", + }, + Spec: v1alpha1.SubnetConnectionBindingMapSpec{ + SubnetName: "child", + TargetSubnetName: "parent2", + VLANTrafficTag: 101, + }, + Status: v1alpha1.SubnetConnectionBindingMapStatus{ + Conditions: []v1alpha1.Condition{ + { + Type: v1alpha1.Ready, + Status: corev1.ConditionTrue, + }, + }, + }, + } + + subnet1 = &v1alpha1.Subnet{ + ObjectMeta: metav1.ObjectMeta{Name: "child", Namespace: "default"}, + } + subnet2 = &v1alpha1.Subnet{ + ObjectMeta: metav1.ObjectMeta{Name: "parent", Namespace: "default", Finalizers: []string{servicecommon.SubnetFinalizerName}}, + } + subnet3 = &v1alpha1.Subnet{ + ObjectMeta: metav1.ObjectMeta{Name: "child2", Namespace: "default", Finalizers: []string{servicecommon.SubnetFinalizerName}}, + } + subnet4 = &v1alpha1.Subnet{ + ObjectMeta: metav1.ObjectMeta{Name: "parent2", Namespace: "default", Finalizers: []string{servicecommon.SubnetFinalizerName}}, + } + req1 = reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: "child", + Namespace: "default", + }, + } + req2 = reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: "parent", + Namespace: "default", + }, + } + req3 = reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: "child2", + Namespace: "default", + }, + } + req4 = reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: "parent2", + Namespace: "default", + }, + } +) + +func TestRequeueSubnetByBindingMap(t *testing.T) { + oriRemoveFinalizerDelay := removeFinalizerDelay + removeFinalizerDelay = -1 + t.Cleanup(func() { + removeFinalizerDelay = oriRemoveFinalizerDelay + }) + + myQueue := workqueue.NewTypedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[reconcile.Request]()) + defer myQueue.ShutDown() + + ctx := context.TODO() + newScheme := runtime.NewScheme() + utilruntime.Must(clientgoscheme.AddToScheme(newScheme)) + utilruntime.Must(v1alpha1.AddToScheme(newScheme)) + + fakeClient := fake.NewClientBuilder().WithScheme(newScheme).WithObjects(subnet1, subnet2, subnet3, subnet4).Build() + + requeueSubnetByBindingMapUpdate(ctx, fakeClient, bm1, bm1, myQueue) + require.Equal(t, 1, myQueue.Len()) + item, _ := myQueue.Get() + assert.Equal(t, req1, item) + myQueue.Done(item) + requeueSubnetByBindingMapUpdate(ctx, fakeClient, bm3, bm1, myQueue) + require.Equal(t, 2, myQueue.Len()) + item, _ = myQueue.Get() + assert.Equal(t, req1, item) + myQueue.Done(item) + item, _ = myQueue.Get() + assert.Equal(t, req4, item) + myQueue.Done(item) + + requeueSubnetByBindingMapUpdate(ctx, fakeClient, bm2, bm2, myQueue) + require.Equal(t, 1, myQueue.Len()) + item, _ = myQueue.Get() + assert.Equal(t, req3, item) + myQueue.Done(item) + + requeueSubnetByBindingMapDelete(ctx, fakeClient, bm1, myQueue) + require.Equal(t, 1, myQueue.Len()) + item, _ = myQueue.Get() + assert.Equal(t, req2, item) + myQueue.Done(item) + + requeueSubnetByBindingMapDelete(ctx, fakeClient, bm2, myQueue) + require.Equal(t, 1, myQueue.Len()) + item, _ = myQueue.Get() + assert.Equal(t, req3, item) + myQueue.Done(item) +} diff --git a/pkg/controllers/subnetbinding/controller.go b/pkg/controllers/subnetbinding/controller.go new file mode 100644 index 000000000..9dcddafcc --- /dev/null +++ b/pkg/controllers/subnetbinding/controller.go @@ -0,0 +1,333 @@ +package subnetbinding + +import ( + "context" + "fmt" + "os" + "reflect" + "time" + + "github.com/vmware/vsphere-automation-sdk-go/services/nsxt/model" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/sets" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/predicate" + + "github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1" + "github.com/vmware-tanzu/nsx-operator/pkg/controllers/common" + "github.com/vmware-tanzu/nsx-operator/pkg/logger" + 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" +) + +var ( + log = &logger.Log + MetricResTypeSubnetConnectionBindingMap = common.MetricResTypeSubnetConnectionBindingMap + + ResultNormal = common.ResultNormal + ResultRequeue = common.ResultRequeue + ResultRequeueAfter10sec = common.ResultRequeueAfter10sec + + reasonDependencyNotReady = "DependencyNotReady" + reasonConfigureFailure = "ConfigureFailed" + msgGetSubnetCR = "Unable to get Subnet CR %s" + msgGetSubnetSetCR = "Unable to get SubnetSet CR %s" + msgGetNSXSubnetsBySubnet = "Subnet CR %s is not realized on NSX" + msgGetNSXSubnetsBySubnetSet = "SubnetSet CR %s is not realized on NSX" + msgChildWorkAsParent = "Subnet CR %s is working as target by %s" + msgParentWorkAsChild = "Target Subnet CR %s is attached by %s" + msgRealizeSubnetBinding = "Failed to realize SubnetConnectionBindingMap %s on NSX" +) + +// Reconciler reconciles a SubnetConnectionBindingMap object +type Reconciler struct { + Client client.Client + Scheme *runtime.Scheme + SubnetService *subnet.SubnetService + SubnetBindingService *subnetbinding.BindingService + StatusUpdater common.StatusUpdater +} + +func StartSubnetBindingController(mgr ctrl.Manager, subnetService *subnet.SubnetService, subnetBindingService *subnetbinding.BindingService) { + reconciler := newReconciler(mgr, subnetService, subnetBindingService) + // Start the controller + if err := reconciler.setupWithManager(mgr); err != nil { + log.Error(err, "Failed to create controller", "controller", "SubnetConnectionBindingMap") + os.Exit(1) + } + // Start garbage collector in a separate goroutine + go common.GenericGarbageCollector(make(chan bool), servicecommon.GCInterval, reconciler.CollectGarbage) +} + +func newReconciler(mgr ctrl.Manager, subnetService *subnet.SubnetService, subnetBindingService *subnetbinding.BindingService) *Reconciler { + recorder := mgr.GetEventRecorderFor("subnetconnectionbindingmap-controller") + // Create the SubnetConnectionBindingMap Reconciler with the necessary services and configuration + return &Reconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + SubnetService: subnetService, + SubnetBindingService: subnetBindingService, + StatusUpdater: common.NewStatusUpdater(mgr.GetClient(), subnetBindingService.NSXConfig, recorder, MetricResTypeSubnetConnectionBindingMap, "SubnetConnectionBindingMap", "SubnetConnectionBindingMap"), + } +} + +func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + startTime := time.Now() + defer func() { + log.Info("Finished reconciling SubnetConnectionBindingMap", "SubnetConnectionBindingMap", req.NamespacedName, "duration(ms)", time.Since(startTime).Milliseconds()) + }() + + r.StatusUpdater.IncreaseSyncTotal() + + bindingMapCR := &v1alpha1.SubnetConnectionBindingMap{} + if err := r.Client.Get(ctx, req.NamespacedName, bindingMapCR); err != nil { + if apierrors.IsNotFound(err) { + // Try to delete stale NSX SubnetConnectionBindingMaps if exists + if err := r.SubnetBindingService.DeleteSubnetConnectionBindingMapsByCRName(req.Name, req.Namespace); err != nil { + log.Error(err, "Failed to delete NSX SubnetConnectionBindingMap", "SubnetConnectionBindingMap", req.NamespacedName) + return ResultRequeue, nil + } + return ResultNormal, nil + } + log.Error(err, "Unable to fetch SubnetConnectionBindingMap CR", "SubnetConnectionBindingMap", req.NamespacedName) + return ResultRequeue, nil + } + + // Delete SubnetConnectionBindingMap. + if !bindingMapCR.DeletionTimestamp.IsZero() { + r.StatusUpdater.IncreaseDeleteTotal() + if err := r.SubnetBindingService.DeleteSubnetConnectionBindingMapsByCRUID(string(bindingMapCR.UID)); err != nil { + r.StatusUpdater.DeleteFail(req.NamespacedName, bindingMapCR, err) + return ResultRequeue, nil + } + r.StatusUpdater.DeleteSuccess(req.NamespacedName, bindingMapCR) + return ResultNormal, nil + } + + // Create or update SubnetConnectionBindingMap + r.StatusUpdater.IncreaseUpdateTotal() + childSubnet, parentSubnets, msg, err := r.validateDependency(ctx, bindingMapCR) + if err != nil { + // Update SubnetConnectionBindingMap with not-ready condition + r.StatusUpdater.UpdateFail(ctx, bindingMapCR, err, "dependent Subnets are not ready", updateBindingMapStatusWithUnreadyCondition, reasonDependencyNotReady, msg) + return ResultRequeueAfter10sec, nil + } + + if err = r.SubnetBindingService.CreateOrUpdateSubnetConnectionBindingMap(bindingMapCR, childSubnet, parentSubnets); err != nil { + // Update SubnetConnectionBindingMap with not-ready condition + r.StatusUpdater.UpdateFail(ctx, bindingMapCR, err, "failure to configure SubnetConnectionBindingMaps on NSX", updateBindingMapStatusWithUnreadyCondition, reasonConfigureFailure, fmt.Sprintf(msgRealizeSubnetBinding, req.Name)) + return ResultRequeue, nil + } + // Update SubnetConnectionBindingMap with ready condition + r.StatusUpdater.UpdateSuccess(ctx, bindingMapCR, updateBindingMapStatusWithReadyCondition) + return ResultNormal, nil +} + +// CollectGarbage collects the stale SubnetConnectionBindingMaps and deletes them on NSX which has been removed from k8s, +// it implements the interface GarbageCollector method. +func (r *Reconciler) CollectGarbage(ctx context.Context) { + startTime := time.Now() + defer func() { + log.Info("SubnetConnectionBindingMap garbage collection completed", "duration(ms)", time.Since(startTime).Milliseconds()) + }() + + bindingMapIdSetByCRs, err := r.listBindingMapIDsFromCRs(ctx) + if err != nil { + log.Error(err, "Failed to list SubnetConnectionBindingMap CRs") + return + } + bindingMapIdSetInStore := r.SubnetBindingService.ListSubnetConnectionBindingMapCRUIDsInStore() + + if err = r.SubnetBindingService.DeleteMultiSubnetConnectionBindingMapsByCRs(bindingMapIdSetInStore.Difference(bindingMapIdSetByCRs)); err != nil { + log.Error(err, "Failed to delete stale SubnetConnectionBindingMaps") + } +} + +var PredicateFuncsBindingMaps = predicate.Funcs{ + UpdateFunc: func(e event.UpdateEvent) bool { + oldBindingMap, okOld := e.ObjectOld.(*v1alpha1.SubnetConnectionBindingMap) + newBindingMap, okNew := e.ObjectNew.(*v1alpha1.SubnetConnectionBindingMap) + if !okOld || !okNew { + return true + } + if !reflect.DeepEqual(oldBindingMap.Spec, newBindingMap.Spec) { + return true + } + return false + }, + CreateFunc: func(e event.CreateEvent) bool { + return true + }, + DeleteFunc: func(e event.DeleteEvent) bool { return true }, + GenericFunc: func(e event.GenericEvent) bool { + return false + }, +} + +func (r *Reconciler) setupWithManager(mgr ctrl.Manager) error { + return ctrl.NewControllerManagedBy(mgr). + For(&v1alpha1.SubnetConnectionBindingMap{}). + WithEventFilter(PredicateFuncsBindingMaps). + WithOptions(controller.Options{ + MaxConcurrentReconciles: common.NumReconcile(), + }). + Watches( + &v1alpha1.Subnet{}, + &common.EnqueueRequestForDependency{ + Client: r.Client, + RequeueByDelete: requeueBindingMapsBySubnetDelete, + RequeueByUpdate: requeueBindingMapsBySubnetUpdate, + ResourceType: "Subnet"}, + builder.WithPredicates(PredicateFuncsSubnets), + ). + Watches( + &v1alpha1.SubnetSet{}, + &common.EnqueueRequestForDependency{ + Client: r.Client, + RequeueByDelete: requeueBindingMapsBySubnetSetDelete, + RequeueByUpdate: requeueBindingMapsBySubnetSetUpdate, + ResourceType: "SubnetSet"}, + builder.WithPredicates(PredicateFuncsSubnetSets), + ). + Complete(r) +} + +func (r *Reconciler) listBindingMapIDsFromCRs(ctx context.Context) (sets.Set[string], error) { + bmIDs := sets.New[string]() + connectionBindingMapList := &v1alpha1.SubnetConnectionBindingMapList{} + err := r.Client.List(ctx, connectionBindingMapList) + if err != nil { + return nil, err + } + for _, bm := range connectionBindingMapList.Items { + bmIDs.Insert(string(bm.UID)) + } + return bmIDs, nil +} + +func (r *Reconciler) validateDependency(ctx context.Context, bindingMap *v1alpha1.SubnetConnectionBindingMap) (*model.VpcSubnet, []*model.VpcSubnet, string, error) { + childSubnets, msg, err := r.validateVpcSubnetsBySubnetCR(ctx, bindingMap.Namespace, bindingMap.Spec.SubnetName, false) + if err != nil { + return nil, nil, msg, err + } + childSubnet := childSubnets[0] + + if bindingMap.Spec.TargetSubnetName != "" { + parentSubnets, msg, err := r.validateVpcSubnetsBySubnetCR(ctx, bindingMap.Namespace, bindingMap.Spec.TargetSubnetName, true) + if err != nil { + return nil, nil, msg, err + } + return childSubnet, parentSubnets, "", nil + } + + parentSubnets, msg, err := r.validateVpcSubnetsBySubnetSetCR(ctx, bindingMap.Namespace, bindingMap.Spec.TargetSubnetSetName) + if err != nil { + return nil, nil, msg, err + } + return childSubnet, parentSubnets, "", nil +} + +func (r *Reconciler) validateVpcSubnetsBySubnetCR(ctx context.Context, namespace, name string, isTarget bool) ([]*model.VpcSubnet, string, error) { + subnetCR := &v1alpha1.Subnet{} + subnetKey := types.NamespacedName{Namespace: namespace, Name: name} + // Check the Subnet CR existence. + err := r.Client.Get(ctx, subnetKey, subnetCR) + if err != nil { + log.Error(err, "Failed to get Subnet CR", "key", subnetKey.String()) + return nil, fmt.Sprintf(msgGetSubnetCR, name), fmt.Errorf("failed to get subnet %s in Namespace %s with error: %v", name, namespace, err) + } + + // Check the Subnet CR realization. + subnets := r.SubnetService.ListSubnetCreatedBySubnet(string(subnetCR.UID)) + if len(subnets) == 0 { + log.Info("NSX VpcSubnets by subnet CR do not exist", "key", subnetKey.String()) + return nil, fmt.Sprintf(msgGetNSXSubnetsBySubnet, name), fmt.Errorf("not found NSX VpcSubnets created by Subnet CR '%s/%s'", namespace, name) + } + + // Check if the Subnet CR is nested. + if !isTarget { + bms := r.SubnetBindingService.GetSubnetConnectionBindingMapsByParentSubnet(subnets[0]) + if len(bms) > 0 { + dependency := r.SubnetBindingService.GetCRNameBySubnetConnectionBindingMap(bms[0]) + msg := fmt.Sprintf(msgChildWorkAsParent, name, dependency) + return nil, msg, fmt.Errorf("Subnet %s already works as target in SegmentConnectionBindingMap %s", name, dependency) + } + } else { + bms := r.SubnetBindingService.GetSubnetConnectionBindingMapsByChildSubnet(subnets[0]) + if len(bms) > 0 { + dependency := r.SubnetBindingService.GetCRNameBySubnetConnectionBindingMap(bms[0]) + msg := fmt.Sprintf(msgParentWorkAsChild, name, dependency) + return nil, msg, fmt.Errorf("target Subnet %s is already attached by SegmentConnectionBindingMap %s", name, dependency) + } + } + + return subnets, "", nil +} + +func (r *Reconciler) validateVpcSubnetsBySubnetSetCR(ctx context.Context, namespace, name string) ([]*model.VpcSubnet, string, error) { + subnetSetCR := &v1alpha1.SubnetSet{} + subnetSetKey := types.NamespacedName{Namespace: namespace, Name: name} + err := r.Client.Get(ctx, subnetSetKey, subnetSetCR) + if err != nil { + log.Error(err, "Failed to get SubnetSet CR", "key", subnetSetKey.String()) + return nil, fmt.Sprintf(msgGetSubnetSetCR, name), fmt.Errorf("failed to get SubnetSet %s in Namespace %s with error: %v", name, namespace, err) + } + + subnets := r.SubnetService.ListSubnetCreatedBySubnetSet(string(subnetSetCR.UID)) + if len(subnets) == 0 { + log.Info("NSX VpcSubnets by SubnetSet CR do not exist", "key", subnetSetKey.String()) + return nil, fmt.Sprintf(msgGetNSXSubnetsBySubnetSet, name), fmt.Errorf("no existing NSX VpcSubnet created by SubnetSet CR '%s/%s'", namespace, name) + } + return subnets, "", nil +} + +func updateBindingMapStatusWithUnreadyCondition(c client.Client, ctx context.Context, obj client.Object, _ metav1.Time, _ error, args ...interface{}) { + bindingMap := obj.(*v1alpha1.SubnetConnectionBindingMap) + reason := args[0].(string) + msg := args[1].(string) + condition := v1alpha1.Condition{ + Type: v1alpha1.Ready, + Status: corev1.ConditionFalse, + Reason: reason, + Message: msg, + } + updateBindingMapCondition(c, ctx, bindingMap, condition) +} + +func updateBindingMapStatusWithReadyCondition(c client.Client, ctx context.Context, obj client.Object, _ metav1.Time, _ ...interface{}) { + bindingMap := obj.(*v1alpha1.SubnetConnectionBindingMap) + condition := v1alpha1.Condition{ + Type: v1alpha1.Ready, + Status: corev1.ConditionTrue, + } + updateBindingMapCondition(c, ctx, bindingMap, condition) +} + +func updateBindingMapCondition(c client.Client, ctx context.Context, bindingMap *v1alpha1.SubnetConnectionBindingMap, condition v1alpha1.Condition) { + condition.LastTransitionTime = metav1.Now() + newConditions := []v1alpha1.Condition{condition} + for _, cond := range bindingMap.Status.Conditions { + if cond.Type == condition.Type { + if cond.Status == condition.Status && cond.Reason == condition.Reason && cond.Message == condition.Message { + return + } + continue + } + newConditions = append(newConditions, cond) + } + bindingMap.Status.Conditions = newConditions + err := c.Status().Update(ctx, bindingMap) + if err != nil { + log.Error(err, "Failed to update SubnetConnectionBindingMap status", "Namespace", bindingMap.Namespace, "Name", bindingMap.Name) + } + log.V(1).Info("Updated SubnetConnectionBindingMap status", "Namespace", bindingMap.Namespace, "Name", bindingMap.Name) +} diff --git a/pkg/controllers/subnetbinding/controller_test.go b/pkg/controllers/subnetbinding/controller_test.go new file mode 100644 index 000000000..f23797918 --- /dev/null +++ b/pkg/controllers/subnetbinding/controller_test.go @@ -0,0 +1,900 @@ +package subnetbinding + +import ( + "context" + "fmt" + "reflect" + "testing" + + "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" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/apimachinery/pkg/util/sets" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/tools/record" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/manager" + + "github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1" + "github.com/vmware-tanzu/nsx-operator/pkg/config" + "github.com/vmware-tanzu/nsx-operator/pkg/nsx" + "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" +) + +type fakeRecorder struct{} + +func (recorder fakeRecorder) Event(object runtime.Object, eventtype, reason, message string) { +} + +func (recorder fakeRecorder) Eventf(object runtime.Object, eventtype, reason, messageFmt string, args ...interface{}) { +} + +func (recorder fakeRecorder) AnnotatedEventf(object runtime.Object, annotations map[string]string, eventtype, reason, messageFmt string, args ...interface{}) { +} + +type MockManager struct { + ctrl.Manager + client client.Client + scheme *runtime.Scheme + recorder record.EventRecorder +} + +func (m *MockManager) GetClient() client.Client { + return m.client +} + +func (m *MockManager) GetScheme() *runtime.Scheme { + return m.scheme +} + +func (m *MockManager) GetEventRecorderFor(name string) record.EventRecorder { + return m.recorder +} + +func (m *MockManager) Add(runnable manager.Runnable) error { + return nil +} + +func (m *MockManager) Start(context.Context) error { + return nil +} + +func newMockManager(objs ...client.Object) ctrl.Manager { + newScheme := runtime.NewScheme() + utilruntime.Must(clientgoscheme.AddToScheme(newScheme)) + utilruntime.Must(v1alpha1.AddToScheme(newScheme)) + fakeClient := fake.NewClientBuilder().WithScheme(newScheme).WithObjects(objs...).Build() + return &MockManager{ + client: fakeClient, + scheme: newScheme, + recorder: &fakeRecorder{}, + } +} + +func TestReconcile(t *testing.T) { + crName := "binding1" + crNS := "default" + now := metav1.Now() + request := ctrl.Request{ + NamespacedName: types.NamespacedName{ + Name: crName, + Namespace: crNS, + }, + } + deletedBM := &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: metav1.ObjectMeta{ + UID: "binding-uuid", + Namespace: crNS, + Name: crName, + DeletionTimestamp: &now, + Finalizers: []string{"deletion"}, + }, + } + validBM1 := &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: metav1.ObjectMeta{ + UID: "binding-uuid", + Namespace: crNS, + Name: crName, + }, + Spec: v1alpha1.SubnetConnectionBindingMapSpec{ + SubnetName: "child", + TargetSubnetSetName: "parentSubnetSet", + VLANTrafficTag: 101, + }, + } + for _, tc := range []struct { + name string + objects []client.Object + expectRes ctrl.Result + patches func(t *testing.T, r *Reconciler) *gomonkey.Patches + }{ + { + name: "Failed to reconcile with get SubnetConnectionBindingMap CR errors", + patches: func(t *testing.T, r *Reconciler) *gomonkey.Patches { + patches := 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("unable to get CR") + }) + patches.ApplyMethod(reflect.TypeOf(r.SubnetBindingService), "DeleteSubnetConnectionBindingMapsByCRName", func(_ *subnetbinding.BindingService, bindingName string, bindingNamespace string) error { + require.Fail(t, "SubnetBindingService.DeleteSubnetConnectionBindingMapsByCRName should not called when failed to get SubnetConnectionBindingMap CR") + return nil + }) + return patches + }, + expectRes: ResultRequeue, + }, + { + name: "Failed to reconcile with SubnetConnectionBindingMap CR not exist", + patches: func(t *testing.T, r *Reconciler) *gomonkey.Patches { + patches := gomonkey.ApplyMethod(reflect.TypeOf(r.Client), "Get", func(_ client.Client, ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + return apierrors.NewNotFound(v1alpha1.Resource("subnetconnectionbindingmap"), crName) + }) + patches.ApplyMethod(reflect.TypeOf(r.SubnetBindingService), "DeleteSubnetConnectionBindingMapsByCRName", func(_ *subnetbinding.BindingService, bindingName string, bindingNamespace string) error { + return fmt.Errorf("NSX deletion failure") + }) + return patches + }, + expectRes: ResultRequeue, + }, { + name: "Succeeded to delete stale SubnetConnectionBindingMaps if CR not exist", + patches: func(t *testing.T, r *Reconciler) *gomonkey.Patches { + patches := gomonkey.ApplyMethod(reflect.TypeOf(r.Client), "Get", func(_ client.Client, ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + return apierrors.NewNotFound(v1alpha1.Resource("subnetconnectionbindingmap"), crName) + }) + patches.ApplyMethod(reflect.TypeOf(r.SubnetBindingService), "DeleteSubnetConnectionBindingMapsByCRName", func(_ *subnetbinding.BindingService, bindingName string, bindingNamespace string) error { + return nil + }) + return patches + }, + expectRes: ResultNormal, + }, { + name: "Failed to delete SubnetConnectionBindingMap by CR deletion", + objects: []client.Object{deletedBM}, + patches: func(t *testing.T, r *Reconciler) *gomonkey.Patches { + patches := gomonkey.ApplyMethod(reflect.TypeOf(r.SubnetBindingService), "DeleteSubnetConnectionBindingMapsByCRUID", func(_ *subnetbinding.BindingService, bindingUID string) error { + require.Equal(t, "binding-uuid", bindingUID) + return fmt.Errorf("NSX deletion failure") + }) + return patches + }, + expectRes: ResultRequeue, + }, { + name: "Succeeded to delete SubnetConnectionBindingMap by CR deletion", + objects: []client.Object{deletedBM}, + patches: func(t *testing.T, r *Reconciler) *gomonkey.Patches { + patches := gomonkey.ApplyMethod(reflect.TypeOf(r.SubnetBindingService), "DeleteSubnetConnectionBindingMapsByCRUID", func(_ *subnetbinding.BindingService, bindingUID string) error { + require.Equal(t, "binding-uuid", bindingUID) + return nil + }) + return patches + }, + expectRes: ResultNormal, + }, { + name: "Failed to create/update SubnetConnectionBindingMap by unready dependencies", + objects: []client.Object{validBM1}, + patches: func(t *testing.T, r *Reconciler) *gomonkey.Patches { + patches := gomonkey.ApplyPrivateMethod(reflect.TypeOf(r), "validateDependency", func(_ *Reconciler, ctx context.Context, bindingMap *v1alpha1.SubnetConnectionBindingMap) (*model.VpcSubnet, []*model.VpcSubnet, string, error) { + return nil, nil, "Unable to get Subnet CR net1", fmt.Errorf("cr not ready") + }) + return patches + }, + expectRes: ResultRequeueAfter10sec, + }, { + name: "Failed to create/update SubnetConnectionBindingMap on NSX", + objects: []client.Object{validBM1}, + patches: func(t *testing.T, r *Reconciler) *gomonkey.Patches { + patches := gomonkey.ApplyPrivateMethod(reflect.TypeOf(r), "validateDependency", func(_ *Reconciler, ctx context.Context, bindingMap *v1alpha1.SubnetConnectionBindingMap) (*model.VpcSubnet, []*model.VpcSubnet, string, error) { + return &model.VpcSubnet{Id: common.String("child")}, []*model.VpcSubnet{{Id: common.String("parent")}}, "", nil + }) + patches.ApplyMethod(reflect.TypeOf(r.SubnetBindingService), "CreateOrUpdateSubnetConnectionBindingMap", + func(_ *subnetbinding.BindingService, subnetBinding *v1alpha1.SubnetConnectionBindingMap, childSubnet *model.VpcSubnet, parentSubnets []*model.VpcSubnet) error { + return fmt.Errorf("failed to configure NSX") + }) + return patches + }, + expectRes: ResultRequeue, + }, { + name: "Succeeded to create/update SubnetConnectionBindingMap", + objects: []client.Object{validBM1}, + patches: func(t *testing.T, r *Reconciler) *gomonkey.Patches { + patches := gomonkey.ApplyPrivateMethod(reflect.TypeOf(r), "validateDependency", func(_ *Reconciler, ctx context.Context, bindingMap *v1alpha1.SubnetConnectionBindingMap) (*model.VpcSubnet, []*model.VpcSubnet, string, error) { + return &model.VpcSubnet{Id: common.String("child")}, []*model.VpcSubnet{{Id: common.String("parent")}}, "", nil + }) + patches.ApplyMethod(reflect.TypeOf(r.SubnetBindingService), "CreateOrUpdateSubnetConnectionBindingMap", + func(_ *subnetbinding.BindingService, subnetBinding *v1alpha1.SubnetConnectionBindingMap, childSubnet *model.VpcSubnet, parentSubnets []*model.VpcSubnet) error { + return nil + }) + return patches + }, + expectRes: ResultNormal, + }, + } { + t.Run(tc.name, func(t *testing.T) { + ctx := context.Background() + r := createFakeReconciler(tc.objects...) + patches := tc.patches(t, r) + defer patches.Reset() + + rst, _ := r.Reconcile(ctx, request) + assert.Equal(t, tc.expectRes, rst) + }) + } +} + +func TestCollectGarbage(t *testing.T) { + for _, tc := range []struct { + name string + patches func(t *testing.T, r *Reconciler) *gomonkey.Patches + }{ + { + name: "Failed to list from CRs", + patches: func(t *testing.T, r *Reconciler) *gomonkey.Patches { + patches := gomonkey.ApplyPrivateMethod(reflect.TypeOf(r), "listBindingMapIDsFromCRs", func(_ *Reconciler, ctx context.Context) (sets.Set[string], error) { + return sets.New[string](), fmt.Errorf("unable to list CRs") + }) + return patches + }, + }, { + name: "Failed to delete on NSX", + patches: func(t *testing.T, r *Reconciler) *gomonkey.Patches { + patches := gomonkey.ApplyPrivateMethod(reflect.TypeOf(r), "listBindingMapIDsFromCRs", func(_ *Reconciler, ctx context.Context) (sets.Set[string], error) { + return sets.New[string](), nil + }) + patches.ApplyMethod(reflect.TypeOf(r.SubnetBindingService), "ListSubnetConnectionBindingMapCRUIDsInStore", func(s *subnetbinding.BindingService) sets.Set[string] { + return sets.New[string]() + }) + patches.ApplyMethod(reflect.TypeOf(r.SubnetBindingService), "DeleteMultiSubnetConnectionBindingMapsByCRs", func(s *subnetbinding.BindingService, bindingCRs sets.Set[string]) error { + return fmt.Errorf("deletion failed") + }) + return patches + }, + }, { + name: "Success", + patches: func(t *testing.T, r *Reconciler) *gomonkey.Patches { + patches := gomonkey.ApplyPrivateMethod(reflect.TypeOf(r), "listBindingMapIDsFromCRs", func(_ *Reconciler, ctx context.Context) (sets.Set[string], error) { + return sets.New[string](), nil + }) + patches.ApplyMethod(reflect.TypeOf(r.SubnetBindingService), "ListSubnetConnectionBindingMapCRUIDsInStore", func(s *subnetbinding.BindingService) sets.Set[string] { + return sets.New[string]() + }) + patches.ApplyMethod(reflect.TypeOf(r.SubnetBindingService), "DeleteMultiSubnetConnectionBindingMapsByCRs", func(s *subnetbinding.BindingService, bindingCRs sets.Set[string]) error { + return nil + }) + return patches + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + ctx := context.Background() + r := createFakeReconciler() + patches := tc.patches(t, r) + defer patches.Reset() + + r.CollectGarbage(ctx) + }) + } +} + +func TestValidateDependency(t *testing.T) { + name := "binding1" + namespace := "default" + childSubnet := "subnet" + targetSubnet := "targetSubnet" + targetSubnetSet := "targetSubnetSet" + bindingCR1 := &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: v1alpha1.SubnetConnectionBindingMapSpec{ + SubnetName: childSubnet, + TargetSubnetName: targetSubnet, + VLANTrafficTag: 101, + }, + } + bindingCR2 := &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: v1alpha1.SubnetConnectionBindingMapSpec{ + SubnetName: childSubnet, + TargetSubnetSetName: targetSubnetSet, + VLANTrafficTag: 101, + }, + } + + for _, tc := range []struct { + name string + patches func(t *testing.T, r *Reconciler) *gomonkey.Patches + bindingMap *v1alpha1.SubnetConnectionBindingMap + expErr string + expMsg string + expChild *model.VpcSubnet + expParents []*model.VpcSubnet + }{ + { + name: "child subnet is not ready", + bindingMap: bindingCR1, + patches: func(t *testing.T, r *Reconciler) *gomonkey.Patches { + patches := gomonkey.ApplyPrivateMethod(reflect.TypeOf(r), "validateVpcSubnetsBySubnetCR", func(_ *Reconciler, ctx context.Context, namespace, name string, isTarget bool) ([]*model.VpcSubnet, string, error) { + return nil, "Unable to get Subnet CR net1", fmt.Errorf("unable to get CR") + }) + return patches + }, + expErr: "unable to get CR", + expMsg: "Unable to get Subnet CR net1", + expChild: nil, + }, { + name: "parent subnet is not ready", + bindingMap: bindingCR1, + patches: func(t *testing.T, r *Reconciler) *gomonkey.Patches { + patches := gomonkey.ApplyPrivateMethod(reflect.TypeOf(r), "validateVpcSubnetsBySubnetCR", func(_ *Reconciler, ctx context.Context, namespace, name string, isTarget bool) ([]*model.VpcSubnet, string, error) { + if !isTarget { + return []*model.VpcSubnet{{Id: common.String("child")}}, "", nil + } + return nil, "Unable to get Subnet CR net1", fmt.Errorf("unable to get CR") + }) + return patches + }, + expErr: "unable to get CR", + expMsg: "Unable to get Subnet CR net1", + expChild: nil, + }, { + name: "parent subnet is ready", + bindingMap: bindingCR1, + patches: func(t *testing.T, r *Reconciler) *gomonkey.Patches { + patches := gomonkey.ApplyPrivateMethod(reflect.TypeOf(r), "validateVpcSubnetsBySubnetCR", func(_ *Reconciler, ctx context.Context, namespace, name string, isTarget bool) ([]*model.VpcSubnet, string, error) { + if !isTarget { + return []*model.VpcSubnet{{Id: common.String("child")}}, "", nil + } + return []*model.VpcSubnet{{Id: common.String("parent")}}, "", nil + }) + return patches + }, + expChild: &model.VpcSubnet{Id: common.String("child")}, + expParents: []*model.VpcSubnet{{Id: common.String("parent")}}, + }, { + name: "parent subnetSet is not ready", + bindingMap: bindingCR2, + patches: func(t *testing.T, r *Reconciler) *gomonkey.Patches { + patches := gomonkey.ApplyPrivateMethod(reflect.TypeOf(r), "validateVpcSubnetsBySubnetCR", func(_ *Reconciler, ctx context.Context, namespace, name string, isTarget bool) ([]*model.VpcSubnet, string, error) { + return []*model.VpcSubnet{{Id: common.String("child")}}, "", nil + }) + patches.ApplyPrivateMethod(reflect.TypeOf(r), "validateVpcSubnetsBySubnetSetCR", func(_ *Reconciler, ctx context.Context, namespace, name string) ([]*model.VpcSubnet, string, error) { + return nil, "Unable to get Subnet CR net1", fmt.Errorf("unable to get CR") + }) + return patches + }, + expErr: "unable to get CR", + expMsg: "Unable to get Subnet CR net1", + expChild: nil, + }, { + name: "parent subnetSet is ready", + bindingMap: bindingCR2, + patches: func(t *testing.T, r *Reconciler) *gomonkey.Patches { + patches := gomonkey.ApplyPrivateMethod(reflect.TypeOf(r), "validateVpcSubnetsBySubnetCR", func(_ *Reconciler, ctx context.Context, namespace, name string, isTarget bool) ([]*model.VpcSubnet, string, error) { + return []*model.VpcSubnet{{Id: common.String("child")}}, "", nil + }) + patches.ApplyPrivateMethod(reflect.TypeOf(r), "validateVpcSubnetsBySubnetSetCR", func(_ *Reconciler, ctx context.Context, namespace, name string) ([]*model.VpcSubnet, string, error) { + return []*model.VpcSubnet{{Id: common.String("parent")}}, "", nil + }) + return patches + }, + expChild: &model.VpcSubnet{Id: common.String("child")}, + expParents: []*model.VpcSubnet{{Id: common.String("parent")}}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + ctx := context.TODO() + r := createFakeReconciler() + patches := tc.patches(t, r) + defer patches.Reset() + + child, parents, msg, err := r.validateDependency(ctx, tc.bindingMap) + if tc.expErr != "" { + require.EqualError(t, err, tc.expErr) + } + require.Equal(t, tc.expMsg, msg) + require.Equal(t, tc.expChild, child) + require.ElementsMatch(t, tc.expParents, parents) + }) + } +} + +func TestValidateVpcSubnetsBySubnetCR(t *testing.T) { + subnetName := "net1" + subnetNamespace := "default" + subnetCR := &v1alpha1.Subnet{ + ObjectMeta: metav1.ObjectMeta{ + Name: subnetName, + Namespace: subnetNamespace, + UID: "subnet-uuid", + }, + } + for _, tc := range []struct { + name string + isTarget bool + objects []client.Object + patches func(t *testing.T, r *Reconciler) *gomonkey.Patches + expErr string + expMsg string + subnets []*model.VpcSubnet + }{ + { + name: "Failed to get Subnet CR", + isTarget: false, + patches: func(t *testing.T, r *Reconciler) *gomonkey.Patches { + patches := 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("unable to get CR") + }) + return patches + }, + expMsg: "Unable to get Subnet CR net1", + expErr: "failed to get subnet net1 in Namespace default with error: unable to get CR", + }, { + name: "Subnet CR is not realized", + isTarget: false, + patches: func(t *testing.T, r *Reconciler) *gomonkey.Patches { + patches := gomonkey.ApplyMethod(reflect.TypeOf(r.SubnetService), "ListSubnetCreatedBySubnet", func(_ *subnet.SubnetService, id string) []*model.VpcSubnet { + return []*model.VpcSubnet{} + }) + return patches + }, + objects: []client.Object{subnetCR}, + expMsg: "Subnet CR net1 is not realized on NSX", + expErr: "not found NSX VpcSubnets created by Subnet CR 'default/net1'", + }, { + name: "Child subnet CR is also used as parent", + isTarget: false, + patches: func(t *testing.T, r *Reconciler) *gomonkey.Patches { + patches := gomonkey.ApplyMethod(reflect.TypeOf(r.SubnetService), "ListSubnetCreatedBySubnet", func(_ *subnet.SubnetService, id string) []*model.VpcSubnet { + return []*model.VpcSubnet{{Id: common.String("net1")}} + }) + patches.ApplyMethod(reflect.TypeOf(r.SubnetBindingService), "GetSubnetConnectionBindingMapsByParentSubnet", func(_ *subnetbinding.BindingService, subnet *model.VpcSubnet) []*model.SubnetConnectionBindingMap { + return []*model.SubnetConnectionBindingMap{{Id: common.String("binding1")}} + }) + patches.ApplyMethod(reflect.TypeOf(r.SubnetBindingService), "GetCRNameBySubnetConnectionBindingMap", func(_ *subnetbinding.BindingService, bindingMap *model.SubnetConnectionBindingMap) string { + return "binding1" + }) + return patches + }, + objects: []client.Object{subnetCR}, + expMsg: "Subnet CR net1 is working as target by binding1", + expErr: "Subnet net1 already works as target in SegmentConnectionBindingMap binding1", + }, { + name: "Child subnet CR is not used as parent", + isTarget: false, + patches: func(t *testing.T, r *Reconciler) *gomonkey.Patches { + patches := gomonkey.ApplyMethod(reflect.TypeOf(r.SubnetService), "ListSubnetCreatedBySubnet", func(_ *subnet.SubnetService, id string) []*model.VpcSubnet { + return []*model.VpcSubnet{{Id: common.String("net1")}} + }) + patches.ApplyMethod(reflect.TypeOf(r.SubnetBindingService), "GetSubnetConnectionBindingMapsByParentSubnet", func(_ *subnetbinding.BindingService, subnet *model.VpcSubnet) []*model.SubnetConnectionBindingMap { + return []*model.SubnetConnectionBindingMap{} + }) + return patches + }, + objects: []client.Object{subnetCR}, + expMsg: "", + expErr: "", + subnets: []*model.VpcSubnet{{Id: common.String("net1")}}, + }, { + name: "Parent subnet CR is also used as child", + isTarget: true, + patches: func(t *testing.T, r *Reconciler) *gomonkey.Patches { + patches := gomonkey.ApplyMethod(reflect.TypeOf(r.SubnetService), "ListSubnetCreatedBySubnet", func(_ *subnet.SubnetService, id string) []*model.VpcSubnet { + return []*model.VpcSubnet{{Id: common.String("net1")}} + }) + patches.ApplyMethod(reflect.TypeOf(r.SubnetBindingService), "GetSubnetConnectionBindingMapsByChildSubnet", func(_ *subnetbinding.BindingService, subnet *model.VpcSubnet) []*model.SubnetConnectionBindingMap { + return []*model.SubnetConnectionBindingMap{{Id: common.String("binding1")}} + }) + patches.ApplyMethod(reflect.TypeOf(r.SubnetBindingService), "GetCRNameBySubnetConnectionBindingMap", func(_ *subnetbinding.BindingService, bindingMap *model.SubnetConnectionBindingMap) string { + return "binding1" + }) + return patches + }, + objects: []client.Object{subnetCR}, + expMsg: "Target Subnet CR net1 is attached by binding1", + expErr: "target Subnet net1 is already attached by SegmentConnectionBindingMap binding1", + }, { + name: "Child subnet CR is not used as parent", + isTarget: true, + patches: func(t *testing.T, r *Reconciler) *gomonkey.Patches { + patches := gomonkey.ApplyMethod(reflect.TypeOf(r.SubnetService), "ListSubnetCreatedBySubnet", func(_ *subnet.SubnetService, id string) []*model.VpcSubnet { + return []*model.VpcSubnet{{Id: common.String("net1")}} + }) + patches.ApplyMethod(reflect.TypeOf(r.SubnetBindingService), "GetSubnetConnectionBindingMapsByChildSubnet", func(_ *subnetbinding.BindingService, subnet *model.VpcSubnet) []*model.SubnetConnectionBindingMap { + return []*model.SubnetConnectionBindingMap{} + }) + return patches + }, + objects: []client.Object{subnetCR}, + expMsg: "", + expErr: "", + subnets: []*model.VpcSubnet{{Id: common.String("net1")}}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + ctx := context.TODO() + r := createFakeReconciler(tc.objects...) + patches := tc.patches(t, r) + defer patches.Reset() + + subnets, msg, err := r.validateVpcSubnetsBySubnetCR(ctx, subnetNamespace, subnetName, tc.isTarget) + if tc.expErr != "" { + require.EqualError(t, err, tc.expErr) + } + require.Equal(t, tc.expMsg, msg) + require.ElementsMatch(t, tc.subnets, subnets) + }) + } +} + +func TestValidateVpcSubnetsBySubnetSetCR(t *testing.T) { + name := "net1" + namespace := "default" + subnetSetCR := &v1alpha1.SubnetSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + UID: "subnetset-uuid", + }, + } + for _, tc := range []struct { + name string + objects []client.Object + patches func(t *testing.T, r *Reconciler) *gomonkey.Patches + expErr string + expMsg string + subnets []*model.VpcSubnet + }{ + { + name: "Failed to get SubnetSet CR", + patches: func(t *testing.T, r *Reconciler) *gomonkey.Patches { + patches := 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("unable to get CR") + }) + return patches + }, + expMsg: "Unable to get SubnetSet CR net1", + expErr: "failed to get SubnetSet net1 in Namespace default with error: unable to get CR", + }, { + name: "SubnetSet CR is not realized", + patches: func(t *testing.T, r *Reconciler) *gomonkey.Patches { + patches := gomonkey.ApplyMethod(reflect.TypeOf(r.SubnetService), "ListSubnetCreatedBySubnetSet", func(_ *subnet.SubnetService, id string) []*model.VpcSubnet { + return []*model.VpcSubnet{} + }) + return patches + }, + objects: []client.Object{subnetSetCR}, + expMsg: "SubnetSet CR net1 is not realized on NSX", + expErr: "no existing NSX VpcSubnet created by SubnetSet CR 'default/net1'", + }, { + name: "SubnetSet CR is realized", + patches: func(t *testing.T, r *Reconciler) *gomonkey.Patches { + patches := gomonkey.ApplyMethod(reflect.TypeOf(r.SubnetService), "ListSubnetCreatedBySubnetSet", func(_ *subnet.SubnetService, id string) []*model.VpcSubnet { + return []*model.VpcSubnet{{Id: common.String("net1")}} + }) + return patches + }, + objects: []client.Object{subnetSetCR}, + expMsg: "", + expErr: "", + subnets: []*model.VpcSubnet{{Id: common.String("net1")}}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + ctx := context.TODO() + r := createFakeReconciler(tc.objects...) + patches := tc.patches(t, r) + defer patches.Reset() + + subnets, msg, err := r.validateVpcSubnetsBySubnetSetCR(ctx, namespace, name) + if tc.expErr != "" { + require.EqualError(t, err, tc.expErr) + } + require.Equal(t, tc.expMsg, msg) + require.ElementsMatch(t, tc.subnets, subnets) + }) + } +} + +func TestUpdateBindingMapStatusWithConditions(t *testing.T) { + newScheme := runtime.NewScheme() + utilruntime.Must(clientgoscheme.AddToScheme(newScheme)) + utilruntime.Must(v1alpha1.AddToScheme(newScheme)) + + name := "binding1" + namespace := "default" + bindingMap1 := &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: v1alpha1.SubnetConnectionBindingMapSpec{ + SubnetName: "child", + TargetSubnetSetName: "parent", + VLANTrafficTag: 101, + }, + } + bindingMap2 := &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: v1alpha1.SubnetConnectionBindingMapSpec{ + SubnetName: "child", + TargetSubnetSetName: "parent", + VLANTrafficTag: 101, + }, + Status: v1alpha1.SubnetConnectionBindingMapStatus{ + Conditions: []v1alpha1.Condition{ + { + Type: v1alpha1.Ready, + Status: corev1.ConditionTrue, + }, + }, + }, + } + bindingMap3 := &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: v1alpha1.SubnetConnectionBindingMapSpec{ + SubnetName: "child", + TargetSubnetSetName: "parent", + VLANTrafficTag: 101, + }, + Status: v1alpha1.SubnetConnectionBindingMapStatus{ + Conditions: []v1alpha1.Condition{ + { + Type: v1alpha1.Ready, + Status: corev1.ConditionFalse, + Message: "old message", + Reason: "crNotFound", + }, + }, + }, + } + msg := "Subnet CR net1 is not realized on NSX" + bindingMap4 := &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: v1alpha1.SubnetConnectionBindingMapSpec{ + SubnetName: "child", + TargetSubnetSetName: "parent", + VLANTrafficTag: 101, + }, + Status: v1alpha1.SubnetConnectionBindingMapStatus{ + Conditions: []v1alpha1.Condition{ + { + Type: v1alpha1.Ready, + Status: corev1.ConditionFalse, + Message: msg, + Reason: reasonDependencyNotReady, + }, + }, + }, + } + + for _, tc := range []struct { + name string + existingBM *v1alpha1.SubnetConnectionBindingMap + }{ + { + name: "Add new condition", + existingBM: bindingMap1, + }, { + name: "Update ready condition to unready", + existingBM: bindingMap2, + }, { + name: "Update unready condition message and reason", + existingBM: bindingMap3, + }, { + name: "Not update unready condition if message and ready equals", + existingBM: bindingMap4, + }, + } { + t.Run(tc.name, func(t *testing.T) { + ctx := context.Background() + + fakeClient := fake.NewClientBuilder().WithScheme(newScheme).WithObjects(tc.existingBM).WithStatusSubresource(tc.existingBM).Build() + updateBindingMapStatusWithUnreadyCondition(fakeClient, ctx, tc.existingBM, metav1.Now(), nil, reasonDependencyNotReady, msg) + + updatedBM := &v1alpha1.SubnetConnectionBindingMap{} + err := fakeClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: name}, updatedBM) + require.NoError(t, err) + require.Equal(t, 1, len(updatedBM.Status.Conditions)) + cond := updatedBM.Status.Conditions[0] + assert.Equal(t, reasonDependencyNotReady, cond.Reason) + assert.Equal(t, msg, cond.Message) + assert.Equal(t, v1alpha1.Ready, cond.Type) + assert.Equal(t, corev1.ConditionFalse, cond.Status) + + fakeClient2 := fake.NewClientBuilder().WithScheme(newScheme).WithObjects(tc.existingBM).WithStatusSubresource(tc.existingBM).Build() + updateBindingMapStatusWithReadyCondition(fakeClient2, ctx, tc.existingBM, metav1.Now()) + + updatedBM2 := &v1alpha1.SubnetConnectionBindingMap{} + err = fakeClient2.Get(ctx, client.ObjectKey{Namespace: namespace, Name: name}, updatedBM2) + require.NoError(t, err) + require.Equal(t, 1, len(updatedBM2.Status.Conditions)) + cond = updatedBM2.Status.Conditions[0] + assert.Equal(t, v1alpha1.Ready, cond.Type) + assert.Equal(t, corev1.ConditionTrue, cond.Status) + }) + } +} + +func TestListBindingMapIDsFromCRs(t *testing.T) { + bm1 := &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: metav1.ObjectMeta{ + UID: "binding1-uuid", + Namespace: "default", + Name: "binding1", + }, + } + bm2 := &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: metav1.ObjectMeta{ + UID: "binding2-uuid", + Namespace: "ns1", + Name: "binding2", + }, + } + for _, tc := range []struct { + name string + patches func(t *testing.T, r *Reconciler) *gomonkey.Patches + objects []client.Object + expCRs []string + expErr string + }{ + { + name: "Failed to list CRs", + patches: func(t *testing.T, r *Reconciler) *gomonkey.Patches { + patches := gomonkey.ApplyMethod(reflect.TypeOf(r.Client), "List", func(_ client.Client, ctx context.Context, list client.ObjectList, opts ...client.ListOption) error { + return fmt.Errorf("unable to list CRs") + }) + return patches + }, + expCRs: []string{}, + expErr: "unable to list CRs", + }, { + name: "Success", + objects: []client.Object{bm1, bm2}, + expCRs: []string{"binding1-uuid", "binding2-uuid"}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + ctx := context.Background() + r := createFakeReconciler(tc.objects...) + if tc.patches != nil { + patches := tc.patches(t, r) + defer patches.Reset() + } + + crIDs, err := r.listBindingMapIDsFromCRs(ctx) + if tc.expErr != "" { + require.EqualError(t, err, tc.expErr) + } + assert.ElementsMatch(t, tc.expCRs, crIDs.UnsortedList()) + }) + } +} + +func TestPredicateFuncsBindingMaps(t *testing.T) { + name := "binding1" + namespace := "default" + bindingMap1 := &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: v1alpha1.SubnetConnectionBindingMapSpec{ + SubnetName: "child", + TargetSubnetSetName: "parent", + VLANTrafficTag: 101, + }, + Status: v1alpha1.SubnetConnectionBindingMapStatus{ + Conditions: []v1alpha1.Condition{ + { + Type: v1alpha1.Ready, + Status: corev1.ConditionTrue, + }, + }, + }, + } + bindingMap2 := &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: v1alpha1.SubnetConnectionBindingMapSpec{ + SubnetName: "child", + TargetSubnetSetName: "parent", + VLANTrafficTag: 102, + }, + Status: v1alpha1.SubnetConnectionBindingMapStatus{ + Conditions: []v1alpha1.Condition{ + { + Type: v1alpha1.Ready, + Status: corev1.ConditionTrue, + }, + }, + }, + } + bindingMap3 := &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: v1alpha1.SubnetConnectionBindingMapSpec{ + SubnetName: "child", + TargetSubnetSetName: "parent", + VLANTrafficTag: 101, + }, + Status: v1alpha1.SubnetConnectionBindingMapStatus{ + Conditions: []v1alpha1.Condition{ + { + Type: v1alpha1.Ready, + Status: corev1.ConditionFalse, + Message: "old message", + Reason: "crNotFound", + }, + }, + }, + } + createEvent := event.CreateEvent{Object: bindingMap1} + updateEvent1 := event.UpdateEvent{ObjectOld: bindingMap1, ObjectNew: bindingMap2} + updateEvent2 := event.UpdateEvent{ObjectOld: bindingMap1, ObjectNew: bindingMap3} + deleteEvent := event.DeleteEvent{Object: bindingMap1} + genericEvent := event.GenericEvent{Object: bindingMap1} + assert.True(t, PredicateFuncsBindingMaps.CreateFunc(createEvent)) + assert.True(t, PredicateFuncsBindingMaps.Update(updateEvent1)) + assert.False(t, PredicateFuncsBindingMaps.Update(updateEvent2)) + assert.True(t, PredicateFuncsBindingMaps.Delete(deleteEvent)) + assert.False(t, PredicateFuncsBindingMaps.GenericFunc(genericEvent)) +} + +func createFakeReconciler(objs ...client.Object) *Reconciler { + var mgr ctrl.Manager + if len(objs) == 0 { + mgr = newMockManager() + } else { + mgr = newMockManager(objs...) + } + + svc := common.Service{ + Client: mgr.GetClient(), + NSXClient: &nsx.Client{}, + + NSXConfig: &config.NSXOperatorConfig{ + NsxConfig: &config.NsxConfig{ + EnforcementPoint: "vmc-enforcementpoint", + UseAVILoadBalancer: false, + }, + }, + } + subnetService := &subnet.SubnetService{ + Service: svc, + SubnetStore: &subnet.SubnetStore{}, + } + bindingService := &subnetbinding.BindingService{ + Service: svc, + BindingStore: subnetbinding.SetupStore(), + } + + return newReconciler(mgr, subnetService, bindingService) +} diff --git a/pkg/controllers/subnetbinding/subnets_handler.go b/pkg/controllers/subnetbinding/subnets_handler.go new file mode 100644 index 000000000..6a5599ce7 --- /dev/null +++ b/pkg/controllers/subnetbinding/subnets_handler.go @@ -0,0 +1,110 @@ +package subnetbinding + +import ( + "context" + "reflect" + + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/util/workqueue" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1" + "github.com/vmware-tanzu/nsx-operator/pkg/controllers/common" +) + +var PredicateFuncsSubnets = predicate.Funcs{ + CreateFunc: func(e event.CreateEvent) bool { + return false + }, + UpdateFunc: func(e event.UpdateEvent) bool { + oldObj := e.ObjectOld.(*v1alpha1.Subnet) + newObj := e.ObjectNew.(*v1alpha1.Subnet) + return common.IsObjectUpdateToReady(oldObj.Status.Conditions, newObj.Status.Conditions) + }, + DeleteFunc: func(e event.DeleteEvent) bool { + return false + }, + GenericFunc: func(e event.GenericEvent) bool { + return false + }, +} + +var PredicateFuncsSubnetSets = predicate.Funcs{ + CreateFunc: func(e event.CreateEvent) bool { + return false + }, + UpdateFunc: func(e event.UpdateEvent) bool { + oldObj := e.ObjectOld.(*v1alpha1.SubnetSet) + newObj := e.ObjectNew.(*v1alpha1.SubnetSet) + return !reflect.DeepEqual(oldObj.Status.Subnets, newObj.Status.Subnets) + }, + DeleteFunc: func(e event.DeleteEvent) bool { + return false + }, + GenericFunc: func(e event.GenericEvent) bool { + return false + }, +} + +func requeueBindingMapsBySubnetDelete(ctx context.Context, c client.Client, obj client.Object, q workqueue.TypedRateLimitingInterface[reconcile.Request]) { + subnet := obj.(*v1alpha1.Subnet) + requeueSubnetConnectionBindingMapsBySubnet(ctx, c, subnet.Namespace, subnet.Name, q) +} + +func requeueBindingMapsBySubnetUpdate(ctx context.Context, c client.Client, _, objNew client.Object, q workqueue.TypedRateLimitingInterface[reconcile.Request]) { + subnet := objNew.(*v1alpha1.Subnet) + requeueSubnetConnectionBindingMapsBySubnet(ctx, c, subnet.Namespace, subnet.Name, q) +} + +func requeueSubnetConnectionBindingMapsBySubnet(ctx context.Context, c client.Client, namespace string, subnet string, q workqueue.TypedRateLimitingInterface[reconcile.Request]) { + bindingMapList := &v1alpha1.SubnetConnectionBindingMapList{} + err := c.List(ctx, bindingMapList, client.InNamespace(namespace)) + if err != nil { + log.Error(err, "Failed to list SubnetConnectionBindingMaps with Subnet event", "Namespace", namespace, "Subnet", subnet) + return + } + for _, bm := range bindingMapList.Items { + if bm.Spec.SubnetName == subnet || bm.Spec.TargetSubnetName == subnet { + log.Info("Requeue SubnetConnectionBindingMap because because the dependent Subnet is ready", "Namespace", namespace, "Name", bm.Name) + q.Add(reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: bm.Name, + Namespace: bm.Namespace, + }, + }) + } + } +} + +func requeueBindingMapsBySubnetSetDelete(ctx context.Context, c client.Client, obj client.Object, q workqueue.TypedRateLimitingInterface[reconcile.Request]) { + subnetSet := obj.(*v1alpha1.SubnetSet) + requeueSubnetConnectionBindingMapsBySubnetSet(ctx, c, subnetSet.Namespace, subnetSet.Name, q) +} + +func requeueBindingMapsBySubnetSetUpdate(ctx context.Context, c client.Client, objOld, objNew client.Object, q workqueue.TypedRateLimitingInterface[reconcile.Request]) { + subnetSet := objNew.(*v1alpha1.SubnetSet) + requeueSubnetConnectionBindingMapsBySubnetSet(ctx, c, subnetSet.Namespace, subnetSet.Name, q) +} + +func requeueSubnetConnectionBindingMapsBySubnetSet(ctx context.Context, c client.Client, namespace string, subnetSet string, q workqueue.TypedRateLimitingInterface[reconcile.Request]) { + bindingMapList := &v1alpha1.SubnetConnectionBindingMapList{} + err := c.List(ctx, bindingMapList, client.InNamespace(namespace)) + if err != nil { + log.Error(err, "Failed to list SubnetConnectionBindingMaps with SubnetSet event", "Namespace", namespace, "Subnet", subnetSet) + return + } + for _, bm := range bindingMapList.Items { + if bm.Spec.TargetSubnetSetName == subnetSet { + log.Info("Requeue SubnetConnectionBindingMap because because the dependent SubnetSet is ready or updated", "Namespace", namespace, "Name", bm.Name) + q.Add(reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: bm.Name, + Namespace: bm.Namespace, + }, + }) + } + } +} diff --git a/pkg/controllers/subnetbinding/subnets_handler_test.go b/pkg/controllers/subnetbinding/subnets_handler_test.go new file mode 100644 index 000000000..39edc9d15 --- /dev/null +++ b/pkg/controllers/subnetbinding/subnets_handler_test.go @@ -0,0 +1,258 @@ +package subnetbinding + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/util/workqueue" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1" +) + +func TestPredicateFuncsSubnets(t *testing.T) { + name := "net1" + namespace := "default" + net1 := &v1alpha1.Subnet{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: v1alpha1.SubnetSpec{ + IPv4SubnetSize: 64, + }, + Status: v1alpha1.SubnetStatus{ + Conditions: []v1alpha1.Condition{ + { + Type: v1alpha1.Ready, + Status: corev1.ConditionTrue, + }, + }, + }, + } + net2 := &v1alpha1.Subnet{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: v1alpha1.SubnetSpec{ + IPv4SubnetSize: 128, + }, + Status: v1alpha1.SubnetStatus{ + Conditions: []v1alpha1.Condition{ + { + Type: v1alpha1.Ready, + Status: corev1.ConditionTrue, + }, + }, + }, + } + net3 := &v1alpha1.Subnet{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: v1alpha1.SubnetSpec{ + IPv4SubnetSize: 64, + }, + Status: v1alpha1.SubnetStatus{ + Conditions: []v1alpha1.Condition{ + { + Type: v1alpha1.Ready, + Status: corev1.ConditionFalse, + Message: "old message", + Reason: "crNotFound", + }, + }, + }, + } + createEvent := event.CreateEvent{Object: net1} + updateEvent1 := event.UpdateEvent{ObjectOld: net1, ObjectNew: net2} + updateEvent2 := event.UpdateEvent{ObjectOld: net1, ObjectNew: net3} + updateEvent3 := event.UpdateEvent{ObjectOld: net3, ObjectNew: net1} + deleteEvent := event.DeleteEvent{Object: net1} + genericEvent := event.GenericEvent{Object: net1} + assert.False(t, PredicateFuncsSubnets.CreateFunc(createEvent)) + assert.False(t, PredicateFuncsSubnets.Update(updateEvent1)) + assert.False(t, PredicateFuncsSubnets.Update(updateEvent2)) + assert.True(t, PredicateFuncsSubnets.Update(updateEvent3)) + assert.False(t, PredicateFuncsSubnets.Delete(deleteEvent)) + assert.False(t, PredicateFuncsSubnets.GenericFunc(genericEvent)) +} + +func TestPredicateFuncsSubnetSets(t *testing.T) { + name := "net1" + namespace := "default" + net1 := &v1alpha1.SubnetSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: v1alpha1.SubnetSetSpec{ + IPv4SubnetSize: 64, + }, + Status: v1alpha1.SubnetSetStatus{ + Subnets: []v1alpha1.SubnetInfo{}, + }, + } + net2 := &v1alpha1.SubnetSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: v1alpha1.SubnetSetSpec{ + IPv4SubnetSize: 128, + }, + Status: v1alpha1.SubnetSetStatus{ + Subnets: []v1alpha1.SubnetInfo{}, + }, + } + net3 := &v1alpha1.SubnetSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: v1alpha1.SubnetSetSpec{ + IPv4SubnetSize: 64, + }, + Status: v1alpha1.SubnetSetStatus{ + Subnets: []v1alpha1.SubnetInfo{ + { + NetworkAddresses: []string{ + "192.168.26.0/24", + }, + }, + }, + }, + } + createEvent := event.CreateEvent{Object: net1} + updateEvent1 := event.UpdateEvent{ObjectOld: net1, ObjectNew: net2} + updateEvent2 := event.UpdateEvent{ObjectOld: net1, ObjectNew: net3} + deleteEvent := event.DeleteEvent{Object: net1} + genericEvent := event.GenericEvent{Object: net1} + assert.False(t, PredicateFuncsSubnetSets.CreateFunc(createEvent)) + assert.False(t, PredicateFuncsSubnetSets.Update(updateEvent1)) + assert.True(t, PredicateFuncsSubnetSets.Update(updateEvent2)) + assert.False(t, PredicateFuncsSubnetSets.Delete(deleteEvent)) + assert.False(t, PredicateFuncsSubnetSets.GenericFunc(genericEvent)) +} + +func TestRequeueSubnetConnectionBindingMapsBySubnet(t *testing.T) { + myQueue := workqueue.NewTypedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[reconcile.Request]()) + defer myQueue.ShutDown() + + ctx := context.TODO() + crName := "binding1" + crNS := "default" + bm1 := &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: metav1.ObjectMeta{ + UID: "binding-uuid", + Namespace: crNS, + Name: crName, + }, + Spec: v1alpha1.SubnetConnectionBindingMapSpec{ + SubnetName: "child", + TargetSubnetName: "parent", + VLANTrafficTag: 101, + }, + } + req := reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: crName, + Namespace: crNS, + }, + } + newScheme := runtime.NewScheme() + utilruntime.Must(clientgoscheme.AddToScheme(newScheme)) + utilruntime.Must(v1alpha1.AddToScheme(newScheme)) + fakeClient := fake.NewClientBuilder().WithScheme(newScheme).WithObjects(bm1).Build() + + subnet1 := &v1alpha1.Subnet{ + ObjectMeta: metav1.ObjectMeta{Name: "child", Namespace: crNS}, + } + subnet2 := &v1alpha1.Subnet{ + ObjectMeta: metav1.ObjectMeta{Name: "parent", Namespace: crNS}, + } + subnet3 := &v1alpha1.Subnet{ + ObjectMeta: metav1.ObjectMeta{Name: "child2", Namespace: crNS}, + } + + requeueBindingMapsBySubnetDelete(ctx, fakeClient, subnet1, myQueue) + require.Equal(t, 1, myQueue.Len()) + item, _ := myQueue.Get() + assert.Equal(t, req, item) + myQueue.Done(item) + + requeueBindingMapsBySubnetUpdate(ctx, fakeClient, subnet2, subnet2, myQueue) + require.Equal(t, 1, myQueue.Len()) + item, _ = myQueue.Get() + assert.Equal(t, req, item) + myQueue.Done(item) + + requeueBindingMapsBySubnetUpdate(ctx, fakeClient, subnet3, subnet3, myQueue) + require.Equal(t, 0, myQueue.Len()) +} + +func TestRequeueSubnetConnectionBindingMapsBySubnetSet(t *testing.T) { + myQueue := workqueue.NewTypedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[reconcile.Request]()) + defer myQueue.ShutDown() + + ctx := context.TODO() + crName := "binding1" + crNS := "default" + bm1 := &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: metav1.ObjectMeta{ + UID: "binding-uuid", + Namespace: crNS, + Name: crName, + }, + Spec: v1alpha1.SubnetConnectionBindingMapSpec{ + SubnetName: "child", + TargetSubnetSetName: "parent", + VLANTrafficTag: 101, + }, + } + req := reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: crName, + Namespace: crNS, + }, + } + newScheme := runtime.NewScheme() + utilruntime.Must(clientgoscheme.AddToScheme(newScheme)) + utilruntime.Must(v1alpha1.AddToScheme(newScheme)) + fakeClient := fake.NewClientBuilder().WithScheme(newScheme).WithObjects(bm1).Build() + + subnetSet1 := &v1alpha1.SubnetSet{ + ObjectMeta: metav1.ObjectMeta{Name: "child", Namespace: crNS}, + } + subnetSet2 := &v1alpha1.SubnetSet{ + ObjectMeta: metav1.ObjectMeta{Name: "parent", Namespace: crNS}, + } + + requeueBindingMapsBySubnetSetDelete(ctx, fakeClient, subnetSet1, myQueue) + require.Equal(t, 0, myQueue.Len()) + requeueBindingMapsBySubnetSetDelete(ctx, fakeClient, subnetSet2, myQueue) + require.Equal(t, 1, myQueue.Len()) + item, _ := myQueue.Get() + assert.Equal(t, req, item) + myQueue.Done(item) + + requeueBindingMapsBySubnetSetUpdate(ctx, fakeClient, subnetSet1, subnetSet1, myQueue) + require.Equal(t, 0, myQueue.Len()) + requeueBindingMapsBySubnetSetUpdate(ctx, fakeClient, subnetSet2, subnetSet2, myQueue) + require.Equal(t, 1, myQueue.Len()) + item, _ = myQueue.Get() + assert.Equal(t, req, item) + myQueue.Done(item) +} diff --git a/pkg/controllers/subnetset/subnetbinding_handler.go b/pkg/controllers/subnetset/subnetbinding_handler.go new file mode 100644 index 000000000..5d8de0097 --- /dev/null +++ b/pkg/controllers/subnetset/subnetbinding_handler.go @@ -0,0 +1,101 @@ +package subnetset + +import ( + "context" + "time" + + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/util/workqueue" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1" + "github.com/vmware-tanzu/nsx-operator/pkg/controllers/common" + servicecommon "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/common" +) + +var ( + removeFinalizerDelay = 1 * time.Second +) + +func requeueSubnetSetByBindingMapUpdate(ctx context.Context, c client.Client, objOld, objNew client.Object, q workqueue.TypedRateLimitingInterface[reconcile.Request]) { + bindingMap := objNew.(*v1alpha1.SubnetConnectionBindingMap) + if bindingMap.Spec.TargetSubnetSetName == "" { + return + } + needFinalizer := common.IsObjectReady(bindingMap.Status.Conditions) + err := enqueue(ctx, c, bindingMap.Namespace, bindingMap.Spec.TargetSubnetSetName, needFinalizer, q) + if err != nil { + log.Error(err, "Failed to requeue SubnetSet", "Namespace", bindingMap.Namespace, "Name", bindingMap.Spec.TargetSubnetSetName) + } + oldBM := objOld.(*v1alpha1.SubnetConnectionBindingMap) + if common.IsObjectReady(oldBM.Status.Conditions) && oldBM.Spec.TargetSubnetSetName != "" && + oldBM.Spec.TargetSubnetSetName != bindingMap.Spec.TargetSubnetSetName { + // Remove the finalizer from old target Subnet if it is not used. + err := enqueue(ctx, c, bindingMap.Namespace, oldBM.Spec.TargetSubnetSetName, false, q) + if err != nil { + log.Error(err, "Failed to requeue SubnetSet", "Namespace", bindingMap.Namespace, "Name", oldBM.Spec.SubnetName) + return + } + } +} + +func enqueue(ctx context.Context, c client.Client, namespace, name string, needFinalizer bool, q workqueue.TypedRateLimitingInterface[reconcile.Request]) error { + subnetSetCR := &v1alpha1.SubnetSet{} + subnetKey := types.NamespacedName{Namespace: namespace, Name: name} + err := c.Get(ctx, subnetKey, subnetSetCR) + if err != nil { + log.Error(err, "Failed to get Subnet CR", "key", subnetKey.String()) + return err + } + addedFinalizer := controllerutil.ContainsFinalizer(subnetSetCR, servicecommon.SubnetSetFinalizerName) + if addedFinalizer != needFinalizer { + req := reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: name, + Namespace: namespace, + }, + } + if needFinalizer { + q.Add(req) + } else { + // Enqueue the SubnetSet request with 1s delay with the SubnetConnectionBindingMap deletion event. + // This is because when SubnetSet controller watches the deletion event at the same time as + // SubnetConnectionBindingMap Controller, and it may cause a wrong result of the SubnetSet dependency + // calculation. So we leave 1s to ensure the SubnetSet Controller reconciles SubnetSet status after + // the binding calculation is completed. + q.AddAfter(req, removeFinalizerDelay) + } + log.Info("Requeue SubnetSet", "key", subnetKey.String()) + } + return nil +} + +func requeueSubnetSetByBindingMapDelete(ctx context.Context, c client.Client, obj client.Object, q workqueue.TypedRateLimitingInterface[reconcile.Request]) { + bindingMap := obj.(*v1alpha1.SubnetConnectionBindingMap) + if bindingMap.Spec.TargetSubnetSetName == "" { + return + } + err := enqueue(ctx, c, bindingMap.Namespace, bindingMap.Spec.TargetSubnetSetName, false, q) + if err != nil { + log.Error(err, "Failed to requeue SubnetSet", "Namespace", bindingMap.Namespace, "Name", bindingMap.Spec.TargetSubnetSetName) + return + } +} + +func (r *SubnetSetReconciler) subnetSetHasBindings(subnetSetCRUID string) []*v1alpha1.SubnetConnectionBindingMap { + vpcSubnets := r.SubnetService.ListSubnetCreatedBySubnetSet(subnetSetCRUID) + if len(vpcSubnets) == 0 { + log.Info("No VpcSubnet found with SubnetSet", "SubnetSetID", subnetSetCRUID) + return nil + } + bindingMaps := make([]*v1alpha1.SubnetConnectionBindingMap, 0) + for _, vpcSubnet := range vpcSubnets { + bindings := r.BindingService.GetSubnetConnectionBindingMapCRsBySubnet(vpcSubnet) + if len(bindings) > 0 { + bindingMaps = append(bindingMaps, bindings...) + } + } + return bindingMaps +} diff --git a/pkg/controllers/subnetset/subnetbinding_handler_test.go b/pkg/controllers/subnetset/subnetbinding_handler_test.go new file mode 100644 index 000000000..f7092b570 --- /dev/null +++ b/pkg/controllers/subnetset/subnetbinding_handler_test.go @@ -0,0 +1,153 @@ +package subnetset + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/util/workqueue" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1" + servicecommon "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/common" +) + +var ( + bm1 = &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "binding1", + Namespace: "default", + }, + Spec: v1alpha1.SubnetConnectionBindingMapSpec{ + SubnetName: "child", + TargetSubnetName: "parent", + VLANTrafficTag: 101, + }, + Status: v1alpha1.SubnetConnectionBindingMapStatus{ + Conditions: []v1alpha1.Condition{ + { + Type: v1alpha1.Ready, + Status: corev1.ConditionTrue, + }, + }, + }, + } + + bm2 = &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "binding1", + Namespace: "default", + }, + Spec: v1alpha1.SubnetConnectionBindingMapSpec{ + SubnetName: "child1", + TargetSubnetSetName: "parentSet", + VLANTrafficTag: 102, + }, + } + bm3 = &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "binding1", + Namespace: "default", + }, + Spec: v1alpha1.SubnetConnectionBindingMapSpec{ + SubnetName: "child1", + TargetSubnetSetName: "parentSet2", + VLANTrafficTag: 102, + }, + } + + subnet1 = &v1alpha1.Subnet{ + ObjectMeta: metav1.ObjectMeta{Name: "child", Namespace: "default"}, + } + subnet2 = &v1alpha1.Subnet{ + ObjectMeta: metav1.ObjectMeta{Name: "parent", Namespace: "default"}, + } + subnetSet1 = &v1alpha1.SubnetSet{ + ObjectMeta: metav1.ObjectMeta{Name: "parentSet", Namespace: "default", Finalizers: []string{servicecommon.SubnetSetFinalizerName}}, + } + subnetSet2 = &v1alpha1.SubnetSet{ + ObjectMeta: metav1.ObjectMeta{Name: "parentSet2", Namespace: "default"}, + } + req1 = reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: "parentSet", + Namespace: "default", + }, + } + req2 = reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: "parentSet2", + Namespace: "default", + }, + } +) + +func TestRequeueSubnetByBindingMap(t *testing.T) { + oriRemoveFinalizerDelay := removeFinalizerDelay + removeFinalizerDelay = -1 + t.Cleanup(func() { + removeFinalizerDelay = oriRemoveFinalizerDelay + }) + + myQueue := workqueue.NewTypedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[reconcile.Request]()) + defer myQueue.ShutDown() + + ctx := context.TODO() + newScheme := runtime.NewScheme() + utilruntime.Must(clientgoscheme.AddToScheme(newScheme)) + utilruntime.Must(v1alpha1.AddToScheme(newScheme)) + + fakeClient := fake.NewClientBuilder().WithScheme(newScheme).WithObjects(subnet1, subnet2, subnetSet1, subnetSet2).Build() + + requeueSubnetSetByBindingMapUpdate(ctx, fakeClient, bm1, bm1, myQueue) + require.Equal(t, 0, myQueue.Len()) + + bm2.Status = v1alpha1.SubnetConnectionBindingMapStatus{ + Conditions: []v1alpha1.Condition{ + { + Type: v1alpha1.Ready, + Status: corev1.ConditionTrue, + }, + }, + } + requeueSubnetSetByBindingMapUpdate(ctx, fakeClient, bm2, bm2, myQueue) + require.Equal(t, 0, myQueue.Len()) + requeueSubnetSetByBindingMapDelete(ctx, fakeClient, bm2, myQueue) + require.Equal(t, 1, myQueue.Len()) + item, _ := myQueue.Get() + assert.Equal(t, req1, item) + myQueue.Done(item) + + bm3.Status = v1alpha1.SubnetConnectionBindingMapStatus{ + Conditions: []v1alpha1.Condition{ + { + Type: v1alpha1.Ready, + Status: corev1.ConditionTrue, + }, + }, + } + requeueSubnetSetByBindingMapUpdate(ctx, fakeClient, bm3, bm3, myQueue) + require.Equal(t, 1, myQueue.Len()) + item, _ = myQueue.Get() + assert.Equal(t, req2, item) + myQueue.Done(item) + requeueSubnetSetByBindingMapDelete(ctx, fakeClient, bm3, myQueue) + require.Equal(t, 0, myQueue.Len()) + + requeueSubnetSetByBindingMapUpdate(ctx, fakeClient, bm2, bm3, myQueue) + require.Equal(t, 2, myQueue.Len()) + item, _ = myQueue.Get() + assert.Equal(t, req2, item) + myQueue.Done(item) + item, _ = myQueue.Get() + assert.Equal(t, req1, item) + myQueue.Done(item) +} diff --git a/pkg/controllers/subnetset/subnetset_controller.go b/pkg/controllers/subnetset/subnetset_controller.go index a0ca8e5c4..a57c32c72 100644 --- a/pkg/controllers/subnetset/subnetset_controller.go +++ b/pkg/controllers/subnetset/subnetset_controller.go @@ -18,6 +18,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" @@ -26,6 +27,7 @@ import ( "github.com/vmware-tanzu/nsx-operator/pkg/logger" 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" nsxutil "github.com/vmware-tanzu/nsx-operator/pkg/nsx/util" "github.com/vmware-tanzu/nsx-operator/pkg/util" ) @@ -36,6 +38,11 @@ var ( ResultRequeue = common.ResultRequeue ResultRequeueAfter5mins = common.ResultRequeueAfter5mins MetricResTypeSubnetSet = common.MetricResTypeSubnetSet + + msgFailAddFinalizer = "Failed to add finalizer on SubnetSet for the dependency by SubnetConnectionBindingMap %s" + msgFailDelFinalizer = "Failed to remove finalizer on SubnetSet after no the dependency by SubnetConnectionBindingMaps" + msgDeleteInUse = "SubnetSet is used by SubnetConnectionBindingMap %s and not able to delete" + reasonDeleteInUse = "SubnetSetInUse" ) // SubnetSetReconciler reconciles a SubnetSet object @@ -45,6 +52,7 @@ type SubnetSetReconciler struct { SubnetService *subnet.SubnetService SubnetPortService servicecommon.SubnetPortServiceProvider VPCService servicecommon.VPCServiceProvider + BindingService *subnetbinding.BindingService Recorder record.EventRecorder StatusUpdater common.StatusUpdater } @@ -70,8 +78,40 @@ func (r *SubnetSetReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( log.Error(err, "Unable to fetch SubnetSet CR", "SubnetSet", req.NamespacedName) return ResultRequeue, err } + + hasBindings := r.subnetSetHasBindings(string(subnetsetCR.UID)) + if len(hasBindings) > 0 { + if !controllerutil.ContainsFinalizer(subnetsetCR, servicecommon.SubnetSetFinalizerName) { + controllerutil.AddFinalizer(subnetsetCR, servicecommon.SubnetSetFinalizerName) + if err := r.Client.Update(ctx, subnetsetCR); err != nil { + log.Error(err, "Failed to add finalizer", "subnetSet", req.NamespacedName) + r.StatusUpdater.UpdateFail(ctx, subnetsetCR, err, "Unable to add finalizer on SubnetSet used by SubnetConnectionBindingMap", + setSubnetSetReadyStatusFalse, fmt.Sprintf(msgFailAddFinalizer, hasBindings[0].Name)) + return ResultRequeue, err + } + } + } else { + if controllerutil.ContainsFinalizer(subnetsetCR, servicecommon.SubnetSetFinalizerName) { + controllerutil.RemoveFinalizer(subnetsetCR, servicecommon.SubnetSetFinalizerName) + if err := r.Client.Update(ctx, subnetsetCR); err != nil { + log.Error(err, "Failed to delete finalizer", "subnet", req.NamespacedName) + r.StatusUpdater.UpdateFail(ctx, subnetsetCR, err, "Unable to remove finalizer from SubnetSet", + setSubnetSetReadyStatusFalse, fmt.Sprint(msgFailDelFinalizer)) + return ResultRequeue, err + } + } + } + if !subnetsetCR.ObjectMeta.DeletionTimestamp.IsZero() { r.StatusUpdater.IncreaseDeleteTotal() + if len(hasBindings) > 0 { + err := fmt.Errorf("subnetSet %s is used by SubnetConnectionBindingMaps %s", req.String(), hasBindings[0].GetName()) + log.Error(err, "Failed to delete SubnetSet CR because it is used by SubnetConnectionBindingMaps, retrying", "Subnet", req.NamespacedName) + r.StatusUpdater.DeleteFail(req.NamespacedName, nil, err) + r.setSubnetDeletionFailedStatus(ctx, subnetsetCR, metav1.Now(), fmt.Sprintf(msgDeleteInUse, hasBindings[0].GetName()), reasonDeleteInUse) + return ResultRequeue, err + } + err := r.deleteSubnetForSubnetSet(*subnetsetCR, false, false) if err != nil { r.StatusUpdater.DeleteFail(req.NamespacedName, nil, err) @@ -149,7 +189,7 @@ func setSubnetSetReadyStatusTrue(client client.Client, ctx context.Context, obj updateSubnetSetStatusConditions(client, ctx, subnetSet, newConditions) } -func setSubnetSetReadyStatusFalse(client client.Client, ctx context.Context, obj client.Object, transitionTime metav1.Time, err error, _ ...interface{}) { +func setSubnetSetReadyStatusFalse(client client.Client, ctx context.Context, obj client.Object, transitionTime metav1.Time, err error, args ...interface{}) { subnetSet := obj.(*v1alpha1.SubnetSet) newConditions := []v1alpha1.Condition{ { @@ -160,12 +200,36 @@ func setSubnetSetReadyStatusFalse(client client.Client, ctx context.Context, obj LastTransitionTime: transitionTime, }, } - if err != nil { - newConditions[0].Message = fmt.Sprintf("Error occurred while processing the SubnetSet CR. Please check the config and try again. Error: %v", err) + + if len(args) > 0 { + newConditions[0].Message = args[0].(string) + } else { + if err != nil { + newConditions[0].Message = fmt.Sprintf("Error occurred while processing the SubnetSet CR. Please check the config and try again. Error: %v", err) + } } updateSubnetSetStatusConditions(client, ctx, subnetSet, newConditions) } +func (r *SubnetSetReconciler) setSubnetDeletionFailedStatus(ctx context.Context, subnetSet *v1alpha1.SubnetSet, transitionTime metav1.Time, msg string, reason string) { + newConditions := []v1alpha1.Condition{ + { + Type: v1alpha1.DeleteFailure, + Status: v1.ConditionTrue, + Message: "SubnetSet could not be deleted", + Reason: "NSXOperationFailed", + LastTransitionTime: transitionTime, + }, + } + if msg != "" { + newConditions[0].Message = msg + } + if reason != "" { + newConditions[0].Reason = reason + } + updateSubnetSetStatusConditions(r.Client, ctx, subnetSet, newConditions) +} + func updateSubnetSetStatusConditions(client client.Client, ctx context.Context, subnetSet *v1alpha1.SubnetSet, newConditions []v1alpha1.Condition) { conditionsUpdated := false for i := range newConditions { @@ -220,6 +284,16 @@ func (r *SubnetSetReconciler) setupWithManager(mgr ctrl.Manager) error { &EnqueueRequestForNamespace{Client: mgr.GetClient()}, builder.WithPredicates(PredicateFuncsNs), ). + Watches( + &v1alpha1.SubnetConnectionBindingMap{}, + &common.EnqueueRequestForDependency{ + Client: r.Client, + ResourceType: "SubnetConnectionBindingMap", + RequeueByUpdate: requeueSubnetSetByBindingMapUpdate, + RequeueByDelete: requeueSubnetSetByBindingMapDelete, + }, + builder.WithPredicates(common.PredicateFuncsWithBindingMapUpdateDelete), + ). Complete(r) } @@ -252,7 +326,7 @@ func (r *SubnetSetReconciler) CollectGarbage(ctx context.Context) { for subnetSetID := range subnetSetIDsToDelete { nsxSubnets := r.SubnetService.ListSubnetCreatedBySubnetSet(subnetSetID) log.Info("SubnetSet garbage collection, cleaning stale Subnets for SubnetSet", "Count", len(nsxSubnets)) - if _, err := r.deleteSubnets(nsxSubnets); err != nil { + if _, err := r.deleteSubnets(nsxSubnets, true); err != nil { log.Error(err, "SubnetSet garbage collection, failed to delete NSX subnet", "SubnetSetUID", subnetSetID) r.StatusUpdater.IncreaseDeleteFailTotal() } else { @@ -269,7 +343,11 @@ func (r *SubnetSetReconciler) deleteSubnetBySubnetSetName(ctx context.Context, s func (r *SubnetSetReconciler) deleteSubnetForSubnetSet(subnetSet v1alpha1.SubnetSet, updateStatus, ignoreStaleSubnetPort bool) error { nsxSubnets := r.SubnetService.SubnetStore.GetByIndex(servicecommon.TagScopeSubnetSetCRUID, string(subnetSet.GetUID())) - hasStaleSubnetPort, deleteErr := r.deleteSubnets(nsxSubnets) + // If ignoreStaleSubnetPort is true, we will actively delete the existing SubnetConnectionBindingMaps connected to the + // corresponding NSX Subnet. This happens in the GC case to scale-in the NSX Subnet if no subnet Port exists. + // For SubnetSet CR deletion event, we don't delete the existing SubnetConnectionBindingMaps but let the + // SubnetConnectionBindingMap controller do it after the binding CR is removed.. + hasStaleSubnetPort, deleteErr := r.deleteSubnets(nsxSubnets, ignoreStaleSubnetPort) if updateStatus { if err := r.SubnetService.UpdateSubnetSetStatus(&subnetSet); err != nil { return err @@ -287,28 +365,38 @@ func (r *SubnetSetReconciler) deleteSubnetForSubnetSet(subnetSet v1alpha1.Subnet // deleteSubnets deletes all the specified NSX Subnets. // If any of the Subnets have stale SubnetPorts, they are skipped. The final result returns true. // If there is an error while deleting any NSX Subnet, it is skipped, and the final result returns an error. -func (r *SubnetSetReconciler) deleteSubnets(nsxSubnets []*model.VpcSubnet) (hasStalePort bool, err error) { +func (r *SubnetSetReconciler) deleteSubnets(nsxSubnets []*model.VpcSubnet, deleteBindingMaps bool) (hasStalePort bool, err error) { if len(nsxSubnets) == 0 { return } var deleteErrs []error for _, nsxSubnet := range nsxSubnets { r.SubnetService.LockSubnet(nsxSubnet.Path) - portNums := len(r.SubnetPortService.GetPortsOfSubnet(*nsxSubnet.Id)) - if portNums > 0 { - r.SubnetService.UnlockSubnet(nsxSubnet.Path) - hasStalePort = true - log.Info("Skipped deleting NSX Subnet due to stale ports", "nsxSubnet", *nsxSubnet.Id) - continue - } - if err := r.SubnetService.DeleteSubnet(*nsxSubnet); err != nil { - r.SubnetService.UnlockSubnet(nsxSubnet.Path) - deleteErr := fmt.Errorf("failed to delete NSX Subnet/%s: %+v", *nsxSubnet.Id, err) - deleteErrs = append(deleteErrs, deleteErr) - log.Error(deleteErr, "Skipping to next Subnet") - continue - } - r.SubnetService.UnlockSubnet(nsxSubnet.Path) + func() { + defer r.SubnetService.UnlockSubnet(nsxSubnet.Path) + + portNums := len(r.SubnetPortService.GetPortsOfSubnet(*nsxSubnet.Id)) + if portNums > 0 { + hasStalePort = true + log.Info("Skipped deleting NSX Subnet due to stale ports", "nsxSubnet", *nsxSubnet.Id) + return + } + + if deleteBindingMaps { + if err := r.BindingService.DeleteSubnetConnectionBindingMapsByParentSubnets([]*model.VpcSubnet{nsxSubnet}); err != nil { + deleteErr := fmt.Errorf("failed to delete NSX SubnetConnectionBindingMaps referred to Subnet/%s: %+v", *nsxSubnet.Id, err) + deleteErrs = append(deleteErrs, deleteErr) + log.Error(deleteErr, "Skipping to next Subnet") + return + } + } + + if err := r.SubnetService.DeleteSubnet(*nsxSubnet); err != nil { + deleteErr := fmt.Errorf("failed to delete NSX Subnet/%s: %+v", *nsxSubnet.Id, err) + deleteErrs = append(deleteErrs, deleteErr) + log.Error(deleteErr, "Skipping to next Subnet") + } + }() } if len(deleteErrs) > 0 { err = fmt.Errorf("multiple errors occurred while deleting Subnets: %v", deleteErrs) @@ -334,7 +422,8 @@ func (r *SubnetSetReconciler) deleteStaleSubnets(ctx context.Context, nsxSubnets nsxSubnetsToDelete = append(nsxSubnetsToDelete, nsxSubnet) } log.Info("Cleaning stale Subnets for SubnetSet", "Count", len(nsxSubnetsToDelete)) - hasStaleSubnetPort, err := r.deleteSubnets(nsxSubnetsToDelete) + // We also actively delete the existing SubnetConnectionBindingMaps connected to the stale NSX Subnets. + hasStaleSubnetPort, err := r.deleteSubnets(nsxSubnetsToDelete, true) if err != nil || hasStaleSubnetPort { return fmt.Errorf("failed to delete stale Subnets, error: %v, hasStaleSubnetPort: %t", err, hasStaleSubnetPort) } @@ -343,15 +432,17 @@ func (r *SubnetSetReconciler) deleteStaleSubnets(ctx context.Context, nsxSubnets func StartSubnetSetController(mgr ctrl.Manager, subnetService *subnet.SubnetService, subnetPortService servicecommon.SubnetPortServiceProvider, vpcService servicecommon.VPCServiceProvider, - hookServer webhook.Server, + bindingService *subnetbinding.BindingService, hookServer webhook.Server, ) error { subnetsetReconciler := &SubnetSetReconciler{ Client: mgr.GetClient(), Scheme: mgr.GetScheme(), SubnetService: subnetService, SubnetPortService: subnetPortService, - VPCService: vpcService, - Recorder: mgr.GetEventRecorderFor("subnetset-controller"), + BindingService: bindingService, + + VPCService: vpcService, + Recorder: mgr.GetEventRecorderFor("subnetset-controller"), } subnetsetReconciler.StatusUpdater = common.NewStatusUpdater(subnetsetReconciler.Client, subnetsetReconciler.SubnetService.NSXConfig, subnetsetReconciler.Recorder, MetricResTypeSubnetSet, "Subnet", "SubnetSet") if err := subnetsetReconciler.Start(mgr, hookServer); err != nil { diff --git a/pkg/controllers/subnetset/subnetset_controller_test.go b/pkg/controllers/subnetset/subnetset_controller_test.go index f9375c878..3ae051d0a 100644 --- a/pkg/controllers/subnetset/subnetset_controller_test.go +++ b/pkg/controllers/subnetset/subnetset_controller_test.go @@ -34,6 +34,7 @@ import ( "github.com/vmware-tanzu/nsx-operator/pkg/nsx" "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/subnetport" "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/vpc" ) @@ -108,7 +109,12 @@ func TestReconcile(t *testing.T) { name: "Create a SubnetSet with find VPCNetworkConfig error", expectRes: ResultRequeue, expectErrStr: "failed to find VPCNetworkConfig for Namespace", - patches: nil, + patches: func(r *SubnetSetReconciler) *gomonkey.Patches { + patches := gomonkey.ApplyPrivateMethod(reflect.TypeOf(r), "subnetSetHasBindings", func(_ *SubnetSetReconciler, subnetSetCRUID string) []*v1alpha1.SubnetConnectionBindingMap { + return []*v1alpha1.SubnetConnectionBindingMap{} + }) + return patches + }, }, { // TODO: should check the SubnetSet status has error message, which contains 'ipv4SubnetSize has invalid size' @@ -120,6 +126,9 @@ func TestReconcile(t *testing.T) { patches := gomonkey.ApplyMethod(reflect.TypeOf(r.VPCService), "GetVPCNetworkConfigByNamespace", func(_ *vpc.VPCService, ns string) *common.VPCNetworkConfigInfo { return vpcnetworkInfo }) + patches.ApplyPrivateMethod(reflect.TypeOf(r), "subnetSetHasBindings", func(_ *SubnetSetReconciler, subnetSetCRUID string) []*v1alpha1.SubnetConnectionBindingMap { + return []*v1alpha1.SubnetConnectionBindingMap{} + }) return patches }, }, @@ -141,6 +150,9 @@ func TestReconcile(t *testing.T) { &vpcSubnet, } }) + patches.ApplyPrivateMethod(reflect.TypeOf(r), "subnetSetHasBindings", func(_ *SubnetSetReconciler, subnetSetCRUID string) []*v1alpha1.SubnetConnectionBindingMap { + return []*v1alpha1.SubnetConnectionBindingMap{} + }) return patches }, }, @@ -155,6 +167,10 @@ func TestReconcile(t *testing.T) { return vpcnetworkInfo }) + patches.ApplyPrivateMethod(reflect.TypeOf(r), "subnetSetHasBindings", func(_ *SubnetSetReconciler, subnetSetCRUID string) []*v1alpha1.SubnetConnectionBindingMap { + return []*v1alpha1.SubnetConnectionBindingMap{} + }) + tags := []model.Tag{{Scope: common.String(common.TagScopeVMNamespace), Tag: common.String(ns)}} patches.ApplyMethod(reflect.TypeOf(r.SubnetService.SubnetStore), "GetByIndex", func(_ *subnet.SubnetStore, key string, value string) []*model.VpcSubnet { id1 := "fake-id" @@ -180,6 +196,9 @@ func TestReconcile(t *testing.T) { patches := gomonkey.ApplyMethod(reflect.TypeOf(r.VPCService), "GetVPCNetworkConfigByNamespace", func(_ *vpc.VPCService, ns string) *common.VPCNetworkConfigInfo { return vpcnetworkInfo }) + patches.ApplyPrivateMethod(reflect.TypeOf(r), "subnetSetHasBindings", func(_ *SubnetSetReconciler, subnetSetCRUID string) []*v1alpha1.SubnetConnectionBindingMap { + return []*v1alpha1.SubnetConnectionBindingMap{} + }) patches.ApplyMethod(reflect.TypeOf(r.SubnetService.SubnetStore), "GetByIndex", func(_ *subnet.SubnetStore, key string, value string) []*model.VpcSubnet { id1 := "fake-id" @@ -211,6 +230,9 @@ func TestReconcile(t *testing.T) { patches := gomonkey.ApplyMethod(reflect.TypeOf(r.VPCService), "GetVPCNetworkConfigByNamespace", func(_ *vpc.VPCService, ns string) *common.VPCNetworkConfigInfo { return vpcnetworkInfo }) + patches.ApplyPrivateMethod(reflect.TypeOf(r), "subnetSetHasBindings", func(_ *SubnetSetReconciler, subnetSetCRUID string) []*v1alpha1.SubnetConnectionBindingMap { + return []*v1alpha1.SubnetConnectionBindingMap{} + }) patches.ApplyMethod(reflect.TypeOf(r.SubnetService.SubnetStore), "GetByIndex", func(_ *subnet.SubnetStore, key string, value string) []*model.VpcSubnet { id1 := "fake-id" @@ -306,6 +328,10 @@ func TestReconcile_DeleteSubnetSet(t *testing.T) { } }) + patches.ApplyMethod(reflect.TypeOf(r.BindingService), "DeleteSubnetConnectionBindingMapsByParentSubnets", func(_ *subnetbinding.BindingService, parentSubnets []*model.VpcSubnet) error { + return nil + }) + patches.ApplyMethod(reflect.TypeOf(r.SubnetPortService), "GetPortsOfSubnet", func(_ *subnetport.SubnetPortService, _ string) (ports []*model.VpcSubnetPort) { return nil }) @@ -341,6 +367,10 @@ func TestReconcile_DeleteSubnetSet(t *testing.T) { } }) + patches.ApplyMethod(reflect.TypeOf(r.BindingService), "DeleteSubnetConnectionBindingMapsByParentSubnets", func(_ *subnetbinding.BindingService, parentSubnets []*model.VpcSubnet) error { + return nil + }) + patches.ApplyMethod(reflect.TypeOf(r.SubnetPortService), "GetPortsOfSubnet", func(_ *subnetport.SubnetPortService, _ string) (ports []*model.VpcSubnetPort) { id := "fake-subnetport-0" return []*model.VpcSubnetPort{ @@ -385,6 +415,10 @@ func TestReconcile_DeleteSubnetSet(t *testing.T) { } }) + patches.ApplyMethod(reflect.TypeOf(r.BindingService), "DeleteSubnetConnectionBindingMapsByParentSubnets", func(_ *subnetbinding.BindingService, parentSubnets []*model.VpcSubnet) error { + return nil + }) + patches.ApplyMethod(reflect.TypeOf(r.SubnetPortService), "GetPortsOfSubnet", func(_ *subnetport.SubnetPortService, _ string) (ports []*model.VpcSubnetPort) { return nil }) @@ -449,8 +483,13 @@ func TestReconcile_DeleteSubnetSet_WithFinalizer(t *testing.T) { &vpcSubnet, } }) + defer patches.Reset() + patches.ApplyPrivateMethod(reflect.TypeOf(r), "subnetSetHasBindings", func(_ *SubnetSetReconciler, subnetSetCRUID string) []*v1alpha1.SubnetConnectionBindingMap { + return []*v1alpha1.SubnetConnectionBindingMap{} + }) + patches.ApplyMethod(reflect.TypeOf(r.SubnetPortService), "GetPortsOfSubnet", func(_ *subnetport.SubnetPortService, _ string) (ports []*model.VpcSubnetPort) { return nil }) @@ -542,6 +581,9 @@ func TestSubnetSetReconciler_CollectGarbage(t *testing.T) { patches.ApplyMethod(reflect.TypeOf(r.SubnetPortService), "GetPortsOfSubnet", func(_ *subnetport.SubnetPortService, _ string) (ports []*model.VpcSubnetPort) { return nil }) + patches.ApplyMethod(reflect.TypeOf(r.BindingService), "DeleteSubnetConnectionBindingMapsByParentSubnets", func(_ *subnetbinding.BindingService, parentSubnets []*model.VpcSubnet) error { + return nil + }) patches.ApplyMethod(reflect.TypeOf(r.SubnetService), "DeleteSubnet", func(_ *subnet.SubnetService, subnet model.VpcSubnet) error { return nil }) @@ -628,6 +670,10 @@ func TestStartSubnetSetController(t *testing.T) { Service: common.Service{}, SubnetPortStore: nil, } + subnetBindingService := &subnetbinding.BindingService{ + Service: common.Service{}, + BindingStore: nil, + } mockMgr := &MockManager{scheme: runtime.NewScheme()} @@ -695,7 +741,7 @@ func TestStartSubnetSetController(t *testing.T) { patches := testCase.patches() defer patches.Reset() - err := StartSubnetSetController(mockMgr, subnetService, subnetPortService, vpcService, testCase.webHookServer) + err := StartSubnetSetController(mockMgr, subnetService, subnetPortService, vpcService, subnetBindingService, testCase.webHookServer) if testCase.expectErrStr != "" { assert.ErrorContains(t, err, testCase.expectErrStr) diff --git a/pkg/mock/orgrootclient/client.go b/pkg/mock/orgrootclient/client.go new file mode 100644 index 000000000..65e0c75a4 --- /dev/null +++ b/pkg/mock/orgrootclient/client.go @@ -0,0 +1,64 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/vmware/vsphere-automation-sdk-go/services/nsxt (interfaces: OrgRootClient) + +// Package mocks is a generated GoMock package. +package mocks + +import ( + reflect "reflect" + + gomock "github.com/golang/mock/gomock" + model "github.com/vmware/vsphere-automation-sdk-go/services/nsxt/model" +) + +// MockOrgRootClient is a mock of OrgRootClient interface. +type MockOrgRootClient struct { + ctrl *gomock.Controller + recorder *MockOrgRootClientMockRecorder +} + +// MockOrgRootClientMockRecorder is the mock recorder for MockOrgRootClient. +type MockOrgRootClientMockRecorder struct { + mock *MockOrgRootClient +} + +// NewMockOrgRootClient creates a new mock instance. +func NewMockOrgRootClient(ctrl *gomock.Controller) *MockOrgRootClient { + mock := &MockOrgRootClient{ctrl: ctrl} + mock.recorder = &MockOrgRootClientMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockOrgRootClient) EXPECT() *MockOrgRootClientMockRecorder { + return m.recorder +} + +// Get mocks base method. +func (m *MockOrgRootClient) Get(arg0, arg1, arg2 *string) (model.OrgRoot, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Get", arg0, arg1, arg2) + ret0, _ := ret[0].(model.OrgRoot) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Get indicates an expected call of Get. +func (mr *MockOrgRootClientMockRecorder) Get(arg0, arg1, arg2 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockOrgRootClient)(nil).Get), arg0, arg1, arg2) +} + +// Patch mocks base method. +func (m *MockOrgRootClient) Patch(arg0 model.OrgRoot, arg1 *bool) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Patch", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// Patch indicates an expected call of Patch. +func (mr *MockOrgRootClientMockRecorder) Patch(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Patch", reflect.TypeOf((*MockOrgRootClient)(nil).Patch), arg0, arg1) +} diff --git a/pkg/mock/searchclient/client.go b/pkg/mock/searchclient/client.go new file mode 100644 index 000000000..915818d8c --- /dev/null +++ b/pkg/mock/searchclient/client.go @@ -0,0 +1,50 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/vmware/vsphere-automation-sdk-go/services/nsxt/search (interfaces: QueryClient) + +// Package mocks is a generated GoMock package. +package mocks + +import ( + reflect "reflect" + + gomock "github.com/golang/mock/gomock" + model "github.com/vmware/vsphere-automation-sdk-go/services/nsxt/model" +) + +// MockQueryClient is a mock of QueryClient interface. +type MockQueryClient struct { + ctrl *gomock.Controller + recorder *MockQueryClientMockRecorder +} + +// MockQueryClientMockRecorder is the mock recorder for MockQueryClient. +type MockQueryClientMockRecorder struct { + mock *MockQueryClient +} + +// NewMockQueryClient creates a new mock instance. +func NewMockQueryClient(ctrl *gomock.Controller) *MockQueryClient { + mock := &MockQueryClient{ctrl: ctrl} + mock.recorder = &MockQueryClientMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockQueryClient) EXPECT() *MockQueryClientMockRecorder { + return m.recorder +} + +// List mocks base method. +func (m *MockQueryClient) List(arg0 string, arg1, arg2 *string, arg3 *int64, arg4 *bool, arg5 *string) (model.SearchResponse, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "List", arg0, arg1, arg2, arg3, arg4, arg5) + ret0, _ := ret[0].(model.SearchResponse) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// List indicates an expected call of List. +func (mr *MockQueryClientMockRecorder) List(arg0, arg1, arg2, arg3, arg4, arg5 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "List", reflect.TypeOf((*MockQueryClient)(nil).List), arg0, arg1, arg2, arg3, arg4, arg5) +} diff --git a/pkg/mock/subnetconnectionbindingmapclient/client.go b/pkg/mock/subnetconnectionbindingmapclient/client.go new file mode 100644 index 000000000..923dd5ba9 --- /dev/null +++ b/pkg/mock/subnetconnectionbindingmapclient/client.go @@ -0,0 +1,108 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/vmware/vsphere-automation-sdk-go/services/nsxt/orgs/projects/vpcs/subnets (interfaces: SubnetConnectionBindingMapsClient) + +// Package mocks is a generated GoMock package. +package mocks + +import ( + reflect "reflect" + + gomock "github.com/golang/mock/gomock" + model "github.com/vmware/vsphere-automation-sdk-go/services/nsxt/model" +) + +// MockSubnetConnectionBindingMapsClient is a mock of SubnetConnectionBindingMapsClient interface. +type MockSubnetConnectionBindingMapsClient struct { + ctrl *gomock.Controller + recorder *MockSubnetConnectionBindingMapsClientMockRecorder +} + +// MockSubnetConnectionBindingMapsClientMockRecorder is the mock recorder for MockSubnetConnectionBindingMapsClient. +type MockSubnetConnectionBindingMapsClientMockRecorder struct { + mock *MockSubnetConnectionBindingMapsClient +} + +// NewMockSubnetConnectionBindingMapsClient creates a new mock instance. +func NewMockSubnetConnectionBindingMapsClient(ctrl *gomock.Controller) *MockSubnetConnectionBindingMapsClient { + mock := &MockSubnetConnectionBindingMapsClient{ctrl: ctrl} + mock.recorder = &MockSubnetConnectionBindingMapsClientMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockSubnetConnectionBindingMapsClient) EXPECT() *MockSubnetConnectionBindingMapsClientMockRecorder { + return m.recorder +} + +// Delete mocks base method. +func (m *MockSubnetConnectionBindingMapsClient) Delete(arg0, arg1, arg2, arg3, arg4 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Delete", arg0, arg1, arg2, arg3, arg4) + ret0, _ := ret[0].(error) + return ret0 +} + +// Delete indicates an expected call of Delete. +func (mr *MockSubnetConnectionBindingMapsClientMockRecorder) Delete(arg0, arg1, arg2, arg3, arg4 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Delete", reflect.TypeOf((*MockSubnetConnectionBindingMapsClient)(nil).Delete), arg0, arg1, arg2, arg3, arg4) +} + +// Get mocks base method. +func (m *MockSubnetConnectionBindingMapsClient) Get(arg0, arg1, arg2, arg3, arg4 string) (model.SubnetConnectionBindingMap, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Get", arg0, arg1, arg2, arg3, arg4) + ret0, _ := ret[0].(model.SubnetConnectionBindingMap) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Get indicates an expected call of Get. +func (mr *MockSubnetConnectionBindingMapsClientMockRecorder) Get(arg0, arg1, arg2, arg3, arg4 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockSubnetConnectionBindingMapsClient)(nil).Get), arg0, arg1, arg2, arg3, arg4) +} + +// List mocks base method. +func (m *MockSubnetConnectionBindingMapsClient) List(arg0, arg1, arg2, arg3 string, arg4 *string, arg5 *bool, arg6 *string, arg7 *int64, arg8 *bool, arg9 *string) (model.SubnetConnectionBindingMapListResult, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "List", arg0, arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9) + ret0, _ := ret[0].(model.SubnetConnectionBindingMapListResult) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// List indicates an expected call of List. +func (mr *MockSubnetConnectionBindingMapsClientMockRecorder) List(arg0, arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "List", reflect.TypeOf((*MockSubnetConnectionBindingMapsClient)(nil).List), arg0, arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9) +} + +// Patch mocks base method. +func (m *MockSubnetConnectionBindingMapsClient) Patch(arg0, arg1, arg2, arg3, arg4 string, arg5 model.SubnetConnectionBindingMap) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Patch", arg0, arg1, arg2, arg3, arg4, arg5) + ret0, _ := ret[0].(error) + return ret0 +} + +// Patch indicates an expected call of Patch. +func (mr *MockSubnetConnectionBindingMapsClientMockRecorder) Patch(arg0, arg1, arg2, arg3, arg4, arg5 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Patch", reflect.TypeOf((*MockSubnetConnectionBindingMapsClient)(nil).Patch), arg0, arg1, arg2, arg3, arg4, arg5) +} + +// Update mocks base method. +func (m *MockSubnetConnectionBindingMapsClient) Update(arg0, arg1, arg2, arg3, arg4 string, arg5 model.SubnetConnectionBindingMap) (model.SubnetConnectionBindingMap, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Update", arg0, arg1, arg2, arg3, arg4, arg5) + ret0, _ := ret[0].(model.SubnetConnectionBindingMap) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Update indicates an expected call of Update. +func (mr *MockSubnetConnectionBindingMapsClientMockRecorder) Update(arg0, arg1, arg2, arg3, arg4, arg5 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Update", reflect.TypeOf((*MockSubnetConnectionBindingMapsClient)(nil).Update), arg0, arg1, arg2, arg3, arg4, arg5) +} diff --git a/pkg/nsx/client.go b/pkg/nsx/client.go index e6bb1ed68..11c6f0ced 100644 --- a/pkg/nsx/client.go +++ b/pkg/nsx/client.go @@ -74,34 +74,35 @@ type Client struct { VPCSecurityClient vpcs.SecurityPoliciesClient VPCRuleClient vpc_sp.RulesClient - OrgRootClient nsx_policy.OrgRootClient - ProjectInfraClient projects.InfraClient - VPCClient projects.VpcsClient - VPCConnectivityProfilesClient projects.VpcConnectivityProfilesClient - IPBlockClient project_infra.IpBlocksClient - StaticRouteClient vpcs.StaticRoutesClient - NATRuleClient nat.NatRulesClient - VpcGroupClient vpcs.GroupsClient - PortClient subnets.PortsClient - PortStateClient ports.StateClient - IPPoolClient subnets.IpPoolsClient - IPAllocationClient ip_pools.IpAllocationsClient - SubnetsClient vpcs.SubnetsClient - RealizedStateClient realized_state.RealizedEntitiesClient - IPAddressAllocationClient vpcs.IpAddressAllocationsClient - VPCLBSClient vpcs.VpcLbsClient - VpcLbVirtualServersClient vpcs.VpcLbVirtualServersClient - VpcLbPoolsClient vpcs.VpcLbPoolsClient - VpcAttachmentClient vpcs.AttachmentsClient - ProjectClient orgs.ProjectsClient - TransitGatewayClient projects.TransitGatewaysClient - TransitGatewayAttachmentClient transit_gateways.AttachmentsClient - CertificateClient infra.CertificatesClient - ShareClient infra.SharesClient - SharedResourceClient shares.ResourcesClient - LbAppProfileClient infra.LbAppProfilesClient - LbPersistenceProfilesClient infra.LbPersistenceProfilesClient - LbMonitorProfilesClient infra.LbMonitorProfilesClient + OrgRootClient nsx_policy.OrgRootClient + ProjectInfraClient projects.InfraClient + VPCClient projects.VpcsClient + VPCConnectivityProfilesClient projects.VpcConnectivityProfilesClient + IPBlockClient project_infra.IpBlocksClient + StaticRouteClient vpcs.StaticRoutesClient + NATRuleClient nat.NatRulesClient + VpcGroupClient vpcs.GroupsClient + PortClient subnets.PortsClient + PortStateClient ports.StateClient + IPPoolClient subnets.IpPoolsClient + IPAllocationClient ip_pools.IpAllocationsClient + SubnetsClient vpcs.SubnetsClient + RealizedStateClient realized_state.RealizedEntitiesClient + IPAddressAllocationClient vpcs.IpAddressAllocationsClient + VPCLBSClient vpcs.VpcLbsClient + VpcLbVirtualServersClient vpcs.VpcLbVirtualServersClient + VpcLbPoolsClient vpcs.VpcLbPoolsClient + VpcAttachmentClient vpcs.AttachmentsClient + ProjectClient orgs.ProjectsClient + TransitGatewayClient projects.TransitGatewaysClient + TransitGatewayAttachmentClient transit_gateways.AttachmentsClient + CertificateClient infra.CertificatesClient + ShareClient infra.SharesClient + SharedResourceClient shares.ResourcesClient + LbAppProfileClient infra.LbAppProfilesClient + LbPersistenceProfilesClient infra.LbPersistenceProfilesClient + LbMonitorProfilesClient infra.LbMonitorProfilesClient + SubnetConnectionBindingMapsClient subnets.SubnetConnectionBindingMapsClient NSXChecker NSXHealthChecker NSXVerChecker NSXVersionChecker @@ -195,6 +196,8 @@ func GetClient(cf *config.NSXOperatorConfig) *Client { transitGatewayClient := projects.NewTransitGatewaysClient(restConnector(cluster)) transitGatewayAttachmentClient := transit_gateways.NewAttachmentsClient(restConnector(cluster)) + subnetConnectionBindingMapClient := subnets.NewSubnetConnectionBindingMapsClient(restConnector(cluster)) + nsxChecker := &NSXHealthChecker{ cluster: cluster, } @@ -220,33 +223,34 @@ func GetClient(cf *config.NSXOperatorConfig) *Client { PrincipalIdentitiesClient: principalIdentitiesClient, WithCertificateClient: withCertificateClient, - OrgRootClient: orgRootClient, - ProjectInfraClient: projectInfraClient, - VPCClient: vpcClient, - VPCConnectivityProfilesClient: vpcConnectivityProfilesClient, - IPBlockClient: ipBlockClient, - StaticRouteClient: staticRouteClient, - NATRuleClient: natRulesClient, - VpcGroupClient: vpcGroupClient, - PortClient: portClient, - PortStateClient: portStateClient, - SubnetStatusClient: subnetStatusClient, - VPCSecurityClient: vpcSecurityClient, - VPCRuleClient: vpcRuleClient, - VPCLBSClient: vpcLBSClient, - VpcLbVirtualServersClient: vpcLbVirtualServersClient, - VpcLbPoolsClient: vpcLbPoolsClient, - VpcAttachmentClient: vpcAttachmentClient, - ProjectClient: projectClient, - NSXChecker: *nsxChecker, - NSXVerChecker: *nsxVersionChecker, - IPPoolClient: ipPoolClient, - IPAllocationClient: ipAllocationClient, - SubnetsClient: subnetsClient, - RealizedStateClient: realizedStateClient, - IPAddressAllocationClient: ipAddressAllocationClient, - TransitGatewayClient: transitGatewayClient, - TransitGatewayAttachmentClient: transitGatewayAttachmentClient, + OrgRootClient: orgRootClient, + ProjectInfraClient: projectInfraClient, + VPCClient: vpcClient, + VPCConnectivityProfilesClient: vpcConnectivityProfilesClient, + IPBlockClient: ipBlockClient, + StaticRouteClient: staticRouteClient, + NATRuleClient: natRulesClient, + VpcGroupClient: vpcGroupClient, + PortClient: portClient, + PortStateClient: portStateClient, + SubnetStatusClient: subnetStatusClient, + VPCSecurityClient: vpcSecurityClient, + VPCRuleClient: vpcRuleClient, + VPCLBSClient: vpcLBSClient, + VpcLbVirtualServersClient: vpcLbVirtualServersClient, + VpcLbPoolsClient: vpcLbPoolsClient, + VpcAttachmentClient: vpcAttachmentClient, + ProjectClient: projectClient, + NSXChecker: *nsxChecker, + NSXVerChecker: *nsxVersionChecker, + IPPoolClient: ipPoolClient, + IPAllocationClient: ipAllocationClient, + SubnetsClient: subnetsClient, + RealizedStateClient: realizedStateClient, + IPAddressAllocationClient: ipAddressAllocationClient, + TransitGatewayClient: transitGatewayClient, + TransitGatewayAttachmentClient: transitGatewayAttachmentClient, + SubnetConnectionBindingMapsClient: subnetConnectionBindingMapClient, } // NSX version check will be restarted during SecurityPolicy reconcile // So, it's unnecessary to exit even if failed in the first time diff --git a/pkg/nsx/services/common/types.go b/pkg/nsx/services/common/types.go index 09b2f76e3..30c4fd338 100644 --- a/pkg/nsx/services/common/types.go +++ b/pkg/nsx/services/common/types.go @@ -71,6 +71,8 @@ const ( TagScopeSubnetCRName string = "nsx-op/subnet_name" TagScopeSubnetSetCRName string = "nsx-op/subnetset_name" TagScopeSubnetSetCRUID string = "nsx-op/subnetset_uid" + TagScopeSubnetBindingCRName string = "nsx-op/subnetbinding_name" + TagScopeSubnetBindingCRUID string = "nsx-op/subnetbinding_uid" TagValueGroupScope string = "scope" TagValueGroupSource string = "source" TagValueGroupDestination string = "destination" @@ -95,6 +97,8 @@ const ( NSXServiceAccountFinalizerName = "nsxserviceaccount.nsx.vmware.com/finalizer" T1SecurityPolicyFinalizerName = "securitypolicy.nsx.vmware.com/finalizer" + SubnetFinalizerName = "subnet.nsx.vmware.com/finalizer" + SubnetSetFinalizerName = "subnetset.nsx.vmware.com/finalizer" IndexKeySubnetID = "IndexKeySubnetID" IndexKeyNodeName = "IndexKeyNodeName" @@ -160,6 +164,7 @@ var ( ResourceTypeLBTcpMonitorProfile = "LBTcpMonitorProfile" ResourceTypeLBVirtualServer = "LBVirtualServer" ResourceTypeLBPool = "LBPool" + ResourceTypeSubnetConnectionBIndingMap = "SubnetConnectionBindingMap" // ResourceTypeClusterControlPlane is used by NSXServiceAccountController ResourceTypeClusterControlPlane = "clustercontrolplane" diff --git a/pkg/nsx/services/subnetbinding/builder.go b/pkg/nsx/services/subnetbinding/builder.go new file mode 100644 index 000000000..a26596f36 --- /dev/null +++ b/pkg/nsx/services/subnetbinding/builder.go @@ -0,0 +1,66 @@ +package subnetbinding + +import ( + "fmt" + + "github.com/vmware/vsphere-automation-sdk-go/services/nsxt/model" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + + "github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1" + "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/common" + "github.com/vmware-tanzu/nsx-operator/pkg/util" +) + +var ( + String = common.String + Int64 = common.Int64 + Bool = common.Bool +) + +func (s *BindingService) buildSubnetBindings(binding *v1alpha1.SubnetConnectionBindingMap, parentSubnets []*model.VpcSubnet) []*model.SubnetConnectionBindingMap { + tags := util.BuildBasicTags(s.NSXConfig.Cluster, binding, "") + bindingMaps := make([]*model.SubnetConnectionBindingMap, len(parentSubnets)) + for i := range parentSubnets { + parent := parentSubnets[i] + bindingMaps[i] = &model.SubnetConnectionBindingMap{ + Id: String(buildSubnetBindingID(binding, *parent.Id)), + DisplayName: String(binding.Name), + VlanTrafficTag: Int64(binding.Spec.VLANTrafficTag), + SubnetPath: parent.Path, + Tags: tags, + } + } + return bindingMaps +} + +func buildSubnetBindingID(binding *v1alpha1.SubnetConnectionBindingMap, parentSubnetID string) string { + suffix := util.Sha1(parentSubnetID)[:common.HashLength] + return util.GenerateID(binding.Name, "", suffix, "") +} + +func buildSubnetConnectionBindingMapCR(bindingMap *model.SubnetConnectionBindingMap) (*v1alpha1.SubnetConnectionBindingMap, error) { + var crName, crNamespace, crUID string + for _, tag := range bindingMap.Tags { + switch *tag.Scope { + case common.TagScopeNamespace: + crNamespace = *tag.Tag + case common.TagScopeSubnetBindingCRName: + crName = *tag.Tag + case common.TagScopeSubnetBindingCRUID: + crUID = *tag.Tag + default: + continue + } + } + if crName == "" || crNamespace == "" || crUID == "" { + return nil, fmt.Errorf("missing tags to convert to CR SubnetConnectionBindingMap, Namespace %s, Name %s, UID %s", crNamespace, crName, crUID) + } + return &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: v1.ObjectMeta{ + Name: crName, + Namespace: crNamespace, + UID: types.UID(crUID), + }, + }, nil +} diff --git a/pkg/nsx/services/subnetbinding/builder_test.go b/pkg/nsx/services/subnetbinding/builder_test.go new file mode 100644 index 000000000..8023b27de --- /dev/null +++ b/pkg/nsx/services/subnetbinding/builder_test.go @@ -0,0 +1,135 @@ +package subnetbinding + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/vmware/vsphere-automation-sdk-go/services/nsxt/model" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + + "github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1" + "github.com/vmware-tanzu/nsx-operator/pkg/config" + "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/common" +) + +var ( + bm1ID = "binding1_411f59c1" + bm2ID = "binding1_9bc22a0c" + bindingMap1 = &model.SubnetConnectionBindingMap{ + Id: String(bm1ID), + DisplayName: String("binding1"), + SubnetPath: String(parentSubnetPath1), + VlanTrafficTag: Int64(201), + Tags: []model.Tag{ + { + Scope: String(common.TagScopeCluster), + Tag: String("fake_cluster"), + }, + { + Scope: String(common.TagScopeVersion), + Tag: String("1.0.0"), + }, + { + Scope: String(common.TagScopeNamespace), + Tag: String("default"), + }, + { + Scope: String(common.TagScopeSubnetBindingCRName), + Tag: String("binding1"), + }, + { + Scope: String(common.TagScopeSubnetBindingCRUID), + Tag: String("uuid-binding1"), + }, + }, + } + bindingMap2 = &model.SubnetConnectionBindingMap{ + Id: String(bm2ID), + DisplayName: String("binding1"), + SubnetPath: String(parentSubnetPath2), + VlanTrafficTag: Int64(201), + Tags: []model.Tag{ + { + Scope: String(common.TagScopeCluster), + Tag: String("fake_cluster"), + }, + { + Scope: String(common.TagScopeVersion), + Tag: String("1.0.0"), + }, + { + Scope: String(common.TagScopeNamespace), + Tag: String("default"), + }, + { + Scope: String(common.TagScopeSubnetBindingCRName), + Tag: String("binding1"), + }, + { + Scope: String(common.TagScopeSubnetBindingCRUID), + Tag: String("uuid-binding1"), + }, + }, + } + parentSubnet1 = &model.VpcSubnet{ + Id: String("parent1"), + Path: String(parentSubnetPath1), + } + parentSubnet2 = &model.VpcSubnet{ + Id: String("parent2"), + Path: String(parentSubnetPath2), + } + childSubnet = &model.VpcSubnet{ + Id: String("child"), + Path: String(childSubnetPath1), + } +) + +func TestBuildSubnetBindings(t *testing.T) { + service := mockService() + parentSubnets := []*model.VpcSubnet{parentSubnet1, parentSubnet2} + bindingMaps := service.buildSubnetBindings(binding1, parentSubnets) + require.Equal(t, 2, len(bindingMaps)) + expBindingMaps := []*model.SubnetConnectionBindingMap{ + bindingMap1, bindingMap2, + } + require.ElementsMatch(t, expBindingMaps, bindingMaps) +} + +func TestBuildSubnetConnectionBindingMapCR(t *testing.T) { + expCR := &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: v1.ObjectMeta{ + UID: types.UID("uuid-binding1"), + Name: "binding1", + Namespace: "default", + }, + } + cr, err := buildSubnetConnectionBindingMapCR(bindingMap1) + require.NoError(t, err) + assert.Equal(t, expCR, cr) +} + +func genSubnetConnectionBindingMap(bmID, displayName, subnetPath, parentPath string, vlanTag int64) *model.SubnetConnectionBindingMap { + return &model.SubnetConnectionBindingMap{ + Id: String(bmID), + DisplayName: String(displayName), + SubnetPath: String(subnetPath), + VlanTrafficTag: Int64(vlanTag), + ParentPath: String(parentPath), + } +} + +func mockService() *BindingService { + return &BindingService{ + Service: common.Service{ + NSXConfig: &config.NSXOperatorConfig{ + CoeConfig: &config.CoeConfig{ + Cluster: "fake_cluster", + }, + }, + }, + BindingStore: SetupStore(), + } +} diff --git a/pkg/nsx/services/subnetbinding/compare.go b/pkg/nsx/services/subnetbinding/compare.go new file mode 100644 index 000000000..0d9b20f5e --- /dev/null +++ b/pkg/nsx/services/subnetbinding/compare.go @@ -0,0 +1,36 @@ +package subnetbinding + +import ( + "github.com/vmware/vsphere-automation-sdk-go/runtime/data" + "github.com/vmware/vsphere-automation-sdk-go/services/nsxt/model" + + "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/common" +) + +type ( + SubnetConnectionBindingMap model.SubnetConnectionBindingMap +) + +func (m *SubnetConnectionBindingMap) Key() string { + return *m.Id +} + +func (m *SubnetConnectionBindingMap) Value() data.DataValue { + s := &SubnetConnectionBindingMap{ + Id: m.Id, + DisplayName: m.DisplayName, + Tags: m.Tags, + SubnetPath: m.SubnetPath, + VlanTrafficTag: m.VlanTrafficTag, + } + dataValue, _ := ComparableToSubnetConnectionBindingMap(s).GetDataValue__() + return dataValue +} + +func SubnetConnectionBindingMapToComparable(bindingMap *model.SubnetConnectionBindingMap) common.Comparable { + return (*SubnetConnectionBindingMap)(bindingMap) +} + +func ComparableToSubnetConnectionBindingMap(bindingMap common.Comparable) *model.SubnetConnectionBindingMap { + return (*model.SubnetConnectionBindingMap)(bindingMap.(*SubnetConnectionBindingMap)) +} diff --git a/pkg/nsx/services/subnetbinding/store.go b/pkg/nsx/services/subnetbinding/store.go new file mode 100644 index 000000000..246546b8d --- /dev/null +++ b/pkg/nsx/services/subnetbinding/store.go @@ -0,0 +1,162 @@ +package subnetbinding + +import ( + "errors" + + "github.com/vmware/vsphere-automation-sdk-go/services/nsxt/model" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/cache" + + "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/common" +) + +type BindingStore struct { + common.ResourceStore +} + +const ( + parentSubnetIndexKey = "parentSubnet" + childSubnetIndexKey = "childSubnet" + bindingMapCRUIDIndexKey = "bindingMapCRUID" + bindingMapCRNameIndexKey = "bindingMapCRName" +) + +func (s *BindingStore) GetByIndex(key string, value string) []*model.SubnetConnectionBindingMap { + bindings := make([]*model.SubnetConnectionBindingMap, 0) + objs := s.ResourceStore.GetByIndex(key, value) + for _, binding := range objs { + bindings = append(bindings, binding.(*model.SubnetConnectionBindingMap)) + } + return bindings +} + +func (s *BindingStore) Apply(i interface{}) error { + if i == nil { + return nil + } + binding := i.(*model.SubnetConnectionBindingMap) + if binding.MarkedForDelete != nil && *binding.MarkedForDelete { + err := s.Delete(binding) + if err != nil { + log.Error(err, "Failed to delete SubnetConnectionBindingMap", "subnetConnectionBindingMap", binding) + return err + } + log.Info("Deleted SubnetConnectionBindingMap from store", "subnetConnectionBindingMap", binding) + } else { + err := s.Add(binding) + if err != nil { + log.Error(err, "Failed to add SubnetConnectionBindingMap", "subnetConnectionBindingMap", binding) + return err + } + log.Info("Added SubnetConnectionBindingMap to store", "subnetConnectionBindingMap", binding) + } + return nil +} + +func (s *BindingStore) GetByKey(key string) *model.SubnetConnectionBindingMap { + obj := s.ResourceStore.GetByKey(key) + if obj == nil { + return nil + } + binding := obj.(*model.SubnetConnectionBindingMap) + return binding +} + +func (s *BindingStore) getBindingsByParentSubnet(subnetPath string) []*model.SubnetConnectionBindingMap { + return s.GetByIndex(parentSubnetIndexKey, subnetPath) +} + +func (s *BindingStore) getBindingsByChildSubnet(subnetPath string) []*model.SubnetConnectionBindingMap { + return s.GetByIndex(childSubnetIndexKey, subnetPath) +} + +func (s *BindingStore) getBindingsByBindingMapCRUID(bindingMapUID string) []*model.SubnetConnectionBindingMap { + return s.GetByIndex(bindingMapCRUIDIndexKey, bindingMapUID) +} + +func (s *BindingStore) getBindingsByBindingMapCRName(bindingName string, bindingNamespace string) []*model.SubnetConnectionBindingMap { + nn := types.NamespacedName{Name: bindingName, Namespace: bindingNamespace} + return s.GetByIndex(bindingMapCRNameIndexKey, nn.String()) +} + +func keyFunc(obj interface{}) (string, error) { + switch v := obj.(type) { + case *model.SubnetConnectionBindingMap: + return *v.Id, nil + default: + return "", errors.New("keyFunc doesn't support unknown type") + } +} + +func bindingMapCRUIDIndexFunc(obj interface{}) ([]string, error) { + switch o := obj.(type) { + case *model.SubnetConnectionBindingMap: + var res []string + for _, tag := range o.Tags { + if *tag.Scope == common.TagScopeSubnetBindingCRUID { + res = append(res, *tag.Tag) + } + } + return res, nil + default: + return nil, errors.New("bindingMapCRUIDIndexFunc doesn't support unknown type") + } +} + +func bindingMapCRNameIndexFunc(obj interface{}) ([]string, error) { + switch o := obj.(type) { + case *model.SubnetConnectionBindingMap: + var res []string + var crName, crNamespace string + for _, tag := range o.Tags { + if *tag.Scope == common.TagScopeSubnetBindingCRName { + crName = *tag.Tag + } else if *tag.Scope == common.TagScopeNamespace { + crNamespace = *tag.Tag + } + } + if crName != "" && crNamespace != "" { + res = append(res, types.NamespacedName{Name: crName, Namespace: crNamespace}.String()) + } + return res, nil + default: + return nil, errors.New("bindingMapCRUIDIndexFunc doesn't support unknown type") + } +} + +func childSubnetIndexFunc(obj interface{}) ([]string, error) { + switch o := obj.(type) { + case *model.SubnetConnectionBindingMap: + if o.ParentPath != nil { + return []string{*o.ParentPath}, nil + } + return []string{}, nil + default: + return nil, errors.New("bindingMapCRUIDIndexFunc doesn't support unknown type") + } +} + +func parentSubnetIndexFunc(obj interface{}) ([]string, error) { + switch o := obj.(type) { + case *model.SubnetConnectionBindingMap: + if o.SubnetPath != nil { + return []string{*o.SubnetPath}, nil + } + return []string{}, nil + default: + return nil, errors.New("bindingMapCRUIDIndexFunc doesn't support unknown type") + } +} + +func SetupStore() *BindingStore { + return &BindingStore{ResourceStore: common.ResourceStore{ + Indexer: cache.NewIndexer( + keyFunc, cache.Indexers{ + bindingMapCRUIDIndexKey: bindingMapCRUIDIndexFunc, + bindingMapCRNameIndexKey: bindingMapCRNameIndexFunc, + childSubnetIndexKey: childSubnetIndexFunc, + parentSubnetIndexKey: parentSubnetIndexFunc, + }), + BindingType: model.SubnetConnectionBindingMapBindingType(), + }} +} diff --git a/pkg/nsx/services/subnetbinding/store_test.go b/pkg/nsx/services/subnetbinding/store_test.go new file mode 100644 index 000000000..fcb152c21 --- /dev/null +++ b/pkg/nsx/services/subnetbinding/store_test.go @@ -0,0 +1,148 @@ +package subnetbinding + +import ( + "testing" + + "github.com/stretchr/testify/require" + "github.com/vmware/vsphere-automation-sdk-go/services/nsxt/model" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1" + "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/common" +) + +var ( + childSubnetPath1 = "/orgs/default/projects/default/vpcs/vpc1/subnets/subnet1" + childSubnetPath2 = "/orgs/default/projects/default/vpcs/vpc1/subnets/subnet2" + parentSubnetPath1 = "/orgs/default/projects/default/vpcs/vpc1/subnets/parent1" + parentSubnetPath2 = "/orgs/default/projects/default/vpcs/vpc1/subnets/parent2" + binding1 = &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: v1.ObjectMeta{ + Name: "binding1", + Namespace: "default", + UID: "uuid-binding1", + }, + Spec: v1alpha1.SubnetConnectionBindingMapSpec{ + SubnetName: "child", + VLANTrafficTag: 201, + TargetSubnetSetName: "parent", + }, + } + binding2 = &v1alpha1.SubnetConnectionBindingMap{ + ObjectMeta: v1.ObjectMeta{ + Name: "binding2", + Namespace: "ns1", + UID: "uuid-binding2", + }, + Spec: v1alpha1.SubnetConnectionBindingMapSpec{ + SubnetName: "child2", + VLANTrafficTag: 202, + TargetSubnetSetName: "parent2", + }, + } + incompleteBindingMap = &model.SubnetConnectionBindingMap{ + Id: String("incomplete"), + DisplayName: String("binding1"), + SubnetPath: String(parentSubnetPath1), + ParentPath: childSubnet.Path, + VlanTrafficTag: Int64(201), + Tags: []model.Tag{ + { + Scope: String(common.TagScopeCluster), + Tag: String("fake_cluster"), + }, + { + Scope: String(common.TagScopeVersion), + Tag: String("1.0.0"), + }, + { + Scope: String(common.TagScopeSubnetBindingCRName), + Tag: String("binding1"), + }, + }, + } +) + +func TestStore(t *testing.T) { + store := SetupStore() + bm1 := &model.SubnetConnectionBindingMap{ + Id: String("binding1-parent1"), + DisplayName: String("binding1"), + Path: String("/orgs/default/projects/default/vpcs/vpc1/subnets/subnet1/subnet-connection-binding-maps/binding1-parent1"), + ParentPath: String(childSubnetPath1), + SubnetPath: String(parentSubnetPath1), + VlanTrafficTag: Int64(201), + Tags: []model.Tag{ + { + Scope: String(common.TagScopeNamespace), + Tag: String("default"), + }, + { + Scope: String(common.TagScopeSubnetBindingCRName), + Tag: String("binding1"), + }, + { + Scope: String(common.TagScopeSubnetBindingCRUID), + Tag: String("uuid-binding1"), + }, + }, + } + store.Apply(bm1) + bm2 := &model.SubnetConnectionBindingMap{ + Id: String("binding1-parent2"), + DisplayName: String("binding1"), + Path: String("/orgs/default/projects/default/vpcs/vpc1/subnets/subnet1/subnet-connection-binding-maps/binding1-parent2"), + ParentPath: String(childSubnetPath1), + SubnetPath: String(parentSubnetPath2), + VlanTrafficTag: Int64(201), + Tags: []model.Tag{ + { + Scope: String(common.TagScopeNamespace), + Tag: String("default"), + }, + { + Scope: String(common.TagScopeSubnetBindingCRName), + Tag: String("binding1"), + }, + { + Scope: String(common.TagScopeSubnetBindingCRUID), + Tag: String("uuid-binding1"), + }, + }, + } + store.Apply(bm2) + + bindings := store.getBindingsByChildSubnet(childSubnetPath1) + require.Equal(t, 2, len(bindings)) + require.ElementsMatch(t, []*model.SubnetConnectionBindingMap{bm1, bm2}, bindings) + + bindings = store.getBindingsByParentSubnet(parentSubnetPath1) + require.Equal(t, 1, len(bindings)) + require.Equal(t, bm1, bindings[0]) + + bindings = store.getBindingsByParentSubnet(parentSubnetPath2) + require.Equal(t, 1, len(bindings)) + require.Equal(t, bm2, bindings[0]) + + bindings = store.getBindingsByBindingMapCRUID(string(binding1.UID)) + require.Equal(t, 2, len(bindings)) + require.ElementsMatch(t, []*model.SubnetConnectionBindingMap{bm1, bm2}, bindings) + + bindings = store.getBindingsByBindingMapCRName(binding1.Name, binding1.Namespace) + require.Equal(t, 2, len(bindings)) + require.ElementsMatch(t, []*model.SubnetConnectionBindingMap{bm1, bm2}, bindings) + + bindingMap := store.GetByKey(*bm1.Id) + require.NotNil(t, bindingMap) + require.Equal(t, bm1, bindingMap) + + delBindingMap1 := *bm1 + delBindingMap1.MarkedForDelete = Bool(true) + delBindingMap2 := *bm2 + delBindingMap2.MarkedForDelete = Bool(true) + store.Apply(&delBindingMap1) + store.Apply(&delBindingMap2) + + bindings = store.getBindingsByBindingMapCRUID(string(binding1.UID)) + require.Equal(t, 0, len(bindings)) +} diff --git a/pkg/nsx/services/subnetbinding/subnetbinding.go b/pkg/nsx/services/subnetbinding/subnetbinding.go new file mode 100644 index 000000000..bea429287 --- /dev/null +++ b/pkg/nsx/services/subnetbinding/subnetbinding.go @@ -0,0 +1,224 @@ +package subnetbinding + +import ( + "context" + "sync" + + "github.com/vmware/vsphere-automation-sdk-go/services/nsxt/model" + "k8s.io/apimachinery/pkg/util/sets" + + "github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1" + "github.com/vmware-tanzu/nsx-operator/pkg/logger" + servicecommon "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/common" +) + +var ( + log = &logger.Log + ResourceTypeSubnetConnectionBindingMap = servicecommon.ResourceTypeSubnetConnectionBIndingMap + enforceRevisionCheckParam = false +) + +type BindingService struct { + servicecommon.Service + BindingStore *BindingStore +} + +// InitializeService initialize SubnetConnectionBindingMap service. +func InitializeService(service servicecommon.Service) (*BindingService, error) { + wg := sync.WaitGroup{} + fatalErrors := make(chan error, 1) + defer close(fatalErrors) + + bindingService := &BindingService{ + Service: service, + BindingStore: SetupStore(), + } + + wg.Add(1) + go service.InitializeResourceStore(&wg, fatalErrors, ResourceTypeSubnetConnectionBindingMap, nil, bindingService.BindingStore) + wg.Wait() + + if len(fatalErrors) > 0 { + err := <-fatalErrors + return bindingService, err + } + + return bindingService, nil +} + +// CreateOrUpdateSubnetConnectionBindingMap creates or updates the SubnetConnectionBindingMaps with the given +// subnetBinding CR and attaching to the parentSubnets. +func (s *BindingService) CreateOrUpdateSubnetConnectionBindingMap( + subnetBinding *v1alpha1.SubnetConnectionBindingMap, + childSubnet *model.VpcSubnet, + parentSubnets []*model.VpcSubnet) error { + desiredBMmap := bindingMapsToMap(s.buildSubnetBindings(subnetBinding, parentSubnets)) + existingBMmap := bindingMapsToMap(s.BindingStore.getBindingsByBindingMapCRUID(string(subnetBinding.UID))) + updatedBindingMaps := make([]*model.SubnetConnectionBindingMap, 0) + for k, v := range desiredBMmap { + existBindingMap, found := existingBMmap[k] + if !found { + updatedBindingMaps = append(updatedBindingMaps, v) + continue + } + changed := servicecommon.CompareResource(SubnetConnectionBindingMapToComparable(existBindingMap), SubnetConnectionBindingMapToComparable(v)) + if changed { + updatedBindingMaps = append(updatedBindingMaps, v) + } + } + // Mark the SubnetConnectionBindingMap as for-delete if it exists in the store but does not exist in the desired resources. + for k, v := range existingBMmap { + _, found := desiredBMmap[k] + if !found { + toDelBindingMap := *v + toDelBindingMap.MarkedForDelete = Bool(true) + updatedBindingMaps = append(updatedBindingMaps, &toDelBindingMap) + } + } + + if err := s.Apply(*childSubnet.Path, updatedBindingMaps); err != nil { + return err + } + + return nil +} + +// DeleteSubnetConnectionBindingMapsByCRName deletes all the SubnetConnectionBindingMaps generated by the given subnetBinding +// CR's UID. +func (s *BindingService) DeleteSubnetConnectionBindingMapsByCRName(bindingName string, bindingNamespace string) error { + bindingMaps := s.BindingStore.getBindingsByBindingMapCRName(bindingName, bindingNamespace) + return s.deleteSubnetConnectionBindingMaps(bindingMaps) +} + +// DeleteSubnetConnectionBindingMapsByCRUID deletes all the SubnetConnectionBindingMaps generated by the given subnetBinding +// CR's UID. +func (s *BindingService) DeleteSubnetConnectionBindingMapsByCRUID(bindingUID string) error { + bindingMaps := s.BindingStore.getBindingsByBindingMapCRUID(bindingUID) + return s.deleteSubnetConnectionBindingMaps(bindingMaps) +} + +// DeleteSubnetConnectionBindingMapsByParentSubnets deletes all the SubnetConnectionBindingMaps bound to the +// given parentSubnets. +func (s *BindingService) DeleteSubnetConnectionBindingMapsByParentSubnets(parentSubnets []*model.VpcSubnet) error { + bindingMaps := make([]*model.SubnetConnectionBindingMap, 0) + for _, subnet := range parentSubnets { + if subnet.Path == nil { + log.Info("Parent VpcSubnet had no configured Path, ignoring", "subnet", *subnet.Id) + continue + } + subnetPath := *subnet.Path + bindingMaps = append(bindingMaps, s.BindingStore.getBindingsByParentSubnet(subnetPath)...) + } + return s.deleteSubnetConnectionBindingMaps(bindingMaps) +} + +// GetSubnetConnectionBindingMapsBySubnet returns all the SubnetConnectionBindingMaps referred to the given subnet. +// The function returns the SubnetConnectionBindingMaps associated with the subnet if exists, otherwise it returns +// the SubnetConnectionBindingMaps connected to the subnet. +func (s *BindingService) GetSubnetConnectionBindingMapsBySubnet(subnet *model.VpcSubnet) []*model.SubnetConnectionBindingMap { + bindingMapsByChild := s.GetSubnetConnectionBindingMapsByChildSubnet(subnet) + if len(bindingMapsByChild) > 0 { + return bindingMapsByChild + } + return s.GetSubnetConnectionBindingMapsByParentSubnet(subnet) +} + +// GetSubnetConnectionBindingMapsByChildSubnet returns the SubnetConnectionBindingMaps associated with the subnet. +func (s *BindingService) GetSubnetConnectionBindingMapsByChildSubnet(subnet *model.VpcSubnet) []*model.SubnetConnectionBindingMap { + return s.BindingStore.getBindingsByChildSubnet(*subnet.Path) +} + +// GetSubnetConnectionBindingMapsByParentSubnet returns the SubnetConnectionBindingMaps connected to the subnet. +func (s *BindingService) GetSubnetConnectionBindingMapsByParentSubnet(subnet *model.VpcSubnet) []*model.SubnetConnectionBindingMap { + return s.BindingStore.getBindingsByParentSubnet(*subnet.Path) +} + +func (s *BindingService) GetSubnetConnectionBindingMapCRsBySubnet(subnet *model.VpcSubnet) []*v1alpha1.SubnetConnectionBindingMap { + nsxBindingMaps := s.GetSubnetConnectionBindingMapsBySubnet(subnet) + if len(nsxBindingMaps) == 0 { + return nil + } + + subnetConnectionBindingMaps := sets.New[*v1alpha1.SubnetConnectionBindingMap]() + for _, bm := range nsxBindingMaps { + obj, err := buildSubnetConnectionBindingMapCR(bm) + if err != nil { + log.Error(err, "Unable to get SubnetConnectionBindingMap CR from NSX resource", "bindingMap", *bm.Id) + continue + } + subnetConnectionBindingMaps.Insert(obj) + } + return subnetConnectionBindingMaps.UnsortedList() +} + +func (s *BindingService) ListSubnetConnectionBindingMapCRUIDsInStore() sets.Set[string] { + crUIDs := sets.New[string]() + for _, obj := range s.BindingStore.List() { + bm, _ := obj.(*model.SubnetConnectionBindingMap) + res, err := buildSubnetConnectionBindingMapCR(bm) + if err != nil { + log.Error(err, "Unable to get SubnetConnectionBindingMap CR from NSX resource", "bindingMap", *bm.Id) + continue + } + crUIDs.Insert(string(res.UID)) + } + return crUIDs +} + +// Apply sync bindingMaps on NSX and save into the store if succeeded to realize. +func (s *BindingService) Apply(subnetPath string, bindingMaps []*model.SubnetConnectionBindingMap) error { + return s.hUpdateSubnetConnectionBindingMaps(subnetPath, bindingMaps) +} + +func (s *BindingService) deleteSubnetConnectionBindingMaps(bindingMaps []*model.SubnetConnectionBindingMap) error { + // Add this check is for security purpose. The caller has similar pre-check and returned if no items exist in the bindingMaps. + if len(bindingMaps) == 0 { + log.Info("No existing SubnetConnectionBindingMaps found in the store") + return nil + } + return s.hDeleteSubnetConnectionBindingMap(bindingMaps) +} + +func (s *BindingService) DeleteMultiSubnetConnectionBindingMapsByCRs(bindingCRs sets.Set[string]) error { + if bindingCRs.Len() == 0 { + return nil + } + finalBindingMaps := make([]*model.SubnetConnectionBindingMap, 0) + for _, crID := range bindingCRs.UnsortedList() { + bms := s.BindingStore.getBindingsByBindingMapCRUID(crID) + finalBindingMaps = append(finalBindingMaps, bms...) + } + return s.deleteSubnetConnectionBindingMaps(finalBindingMaps) +} + +func (s *BindingService) GetCRNameBySubnetConnectionBindingMap(bindingMap *model.SubnetConnectionBindingMap) string { + if bindingMap == nil { + return "" + } + for _, tag := range bindingMap.Tags { + if *tag.Scope == servicecommon.TagScopeSubnetBindingCRName { + return *tag.Tag + } + } + return "" +} + +func (s *BindingService) Cleanup(ctx context.Context) error { + allNSXBindings := s.BindingStore.List() + log.Info("Cleaning up SubnetConnectionBindingMaps", "Count", len(allNSXBindings)) + finalBindingMaps := make([]*model.SubnetConnectionBindingMap, len(allNSXBindings)) + for i, obj := range allNSXBindings { + binding, _ := obj.(*model.SubnetConnectionBindingMap) + finalBindingMaps[i] = binding + } + + return s.deleteSubnetConnectionBindingMaps(finalBindingMaps) +} + +func bindingMapsToMap(bindingMaps []*model.SubnetConnectionBindingMap) map[string]*model.SubnetConnectionBindingMap { + bmMap := make(map[string]*model.SubnetConnectionBindingMap) + for _, bm := range bindingMaps { + bmMap[*bm.Id] = bm + } + return bmMap +} diff --git a/pkg/nsx/services/subnetbinding/subnetbinding_test.go b/pkg/nsx/services/subnetbinding/subnetbinding_test.go new file mode 100644 index 000000000..50a8e5829 --- /dev/null +++ b/pkg/nsx/services/subnetbinding/subnetbinding_test.go @@ -0,0 +1,516 @@ +package subnetbinding + +import ( + "context" + "fmt" + "testing" + + "github.com/golang/mock/gomock" + "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" + "k8s.io/apimachinery/pkg/util/sets" + + "github.com/vmware-tanzu/nsx-operator/pkg/config" + orgroot_mocks "github.com/vmware-tanzu/nsx-operator/pkg/mock/orgrootclient" + search_mocks "github.com/vmware-tanzu/nsx-operator/pkg/mock/searchclient" + bindingmap_mocks "github.com/vmware-tanzu/nsx-operator/pkg/mock/subnetconnectionbindingmapclient" + "github.com/vmware-tanzu/nsx-operator/pkg/nsx" + "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/common" +) + +var ( + createdBM1 = &model.SubnetConnectionBindingMap{ + Id: String(bm1ID), + DisplayName: String("binding1"), + SubnetPath: String(parentSubnetPath1), + ParentPath: String(childSubnetPath1), + Path: String(fmt.Sprintf("%s/subnet-connection-binding-maps/%s", childSubnetPath1, bm1ID)), + VlanTrafficTag: Int64(201), + ResourceType: String(ResourceTypeSubnetConnectionBindingMap), + Tags: []model.Tag{ + { + Scope: String(common.TagScopeCluster), + Tag: String("fake_cluster"), + }, + { + Scope: String(common.TagScopeVersion), + Tag: String("1.0.0"), + }, + { + Scope: String(common.TagScopeNamespace), + Tag: String("default"), + }, + { + Scope: String(common.TagScopeSubnetBindingCRName), + Tag: String("binding1"), + }, + { + Scope: String(common.TagScopeSubnetBindingCRUID), + Tag: String("uuid-binding1"), + }, + }, + } + createdBM2 = &model.SubnetConnectionBindingMap{ + Id: String(bm2ID), + DisplayName: String("binding1"), + SubnetPath: String(parentSubnetPath2), + ParentPath: String(childSubnetPath1), + Path: String(fmt.Sprintf("%s/subnet-connection-binding-maps/%s", childSubnetPath1, bm2ID)), + VlanTrafficTag: Int64(201), + ResourceType: String(ResourceTypeSubnetConnectionBindingMap), + Tags: []model.Tag{ + { + Scope: String(common.TagScopeCluster), + Tag: String("fake_cluster"), + }, + { + Scope: String(common.TagScopeVersion), + Tag: String("1.0.0"), + }, + { + Scope: String(common.TagScopeNamespace), + Tag: String("default"), + }, + { + Scope: String(common.TagScopeSubnetBindingCRName), + Tag: String("binding1"), + }, + { + Scope: String(common.TagScopeSubnetBindingCRUID), + Tag: String("uuid-binding1"), + }, + }, + } +) + +func TestGetCRNameBySubnetConnectionBindingMap(t *testing.T) { + svc := mockService() + crName := svc.GetCRNameBySubnetConnectionBindingMap(bindingMap1) + require.Equal(t, "binding1", crName) + crName = svc.GetCRNameBySubnetConnectionBindingMap(nil) + require.Equal(t, "", crName) + crName = svc.GetCRNameBySubnetConnectionBindingMap(&model.SubnetConnectionBindingMap{ + Id: String(bm1ID), + DisplayName: String("binding1"), + SubnetPath: String(parentSubnetPath1), + VlanTrafficTag: Int64(201)}) + require.Equal(t, "", crName) +} + +func TestGetSubnetConnectionBindingMapCRsBySubnet(t *testing.T) { + svc := mockService() + svc.BindingStore = SetupStore() + + // Case: SubnetConnectionBindingMap does not exist. + svc.BindingStore.Apply(nil) + gotBMs := svc.GetSubnetConnectionBindingMapCRsBySubnet(parentSubnet1) + require.Equal(t, 0, len(gotBMs)) + gotBMs = svc.GetSubnetConnectionBindingMapCRsBySubnet(childSubnet) + require.Equal(t, 0, len(gotBMs)) + + // Case: tags are missing on NSX SubnetConnectionBindingMap + svc.BindingStore.Apply(incompleteBindingMap) + gotBMs1 := svc.GetSubnetConnectionBindingMapCRsBySubnet(parentSubnet1) + require.Equal(t, 0, len(gotBMs1)) + + // Case: success. + bindingMaps := svc.buildSubnetBindings(binding1, []*model.VpcSubnet{parentSubnet1}) + require.Equal(t, 1, len(bindingMaps)) + bm := bindingMaps[0] + bm.ParentPath = childSubnet.Path + svc.BindingStore.Apply(bm) + + gotBMs1 = svc.GetSubnetConnectionBindingMapCRsBySubnet(parentSubnet1) + require.Equal(t, 1, len(gotBMs1)) + gotBinding := gotBMs1[0] + assert.Equal(t, binding1.UID, gotBinding.UID) + assert.Equal(t, binding1.Namespace, gotBinding.Namespace) + assert.Equal(t, binding1.Name, gotBinding.Name) + + gotBMs2 := svc.GetSubnetConnectionBindingMapCRsBySubnet(childSubnet) + require.Equal(t, 1, len(gotBMs1)) + gotBinding2 := gotBMs2[0] + assert.Equal(t, gotBinding, gotBinding2) +} + +func TestListSubnetConnectionBindingMapCRUIDsInStore(t *testing.T) { + svc := mockService() + svc.BindingStore = SetupStore() + + // Case: SubnetConnectionBindingMap with incomplete tags in store. + svc.BindingStore.Apply(incompleteBindingMap) + crIDs := svc.ListSubnetConnectionBindingMapCRUIDsInStore() + require.Equal(t, 0, crIDs.Len()) + + // Case: success + bm := svc.buildSubnetBindings(binding1, []*model.VpcSubnet{parentSubnet1})[0] + bm.ParentPath = String(childSubnetPath1) + bm2 := svc.buildSubnetBindings(binding2, []*model.VpcSubnet{parentSubnet2})[0] + bm2.ParentPath = String(childSubnetPath2) + svc.BindingStore.Apply(bm) + svc.BindingStore.Apply(bm2) + crIDs = svc.ListSubnetConnectionBindingMapCRUIDsInStore() + require.Equal(t, 2, crIDs.Len()) + assert.ElementsMatch(t, []string{"uuid-binding1", "uuid-binding2"}, crIDs.UnsortedList()) +} + +func TestInitializeService(t *testing.T) { + ctrl := gomock.NewController(t) + fakeQueryClient := search_mocks.NewMockQueryClient(ctrl) + commonService := common.Service{ + NSXClient: &nsx.Client{ + QueryClient: fakeQueryClient, + NsxConfig: &config.NSXOperatorConfig{ + CoeConfig: &config.CoeConfig{ + Cluster: "k8scl-one:test", + }, + }, + }, + NSXConfig: &config.NSXOperatorConfig{ + CoeConfig: &config.CoeConfig{ + Cluster: "k8scl-one:test", + }, + }, + } + + for _, tc := range []struct { + name string + prepareFunc func() + expErrString string + expCountInStore int + }{ + { + name: "Failed to search SubnetConnectionBindingMaps", + prepareFunc: func() { + fakeQueryClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + Return(model.SearchResponse{}, fmt.Errorf("NSX access error")) + }, + expErrString: "NSX access error", + }, { + name: "Success to search SubnetConnectionBindingMaps", + prepareFunc: func() { + fakeQueryClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + Return(model.SearchResponse{}, nil) + }, + expErrString: "", + }, { + name: "Multiple SubnetConnectionBindingMaps are searched", + prepareFunc: func() { + cursor := "1" + resultCount := int64(1) + dv, _ := common.NewConverter().ConvertToVapi(createdBM1, model.SubnetConnectionBindingMapBindingType()) + fakeQueryClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + Return(model.SearchResponse{ + ResultCount: &resultCount, + Results: []*data.StructValue{dv.(*data.StructValue)}, + Cursor: &cursor, + }, nil) + }, + expErrString: "", + expCountInStore: 1, + }, + } { + t.Run(tc.name, func(t *testing.T) { + tc.prepareFunc() + svc, err := InitializeService(commonService) + if tc.expErrString != "" { + require.EqualError(t, err, tc.expErrString) + } else { + require.Nil(t, err) + values := svc.BindingStore.List() + assert.Equal(t, tc.expCountInStore, len(values)) + } + }) + } +} + +func TestCreateOrUpdateSubnetConnectionBindingMap(t *testing.T) { + ctrl := gomock.NewController(t) + mockOrgRootClient := orgroot_mocks.NewMockOrgRootClient(ctrl) + mockSubnetBindingClient := bindingmap_mocks.NewMockSubnetConnectionBindingMapsClient(ctrl) + + oriBM2 := *bindingMap2 + oriBM2.VlanTrafficTag = Int64(200) + + expAddBM := *bindingMap2 + expAddBM.ResourceType = String(ResourceTypeSubnetConnectionBindingMap) + expDelBM := *createdBM1 + expDelBM.MarkedForDelete = Bool(true) + + for _, tc := range []struct { + name string + prepareFunc func() + existingBindingMaps []*model.SubnetConnectionBindingMap + expErr string + expBindingMapsInStore []*model.SubnetConnectionBindingMap + }{ + { + name: "success to create new", + prepareFunc: func() { + count := int64(1) + orgConfig := map[string]map[string]map[string]map[string][]*model.SubnetConnectionBindingMap{ + "default": { + "default": { + "vpc1": { + "subnet1": []*model.SubnetConnectionBindingMap{ + &expAddBM, + }, + }, + }, + }} + expOrg, _ := wrapOrgRoot(orgConfig) + mockOrgRootClient.EXPECT().Patch(&orgRootMatcher{expOrg}, &enforceRevisionCheckParam).Return(nil) + mockSubnetBindingClient.EXPECT().List("default", "default", "vpc1", "subnet1", nil, nil, nil, nil, nil, nil). + Return(model.SubnetConnectionBindingMapListResult{ + ResultCount: &count, + Results: []model.SubnetConnectionBindingMap{ + *createdBM2, + }, + }, nil) + }, + existingBindingMaps: []*model.SubnetConnectionBindingMap{}, + expBindingMapsInStore: []*model.SubnetConnectionBindingMap{createdBM2}, + }, { + name: "success to update existing ones and delete stale ones", + prepareFunc: func() { + count := int64(1) + orgConfig := map[string]map[string]map[string]map[string][]*model.SubnetConnectionBindingMap{ + "default": { + "default": { + "vpc1": { + "subnet1": []*model.SubnetConnectionBindingMap{ + &expAddBM, + &expDelBM, + }, + }, + }, + }} + expOrg, _ := wrapOrgRoot(orgConfig) + mockOrgRootClient.EXPECT().Patch(&orgRootMatcher{expOrg}, &enforceRevisionCheckParam).Return(nil) + mockSubnetBindingClient.EXPECT().List("default", "default", "vpc1", "subnet1", nil, nil, nil, nil, nil, nil). + Return(model.SubnetConnectionBindingMapListResult{ + ResultCount: &count, + Results: []model.SubnetConnectionBindingMap{ + *createdBM2, + }, + }, nil) + }, + existingBindingMaps: []*model.SubnetConnectionBindingMap{createdBM1, &oriBM2}, + expBindingMapsInStore: []*model.SubnetConnectionBindingMap{createdBM2}, + }, { + name: "failed to patch org root", + prepareFunc: func() { + mockOrgRootClient.EXPECT().Patch(gomock.Any(), &enforceRevisionCheckParam).Return(fmt.Errorf("fake-error")) + }, + expErr: "fake-error", + existingBindingMaps: []*model.SubnetConnectionBindingMap{createdBM1, &oriBM2}, + expBindingMapsInStore: []*model.SubnetConnectionBindingMap{createdBM1, &oriBM2}, + }, { + name: "failed to list from NSX", + prepareFunc: func() { + mockOrgRootClient.EXPECT().Patch(gomock.Any(), &enforceRevisionCheckParam).Return(nil) + mockSubnetBindingClient.EXPECT().List("default", "default", "vpc1", "subnet1", nil, nil, nil, nil, nil, nil). + Return(model.SubnetConnectionBindingMapListResult{}, fmt.Errorf("fake-error")) + }, + expErr: "fake-error", + existingBindingMaps: []*model.SubnetConnectionBindingMap{createdBM1, &oriBM2}, + expBindingMapsInStore: []*model.SubnetConnectionBindingMap{createdBM1, &oriBM2}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + svc := &BindingService{ + Service: common.Service{ + NSXClient: &nsx.Client{ + OrgRootClient: mockOrgRootClient, + SubnetConnectionBindingMapsClient: mockSubnetBindingClient, + }, + NSXConfig: &config.NSXOperatorConfig{ + CoeConfig: &config.CoeConfig{ + Cluster: "fake_cluster", + }, + }, + }, + BindingStore: SetupStore(), + } + for _, bm := range tc.existingBindingMaps { + svc.BindingStore.Add(bm) + } + tc.prepareFunc() + + err := svc.CreateOrUpdateSubnetConnectionBindingMap(binding1, childSubnet, []*model.VpcSubnet{parentSubnet2}) + if tc.expErr != "" { + require.EqualError(t, err, tc.expErr) + } else { + require.Nil(t, err) + } + + bms := svc.BindingStore.List() + assert.ElementsMatch(t, tc.expBindingMapsInStore, bms) + }) + } +} + +func TestDeleteMultiSubnetConnectionBindingMapsByCRs(t *testing.T) { + ctrl := gomock.NewController(t) + mockOrgRootClient := orgroot_mocks.NewMockOrgRootClient(ctrl) + + for _, tc := range []struct { + name string + bindingCRIDs []string + prepareFunc func() + expErr string + expBindingMapsInStore []*model.SubnetConnectionBindingMap + }{ + { + name: "Empty CR IDs", + bindingCRIDs: []string{}, + expErr: "", + expBindingMapsInStore: []*model.SubnetConnectionBindingMap{createdBM1, createdBM2}, + }, + { + name: "Succeeded deletion", + bindingCRIDs: []string{"uuid-binding1"}, + prepareFunc: func() { + expDelBM1 := *createdBM1 + expDelBM1.MarkedForDelete = Bool(true) + expDelBM2 := *createdBM2 + expDelBM2.MarkedForDelete = Bool(true) + orgConfig := map[string]map[string]map[string]map[string][]*model.SubnetConnectionBindingMap{ + "default": { + "default": { + "vpc1": { + "subnet1": []*model.SubnetConnectionBindingMap{ + &expDelBM1, &expDelBM2, + }, + }, + }, + }} + + expOrg, _ := wrapOrgRoot(orgConfig) + mockOrgRootClient.EXPECT().Patch(&orgRootMatcher{expOrg}, &enforceRevisionCheckParam).Return(nil) + }, + expBindingMapsInStore: []*model.SubnetConnectionBindingMap{}, + }, { + name: "failed to patch on NSX", + bindingCRIDs: []string{"uuid-binding1"}, + prepareFunc: func() { + mockOrgRootClient.EXPECT().Patch(gomock.Any(), &enforceRevisionCheckParam).Return(fmt.Errorf("fake error")) + }, + expErr: "fake error", + expBindingMapsInStore: []*model.SubnetConnectionBindingMap{createdBM1, createdBM2}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + svc := &BindingService{ + Service: common.Service{ + NSXClient: &nsx.Client{ + OrgRootClient: mockOrgRootClient, + }, + NSXConfig: &config.NSXOperatorConfig{ + CoeConfig: &config.CoeConfig{ + Cluster: "fake_cluster", + }, + }, + }, + BindingStore: SetupStore(), + } + svc.BindingStore.Add(createdBM1) + svc.BindingStore.Add(createdBM2) + + if tc.prepareFunc != nil { + tc.prepareFunc() + } + + err := svc.DeleteMultiSubnetConnectionBindingMapsByCRs(sets.New[string](tc.bindingCRIDs...)) + if tc.expErr != "" { + require.EqualError(t, err, tc.expErr) + } else { + require.NoError(t, err) + } + bms := svc.BindingStore.List() + assert.ElementsMatch(t, tc.expBindingMapsInStore, bms) + }) + } +} + +func TestDeleteSubnetConnectionBindingMaps(t *testing.T) { + ctrl := gomock.NewController(t) + mockOrgRootClient := orgroot_mocks.NewMockOrgRootClient(ctrl) + + for _, tc := range []struct { + name string + deleteFn func(svc *BindingService) error + expErr string + expBindingMapsInStore []*model.SubnetConnectionBindingMap + }{ + { + name: "test with DeleteSubnetConnectionBindingMapsByCRName", + deleteFn: func(svc *BindingService) error { + mockOrgRootClient.EXPECT().Patch(gomock.Any(), &enforceRevisionCheckParam).Return(nil) + return svc.DeleteSubnetConnectionBindingMapsByCRName(binding1.Name, binding1.Namespace) + }, + expErr: "", + expBindingMapsInStore: []*model.SubnetConnectionBindingMap{}, + }, { + name: "test with DeleteSubnetConnectionBindingMapsByCRUID", + deleteFn: func(svc *BindingService) error { + mockOrgRootClient.EXPECT().Patch(gomock.Any(), &enforceRevisionCheckParam).Return(nil) + return svc.DeleteSubnetConnectionBindingMapsByCRUID(string(binding1.UID)) + }, + expErr: "", + expBindingMapsInStore: []*model.SubnetConnectionBindingMap{}, + }, { + name: "test with non-existing BindingMaps by CR UID", + deleteFn: func(svc *BindingService) error { + return svc.DeleteSubnetConnectionBindingMapsByCRUID("invalid-uid") + }, + expErr: "", + expBindingMapsInStore: []*model.SubnetConnectionBindingMap{createdBM1}, + }, { + name: "test with DeleteSubnetConnectionBindingMapsByParentSubnets", + deleteFn: func(svc *BindingService) error { + mockOrgRootClient.EXPECT().Patch(gomock.Any(), &enforceRevisionCheckParam).Return(nil) + return svc.DeleteSubnetConnectionBindingMapsByParentSubnets([]*model.VpcSubnet{parentSubnet1}) + }, + expErr: "", + expBindingMapsInStore: []*model.SubnetConnectionBindingMap{}, + }, { + name: "test with clean up", + deleteFn: func(svc *BindingService) error { + mockOrgRootClient.EXPECT().Patch(gomock.Any(), &enforceRevisionCheckParam).Return(nil) + ctx := context.Background() + return svc.Cleanup(ctx) + }, + expErr: "", + expBindingMapsInStore: []*model.SubnetConnectionBindingMap{}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + + svc := &BindingService{ + Service: common.Service{ + NSXClient: &nsx.Client{ + OrgRootClient: mockOrgRootClient, + }, + NSXConfig: &config.NSXOperatorConfig{ + CoeConfig: &config.CoeConfig{ + Cluster: "fake_cluster", + }, + }, + }, + BindingStore: SetupStore(), + } + svc.BindingStore.Add(createdBM1) + err := tc.deleteFn(svc) + if tc.expErr != "" { + require.EqualError(t, err, tc.expErr) + } else { + require.NoError(t, err) + } + bms := svc.BindingStore.List() + assert.ElementsMatch(t, tc.expBindingMapsInStore, bms) + }) + } +} diff --git a/pkg/nsx/services/subnetbinding/tree.go b/pkg/nsx/services/subnetbinding/tree.go new file mode 100644 index 000000000..41509b71b --- /dev/null +++ b/pkg/nsx/services/subnetbinding/tree.go @@ -0,0 +1,221 @@ +package subnetbinding + +import ( + "github.com/vmware/vsphere-automation-sdk-go/runtime/data" + "github.com/vmware/vsphere-automation-sdk-go/services/nsxt/model" + + "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/common" + nsxutil "github.com/vmware-tanzu/nsx-operator/pkg/nsx/util" +) + +var leafType = "SubnetConnectionBindingMap" + +type hNode struct { + resourceType string + resourceID string + bindingMap *model.SubnetConnectionBindingMap + childNodes []*hNode +} + +func (n *hNode) mergeChildNode(node *hNode) { + if node.resourceType == leafType { + n.childNodes = append(n.childNodes, node) + return + } + + for _, cn := range n.childNodes { + if cn.resourceType == node.resourceType && cn.resourceID == node.resourceID { + for _, chN := range node.childNodes { + cn.mergeChildNode(chN) + } + return + } + } + n.childNodes = append(n.childNodes, node) +} + +func (n *hNode) buildTree() ([]*data.StructValue, error) { + if n.resourceType == leafType { + dataValue, err := wrapSubnetBindingMap(n.bindingMap) + if err != nil { + return nil, err + } + return []*data.StructValue{dataValue}, nil + } + + children := make([]*data.StructValue, 0) + for _, cn := range n.childNodes { + cnDataValues, err := cn.buildTree() + if err != nil { + return nil, err + } + children = append(children, cnDataValues...) + } + if n.resourceType == "OrgRoot" { + return children, nil + } + + return wrapChildResourceReference(n.resourceType, n.resourceID, children) +} + +func buildHNodeFromSubnetConnectionBindingMap(subnetPath string, bindingMap *model.SubnetConnectionBindingMap) (*hNode, error) { + vpcInfo, err := common.ParseVPCResourcePath(subnetPath) + if err != nil { + return nil, err + } + return &hNode{ + resourceType: "Org", + resourceID: vpcInfo.OrgID, + childNodes: []*hNode{ + { + resourceType: "Project", + resourceID: vpcInfo.ProjectID, + childNodes: []*hNode{ + { + resourceID: vpcInfo.VPCID, + resourceType: "Vpc", + childNodes: []*hNode{ + { + resourceID: vpcInfo.ID, + resourceType: "VpcSubnet", + childNodes: []*hNode{ + { + resourceID: *bindingMap.Id, + resourceType: leafType, + bindingMap: bindingMap, + }, + }, + }, + }, + }, + }, + }, + }, + }, nil +} + +func buildOrgRootBySubnetConnectionBindingMaps(bindingMaps []*model.SubnetConnectionBindingMap, subnetPath string) (*model.OrgRoot, error) { + rootNode := &hNode{ + resourceType: "OrgRoot", + } + + for _, bm := range bindingMaps { + parentPath := subnetPath + if parentPath == "" { + parentPath = *bm.ParentPath + } + orgNode, err := buildHNodeFromSubnetConnectionBindingMap(parentPath, bm) + if err != nil { + log.Error(err, "Failed to build data value for SubnetConnectionBindingMap, ignore", "bindingMap", *bm.Path) + continue + } + rootNode.mergeChildNode(orgNode) + } + + children, err := rootNode.buildTree() + if err != nil { + log.Error(err, "Failed to build data values for multiple SubnetConnectionBindingMaps") + return nil, err + } + + return &model.OrgRoot{ + Children: children, + ResourceType: String("OrgRoot"), + }, nil +} + +func wrapChildResourceReference(targetType, resID string, children []*data.StructValue) ([]*data.StructValue, error) { + childRes := model.ChildResourceReference{ + Id: &resID, + ResourceType: "ChildResourceReference", + TargetType: &targetType, + Children: children, + } + dataValue, errors := common.NewConverter().ConvertToVapi(childRes, model.ChildResourceReferenceBindingType()) + if len(errors) > 0 { + return nil, errors[0] + } + return []*data.StructValue{dataValue.(*data.StructValue)}, nil +} + +func wrapSubnetBindingMap(bindingMap *model.SubnetConnectionBindingMap) (*data.StructValue, error) { + bindingMap.ResourceType = &common.ResourceTypeSubnetConnectionBIndingMap + childBindingMap := model.ChildSubnetConnectionBindingMap{ + Id: bindingMap.Id, + MarkedForDelete: bindingMap.MarkedForDelete, + ResourceType: "ChildSubnetConnectionBindingMap", + SubnetConnectionBindingMap: bindingMap, + } + dataValue, errors := common.NewConverter().ConvertToVapi(childBindingMap, model.ChildSubnetConnectionBindingMapBindingType()) + if len(errors) > 0 { + return nil, errors[0] + } + return dataValue.(*data.StructValue), nil +} + +func (s *BindingService) hUpdateSubnetConnectionBindingMaps(subnetPath string, bindingMaps []*model.SubnetConnectionBindingMap) error { + vpcInfo, err := common.ParseVPCResourcePath(subnetPath) + if err != nil { + return err + } + subnetID := vpcInfo.ID + orgRoot, err := buildOrgRootBySubnetConnectionBindingMaps(bindingMaps, subnetPath) + if err != nil { + return err + } + + if err = s.NSXClient.OrgRootClient.Patch(*orgRoot, &enforceRevisionCheckParam); err != nil { + log.Error(err, "Failed to patch SubnetConnectionBindingMaps on NSX", "orgID", vpcInfo.OrgID, "projectID", vpcInfo.ProjectID, "vpcID", vpcInfo.VPCID, "subnetID", subnetID, "subnetConnectionBindingMaps", bindingMaps) + err = nsxutil.TransNSXApiError(err) + return err + } + + // Get SubnetConnectionBindingMaps from NSX after patch operation as NSX renders several fields like `path`/`parent_path`. + subnetBindingListResult, err := s.NSXClient.SubnetConnectionBindingMapsClient.List(vpcInfo.OrgID, vpcInfo.ProjectID, vpcInfo.VPCID, subnetID, nil, nil, nil, nil, nil, nil) + if err != nil { + log.Error(err, "Failed to list SubnetConnectionBindingMaps from NSX under subnet", "orgID", vpcInfo.OrgID, "projectID", vpcInfo.ProjectID, "vpcID", vpcInfo.VPCID, "subnetID", subnetID, "subnetConnectionBindingMaps", bindingMaps) + err = nsxutil.TransNSXApiError(err) + return err + } + + nsxBindingMaps := make(map[string]model.SubnetConnectionBindingMap) + for _, bm := range subnetBindingListResult.Results { + nsxBindingMaps[*bm.Id] = bm + } + + for i := range bindingMaps { + bm := bindingMaps[i] + if bm.MarkedForDelete != nil && *bm.MarkedForDelete { + s.BindingStore.Apply(bm) + } else { + nsxBindingMap := nsxBindingMaps[*bm.Id] + s.BindingStore.Apply(&nsxBindingMap) + } + } + + return nil +} + +func (s *BindingService) hDeleteSubnetConnectionBindingMap(bindingMaps []*model.SubnetConnectionBindingMap) error { + markForDelete := true + for _, bm := range bindingMaps { + bm.MarkedForDelete = &markForDelete + } + + orgRoot, err := buildOrgRootBySubnetConnectionBindingMaps(bindingMaps, "") + if err != nil { + return err + } + + if err = s.NSXClient.OrgRootClient.Patch(*orgRoot, &enforceRevisionCheckParam); err != nil { + log.Error(err, "Failed to delete multiple SubnetConnectionBindingMaps on NSX with HAPI") + err = nsxutil.TransNSXApiError(err) + return err + } + + // Remove SubnetConnectionBindingMap from local store. + for _, bm := range bindingMaps { + s.BindingStore.Apply(bm) + } + return nil +} diff --git a/pkg/nsx/services/subnetbinding/tree_test.go b/pkg/nsx/services/subnetbinding/tree_test.go new file mode 100644 index 000000000..c2a46d9f7 --- /dev/null +++ b/pkg/nsx/services/subnetbinding/tree_test.go @@ -0,0 +1,359 @@ +package subnetbinding + +import ( + "fmt" + "strings" + "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" + + "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/common" +) + +type orgRootMatcher struct { + expectedRoot *model.OrgRoot +} + +func (m *orgRootMatcher) Matches(obj interface{}) bool { + dst, ok := obj.(model.OrgRoot) + if !ok { + return false + } + return m.matches(&dst) +} + +func (m *orgRootMatcher) matches(dst *model.OrgRoot) bool { + return stringEquals(m.expectedRoot.ResourceType, dst.ResourceType) && + childrenResourceEquals(m.expectedRoot.Children, dst.Children) +} + +func (m *orgRootMatcher) String() string { + return fmt.Sprintf("%v", m.expectedRoot) +} + +func childrenResourceEquals(children []*data.StructValue, children2 []*data.StructValue) bool { + if len(children) != len(children2) { + return false + } + for _, child := range children { + if !childExists(children2, child) { + return false + } + } + return true +} + +func childExists(children2 []*data.StructValue, child *data.StructValue) bool { + for _, cn := range children2 { + if dataStructEqual(cn, child) { + return true + } + } + return false +} + +func dataStructEqual(v1, v2 *data.StructValue) bool { + if v1.Name() != v2.Name() { + return false + } + if strings.Contains(v1.Name(), "child_resource_reference") { + v1Obj, err := convertToChildResourceReference(v1) + if err != nil { + return false + } + v2Obj, err := convertToChildResourceReference(v2) + if err != nil { + return false + } + return childResourceReferenceEquals(v1Obj, v2Obj) + } else if strings.Contains(v1.Name(), "child_subnet_connection_binding_map") { + v1Obj, err := convertToSubnetConnectionBindingMap(v1) + if err != nil { + return false + } + v2Obj, err := convertToSubnetConnectionBindingMap(v2) + if err != nil { + return false + } + + return childSubnetConnectionBindingMapEquals(v1Obj, v2Obj) + } + return false +} + +func convertToSubnetConnectionBindingMap(v *data.StructValue) (model.ChildSubnetConnectionBindingMap, error) { + res, err := common.NewConverter().ConvertToGolang(v, model.ChildSubnetConnectionBindingMapBindingType()) + if err != nil { + return model.ChildSubnetConnectionBindingMap{}, err[0] + } + obj := res.(model.ChildSubnetConnectionBindingMap) + return obj, nil +} + +func convertToChildResourceReference(v *data.StructValue) (model.ChildResourceReference, error) { + res, err := common.NewConverter().ConvertToGolang(v, model.ChildResourceReferenceBindingType()) + if err != nil { + return model.ChildResourceReference{}, err[0] + } + obj := res.(model.ChildResourceReference) + return obj, nil +} + +func childResourceReferenceEquals(v1, v2 model.ChildResourceReference) bool { + return stringEquals(v1.Id, v2.Id) && stringEquals(v1.TargetType, v2.TargetType) && + v1.ResourceType == v2.ResourceType && childrenResourceEquals(v1.Children, v2.Children) +} + +func childSubnetConnectionBindingMapEquals(v1, v2 model.ChildSubnetConnectionBindingMap) bool { + return stringEquals(v1.Id, v2.Id) && boolEquals(v1.MarkedForDelete, v2.MarkedForDelete) && + v1.ResourceType == v2.ResourceType && segmentConnectionBindingMapEquals(v1.SubnetConnectionBindingMap, v2.SubnetConnectionBindingMap) +} + +func segmentConnectionBindingMapEquals(bm1, bm2 *model.SubnetConnectionBindingMap) bool { + if bm1 == nil && bm2 == nil { + return true + } + if bm1 == nil && bm2 != nil { + return false + } + if bm1 != nil && bm2 == nil { + return false + } + return stringEquals(bm1.Id, bm2.Id) && stringEquals(bm1.SubnetPath, bm2.SubnetPath) && + stringEquals(bm1.ParentPath, bm2.ParentPath) && int64Equals(bm1.VlanTrafficTag, bm2.VlanTrafficTag) && + stringEquals(bm1.Path, bm2.Path) && boolEquals(bm1.MarkedForDelete, bm2.MarkedForDelete) +} + +func int64Equals(i1, i2 *int64) bool { + if i1 == nil && i2 == nil { + return true + } + if i1 == nil && i2 != nil { + return false + } + if i1 != nil && i2 == nil { + return false + } + return *i1 == *i2 +} + +func boolEquals(b1 *bool, b2 *bool) bool { + if b1 == nil && b2 == nil { + return true + } + if b1 == nil && b2 != nil { + return false + } + if b1 != nil && b2 == nil { + return false + } + return *b1 == *b2 +} + +func stringEquals(s1, s2 *string) bool { + if s1 == nil && s2 == nil { + return true + } + if s1 == nil && s2 != nil { + return false + } + if s1 != nil && s2 == nil { + return false + } + return *s1 == *s2 +} + +func TestBuildHNodeFromSubnetConnectionBindingMap(t *testing.T) { + for _, tc := range []struct { + name string + subnetBindings []*model.SubnetConnectionBindingMap + expOrgRootConfig map[string]map[string]map[string]map[string][]string + }{ + { + name: "bindings under same subnets", + subnetBindings: []*model.SubnetConnectionBindingMap{ + genSubnetConnectionBindingMap(bm1ID, "binding1", parentSubnetPath1, "/orgs/default/projects/default/vpcs/vpc1/subnets/subnet1", 201), + genSubnetConnectionBindingMap(bm2ID, "binding2", parentSubnetPath2, "/orgs/default/projects/default/vpcs/vpc1/subnets/subnet1", 202), + }, + expOrgRootConfig: map[string]map[string]map[string]map[string][]string{ + "default": { + "default": { + "vpc1": { + "subnet1": []string{bm1ID, bm2ID}, + }, + }, + }, + }, + }, + { + name: "bindings under different subnets", + subnetBindings: []*model.SubnetConnectionBindingMap{ + genSubnetConnectionBindingMap(bm1ID, "binding1", parentSubnetPath1, "/orgs/default/projects/default/vpcs/vpc1/subnets/subnet1", 201), + genSubnetConnectionBindingMap(bm2ID, "binding2", parentSubnetPath2, "/orgs/default/projects/default/vpcs/vpc1/subnets/subnet2", 202), + }, + expOrgRootConfig: map[string]map[string]map[string]map[string][]string{ + "default": { + "default": { + "vpc1": { + "subnet1": []string{bm1ID}, + "subnet2": []string{bm2ID}, + }, + }, + }, + }, + }, { + name: "bindings under different VPCs", + subnetBindings: []*model.SubnetConnectionBindingMap{ + genSubnetConnectionBindingMap(bm1ID, "binding1", parentSubnetPath1, "/orgs/default/projects/default/vpcs/vpc1/subnets/subnet1", 201), + genSubnetConnectionBindingMap(bm2ID, "binding2", parentSubnetPath2, "/orgs/default/projects/default/vpcs/vpc2/subnets/subnet1", 202), + }, + expOrgRootConfig: map[string]map[string]map[string]map[string][]string{ + "default": { + "default": { + "vpc1": { + "subnet1": []string{bm1ID}, + }, + "vpc2": { + "subnet1": []string{bm2ID}, + }, + }, + }, + }, + }, { + name: "bindings under different projects", + subnetBindings: []*model.SubnetConnectionBindingMap{ + genSubnetConnectionBindingMap(bm1ID, "binding1", parentSubnetPath1, "/orgs/default/projects/default/vpcs/vpc1/subnets/subnet1", 201), + genSubnetConnectionBindingMap(bm2ID, "binding2", parentSubnetPath2, "/orgs/default/projects/project1/vpcs/vpc2/subnets/subnet1", 202), + }, + expOrgRootConfig: map[string]map[string]map[string]map[string][]string{ + "default": { + "default": { + "vpc1": { + "subnet1": []string{bm1ID}, + }, + }, + "project1": { + "vpc2": { + "subnet1": []string{bm2ID}, + }, + }, + }, + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + bmMappings := make(map[string]*model.SubnetConnectionBindingMap) + for _, bm := range tc.subnetBindings { + bmMappings[*bm.Id] = bm + } + orgRootConfig := convertToOrgConfig(tc.expOrgRootConfig, bmMappings) + expOrgRoot, err := wrapOrgRoot(orgRootConfig) + require.Nil(t, err) + + orgRoot, err := buildOrgRootBySubnetConnectionBindingMaps(tc.subnetBindings, "") + require.NoError(t, err) + + expRoot := orgRootMatcher{expOrgRoot} + assert.True(t, expRoot.matches(orgRoot)) + }) + } +} + +func convertToOrgConfig(testCfg map[string]map[string]map[string]map[string][]string, subnetBindings map[string]*model.SubnetConnectionBindingMap) map[string]map[string]map[string]map[string][]*model.SubnetConnectionBindingMap { + out := make(map[string]map[string]map[string]map[string][]*model.SubnetConnectionBindingMap) + for k1, v1 := range testCfg { + out[k1] = make(map[string]map[string]map[string][]*model.SubnetConnectionBindingMap) + for k2, v2 := range v1 { + out[k1][k2] = make(map[string]map[string][]*model.SubnetConnectionBindingMap) + for k3, v3 := range v2 { + out[k1][k2][k3] = make(map[string][]*model.SubnetConnectionBindingMap) + for k4, v4 := range v3 { + bms := make([]*model.SubnetConnectionBindingMap, len(v4)) + for i, bmID := range v4 { + bms[i] = subnetBindings[bmID] + } + out[k1][k2][k3][k4] = bms + } + } + } + } + return out +} + +func wrapOrgRoot(orgConfigs map[string]map[string]map[string]map[string][]*model.SubnetConnectionBindingMap) (*model.OrgRoot, error) { + // This is the outermost layer of the hierarchy SubnetConnectionBindingMaps. + // It doesn't need ID field. + resourceType := "OrgRoot" + children := make([]*data.StructValue, 0) + for orgID, orgConfig := range orgConfigs { + child, err := wrapOrg(orgID, orgConfig) + if err != nil { + return nil, err + } + children = append(children, child...) + } + orgRoot := model.OrgRoot{ + Children: children, + ResourceType: &resourceType, + } + return &orgRoot, nil +} + +func wrapOrg(orgID string, orgConfig map[string]map[string]map[string][]*model.SubnetConnectionBindingMap) ([]*data.StructValue, error) { + children := make([]*data.StructValue, 0) + for projectID, projectConfig := range orgConfig { + child, err := wrapProject(projectID, projectConfig) + if err != nil { + return nil, err + } + children = append(children, child...) + } + return wrapChildResourceReference("Org", orgID, children) +} + +func wrapProject(projectID string, projectConfig map[string]map[string][]*model.SubnetConnectionBindingMap) ([]*data.StructValue, error) { + children := make([]*data.StructValue, 0) + for vpcID, vpcConfig := range projectConfig { + child, err := wrapVPC(vpcID, vpcConfig) + if err != nil { + return nil, err + } + children = append(children, child...) + } + return wrapChildResourceReference("Project", projectID, children) +} + +func wrapVPC(vpcID string, vpcConfig map[string][]*model.SubnetConnectionBindingMap) ([]*data.StructValue, error) { + children := make([]*data.StructValue, 0) + for subnetID, subnetConfig := range vpcConfig { + child, err := wrapSubnet(subnetID, subnetConfig) + if err != nil { + return nil, err + } + children = append(children, child...) + } + return wrapChildResourceReference("Vpc", vpcID, children) +} + +func wrapSubnet(subnetId string, bindingMaps []*model.SubnetConnectionBindingMap) ([]*data.StructValue, error) { + children, err := wrapSubnetBindingMaps(bindingMaps) + if err != nil { + return nil, err + } + return wrapChildResourceReference("VpcSubnet", subnetId, children) +} + +func wrapSubnetBindingMaps(bindingMaps []*model.SubnetConnectionBindingMap) ([]*data.StructValue, error) { + dataValues := make([]*data.StructValue, 0) + for _, bindingMap := range bindingMaps { + dataValue, err := wrapSubnetBindingMap(bindingMap) + if err != nil { + return nil, err + } + dataValues = append(dataValues, dataValue) + } + return dataValues, nil +} diff --git a/pkg/util/utils.go b/pkg/util/utils.go index 39f6ae819..fe0169afd 100644 --- a/pkg/util/utils.go +++ b/pkg/util/utils.go @@ -439,6 +439,10 @@ func BuildBasicTags(cluster string, obj interface{}, namespaceID types.UID) []mo tags = append(tags, model.Tag{Scope: String(common.TagScopeNamespace), Tag: String(i.ObjectMeta.Namespace)}) tags = append(tags, model.Tag{Scope: String(common.TagScopeIPAddressAllocationCRName), Tag: String(i.ObjectMeta.Name)}) tags = append(tags, model.Tag{Scope: String(common.TagScopeIPAddressAllocationCRUID), Tag: String(string(i.UID))}) + case *v1alpha1.SubnetConnectionBindingMap: + tags = append(tags, model.Tag{Scope: String(common.TagScopeNamespace), Tag: String(i.ObjectMeta.Namespace)}) + tags = append(tags, model.Tag{Scope: String(common.TagScopeSubnetBindingCRName), Tag: String(i.ObjectMeta.Name)}) + tags = append(tags, model.Tag{Scope: String(common.TagScopeSubnetBindingCRUID), Tag: String(string(i.ObjectMeta.UID))}) default: log.Info("Unknown obj type", "obj", obj) } diff --git a/test/e2e/nsx_networkinfo_test.go b/test/e2e/nsx_networkinfo_test.go index 59dbf27d0..a54c63384 100644 --- a/test/e2e/nsx_networkinfo_test.go +++ b/test/e2e/nsx_networkinfo_test.go @@ -271,7 +271,7 @@ func getVPCPathFromNetworkInfo(t *testing.T, ns, networkInfoName string) (networ t.Logf("Check VPC path of vpcnetworkconfigurations: %v, error: %+v", networkInfo, err) return false, fmt.Errorf("error when waiting for vpcnetworkconfigurations VPC path: %s", networkInfoName) } - if len(networkInfo.VPCs) > 0 && networkInfo.VPCs[0].VPCPath != "" { + if len(networkInfo.VPCs) > 0 { return true, nil } return false, nil