Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Jul 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions controllers/awsmachine_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down Expand Up @@ -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)
Expand Down
14 changes: 14 additions & 0 deletions pkg/cloud/services/elb/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down