Merge pull request vmware-tanzu#921 from TaoZou1/subnetrealize

Check more resources realized status when creating subnet
TaoZou1 · Dec 2, 2024 · 19f60cc · 19f60cc
2 parents 7808100 + 5259e2b
commit 19f60cc
Show file tree

Hide file tree

Showing 6 changed files with 117 additions and 7 deletions.
diff --git a/pkg/nsx/services/realizestate/realize_state.go b/pkg/nsx/services/realizestate/realize_state.go
@@ -14,6 +14,11 @@ import (
 	nsxutil "github.com/vmware-tanzu/nsx-operator/pkg/nsx/util"
 )
 
+const (
+	RealizedLogicalPort   = "RealizedLogicalPort"
+	RealizedLogicalRouter = "RealizedLogicalRouter"
+)
+
 type RealizeStateService struct {
 	common.Service
 }
@@ -43,6 +48,7 @@ func IsRealizeStateError(err error) bool {
 
 // CheckRealizeState allows the caller to check realize status of entityType with retries.
 // backoff defines the maximum retries and the wait interval between two retries.
+// if entityType == "", check all the entities, all entities should be in the REALIZED state to be tereated as REALIZED
 func (service *RealizeStateService) CheckRealizeState(backoff wait.Backoff, intentPath, entityType 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 {
@@ -54,12 +60,14 @@ 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 +76,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", *result.EntityType, errMsg))
 			}
 		}
+		if entitiesRealized == len(results.Results) {
+			return nil
+		}
 		return fmt.Errorf("%s not realized", entityType)
 	})
 }
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,
@@ -76,4 +75,102 @@ func TestRealizeStateService_CheckRealizeState(t *testing.T) {
 	realizeStateError, ok = err.(*RealizeStateError)
 	assert.True(t, ok)
 	assert.Equal(t, realizeStateError.Error(), "RealizedLogicalPort 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(), "RealizedLogicalPort 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, realizestate.RealizedLogicalPort); 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
@@ -854,7 +854,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, realizestate.RealizedLogicalRouter); 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)