From 5259e2bc92531541c9c3db0097ca2c490419024f Mon Sep 17 00:00:00 2001 From: Tao Zou Date: Fri, 22 Nov 2024 11:16:06 +0800 Subject: [PATCH] Check all resources realized status when creating subnet Subnet includes child resources which also need checking realized status. Check subnet for all child resources --- .../services/realizestate/realize_state.go | 15 ++- .../realizestate/realize_state_test.go | 99 ++++++++++++++++++- pkg/nsx/services/subnet/subnet.go | 3 +- pkg/nsx/services/subnet/subnet_test.go | 3 +- pkg/nsx/services/subnetport/subnetport.go | 2 +- pkg/nsx/services/vpc/vpc.go | 2 +- 6 files changed, 117 insertions(+), 7 deletions(-) diff --git a/pkg/nsx/services/realizestate/realize_state.go b/pkg/nsx/services/realizestate/realize_state.go index 33a8672f8..041457ff4 100644 --- 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 index 3f8bc1fe9..9a7a08b5b 100644 --- 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 index 0b8cc101a..0f3c76e91 100644 --- 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 index 5c0c02e0c..7c03601da 100644 --- 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 index 3d0c466f3..87cb8dfd3 100644 --- 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 index c840a45c6..b6ddb702e 100644 --- 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)