Merge pull request #3145 from k8s-infra-cherrypick-robot/cherry-pick-3096-to-release-1.7

[release-1.7] Don't delete VMSS upon failure and add bootstrap status condition

Author: k8s-ci-robot

azure/converters/vmss.go

@@ -19,6 +19,7 @@ package converters
 import (
 	"github.com/Azure/azure-sdk-for-go/services/compute/mgmt/2021-11-01/compute"
 	"github.com/Azure/go-autorest/autorest/to"
+	"k8s.io/utils/pointer"
 	azprovider "sigs.k8s.io/cloud-provider-azure/pkg/provider"
 	infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
 	"sigs.k8s.io/cluster-api-provider-azure/azure"
@@ -121,6 +122,16 @@ func SDKToVMSSVM(sdkInstance compute.VirtualMachineScaleSetVM) *azure.VMSSVM {
 		instance.Name = *sdkInstance.OsProfile.ComputerName
 	}
 
+	if sdkInstance.Resources != nil {
+		for _, r := range *sdkInstance.Resources {
+			if r.ProvisioningState != nil && r.Name != nil &&
+				(*r.Name == azure.BootstrappingExtensionLinux || *r.Name == azure.BootstrappingExtensionWindows) {
+				instance.BootstrappingState = infrav1.ProvisioningState(pointer.StringDeref(r.ProvisioningState, ""))
+				break
+			}
+		}
+	}
+
 	if sdkInstance.StorageProfile != nil && sdkInstance.StorageProfile.ImageReference != nil {
 		imageRef := sdkInstance.StorageProfile.ImageReference
 		instance.Image = SDKImageToImage(imageRef, sdkInstance.Plan != nil)

azure/defaults.go

@@ -51,6 +51,13 @@ const (
 	WindowsOS = "Windows"
 )
 
+const (
+	// BootstrappingExtensionLinux is the name of the Linux CAPZ bootstrapping VM extension.
+	BootstrappingExtensionLinux = "CAPZ.Linux.Bootstrapping"
+	// BootstrappingExtensionWindows is the name of the Windows CAPZ bootstrapping VM extension.
+	BootstrappingExtensionWindows = "CAPZ.Windows.Bootstrapping"
+)
+
 const (
 	// DefaultWindowsOsAndVersion is the default Windows Server version to use when
 	// genearating default images for Windows nodes.
@@ -309,7 +316,7 @@ func GetBootstrappingVMExtension(osType string, cloud string, vmName string) *Ex
 	if osType == LinuxOS && cloud == azureautorest.PublicCloud.Name {
 		// The command checks for the existence of the bootstrapSentinelFile on the machine, with retries and sleep between retries.
 		return &ExtensionSpec{
-			Name:      "CAPZ.Linux.Bootstrapping",
+			Name:      BootstrappingExtensionLinux,
 			VMName:    vmName,
 			Publisher: "Microsoft.Azure.ContainerUpstream",
 			Version:   "1.0",
@@ -321,7 +328,7 @@ func GetBootstrappingVMExtension(osType string, cloud string, vmName string) *Ex
 		// This command for the existence of the bootstrapSentinelFile on the machine, with retries and sleep between reties.
 		// If the file is not present after the retries are exhausted the extension fails with return code '-2' - ERROR_FILE_NOT_FOUND.
 		return &ExtensionSpec{
-			Name:      "CAPZ.Windows.Bootstrapping",
+			Name:      BootstrappingExtensionWindows,
 			VMName:    vmName,
 			Publisher: "Microsoft.Azure.ContainerUpstream",
 			Version:   "1.0",

azure/scope/machinepoolmachine.go

@@ -273,16 +273,29 @@ func (s *MachinePoolMachineScope) Close(ctx context.Context) error {
 	return s.PatchObject(ctx)
 }
 
-// UpdateNodeStatus AzureMachinePoolMachine conditions and ready status. It will also update the node ref and the Kubernetes
+// UpdateNodeStatus updates AzureMachinePoolMachine conditions and ready status. It will also update the node ref and the Kubernetes
 // version of the VM instance if the node is found.
 // Note: This func should be called at the end of a reconcile request and after updating the scope with the most recent Azure data.
 func (s *MachinePoolMachineScope) UpdateNodeStatus(ctx context.Context) error {
-	ctx, _, done := tele.StartSpanWithLogger(
+	ctx, log, done := tele.StartSpanWithLogger(
 		ctx,
 		"scope.MachinePoolMachineScope.UpdateNodeStatus",
 	)
 	defer done()
 
+	if s.instance != nil {
+		switch s.instance.BootstrappingState {
+		case infrav1.Creating:
+			conditions.MarkFalse(s.AzureMachinePoolMachine, infrav1.BootstrapSucceededCondition, infrav1.BootstrapInProgressReason, clusterv1.ConditionSeverityInfo, "VM bootstrapping")
+		case infrav1.Failed:
+			log.Info("VM bootstrapping failed")
+			conditions.MarkFalse(s.AzureMachinePoolMachine, infrav1.BootstrapSucceededCondition, infrav1.BootstrapFailedReason, clusterv1.ConditionSeverityInfo, "VM bootstrapping failed")
+		case infrav1.Succeeded:
+			log.Info("VM bootstrapping succeeded")
+			conditions.MarkTrue(s.AzureMachinePoolMachine, infrav1.BootstrapSucceededCondition)
+		}
+	}
+
 	var node *corev1.Node
 	nodeRef := s.AzureMachinePoolMachine.Status.NodeRef
 

azure/types.go

@@ -94,12 +94,13 @@ type ExtensionSpec struct {
 type (
 	// VMSSVM defines a VM in a virtual machine scale set.
 	VMSSVM struct {
-		ID               string                    `json:"id,omitempty"`
-		InstanceID       string                    `json:"instanceID,omitempty"`
-		Image            infrav1.Image             `json:"image,omitempty"`
-		Name             string                    `json:"name,omitempty"`
-		AvailabilityZone string                    `json:"availabilityZone,omitempty"`
-		State            infrav1.ProvisioningState `json:"vmState,omitempty"`
+		ID                 string                    `json:"id,omitempty"`
+		InstanceID         string                    `json:"instanceID,omitempty"`
+		Image              infrav1.Image             `json:"image,omitempty"`
+		Name               string                    `json:"name,omitempty"`
+		AvailabilityZone   string                    `json:"availabilityZone,omitempty"`
+		State              infrav1.ProvisioningState `json:"vmState,omitempty"`
+		BootstrappingState infrav1.ProvisioningState `json:"bootstrappingState,omitempty"`
 	}
 
 	// VMSS defines a virtual machine scale set.

exp/controllers/azuremachinepool_controller.go

@@ -315,11 +315,8 @@ func (ampr *AzureMachinePoolReconciler) reconcileNormal(ctx context.Context, mac
 		log.Info("Unexpected scale set deletion", "id", machinePoolScope.ProviderID())
 		ampr.Recorder.Eventf(machinePoolScope.AzureMachinePool, corev1.EventTypeWarning, "UnexpectedVMDeletion", "Unexpected Azure scale set deletion")
 	case infrav1.Failed:
-		err := ams.Delete(ctx)
-		if err != nil {
-			return reconcile.Result{}, errors.Wrap(err, "failed to delete scale set in a failed state")
-		}
-		return reconcile.Result{}, errors.Wrap(err, "Scale set deleted, retry creating in next reconcile")
+		log.Info("Unexpected scale set failure", "id", machinePoolScope.ProviderID())
+		ampr.Recorder.Eventf(machinePoolScope.AzureMachinePool, corev1.EventTypeWarning, "UnexpectedVMFailure", "Unexpected Azure scale set failure")
 	}
 
 	if machinePoolScope.NeedsRequeue() {

exp/controllers/azuremachinepoolmachine_controller.go

@@ -38,6 +38,7 @@ import (
 	capierrors "sigs.k8s.io/cluster-api/errors"
 	"sigs.k8s.io/cluster-api/util"
 	"sigs.k8s.io/cluster-api/util/annotations"
+	"sigs.k8s.io/cluster-api/util/conditions"
 	"sigs.k8s.io/cluster-api/util/predicates"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -287,6 +288,11 @@ func (ampmr *AzureMachinePoolMachineController) reconcileNormal(ctx context.Cont
 
 	log.V(2).Info(fmt.Sprintf("Scale Set VM is %s", state), "id", machineScope.ProviderID())
 
+	bootstrappingCondition := conditions.Get(machineScope.AzureMachinePoolMachine, infrav1.BootstrapSucceededCondition)
+	if bootstrappingCondition != nil && bootstrappingCondition.Reason == infrav1.BootstrapFailedReason {
+		return reconcile.Result{}, nil
+	}
+
 	if !infrav1.IsTerminalProvisioningState(state) || !machineScope.IsReady() {
 		log.V(2).Info("Requeuing", "state", state, "ready", machineScope.IsReady())
 		// we are in a non-terminal state, retry in a bit
@@ -377,7 +383,7 @@ func (r *azureMachinePoolMachineReconciler) Delete(ctx context.Context) error {
 		}
 
 		if err := r.Scope.UpdateInstanceStatus(ctx); err != nil {
-			log.V(4).Info("failed to update VMSS VM instanace status during delete")
+			log.V(4).Info("failed to update VMSS VM instance status during delete")
 		}
 	}()