Merge pull request #5040 from r4f4/fix-5037

🐛 awsmachine: only register machine to LB when it's running

Author: k8s-ci-robot

controllers/awsmachine_controller.go

@@ -619,8 +619,14 @@ func (r *AWSMachineReconciler) reconcileNormal(_ context.Context, machineScope *
 		}
 
 		if err := r.reconcileLBAttachment(machineScope, elbScope, instance); err != nil {
-			machineScope.Error(err, "failed to reconcile LB attachment")
-			return ctrl.Result{}, err
+			// We are tolerating InstanceNotRunning error, so we don't report it as an error condition.
+			// Because we are reconciling all load balancers, attempt to treat the error as a list of errors.
+			if err := kerrors.FilterOut(err, elb.IsInstanceNotRunning); err != nil {
+				machineScope.Error(err, "failed to reconcile LB attachment")
+				return ctrl.Result{}, err
+			}
+			// Cannot attach non-running instances to LB
+			shouldRequeue = true
 		}
 	}
 
@@ -1002,6 +1008,14 @@ func (r *AWSMachineReconciler) registerInstanceToV2LB(machineScope *scope.Machin
 		return nil
 	}
 
+	// See https://docs.aws.amazon.com/elasticloadbalancing/latest/application/target-group-register-targets.html#register-instances
+	if ptr.Deref(machineScope.GetInstanceState(), infrav1.InstanceStatePending) != infrav1.InstanceStateRunning {
+		r.Recorder.Eventf(machineScope.AWSMachine, corev1.EventTypeWarning, "FailedAttachControlPlaneELB",
+			"Cannot register control plane instance %q with load balancer: instance is not running", instance.ID)
+		conditions.MarkFalse(machineScope.AWSMachine, infrav1.ELBAttachedCondition, infrav1.ELBAttachFailedReason, clusterv1.ConditionSeverityInfo, "instance not running")
+		return elb.NewInstanceNotRunning("instance is not running")
+	}
+
 	if err := elbsvc.RegisterInstanceWithAPIServerLB(instance, lb); err != nil {
 		r.Recorder.Eventf(machineScope.AWSMachine, corev1.EventTypeWarning, "FailedAttachControlPlaneELB",
 			"Failed to register control plane instance %q with load balancer: %v", instance.ID, err)

pkg/cloud/services/elb/errors.go

@@ -56,6 +56,15 @@ func NewConflict(msg string) error {
 	}
 }
 
+// NewInstanceNotRunning returns an error which indicates that the request cannot be processed due to the instance not
+// being in a running state.
+func NewInstanceNotRunning(msg string) error {
+	return &ELBError{
+		msg:  msg,
+		Code: http.StatusTooEarly,
+	}
+}
+
 // IsNotFound returns true if the error was created by NewNotFound.
 func IsNotFound(err error) bool {
 	if ReasonForError(err) == http.StatusNotFound {
@@ -90,6 +99,11 @@ func IsSDKError(err error) (ok bool) {
 	return
 }
 
+// IsInstanceNotRunning returns true if the error was created by NewInstanceNotRunning.
+func IsInstanceNotRunning(err error) (ok bool) {
+	return ReasonForError(err) == http.StatusTooEarly
+}
+
 // ReasonForError returns the HTTP status for a particular error.
 func ReasonForError(err error) int {
 	if t, ok := errors.Cause(err).(*ELBError); ok {