Explicitly check 5 GPRRs for vpc RealizedState

Signed-off-by: Yanjun Zhou <[email protected]>

pkg/nsx/services/common/types.go

@@ -120,8 +120,6 @@ const (
 	DstGroupSuffix         = "dst"
 	IpSetGroupSuffix       = "ipset"
 	ShareSuffix            = "share"
-
-	GatewayInterfaceId = "gateway-interface"
 )
 
 var (

pkg/nsx/services/realizestate/realize_state.go

@@ -32,7 +32,7 @@ func InitializeRealizeState(service common.Service) *RealizeStateService {
 // 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, extraIds []string) error {
+func (service *RealizeStateService) CheckRealizeState(backoff wait.Backoff, intentPath string, GPRRTypeList []nsxutil.GPRRType) 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`.
@@ -44,12 +44,12 @@ func (service *RealizeStateService) CheckRealizeState(backoff wait.Backoff, inte
 			return err
 		}
 		entitiesRealized := 0
-		extraIdsRealized := 0
+		gprrRealized := 0
 		for _, result := range results.Results {
 			if *result.State == model.GenericPolicyRealizedResource_STATE_REALIZED {
-				for _, id := range extraIds {
-					if *result.Id == id {
-						extraIdsRealized++
+				for _, gprr := range GPRRTypeList {
+					if *result.Id == gprr.Id && *result.EntityType == gprr.EntityType {
+						gprrRealized++
 					}
 				}
 				entitiesRealized++
@@ -69,8 +69,8 @@ func (service *RealizeStateService) CheckRealizeState(backoff wait.Backoff, inte
 				return nsxutil.NewRealizeStateError(fmt.Sprintf("%s realized with errors: %s", intentPath, errMsg))
 			}
 		}
-		// extraIdsRealized can be greater than extraIds length as id is not unique in result list.
-		if len(results.Results) != 0 && entitiesRealized == len(results.Results) && extraIdsRealized >= len(extraIds) {
+		// gprrRealized can be greater than GPRRTypeList length as (id, entity_type) is not unique in result list.
+		if len(results.Results) != 0 && entitiesRealized == len(results.Results) && gprrRealized >= len(GPRRTypeList) {
 			return nil
 		}
 		return fmt.Errorf("%s not realized", intentPath)

pkg/nsx/services/realizestate/realize_state_test.go

@@ -64,14 +64,14 @@ func TestRealizeStateService_CheckRealizeState(t *testing.T) {
 		Steps:    6,
 	}
 	// default project
-	err := s.CheckRealizeState(backoff, "/orgs/default/projects/default/vpcs/vpc/subnets/subnet/ports/port", []string{})
+	err := s.CheckRealizeState(backoff, "/orgs/default/projects/default/vpcs/vpc/subnets/subnet/ports/port", []nsxutil.GPRRType{})
 
 	realizeStateError, ok := err.(*nsxutil.RealizeStateError)
 	assert.True(t, ok)
 	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", []string{})
+	err = s.CheckRealizeState(backoff, "/orgs/default/projects/project-quality/vpcs/vpc/subnets/subnet/ports/port", []nsxutil.GPRRType{})
 
 	realizeStateError, ok = err.(*nsxutil.RealizeStateError)
 	assert.True(t, ok)
@@ -86,7 +86,7 @@ func TestRealizeStateService_CheckRealizeState(t *testing.T) {
 					State:      common.String(model.GenericPolicyRealizedResource_STATE_REALIZED),
 					Alarms:     []model.PolicyAlarmResource{},
 					EntityType: common.String("RealizedLogicalRouterPort"),
-					Id:         common.String(common.GatewayInterfaceId),
+					Id:         common.String("gateway-interface"),
 				},
 				{
 					State:      common.String(model.GenericPolicyRealizedResource_STATE_REALIZED),
@@ -97,7 +97,7 @@ func TestRealizeStateService_CheckRealizeState(t *testing.T) {
 			},
 		}, nil
 	})
-	err = s.CheckRealizeState(backoff, "/orgs/default/projects/project-quality/vpcs/vpc", []string{common.GatewayInterfaceId})
+	err = s.CheckRealizeState(backoff, "/orgs/default/projects/project-quality/vpcs/vpc", []nsxutil.GPRRType{{Id: "gateway-interface", EntityType: "RealizedLogicalRouterPort"}})
 	assert.Equal(t, err, nil)
 
 	// for lbs, realized with ProviderNotReady and need retry
@@ -128,7 +128,7 @@ func TestRealizeStateService_CheckRealizeState(t *testing.T) {
 		Jitter:   0,
 		Steps:    1,
 	}
-	err = s.CheckRealizeState(backoff, "/orgs/default/projects/default/vpcs/vpc/vpc-lbs/default", []string{})
+	err = s.CheckRealizeState(backoff, "/orgs/default/projects/default/vpcs/vpc/vpc-lbs/default", []nsxutil.GPRRType{})
 	assert.NotEqual(t, err, nil)
 	_, ok = err.(*nsxutil.RetryRealizeError)
 	assert.Equal(t, ok, true)
@@ -161,7 +161,7 @@ func TestRealizeStateService_CheckRealizeState(t *testing.T) {
 			},
 		}, nil
 	})
-	err = s.CheckRealizeState(backoff, "/orgs/default/projects/project-quality/vpcs/vpc/subnets/subnet/", []string{})
+	err = s.CheckRealizeState(backoff, "/orgs/default/projects/project-quality/vpcs/vpc/subnets/subnet/", []nsxutil.GPRRType{})
 
 	realizeStateError, ok = err.(*nsxutil.RealizeStateError)
 	assert.True(t, ok)
@@ -191,7 +191,7 @@ func TestRealizeStateService_CheckRealizeState(t *testing.T) {
 			},
 		}, nil
 	})
-	err = s.CheckRealizeState(backoff, "/orgs/default/projects/project-quality/vpcs/vpc/subnets/subnet/", []string{})
+	err = s.CheckRealizeState(backoff, "/orgs/default/projects/project-quality/vpcs/vpc/subnets/subnet/", []nsxutil.GPRRType{})
 	assert.Equal(t, err, nil)
 
 	// for subnet, need retry
@@ -224,7 +224,7 @@ func TestRealizeStateService_CheckRealizeState(t *testing.T) {
 		Jitter:   0,
 		Steps:    1,
 	}
-	err = s.CheckRealizeState(backoff, "/orgs/default/projects/project-quality/vpcs/vpc/subnets/subnet/", []string{})
+	err = s.CheckRealizeState(backoff, "/orgs/default/projects/project-quality/vpcs/vpc/subnets/subnet/", []nsxutil.GPRRType{})
 	assert.NotEqual(t, err, nil)
 	_, ok = err.(*nsxutil.RealizeStateError)
 	assert.Equal(t, ok, false)

pkg/nsx/services/subnet/subnet.go

@@ -135,7 +135,7 @@ func (service *SubnetService) createOrUpdateSubnet(obj client.Object, nsxSubnet
 	// 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(util.NSXTRealizeRetry, *nsxSubnet.Path, []string{}); err != nil {
+	if err = realizeService.CheckRealizeState(util.NSXTRealizeRetry, *nsxSubnet.Path, []nsxutil.GPRRType{}); 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)

pkg/nsx/services/subnet/subnet_test.go

@@ -481,7 +481,7 @@ func TestSubnetService_createOrUpdateSubnet(t *testing.T) {
 			name: "Update Subnet with RealizedState and deletion error",
 			prepareFunc: func() *gomonkey.Patches {
 				patches := gomonkey.ApplyFunc((*realizestate.RealizeStateService).CheckRealizeState,
-					func(_ *realizestate.RealizeStateService, _ wait.Backoff, _ string, _ []string) error {
+					func(_ *realizestate.RealizeStateService, _ wait.Backoff, _ string, _ []nsxutil.GPRRType) error {
 						return nsxutil.NewRealizeStateError("mocked realized error")
 					})
 				patches.ApplyFunc((*SubnetService).DeleteSubnet, func(_ *SubnetService, _ model.VpcSubnet) error {
@@ -499,7 +499,7 @@ func TestSubnetService_createOrUpdateSubnet(t *testing.T) {
 			name: "Create Subnet for SubnetSet Success",
 			prepareFunc: func() *gomonkey.Patches {
 				patches := gomonkey.ApplyFunc((*realizestate.RealizeStateService).CheckRealizeState,
-					func(_ *realizestate.RealizeStateService, _ wait.Backoff, _ string, _ []string) error {
+					func(_ *realizestate.RealizeStateService, _ wait.Backoff, _ string, _ []nsxutil.GPRRType) error {
 						return nil
 					})
 				patches.ApplyFunc(fakeSubnetsClient.Get,

pkg/nsx/services/subnetport/subnetport.go

@@ -168,7 +168,7 @@ func (service *SubnetPortService) CheckSubnetPortState(obj interface{}, nsxSubne
 	}
 	realizeService := realizestate.InitializeRealizeState(service.Service)
 
-	if err := realizeService.CheckRealizeState(util.NSXTRealizeRetry, *nsxSubnetPort.Path, []string{}); err != nil {
+	if err := realizeService.CheckRealizeState(util.NSXTRealizeRetry, *nsxSubnetPort.Path, []nsxutil.GPRRType{}); err != nil {
 		log.Error(err, "failed to get realized status", "subnetport path", *nsxSubnetPort.Path)
 		if nsxutil.IsRealizeStateError(err) {
 			log.Error(err, "the created subnet port is in error realization state, cleaning the resource", "subnetport", portID)

pkg/nsx/services/vpc/vpc.go

@@ -43,6 +43,31 @@ var (
 	EnforceRevisionCheckParam = false
 )
 
+func GetGPRRTypeList(id string) []nsxutil.GPRRType {
+	return []nsxutil.GPRRType{
+		{
+			Id:         fmt.Sprintf("%s-folder", id),
+			EntityType: "RealizedVpcFolder",
+		},
+		{
+			Id:         "gateway-interface",
+			EntityType: "RealizedLogicalRouterPort",
+		},
+		{
+			Id:         id,
+			EntityType: "RealizedLogicalRouter",
+		},
+		{
+			Id:         id,
+			EntityType: "RealizedLogicalRouterPort",
+		},
+		{
+			Id:         "security-config",
+			EntityType: "RealizedSecurityFeatureToggle",
+		},
+	}
+}
+
 type VPCNetworkInfoStore struct {
 	sync.RWMutex
 	VPCNetworkConfigMap map[string]common.VPCNetworkConfigInfo
@@ -856,7 +881,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.NSXTRealizeRetry, newVpcPath, []string{common.GatewayInterfaceId}); err != nil {
+	if err := realizeService.CheckRealizeState(util.NSXTRealizeRetry, newVpcPath, GetGPRRTypeList(*createdVpc.Id)); err != nil {
 		log.Error(err, "Failed to check VPC realization state", "VPC", *createdVpc.Id)
 		if nsxutil.IsRealizeStateError(err) {
 			log.Error(err, "The created VPC is in error realization state, cleaning the resource", "VPC", *createdVpc.Id)
@@ -884,7 +909,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.NSXTRealizeRetry, *newLBS.Path, []string{}); err != nil {
+	if err = realizeService.CheckRealizeState(util.NSXTRealizeRetry, *newLBS.Path, []nsxutil.GPRRType{}); err != nil {
 		log.Error(err, "Failed to check LBS realization state", "LBS", *createdLBS.Id)
 		if nsxutil.IsRealizeStateError(err) {
 			log.Error(err, "The created LBS is in error realization state, cleaning the resource", "LBS", *createdLBS.Id)
@@ -910,7 +935,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.NSXTRealizeRetry, *newAttachment.Path, []string{}); err != nil {
+	if err = realizeService.CheckRealizeState(util.NSXTRealizeRetry, *newAttachment.Path, []nsxutil.GPRRType{}); err != nil {
 		log.Error(err, "Failed to check VPC attachment realization state", "VpcAttachment", *createdAttachment.Id)
 		if nsxutil.IsRealizeStateError(err) {
 			log.Error(err, "The created VPC attachment is in error realization state, cleaning the resource", "VpcAttachment", *createdAttachment.Id)

pkg/nsx/services/vpc/vpc_test.go

@@ -13,6 +13,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
+	"k8s.io/apimachinery/pkg/util/wait"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
@@ -35,6 +36,7 @@ import (
 	"github.com/vmware-tanzu/nsx-operator/pkg/nsx"
 	"github.com/vmware-tanzu/nsx-operator/pkg/nsx/ratelimiter"
 	"github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/common"
+	"github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/realizestate"
 	nsxUtil "github.com/vmware-tanzu/nsx-operator/pkg/nsx/util"
 )
 
@@ -2133,3 +2135,99 @@ func TestInitializeVPC(t *testing.T) {
 		assert.Equal(t, tc.expectAllVPCNum, len(allVPCs))
 	}
 }
+
+func TestCheckVPCRealizationState(t *testing.T) {
+	service := &VPCService{
+		Service: common.Service{
+			NSXClient: &nsx.Client{
+				RealizedEntitiesClient: &fakeRealizedEntitiesClient{},
+			},
+		},
+	}
+
+	testCases := []struct {
+		name        string
+		prepareFunc func() *gomonkey.Patches
+		expectedErr string
+	}{
+		{
+			name: "Successful realization",
+			prepareFunc: func() *gomonkey.Patches {
+				patches := gomonkey.ApplyFunc(fakeRealizedEntitiesClient.List,
+					func(_ fakeRealizedEntitiesClient, intentPathParam string, sitePathParam *string) (model.GenericPolicyRealizedResourceListResult, error) {
+						return model.GenericPolicyRealizedResourceListResult{
+							Results: []model.GenericPolicyRealizedResource{
+								{
+									Id:         common.String("vpc-1-folder"),
+									EntityType: common.String("RealizedVpcFolder"),
+									State:      common.String(model.GenericPolicyRealizedResource_STATE_REALIZED),
+								},
+								{
+									Id:         common.String("gateway-interface"),
+									EntityType: common.String("RealizedLogicalRouterPort"),
+									State:      common.String(model.GenericPolicyRealizedResource_STATE_REALIZED),
+								},
+								{
+									Id:         common.String("vpc-1"),
+									EntityType: common.String("RealizedLogicalRouter"),
+									State:      common.String(model.GenericPolicyRealizedResource_STATE_REALIZED),
+								},
+								{
+									Id:         common.String("vpc-1"),
+									EntityType: common.String("RealizedLogicalRouterPort"),
+									State:      common.String(model.GenericPolicyRealizedResource_STATE_REALIZED),
+								},
+								{
+									Id:         common.String("security-config"),
+									EntityType: common.String("RealizedSecurityFeatureToggle"),
+									State:      common.String(model.GenericPolicyRealizedResource_STATE_REALIZED),
+								},
+							},
+						}, nil
+					})
+				return patches
+			},
+		},
+		{
+			name: "Failed realization",
+			prepareFunc: func() *gomonkey.Patches {
+				patches := gomonkey.ApplyFunc((*realizestate.RealizeStateService).CheckRealizeState,
+					func(_ *realizestate.RealizeStateService, _ wait.Backoff, _ string, _ []nsxUtil.GPRRType) error {
+						return nsxUtil.NewRealizeStateError("mocked realized error")
+					})
+				patches.ApplyFunc((*VPCService).DeleteVPC, func(_ *VPCService, _ string) error {
+					return nil
+				})
+				return patches
+			},
+			expectedErr: "mocked realized error",
+		},
+		{
+			name: "Failed deletion",
+			prepareFunc: func() *gomonkey.Patches {
+				patches := gomonkey.ApplyFunc((*realizestate.RealizeStateService).CheckRealizeState,
+					func(_ *realizestate.RealizeStateService, _ wait.Backoff, _ string, _ []nsxUtil.GPRRType) error {
+						return nsxUtil.NewRealizeStateError("mocked realized error")
+					})
+				patches.ApplyFunc((*VPCService).DeleteVPC, func(_ *VPCService, _ string) error {
+					return nsxUtil.NewRealizeStateError("mocked deletion error")
+				})
+				return patches
+			},
+			expectedErr: "mocked deletion error",
+		},
+	}
+
+	for _, tc := range testCases {
+		if tc.prepareFunc != nil {
+			patches := tc.prepareFunc()
+			defer patches.Reset()
+		}
+		err := service.checkVPCRealizationState(&model.Vpc{Id: common.String("vpc-1")}, "/orgs/org/projects/project/vpcs/vpc-1")
+		if tc.expectedErr != "" {
+			assert.Contains(t, err.Error(), tc.expectedErr)
+		} else {
+			assert.Nil(t, err)
+		}
+	}
+}

pkg/nsx/util/utils.go

@@ -57,6 +57,11 @@ type PortAddress struct {
 	IPs []string `json:"ips"`
 }
 
+type GPRRType struct {
+	Id         string
+	EntityType string
+}
+
 func (e *ErrorDetail) Error() string {
 	msg := fmt.Sprintf("StatusCode is %d,", e.StatusCode)
 	if e.ErrorCode > 0 {

test/e2e/precreated_vpc_test.go

@@ -20,6 +20,7 @@ import (
 	"github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/common"
 	"github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/realizestate"
 	"github.com/vmware-tanzu/nsx-operator/pkg/nsx/services/vpc"
+	nsxutil "github.com/vmware-tanzu/nsx-operator/pkg/nsx/util"
 	pkgutil "github.com/vmware-tanzu/nsx-operator/pkg/util"
 )
 
@@ -292,17 +293,17 @@ func (data *TestData) createVPC(orgID, projectID, vpcID string, privateIPs []str
 	log.Info("Successfully requested VPC on NSX", "path", vpcPath)
 	realizeService := realizestate.InitializeRealizeState(common.Service{NSXClient: data.nsxClient.Client})
 	if pollErr := wait.PollUntilContextTimeout(context.Background(), 10*time.Second, 5*time.Minute, true, func(ctx context.Context) (done bool, err error) {
-		if err = realizeService.CheckRealizeState(pkgutil.NSXTRealizeRetry, vpcPath, []string{common.GatewayInterfaceId}); err != nil {
+		if err = realizeService.CheckRealizeState(pkgutil.NSXTRealizeRetry, vpcPath, vpc.GetGPRRTypeList(vpcID)); err != nil {
 			log.Error(err, "NSX VPC is not yet realized", "path", vpcPath)
 			return false, nil
 		}
 		if lbsPath != "" {
-			if err := realizeService.CheckRealizeState(pkgutil.NSXTRealizeRetry, lbsPath, []string{}); err != nil {
+			if err := realizeService.CheckRealizeState(pkgutil.NSXTRealizeRetry, lbsPath, []nsxutil.GPRRType{}); err != nil {
 				log.Error(err, "NSX LBS is not yet realized", "path", lbsPath)
 				return false, nil
 			}
 		}
-		if err = realizeService.CheckRealizeState(pkgutil.NSXTRealizeRetry, attachmentPath, []string{}); err != nil {
+		if err = realizeService.CheckRealizeState(pkgutil.NSXTRealizeRetry, attachmentPath, []nsxutil.GPRRType{}); err != nil {
 			log.Error(err, "VPC attachment is not yet realized", "path", attachmentPath)
 			return false, nil
 		}