Skip to content

Commit

Permalink
Remove handling for orphaned VSphereMachines
Browse files Browse the repository at this point in the history
Signed-off-by: killianmuldoon <[email protected]>
  • Loading branch information
killianmuldoon committed Oct 12, 2023
1 parent 6923f2b commit 93511ad
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 43 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -75,3 +75,4 @@ _releasenotes
*~
*.tmp
.DS_Store
*ginkgo-log.txt*
41 changes: 0 additions & 41 deletions controllers/vspherecluster_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,10 +121,6 @@ func (r *clusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_
}
}()

if err := r.setOwnerRefsOnVsphereMachines(ctx, clusterContext); err != nil {
return reconcile.Result{}, errors.Wrapf(err, "failed to set owner refs on VSphereMachine objects")
}

// Handle deleted clusters
if !vsphereCluster.DeletionTimestamp.IsZero() {
return r.reconcileDelete(ctx, clusterContext)
Expand Down Expand Up @@ -158,12 +154,6 @@ func (r *clusterReconciler) reconcileDelete(ctx context.Context, clusterCtx *cap
log := log.WithValues("VSphereMachine", klog.KObj(vsphereMachine))
ctx := ctrl.LoggerInto(ctx, log)

// If the VSphereMachine is not owned by the CAPI Machine object because the machine object was deleted
// before setting the owner references, then proceed with the deletion of the VSphereMachine object.
// This is required until CAPI has a solution for https://github.com/kubernetes-sigs/cluster-api/issues/5483
if !clusterutilv1.IsOwnedByObject(vsphereMachine, clusterCtx.VSphereCluster) || len(vsphereMachine.OwnerReferences) != 1 {
continue
}
machineDeletionCount++
// Remove the finalizer since VM creation wouldn't proceed
log.Info("Removing finalizer from VSphereMachine")
Expand Down Expand Up @@ -528,37 +518,6 @@ func (r *clusterReconciler) isControlPlaneInitialized(ctx context.Context, clust
return conditions.IsTrue(clusterCtx.Cluster, clusterv1.ControlPlaneInitializedCondition)
}

func (r *clusterReconciler) setOwnerRefsOnVsphereMachines(ctx context.Context, clusterCtx *capvcontext.ClusterContext) error {
vsphereMachines, err := infrautilv1.GetVSphereMachinesInCluster(ctx, r.Client, clusterCtx.Cluster.Namespace, clusterCtx.Cluster.Name)
if err != nil {
return errors.Wrapf(err,
"unable to list VSphereMachines part of VSphereCluster %s/%s", clusterCtx.VSphereCluster.Namespace, clusterCtx.VSphereCluster.Name)
}

var patchErrors []error
for _, vsphereMachine := range vsphereMachines {
patchHelper, err := patch.NewHelper(vsphereMachine, r.Client)
if err != nil {
patchErrors = append(patchErrors, err)
continue
}

vsphereMachine.SetOwnerReferences(clusterutilv1.EnsureOwnerRef(
vsphereMachine.OwnerReferences,
metav1.OwnerReference{
APIVersion: clusterCtx.VSphereCluster.APIVersion,
Kind: clusterCtx.VSphereCluster.Kind,
Name: clusterCtx.VSphereCluster.Name,
UID: clusterCtx.VSphereCluster.UID,
}))

if err := patchHelper.Patch(ctx, vsphereMachine); err != nil {
patchErrors = append(patchErrors, err)
}
}
return kerrors.NewAggregate(patchErrors)
}

func (r *clusterReconciler) reconcileClusterModules(ctx context.Context, clusterCtx *capvcontext.ClusterContext) (reconcile.Result, error) {
if feature.Gates.Enabled(feature.NodeAntiAffinity) {
return r.clusterModuleReconciler.Reconcile(ctx, clusterCtx)
Expand Down
24 changes: 23 additions & 1 deletion controllers/vspheremachine_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,11 +199,22 @@ func (r *machineReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_
log.Info("Waiting on Machine controller to set OwnerRef on VSphereMachine")
return reconcile.Result{}, nil
}

log = log.WithValues("Machine", klog.KObj(machine))
ctx = ctrl.LoggerInto(ctx, log)

cluster := r.fetchCAPICluster(ctx, machine, machineContext.GetVSphereMachine())

// TODO: Remove an ownerReference to the VSphereCluster if one exists. At this point we know the VSphereMachine has
// TODO: Do a nil check on the Cluster here.
// an owner CAPI Machine.
// if clusterutilv1.IsOwnedByObject()

if cluster != nil {
vsphereCluster, err := r.fetchVSphereCluster(ctx, cluster)

Check failure on line 213 in controllers/vspheremachine_controller.go

View workflow job for this annotation

GitHub Actions / lint

vsphereCluster declared and not used) (typecheck)

Check failure on line 213 in controllers/vspheremachine_controller.go

View workflow job for this annotation

GitHub Actions / lint

vsphereCluster declared and not used (typecheck)

Check failure on line 213 in controllers/vspheremachine_controller.go

View workflow job for this annotation

GitHub Actions / lint

vsphereCluster declared and not used) (typecheck)

Check failure on line 213 in controllers/vspheremachine_controller.go

View workflow job for this annotation

GitHub Actions / lint

vsphereCluster declared and not used) (typecheck)

Check failure on line 213 in controllers/vspheremachine_controller.go

View workflow job for this annotation

GitHub Actions / lint

vsphereCluster declared and not used (typecheck)

Check failure on line 213 in controllers/vspheremachine_controller.go

View workflow job for this annotation

GitHub Actions / coverage

vsphereCluster declared and not used

Check failure on line 213 in controllers/vspheremachine_controller.go

View workflow job for this annotation

GitHub Actions / coverage

vsphereCluster declared and not used

Check failure on line 213 in controllers/vspheremachine_controller.go

View workflow job for this annotation

GitHub Actions / lint

vsphereCluster declared and not used) (typecheck)

Check failure on line 213 in controllers/vspheremachine_controller.go

View workflow job for this annotation

GitHub Actions / lint

vsphereCluster declared and not used (typecheck)

Check failure on line 213 in controllers/vspheremachine_controller.go

View workflow job for this annotation

GitHub Actions / lint

vsphereCluster declared and not used) (typecheck)

