Skip to content

Commit

Permalink
Fix e2e test for dockermachinePool
Browse files Browse the repository at this point in the history
Signed-off-by: serngawy <[email protected]>
  • Loading branch information
serngawy committed Nov 18, 2024
1 parent cf5979e commit 0213ff1
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -146,13 +146,16 @@ func (r *DockerMachinePoolReconciler) reconcileDockerMachines(ctx context.Contex
if existingMachine, ok := dockerMachineMap[machine.Name()]; ok {
log.V(2).Info("Patching existing DockerMachine", "DockerMachine", klog.KObj(&existingMachine))
desiredMachine := computeDesiredDockerMachine(machine.Name(), cluster, machinePool, dockerMachinePool, &existingMachine)
if err := ssa.Patch(ctx, r.Client, dockerMachinePoolControllerName, desiredMachine, ssa.WithCachingProxy{Cache: r.ssaCache, Original: &existingMachine}); err != nil {
return errors.Wrapf(err, "failed to update DockerMachine %q", klog.KObj(desiredMachine))
// check if the dockerMachine not deleted before patching
if existingMachine.DeletionTimestamp.IsZero() {
if err := ssa.Patch(ctx, r.Client, dockerMachinePoolControllerName, desiredMachine, ssa.WithCachingProxy{Cache: r.ssaCache, Original: &existingMachine}); err != nil {
return errors.Wrapf(err, "failed to update DockerMachine %q", klog.KObj(desiredMachine))
}
}

dockerMachineMap[desiredMachine.Name] = *desiredMachine
} else {
log.V(2).Info("Creating a new DockerMachine for Docker container", "container", machine.Name())
log.Info("Creating a new DockerMachine for Docker container", "container", machine.Name())
desiredMachine := computeDesiredDockerMachine(machine.Name(), cluster, machinePool, dockerMachinePool, nil)
if err := ssa.Patch(ctx, r.Client, dockerMachinePoolControllerName, desiredMachine); err != nil {
return errors.Wrap(err, "failed to create a new docker machine")
Expand All @@ -169,7 +172,7 @@ func (r *DockerMachinePoolReconciler) reconcileDockerMachines(ctx context.Contex
for _, dockerMachine := range dockerMachineMap {
if _, ok := externalMachineMap[dockerMachine.Name]; !ok {
dockerMachine := dockerMachine
log.V(2).Info("Deleting DockerMachine with no underlying infrastructure", "DockerMachine", klog.KObj(&dockerMachine))
log.Info("Deleting DockerMachine with no underlying infrastructure", "DockerMachine", klog.KObj(&dockerMachine))
if err := r.deleteMachinePoolMachine(ctx, dockerMachine); err != nil {
return err
}
Expand Down Expand Up @@ -263,7 +266,7 @@ func computeDesiredDockerMachine(name string, cluster *clusterv1.Cluster, machin
if existingDockerMachine != nil {
dockerMachine.SetUID(existingDockerMachine.UID)
dockerMachine.SetOwnerReferences(existingDockerMachine.OwnerReferences)
}
}

// Note: Since the MachinePool controller has not created its owner Machine yet, we want to set the DockerMachinePool as the owner so it's not orphaned.
dockerMachine.SetOwnerReferences(util.EnsureOwnerRef(dockerMachine.OwnerReferences, metav1.OwnerReference{
Expand All @@ -272,6 +275,7 @@ func computeDesiredDockerMachine(name string, cluster *clusterv1.Cluster, machin
Name: dockerMachinePool.Name,
UID: dockerMachinePool.UID,
}))

dockerMachine.Labels[clusterv1.ClusterNameLabel] = cluster.Name
dockerMachine.Labels[clusterv1.MachinePoolNameLabel] = format.MustFormatValue(machinePool.Name)

Expand All @@ -288,7 +292,7 @@ func (r *DockerMachinePoolReconciler) deleteMachinePoolMachine(ctx context.Conte
}
// util.GetOwnerMachine() returns a nil Machine without error if there is no Machine kind in the ownerRefs, so we must verify that machine is not nil.
if machine == nil {
log.V(2).Info("No owner Machine exists for DockerMachine", "dockerMachine", klog.KObj(&dockerMachine))
log.Info("No owner Machine exists for DockerMachine", "dockerMachine", klog.KObj(&dockerMachine))

// If the DockerMachine does not have an owner Machine, do not attempt to delete the DockerMachine as the MachinePool controller will create the
// Machine and we want to let it catch up. If we are too hasty to delete, that introduces a race condition where the DockerMachine could be deleted
Expand All @@ -297,7 +301,8 @@ func (r *DockerMachinePoolReconciler) deleteMachinePoolMachine(ctx context.Conte
// In the case where the MachinePool is being deleted and the Machine will never come online, the DockerMachine will be deleted via its ownerRef to the
// DockerMachinePool, so that is covered as well.

return nil
// Returning error as we need the dockerMachine not to proceed.
return errors.New("No owner Machine exists for DockerMachine")
}

log.Info("Deleting Machine for DockerMachine", "Machine", klog.KObj(machine), "DockerMachine", klog.KObj(&dockerMachine))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ func (r *DockerMachineReconciler) Reconcile(ctx context.Context, req ctrl.Reques
}

// Handle deleted machines
if !dockerMachine.ObjectMeta.DeletionTimestamp.IsZero() {
if !dockerMachine.DeletionTimestamp.IsZero() {
return ctrl.Result{}, r.reconcileDelete(ctx, dockerCluster, machine, dockerMachine, externalMachine, externalLoadBalancer)
}

Expand Down Expand Up @@ -455,11 +455,11 @@ func (r *DockerMachineReconciler) reconcileDelete(ctx context.Context, dockerClu

// delete the machine
if err := externalMachine.Delete(ctx); err != nil {
return errors.Wrap(err, "failed to delete DockerMachine")
return errors.Wrap(err, "failed to delete external DockerMachine")
}

// if the deleted machine is a control-plane node, remove it from the load balancer configuration;
if util.IsControlPlaneMachine(machine) {
if machine != nil && util.IsControlPlaneMachine(machine) {
unsafeLoadBalancerConfigTemplate, err := r.getUnsafeLoadBalancerConfigTemplate(ctx, dockerCluster)
if err != nil {
return errors.Wrap(err, "failed to retrieve HAProxy configuration from CustomHAProxyConfigTemplateRef")
Expand Down

0 comments on commit 0213ff1

Please sign in to comment.