Make VNet and NSGs reconcile/delete async

api/v1beta1/types.go

@@ -130,11 +130,6 @@ type VnetPeeringSpec struct {
 // VnetPeerings is a slice of VnetPeering.
 type VnetPeerings []VnetPeeringSpec
 
-// IsManaged returns true if the vnet is managed.
-func (v *VnetSpec) IsManaged(clusterName string) bool {
-	return v.ID == "" || v.Tags.HasOwned(clusterName)
-}
-
 // Subnets is a slice of Subnet.
 type Subnets []SubnetSpec
 

azure/interfaces.go

@@ -32,6 +32,15 @@ type Reconciler interface {
 	Delete(ctx context.Context) error
 }
 
+// SharedReconciler is a generic interface used by components offering a type of service.
+// Example: virtualnetworks service would offer Reconcile/Delete methods.
+// TODO: remove this.
+type SharedReconciler interface {
+	Reconcile(ctx context.Context) error
+	Delete(ctx context.Context) error
+	IsManaged(ctx context.Context, spec ResourceSpecGetter) (bool, error)
+}
+
 // CredentialGetter is a Service which knows how to retrieve credentials for an Azure
 // resource in a resource group.
 type CredentialGetter interface {
@@ -54,7 +63,6 @@ type Authorizer interface {
 // NetworkDescriber is an interface which can get common Azure Cluster Networking information.
 type NetworkDescriber interface {
 	Vnet() *infrav1.VnetSpec
-	IsVnetManaged() bool
 	ControlPlaneSubnet() infrav1.SubnetSpec
 	Subnets() infrav1.Subnets
 	Subnet(string) infrav1.SubnetSpec

azure/scope/cluster.go

@@ -36,6 +36,8 @@ import (
 	infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
 	"sigs.k8s.io/cluster-api-provider-azure/azure"
 	"sigs.k8s.io/cluster-api-provider-azure/azure/services/groups"
+	"sigs.k8s.io/cluster-api-provider-azure/azure/services/securitygroups"
+	"sigs.k8s.io/cluster-api-provider-azure/azure/services/virtualnetworks"
 	"sigs.k8s.io/cluster-api-provider-azure/azure/services/vnetpeerings"
 	"sigs.k8s.io/cluster-api-provider-azure/util/futures"
 	"sigs.k8s.io/cluster-api-provider-azure/util/tele"
@@ -89,6 +91,7 @@ func NewClusterScope(ctx context.Context, params ClusterScopeParams) (*ClusterSc
 		Cluster:      params.Cluster,
 		AzureCluster: params.AzureCluster,
 		patchHelper:  helper,
+		cache:        &clusterCache{},
 	}, nil
 }
 
@@ -100,6 +103,22 @@ type ClusterScope struct {
 	AzureClients
 	Cluster      *clusterv1.Cluster
 	AzureCluster *infrav1.AzureCluster
+	cache        *clusterCache
+}
+
+// clusterCache stores common cluster information so we don't have to hit the API multiple times within the same reconcile loop.
+type clusterCache struct {
+	IsVnetManaged *bool
+}
+
+// GetVnetManagedCache gets the value of VNet management in the cluster cache.
+func (s *ClusterScope) GetVnetManagedCache() *bool {
+	return s.cache.IsVnetManaged
+}
+
+// SetVnetManagedCache stores the value of VNet management in the cluster cache so it can be accessed later in the reconcile.
+func (s *ClusterScope) SetVnetManagedCache(managed bool) {
+	s.cache.IsVnetManaged = &managed
 }
 
 // BaseURI returns the Azure ResourceManagerEndpoint.
@@ -241,15 +260,16 @@ func (s *ClusterScope) NatGatewaySpecs() []azure.NatGatewaySpec {
 }
 
 // NSGSpecs returns the security group specs.
-func (s *ClusterScope) NSGSpecs() []azure.NSGSpec {
-	nsgspecs := make([]azure.NSGSpec, len(s.AzureCluster.Spec.NetworkSpec.Subnets))
+func (s *ClusterScope) NSGSpecs() []azure.ResourceSpecGetter {
+	nsgspecs := make([]azure.ResourceSpecGetter, len(s.AzureCluster.Spec.NetworkSpec.Subnets))
 	for i, subnet := range s.AzureCluster.Spec.NetworkSpec.Subnets {
-		nsgspecs[i] = azure.NSGSpec{
+		nsgspecs[i] = &securitygroups.NSGSpec{
 			Name:          subnet.SecurityGroup.Name,
 			SecurityRules: subnet.SecurityGroup.SecurityRules,
+			ResourceGroup: s.ResourceGroup(),
+			Location:      s.Location(),
 		}
 	}
-
 	return nsgspecs
 }
 
@@ -327,11 +347,14 @@ func (s *ClusterScope) VnetPeeringSpecs() []azure.ResourceSpecGetter {
 }
 
 // VNetSpec returns the virtual network spec.
-func (s *ClusterScope) VNetSpec() azure.VNetSpec {
-	return azure.VNetSpec{
-		ResourceGroup: s.Vnet().ResourceGroup,
-		Name:          s.Vnet().Name,
-		CIDRs:         s.Vnet().CIDRBlocks,
+func (s *ClusterScope) VNetSpec() azure.ResourceSpecGetter {
+	return &virtualnetworks.VNetSpec{
+		ResourceGroup:  s.Vnet().ResourceGroup,
+		Name:           s.Vnet().Name,
+		CIDRs:          s.Vnet().CIDRBlocks,
+		Location:       s.Location(),
+		ClusterName:    s.ClusterName(),
+		AdditionalTags: s.AdditionalTags(),
 	}
 }
 
@@ -387,11 +410,6 @@ func (s *ClusterScope) Vnet() *infrav1.VnetSpec {
 	return &s.AzureCluster.Spec.NetworkSpec.Vnet
 }
 
-// IsVnetManaged returns true if the vnet is managed.
-func (s *ClusterScope) IsVnetManaged() bool {
-	return s.Vnet().ID == "" || s.Vnet().Tags.HasOwned(s.ClusterName())
-}
-
 // IsIPv6Enabled returns true if IPv6 is enabled.
 func (s *ClusterScope) IsIPv6Enabled() bool {
 	for _, cidr := range s.AzureCluster.Spec.NetworkSpec.Vnet.CIDRBlocks {
@@ -597,6 +615,8 @@ func (s *ClusterScope) PatchObject(ctx context.Context) error {
 			infrav1.NetworkInfrastructureReadyCondition,
 			infrav1.VnetPeeringReadyCondition,
 			infrav1.DisksReadyCondition,
+			infrav1.VNetReadyCondition,
+			infrav1.SecurityGroupsReadyCondition,
 		),
 	)
 
@@ -609,6 +629,8 @@ func (s *ClusterScope) PatchObject(ctx context.Context) error {
 			infrav1.NetworkInfrastructureReadyCondition,
 			infrav1.VnetPeeringReadyCondition,
 			infrav1.DisksReadyCondition,
+			infrav1.VNetReadyCondition,
+			infrav1.SecurityGroupsReadyCondition,
 		}})
 }
 

azure/scope/managedcontrolplane.go

@@ -39,6 +39,7 @@ import (
 	infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
 	"sigs.k8s.io/cluster-api-provider-azure/azure"
 	"sigs.k8s.io/cluster-api-provider-azure/azure/services/groups"
+	"sigs.k8s.io/cluster-api-provider-azure/azure/services/virtualnetworks"
 	infrav1exp "sigs.k8s.io/cluster-api-provider-azure/exp/api/v1beta1"
 	"sigs.k8s.io/cluster-api-provider-azure/util/futures"
 	"sigs.k8s.io/cluster-api-provider-azure/util/tele"
@@ -99,6 +100,7 @@ func NewManagedControlPlaneScope(ctx context.Context, params ManagedControlPlane
 		InfraMachinePool: params.InfraMachinePool,
 		PatchTarget:      params.PatchTarget,
 		patchHelper:      helper,
+		cache:            &clusterCache{},
 	}, nil
 }
 
@@ -114,6 +116,7 @@ type ManagedControlPlaneScope struct {
 	ControlPlane     *infrav1exp.AzureManagedControlPlane
 	InfraMachinePool *infrav1exp.AzureManagedMachinePool
 	PatchTarget      client.Object
+	cache            *clusterCache
 
 	AllNodePools []infrav1exp.AzureManagedMachinePool
 }
@@ -192,6 +195,16 @@ func (s *ManagedControlPlaneScope) Close(ctx context.Context) error {
 	return s.PatchObject(ctx)
 }
 
+// GetVnetManagedCache gets the value of VNet management in the cluster cache.
+func (s *ManagedControlPlaneScope) GetVnetManagedCache() *bool {
+	return s.cache.IsVnetManaged
+}
+
+// SetVnetManagedCache stores the value of VNet management in the cluster cache so it can be accessed later in the reconcile.
+func (s *ManagedControlPlaneScope) SetVnetManagedCache(managed bool) {
+	s.cache.IsVnetManaged = &managed
+}
+
 // Vnet returns the cluster Vnet.
 func (s *ManagedControlPlaneScope) Vnet() *infrav1.VnetSpec {
 	return &infrav1.VnetSpec{
@@ -212,11 +225,14 @@ func (s *ManagedControlPlaneScope) GroupSpec() azure.ResourceSpecGetter {
 }
 
 // VNetSpec returns the virtual network spec.
-func (s *ManagedControlPlaneScope) VNetSpec() azure.VNetSpec {
-	return azure.VNetSpec{
-		ResourceGroup: s.Vnet().ResourceGroup,
-		Name:          s.Vnet().Name,
-		CIDRs:         s.Vnet().CIDRBlocks,
+func (s *ManagedControlPlaneScope) VNetSpec() azure.ResourceSpecGetter {
+	return &virtualnetworks.VNetSpec{
+		ResourceGroup:  s.Vnet().ResourceGroup,
+		Name:           s.Vnet().Name,
+		CIDRs:          s.Vnet().CIDRBlocks,
+		Location:       s.Location(),
+		ClusterName:    s.ClusterName(),
+		AdditionalTags: s.AdditionalTags(),
 	}
 }
 
@@ -297,11 +313,6 @@ func (s *ManagedControlPlaneScope) IsIPv6Enabled() bool {
 	return false
 }
 
-// IsVnetManaged returns true if the vnet is managed.
-func (s *ManagedControlPlaneScope) IsVnetManaged() bool {
-	return true
-}
-
 // APIServerLBName returns the API Server LB name.
 func (s *ManagedControlPlaneScope) APIServerLBName() string {
 	return "" // does not apply for AKS

azure/services/availabilitysets/availabilitysets.go

@@ -120,7 +120,7 @@ func (s *Service) Delete(ctx context.Context) error {
 	}
 
 	as, err := s.Client.Get(ctx, s.Scope.ResourceGroup(), availabilitySetName)
-	if err != nil && azure.ResourceNotFound(err) {
+	if azure.ResourceNotFound(err) {
 		// already deleted
 		return nil
 	}
@@ -136,7 +136,7 @@ func (s *Service) Delete(ctx context.Context) error {
 
 	log.V(2).Info("deleting availability set", "availability set", availabilitySetName)
 	err = s.Client.Delete(ctx, s.Scope.ResourceGroup(), availabilitySetName)
-	if err != nil && azure.ResourceNotFound(err) {
+	if azure.ResourceNotFound(err) {
 		// already deleted
 		return nil
 	}

azure/services/bastionhosts/azurebastion.go

@@ -96,7 +96,7 @@ func (s *Service) ensureAzureBastionDeleted(ctx context.Context, azureBastionSpe
 	log.V(2).Info("deleting bastion host", "bastion", azureBastionSpec.Name)
 
 	err := s.client.Delete(ctx, s.Scope.ResourceGroup(), azureBastionSpec.Name)
-	if err != nil && azure.ResourceNotFound(err) {
+	if azure.ResourceNotFound(err) {
 		// Resource already deleted, all good.
 	} else if err != nil {
 		return errors.Wrapf(err, "failed to delete Azure Bastion %s in resource group %s", azureBastionSpec.Name, s.Scope.ResourceGroup())

azure/services/groups/groups.go

@@ -82,7 +82,7 @@ func (s *Service) Delete(ctx context.Context) error {
 	groupSpec := s.Scope.GroupSpec()
 
 	// check that the resource group is not BYO.
-	managed, err := s.IsGroupManaged(ctx)
+	managed, err := s.IsManaged(ctx)
 	if err != nil {
 		if azure.ResourceNotFound(err) {
 			// already deleted or doesn't exist, cleanup status and return.
@@ -102,10 +102,10 @@ func (s *Service) Delete(ctx context.Context) error {
 	return err
 }
 
-// IsGroupManaged returns true if the resource group has an owned tag with the cluster name as value,
+// IsManaged returns true if the resource group has an owned tag with the cluster name as value,
 // meaning that the resource group's lifecycle is managed.
-func (s *Service) IsGroupManaged(ctx context.Context) (bool, error) {
-	ctx, _, done := tele.StartSpanWithLogger(ctx, "groups.Service.IsGroupManaged")
+func (s *Service) IsManaged(ctx context.Context) (bool, error) {
+	ctx, _, done := tele.StartSpanWithLogger(ctx, "groups.Service.IsManaged")
 	defer done()
 
 	groupSpec := s.Scope.GroupSpec()

azure/services/loadbalancers/loadbalancers.go

@@ -179,7 +179,7 @@ func (s *Service) Delete(ctx context.Context) error {
 	for _, lbSpec := range s.Scope.LBSpecs() {
 		log.V(2).Info("deleting load balancer", "load balancer", lbSpec.Name)
 		err := s.Client.Delete(ctx, s.Scope.ResourceGroup(), lbSpec.Name)
-		if err != nil && azure.ResourceNotFound(err) {
+		if azure.ResourceNotFound(err) {
 			// already deleted
 			continue
 		}