Check failure on line 213 in controllers/vspheremachine_controller.go

View workflow job for this annotation

GitHub Actions / lint

vsphereCluster declared and not used) (typecheck)

Check failure on line 213 in controllers/vspheremachine_controller.go

View workflow job for this annotation

GitHub Actions / lint

vsphereCluster declared and not used (typecheck)
if err != nil {
return reconcile.Result{}, err
}
}
// Create the patch helper.
patchHelper, err := patch.NewHelper(machineContext.GetVSphereMachine(), r.Client)
if err != nil {
Expand Down Expand Up @@ -259,6 +270,17 @@ func (r *machineReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_
return r.reconcileNormal(ctx, machineContext)
}

func (r *machineReconciler) fetchVSphereCluster(ctx context.Context, cluster *clusterv1.Cluster) (*infrav1.VSphereCluster, error) {
vsphereCluster := &infrav1.VSphereCluster{}
if err := r.Client.Get(ctx, client.ObjectKey{
Namespace: cluster.Spec.InfrastructureRef.Namespace,
Name: cluster.Spec.InfrastructureRef.Name,
}, vsphereCluster); err != nil {
return nil, err
}
return vsphereCluster, nil
}

func (r *machineReconciler) reconcileDelete(ctx context.Context, machineCtx capvcontext.MachineContext) (reconcile.Result, error) {
log := ctrl.LoggerFrom(ctx)

Expand Down
2 changes: 1 addition & 1 deletion test/e2e/ownerreference_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ var (
},
"VSphereMachine": func(owners []metav1.OwnerReference) error {
// The vSphereCluster takes ownership of all vSphereMachines in addition to the core Machine.
return framework.HasExactOwners(owners, vSphereClusterOwner, machineController)
return framework.HasExactOwners(owners, machineController)
},
"VSphereMachineTemplate": func(owners []metav1.OwnerReference) error {
// The vSphereMachineTemplate can be owned by the Cluster or the ClusterClass.
Expand Down

0 comments on commit 93511ad

Please sign in to comment.