Only show applicable conditions for AzureClusters and AzureMachines on async services

azure/scope/cluster.go

@@ -679,20 +679,7 @@ func (s *ClusterScope) PatchObject(ctx context.Context) error {
 	ctx, _, done := tele.StartSpanWithLogger(ctx, "scope.ClusterScope.PatchObject")
 	defer done()
 
-	conditions.SetSummary(s.AzureCluster,
-		conditions.WithConditions(
-			infrav1.ResourceGroupReadyCondition,
-			infrav1.RouteTablesReadyCondition,
-			infrav1.NetworkInfrastructureReadyCondition,
-			infrav1.VnetPeeringReadyCondition,
-			infrav1.DisksReadyCondition,
-			infrav1.NATGatewaysReadyCondition,
-			infrav1.LoadBalancersReadyCondition,
-			infrav1.BastionHostReadyCondition,
-			infrav1.VNetReadyCondition,
-			infrav1.SubnetsReadyCondition,
-		),
-	)
+	conditions.SetSummary(s.AzureCluster)
 
 	return s.patchHelper.Patch(
 		ctx,

azure/scope/machine.go

@@ -547,13 +547,7 @@ func (m *MachineScope) SetAddresses(addrs []corev1.NodeAddress) {
 
 // PatchObject persists the machine spec and status.
 func (m *MachineScope) PatchObject(ctx context.Context) error {
-	conditions.SetSummary(m.AzureMachine,
-		conditions.WithConditions(
-			infrav1.VMRunningCondition,
-			infrav1.AvailabilitySetReadyCondition,
-			infrav1.NetworkInterfaceReadyCondition,
-		),
-	)
+	conditions.SetSummary(m.AzureMachine)
 
 	return m.patchHelper.Patch(
 		ctx,

azure/services/availabilitysets/availabilitysets.go

@@ -70,6 +70,7 @@ func (s *Service) Reconcile(ctx context.Context) error {
 		_, err = s.CreateResource(ctx, setSpec, serviceName)
 	} else {
 		log.V(2).Info("skip creation when no availability set spec is found")
+		return nil
 	}
 
 	s.Scope.UpdatePutStatus(infrav1.AvailabilitySetReadyCondition, serviceName, err)
@@ -85,25 +86,27 @@ func (s *Service) Delete(ctx context.Context) error {
 	defer cancel()
 
 	var resultingErr error
-	if setSpec := s.Scope.AvailabilitySetSpec(); setSpec == nil {
+	setSpec := s.Scope.AvailabilitySetSpec()
+	if setSpec == nil {
 		log.V(2).Info("skip deletion when no availability set spec is found")
+		return nil
+	}
+
+	existingSet, err := s.Get(ctx, setSpec)
+	if err != nil {
+		if !azure.ResourceNotFound(err) {
+			resultingErr = errors.Wrapf(err, "failed to get availability set %s in resource group %s", setSpec.ResourceName(), setSpec.ResourceGroupName())
+		}
 	} else {
-		existingSet, err := s.Get(ctx, setSpec)
-		if err != nil {
-			if !azure.ResourceNotFound(err) {
-				resultingErr = errors.Wrapf(err, "failed to get availability set %s in resource group %s", setSpec.ResourceName(), setSpec.ResourceGroupName())
-			}
+		availabilitySet, ok := existingSet.(compute.AvailabilitySet)
+		if !ok {
+			resultingErr = errors.Errorf("%T is not a compute.AvailabilitySet", existingSet)
 		} else {
-			availabilitySet, ok := existingSet.(compute.AvailabilitySet)
-			if !ok {
-				resultingErr = errors.Errorf("%T is not a compute.AvailabilitySet", existingSet)
+			// only delete when the availability set does not have any vms
+			if availabilitySet.AvailabilitySetProperties != nil && availabilitySet.VirtualMachines != nil && len(*availabilitySet.VirtualMachines) > 0 {
+				log.V(2).Info("skip deleting availability set with VMs", "availability set", setSpec.ResourceName())
 			} else {
-				// only delete when the availability set does not have any vms
-				if availabilitySet.AvailabilitySetProperties != nil && availabilitySet.VirtualMachines != nil && len(*availabilitySet.VirtualMachines) > 0 {
-					log.V(2).Info("skip deleting availability set with VMs", "availability set", setSpec.ResourceName())
-				} else {
-					resultingErr = s.DeleteResource(ctx, setSpec, serviceName)
-				}
+				resultingErr = s.DeleteResource(ctx, setSpec, serviceName)
 			}
 		}
 	}

azure/services/availabilitysets/availabilitysets_test.go

@@ -89,11 +89,10 @@ func TestReconcileAvailabilitySets(t *testing.T) {
 			},
 		},
 		{
-			name:          "noop if no availability set spec is found",
+			name:          "noop if no availability set spec returns nil",
 			expectedError: "",
 			expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) {
 				s.AvailabilitySetSpec().Return(nil)
-				s.UpdatePutStatus(infrav1.AvailabilitySetReadyCondition, serviceName, nil)
 			},
 		},
 		{
@@ -167,7 +166,6 @@ func TestDeleteAvailabilitySets(t *testing.T) {
 			expectedError: "",
 			expect: func(s *mock_availabilitysets.MockAvailabilitySetScopeMockRecorder, m *mock_async.MockGetterMockRecorder, r *mock_async.MockReconcilerMockRecorder) {
 				s.AvailabilitySetSpec().Return(nil)
-				s.UpdateDeleteStatus(infrav1.AvailabilitySetReadyCondition, serviceName, nil)
 			},
 		},
 		{

azure/services/bastionhosts/bastionhosts.go

@@ -61,6 +61,8 @@ func (s *Service) Reconcile(ctx context.Context) error {
 	var resultingErr error
 	if bastionSpec := s.Scope.AzureBastionSpec(); bastionSpec != nil {
 		_, resultingErr = s.CreateResource(ctx, bastionSpec, serviceName)
+	} else {
+		return nil
 	}
 
 	s.Scope.UpdatePutStatus(infrav1.BastionHostReadyCondition, serviceName, resultingErr)
@@ -78,6 +80,8 @@ func (s *Service) Delete(ctx context.Context) error {
 	var resultingErr error
 	if bastionSpec := s.Scope.AzureBastionSpec(); bastionSpec != nil {
 		resultingErr = s.DeleteResource(ctx, bastionSpec, serviceName)
+	} else {
+		return nil
 	}
 
 	s.Scope.UpdateDeleteStatus(infrav1.BastionHostReadyCondition, serviceName, resultingErr)

azure/services/bastionhosts/bastionhosts_test.go

@@ -69,7 +69,6 @@ func TestReconcileBastionHosts(t *testing.T) {
 			expectedError: "",
 			expect: func(s *mock_bastionhosts.MockBastionScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) {
 				s.AzureBastionSpec().Return(nil)
-				s.UpdatePutStatus(infrav1.BastionHostReadyCondition, serviceName, nil)
 			},
 		},
 		{
@@ -140,7 +139,6 @@ func TestDeleteBastionHost(t *testing.T) {
 			expectedError: "",
 			expect: func(s *mock_bastionhosts.MockBastionScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) {
 				s.AzureBastionSpec().Return(nil)
-				s.UpdateDeleteStatus(infrav1.BastionHostReadyCondition, serviceName, nil)
 			},
 		},
 	}

azure/services/disks/disks.go

@@ -67,12 +67,16 @@ func (s *Service) Delete(ctx context.Context) error {
 	ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout)
 	defer cancel()
 
-	var result error
+	specs := s.Scope.DiskSpecs()
+	if len(specs) == 0 {
+		return nil
+	}
 
 	// We go through the list of DiskSpecs to delete each one, independently of the result of the previous one.
 	// If multiple errors occur, we return the most pressing one.
-	//  Order of precedence (highest -> lowest) is: error that is not an operationNotDoneError (ie. error creating) -> operationNotDoneError (ie. creating in progress) -> no error (ie. created)
-	for _, diskSpec := range s.Scope.DiskSpecs() {
+	//  Order of precedence (highest -> lowest) is: error that is not an operationNotDoneError (i.e. error creating) -> operationNotDoneError (i.e. creating in progress) -> no error (i.e. created)
+	var result error
+	for _, diskSpec := range specs {
 		if err := s.DeleteResource(ctx, diskSpec, serviceName); err != nil {
 			if !azure.IsOperationNotDoneError(err) || result == nil {
 				result = err

azure/services/disks/disks_test.go

@@ -56,6 +56,13 @@ func TestDeleteDisk(t *testing.T) {
 		expectedError string
 		expect        func(s *mock_disks.MockDiskScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder)
 	}{
+		{
+			name:          "noop if no disk specs are found",
+			expectedError: "",
+			expect: func(s *mock_disks.MockDiskScopeMockRecorder, r *mock_async.MockReconcilerMockRecorder) {
+				s.DiskSpecs().Return([]azure.ResourceSpecGetter{})
+			},
+		},
 		{
 			name:          "delete the disk",
 			expectedError: "",

azure/services/groups/groups.go

@@ -65,6 +65,9 @@ func (s *Service) Reconcile(ctx context.Context) error {
 	defer cancel()
 
 	groupSpec := s.Scope.GroupSpec()
+	if groupSpec == nil {
+		return nil
+	}
 
 	_, err := s.CreateResource(ctx, groupSpec, serviceName)
 	s.Scope.UpdatePutStatus(infrav1.ResourceGroupReadyCondition, serviceName, err)
@@ -80,6 +83,9 @@ func (s *Service) Delete(ctx context.Context) error {
 	defer cancel()
 
 	groupSpec := s.Scope.GroupSpec()
+	if groupSpec == nil {
+		return nil
+	}
 
 	// check that the resource group is not BYO.
 	managed, err := s.IsGroupManaged(ctx)

azure/services/groups/groups_test.go

@@ -62,6 +62,13 @@ func TestReconcileGroups(t *testing.T) {
 		expectedError string
 		expect        func(s *mock_groups.MockGroupScopeMockRecorder, m *mock_groups.MockclientMockRecorder, r *mock_async.MockReconcilerMockRecorder)
 	}{
+		{
+			name:          "noop if no group spec is found",
+			expectedError: "",
+			expect: func(s *mock_groups.MockGroupScopeMockRecorder, m *mock_groups.MockclientMockRecorder, r *mock_async.MockReconcilerMockRecorder) {
+				s.GroupSpec().Return(nil)
+			},
+		},
 		{
 			name:          "create group succeeds",
 			expectedError: "",
@@ -119,6 +126,13 @@ func TestDeleteGroups(t *testing.T) {
 		expectedError string
 		expect        func(s *mock_groups.MockGroupScopeMockRecorder, m *mock_groups.MockclientMockRecorder, r *mock_async.MockReconcilerMockRecorder)
 	}{
+		{
+			name:          "noop if no group spec is found",
+			expectedError: "",
+			expect: func(s *mock_groups.MockGroupScopeMockRecorder, m *mock_groups.MockclientMockRecorder, r *mock_async.MockReconcilerMockRecorder) {
+				s.GroupSpec().Return(nil)
+			},
+		},
 		{
 			name:          "delete operation is successful for managed resource group",
 			expectedError: "",

azure/services/inboundnatrules/inboundnatrules.go

@@ -62,10 +62,6 @@ func (s *Service) Reconcile(ctx context.Context) error {
 	// Externally managed clusters might not have an LB
 	if s.Scope.APIServerLBName() == "" {
 		log.V(4).Info("Skipping InboundNatRule reconciliation as the cluster has no LB configured")
-		// Until https://github.com/kubernetes-sigs/cluster-api-provider-azure/issues/1868 is
-		// resolved, this needs to be set for the machine to be able to reach the ready condition:
-		// https://github.com/kubernetes-sigs/cluster-api-provider-azure/pull/2066#discussion_r806150004
-		s.Scope.UpdatePutStatus(infrav1.InboundNATRulesReadyCondition, serviceName, nil)
 		return nil
 	}
 
@@ -84,11 +80,16 @@ func (s *Service) Reconcile(ctx context.Context) error {
 		portsInUse[*rule.InboundNatRulePropertiesFormat.FrontendPort] = struct{}{} // Mark frontend port as in use
 	}
 
+	specs := s.Scope.InboundNatSpecs(portsInUse)
+	if len(specs) == 0 {
+		return nil
+	}
+
 	// We go through the list of InboundNatSpecs to reconcile each one, independently of the result of the previous one.
 	// If multiple errors occur, we return the most pressing one.
 	//  Order of precedence (highest -> lowest) is: error that is not an operationNotDoneError (i.e. error creating) -> operationNotDoneError (i.e. creating in progress) -> no error (i.e. created)
 	var result error
-	for _, natRule := range s.Scope.InboundNatSpecs(portsInUse) {
+	for _, natRule := range specs {
 		// If we are creating multiple inbound NAT rules, we could have a collision in finding an available frontend port since the newly created rule takes an available port, and we do not update portsInUse in the specs.
 		// It doesn't matter in this case since we only create one rule per machine, but for multiple rules, we could end up restarting the Reconcile function each time to get the updated available ports.
 		// TODO: We can update the available ports and recompute the specs each time, or alternatively, we could deterministically calculate the ports we plan on using to avoid collisions, i.e. rule #1 uses the first available port, rule #2 uses the second available port, etc.
@@ -100,6 +101,7 @@ func (s *Service) Reconcile(ctx context.Context) error {
 	}
 
 	s.Scope.UpdatePutStatus(infrav1.InboundNATRulesReadyCondition, serviceName, result)
+
 	return result
 }
 
@@ -111,18 +113,23 @@ func (s *Service) Delete(ctx context.Context) error {
 	ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout)
 	defer cancel()
 
-	var result error
+	specs := s.Scope.InboundNatSpecs(make(map[int32]struct{}))
+	if len(specs) == 0 {
+		return nil
+	}
 
 	// We go through the list of InboundNatSpecs to delete each one, independently of the result of the previous one.
 	// If multiple errors occur, we return the most pressing one.
 	//  Order of precedence (highest -> lowest) is: error that is not an operationNotDoneError (i.e. error deleting) -> operationNotDoneError (i.e. deleting in progress) -> no error (i.e. deleted)
-	for _, natRule := range s.Scope.InboundNatSpecs(make(map[int32]struct{})) {
+	var result error
+	for _, natRule := range specs {
 		if err := s.DeleteResource(ctx, natRule, serviceName); err != nil {
 			if !azure.IsOperationNotDoneError(err) || result == nil {
 				result = err
 			}
 		}
 	}
+
 	s.Scope.UpdateDeleteStatus(infrav1.InboundNATRulesReadyCondition, serviceName, result)
 	return result
 }

azure/services/inboundnatrules/inboundnatrules_test.go

@@ -92,6 +92,18 @@ func TestReconcileInboundNATRule(t *testing.T) {
 			m *mock_inboundnatrules.MockclientMockRecorder,
 			r *mock_async.MockReconcilerMockRecorder)
 	}{
+		{
+			name:          "noop if no NAT rule specs are found",
+			expectedError: "",
+			expect: func(s *mock_inboundnatrules.MockInboundNatScopeMockRecorder,
+				m *mock_inboundnatrules.MockclientMockRecorder,
+				r *mock_async.MockReconcilerMockRecorder) {
+				s.ResourceGroup().AnyTimes().Return(fakeGroupName)
+				s.APIServerLBName().AnyTimes().Return(fakeLBName)
+				m.List(gomockinternal.AContext(), fakeGroupName, fakeLBName).Return(noExistingRules, nil)
+				s.InboundNatSpecs(noPortsInUse).Return([]azure.ResourceSpecGetter{})
+			},
+		},
 		{
 			name:          "NAT rule successfully created with with no existing rules",
 			expectedError: "",
@@ -131,7 +143,6 @@ func TestReconcileInboundNATRule(t *testing.T) {
 				m *mock_inboundnatrules.MockclientMockRecorder,
 				r *mock_async.MockReconcilerMockRecorder) {
 				s.APIServerLBName().AnyTimes().Return("")
-				s.UpdatePutStatus(infrav1.InboundNATRulesReadyCondition, serviceName, nil)
 			},
 		},
 		{
@@ -201,6 +212,14 @@ func TestDeleteNetworkInterface(t *testing.T) {
 		expect        func(s *mock_inboundnatrules.MockInboundNatScopeMockRecorder,
 			m *mock_inboundnatrules.MockclientMockRecorder, r *mock_async.MockReconcilerMockRecorder)
 	}{
+		{
+			name:          "noop if no NAT rules are found",
+			expectedError: "",
+			expect: func(s *mock_inboundnatrules.MockInboundNatScopeMockRecorder,
+				m *mock_inboundnatrules.MockclientMockRecorder, r *mock_async.MockReconcilerMockRecorder) {
+				s.InboundNatSpecs(noPortsInUse).Return([]azure.ResourceSpecGetter{})
+			},
+		},
 		{
 			name:          "successfully delete an existing NAT rule",
 			expectedError: "",

azure/services/loadbalancers/loadbalancers.go

@@ -63,11 +63,16 @@ func (s *Service) Reconcile(ctx context.Context) error {
 	ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout)
 	defer cancel()
 
+	specs := s.Scope.LBSpecs()
+	if len(specs) == 0 {
+		return nil
+	}
+
 	// We go through the list of LBSpecs to reconcile each one, independently of the result of the previous one.
 	// If multiple errors occur, we return the most pressing one.
 	//  Order of precedence (highest -> lowest) is: error that is not an operationNotDoneError (i.e. error creating) -> operationNotDoneError (i.e. creating in progress) -> no error (i.e. created)
 	var result error
-	for _, lbSpec := range s.Scope.LBSpecs() {
+	for _, lbSpec := range specs {
 		if _, err := s.CreateResource(ctx, lbSpec, serviceName); err != nil {
 			if !azure.IsOperationNotDoneError(err) || result == nil {
 				result = err
@@ -87,18 +92,23 @@ func (s *Service) Delete(ctx context.Context) error {
 	ctx, cancel := context.WithTimeout(ctx, reconciler.DefaultAzureServiceReconcileTimeout)
 	defer cancel()
 
-	var result error
+	specs := s.Scope.LBSpecs()
+	if len(specs) == 0 {
+		return nil
+	}
 
 	// We go through the list of LBSpecs to delete each one, independently of the result of the previous one.
 	// If multiple errors occur, we return the most pressing one.
 	//  Order of precedence (highest -> lowest) is: error that is not an operationNotDoneError (i.e. error deleting) -> operationNotDoneError (i.e. deleting in progress) -> no error (i.e. deleted)
-	for _, lbSpec := range s.Scope.LBSpecs() {
+	var result error
+	for _, lbSpec := range specs {
 		if err := s.DeleteResource(ctx, lbSpec, serviceName); err != nil {
 			if !azure.IsOperationNotDoneError(err) || result == nil {
 				result = err
 			}
 		}
 	}
+
 	s.Scope.UpdateDeleteStatus(infrav1.LoadBalancersReadyCondition, serviceName, result)
 	return result
 }