diff --git a/pkg/controllers/subnet/namespace_handler.go b/pkg/controllers/subnet/namespace_handler.go index 97bf5a0b7..57d667248 100644 --- a/pkg/controllers/subnet/namespace_handler.go +++ b/pkg/controllers/subnet/namespace_handler.go @@ -41,7 +41,7 @@ func (e *EnqueueRequestForNamespace) Update(_ context.Context, updateEvent event obj := updateEvent.ObjectNew.(*v1.Namespace) 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 @@ -68,13 +68,13 @@ func requeueSubnet(c client.Client, ns string, q workqueue.RateLimitingInterface subnetList := &v1alpha1.SubnetList{} 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/subnet_controller.go b/pkg/controllers/subnet/subnet_controller.go index a8e3b6c0e..27034c47b 100644 --- a/pkg/controllers/subnet/subnet_controller.go +++ b/pkg/controllers/subnet/subnet_controller.go @@ -51,7 +51,7 @@ type SubnetReconciler struct { func (r *SubnetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { startTime := time.Now() defer func() { - log.Info("Finished reconciling Subnet", "Subnet", req.NamespacedName, "duration", time.Since(startTime)) + log.Info("Finished reconciling Subnet", "Subnet", req.NamespacedName, "duration(ms)", time.Since(startTime).Milliseconds()) }() metrics.CounterInc(r.SubnetService.NSXConfig, metrics.ControllerSyncTotal, MetricResTypeSubnet) @@ -59,23 +59,23 @@ func (r *SubnetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr if err := r.Client.Get(ctx, req.NamespacedName, subnetCR); err != nil { if apierrors.IsNotFound(err) { if err := r.deleteSubnetByName(req.Name, req.Namespace); err != nil { - log.Error(err, "failed to delete NSX Subnet", "Subnet", req.NamespacedName) + log.Error(err, "Failed to delete NSX Subnet", "Subnet", req.NamespacedName) return ResultRequeue, err } return ResultNormal, nil } - log.Error(err, "unable to fetch Subnet CR", "req", req.NamespacedName) + log.Error(err, "Unable to fetch Subnet CR", "req", req.NamespacedName) return ResultRequeue, err } if !subnetCR.DeletionTimestamp.IsZero() { metrics.CounterInc(r.SubnetService.NSXConfig, metrics.ControllerDeleteTotal, MetricResTypeSubnet) if err := r.deleteSubnetByID(string(subnetCR.GetUID())); err != nil { - log.Error(err, "failed to delete NSX Subnet, retrying", "Subnet", req.NamespacedName) + log.Error(err, "Failed to delete NSX Subnet, retrying", "Subnet", req.NamespacedName) deleteFail(r, ctx, subnetCR, err.Error()) return ResultRequeue, err } if err := r.Client.Delete(ctx, subnetCR); err != nil { - log.Error(err, "failed to delete Subnet CR, retrying", "Subnet", req.NamespacedName) + log.Error(err, "Failed to delete Subnet CR, retrying", "Subnet", req.NamespacedName) deleteFail(r, ctx, subnetCR, err.Error()) return ResultRequeue, err } @@ -97,7 +97,7 @@ func (r *SubnetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr vpcNetworkConfig := r.VPCService.GetVPCNetworkConfigByNamespace(subnetCR.Namespace) if vpcNetworkConfig == nil { err := fmt.Errorf("VPCNetworkConfig not found for Subnet CR") - log.Error(nil, "failed to find VPCNetworkConfig", "Subnet", req.NamespacedName) + log.Error(nil, "Failed to find VPCNetworkConfig", "Subnet", req.NamespacedName) updateFail(r, ctx, subnetCR, err.Error()) return ResultRequeue, err } @@ -106,7 +106,7 @@ func (r *SubnetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr } if specChanged { if err := r.Client.Update(ctx, subnetCR); err != nil { - log.Error(err, "failed to update Subnet", "Subnet", req.NamespacedName) + log.Error(err, "Failed to update Subnet", "Subnet", req.NamespacedName) updateFail(r, ctx, subnetCR, err.Error()) return ResultRequeue, err } @@ -115,7 +115,7 @@ func (r *SubnetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr tags := r.SubnetService.GenerateSubnetNSTags(subnetCR) if tags == nil { - log.Error(nil, "failed to generate Subnet tags", "Subnet", req.NamespacedName) + log.Error(nil, "Failed to generate Subnet tags", "Subnet", req.NamespacedName) return ResultRequeue, errors.New("failed to generate Subnet tags") } // List VPC Info @@ -127,17 +127,17 @@ func (r *SubnetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr // Create or update the subnet in NSX if _, err := r.SubnetService.CreateOrUpdateSubnet(subnetCR, vpcInfoList[0], tags); err != nil { if errors.As(err, &nsxutil.ExceedTagsError{}) { - log.Error(err, "tags limit exceeded, not retrying", "Subnet", req.NamespacedName) + log.Error(err, "Tags limit exceeded, not retrying", "Subnet", req.NamespacedName) updateFail(r, ctx, subnetCR, err.Error()) return ResultNormal, nil } - log.Error(err, "failed to create/update Subnet, retrying", "Subnet", req.NamespacedName) + log.Error(err, "Failed to create/update Subnet, retrying", "Subnet", req.NamespacedName) updateFail(r, ctx, subnetCR, err.Error()) return ResultRequeue, err } // Update status if err := r.updateSubnetStatus(subnetCR); err != nil { - log.Error(err, "failed to update Subnet status, retrying", "Subnet", req.NamespacedName) + log.Error(err, "Failed to update Subnet status, retrying", "Subnet", req.NamespacedName) updateFail(r, ctx, subnetCR, err.Error()) return ResultRequeue, err } @@ -155,11 +155,11 @@ func (r *SubnetReconciler) deleteSubnets(nsxSubnets []*model.VpcSubnet) error { portNums := len(r.SubnetPortService.GetPortsOfSubnet(*nsxSubnet.Id)) if portNums > 0 { err := fmt.Errorf("cannot delete Subnet %s, still attached by %d port(s)", *nsxSubnet.Id, portNums) - log.Error(err, "delete Subnet from NSX failed") + log.Error(err, "Delete Subnet from NSX failed") return err } if err := r.SubnetService.DeleteSubnet(*nsxSubnet); err != nil { - log.Error(err, "failed to delete Subnet", "ID", *nsxSubnet.Id) + log.Error(err, "Failed to delete Subnet", "ID", *nsxSubnet.Id) return err } log.Info("Successfully deleted Subnet", "ID", *nsxSubnet.Id) @@ -254,9 +254,9 @@ func (r *SubnetReconciler) updateSubnetStatusConditions(ctx context.Context, sub } if conditionsUpdated { if err := r.Client.Status().Update(ctx, subnet); err != nil { - log.Error(err, "failed to update Subnet status", "Name", subnet.Name, "Namespace", subnet.Namespace) + log.Error(err, "Failed to update Subnet status", "Name", subnet.Name, "Namespace", subnet.Namespace) } else { - log.Info("updated Subnet", "Name", subnet.Name, "Namespace", subnet.Namespace, "New Conditions", newConditions) + log.Info("Updated Subnet", "Name", subnet.Name, "Namespace", subnet.Namespace, "New Conditions", newConditions) } } } @@ -265,7 +265,7 @@ func (r *SubnetReconciler) mergeSubnetStatusCondition(ctx context.Context, subne matchedCondition := getExistingConditionOfType(newCondition.Type, subnet.Status.Conditions) if reflect.DeepEqual(matchedCondition, newCondition) { - log.V(2).Info("conditions already match", "New Condition", newCondition, "Existing Condition", matchedCondition) + log.V(2).Info("Conditions already match", "New Condition", newCondition, "Existing Condition", matchedCondition) return false } @@ -323,7 +323,7 @@ func StartSubnetController(mgr ctrl.Manager, subnetService *subnet.SubnetService } // Start the controller if err := subnetReconciler.start(mgr); err != nil { - log.Error(err, "failed to create controller", "controller", "Subnet") + log.Error(err, "Failed to create controller", "controller", "Subnet") return err } // Start garbage collector in a separate goroutine @@ -372,12 +372,12 @@ func (r *SubnetReconciler) listSubnetIDsFromCRs(ctx context.Context) ([]string, func (r *SubnetReconciler) collectGarbage(ctx context.Context) { startTime := time.Now() defer func() { - log.Info("Subnet garbage collection completed", "time", time.Since(startTime)) + log.Info("Subnet garbage collection completed", "duration(ms)", time.Since(startTime).Milliseconds()) }() crdSubnetIDs, err := r.listSubnetIDsFromCRs(ctx) if err != nil { - log.Error(err, "failed to list Subnet CRs") + log.Error(err, "Failed to list Subnet CRs") return } @@ -401,7 +401,7 @@ func (r *SubnetReconciler) collectGarbage(ctx context.Context) { metrics.CounterInc(r.SubnetService.NSXConfig, metrics.ControllerDeleteTotal, common.MetricResTypeSubnet) if err := r.SubnetService.DeleteSubnet(*nsxSubnet); err != nil { - log.Error(err, "failed to delete NSX subnet", "NSX Subnet UID", uid) + log.Error(err, "Failed to delete NSX subnet", "NSX Subnet UID", uid) metrics.CounterInc(r.SubnetService.NSXConfig, metrics.ControllerDeleteFailTotal, common.MetricResTypeSubnet) } else { log.Info("Successfully deleted NSX subnet", "NSX Subnet UID", uid) diff --git a/pkg/controllers/subnetset/subnetset_controller.go b/pkg/controllers/subnetset/subnetset_controller.go index 4b7c79505..10e8f51cd 100644 --- a/pkg/controllers/subnetset/subnetset_controller.go +++ b/pkg/controllers/subnetset/subnetset_controller.go @@ -53,7 +53,7 @@ type SubnetSetReconciler struct { func (r *SubnetSetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { startTime := time.Now() defer func() { - log.Info("Finished reconciling SubnetSet", "SubnetSet", req.NamespacedName, "time", time.Since(startTime)) + log.Info("Finished reconciling SubnetSet", "SubnetSet", req.NamespacedName, "duration(ms)", time.Since(startTime).Milliseconds()) }() subnetsetCR := &v1alpha1.SubnetSet{} @@ -258,7 +258,7 @@ func (r *SubnetSetReconciler) setupWithManager(mgr ctrl.Manager) error { func (r *SubnetSetReconciler) CollectGarbage(ctx context.Context) { startTime := time.Now() defer func() { - log.Info("SubnetSet garbage collection completed", "time", time.Since(startTime)) + log.Info("SubnetSet garbage collection completed", "duration(ms)", time.Since(startTime).Milliseconds()) }() subnetSetList := &v1alpha1.SubnetSetList{} err := r.Client.List(ctx, subnetSetList) @@ -379,7 +379,7 @@ func StartSubnetSetController(mgr ctrl.Manager, subnetService *subnet.SubnetServ Recorder: mgr.GetEventRecorderFor("subnetset-controller"), } if err := subnetsetReconciler.Start(mgr, enableWebhook); err != nil { - log.Error(err, "failed to create controller", "controller", "SubnetSet") + log.Error(err, "Failed to create controller", "controller", "SubnetSet") return err } go common.GenericGarbageCollector(make(chan bool), servicecommon.GCInterval, subnetsetReconciler.CollectGarbage) diff --git a/pkg/nsx/services/subnet/subnet.go b/pkg/nsx/services/subnet/subnet.go index acc62c603..e83a4cb12 100644 --- a/pkg/nsx/services/subnet/subnet.go +++ b/pkg/nsx/services/subnet/subnet.go @@ -87,10 +87,10 @@ func (service *SubnetService) CreateOrUpdateSubnet(obj client.Object, vpcInfo co uid := string(obj.GetUID()) nsxSubnet, err := service.buildSubnet(obj, tags) if err != nil { - log.Error(err, "failed to build Subnet") + log.Error(err, "Failed to build Subnet") return "", err } - // Only check whether needs update when obj is v1alpha1.Subnet + // Only check whether it needs update when obj is v1alpha1.Subnet if subnet, ok := obj.(*v1alpha1.Subnet); ok { existingSubnet := service.SubnetStore.GetByKey(service.BuildSubnetID(subnet)) changed := false @@ -106,7 +106,7 @@ func (service *SubnetService) CreateOrUpdateSubnet(obj client.Object, vpcInfo co } } if !changed { - log.Info("subnet not changed, skip updating", "subnet.Id", uid) + log.Info("Subnet not changed, skip updating", "SubnetId", uid) return uid, nil } } @@ -135,6 +135,7 @@ func (service *SubnetService) createOrUpdateSubnet(obj client.Object, nsxSubnet Jitter: 0, Steps: 6, } + // if err = realizeService.CheckRealizeState(backoff, *nsxSubnet.Path, "RealizedLogicalSwitch"); err != nil { log.Error(err, "failed to check subnet realization state", "ID", *nsxSubnet.Id) return "", err diff --git a/test/e2e/nsx_subnet_test.go b/test/e2e/nsx_subnet_test.go index a54d5281a..cf15c438f 100644 --- a/test/e2e/nsx_subnet_test.go +++ b/test/e2e/nsx_subnet_test.go @@ -2,7 +2,6 @@ package e2e import ( "context" - "fmt" "log" "net" "path/filepath" @@ -89,7 +88,7 @@ func transSearchResponsetoSubnet(response model.SearchResponse) []model.VpcSubne return resources } -func fetchSubnet(t *testing.T, subnetSet *v1alpha1.SubnetSet) model.VpcSubnet { +func fetchSubnetBySubnetSet(t *testing.T, subnetSet *v1alpha1.SubnetSet) model.VpcSubnet { tags := []string{common.TagScopeSubnetSetCRUID, string(subnetSet.UID)} results, err := testData.queryResource(common.ResourceTypeSubnet, tags) assertNil(t, err) @@ -133,7 +132,7 @@ func defaultSubnetSet(t *testing.T) { time.Sleep(5 * time.Second) assertNil(t, err) - vpcSubnet := fetchSubnet(t, subnetSet) + vpcSubnet := fetchSubnetBySubnetSet(t, subnetSet) found := false for _, tag := range vpcSubnet.Tags { if *tag.Scope == labelKey && *tag.Tag == labelValue { @@ -151,7 +150,7 @@ func defaultSubnetSet(t *testing.T) { _, err = testData.clientset.CoreV1().Namespaces().Update(context.TODO(), ns, v1.UpdateOptions{}) time.Sleep(5 * time.Second) assertNil(t, err) - vpcSubnet = fetchSubnet(t, subnetSet) + vpcSubnet = fetchSubnetBySubnetSet(t, subnetSet) found = false for _, tag := range vpcSubnet.Tags { if *tag.Scope == labelKey && *tag.Tag == labelValue { @@ -169,7 +168,7 @@ func defaultSubnetSet(t *testing.T) { time.Sleep(5 * time.Second) assertNil(t, err) t.Logf("new Namespace: %+v", newNs) - vpcSubnet = fetchSubnet(t, subnetSet) + vpcSubnet = fetchSubnetBySubnetSet(t, subnetSet) found = false for _, tag := range vpcSubnet.Tags { if *tag.Scope == labelKey { @@ -295,6 +294,9 @@ func SubnetCIDR(t *testing.T) { assertNil(t, err) allocatedSubnet, err := testData.crdClientset.CrdV1alpha1().Subnets(E2ENamespace).Get(context.TODO(), subnet.Name, v1.GetOptions{}) assertNil(t, err) + nsxSubnets := testData.fetchSubnetByNamespace(t, E2ENamespace, false) + assert.Equal(t, 1, len(nsxSubnets)) + targetCIDR := allocatedSubnet.Status.NetworkAddresses[0] err = testData.crdClientset.CrdV1alpha1().Subnets(E2ENamespace).Delete(context.TODO(), subnet.Name, v1.DeleteOptions{}) assertNil(t, err) @@ -307,17 +309,13 @@ func SubnetCIDR(t *testing.T) { return false, err }) assertNil(t, err) - - nsxSubnets, err := testData.getSubnet(E2ENamespace, true) - for i, nsxSubnet := range nsxSubnets { - t.Logf("Subnet in NSX: %d, id: %s, PATH: %s, DisplayName: %s, MarkedForDelete: %t, Tags: %v", i, *nsxSubnet.Id, *nsxSubnet.Path, *nsxSubnet.DisplayName, *nsxSubnet.MarkedForDelete, nsxSubnet.Tags) - } - t.Logf("Check NSX Subnet num: %d, error: %+v", len(nsxSubnets), err) + nsxSubnets = testData.fetchSubnetByNamespace(t, E2ENamespace, true) + assert.Equal(t, true, len(nsxSubnets) <= 1) subnet.Spec.IPAddresses = []string{targetCIDR} _, err = testData.crdClientset.CrdV1alpha1().Subnets(E2ENamespace).Create(context.TODO(), subnet, v1.CreateOptions{}) if err != nil && errors.IsAlreadyExists(err) { - t.Logf("Create Subnet: %+v", err) + t.Logf("Create Subnet error: %+v", err) err = nil } assertNil(t, err) @@ -327,6 +325,9 @@ func SubnetCIDR(t *testing.T) { assertNil(t, err) assert.Equal(t, targetCIDR, allocatedSubnet.Status.NetworkAddresses[0]) + nsxSubnets = testData.fetchSubnetByNamespace(t, E2ENamespace, false) + assert.Equal(t, 1, len(nsxSubnets)) + err = testData.crdClientset.CrdV1alpha1().Subnets(E2ENamespace).Delete(context.TODO(), subnet.Name, v1.DeleteOptions{}) assertNil(t, err) @@ -339,38 +340,19 @@ func SubnetCIDR(t *testing.T) { }) assertNil(t, err) - nsxSubnets, err = testData.getSubnet(E2ENamespace, true) - for i, nsxSubnet := range nsxSubnets { - t.Logf("Subnet in NSX: %d, id: %s, PATH: %s, DisplayName: %s, MarkedForDelete: %t, Tags: %v", i, *nsxSubnet.Id, *nsxSubnet.Path, *nsxSubnet.DisplayName, *nsxSubnet.MarkedForDelete, nsxSubnet.Tags) - } - t.Logf("Check NSX Subnet num: %d, error: %+v", len(nsxSubnets), err) - - nsxSubnets, err = testData.getSubnet(E2ENamespace, false) - for i, nsxSubnet := range nsxSubnets { - t.Logf("Subnet in NSX: %d, id: %s, PATH: %s, DisplayName: %s, MarkedForDelete: %t, Tags: %v", i, *nsxSubnet.Id, *nsxSubnet.Path, *nsxSubnet.DisplayName, *nsxSubnet.MarkedForDelete, nsxSubnet.Tags) - } - t.Logf("Check NSX Subnet num(not include MarkForDelete): %d, error: %+v", len(nsxSubnets), err) + nsxSubnets = testData.fetchSubnetByNamespace(t, E2ENamespace, true) + assert.Equal(t, true, len(nsxSubnets) <= 1) } -func (data *TestData) getSubnet(ns string, includeMarkForDelete bool) ([]model.VpcSubnet, error) { - ctx := context.Background() - networkinfo, err := data.crdClientset.CrdV1alpha1().NetworkInfos(ns).Get(ctx, ns, v1.GetOptions{}) - if err != nil { - return nil, err - } - - vpcInfo := "" - for _, vpc := range networkinfo.VPCs { - if vpc.VPCPath != "" { - vpcInfo = vpc.VPCPath +func (data *TestData) fetchSubnetByNamespace(t *testing.T, ns string, isMarkForDelete bool) (res []model.VpcSubnet) { + tags := []string{common.TagScopeNamespace, ns} + results, err := testData.queryResource(common.ResourceTypeSubnet, tags) + assertNil(t, err) + subnets := transSearchResponsetoSubnet(results) + for _, subnet := range subnets { + if *subnet.MarkedForDelete == isMarkForDelete { + res = append(res, subnet) } } - if vpcInfo == "" { - return nil, fmt.Errorf("VPCPath not found") - } - - parts := strings.Split(vpcInfo, "/") - orgID, projectID, vpcID := parts[2], parts[4], parts[6] - nsxSubnets, err := testData.nsxClient.SubnetsClient.List(orgID, projectID, vpcID, nil, &includeMarkForDelete, nil, nil, nil, nil) - return nsxSubnets.Results, err + return }