Merge pull request vmware-tanzu#940 from TaoZou1/v42subnetrealize

Check all resources realized status when creating subnet
TaoZou1 · Dec 12, 2024 · 017a469 · 017a469
2 parents 17e39f9 + 7a03bc2
commit 017a469
Show file tree

Hide file tree

Showing 6 changed files with 122 additions and 20 deletions.
diff --git a/pkg/nsx/services/realizestate/realize_state.go b/pkg/nsx/services/realizestate/realize_state.go
@@ -41,9 +41,10 @@ func IsRealizeStateError(err error) bool {
 	return ok
 }
 
-// CheckRealizeState allows the caller to check realize status of entityType with retries.
-// backoff defines the maximum retries and the wait interval between two retries.
-func (service *RealizeStateService) CheckRealizeState(backoff wait.Backoff, intentPath, entityType string) error {
+// CheckRealizeState allows the caller to check realize status of intentPath with retries.
+// Backoff defines the maximum retries and the wait interval between two retries.
+// Check all the entities, all entities should be in the REALIZED state to be treated as REALIZED
+func (service *RealizeStateService) CheckRealizeState(backoff wait.Backoff, intentPath string) error {
 	// TODO， ask NSX if there were multiple realize states could we check only the latest one?
 	return retry.OnError(backoff, func(err error) bool {
 		// Won't retry when realized state is `ERROR`.
@@ -54,12 +55,11 @@ func (service *RealizeStateService) CheckRealizeState(backoff wait.Backoff, inte
 		if err != nil {
 			return err
 		}
+		entitiesRealized := 0
 		for _, result := range results.Results {
-			if entityType != "" && result.EntityType != nil && *result.EntityType != entityType {
-				continue
-			}
 			if *result.State == model.GenericPolicyRealizedResource_STATE_REALIZED {
-				return nil
+				entitiesRealized++
+				continue
 			}
 			if *result.State == model.GenericPolicyRealizedResource_STATE_ERROR {
 				var errMsg []string
@@ -68,9 +68,12 @@ func (service *RealizeStateService) CheckRealizeState(backoff wait.Backoff, inte
 						errMsg = append(errMsg, *alarm.Message)
 					}
 				}
-				return NewRealizeStateError(fmt.Sprintf("%s realized with errors: %s", entityType, errMsg))
+				return NewRealizeStateError(fmt.Sprintf("%s realized with errors: %s", intentPath, errMsg))
 			}
 		}
-		return fmt.Errorf("%s not realized", entityType)
+		if entitiesRealized == len(results.Results) {
+			return nil
+		}
+		return fmt.Errorf("%s not realized", intentPath)
 	})
 }
diff --git a/pkg/nsx/services/realizestate/realize_state_test.go b/pkg/nsx/services/realizestate/realize_state_test.go
@@ -55,7 +55,6 @@ func TestRealizeStateService_CheckRealizeState(t *testing.T) {
 			},
 		}, nil
 	})
