diff --git a/pkg/nsx/services/subnet/subnet.go b/pkg/nsx/services/subnet/subnet.go index 835d8a7bf..f3d134994 100644 --- a/pkg/nsx/services/subnet/subnet.go +++ b/pkg/nsx/services/subnet/subnet.go @@ -89,6 +89,7 @@ func (service *SubnetService) CreateOrUpdateSubnet(obj client.Object, vpcInfo co log.Error(err, "Failed to build Subnet") return nil, err } + isUpdate := false // Only check whether it needs update when obj is v1alpha1.Subnet if subnet, ok := obj.(*v1alpha1.Subnet); ok { existingSubnet := service.SubnetStore.GetByKey(service.BuildSubnetID(subnet)) @@ -100,12 +101,14 @@ func (service *SubnetService) CreateOrUpdateSubnet(obj client.Object, vpcInfo co if changed { // Only tags and dhcp are expected to be updated // inherit other fields from the existing Subnet - existingSubnet.Tags = nsxSubnet.Tags - if existingSubnet.SubnetDhcpConfig != nil { - existingSubnet.SubnetDhcpConfig = nsxSubnet.SubnetDhcpConfig - existingSubnet.AdvancedConfig.StaticIpAllocation.Enabled = nsxSubnet.AdvancedConfig.StaticIpAllocation.Enabled - } - nsxSubnet = existingSubnet + // Avoid modification on existingSubnet to ensure + // Subnet store is only updated after the updating succeeds. + updatedSubnet := *existingSubnet + updatedSubnet.Tags = nsxSubnet.Tags + updatedSubnet.SubnetDhcpConfig = nsxSubnet.SubnetDhcpConfig + updatedSubnet.AdvancedConfig.StaticIpAllocation.Enabled = nsxSubnet.AdvancedConfig.StaticIpAllocation.Enabled + nsxSubnet = &updatedSubnet + isUpdate = true } } if !changed { @@ -113,10 +116,10 @@ func (service *SubnetService) CreateOrUpdateSubnet(obj client.Object, vpcInfo co return existingSubnet, nil } } - return service.createOrUpdateSubnet(obj, nsxSubnet, &vpcInfo) + return service.createOrUpdateSubnet(obj, nsxSubnet, &vpcInfo, isUpdate) } -func (service *SubnetService) createOrUpdateSubnet(obj client.Object, nsxSubnet *model.VpcSubnet, vpcInfo *common.VPCResourceInfo) (*model.VpcSubnet, error) { +func (service *SubnetService) createOrUpdateSubnet(obj client.Object, nsxSubnet *model.VpcSubnet, vpcInfo *common.VPCResourceInfo, isUpdate bool) (*model.VpcSubnet, error) { orgRoot, err := service.WrapHierarchySubnet(nsxSubnet, vpcInfo) if err != nil { log.Error(err, "Failed to WrapHierarchySubnet") @@ -138,6 +141,11 @@ func (service *SubnetService) createOrUpdateSubnet(obj client.Object, nsxSubnet Jitter: 0, Steps: 6, } + + // For update case, wait for some time to make sure the realized state is updated. + if isUpdate { + time.Sleep(1 * time.Second) + } // 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. @@ -428,28 +436,31 @@ func (service *SubnetService) UpdateSubnetSet(ns string, vpcSubnets []*model.Vpc } newTags := append(service.buildBasicTags(subnetSet), tags...) - var updatedSubnet *model.VpcSubnet + var comparableSubnet *model.VpcSubnet if vpcSubnets[i].SubnetDhcpConfig == nil { - updatedSubnet = &model.VpcSubnet{ + comparableSubnet = &model.VpcSubnet{ Tags: newTags, } } else { - updatedSubnet = &model.VpcSubnet{ + comparableSubnet = &model.VpcSubnet{ Tags: newTags, SubnetDhcpConfig: service.buildSubnetDHCPConfig(dhcpMode), } } - changed := common.CompareResource(SubnetToComparable(vpcSubnets[i]), SubnetToComparable(updatedSubnet)) + changed := common.CompareResource(SubnetToComparable(vpcSubnets[i]), SubnetToComparable(comparableSubnet)) if !changed { log.Info("NSX Subnet unchanged, skipping update", "Subnet", *vpcSubnet.Id) continue } - vpcSubnets[i].Tags = newTags + // Avoid updating vpcSubnets[i] to ensure Subnet store + // is only updated after the updating succeeds. + updatedSubnet := *vpcSubnets[i] + updatedSubnet.Tags = newTags // Update the SubnetSet DHCP Config - if vpcSubnets[i].SubnetDhcpConfig != nil { - vpcSubnets[i].SubnetDhcpConfig = service.buildSubnetDHCPConfig(dhcpMode) - vpcSubnets[i].AdvancedConfig.StaticIpAllocation.Enabled = &staticIpAllocation + if updatedSubnet.SubnetDhcpConfig != nil { + updatedSubnet.SubnetDhcpConfig = service.buildSubnetDHCPConfig(dhcpMode) + updatedSubnet.AdvancedConfig.StaticIpAllocation.Enabled = &staticIpAllocation } vpcInfo, err := common.ParseVPCResourcePath(*vpcSubnets[i].Path) @@ -457,7 +468,7 @@ func (service *SubnetService) UpdateSubnetSet(ns string, vpcSubnets []*model.Vpc err := fmt.Errorf("failed to parse NSX VPC path for Subnet %s: %s", *vpcSubnets[i].Path, err) return err } - if _, err := service.createOrUpdateSubnet(subnetSet, vpcSubnets[i], &vpcInfo); err != nil { + if _, err := service.createOrUpdateSubnet(subnetSet, &updatedSubnet, &vpcInfo, true); err != nil { return fmt.Errorf("failed to update Subnet %s in SubnetSet %s: %w", *vpcSubnet.Id, subnetSet.Name, err) } log.Info("Successfully updated SubnetSet", "subnetSet", subnetSet, "Subnet", *vpcSubnet.Id)