Remove Subnet Finalizer (vmware-tanzu#769)

Remove SubnetSet Finalizer Add unit-test for Subnet controller Signed-off-by: Wenqi Qiu <[email protected]>
timdengyun · Oct 11, 2024 · 762863a · 762863a
1 parent edb6207
commit 762863a
Show file tree

Hide file tree

Showing 15 changed files with 974 additions and 360 deletions.
diff --git a/pkg/controllers/common/utils.go b/pkg/controllers/common/utils.go
@@ -42,7 +42,7 @@ func AllocateSubnetFromSubnetSet(subnetSet *v1alpha1.SubnetSet, vpcService servi
 			return *nsxSubnet.Path, nil
 		}
 	}
-	tags := subnetService.GenerateSubnetNSTags(subnetSet, subnetSet.Namespace)
+	tags := subnetService.GenerateSubnetNSTags(subnetSet)
 	if tags == nil {
 		return "", errors.New("failed to generate subnet tags")
 	}

diff --git a/pkg/controllers/subnet/namespace_handler.go b/pkg/controllers/subnet/namespace_handler.go
@@ -18,30 +18,30 @@ import (
 	"github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1"
 )
 
-// Subnet controller should watch event of namespace, when there are some updates of namespace labels,
+// Subnet controller should watch event of Namespace, when there are some updates of Namespace labels,
 // controller should build tags and update VpcSubnet according to new labels.
 
 type EnqueueRequestForNamespace struct {
 	Client client.Client
 }
 
 func (e *EnqueueRequestForNamespace) Create(_ context.Context, _ event.CreateEvent, _ workqueue.RateLimitingInterface) {
-	log.V(1).Info("namespace create event, do nothing")
+	log.V(1).Info("Namespace create event, do nothing")
 }
 
 func (e *EnqueueRequestForNamespace) Delete(_ context.Context, _ event.DeleteEvent, _ workqueue.RateLimitingInterface) {
-	log.V(1).Info("namespace delete event, do nothing")
+	log.V(1).Info("Namespace delete event, do nothing")
 }
 
 func (e *EnqueueRequestForNamespace) Generic(_ context.Context, _ event.GenericEvent, _ workqueue.RateLimitingInterface) {
-	log.V(1).Info("namespace generic event, do nothing")
+	log.V(1).Info("Namespace generic event, do nothing")
 }
 
 func (e *EnqueueRequestForNamespace) Update(_ context.Context, updateEvent event.UpdateEvent, l workqueue.RateLimitingInterface) {
 	obj := updateEvent.ObjectNew.(*v1.Namespace)
-	err := reconcileSubnet(e.Client, obj.Name, l)
+	err := requeueSubnet(e.Client, obj.Name, l)
 	if err != nil {
-		log.Error(err, "failed to reconcile subnet")
+		log.Error(err, "Failed to requeue Subnet")
 	}
 }
 
