Merge pull request #2093 from Jont828/ready-conditions

Only show applicable conditions for AzureClusters and AzureMachines on async services

Author: k8s-ci-robot

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: "",