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.

Sign up for GitHub

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

Basic E2E test for machine controller #80

Merged
merged 2 commits into from
Feb 8, 2024
Merged

Basic E2E test for machine controller #80

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
190f3fd
Basic E2E test for machine controller
Feb 6, 2024
7cbdc4c
Fix linode machine update
Feb 8, 2024
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
37 changes: 18 additions & 19 deletions controller/linodemachine_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ func (r *LinodeMachineReconciler) reconcile(

logger = logger.WithValues("ID", *machineScope.LinodeMachine.Spec.InstanceID)

res, err = r.reconcileUpdate(ctx, logger, machineScope)
res, linodeInstance, err = r.reconcileUpdate(ctx, logger, machineScope)

return
}
Expand All @@ -284,7 +284,7 @@ func (r *LinodeMachineReconciler) reconcile(

return ctrl.Result{RequeueAfter: reconciler.DefaultMachineControllerWaitForBootstrapDelay}, nil
}
err = r.reconcileCreate(ctx, logger, machineScope, clusterScope)
linodeInstance, err = r.reconcileCreate(ctx, logger, machineScope, clusterScope)

return
}
Expand All @@ -294,7 +294,7 @@ func (r *LinodeMachineReconciler) reconcileCreate(
logger logr.Logger,
machineScope *scope.MachineScope,
clusterScope *scope.ClusterScope,
) error {
) (*linodego.Instance, error) {
Copy link
Contributor Author

@mhmxs mhmxs Feb 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AshleyDumaine there is a defer function which updates the status based on the linode instance status, so in this case we need the instance itself. This changes fixes: c2b2378#diff-8daab1045cecf480419b0a31c2f8558e51197d31a7be910386ef4e420db0f7a4

logger.Info("creating machine")

tags := []string{string(machineScope.LinodeCluster.UID), string(machineScope.LinodeMachine.UID)}
Expand All @@ -303,7 +303,7 @@ func (r *LinodeMachineReconciler) reconcileCreate(
if err != nil {
logger.Error(err, "Failed to list Linode machine instances")

return err
return nil, err
}

var linodeInstance *linodego.Instance
Expand All @@ -318,15 +318,15 @@ func (r *LinodeMachineReconciler) reconcileCreate(
if err != nil {
logger.Error(err, "Failed to create Linode machine create config")

return err
return nil, err
}

if machineScope.LinodeCluster.Spec.VPCRef != nil {
iface, err := r.getVPCInterfaceConfig(ctx, machineScope, createConfig.Interfaces, logger)
if err != nil {
logger.Error(err, "Failed to get VPC interface confiog")

return err
return nil, err
}

createConfig.Interfaces = append(createConfig.Interfaces, *iface)
Expand All @@ -336,22 +336,22 @@ func (r *LinodeMachineReconciler) reconcileCreate(
if err != nil {
logger.Error(err, "Failed to create Linode machine instance")

return err
return nil, err
}
default:
err = errors.New("multiple instances")

logger.Error(err, "Panic! Multiple instances found. This might be a concurrency issue in the controller!!!", "tags", tags)

return err
return nil, err
}

if linodeInstance == nil {
err = errors.New("missing instance")

logger.Error(err, "Panic! Failed to create isntance")

return err
return nil, err
}

machineScope.LinodeMachine.Status.Ready = true
Expand All @@ -369,26 +369,25 @@ func (r *LinodeMachineReconciler) reconcileCreate(
if err = services.AddNodeToNB(ctx, logger, machineScope, clusterScope); err != nil {
logger.Error(err, "Failed to add instance to Node Balancer backend")

return err
return linodeInstance, err
}

return nil
return linodeInstance, nil
}

func (r *LinodeMachineReconciler) reconcileUpdate(
ctx context.Context,
logger logr.Logger,
machineScope *scope.MachineScope,
) (res reconcile.Result, err error) {
) (res reconcile.Result, linodeInstance *linodego.Instance, err error) {
logger.Info("updating machine")

res = ctrl.Result{}

if machineScope.LinodeMachine.Spec.InstanceID == nil {
return res, errors.New("missing instance ID")
return res, nil, errors.New("missing instance ID")
}

var linodeInstance *linodego.Instance
if linodeInstance, err = machineScope.LinodeClient.GetInstance(ctx, *machineScope.LinodeMachine.Spec.InstanceID); err != nil {
err = util.IgnoreLinodeAPIError(err, http.StatusNotFound)
if err != nil {
Expand All @@ -403,27 +402,27 @@ func (r *LinodeMachineReconciler) reconcileUpdate(
conditions.MarkFalse(machineScope.LinodeMachine, clusterv1.ReadyCondition, "missing", clusterv1.ConditionSeverityWarning, "instance not found")
}

return res, err
return res, nil, err
}

if _, ok := requeueInstanceStatuses[linodeInstance.Status]; ok {
if linodeInstance.Updated.Add(reconciler.DefaultMachineControllerWaitForRunningTimeout).After(time.Now()) {
logger.Info("Instance has one operaton running, re-queuing reconciliation", "status", linodeInstance.Status)

return ctrl.Result{RequeueAfter: reconciler.DefaultMachineControllerWaitForRunningDelay}, nil
return ctrl.Result{RequeueAfter: reconciler.DefaultMachineControllerWaitForRunningDelay}, linodeInstance, nil
}

logger.Info("Instance has one operaton long running, skipping reconciliation", "status", linodeInstance.Status)

conditions.MarkFalse(machineScope.LinodeMachine, clusterv1.ReadyCondition, string(linodeInstance.Status), clusterv1.ConditionSeverityInfo, "skipped due to long running operation")

return res, nil
return res, linodeInstance, nil
} else if linodeInstance.Status != linodego.InstanceRunning {
logger.Info("Instance has incompatible status, skipping reconciliation", "status", linodeInstance.Status)

conditions.MarkFalse(machineScope.LinodeMachine, clusterv1.ReadyCondition, string(linodeInstance.Status), clusterv1.ConditionSeverityInfo, "incompatible status")

return res, nil
return res, linodeInstance, nil
}

machineScope.LinodeMachine.Status.Ready = true
Expand All @@ -432,7 +431,7 @@ func (r *LinodeMachineReconciler) reconcileUpdate(

r.Recorder.Event(machineScope.LinodeMachine, corev1.EventTypeNormal, string(clusterv1.ReadyCondition), "instance is running")

return res, nil
return res, linodeInstance, nil
}

func (r *LinodeMachineReconciler) reconcileDelete(
Expand Down