Skip to content

Commit

Permalink
Merge pull request #1284 from srm09/deletion-of-orphaned-vspheremachines
Browse files Browse the repository at this point in the history 
🐛 Adds deletion logic for orphaned VSphereMachines
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Nov 2, 2021
2 parents 245be99 + a5ba8fe commit 75d28b7
Show file tree
Hide file tree
Showing 2 changed files with 190 additions and 5 deletions.
66 changes: 63 additions & 3 deletions controllers/vspherecluster_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
kerrors "k8s.io/apimachinery/pkg/util/errors"
"k8s.io/apimachinery/pkg/util/wait"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
clusterutilv1 "sigs.k8s.io/cluster-api/util"
Expand Down Expand Up @@ -201,6 +202,10 @@ func (r clusterReconciler) Reconcile(ctx goctx.Context, req ctrl.Request) (_ ctr
}
}()

if err := setOwnerRefsOnVsphereMachines(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(clusterContext)
Expand All @@ -213,14 +218,38 @@ func (r clusterReconciler) Reconcile(ctx goctx.Context, req ctrl.Request) (_ ctr
func (r clusterReconciler) reconcileDelete(ctx *context.ClusterContext) (reconcile.Result, error) {
ctx.Logger.Info("Reconciling VSphereCluster delete")

vsphereMachines, err := infrautilv1.GetVSphereMachinesInCluster(ctx, ctx.Client, ctx.VSphereCluster.Namespace, ctx.VSphereCluster.Name)
vsphereMachines, err := infrautilv1.GetVSphereMachinesInCluster(ctx, ctx.Client, ctx.Cluster.Namespace, ctx.Cluster.Name)
if err != nil {
return reconcile.Result{}, errors.Wrapf(err,
"unable to list VSphereMachines part of VSphereCluster %s/%s", ctx.VSphereCluster.Namespace, ctx.VSphereCluster.Name)
}

if len(vsphereMachines) > 0 {
ctx.Logger.Info("Waiting for VSphereMachines to be deleted", "count", len(vsphereMachines))
machineDeletionCount := 0
var deletionErrors []error
for _, vsphereMachine := range vsphereMachines {
// 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, ctx.VSphereCluster) && len(vsphereMachine.OwnerReferences) == 1 {
machineDeletionCount++
// Remove the finalizer since VM creation wouldn't proceed
r.Logger.Info("Removing finalizer from VSphereMachine", "namespace", vsphereMachine.Namespace, "name", vsphereMachine.Name)
ctrlutil.RemoveFinalizer(vsphereMachine, infrav1.MachineFinalizer)
if err := r.Client.Update(ctx, vsphereMachine); err != nil {
return reconcile.Result{}, err
}
if err := r.Client.Delete(ctx, vsphereMachine); err != nil && !apierrors.IsNotFound(err) {
ctx.Logger.Error(err, "Failed to delete for VSphereMachine", "namespace", vsphereMachine.Namespace, "name", vsphereMachine.Name)
deletionErrors = append(deletionErrors, err)
}
}
}
if len(deletionErrors) > 0 {
return reconcile.Result{}, kerrors.NewAggregate(deletionErrors)
}

if len(vsphereMachines)-machineDeletionCount > 0 {
ctx.Logger.Info("Waiting for VSphereMachines to be deleted", "count", len(vsphereMachines)-machineDeletionCount)
return reconcile.Result{RequeueAfter: 10 * time.Second}, nil
}

Expand Down Expand Up @@ -498,6 +527,37 @@ func (r clusterReconciler) isControlPlaneInitialized(ctx *context.ClusterContext
return conditions.IsTrue(ctx.Cluster, clusterv1.ControlPlaneInitializedCondition)
}

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

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

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

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

// controlPlaneMachineToCluster is a handler.ToRequestsFunc to be used
// to enqueue requests for reconciliation for VSphereCluster to update
// its status.apiEndpoints field.
Expand Down
129 changes: 127 additions & 2 deletions controllers/vspherecluster_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/utils/pointer"
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
clusterutil1v1 "sigs.k8s.io/cluster-api/util"
"sigs.k8s.io/cluster-api/util/conditions"
"sigs.k8s.io/cluster-api/util/patch"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -162,7 +163,7 @@ var _ = Describe("ClusterReconciler", func() {
},
Spec: clusterv1.ClusterSpec{
InfrastructureRef: &corev1.ObjectReference{
APIVersion: "infrastructure.cluster.x-k8s.io/v1alpha4",
APIVersion: infrav1.GroupVersion.String(),
Kind: "VsphereCluster",
Name: "vsphere-test1",
},
Expand Down Expand Up @@ -247,7 +248,7 @@ var _ = Describe("ClusterReconciler", func() {
},
Spec: clusterv1.ClusterSpec{
InfrastructureRef: &corev1.ObjectReference{
APIVersion: "infrastructure.cluster.x-k8s.io/v1alpha4",
APIVersion: infrav1.GroupVersion.String(),
Kind: "VsphereCluster",
Name: "vsphere-test1",
},
Expand Down Expand Up @@ -385,8 +386,132 @@ var _ = Describe("ClusterReconciler", func() {
}, timeout).Should(BeTrue())
})
})

Context("For VSphereMachines belonging to the cluster", func() {

var (
namespace string
testNs *corev1.Namespace
)

BeforeEach(func() {
var err error
testNs, err = testEnv.CreateNamespace(ctx, "vsm-owner-ref")
Expect(err).NotTo(HaveOccurred())
namespace = testNs.Name
})

AfterEach(func() {
Expect(testEnv.Delete(ctx, testNs)).To(Succeed())
})

It("sets owner references to those machines", func() {
// Create the VSphereCluster object
instance := &infrav1.VSphereCluster{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "test1-",
Namespace: namespace,
},
Spec: infrav1.VSphereClusterSpec{
Server: testEnv.Simulator.ServerURL().Host,
},
}
Expect(testEnv.Create(ctx, instance)).To(Succeed())

capiCluster := &clusterv1.Cluster{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "capi-test1-",
Namespace: namespace,
},
Spec: clusterv1.ClusterSpec{
InfrastructureRef: &corev1.ObjectReference{
APIVersion: infrav1.GroupVersion.String(),
Kind: "VsphereCluster",
Name: instance.Name,
},
},
}
Expect(testEnv.Create(ctx, capiCluster)).To(Succeed())

// Make sure the VSphereCluster exists.
key := client.ObjectKey{Namespace: namespace, Name: instance.Name}
Eventually(func() error {
return testEnv.Get(ctx, key, instance)
}, timeout).Should(BeNil())

machineCount := 3
for i := 0; i < machineCount; i++ {
Expect(createVsphereMachine(ctx, testEnv, namespace, capiCluster.Name)).To(Succeed())
}

Eventually(func() bool {
ph, err := patch.NewHelper(instance, testEnv)
Expect(err).ShouldNot(HaveOccurred())
instance.OwnerReferences = append(instance.OwnerReferences, metav1.OwnerReference{
Kind: "Cluster",
APIVersion: clusterv1.GroupVersion.String(),
Name: capiCluster.Name,
UID: "blah",
})
Expect(ph.Patch(ctx, instance, patch.WithStatusObservedGeneration{})).ShouldNot(HaveOccurred())
return true
}, timeout).Should(BeTrue())

By("checking for presence of VSphereMachine objects")
Eventually(func() int {
machines := &infrav1.VSphereMachineList{}
if err := testEnv.List(ctx, machines, client.InNamespace(namespace),
client.MatchingLabels(map[string]string{clusterv1.ClusterLabelName: capiCluster.Name})); err != nil {
return -1
}
return len(machines.Items)
}, timeout).Should(Equal(machineCount))

By("checking VSphereMachine owner refs")
Eventually(func() int {
machines := &infrav1.VSphereMachineList{}
if err := testEnv.List(ctx, machines, client.InNamespace(namespace),
client.MatchingLabels(map[string]string{clusterv1.ClusterLabelName: capiCluster.Name})); err != nil {
return 0
}
ownerRefSet := 0
for _, m := range machines.Items {
if len(m.OwnerReferences) >= 1 && clusterutil1v1.HasOwnerRef(m.OwnerReferences, metav1.OwnerReference{
APIVersion: infrav1.GroupVersion.String(),
Kind: instance.Kind,
Name: instance.Name,
UID: instance.UID,
}) {
ownerRefSet++
}
}
return ownerRefSet
}, timeout).Should(Equal(machineCount))
})
})
})

func createVsphereMachine(ctx context.Context, env *helpers.TestEnvironment, namespace, clusterName string) error {
vsphereMachine := &infrav1.VSphereMachine{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "test-vsp",
Namespace: namespace,
Labels: map[string]string{clusterv1.ClusterLabelName: clusterName},
},
Spec: infrav1.VSphereMachineSpec{
VirtualMachineCloneSpec: infrav1.VirtualMachineCloneSpec{
Template: "ubuntu-k9s-1.19",
Network: infrav1.NetworkSpec{
Devices: []infrav1.NetworkDeviceSpec{
{NetworkName: "network-1", DHCP4: true},
},
},
},
},
}
return env.Create(ctx, vsphereMachine)
}

func TestClusterReconciler_ReconcileDeploymentZones(t *testing.T) {
server := "vcenter123.foo.com"
g := NewWithT(t)
Expand Down

0 comments on commit 75d28b7

Please sign in to comment.