@@ -52,9 +52,9 @@ var PredicateFuncsNs = predicate.Funcs{
 	UpdateFunc: func(e event.UpdateEvent) bool {
 		oldObj := e.ObjectOld.(*v1.Namespace)
 		newObj := e.ObjectNew.(*v1.Namespace)
-		log.V(1).Info("receive namespace update event", "name", oldObj.Name)
+		log.V(1).Info("Receive Namespace update event", "Name", oldObj.Name)
 		if reflect.DeepEqual(oldObj.ObjectMeta.Labels, newObj.ObjectMeta.Labels) {
-			log.Info("labels of namespace are not changed", "name", oldObj.Name)
+			log.Info("Labels of Namespace are not changed", "Name", oldObj.Name)
 			return false
 		}
 		return true
@@ -64,17 +64,17 @@ var PredicateFuncsNs = predicate.Funcs{
 	},
 }
 
-func reconcileSubnet(c client.Client, namespace string, q workqueue.RateLimitingInterface) error {
+func requeueSubnet(c client.Client, ns string, q workqueue.RateLimitingInterface) error {
 	subnetList := &v1alpha1.SubnetList{}
-	err := c.List(context.Background(), subnetList, client.InNamespace(namespace))
+	err := c.List(context.Background(), subnetList, client.InNamespace(ns))
 	if err != nil {
-		log.Error(err, "failed to list all the subnets")
+		log.Error(err, "Failed to list all the Subnets")
 		return err
 	}
 
 	for _, subnet_item := range subnetList.Items {
-		log.Info("reconcile subnet because namespace update",
-			"namespace", subnet_item.Namespace, "name", subnet_item.Name)
+		log.Info("Requeue Subnet because Namespace update",
+			"Namespace", subnet_item.Namespace, "Name", subnet_item.Name)
 		q.Add(reconcile.Request{
 			NamespacedName: types.NamespacedName{
 				Name:      subnet_item.Name,

diff --git a/pkg/controllers/subnet/namespace_handler_test.go b/pkg/controllers/subnet/namespace_handler_test.go
@@ -0,0 +1,128 @@
+package subnet
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+	"sigs.k8s.io/controller-runtime/pkg/event"
+
+	"github.com/vmware-tanzu/nsx-operator/pkg/apis/vpc/v1alpha1"
+)
+
+type MockRateLimitingInterface struct {
+	mock.Mock
+}
+
+func (m *MockRateLimitingInterface) Add(item interface{}) {
+}
+
+func (m *MockRateLimitingInterface) Len() int {
+	return 0
+}
+
+func (m *MockRateLimitingInterface) Get() (item interface{}, shutdown bool) {
+	return
+}
+
+func (m *MockRateLimitingInterface) Done(item interface{}) {
+	return
+}
+
+func (m *MockRateLimitingInterface) ShutDown() {
+}
+
+func (m *MockRateLimitingInterface) ShutDownWithDrain() {
+}
+
+func (m *MockRateLimitingInterface) ShuttingDown() bool {
+	return true
+}
+
+func (m *MockRateLimitingInterface) AddAfter(item interface{}, duration time.Duration) {
+	return
+}
+
+func (m *MockRateLimitingInterface) AddRateLimited(item interface{}) {
+	m.Called(item)
+}
+
+func (m *MockRateLimitingInterface) Forget(item interface{}) {
+	m.Called(item)
+}
+
+func (m *MockRateLimitingInterface) NumRequeues(item interface{}) int {
+	args := m.Called(item)
+	return args.Int(0)
+}
+
+func TestEnqueueRequestForNamespace(t *testing.T) {
+	fakeClient := fake.NewClientBuilder().Build()
+	queue := new(MockRateLimitingInterface)
+	handler := &EnqueueRequestForNamespace{Client: fakeClient}
+
+	t.Run("Update event with changed labels", func(t *testing.T) {
+		oldNamespace := &v1.Namespace{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:   "test-ns",
+				Labels: map[string]string{"key": "old"},
+			},
+		}
+		newNamespace := &v1.Namespace{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:   "test-ns",
+				Labels: map[string]string{"key": "new"},
+			},
+		}
+		updateEvent := event.UpdateEvent{
+			ObjectOld: oldNamespace,
+			ObjectNew: newNamespace,
+		}
+		queue.On("Add", mock.Anything).Return()
+
+		handler.Update(context.Background(), updateEvent, queue)
+	})
+
+	t.Run("Update event with unchanged labels", func(t *testing.T) {
+		oldNamespace := &v1.Namespace{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:   "test-ns",
+				Labels: map[string]string{"key": "same"},
+			},
+		}
+		newNamespace := &v1.Namespace{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:   "test-ns",
+				Labels: map[string]string{"key": "same"},
+			},
+		}
+		updateEvent := event.UpdateEvent{
+			ObjectOld: oldNamespace,
+			ObjectNew: newNamespace,
+		}
+
+		queue.On("Add", mock.Anything).Return()
+
+		handler.Update(context.Background(), updateEvent, queue)
+
+		queue.AssertNotCalled(t, "Add", mock.Anything)
+	})
+
+	t.Run("Requeue subnet function", func(t *testing.T) {
+		ns := "test-ns"
+
+		schem := fake.NewClientBuilder().Build().Scheme()
+		v1alpha1.AddToScheme(schem)
+		queue.On("Add", mock.Anything).Return()
+
+		err := requeueSubnet(fakeClient, ns, queue)
+
+		assert.NoError(t, err)
+		queue.AssertNumberOfCalls(t, "Add", 0)
+	})
+}