From 7a03bc2d6786dcffc8a0356cef4c8a16b1d38f55 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. Remove entityType from CheckRealizeState. Since we're not sure if the realized items for intentPath will be changed or not --- .../services/realizestate/realize_state.go | 21 ++-- .../realizestate/realize_state_test.go | 107 +++++++++++++++++- 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 | 6 +- 6 files changed, 122 insertions(+), 20 deletions(-) diff --git a/pkg/nsx/services/realizestate/realize_state.go b/pkg/nsx/services/realizestate/realize_state.go index 33a8672f8..1831c81fd 100644 --- 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 index 3f8bc1fe9..69d60e3d7 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, @@ -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 index 60491b57f..d7feaafad 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..112188191 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); 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..a33c098e4 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); 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) @@ -882,7 +882,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) @@ -908,7 +908,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)