-	defer patches.Reset()
 
 	backoff := wait.Backoff{
 		Duration: 1 * time.Second,
@@ -64,16 +63,114 @@ func TestRealizeStateService_CheckRealizeState(t *testing.T) {
 		Steps:    6,
 	}
 	// default project
-	err := s.CheckRealizeState(backoff, "/orgs/default/projects/default/vpcs/vpc/subnets/subnet/ports/port", "RealizedLogicalPort")
+	err := s.CheckRealizeState(backoff, "/orgs/default/projects/default/vpcs/vpc/subnets/subnet/ports/port")
 
 	realizeStateError, ok := err.(*RealizeStateError)
 	assert.True(t, ok)
-	assert.Equal(t, realizeStateError.Error(), "RealizedLogicalPort realized with errors: [mocked error]")
+	assert.Equal(t, realizeStateError.Error(), "/orgs/default/projects/default/vpcs/vpc/subnets/subnet/ports/port realized with errors: [mocked error]")
 
 	// non default project
-	err = s.CheckRealizeState(backoff, "/orgs/default/projects/project-quality/vpcs/vpc/subnets/subnet/ports/port", "RealizedLogicalPort")
+	err = s.CheckRealizeState(backoff, "/orgs/default/projects/project-quality/vpcs/vpc/subnets/subnet/ports/port")
 
 	realizeStateError, ok = err.(*RealizeStateError)
 	assert.True(t, ok)
-	assert.Equal(t, realizeStateError.Error(), "RealizedLogicalPort realized with errors: [mocked error]")
+	assert.Equal(t, realizeStateError.Error(), "/orgs/default/projects/project-quality/vpcs/vpc/subnets/subnet/ports/port realized with errors: [mocked error]")
+
+	// for subnet, RealizedLogicalPort realized with errors
+	patches.Reset()
+
+	patches = gomonkey.ApplyFunc((*fakeRealizedEntitiesClient).List, func(c *fakeRealizedEntitiesClient, intentPathParam string, sitePathParam *string) (model.GenericPolicyRealizedResourceListResult, error) {
+		return model.GenericPolicyRealizedResourceListResult{
+			Results: []model.GenericPolicyRealizedResource{
+				{
+					State: common.String(model.GenericPolicyRealizedResource_STATE_ERROR),
+					Alarms: []model.PolicyAlarmResource{
+						{Message: common.String("mocked error")},
+					},
+					EntityType: common.String("RealizedLogicalPort"),
+				},
+				{
+					State: common.String(model.GenericPolicyRealizedResource_STATE_UNREALIZED),
+					Alarms: []model.PolicyAlarmResource{
+						{Message: common.String("mocked error")},
+					},
+					EntityType: common.String("RealizedLogicalSwitch"),
+				},
+				{
+					State:      common.String(model.GenericPolicyRealizedResource_STATE_REALIZED),
+					Alarms:     []model.PolicyAlarmResource{},
+					EntityType: common.String("RealizedLogicalRouterPort"),
+				},
+			},
+		}, nil
+	})
+	err = s.CheckRealizeState(backoff, "/orgs/default/projects/project-quality/vpcs/vpc/subnets/subnet/")
+
+	realizeStateError, ok = err.(*RealizeStateError)
+	assert.True(t, ok)
+	assert.Equal(t, realizeStateError.Error(), "/orgs/default/projects/project-quality/vpcs/vpc/subnets/subnet/ realized with errors: [mocked error]")
+
+	// for subnet, realized successfully
+	patches.Reset()
+
+	patches = gomonkey.ApplyFunc((*fakeRealizedEntitiesClient).List, func(c *fakeRealizedEntitiesClient, intentPathParam string, sitePathParam *string) (model.GenericPolicyRealizedResourceListResult, error) {
+		return model.GenericPolicyRealizedResourceListResult{
+			Results: []model.GenericPolicyRealizedResource{
+				{
+					State:      common.String(model.GenericPolicyRealizedResource_STATE_REALIZED),
+					Alarms:     []model.PolicyAlarmResource{},
+					EntityType: common.String("RealizedLogicalPort"),
+				},
+				{
+					State:      common.String(model.GenericPolicyRealizedResource_STATE_REALIZED),
+					Alarms:     []model.PolicyAlarmResource{},
+					EntityType: common.String("RealizedLogicalSwitch"),
+				},
+				{
+					State:      common.String(model.GenericPolicyRealizedResource_STATE_REALIZED),
+					Alarms:     []model.PolicyAlarmResource{},
+					EntityType: common.String("RealizedLogicalRouterPort"),
+				},
+			},
+		}, nil
+	})
+	err = s.CheckRealizeState(backoff, "/orgs/default/projects/project-quality/vpcs/vpc/subnets/subnet/")
+	assert.Equal(t, err, nil)
+
+	// for subnet, need retry
+	patches.Reset()
+
+	patches = gomonkey.ApplyFunc((*fakeRealizedEntitiesClient).List, func(c *fakeRealizedEntitiesClient, intentPathParam string, sitePathParam *string) (model.GenericPolicyRealizedResourceListResult, error) {
+		return model.GenericPolicyRealizedResourceListResult{
+			Results: []model.GenericPolicyRealizedResource{
+				{
+					State:      common.String(model.GenericPolicyRealizedResource_STATE_REALIZED),
+					Alarms:     []model.PolicyAlarmResource{},
+					EntityType: common.String("RealizedLogicalPort"),
+				},
+				{
+					State:      common.String(model.GenericPolicyRealizedResource_STATE_UNREALIZED),
+					Alarms:     []model.PolicyAlarmResource{},
+					EntityType: common.String("RealizedLogicalSwitch"),
+				},
+				{
+					State:      common.String(model.GenericPolicyRealizedResource_STATE_REALIZED),
+					Alarms:     []model.PolicyAlarmResource{},
+					EntityType: common.String("RealizedLogicalRouterPort"),
+				},
+			},
+		}, nil
+	})
+	backoff = wait.Backoff{
+		Duration: 10 * time.Millisecond,
+		Factor:   1,
+		Jitter:   0,
+		Steps:    1,
+	}
+	err = s.CheckRealizeState(backoff, "/orgs/default/projects/project-quality/vpcs/vpc/subnets/subnet/")
+	assert.NotEqual(t, err, nil)
+	_, ok = err.(*RealizeStateError)
+	assert.Equal(t, ok, false)
+	patches.Reset()
+
 }
