diff --git a/build/yaml/crd/legacy/nsx.vmware.com_nsxserviceaccounts.yaml b/build/yaml/crd/legacy/nsx.vmware.com_nsxserviceaccounts.yaml index 4081036ce..3fbd432a0 100644 --- a/build/yaml/crd/legacy/nsx.vmware.com_nsxserviceaccounts.yaml +++ b/build/yaml/crd/legacy/nsx.vmware.com_nsxserviceaccounts.yaml @@ -154,6 +154,8 @@ spec: type: object reason: type: string + supervisorClusterName: + type: string secrets: items: properties: diff --git a/pkg/apis/legacy/v1alpha1/nsxserviceaccount_types.go b/pkg/apis/legacy/v1alpha1/nsxserviceaccount_types.go index 689bde743..714ecd7a5 100644 --- a/pkg/apis/legacy/v1alpha1/nsxserviceaccount_types.go +++ b/pkg/apis/legacy/v1alpha1/nsxserviceaccount_types.go @@ -66,14 +66,15 @@ type NSXServiceAccountStatus struct { Reason string `json:"reason,omitempty"` // Represents the realization status of a NSXServiceAccount's current state. // Known .status.conditions.type is: "Realized" - Conditions []metav1.Condition `json:"conditions,omitempty"` - VPCPath string `json:"vpcPath,omitempty"` - NSXManagers []string `json:"nsxManagers,omitempty"` - ProxyEndpoints NSXProxyEndpoint `json:"proxyEndpoints,omitempty"` - ClusterID string `json:"clusterID,omitempty"` - ClusterName string `json:"clusterName,omitempty"` - Secrets []NSXSecret `json:"secrets,omitempty"` - NSXRestoreStatus *NSXRestoreStatus `json:"nsxRestoreStatus,omitempty"` + Conditions []metav1.Condition `json:"conditions,omitempty"` + VPCPath string `json:"vpcPath,omitempty"` + NSXManagers []string `json:"nsxManagers,omitempty"` + ProxyEndpoints NSXProxyEndpoint `json:"proxyEndpoints,omitempty"` + ClusterID string `json:"clusterID,omitempty"` + ClusterName string `json:"clusterName,omitempty"` + SupervisorClusterName string `json:"supervisorClusterName,omitempty"` + Secrets []NSXSecret `json:"secrets,omitempty"` + NSXRestoreStatus *NSXRestoreStatus `json:"nsxRestoreStatus,omitempty"` } // +genclient diff --git a/pkg/controllers/inventory/inventory_controller.go b/pkg/controllers/inventory/inventory_controller.go index 71ada6b24..f8a4033fa 100644 --- a/pkg/controllers/inventory/inventory_controller.go +++ b/pkg/controllers/inventory/inventory_controller.go @@ -45,6 +45,8 @@ var ( watchIngress, watchNode, watchNetworkPolicy, + watchNSXServiceAccount, + watchVirtualMachine, } ) diff --git a/pkg/controllers/inventory/inventory_controller_test.go b/pkg/controllers/inventory/inventory_controller_test.go index e3419a640..6c21faefd 100644 --- a/pkg/controllers/inventory/inventory_controller_test.go +++ b/pkg/controllers/inventory/inventory_controller_test.go @@ -53,17 +53,27 @@ func (m *MockCache) List(ctx context.Context, list client.ObjectList, opts ...cl } func (m *MockCache) GetInformer(ctx context.Context, obj client.Object, opts ...cache.InformerGetOption) (cache.Informer, error) { args := m.Called(ctx, obj) - return &MockInformer{}, args.Error(1) + informer, _ := args.Get(0).(cache.Informer) + return informer, args.Error(1) } type MockInformer struct { mock.Mock - handlers toolscache.ResourceEventHandlerFuncs + handlers toolscache.ResourceEventHandlerFuncs + registeredHandler toolscache.ResourceEventHandler + addHandlerErr error cache.Informer } func (m *MockInformer) AddEventHandler(handler toolscache.ResourceEventHandler) (toolscache.ResourceEventHandlerRegistration, error) { - if m != nil && m.handlers.AddFunc != nil { + if m == nil { + return nil, nil + } + if m.addHandlerErr != nil { + return nil, m.addHandlerErr + } + m.registeredHandler = handler + if m.handlers.AddFunc != nil { m.handlers.AddFunc(handler) } return m, nil diff --git a/pkg/controllers/inventory/nsxserviceaccount_handler.go b/pkg/controllers/inventory/nsxserviceaccount_handler.go new file mode 100644 index 000000000..078c7466a --- /dev/null +++ b/pkg/controllers/inventory/nsxserviceaccount_handler.go @@ -0,0 +1,126 @@ +package inventory + +import ( + "context" + "fmt" + "strings" + + vmv1alpha1 "github.com/vmware-tanzu/vm-operator/api/v1alpha1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/client-go/tools/cache" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + + nsxvmwarecomv1alpha1 "github.com/vmware-tanzu/nsx-operator/pkg/apis/legacy/v1alpha1" + "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/inventory" +) + +func watchNSXServiceAccount(c *InventoryController, mgr ctrl.Manager) error { + nsxSAInformer, err := mgr.GetCache().GetInformer(context.Background(), &nsxvmwarecomv1alpha1.NSXServiceAccount{}) + if err != nil { + log.Error(err, "Failed to create NSXServiceAccount informer") + return err + } + + _, err = nsxSAInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{ + AddFunc: func(obj interface{}) { + c.handleNSXServiceAccount(obj) + }, + UpdateFunc: func(oldObj, newObj interface{}) { + c.handleNSXServiceAccount(newObj) + }, + DeleteFunc: func(obj interface{}) { + c.handleNSXServiceAccountDelete(obj) + }, + }) + if err != nil { + log.Error(err, "Failed to add NSXServiceAccount event handler") + return err + } + return nil +} + +func (c *InventoryController) handleNSXServiceAccount(obj interface{}) { + var nsxSA *nsxvmwarecomv1alpha1.NSXServiceAccount + switch v := obj.(type) { + case *nsxvmwarecomv1alpha1.NSXServiceAccount: + nsxSA = v + case cache.DeletedFinalStateUnknown: + var ok bool + nsxSA, ok = v.Obj.(*nsxvmwarecomv1alpha1.NSXServiceAccount) + if !ok { + err := fmt.Errorf("obj is not valid *NSXServiceAccount") + log.Error(err, "DeletedFinalStateUnknown Obj is not *NSXServiceAccount") + return + } + } + + if nsxSA.Status.Phase != nsxvmwarecomv1alpha1.NSXServiceAccountPhaseRealized { + log.Debug("Skip NSXServiceAccount not yet realized", "namespace", nsxSA.Namespace, "name", nsxSA.Name) + return + } + + c.enqueueVMsForCluster(nsxSA) +} + +func (c *InventoryController) handleNSXServiceAccountDelete(obj interface{}) { + var nsxSA *nsxvmwarecomv1alpha1.NSXServiceAccount + switch v := obj.(type) { + case *nsxvmwarecomv1alpha1.NSXServiceAccount: + nsxSA = v + case cache.DeletedFinalStateUnknown: + var ok bool + nsxSA, ok = v.Obj.(*nsxvmwarecomv1alpha1.NSXServiceAccount) + if !ok { + err := fmt.Errorf("obj is not valid *NSXServiceAccount") + log.Error(err, "DeletedFinalStateUnknown Obj is not *NSXServiceAccount") + return + } + } + + log.Info("NSXServiceAccount deleted, enqueuing VMs for tag removal", "namespace", nsxSA.Namespace, "name", nsxSA.Name) + c.enqueueVMsForCluster(nsxSA) +} + +// enqueueVMsForCluster lists VirtualMachines belonging to the NSXServiceAccount's +// CAPI cluster and enqueues them to the inventory queue for VM tag processing. +func (c *InventoryController) enqueueVMsForCluster(nsxSA *nsxvmwarecomv1alpha1.NSXServiceAccount) { + clusterName := getClusterNameFromSA(nsxSA) + if clusterName == "" { + log.Info("NSXServiceAccount has no Cluster OwnerReference, skipping VM enqueue", + "namespace", nsxSA.Namespace, "name", nsxSA.Name) + return + } + + vmList := &vmv1alpha1.VirtualMachineList{} + if err := c.Client.List(context.Background(), vmList, &client.ListOptions{ + Namespace: nsxSA.Namespace, + LabelSelector: labels.SelectorFromSet(labels.Set{inventory.CAPIClusterNameLabel: clusterName}), + }); err != nil { + log.Error(err, "Failed to list VirtualMachines for cluster", + "namespace", nsxSA.Namespace, "cluster", clusterName) + return + } + + for i := range vmList.Items { + vm := &vmList.Items[i] + log.Debug("Enqueuing VM from NSXServiceAccount event", + "namespace", vm.Namespace, "name", vm.Name, "cluster", clusterName) + key, _ := keyFunc(vm) + c.inventoryObjectQueue.Add(inventory.InventoryKey{ + InventoryType: inventory.InventoryVirtualMachine, + ExternalId: vm.Status.InstanceUUID, + Key: key, + }) + } +} + +// getClusterNameFromSA extracts the CAPI Cluster name from the NSXServiceAccount's OwnerReferences. +func getClusterNameFromSA(nsxSA *nsxvmwarecomv1alpha1.NSXServiceAccount) string { + for _, ref := range nsxSA.OwnerReferences { + if ref.Kind == "Cluster" && strings.Contains(ref.APIVersion, "cluster.x-k8s.io") { + return ref.Name + } + } + return "" +} diff --git a/pkg/controllers/inventory/nsxserviceaccount_handler_test.go b/pkg/controllers/inventory/nsxserviceaccount_handler_test.go new file mode 100644 index 000000000..e6f2699b6 --- /dev/null +++ b/pkg/controllers/inventory/nsxserviceaccount_handler_test.go @@ -0,0 +1,546 @@ +package inventory + +import ( + "context" + "errors" + "testing" + + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + vmv1alpha1 "github.com/vmware-tanzu/vm-operator/api/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/client-go/tools/cache" + "sigs.k8s.io/controller-runtime/pkg/client" + + nsxvmwarecomv1alpha1 "github.com/vmware-tanzu/nsx-operator/pkg/apis/legacy/v1alpha1" + "github.com/vmware-tanzu/nsx-operator/pkg/config" + mockClient "github.com/vmware-tanzu/nsx-operator/pkg/mock/controller-runtime/client" + "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/inventory" +) + +func TestWatchNSXServiceAccount(t *testing.T) { + t.Run("SuccessfullyCreateInformerAndTriggerCallbacks", func(t *testing.T) { + mockCtrl := gomock.NewController(t) + k8sClient := mockClient.NewMockClient(mockCtrl) + + queue := MockObjectQueue[any]{} + controller := &InventoryController{ + Client: k8sClient, + service: &inventory.InventoryService{}, + keyBuffer: sets.New[inventory.InventoryKey](), + cf: &config.NSXOperatorConfig{NsxConfig: &config.NsxConfig{}}, + inventoryObjectQueue: &queue, + } + mockCacheObj := new(MockCache) + mockInformer := &MockInformer{handlers: cache.ResourceEventHandlerFuncs{}} + mockCacheObj.On("GetInformer", context.Background(), &nsxvmwarecomv1alpha1.NSXServiceAccount{}).Return(mockInformer, nil) + mgr := new(MockMgr) + mgr.On("GetCache").Return(mockCacheObj) + err := watchNSXServiceAccount(controller, mgr) + assert.Nil(t, err) + + nsxSA := &nsxvmwarecomv1alpha1.NSXServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "tks", + Name: "test-sa", + OwnerReferences: []metav1.OwnerReference{ + {APIVersion: "cluster.x-k8s.io/v1beta2", Kind: "Cluster", Name: "test-cluster"}, + }, + }, + Status: nsxvmwarecomv1alpha1.NSXServiceAccountStatus{ + Phase: nsxvmwarecomv1alpha1.NSXServiceAccountPhaseRealized, + }, + } + + k8sClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, list client.ObjectList, opts ...client.ListOption) error { + vmList := list.(*vmv1alpha1.VirtualMachineList) + vmList.Items = []vmv1alpha1.VirtualMachine{} + return nil + }, + ).Times(3) + + queue.On("Add", mock.Anything).Return() + + mockInformer.registeredHandler.OnAdd(nsxSA, false) + mockInformer.registeredHandler.OnUpdate(nsxSA, nsxSA) + mockInformer.registeredHandler.OnDelete(nsxSA) + }) + + t.Run("CreateInformerFailure", func(t *testing.T) { + mockCacheObj := new(MockCache) + mockCacheObj.On("GetInformer", context.Background(), &nsxvmwarecomv1alpha1.NSXServiceAccount{}).Return(nil, errors.New("connection timeout")) + controller := &InventoryController{} + mgr := new(MockMgr) + mgr.On("GetCache").Return(mockCacheObj) + err := watchNSXServiceAccount(controller, mgr) + + assert.NotNil(t, err) + assert.Contains(t, err.Error(), "connection timeout") + }) + + t.Run("AddEventHandlerFailure", func(t *testing.T) { + controller := &InventoryController{} + mockCacheObj := new(MockCache) + mockInformer := &MockInformer{addHandlerErr: errors.New("handler error")} + mockCacheObj.On("GetInformer", context.Background(), &nsxvmwarecomv1alpha1.NSXServiceAccount{}).Return(mockInformer, nil) + mgr := new(MockMgr) + mgr.On("GetCache").Return(mockCacheObj) + err := watchNSXServiceAccount(controller, mgr) + + assert.NotNil(t, err) + assert.Contains(t, err.Error(), "handler error") + }) +} + +func TestHandleNSXServiceAccount(t *testing.T) { + cfg := &config.NSXOperatorConfig{NsxConfig: &config.NsxConfig{}} + + t.Run("RealizedSAEnqueuesOnlyMatchingClusterVMs", func(t *testing.T) { + mockCtrl := gomock.NewController(t) + k8sClient := mockClient.NewMockClient(mockCtrl) + + queue := MockObjectQueue[any]{} + controller := &InventoryController{ + Client: k8sClient, + service: &inventory.InventoryService{}, + keyBuffer: sets.New[inventory.InventoryKey](), + cf: cfg, + inventoryObjectQueue: &queue, + } + + k8sClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, list client.ObjectList, opts ...client.ListOption) error { + vmList := list.(*vmv1alpha1.VirtualMachineList) + vmList.Items = []vmv1alpha1.VirtualMachine{ + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "tks", + Name: "vm-1", + Labels: map[string]string{ + inventory.CAPIClusterNameLabel: "test-cluster", + }, + }, + Status: vmv1alpha1.VirtualMachineStatus{ + InstanceUUID: "uuid-1", + }, + }, + } + return nil + }, + ) + + nsxSA := &nsxvmwarecomv1alpha1.NSXServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "tks", + Name: "test-sa", + OwnerReferences: []metav1.OwnerReference{ + {APIVersion: "cluster.x-k8s.io/v1beta2", Kind: "Cluster", Name: "test-cluster"}, + }, + }, + Status: nsxvmwarecomv1alpha1.NSXServiceAccountStatus{ + Phase: nsxvmwarecomv1alpha1.NSXServiceAccountPhaseRealized, + }, + } + + queue.On("Add", mock.MatchedBy(func(key interface{}) bool { + k, ok := key.(inventory.InventoryKey) + return ok && k.ExternalId == "uuid-1" + })).Return().Once() + + controller.handleNSXServiceAccount(nsxSA) + queue.AssertExpectations(t) + }) + + t.Run("MultipleClustersInSameNamespace", func(t *testing.T) { + mockCtrl := gomock.NewController(t) + k8sClient := mockClient.NewMockClient(mockCtrl) + + queue := MockObjectQueue[any]{} + controller := &InventoryController{ + Client: k8sClient, + service: &inventory.InventoryService{}, + keyBuffer: sets.New[inventory.InventoryKey](), + cf: cfg, + inventoryObjectQueue: &queue, + } + + // The label selector in enqueueVMsForCluster filters VMs by cluster-name label, + // so the K8s API (mocked here) only returns VMs for cluster-A. + k8sClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, list client.ObjectList, opts ...client.ListOption) error { + vmList := list.(*vmv1alpha1.VirtualMachineList) + vmList.Items = []vmv1alpha1.VirtualMachine{ + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "shared-ns", + Name: "vm-cluster-a-1", + Labels: map[string]string{inventory.CAPIClusterNameLabel: "cluster-a"}, + }, + Status: vmv1alpha1.VirtualMachineStatus{InstanceUUID: "uuid-a1"}, + }, + } + return nil + }, + ) + + saClusterA := &nsxvmwarecomv1alpha1.NSXServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "shared-ns", + Name: "sa-cluster-a", + OwnerReferences: []metav1.OwnerReference{ + {APIVersion: "cluster.x-k8s.io/v1beta2", Kind: "Cluster", Name: "cluster-a"}, + }, + }, + Status: nsxvmwarecomv1alpha1.NSXServiceAccountStatus{ + Phase: nsxvmwarecomv1alpha1.NSXServiceAccountPhaseRealized, + }, + } + + queue.On("Add", mock.MatchedBy(func(key interface{}) bool { + k, ok := key.(inventory.InventoryKey) + return ok && k.ExternalId == "uuid-a1" + })).Return().Once() + + controller.handleNSXServiceAccount(saClusterA) + queue.AssertExpectations(t) + }) + + t.Run("UnrealizedSASkipped", func(t *testing.T) { + queue := MockObjectQueue[any]{} + controller := &InventoryController{ + service: &inventory.InventoryService{}, + keyBuffer: sets.New[inventory.InventoryKey](), + cf: cfg, + inventoryObjectQueue: &queue, + } + + nsxSA := &nsxvmwarecomv1alpha1.NSXServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "tks", + Name: "test-sa", + OwnerReferences: []metav1.OwnerReference{ + {APIVersion: "cluster.x-k8s.io/v1beta2", Kind: "Cluster", Name: "test-cluster"}, + }, + }, + Status: nsxvmwarecomv1alpha1.NSXServiceAccountStatus{ + Phase: nsxvmwarecomv1alpha1.NSXServiceAccountPhaseInProgress, + }, + } + + controller.handleNSXServiceAccount(nsxSA) + queue.AssertNotCalled(t, "Add", mock.Anything) + }) + + t.Run("SAWithoutClusterOwnerRefSkipped", func(t *testing.T) { + queue := MockObjectQueue[any]{} + controller := &InventoryController{ + service: &inventory.InventoryService{}, + keyBuffer: sets.New[inventory.InventoryKey](), + cf: cfg, + inventoryObjectQueue: &queue, + } + + nsxSA := &nsxvmwarecomv1alpha1.NSXServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "tks", + Name: "test-sa", + }, + Status: nsxvmwarecomv1alpha1.NSXServiceAccountStatus{ + Phase: nsxvmwarecomv1alpha1.NSXServiceAccountPhaseRealized, + }, + } + + controller.handleNSXServiceAccount(nsxSA) + queue.AssertNotCalled(t, "Add", mock.Anything) + }) + + t.Run("DeletedFinalStateUnknownRealized", func(t *testing.T) { + mockCtrl := gomock.NewController(t) + k8sClient := mockClient.NewMockClient(mockCtrl) + + queue := MockObjectQueue[any]{} + controller := &InventoryController{ + Client: k8sClient, + service: &inventory.InventoryService{}, + keyBuffer: sets.New[inventory.InventoryKey](), + cf: cfg, + inventoryObjectQueue: &queue, + } + + k8sClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, list client.ObjectList, opts ...client.ListOption) error { + vmList := list.(*vmv1alpha1.VirtualMachineList) + vmList.Items = []vmv1alpha1.VirtualMachine{} + return nil + }, + ) + + nsxSA := &nsxvmwarecomv1alpha1.NSXServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "tks", + Name: "test-sa", + OwnerReferences: []metav1.OwnerReference{ + {APIVersion: "cluster.x-k8s.io/v1beta2", Kind: "Cluster", Name: "test-cluster"}, + }, + }, + Status: nsxvmwarecomv1alpha1.NSXServiceAccountStatus{ + Phase: nsxvmwarecomv1alpha1.NSXServiceAccountPhaseRealized, + }, + } + deletedObj := cache.DeletedFinalStateUnknown{Obj: nsxSA} + controller.handleNSXServiceAccount(deletedObj) + queue.AssertNotCalled(t, "Add", mock.Anything) + }) + + t.Run("DeletedFinalStateUnknownInvalidObj", func(t *testing.T) { + queue := MockObjectQueue[any]{} + controller := &InventoryController{ + service: &inventory.InventoryService{}, + keyBuffer: sets.New[inventory.InventoryKey](), + cf: cfg, + inventoryObjectQueue: &queue, + } + + deletedObj := cache.DeletedFinalStateUnknown{Obj: "invalid"} + controller.handleNSXServiceAccount(deletedObj) + queue.AssertNotCalled(t, "Add", mock.Anything) + }) +} + +func TestHandleNSXServiceAccountDelete(t *testing.T) { + cfg := &config.NSXOperatorConfig{NsxConfig: &config.NsxConfig{}} + + t.Run("DeletedSAEnqueuesClusterVMs", func(t *testing.T) { + mockCtrl := gomock.NewController(t) + k8sClient := mockClient.NewMockClient(mockCtrl) + + queue := MockObjectQueue[any]{} + controller := &InventoryController{ + Client: k8sClient, + service: &inventory.InventoryService{}, + keyBuffer: sets.New[inventory.InventoryKey](), + cf: cfg, + inventoryObjectQueue: &queue, + } + + k8sClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, list client.ObjectList, opts ...client.ListOption) error { + vmList := list.(*vmv1alpha1.VirtualMachineList) + vmList.Items = []vmv1alpha1.VirtualMachine{ + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "tks", + Name: "vm-1", + Labels: map[string]string{ + inventory.CAPIClusterNameLabel: "deleted-cluster", + }, + }, + Status: vmv1alpha1.VirtualMachineStatus{ + InstanceUUID: "uuid-1", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "tks", + Name: "vm-2", + Labels: map[string]string{ + inventory.CAPIClusterNameLabel: "deleted-cluster", + }, + }, + Status: vmv1alpha1.VirtualMachineStatus{ + InstanceUUID: "uuid-2", + }, + }, + } + return nil + }, + ) + + nsxSA := &nsxvmwarecomv1alpha1.NSXServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "tks", + Name: "deleted-sa", + OwnerReferences: []metav1.OwnerReference{ + {APIVersion: "cluster.x-k8s.io/v1beta2", Kind: "Cluster", Name: "deleted-cluster"}, + }, + }, + } + + queue.On("Add", mock.Anything).Return().Times(2) + controller.handleNSXServiceAccountDelete(nsxSA) + queue.AssertExpectations(t) + }) + + t.Run("DeletedSAWithoutClusterOwnerRefSkipped", func(t *testing.T) { + queue := MockObjectQueue[any]{} + controller := &InventoryController{ + service: &inventory.InventoryService{}, + keyBuffer: sets.New[inventory.InventoryKey](), + cf: cfg, + inventoryObjectQueue: &queue, + } + + nsxSA := &nsxvmwarecomv1alpha1.NSXServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "tks", + Name: "deleted-sa", + }, + } + + controller.handleNSXServiceAccountDelete(nsxSA) + queue.AssertNotCalled(t, "Add", mock.Anything) + }) + + t.Run("DeletedFinalStateUnknownSA", func(t *testing.T) { + mockCtrl := gomock.NewController(t) + k8sClient := mockClient.NewMockClient(mockCtrl) + + queue := MockObjectQueue[any]{} + controller := &InventoryController{ + Client: k8sClient, + service: &inventory.InventoryService{}, + keyBuffer: sets.New[inventory.InventoryKey](), + cf: cfg, + inventoryObjectQueue: &queue, + } + + k8sClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, list client.ObjectList, opts ...client.ListOption) error { + vmList := list.(*vmv1alpha1.VirtualMachineList) + vmList.Items = []vmv1alpha1.VirtualMachine{ + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "tks", + Name: "vm-1", + Labels: map[string]string{ + inventory.CAPIClusterNameLabel: "deleted-cluster", + }, + }, + Status: vmv1alpha1.VirtualMachineStatus{ + InstanceUUID: "uuid-1", + }, + }, + } + return nil + }, + ) + + nsxSA := &nsxvmwarecomv1alpha1.NSXServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "tks", + Name: "deleted-sa", + OwnerReferences: []metav1.OwnerReference{ + {APIVersion: "cluster.x-k8s.io/v1beta2", Kind: "Cluster", Name: "deleted-cluster"}, + }, + }, + } + deletedObj := cache.DeletedFinalStateUnknown{Obj: nsxSA} + + queue.On("Add", mock.Anything).Return().Once() + controller.handleNSXServiceAccountDelete(deletedObj) + queue.AssertExpectations(t) + }) + + t.Run("DeletedFinalStateUnknownInvalidObj", func(t *testing.T) { + queue := MockObjectQueue[any]{} + controller := &InventoryController{ + service: &inventory.InventoryService{}, + keyBuffer: sets.New[inventory.InventoryKey](), + cf: cfg, + inventoryObjectQueue: &queue, + } + + deletedObj := cache.DeletedFinalStateUnknown{Obj: "invalid"} + controller.handleNSXServiceAccountDelete(deletedObj) + queue.AssertNotCalled(t, "Add", mock.Anything) + }) + + t.Run("DeletedSAListError", func(t *testing.T) { + mockCtrl := gomock.NewController(t) + k8sClient := mockClient.NewMockClient(mockCtrl) + + queue := MockObjectQueue[any]{} + controller := &InventoryController{ + Client: k8sClient, + service: &inventory.InventoryService{}, + keyBuffer: sets.New[inventory.InventoryKey](), + cf: cfg, + inventoryObjectQueue: &queue, + } + + k8sClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()).Return(errors.New("list error")) + + nsxSA := &nsxvmwarecomv1alpha1.NSXServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "tks", + Name: "deleted-sa", + OwnerReferences: []metav1.OwnerReference{ + {APIVersion: "cluster.x-k8s.io/v1beta2", Kind: "Cluster", Name: "deleted-cluster"}, + }, + }, + } + + controller.handleNSXServiceAccountDelete(nsxSA) + queue.AssertNotCalled(t, "Add", mock.Anything) + }) +} + +func TestGetClusterNameFromSA(t *testing.T) { + tests := []struct { + name string + sa *nsxvmwarecomv1alpha1.NSXServiceAccount + expected string + }{ + { + name: "Has CAPI Cluster OwnerRef", + sa: &nsxvmwarecomv1alpha1.NSXServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + OwnerReferences: []metav1.OwnerReference{ + {APIVersion: "cluster.x-k8s.io/v1beta2", Kind: "Cluster", Name: "my-cluster"}, + }, + }, + }, + expected: "my-cluster", + }, + { + name: "No OwnerReferences", + sa: &nsxvmwarecomv1alpha1.NSXServiceAccount{ + ObjectMeta: metav1.ObjectMeta{}, + }, + expected: "", + }, + { + name: "Non-cluster OwnerRef", + sa: &nsxvmwarecomv1alpha1.NSXServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + OwnerReferences: []metav1.OwnerReference{ + {APIVersion: "apps/v1", Kind: "Deployment", Name: "my-deploy"}, + }, + }, + }, + expected: "", + }, + { + name: "Multiple OwnerRefs with one CAPI Cluster", + sa: &nsxvmwarecomv1alpha1.NSXServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + OwnerReferences: []metav1.OwnerReference{ + {APIVersion: "apps/v1", Kind: "Deployment", Name: "my-deploy"}, + {APIVersion: "cluster.x-k8s.io/v1beta2", Kind: "Cluster", Name: "correct-cluster"}, + }, + }, + }, + expected: "correct-cluster", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := getClusterNameFromSA(tt.sa) + assert.Equal(t, tt.expected, result) + }) + } +} diff --git a/pkg/controllers/inventory/virtualmachine_handler.go b/pkg/controllers/inventory/virtualmachine_handler.go new file mode 100644 index 000000000..051d44e98 --- /dev/null +++ b/pkg/controllers/inventory/virtualmachine_handler.go @@ -0,0 +1,80 @@ +package inventory + +import ( + "context" + "fmt" + + vmv1alpha1 "github.com/vmware-tanzu/vm-operator/api/v1alpha1" + "k8s.io/client-go/tools/cache" + ctrl "sigs.k8s.io/controller-runtime" + + "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/inventory" +) + +func watchVirtualMachine(c *InventoryController, mgr ctrl.Manager) error { + vmInformer, err := mgr.GetCache().GetInformer(context.Background(), &vmv1alpha1.VirtualMachine{}) + if err != nil { + log.Error(err, "Failed to create VirtualMachine informer") + return err + } + + _, err = vmInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{ + AddFunc: func(obj interface{}) { + c.handleVirtualMachine(obj) + }, + UpdateFunc: func(oldObj, newObj interface{}) { + c.handleVirtualMachine(newObj) + }, + DeleteFunc: func(obj interface{}) {}, + }) + if err != nil { + log.Error(err, "Failed to add VirtualMachine event handler") + return err + } + return nil +} + +func (c *InventoryController) handleVirtualMachine(obj interface{}) { + var vm *vmv1alpha1.VirtualMachine + switch v := obj.(type) { + case *vmv1alpha1.VirtualMachine: + vm = v + case cache.DeletedFinalStateUnknown: + var ok bool + vm, ok = v.Obj.(*vmv1alpha1.VirtualMachine) + if !ok { + err := fmt.Errorf("obj is not valid *vmv1alpha1.VirtualMachine") + log.Error(err, "DeletedFinalStateUnknown Obj is not *vmv1alpha1.VirtualMachine") + return + } + } + + if !isVMRunning(vm) { + log.Debug("Skip VM not in running state", "namespace", vm.Namespace, "name", vm.Name) + return + } + + if !belongsToVKSCluster(vm) { + log.Debug("Skip VM not belonging to a VKS cluster", "namespace", vm.Namespace, "name", vm.Name) + return + } + + log.Debug("Inventory processing VirtualMachine", "namespace", vm.Namespace, "name", vm.Name) + key, _ := keyFunc(vm) + c.inventoryObjectQueue.Add(inventory.InventoryKey{ + InventoryType: inventory.InventoryVirtualMachine, + ExternalId: vm.Status.InstanceUUID, + Key: key, + }) +} + +func isVMRunning(vm *vmv1alpha1.VirtualMachine) bool { + return vm.Status.PowerState == vmv1alpha1.VirtualMachinePoweredOn +} + +// belongsToVKSCluster checks whether the VM has the CAPI cluster-name label, +// which is guaranteed by the Cluster API contract for all VMs managed by CAPI. +func belongsToVKSCluster(vm *vmv1alpha1.VirtualMachine) bool { + _, exists := vm.Labels[inventory.CAPIClusterNameLabel] + return exists +} diff --git a/pkg/controllers/inventory/virtualmachine_handler_test.go b/pkg/controllers/inventory/virtualmachine_handler_test.go new file mode 100644 index 000000000..f3213b8f1 --- /dev/null +++ b/pkg/controllers/inventory/virtualmachine_handler_test.go @@ -0,0 +1,242 @@ +package inventory + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + vmv1alpha1 "github.com/vmware-tanzu/vm-operator/api/v1alpha1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/client-go/tools/cache" + + "github.com/vmware-tanzu/nsx-operator/pkg/config" + "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/inventory" +) + +func TestWatchVirtualMachine(t *testing.T) { + t.Run("SuccessfullyCreateInformerAndTriggerCallbacks", func(t *testing.T) { + queue := MockObjectQueue[any]{} + controller := &InventoryController{ + service: &inventory.InventoryService{}, + keyBuffer: sets.New[inventory.InventoryKey](), + cf: &config.NSXOperatorConfig{NsxConfig: &config.NsxConfig{}}, + inventoryObjectQueue: &queue, + } + mockCache := new(MockCache) + mockInformer := &MockInformer{handlers: cache.ResourceEventHandlerFuncs{}} + mockCache.On("GetInformer", context.Background(), &vmv1alpha1.VirtualMachine{}).Return(mockInformer, nil) + mgr := new(MockMgr) + mgr.On("GetCache").Return(mockCache) + err := watchVirtualMachine(controller, mgr) + assert.Nil(t, err) + + vm := &vmv1alpha1.VirtualMachine{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "tks", Name: "vm-cb", + Labels: map[string]string{ + inventory.CAPIClusterNameLabel: "test-cluster", + }, + }, + Status: vmv1alpha1.VirtualMachineStatus{ + PowerState: vmv1alpha1.VirtualMachinePoweredOn, InstanceUUID: "uuid-cb", + }, + } + queue.On("Add", mock.Anything).Return() + + mockInformer.registeredHandler.OnAdd(vm, false) + mockInformer.registeredHandler.OnUpdate(vm, vm) + mockInformer.registeredHandler.OnDelete(vm) + }) + + t.Run("CreateInformerFailure", func(t *testing.T) { + mockCache := new(MockCache) + mockCache.On("GetInformer", context.Background(), &vmv1alpha1.VirtualMachine{}).Return(nil, errors.New("connection timeout")) + controller := &InventoryController{} + mgr := new(MockMgr) + mgr.On("GetCache").Return(mockCache) + err := watchVirtualMachine(controller, mgr) + + assert.NotNil(t, err) + assert.Contains(t, err.Error(), "connection timeout") + }) + + t.Run("AddEventHandlerFailure", func(t *testing.T) { + controller := &InventoryController{} + mockCache := new(MockCache) + mockInformer := &MockInformer{addHandlerErr: errors.New("handler error")} + mockCache.On("GetInformer", context.Background(), &vmv1alpha1.VirtualMachine{}).Return(mockInformer, nil) + mgr := new(MockMgr) + mgr.On("GetCache").Return(mockCache) + err := watchVirtualMachine(controller, mgr) + + assert.NotNil(t, err) + assert.Contains(t, err.Error(), "handler error") + }) +} + +func TestHandleVirtualMachine(t *testing.T) { + cfg := &config.NSXOperatorConfig{NsxConfig: &config.NsxConfig{}} + queue := MockObjectQueue[any]{} + inventoryService := &inventory.InventoryService{} + controller := &InventoryController{ + service: inventoryService, + keyBuffer: sets.New[inventory.InventoryKey](), + cf: cfg, + inventoryObjectQueue: &queue, + } + + t.Run("RunningVKSVM", func(t *testing.T) { + vm := &vmv1alpha1.VirtualMachine{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "tks", + Name: "test-vm", + Labels: map[string]string{ + inventory.CAPIClusterNameLabel: "test-cluster", + }, + }, + Status: vmv1alpha1.VirtualMachineStatus{ + PowerState: vmv1alpha1.VirtualMachinePoweredOn, + InstanceUUID: "uuid-1234", + }, + } + queue = MockObjectQueue[any]{} + controller.inventoryObjectQueue = &queue + queue.On("Add", mock.Anything).Return().Once() + controller.handleVirtualMachine(vm) + queue.AssertExpectations(t) + }) + + t.Run("NotRunningVM", func(t *testing.T) { + vm := &vmv1alpha1.VirtualMachine{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "tks", + Name: "test-vm", + Labels: map[string]string{ + inventory.CAPIClusterNameLabel: "test-cluster", + }, + }, + Status: vmv1alpha1.VirtualMachineStatus{ + PowerState: vmv1alpha1.VirtualMachinePoweredOff, + }, + } + queue = MockObjectQueue[any]{} + controller.inventoryObjectQueue = &queue + controller.handleVirtualMachine(vm) + queue.AssertNotCalled(t, "Add", mock.Anything) + }) + + t.Run("NonVKSVM", func(t *testing.T) { + vm := &vmv1alpha1.VirtualMachine{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "tks", + Name: "test-vm", + }, + Status: vmv1alpha1.VirtualMachineStatus{ + PowerState: vmv1alpha1.VirtualMachinePoweredOn, + }, + } + queue = MockObjectQueue[any]{} + controller.inventoryObjectQueue = &queue + controller.handleVirtualMachine(vm) + queue.AssertNotCalled(t, "Add", mock.Anything) + }) + + t.Run("DeletedFinalStateUnknown", func(t *testing.T) { + vm := &vmv1alpha1.VirtualMachine{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "tks", + Name: "deleted-vm", + Labels: map[string]string{ + inventory.CAPIClusterNameLabel: "test-cluster", + }, + }, + Status: vmv1alpha1.VirtualMachineStatus{ + PowerState: vmv1alpha1.VirtualMachinePoweredOn, + InstanceUUID: "uuid-5678", + }, + } + deletedObj := cache.DeletedFinalStateUnknown{Obj: vm} + queue = MockObjectQueue[any]{} + controller.inventoryObjectQueue = &queue + queue.On("Add", mock.Anything).Return().Once() + controller.handleVirtualMachine(deletedObj) + queue.AssertExpectations(t) + }) + + t.Run("DeletedFinalStateUnknownInvalidObj", func(t *testing.T) { + deletedObj := cache.DeletedFinalStateUnknown{Obj: "invalid"} + queue = MockObjectQueue[any]{} + controller.inventoryObjectQueue = &queue + controller.handleVirtualMachine(deletedObj) + queue.AssertNotCalled(t, "Add", mock.Anything) + }) +} + +func TestBelongsToVKSCluster(t *testing.T) { + tests := []struct { + name string + vm *vmv1alpha1.VirtualMachine + expected bool + }{ + { + name: "Has CAPI cluster-name label", + vm: &vmv1alpha1.VirtualMachine{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + inventory.CAPIClusterNameLabel: "my-cluster", + }, + }, + }, + expected: true, + }, + { + name: "No labels", + vm: &vmv1alpha1.VirtualMachine{ + ObjectMeta: metav1.ObjectMeta{}, + }, + expected: false, + }, + { + name: "Has other labels but not CAPI", + vm: &vmv1alpha1.VirtualMachine{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "app": "nginx", + }, + }, + }, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := belongsToVKSCluster(tt.vm) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestIsVMRunning(t *testing.T) { + tests := []struct { + name string + state vmv1alpha1.VirtualMachinePowerState + expected bool + }{ + {"PoweredOn", vmv1alpha1.VirtualMachinePoweredOn, true}, + {"PoweredOff", vmv1alpha1.VirtualMachinePoweredOff, false}, + {"Suspended", vmv1alpha1.VirtualMachineSuspended, false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + vm := &vmv1alpha1.VirtualMachine{ + Status: vmv1alpha1.VirtualMachineStatus{PowerState: tt.state}, + } + assert.Equal(t, tt.expected, isVMRunning(vm)) + }) + } +} diff --git a/pkg/controllers/nsxserviceaccount/nsxserviceaccount_controller.go b/pkg/controllers/nsxserviceaccount/nsxserviceaccount_controller.go index 6a3e02f78..21d82617d 100644 --- a/pkg/controllers/nsxserviceaccount/nsxserviceaccount_controller.go +++ b/pkg/controllers/nsxserviceaccount/nsxserviceaccount_controller.go @@ -178,6 +178,17 @@ func (r *NSXServiceAccountReconciler) Reconcile(ctx context.Context, req ctrl.Re return ResultRequeue, err } } + if obj.Status.SupervisorClusterName == "" { + obj.Status.SupervisorClusterName = r.Service.NSXConfig.CoeConfig.Cluster + if err := r.Client.Status().Update(ctx, obj); err != nil { + log.Error(err, "Failed to backfill SupervisorClusterName", + "nsxserviceaccount", req.NamespacedName) + return ResultRequeue, err + } + log.Info("Backfilled SupervisorClusterName on realized NSXServiceAccount", + "nsxserviceaccount", req.NamespacedName, + "supervisorClusterName", obj.Status.SupervisorClusterName) + } // update ProxyEndpoints if it has changed. if err := r.Service.UpdateProxyEndpointsIfNeeded(ctx, obj); err != nil { r.StatusUpdater.UpdateFail(ctx, obj, err, "", updateNSXServiceAccountStatuswithError) diff --git a/pkg/controllers/nsxserviceaccount/nsxserviceaccount_controller_test.go b/pkg/controllers/nsxserviceaccount/nsxserviceaccount_controller_test.go index acd6a1128..8e27e45f4 100644 --- a/pkg/controllers/nsxserviceaccount/nsxserviceaccount_controller_test.go +++ b/pkg/controllers/nsxserviceaccount/nsxserviceaccount_controller_test.go @@ -25,6 +25,7 @@ import ( controllerruntime "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/client/interceptor" "sigs.k8s.io/controller-runtime/pkg/manager" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -253,11 +254,12 @@ func TestNSXServiceAccountReconciler_Reconcile(t *testing.T) { Namespace: requestArgs.req.Namespace, Name: requestArgs.req.Name, Finalizers: []string{servicecommon.NSXServiceAccountFinalizerName}, - ResourceVersion: "3", + ResourceVersion: "4", }, Spec: nsxvmwarecomv1alpha1.NSXServiceAccountSpec{}, Status: nsxvmwarecomv1alpha1.NSXServiceAccountStatus{ - Phase: nsxvmwarecomv1alpha1.NSXServiceAccountPhaseRealized, + Phase: nsxvmwarecomv1alpha1.NSXServiceAccountPhaseRealized, + SupervisorClusterName: "k8scl-one:test", Conditions: []metav1.Condition{ { Type: "Dummy", @@ -486,6 +488,9 @@ func TestNSXServiceAccountReconciler_Reconcile(t *testing.T) { Service: servicecommon.Service{ NSXClient: &nsx.Client{}, NSXConfig: &config.NSXOperatorConfig{ + CoeConfig: &config.CoeConfig{ + Cluster: "k8scl-one:test", + }, NsxConfig: &config.NsxConfig{ EnforcementPoint: "vmc-enforcementpoint", }, @@ -1103,3 +1108,65 @@ func TestNSXServiceAccountReconciler_serviceMapFunc(t *testing.T) { }, }, requests) } + +func TestNSXServiceAccountReconciler_BackfillSupervisorClusterNameFail(t *testing.T) { + scheme := clientgoscheme.Scheme + nsxvmwarecomv1alpha1.AddToScheme(scheme) + + statusUpdateCallCount := 0 + c := fake.NewClientBuilder().WithScheme(scheme). + WithStatusSubresource(&nsxvmwarecomv1alpha1.NSXServiceAccount{}). + WithInterceptorFuncs(interceptor.Funcs{ + SubResourceUpdate: func(ctx context.Context, cl client.Client, subResourceName string, obj client.Object, opts ...client.SubResourceUpdateOption) error { + statusUpdateCallCount++ + return fmt.Errorf("mock status update error") + }, + }). + Build() + + r := &NSXServiceAccountReconciler{ + Client: c, + Scheme: scheme, + Recorder: fakeRecorder{}, + } + r.Service = &nsxserviceaccount.NSXServiceAccountService{ + Service: servicecommon.Service{ + NSXClient: &nsx.Client{}, + NSXConfig: &config.NSXOperatorConfig{ + CoeConfig: &config.CoeConfig{ + Cluster: "k8scl-one:test", + }, + NsxConfig: &config.NsxConfig{ + EnforcementPoint: "vmc-enforcementpoint", + }, + }, + }, + } + r.StatusUpdater = common.NewStatusUpdater(r.Client, r.Service.NSXConfig, r.Recorder, MetricResType, "ServiceAccount", "NSXServiceAccount") + + ctx := context.TODO() + assert.NoError(t, c.Create(ctx, &nsxvmwarecomv1alpha1.NSXServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns", + Name: "sa", + }, + Status: nsxvmwarecomv1alpha1.NSXServiceAccountStatus{ + Phase: nsxvmwarecomv1alpha1.NSXServiceAccountPhaseRealized, + }, + })) + + cluster := &nsx.Cluster{} + patches := gomonkey.ApplyMethod(reflect.TypeOf(cluster), "GetVersion", func(_ *nsx.Cluster) (*nsx.NsxVersion, error) { + return &nsx.NsxVersion{NodeVersion: "4.0.1"}, nil + }) + defer patches.Reset() + + result, err := r.Reconcile(ctx, controllerruntime.Request{ + NamespacedName: types.NamespacedName{Namespace: "ns", Name: "sa"}, + }) + + assert.Error(t, err) + assert.Contains(t, err.Error(), "mock status update error") + assert.Equal(t, ResultRequeue, result) + assert.Equal(t, 1, statusUpdateCallCount) +} diff --git a/pkg/nsx/services/inventory/inventory.go b/pkg/nsx/services/inventory/inventory.go index d43cdcca6..dc5a1e37d 100644 --- a/pkg/nsx/services/inventory/inventory.go +++ b/pkg/nsx/services/inventory/inventory.go @@ -36,6 +36,8 @@ type InventoryService struct { pendingDelete map[string]interface{} stalePods map[string]interface{} + + taggedVMs map[string]string // key: externalID (instanceUUID), value: clusterName tag } func InitializeService(service commonservice.Service, cleanup bool) (*InventoryService, error) { @@ -50,6 +52,7 @@ func NewInventoryService(service commonservice.Service) *InventoryService { pendingAdd: make(map[string]interface{}), pendingDelete: make(map[string]interface{}), stalePods: make(map[string]interface{}), + taggedVMs: make(map[string]string), } // TODO, Inventory store should have its own store @@ -157,6 +160,10 @@ func (s *InventoryService) SyncInventoryStoreByType(clusterUUID string) error { if err != nil { return err } + err = s.initTaggedVMs() + if err != nil { + return err + } return nil } @@ -206,6 +213,11 @@ func (s *InventoryService) SyncInventoryObject(bufferedKeys sets.Set[InventoryKe if retryKey != nil { retryKeys.Insert(*retryKey) } + case InventoryVirtualMachine: + retryKey := s.SyncVirtualMachineTag(name, namespace, key) + if retryKey != nil { + retryKeys.Insert(*retryKey) + } } } diff --git a/pkg/nsx/services/inventory/inventory_test.go b/pkg/nsx/services/inventory/inventory_test.go index 2c46f5105..7f569a872 100644 --- a/pkg/nsx/services/inventory/inventory_test.go +++ b/pkg/nsx/services/inventory/inventory_test.go @@ -194,6 +194,34 @@ func TestInventoryService_SyncInventoryObject(t *testing.T) { assert.NoError(t, err) }) + t.Run("Valid InventoryVirtualMachine key", func(t *testing.T) { + key := InventoryKey{Key: "tks/vm-1", InventoryType: InventoryVirtualMachine, ExternalId: "uuid-1"} + bufferedKeys := sets.New[InventoryKey]() + bufferedKeys.Insert(key) + patches := gomonkey.ApplyMethod(reflect.TypeOf(inventoryService), "SyncVirtualMachineTag", func(s *InventoryService, name string, namespace string, key InventoryKey) *InventoryKey { + return nil + }) + defer patches.Reset() + retryKeys, err := inventoryService.SyncInventoryObject(bufferedKeys) + assert.Empty(t, retryKeys) + assert.NoError(t, err) + }) + + t.Run("InventoryVirtualMachine key with sync failure", func(t *testing.T) { + key := InventoryKey{Key: "tks/vm-1", InventoryType: InventoryVirtualMachine, ExternalId: "uuid-1"} + bufferedKeys := sets.New[InventoryKey]() + bufferedKeys.Insert(key) + + retryKey := InventoryKey{Key: "tks/vm-1", InventoryType: InventoryVirtualMachine, ExternalId: "uuid-1"} + patches := gomonkey.ApplyMethod(reflect.TypeOf(inventoryService), "SyncVirtualMachineTag", func(s *InventoryService, name string, namespace string, key InventoryKey) *InventoryKey { + return &retryKey + }) + defer patches.Reset() + retryKeys, err := inventoryService.SyncInventoryObject(bufferedKeys) + assert.Contains(t, retryKeys, retryKey) + assert.NoError(t, err) + }) + } func TestInventoryService_DeleteResource(t *testing.T) { @@ -524,3 +552,34 @@ func TestInventoryService_Cleanup(t *testing.T) { assert.Equal(t, 1, len(inventoryService.ClusterStore.List())) }) } + +func TestInventoryService_SyncInventoryStoreByType_InitTaggedVMsError(t *testing.T) { + inventoryService, _ := createService(t) + + patches := gomonkey.ApplyPrivateMethod(reflect.TypeOf(inventoryService), "initContainerProject", func(_ *InventoryService, _ string) error { + return nil + }) + patches.ApplyPrivateMethod(reflect.TypeOf(inventoryService), "initContainerApplicationInstance", func(_ *InventoryService, _ string) error { + return nil + }) + patches.ApplyPrivateMethod(reflect.TypeOf(inventoryService), "initContainerApplication", func(_ *InventoryService, _ string) error { + return nil + }) + patches.ApplyPrivateMethod(reflect.TypeOf(inventoryService), "initContainerClusterNode", func(_ *InventoryService, _ string) error { + return nil + }) + patches.ApplyPrivateMethod(reflect.TypeOf(inventoryService), "initContainerNetworkPolicy", func(_ *InventoryService, _ string) error { + return nil + }) + patches.ApplyPrivateMethod(reflect.TypeOf(inventoryService), "initContainerIngressPolicy", func(_ *InventoryService, _ string) error { + return nil + }) + patches.ApplyPrivateMethod(reflect.TypeOf(inventoryService), "initTaggedVMs", func(_ *InventoryService) error { + return errors.New("failed to init tagged VMs") + }) + defer patches.Reset() + + err := inventoryService.SyncInventoryStoreByType("test-cluster-uuid") + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to init tagged VMs") +} diff --git a/pkg/nsx/services/inventory/types.go b/pkg/nsx/services/inventory/types.go index 9e51f432e..e8d07e23b 100644 --- a/pkg/nsx/services/inventory/types.go +++ b/pkg/nsx/services/inventory/types.go @@ -22,6 +22,9 @@ const ( // typically mapping to Kubernetes network policies. ContainerNetworkPolicy InventoryType = "ContainerNetworkPolicy" ContainerIngressPolicy InventoryType = "ContainerIngressPolicy" + // InventoryVirtualMachine represents the inventory type for tagging + // NSX Inventory VirtualMachine objects with VKS cluster identifiers. + InventoryVirtualMachine InventoryType = "InventoryVirtualMachine" InventoryClusterTypeSupervisor = "SupervisorCluster" InventoryClusterCNIType = "NCP" @@ -48,6 +51,11 @@ const ( InventoryStatusDown = "DOWN" InventoryStatusUnknown = "UNKNOWN" + TagScopeClusterName = "nsx-op/cluster-name" + + // CAPIClusterNameLabel is the CAPI contract label that identifies which cluster a VM belongs to. + CAPIClusterNameLabel = "cluster.x-k8s.io/cluster-name" + NcpLbError = "ncp/error.loadbalancer" NcpLbPortError = "ncp/error.loadbalancer.unrealized_ports" NcpLbEpError = "ncp/error.loadbalancer_endpoints" diff --git a/pkg/nsx/services/inventory/vm.go b/pkg/nsx/services/inventory/vm.go new file mode 100644 index 000000000..8be490fa4 --- /dev/null +++ b/pkg/nsx/services/inventory/vm.go @@ -0,0 +1,233 @@ +package inventory + +import ( + "context" + "fmt" + "net/url" + "strings" + + vmv1alpha1 "github.com/vmware-tanzu/vm-operator/api/v1alpha1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + + nsxvmwarecomv1alpha1 "github.com/vmware-tanzu/nsx-operator/pkg/apis/legacy/v1alpha1" +) + +const ( + vmAddTagsURL = "api/v1/fabric/virtual-machines?action=add_tags" + vmRemoveTagsURL = "api/v1/fabric/virtual-machines?action=remove_tags" + vmSearchURL = "api/v1/search/query" +) + +type vmTagUpdate struct { + ExternalID string `json:"external_id"` + Tags []vmTag `json:"tags"` +} + +type vmTag struct { + Scope string `json:"scope"` + Tag string `json:"tag"` +} + +// initTaggedVMs populates the taggedVMs store from NSX Inventory at startup. +// It uses the NSX search API to query only VMs that have the nsx-op/cluster-name tag, +// avoiding listing all fabric virtual machines. +func (s *InventoryService) initTaggedVMs() error { + log.Info("Populating tagged VM store from NSX Inventory") + tagScopeEscaped := strings.ReplaceAll(TagScopeClusterName, "/", "\\/") + query := fmt.Sprintf("resource_type:VirtualMachine AND tags.scope:%s", tagScopeEscaped) + cursor := "" + for { + searchURL := fmt.Sprintf("%s?query=%s", vmSearchURL, url.QueryEscape(query)) + if cursor != "" { + searchURL = fmt.Sprintf("%s&cursor=%s", searchURL, cursor) + } + resp, err := s.NSXClient.Cluster.HttpGet(searchURL) + if err != nil { + return fmt.Errorf("failed to search tagged virtual machines: %w", err) + } + + results, _ := resp["results"].([]interface{}) + for _, r := range results { + vmMap, ok := r.(map[string]interface{}) + if !ok { + continue + } + externalID, _ := vmMap["external_id"].(string) + displayName, _ := vmMap["display_name"].(string) + tags, _ := vmMap["tags"].([]interface{}) + for _, t := range tags { + tagMap, ok := t.(map[string]interface{}) + if !ok { + continue + } + scope, _ := tagMap["scope"].(string) + if scope == TagScopeClusterName { + tagValue, _ := tagMap["tag"].(string) + s.taggedVMs[externalID] = tagValue + log.Debug("Found previously tagged VM in NSX Inventory", + "displayName", displayName, "externalID", externalID) + break + } + } + } + + nextCursor, _ := resp["cursor"].(string) + if nextCursor == "" { + break + } + cursor = nextCursor + } + log.Info("Tagged VM store populated", "count", len(s.taggedVMs)) + return nil +} + +// SyncVirtualMachineTag handles the tagging/untagging of VirtualMachine objects +// in NSX Inventory with the nsx-op/cluster-name tag. +func (s *InventoryService) SyncVirtualMachineTag(name, namespace string, key InventoryKey) *InventoryKey { + vm := &vmv1alpha1.VirtualMachine{} + err := s.Client.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: namespace}, vm) + if apierrors.IsNotFound(err) { + delete(s.taggedVMs, key.ExternalId) + log.Info("VirtualMachine not found, will be removed from NSX Inventory", + "name", name, "namespace", namespace) + return nil + } + if err != nil { + log.Error(err, "Failed to get VirtualMachine, will retry", "name", name, "namespace", namespace) + return &key + } + + externalID := getVMExternalID(vm) + if externalID == "" { + log.Error(nil, "VM has no InstanceUUID, cannot process", "namespace", namespace, "vm", name) + return nil + } + + capiClusterName := vm.Labels[CAPIClusterNameLabel] + nsxSA, err := s.findRealizedNSXServiceAccountForCluster(namespace, capiClusterName) + if err != nil { + log.Error(err, "Failed to look up NSXServiceAccount, will retry", + "namespace", namespace, "cluster", capiClusterName) + return &key + } + + if nsxSA == nil || nsxSA.Status.Phase != nsxvmwarecomv1alpha1.NSXServiceAccountPhaseRealized { + existingTag, ok := s.taggedVMs[externalID] + if !ok { + return nil + } + if err := s.RemoveClusterNameTagFromVM(externalID, existingTag); err != nil { + log.Error(err, "Failed to remove tag from VM, will retry", + "namespace", namespace, "vm", name, "externalID", externalID) + return &key + } + delete(s.taggedVMs, externalID) + log.Info("Removed cluster-name tag from VM in NSX Inventory", + "namespace", namespace, "vm", name) + return nil + } + + if _, ok := s.taggedVMs[externalID]; ok { + return nil + } + + clusterName := nsxSA.Status.ClusterName + if clusterName == "" { + log.Error(nil, "NSXServiceAccount has empty clusterName", + "namespace", namespace, "vm", name, "serviceAccount", nsxSA.Name) + return nil + } + + if err := s.addClusterNameTagToVM(externalID, clusterName); err != nil { + log.Error(err, "Failed to add cluster-name tag to VM, will retry", + "namespace", namespace, "vm", name, "externalID", externalID) + return &key + } + + s.taggedVMs[externalID] = clusterName + log.Info("Successfully tagged VM in NSX Inventory", + "namespace", namespace, "vm", name, "clusterName", clusterName) + return nil +} + +// findRealizedNSXServiceAccountForCluster looks up a realized NSXServiceAccount +// in the given namespace whose OwnerReference Cluster matches the specified clusterName. +func (s *InventoryService) findRealizedNSXServiceAccountForCluster(namespace, clusterName string) (*nsxvmwarecomv1alpha1.NSXServiceAccount, error) { + nsxSAList := &nsxvmwarecomv1alpha1.NSXServiceAccountList{} + if err := s.Client.List(context.TODO(), nsxSAList, &client.ListOptions{ + Namespace: namespace, + }); err != nil { + return nil, fmt.Errorf("failed to list NSXServiceAccounts in namespace %s: %w", namespace, err) + } + + for i := range nsxSAList.Items { + sa := &nsxSAList.Items[i] + if sa.DeletionTimestamp.IsZero() && sa.Status.Phase == nsxvmwarecomv1alpha1.NSXServiceAccountPhaseRealized && ownerClusterMatches(sa, clusterName) { + return sa, nil + } + } + return nil, nil +} + +// ownerClusterMatches checks if the NSXServiceAccount has an OwnerReference +// to a CAPI Cluster with the given name. +func ownerClusterMatches(sa *nsxvmwarecomv1alpha1.NSXServiceAccount, clusterName string) bool { + if clusterName == "" { + return false + } + for _, ref := range sa.OwnerReferences { + if ref.Kind == "Cluster" && strings.Contains(ref.APIVersion, "cluster.x-k8s.io") && ref.Name == clusterName { + return true + } + } + return false +} + +// getVMExternalID extracts the instance UUID from a VirtualMachine CR. +// NSX Inventory uses the vSphere instanceUuid as external_id for VirtualMachine objects. +func getVMExternalID(vm *vmv1alpha1.VirtualMachine) string { + return vm.Status.InstanceUUID +} + +// addClusterNameTagToVM adds the nsx-op/cluster-name tag to the NSX Inventory +// VirtualMachine object. Uses the add_tags API to preserve any existing tags. +func (s *InventoryService) addClusterNameTagToVM(externalID, clusterName string) error { + update := vmTagUpdate{ + ExternalID: externalID, + Tags: []vmTag{ + { + Scope: TagScopeClusterName, + Tag: clusterName, + }, + }, + } + + _, err := s.NSXClient.Cluster.HttpPost(vmAddTagsURL, update) + if err != nil { + return fmt.Errorf("failed to add VM tags via NSX API: %w", err) + } + return nil +} + +// RemoveClusterNameTagFromVM removes the nsx-op/cluster-name tag with the +// specified value from the NSX Inventory VirtualMachine object. +// The NSX remove_tags API requires both scope and tag value to match exactly. +func (s *InventoryService) RemoveClusterNameTagFromVM(externalID, tagValue string) error { + update := vmTagUpdate{ + ExternalID: externalID, + Tags: []vmTag{ + { + Scope: TagScopeClusterName, + Tag: tagValue, + }, + }, + } + + _, err := s.NSXClient.Cluster.HttpPost(vmRemoveTagsURL, update) + if err != nil { + return fmt.Errorf("failed to remove VM tag via NSX API: %w", err) + } + return nil +} diff --git a/pkg/nsx/services/inventory/vm_test.go b/pkg/nsx/services/inventory/vm_test.go new file mode 100644 index 000000000..81a81bc28 --- /dev/null +++ b/pkg/nsx/services/inventory/vm_test.go @@ -0,0 +1,934 @@ +package inventory + +import ( + "context" + "reflect" + "testing" + + "github.com/agiledragon/gomonkey/v2" + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/assert" + vmv1alpha1 "github.com/vmware-tanzu/vm-operator/api/v1alpha1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + + nsxvmwarecomv1alpha1 "github.com/vmware-tanzu/nsx-operator/pkg/apis/legacy/v1alpha1" + mockClient "github.com/vmware-tanzu/nsx-operator/pkg/mock/controller-runtime/client" + "github.com/vmware-tanzu/nsx-operator/pkg/nsx" + "github.com/vmware-tanzu/nsx-operator/pkg/nsx/ratelimiter" + commonservice "github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/common" +) + +func createVMTestService(t *testing.T) (*InventoryService, *mockClient.MockClient) { + config2 := nsx.NewConfig("localhost", "1", "1", []string{}, 10, 3, 20, 20, true, true, true, ratelimiter.AIMD, nil, nil, []string{"127.0.0.1"}) + cluster, _ := nsx.NewCluster(config2) + rc := cluster.NewRestConnector() + + mockCtrl := gomock.NewController(t) + k8sClient := mockClient.NewMockClient(mockCtrl) + + service := NewInventoryService(commonservice.Service{ + Client: k8sClient, + NSXClient: &nsx.Client{ + RestConnector: rc, + Cluster: cluster, + }, + }) + return service, k8sClient +} + +func TestSyncVirtualMachineTag_VMDeleted(t *testing.T) { + service, k8sClient := createVMTestService(t) + service.taggedVMs["uuid-1234"] = "cluster-a" + + k8sClient.EXPECT().Get(gomock.Any(), types.NamespacedName{Name: "vm-1", Namespace: "tks"}, gomock.Any()). + Return(apierrors.NewNotFound(schema.GroupResource{Group: "vmoperator.vmware.com", Resource: "virtualmachines"}, "vm-1")) + + key := InventoryKey{ + InventoryType: InventoryVirtualMachine, + ExternalId: "uuid-1234", + Key: "tks/vm-1", + } + result := service.SyncVirtualMachineTag("vm-1", "tks", key) + + assert.Nil(t, result) + _, exists := service.taggedVMs["uuid-1234"] + assert.False(t, exists, "taggedVMs should be cleaned up on VM deletion") +} + +func TestSyncVirtualMachineTag_VMGetError(t *testing.T) { + service, k8sClient := createVMTestService(t) + + k8sClient.EXPECT().Get(gomock.Any(), types.NamespacedName{Name: "vm-1", Namespace: "tks"}, gomock.Any()). + Return(assert.AnError) + + key := InventoryKey{ + InventoryType: InventoryVirtualMachine, + ExternalId: "uuid-1234", + Key: "tks/vm-1", + } + result := service.SyncVirtualMachineTag("vm-1", "tks", key) + + assert.NotNil(t, result, "should retry on transient Get error") + assert.Equal(t, key, *result) +} + +func TestSyncVirtualMachineTag_NoInstanceUUID(t *testing.T) { + service, k8sClient := createVMTestService(t) + + k8sClient.EXPECT().Get(gomock.Any(), types.NamespacedName{Name: "vm-1", Namespace: "tks"}, gomock.Any()). + DoAndReturn(func(_ context.Context, _ types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + vm := obj.(*vmv1alpha1.VirtualMachine) + vm.Status.InstanceUUID = "" + return nil + }) + + key := InventoryKey{ + InventoryType: InventoryVirtualMachine, + ExternalId: "", + Key: "tks/vm-1", + } + result := service.SyncVirtualMachineTag("vm-1", "tks", key) + + assert.Nil(t, result, "should not retry when InstanceUUID is empty") +} + +func TestSyncVirtualMachineTag_NoNSXServiceAccount(t *testing.T) { + service, k8sClient := createVMTestService(t) + + k8sClient.EXPECT().Get(gomock.Any(), types.NamespacedName{Name: "vm-1", Namespace: "tks"}, gomock.Any()). + DoAndReturn(func(_ context.Context, _ types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + vm := obj.(*vmv1alpha1.VirtualMachine) + vm.Status.InstanceUUID = "uuid-1234" + vm.Labels = map[string]string{CAPIClusterNameLabel: "test-cluster"} + return nil + }) + + k8sClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, list client.ObjectList, opts ...client.ListOption) error { + saList := list.(*nsxvmwarecomv1alpha1.NSXServiceAccountList) + saList.Items = []nsxvmwarecomv1alpha1.NSXServiceAccount{} + return nil + }, + ) + + key := InventoryKey{ + InventoryType: InventoryVirtualMachine, + ExternalId: "uuid-1234", + Key: "tks/vm-1", + } + result := service.SyncVirtualMachineTag("vm-1", "tks", key) + + assert.Nil(t, result, "should not retry when no SA and VM not in taggedVMs") +} + +func TestSyncVirtualMachineTag_NoNSXServiceAccountWithTaggedVM(t *testing.T) { + service, k8sClient := createVMTestService(t) + service.taggedVMs["uuid-1234"] = "cluster-a" + + k8sClient.EXPECT().Get(gomock.Any(), types.NamespacedName{Name: "vm-1", Namespace: "tks"}, gomock.Any()). + DoAndReturn(func(_ context.Context, _ types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + vm := obj.(*vmv1alpha1.VirtualMachine) + vm.Status.InstanceUUID = "uuid-1234" + vm.Labels = map[string]string{CAPIClusterNameLabel: "test-cluster"} + return nil + }) + + k8sClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, list client.ObjectList, opts ...client.ListOption) error { + saList := list.(*nsxvmwarecomv1alpha1.NSXServiceAccountList) + saList.Items = []nsxvmwarecomv1alpha1.NSXServiceAccount{} + return nil + }, + ) + + patches := gomonkey.ApplyMethod(reflect.TypeOf(service), "RemoveClusterNameTagFromVM", func(_ *InventoryService, externalID, tagValue string) error { + assert.Equal(t, "uuid-1234", externalID) + assert.Equal(t, "cluster-a", tagValue, "should pass the existing tag value for precise removal") + return nil + }) + defer patches.Reset() + + key := InventoryKey{ + InventoryType: InventoryVirtualMachine, + ExternalId: "uuid-1234", + Key: "tks/vm-1", + } + result := service.SyncVirtualMachineTag("vm-1", "tks", key) + + assert.Nil(t, result) + _, exists := service.taggedVMs["uuid-1234"] + assert.False(t, exists, "taggedVMs entry should be removed after successful untag") +} + +func TestSyncVirtualMachineTag_RemoveTagFails(t *testing.T) { + service, k8sClient := createVMTestService(t) + service.taggedVMs["uuid-1234"] = "cluster-a" + + k8sClient.EXPECT().Get(gomock.Any(), types.NamespacedName{Name: "vm-1", Namespace: "tks"}, gomock.Any()). + DoAndReturn(func(_ context.Context, _ types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + vm := obj.(*vmv1alpha1.VirtualMachine) + vm.Status.InstanceUUID = "uuid-1234" + vm.Labels = map[string]string{CAPIClusterNameLabel: "test-cluster"} + return nil + }) + + k8sClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, list client.ObjectList, opts ...client.ListOption) error { + saList := list.(*nsxvmwarecomv1alpha1.NSXServiceAccountList) + saList.Items = []nsxvmwarecomv1alpha1.NSXServiceAccount{} + return nil + }, + ) + + patches := gomonkey.ApplyMethod(reflect.TypeOf(service), "RemoveClusterNameTagFromVM", func(_ *InventoryService, _, _ string) error { + return assert.AnError + }) + defer patches.Reset() + + key := InventoryKey{ + InventoryType: InventoryVirtualMachine, + ExternalId: "uuid-1234", + Key: "tks/vm-1", + } + result := service.SyncVirtualMachineTag("vm-1", "tks", key) + + assert.NotNil(t, result, "should retry when remove tag fails") + _, exists := service.taggedVMs["uuid-1234"] + assert.True(t, exists, "taggedVMs entry should remain on failure") +} + +func TestSyncVirtualMachineTag_AddTag(t *testing.T) { + service, k8sClient := createVMTestService(t) + + k8sClient.EXPECT().Get(gomock.Any(), types.NamespacedName{Name: "vm-1", Namespace: "tks"}, gomock.Any()). + DoAndReturn(func(_ context.Context, _ types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + vm := obj.(*vmv1alpha1.VirtualMachine) + vm.Status.InstanceUUID = "uuid-1234" + vm.Labels = map[string]string{CAPIClusterNameLabel: "test-cluster"} + return nil + }) + + k8sClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, list client.ObjectList, opts ...client.ListOption) error { + saList := list.(*nsxvmwarecomv1alpha1.NSXServiceAccountList) + saList.Items = []nsxvmwarecomv1alpha1.NSXServiceAccount{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-sa", + Namespace: "tks", + OwnerReferences: []metav1.OwnerReference{ + {APIVersion: "cluster.x-k8s.io/v1beta2", Kind: "Cluster", Name: "test-cluster"}, + }, + }, + Status: nsxvmwarecomv1alpha1.NSXServiceAccountStatus{ + Phase: nsxvmwarecomv1alpha1.NSXServiceAccountPhaseRealized, + ClusterName: "cluster-abc", + }, + }, + } + return nil + }, + ) + + patches := gomonkey.ApplyPrivateMethod(reflect.TypeOf(service), "addClusterNameTagToVM", func(_ *InventoryService, _ string, _ string) error { + return nil + }) + defer patches.Reset() + + key := InventoryKey{ + InventoryType: InventoryVirtualMachine, + ExternalId: "uuid-1234", + Key: "tks/vm-1", + } + result := service.SyncVirtualMachineTag("vm-1", "tks", key) + + assert.Nil(t, result) + assert.Equal(t, "cluster-abc", service.taggedVMs["uuid-1234"]) +} + +func TestSyncVirtualMachineTag_MultipleClustersInSameNamespace(t *testing.T) { + service, k8sClient := createVMTestService(t) + + k8sClient.EXPECT().Get(gomock.Any(), types.NamespacedName{Name: "vm-a", Namespace: "shared-ns"}, gomock.Any()). + DoAndReturn(func(_ context.Context, _ types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + vm := obj.(*vmv1alpha1.VirtualMachine) + vm.Status.InstanceUUID = "uuid-a" + vm.Labels = map[string]string{CAPIClusterNameLabel: "cluster-a"} + return nil + }) + + k8sClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, list client.ObjectList, opts ...client.ListOption) error { + saList := list.(*nsxvmwarecomv1alpha1.NSXServiceAccountList) + saList.Items = []nsxvmwarecomv1alpha1.NSXServiceAccount{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "sa-cluster-a", + Namespace: "shared-ns", + OwnerReferences: []metav1.OwnerReference{ + {APIVersion: "cluster.x-k8s.io/v1beta2", Kind: "Cluster", Name: "cluster-a"}, + }, + }, + Status: nsxvmwarecomv1alpha1.NSXServiceAccountStatus{ + Phase: nsxvmwarecomv1alpha1.NSXServiceAccountPhaseRealized, + ClusterName: "nsx-cluster-name-a", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "sa-cluster-b", + Namespace: "shared-ns", + OwnerReferences: []metav1.OwnerReference{ + {APIVersion: "cluster.x-k8s.io/v1beta2", Kind: "Cluster", Name: "cluster-b"}, + }, + }, + Status: nsxvmwarecomv1alpha1.NSXServiceAccountStatus{ + Phase: nsxvmwarecomv1alpha1.NSXServiceAccountPhaseRealized, + ClusterName: "nsx-cluster-name-b", + }, + }, + } + return nil + }, + ) + + patches := gomonkey.ApplyPrivateMethod(reflect.TypeOf(service), "addClusterNameTagToVM", func(_ *InventoryService, externalID string, clusterName string) error { + assert.Equal(t, "uuid-a", externalID) + assert.Equal(t, "nsx-cluster-name-a", clusterName, "should use SA for cluster-a, not cluster-b") + return nil + }) + defer patches.Reset() + + key := InventoryKey{ + InventoryType: InventoryVirtualMachine, + ExternalId: "uuid-a", + Key: "shared-ns/vm-a", + } + result := service.SyncVirtualMachineTag("vm-a", "shared-ns", key) + + assert.Nil(t, result) + assert.Equal(t, "nsx-cluster-name-a", service.taggedVMs["uuid-a"], + "should tag with cluster-a's NSX cluster name, not cluster-b's") +} + +func TestSyncVirtualMachineTag_AddTagIdempotent(t *testing.T) { + service, k8sClient := createVMTestService(t) + service.taggedVMs["uuid-1234"] = "cluster-abc" + + k8sClient.EXPECT().Get(gomock.Any(), types.NamespacedName{Name: "vm-1", Namespace: "tks"}, gomock.Any()). + DoAndReturn(func(_ context.Context, _ types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + vm := obj.(*vmv1alpha1.VirtualMachine) + vm.Status.InstanceUUID = "uuid-1234" + vm.Labels = map[string]string{CAPIClusterNameLabel: "test-cluster"} + return nil + }) + + k8sClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, list client.ObjectList, opts ...client.ListOption) error { + saList := list.(*nsxvmwarecomv1alpha1.NSXServiceAccountList) + saList.Items = []nsxvmwarecomv1alpha1.NSXServiceAccount{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-sa", + Namespace: "tks", + OwnerReferences: []metav1.OwnerReference{ + {APIVersion: "cluster.x-k8s.io/v1beta2", Kind: "Cluster", Name: "test-cluster"}, + }, + }, + Status: nsxvmwarecomv1alpha1.NSXServiceAccountStatus{ + Phase: nsxvmwarecomv1alpha1.NSXServiceAccountPhaseRealized, + ClusterName: "cluster-abc", + }, + }, + } + return nil + }, + ) + + key := InventoryKey{ + InventoryType: InventoryVirtualMachine, + ExternalId: "uuid-1234", + Key: "tks/vm-1", + } + result := service.SyncVirtualMachineTag("vm-1", "tks", key) + + assert.Nil(t, result, "should skip when already tagged (idempotent)") + assert.Equal(t, "cluster-abc", service.taggedVMs["uuid-1234"]) +} + +func TestSyncVirtualMachineTag_AddTagFails(t *testing.T) { + service, k8sClient := createVMTestService(t) + + k8sClient.EXPECT().Get(gomock.Any(), types.NamespacedName{Name: "vm-1", Namespace: "tks"}, gomock.Any()). + DoAndReturn(func(_ context.Context, _ types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + vm := obj.(*vmv1alpha1.VirtualMachine) + vm.Status.InstanceUUID = "uuid-1234" + vm.Labels = map[string]string{CAPIClusterNameLabel: "test-cluster"} + return nil + }) + + k8sClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, list client.ObjectList, opts ...client.ListOption) error { + saList := list.(*nsxvmwarecomv1alpha1.NSXServiceAccountList) + saList.Items = []nsxvmwarecomv1alpha1.NSXServiceAccount{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-sa", + Namespace: "tks", + OwnerReferences: []metav1.OwnerReference{ + {APIVersion: "cluster.x-k8s.io/v1beta2", Kind: "Cluster", Name: "test-cluster"}, + }, + }, + Status: nsxvmwarecomv1alpha1.NSXServiceAccountStatus{ + Phase: nsxvmwarecomv1alpha1.NSXServiceAccountPhaseRealized, + ClusterName: "cluster-abc", + }, + }, + } + return nil + }, + ) + + patches := gomonkey.ApplyPrivateMethod(reflect.TypeOf(service), "addClusterNameTagToVM", func(_ *InventoryService, _ string, _ string) error { + return assert.AnError + }) + defer patches.Reset() + + key := InventoryKey{ + InventoryType: InventoryVirtualMachine, + ExternalId: "uuid-1234", + Key: "tks/vm-1", + } + result := service.SyncVirtualMachineTag("vm-1", "tks", key) + + assert.NotNil(t, result, "should retry when add tag fails") + _, exists := service.taggedVMs["uuid-1234"] + assert.False(t, exists, "taggedVMs should not have entry on failure") +} + +func TestSyncVirtualMachineTag_EmptyClusterName(t *testing.T) { + service, k8sClient := createVMTestService(t) + + k8sClient.EXPECT().Get(gomock.Any(), types.NamespacedName{Name: "vm-1", Namespace: "tks"}, gomock.Any()). + DoAndReturn(func(_ context.Context, _ types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + vm := obj.(*vmv1alpha1.VirtualMachine) + vm.Status.InstanceUUID = "uuid-1234" + vm.Labels = map[string]string{CAPIClusterNameLabel: "test-cluster"} + return nil + }) + + k8sClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, list client.ObjectList, opts ...client.ListOption) error { + saList := list.(*nsxvmwarecomv1alpha1.NSXServiceAccountList) + saList.Items = []nsxvmwarecomv1alpha1.NSXServiceAccount{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "test-sa", + Namespace: "tks", + OwnerReferences: []metav1.OwnerReference{ + {APIVersion: "cluster.x-k8s.io/v1beta2", Kind: "Cluster", Name: "test-cluster"}, + }, + }, + Status: nsxvmwarecomv1alpha1.NSXServiceAccountStatus{ + Phase: nsxvmwarecomv1alpha1.NSXServiceAccountPhaseRealized, + ClusterName: "", + }, + }, + } + return nil + }, + ) + + key := InventoryKey{ + InventoryType: InventoryVirtualMachine, + ExternalId: "uuid-1234", + Key: "tks/vm-1", + } + result := service.SyncVirtualMachineTag("vm-1", "tks", key) + + assert.Nil(t, result, "should not retry when clusterName is empty") + _, exists := service.taggedVMs["uuid-1234"] + assert.False(t, exists) +} + +func TestSyncVirtualMachineTag_ListSAError(t *testing.T) { + service, k8sClient := createVMTestService(t) + + k8sClient.EXPECT().Get(gomock.Any(), types.NamespacedName{Name: "vm-1", Namespace: "tks"}, gomock.Any()). + DoAndReturn(func(_ context.Context, _ types.NamespacedName, obj client.Object, opts ...client.GetOption) error { + vm := obj.(*vmv1alpha1.VirtualMachine) + vm.Status.InstanceUUID = "uuid-1234" + vm.Labels = map[string]string{CAPIClusterNameLabel: "test-cluster"} + return nil + }) + + k8sClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()).Return(assert.AnError) + + key := InventoryKey{ + InventoryType: InventoryVirtualMachine, + ExternalId: "uuid-1234", + Key: "tks/vm-1", + } + result := service.SyncVirtualMachineTag("vm-1", "tks", key) + + assert.NotNil(t, result, "should retry when List NSXServiceAccount fails") +} + +func TestInitTaggedVMs(t *testing.T) { + t.Run("NormalFlow", func(t *testing.T) { + service, _ := createVMTestService(t) + + patches := gomonkey.ApplyMethod(reflect.TypeOf(service.NSXClient.Cluster), "HttpGet", func(_ *nsx.Cluster, url string) (map[string]interface{}, error) { + return map[string]interface{}{ + "results": []interface{}{ + map[string]interface{}{ + "external_id": "uuid-1", + "display_name": "vm-1", + "tags": []interface{}{ + map[string]interface{}{ + "scope": TagScopeClusterName, + "tag": "cluster-abc", + }, + }, + }, + map[string]interface{}{ + "external_id": "uuid-2", + "display_name": "vm-2", + "tags": []interface{}{ + map[string]interface{}{ + "scope": "other-scope", + "tag": "other-value", + }, + }, + }, + map[string]interface{}{ + "external_id": "uuid-3", + "display_name": "vm-3", + "tags": []interface{}{ + map[string]interface{}{ + "scope": TagScopeClusterName, + "tag": "cluster-def", + }, + }, + }, + }, + }, nil + }) + defer patches.Reset() + + err := service.initTaggedVMs() + assert.NoError(t, err) + assert.Equal(t, 2, len(service.taggedVMs)) + assert.Equal(t, "cluster-abc", service.taggedVMs["uuid-1"]) + assert.Equal(t, "cluster-def", service.taggedVMs["uuid-3"]) + _, exists := service.taggedVMs["uuid-2"] + assert.False(t, exists, "VM without cluster-name tag should not be in store") + }) + + t.Run("HttpGetError", func(t *testing.T) { + service, _ := createVMTestService(t) + + patches := gomonkey.ApplyMethod(reflect.TypeOf(service.NSXClient.Cluster), "HttpGet", func(_ *nsx.Cluster, url string) (map[string]interface{}, error) { + return nil, assert.AnError + }) + defer patches.Reset() + + err := service.initTaggedVMs() + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to search tagged virtual machines") + }) + + t.Run("Pagination", func(t *testing.T) { + service, _ := createVMTestService(t) + callCount := 0 + + patches := gomonkey.ApplyMethod(reflect.TypeOf(service.NSXClient.Cluster), "HttpGet", func(_ *nsx.Cluster, url string) (map[string]interface{}, error) { + callCount++ + if callCount == 1 { + return map[string]interface{}{ + "results": []interface{}{ + map[string]interface{}{ + "external_id": "uuid-page1", + "display_name": "vm-page1", + "tags": []interface{}{ + map[string]interface{}{ + "scope": TagScopeClusterName, + "tag": "cluster-1", + }, + }, + }, + }, + "cursor": "page2", + }, nil + } + return map[string]interface{}{ + "results": []interface{}{ + map[string]interface{}{ + "external_id": "uuid-page2", + "display_name": "vm-page2", + "tags": []interface{}{ + map[string]interface{}{ + "scope": TagScopeClusterName, + "tag": "cluster-2", + }, + }, + }, + }, + }, nil + }) + defer patches.Reset() + + err := service.initTaggedVMs() + assert.NoError(t, err) + assert.Equal(t, 2, len(service.taggedVMs)) + assert.Equal(t, "cluster-1", service.taggedVMs["uuid-page1"]) + assert.Equal(t, "cluster-2", service.taggedVMs["uuid-page2"]) + assert.Equal(t, 2, callCount) + }) +} + +func TestInitTaggedVMs_InvalidResults(t *testing.T) { + t.Run("NonMapResult", func(t *testing.T) { + service, _ := createVMTestService(t) + + patches := gomonkey.ApplyMethod(reflect.TypeOf(service.NSXClient.Cluster), "HttpGet", func(_ *nsx.Cluster, url string) (map[string]interface{}, error) { + return map[string]interface{}{ + "results": []interface{}{ + "not-a-map", + }, + }, nil + }) + defer patches.Reset() + + err := service.initTaggedVMs() + assert.NoError(t, err) + assert.Equal(t, 0, len(service.taggedVMs)) + }) + + t.Run("NonMapTag", func(t *testing.T) { + service, _ := createVMTestService(t) + + patches := gomonkey.ApplyMethod(reflect.TypeOf(service.NSXClient.Cluster), "HttpGet", func(_ *nsx.Cluster, url string) (map[string]interface{}, error) { + return map[string]interface{}{ + "results": []interface{}{ + map[string]interface{}{ + "external_id": "uuid-1", + "display_name": "vm-1", + "tags": []interface{}{ + "not-a-map-tag", + }, + }, + }, + }, nil + }) + defer patches.Reset() + + err := service.initTaggedVMs() + assert.NoError(t, err) + assert.Equal(t, 0, len(service.taggedVMs)) + }) + + t.Run("EmptyResults", func(t *testing.T) { + service, _ := createVMTestService(t) + + patches := gomonkey.ApplyMethod(reflect.TypeOf(service.NSXClient.Cluster), "HttpGet", func(_ *nsx.Cluster, url string) (map[string]interface{}, error) { + return map[string]interface{}{}, nil + }) + defer patches.Reset() + + err := service.initTaggedVMs() + assert.NoError(t, err) + assert.Equal(t, 0, len(service.taggedVMs)) + }) +} + +func TestAddClusterNameTagToVM(t *testing.T) { + t.Run("Success", func(t *testing.T) { + service, _ := createVMTestService(t) + + patches := gomonkey.ApplyMethod(reflect.TypeOf(service.NSXClient.Cluster), "HttpPost", func(_ *nsx.Cluster, url string, body interface{}) (map[string]interface{}, error) { + assert.Equal(t, vmAddTagsURL, url) + return nil, nil + }) + defer patches.Reset() + + err := service.addClusterNameTagToVM("uuid-1", "cluster-abc") + assert.NoError(t, err) + }) + + t.Run("Error", func(t *testing.T) { + service, _ := createVMTestService(t) + + patches := gomonkey.ApplyMethod(reflect.TypeOf(service.NSXClient.Cluster), "HttpPost", func(_ *nsx.Cluster, url string, body interface{}) (map[string]interface{}, error) { + return nil, assert.AnError + }) + defer patches.Reset() + + err := service.addClusterNameTagToVM("uuid-1", "cluster-abc") + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to add VM tags via NSX API") + }) +} + +func TestRemoveClusterNameTagFromVM(t *testing.T) { + t.Run("Success", func(t *testing.T) { + service, _ := createVMTestService(t) + + patches := gomonkey.ApplyMethod(reflect.TypeOf(service.NSXClient.Cluster), "HttpPost", func(_ *nsx.Cluster, url string, body interface{}) (map[string]interface{}, error) { + assert.Equal(t, vmRemoveTagsURL, url) + update := body.(vmTagUpdate) + assert.Equal(t, "uuid-1", update.ExternalID) + assert.Equal(t, TagScopeClusterName, update.Tags[0].Scope) + assert.Equal(t, "cluster-abc", update.Tags[0].Tag, "tag value must match the actual tag on the VM") + return nil, nil + }) + defer patches.Reset() + + err := service.RemoveClusterNameTagFromVM("uuid-1", "cluster-abc") + assert.NoError(t, err) + }) + + t.Run("Error", func(t *testing.T) { + service, _ := createVMTestService(t) + + patches := gomonkey.ApplyMethod(reflect.TypeOf(service.NSXClient.Cluster), "HttpPost", func(_ *nsx.Cluster, url string, body interface{}) (map[string]interface{}, error) { + return nil, assert.AnError + }) + defer patches.Reset() + + err := service.RemoveClusterNameTagFromVM("uuid-1", "cluster-abc") + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to remove VM tag via NSX API") + }) +} + +func TestFindRealizedNSXServiceAccountForCluster(t *testing.T) { + t.Run("FoundMatchingCluster", func(t *testing.T) { + service, k8sClient := createVMTestService(t) + + k8sClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, list client.ObjectList, opts ...client.ListOption) error { + saList := list.(*nsxvmwarecomv1alpha1.NSXServiceAccountList) + saList.Items = []nsxvmwarecomv1alpha1.NSXServiceAccount{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "sa-other", + Namespace: "tks", + OwnerReferences: []metav1.OwnerReference{ + {APIVersion: "cluster.x-k8s.io/v1beta2", Kind: "Cluster", Name: "other-cluster"}, + }, + }, + Status: nsxvmwarecomv1alpha1.NSXServiceAccountStatus{ + Phase: nsxvmwarecomv1alpha1.NSXServiceAccountPhaseRealized, + ClusterName: "cluster-other", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "sa-target", + Namespace: "tks", + OwnerReferences: []metav1.OwnerReference{ + {APIVersion: "cluster.x-k8s.io/v1beta2", Kind: "Cluster", Name: "target-cluster"}, + }, + }, + Status: nsxvmwarecomv1alpha1.NSXServiceAccountStatus{ + Phase: nsxvmwarecomv1alpha1.NSXServiceAccountPhaseRealized, + ClusterName: "cluster-found", + }, + }, + } + return nil + }, + ) + + sa, err := service.findRealizedNSXServiceAccountForCluster("tks", "target-cluster") + assert.NoError(t, err) + assert.NotNil(t, sa) + assert.Equal(t, "sa-target", sa.Name) + assert.Equal(t, "cluster-found", sa.Status.ClusterName) + }) + + t.Run("NoMatchingCluster", func(t *testing.T) { + service, k8sClient := createVMTestService(t) + + k8sClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, list client.ObjectList, opts ...client.ListOption) error { + saList := list.(*nsxvmwarecomv1alpha1.NSXServiceAccountList) + saList.Items = []nsxvmwarecomv1alpha1.NSXServiceAccount{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "sa-1", + Namespace: "tks", + OwnerReferences: []metav1.OwnerReference{ + {APIVersion: "cluster.x-k8s.io/v1beta2", Kind: "Cluster", Name: "other-cluster"}, + }, + }, + Status: nsxvmwarecomv1alpha1.NSXServiceAccountStatus{ + Phase: nsxvmwarecomv1alpha1.NSXServiceAccountPhaseRealized, + }, + }, + } + return nil + }, + ) + + sa, err := service.findRealizedNSXServiceAccountForCluster("tks", "target-cluster") + assert.NoError(t, err) + assert.Nil(t, sa) + }) + + t.Run("EmptyClusterName", func(t *testing.T) { + service, k8sClient := createVMTestService(t) + + k8sClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, list client.ObjectList, opts ...client.ListOption) error { + saList := list.(*nsxvmwarecomv1alpha1.NSXServiceAccountList) + saList.Items = []nsxvmwarecomv1alpha1.NSXServiceAccount{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "sa-1", + Namespace: "tks", + OwnerReferences: []metav1.OwnerReference{ + {APIVersion: "cluster.x-k8s.io/v1beta2", Kind: "Cluster", Name: "some-cluster"}, + }, + }, + Status: nsxvmwarecomv1alpha1.NSXServiceAccountStatus{ + Phase: nsxvmwarecomv1alpha1.NSXServiceAccountPhaseRealized, + }, + }, + } + return nil + }, + ) + + sa, err := service.findRealizedNSXServiceAccountForCluster("tks", "") + assert.NoError(t, err) + assert.Nil(t, sa) + }) + + t.Run("NoneRealized", func(t *testing.T) { + service, k8sClient := createVMTestService(t) + + k8sClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, list client.ObjectList, opts ...client.ListOption) error { + saList := list.(*nsxvmwarecomv1alpha1.NSXServiceAccountList) + saList.Items = []nsxvmwarecomv1alpha1.NSXServiceAccount{ + { + ObjectMeta: metav1.ObjectMeta{ + OwnerReferences: []metav1.OwnerReference{ + {APIVersion: "cluster.x-k8s.io/v1beta2", Kind: "Cluster", Name: "target-cluster"}, + }, + }, + Status: nsxvmwarecomv1alpha1.NSXServiceAccountStatus{ + Phase: nsxvmwarecomv1alpha1.NSXServiceAccountPhaseInProgress, + }, + }, + } + return nil + }, + ) + + sa, err := service.findRealizedNSXServiceAccountForCluster("tks", "target-cluster") + assert.NoError(t, err) + assert.Nil(t, sa) + }) + + t.Run("ListError", func(t *testing.T) { + service, k8sClient := createVMTestService(t) + + k8sClient.EXPECT().List(gomock.Any(), gomock.Any(), gomock.Any()).Return(assert.AnError) + + sa, err := service.findRealizedNSXServiceAccountForCluster("tks", "target-cluster") + assert.Error(t, err) + assert.Nil(t, sa) + }) +} + +func TestOwnerClusterMatches(t *testing.T) { + tests := []struct { + name string + sa *nsxvmwarecomv1alpha1.NSXServiceAccount + clusterName string + expected bool + }{ + { + name: "Matching cluster", + sa: &nsxvmwarecomv1alpha1.NSXServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + OwnerReferences: []metav1.OwnerReference{ + {APIVersion: "cluster.x-k8s.io/v1beta2", Kind: "Cluster", Name: "my-cluster"}, + }, + }, + }, + clusterName: "my-cluster", + expected: true, + }, + { + name: "Non-matching cluster name", + sa: &nsxvmwarecomv1alpha1.NSXServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + OwnerReferences: []metav1.OwnerReference{ + {APIVersion: "cluster.x-k8s.io/v1beta2", Kind: "Cluster", Name: "other-cluster"}, + }, + }, + }, + clusterName: "my-cluster", + expected: false, + }, + { + name: "Empty cluster name", + sa: &nsxvmwarecomv1alpha1.NSXServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + OwnerReferences: []metav1.OwnerReference{ + {APIVersion: "cluster.x-k8s.io/v1beta2", Kind: "Cluster", Name: "my-cluster"}, + }, + }, + }, + clusterName: "", + expected: false, + }, + { + name: "Non-CAPI OwnerRef", + sa: &nsxvmwarecomv1alpha1.NSXServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + OwnerReferences: []metav1.OwnerReference{ + {APIVersion: "apps/v1", Kind: "Deployment", Name: "my-cluster"}, + }, + }, + }, + clusterName: "my-cluster", + expected: false, + }, + { + name: "No OwnerReferences", + sa: &nsxvmwarecomv1alpha1.NSXServiceAccount{ + ObjectMeta: metav1.ObjectMeta{}, + }, + clusterName: "my-cluster", + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := ownerClusterMatches(tt.sa, tt.clusterName) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestGetVMExternalID(t *testing.T) { + vm := &vmv1alpha1.VirtualMachine{ + Status: vmv1alpha1.VirtualMachineStatus{ + InstanceUUID: "test-uuid-123", + }, + } + assert.Equal(t, "test-uuid-123", getVMExternalID(vm)) + + emptyVM := &vmv1alpha1.VirtualMachine{} + assert.Equal(t, "", getVMExternalID(emptyVM)) +} diff --git a/pkg/nsx/services/nsxserviceaccount/cluster.go b/pkg/nsx/services/nsxserviceaccount/cluster.go index 3f3dea638..cbbd0eda0 100644 --- a/pkg/nsx/services/nsxserviceaccount/cluster.go +++ b/pkg/nsx/services/nsxserviceaccount/cluster.go @@ -169,6 +169,7 @@ func (s *NSXServiceAccountService) CreateOrUpdateNSXServiceAccount(ctx context.C obj.Status.NSXManagers = s.NSXConfig.NsxApiManagers obj.Status.ClusterID = clusterId obj.Status.ClusterName = normalizedClusterName + obj.Status.SupervisorClusterName = s.NSXConfig.CoeConfig.Cluster obj.Status.Secrets = []v1alpha1.NSXSecret{{ Name: secretName, Namespace: secretNamespace, diff --git a/pkg/nsx/services/nsxserviceaccount/cluster_test.go b/pkg/nsx/services/nsxserviceaccount/cluster_test.go index 06aa79391..15246c21e 100644 --- a/pkg/nsx/services/nsxserviceaccount/cluster_test.go +++ b/pkg/nsx/services/nsxserviceaccount/cluster_test.go @@ -422,12 +422,13 @@ func TestNSXServiceAccountService_CreateOrUpdateNSXServiceAccount(t *testing.T) Message: "Success.", }, }, - VPCPath: "/orgs/default/projects/k8scl-one_test/vpcs/ns1-default-vpc", - NSXManagers: []string{"mgr1:443", "mgr2:443"}, - ProxyEndpoints: v1alpha1.NSXProxyEndpoint{}, - ClusterID: "clusterId1", - ClusterName: "k8scl-one_test-ns1-name1", - Secrets: []v1alpha1.NSXSecret{{Name: "name1-nsx-cert", Namespace: "ns1"}}, + VPCPath: "/orgs/default/projects/k8scl-one_test/vpcs/ns1-default-vpc", + NSXManagers: []string{"mgr1:443", "mgr2:443"}, + ProxyEndpoints: v1alpha1.NSXProxyEndpoint{}, + ClusterID: "clusterId1", + ClusterName: "k8scl-one_test-ns1-name1", + SupervisorClusterName: "k8scl-one:test", + Secrets: []v1alpha1.NSXSecret{{Name: "name1-nsx-cert", Namespace: "ns1"}}, }, }, }, @@ -539,12 +540,13 @@ func TestNSXServiceAccountService_CreateOrUpdateNSXServiceAccount(t *testing.T) Message: "Success.", }, }, - VPCPath: "/orgs/default/projects/k8scl-one_test/vpcs/ns1-default-vpc", - NSXManagers: []string{"mgr1:443", "mgr2:443"}, - ProxyEndpoints: v1alpha1.NSXProxyEndpoint{}, - ClusterID: "clusterId1", - ClusterName: "k8scl-one_test-ns1-name1", - Secrets: []v1alpha1.NSXSecret{{Name: "name1-nsx-cert", Namespace: "ns1"}}, + VPCPath: "/orgs/default/projects/k8scl-one_test/vpcs/ns1-default-vpc", + NSXManagers: []string{"mgr1:443", "mgr2:443"}, + ProxyEndpoints: v1alpha1.NSXProxyEndpoint{}, + ClusterID: "clusterId1", + ClusterName: "k8scl-one_test-ns1-name1", + SupervisorClusterName: "k8scl-one:test", + Secrets: []v1alpha1.NSXSecret{{Name: "name1-nsx-cert", Namespace: "ns1"}}, }, }, }, @@ -652,12 +654,13 @@ func TestNSXServiceAccountService_CreateOrUpdateNSXServiceAccount(t *testing.T) Message: "Success.", }, }, - VPCPath: "/orgs/default/projects/k8scl-one_12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456-e8ad9afc/vpcs/ns1-default-vpc", - NSXManagers: []string{"mgr1:443", "mgr2:443"}, - ProxyEndpoints: v1alpha1.NSXProxyEndpoint{}, - ClusterID: "clusterId1", - ClusterName: "k8scl-one_12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456-1a6417ee", - Secrets: []v1alpha1.NSXSecret{{Name: "name1-nsx-cert", Namespace: "ns1"}}, + VPCPath: "/orgs/default/projects/k8scl-one_12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456-e8ad9afc/vpcs/ns1-default-vpc", + NSXManagers: []string{"mgr1:443", "mgr2:443"}, + ProxyEndpoints: v1alpha1.NSXProxyEndpoint{}, + ClusterID: "clusterId1", + ClusterName: "k8scl-one_12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456-1a6417ee", + SupervisorClusterName: "k8scl-one:1234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890", + Secrets: []v1alpha1.NSXSecret{{Name: "name1-nsx-cert", Namespace: "ns1"}}, }, }, }, diff --git a/pkg/nsx/util/utils.go b/pkg/nsx/util/utils.go index 68448a05b..fdd277684 100644 --- a/pkg/nsx/util/utils.go +++ b/pkg/nsx/util/utils.go @@ -266,7 +266,7 @@ func httpErrortoNSXError(detail *ErrorDetail) NsxError { func HandleHTTPResponse(response *http.Response, result interface{}, debug bool) (error, []byte) { //nolint:staticcheck // ST1008: exported before convention; changing signature would break callers body, err := io.ReadAll(response.Body) defer response.Body.Close() - if response.StatusCode != http.StatusOK && response.StatusCode != http.StatusAccepted && response.StatusCode != http.StatusCreated { + if response.StatusCode != http.StatusOK && response.StatusCode != http.StatusAccepted && response.StatusCode != http.StatusCreated && response.StatusCode != http.StatusNoContent { err := HttpCommonError if response.StatusCode == http.StatusNotFound { err = HttpNotFoundError @@ -277,7 +277,7 @@ func HandleHTTPResponse(response *http.Response, result interface{}, debug bool) log.Error(err, "HTTP resp", "status", response.StatusCode, "request URL", response.Request.URL, "response body", string(body)) return err, nil } - if err != nil || body == nil { + if err != nil || body == nil || len(body) == 0 { return err, body } if result == nil {