diff --git a/pkg/nsx/services/subnet/subnet.go b/pkg/nsx/services/subnet/subnet.go
@@ -155,7 +155,8 @@ func (service *SubnetService) createOrUpdateSubnet(obj client.Object, nsxSubnet
 	// Failure of CheckRealizeState may result in the creation of an existing Subnet.
 	// For Subnets, it's important to reuse the already created NSXSubnet.
 	// For SubnetSets, since the ID includes a random value, the created NSX Subnet needs to be deleted and recreated.
-	if err = realizeService.CheckRealizeState(backoff, *nsxSubnet.Path, "RealizedLogicalSwitch"); err != nil {
+
+	if err = realizeService.CheckRealizeState(backoff, *nsxSubnet.Path); err != nil {
 		log.Error(err, "Failed to check subnet realization state", "ID", *nsxSubnet.Id)
 		// Delete the subnet if realization check fails, avoiding creating duplicate subnets continuously.
 		deleteErr := service.DeleteSubnet(*nsxSubnet)

diff --git a/pkg/nsx/services/subnet/subnet_test.go b/pkg/nsx/services/subnet/subnet_test.go
@@ -144,7 +144,8 @@ func (f fakeRealizedEntitiesClient) List(intentPathParam string, sitePathParam *
 	return model.GenericPolicyRealizedResourceListResult{
 		Results: []model.GenericPolicyRealizedResource{
 			{
-				State: &state,
+				State:      &state,
+				EntityType: common.String("RealizedLogicalPort"),
 			},
 		},
 	}, nil

diff --git a/pkg/nsx/services/subnetport/subnetport.go b/pkg/nsx/services/subnetport/subnetport.go
@@ -176,7 +176,7 @@ func (service *SubnetPortService) CheckSubnetPortState(obj interface{}, nsxSubne
 		Jitter:   0,
 		Steps:    6,
 	}
-	if err := realizeService.CheckRealizeState(backoff, *nsxSubnetPort.Path, "RealizedLogicalPort"); err != nil {
+	if err := realizeService.CheckRealizeState(backoff, *nsxSubnetPort.Path); err != nil {
 		log.Error(err, "failed to get realized status", "subnetport path", *nsxSubnetPort.Path)
 		if realizestate.IsRealizeStateError(err) {
 			log.Error(err, "the created subnet port is in error realization state, cleaning the resource", "subnetport", portID)

diff --git a/pkg/nsx/services/vpc/vpc.go b/pkg/nsx/services/vpc/vpc.go
@@ -856,7 +856,7 @@ func (s *VPCService) createNSXVPC(createdVpc *model.Vpc, nc *common.VPCNetworkCo
 func (s *VPCService) checkVPCRealizationState(createdVpc *model.Vpc, newVpcPath string) error {
 	log.V(2).Info("Check VPC realization state", "VPC", *createdVpc.Id)
 	realizeService := realizestate.InitializeRealizeState(s.Service)
-	if err := realizeService.CheckRealizeState(util.NSXTDefaultRetry, newVpcPath, "RealizedLogicalRouter"); err != nil {
+	if err := realizeService.CheckRealizeState(util.NSXTDefaultRetry, newVpcPath); err != nil {
 		log.Error(err, "Failed to check VPC realization state", "VPC", *createdVpc.Id)
 		if realizestate.IsRealizeStateError(err) {
 			log.Error(err, "The created VPC is in error realization state, cleaning the resource", "VPC", *createdVpc.Id)
@@ -884,7 +884,7 @@ func (s *VPCService) checkLBSRealization(createdLBS *model.LBService, createdVpc
 
 	log.V(2).Info("Check LBS realization state", "LBS", *createdLBS.Id)
 	realizeService := realizestate.InitializeRealizeState(s.Service)
-	if err = realizeService.CheckRealizeState(util.NSXTLBVSDefaultRetry, *newLBS.Path, ""); err != nil {
+	if err = realizeService.CheckRealizeState(util.NSXTLBVSDefaultRetry, *newLBS.Path); err != nil {
 		log.Error(err, "Failed to check LBS realization state", "LBS", *createdLBS.Id)
 		if realizestate.IsRealizeStateError(err) {
 			log.Error(err, "The created LBS is in error realization state, cleaning the resource", "LBS", *createdLBS.Id)
@@ -910,7 +910,7 @@ func (s *VPCService) checkVpcAttachmentRealization(createdAttachment *model.VpcA
 	}
 	log.V(2).Info("Check VPC attachment realization state", "VpcAttachment", *createdAttachment.Id)
 	realizeService := realizestate.InitializeRealizeState(s.Service)
-	if err = realizeService.CheckRealizeState(util.NSXTLBVSDefaultRetry, *newAttachment.Path, ""); err != nil {
+	if err = realizeService.CheckRealizeState(util.NSXTLBVSDefaultRetry, *newAttachment.Path); err != nil {
 		log.Error(err, "Failed to check VPC attachment realization state", "VpcAttachment", *createdAttachment.Id)
 		if realizestate.IsRealizeStateError(err) {
 			log.Error(err, "The created VPC attachment is in error realization state, cleaning the resource", "VpcAttachment", *createdAttachment.Id)