Skip to content

Commit 2841d65

Browse files
shyamradhakrishnanshyamradhakrishnan
committed
Add machine reconcile delete on termination (#336)
* Add support for machine deletion after instance termination
1 parent d21a7df commit 2841d65

File tree

3 files changed

+102
-15
lines changed

3 files changed

+102
-15
lines changed

‎api/v1beta2/ocimachine_types.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@ import (
2828
const (
2929
// MachineFinalizer allows ReconcileMachine to clean up OCI resources associated with OCIMachine before
3030
// removing it from the apiserver.
31-
MachineFinalizer = "ocimachine.infrastructure.cluster.x-k8s.io"
31+
MachineFinalizer = "ocimachine.infrastructure.cluster.x-k8s.io"
32+
DeleteMachineOnInstanceTermination = "ociclusters.x-k8s.io/delete-machine-on-instance-termination"
3233
)
3334

3435
// OCIMachineSpec defines the desired state of OCIMachine

‎controllers/ocimachine_controller.go‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,13 @@ func (r *OCIMachineReconciler) OCIClusterToOCIMachines() handler.MapFunc {
245245
func (r *OCIMachineReconciler) reconcileNormal(ctx context.Context, logger logr.Logger, machineScope *scope.MachineScope) (ctrl.Result, error) {
246246
controllerutil.AddFinalizer(machineScope.OCIMachine, infrastructurev1beta2.MachineFinalizer)
247247
machine := machineScope.OCIMachine
248+
infraMachine := machineScope.Machine
249+
250+
annotations := infraMachine.GetAnnotations()
251+
deleteMachineOnTermination := false
252+
if annotations != nil {
253+
_, deleteMachineOnTermination = annotations[infrastructurev1beta2.DeleteMachineOnInstanceTermination]
254+
}
248255
// Make sure bootstrap data is available and populated.
249256
if machineScope.Machine.Spec.Bootstrap.DataSecretName == nil {
250257
r.Recorder.Event(machine, corev1.EventTypeNormal, infrastructurev1beta2.WaitingForBootstrapDataReason, "Bootstrap data secret reference is not yet available")
@@ -316,7 +323,21 @@ func (r *OCIMachineReconciler) reconcileNormal(ctx context.Context, logger logr.
316323
"Instance is in ready state")
317324
conditions.MarkTrue(machineScope.OCIMachine, infrastructurev1beta2.InstanceReadyCondition)
318325
machineScope.SetReady()
319-
return reconcile.Result{}, nil
326+
if deleteMachineOnTermination {
327+
// typically, if the VM is terminated, we should get machine events, so ideally, the 300 seconds
328+
// requeue time is not required, but in case, the event is missed, adding the requeue time
329+
return reconcile.Result{RequeueAfter: 300 * time.Second}, nil
330+
} else {
331+
return reconcile.Result{}, nil
332+
}
333+
case core.InstanceLifecycleStateTerminated:
334+
if deleteMachineOnTermination && infraMachine.DeletionTimestamp == nil {
335+
logger.Info("Deleting underlying machine as instance is terminated")
336+
if err := machineScope.Client.Delete(ctx, infraMachine); err != nil {
337+
return reconcile.Result{}, errors.Wrapf(err, "failed to delete machine %s/%s", infraMachine.Namespace, infraMachine.Name)
338+
}
339+
}
340+
fallthrough
320341
default:
321342
conditions.MarkFalse(machineScope.OCIMachine, infrastructurev1beta2.InstanceReadyCondition, infrastructurev1beta2.InstanceProvisionFailedReason, clusterv1.ConditionSeverityError, "")
322343
machineScope.SetFailureReason(capierrors.CreateMachineError)

‎controllers/ocimachine_controller_test.go‎

Lines changed: 78 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import (
4040
"k8s.io/client-go/tools/record"
4141
clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
4242
"sigs.k8s.io/cluster-api/util/conditions"
43+
ctrl "sigs.k8s.io/controller-runtime"
4344
"sigs.k8s.io/controller-runtime/pkg/client"
4445
"sigs.k8s.io/controller-runtime/pkg/client/fake"
4546
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
@@ -196,19 +197,22 @@ func TestNormalReconciliationFunction(t *testing.T) {
196197
teardown := func(t *testing.T, g *WithT) {
197198
mockCtrl.Finish()
198199
}
199-
tests := []struct {
200+
type test struct {
200201
name string
201202
errorExpected bool
202203
expectedEvent string
203204
eventNotExpected string
204205
conditionAssertion []conditionAssertion
205-
testSpecificSetup func(machineScope *scope.MachineScope, computeClient *mock_compute.MockComputeClient, vcnClient *mock_vcn.MockClient, nlbclient *mock_nlb.MockNetworkLoadBalancerClient)
206-
}{
206+
deleteMachines []clusterv1.Machine
207+
testSpecificSetup func(t *test, machineScope *scope.MachineScope, computeClient *mock_compute.MockComputeClient, vcnClient *mock_vcn.MockClient, nlbclient *mock_nlb.MockNetworkLoadBalancerClient)
208+
validate func(g *WithT, t *test, result ctrl.Result)
209+
}
210+
tests := []test{
207211
{
208212
name: "instance in provisioning state",
209213
errorExpected: false,
210214
conditionAssertion: []conditionAssertion{{infrastructurev1beta2.InstanceReadyCondition, corev1.ConditionFalse, clusterv1.ConditionSeverityInfo, infrastructurev1beta2.InstanceNotReadyReason}},
211-
testSpecificSetup: func(machineScope *scope.MachineScope, computeClient *mock_compute.MockComputeClient, vcnClient *mock_vcn.MockClient, nlbclient *mock_nlb.MockNetworkLoadBalancerClient) {
215+
testSpecificSetup: func(t *test, machineScope *scope.MachineScope, computeClient *mock_compute.MockComputeClient, vcnClient *mock_vcn.MockClient, nlbclient *mock_nlb.MockNetworkLoadBalancerClient) {
212216
computeClient.EXPECT().GetInstance(gomock.Any(), gomock.Eq(core.GetInstanceRequest{
213217
InstanceId: common.String("test"),
214218
})).
@@ -224,7 +228,59 @@ func TestNormalReconciliationFunction(t *testing.T) {
224228
name: "instance in running state",
225229
errorExpected: false,
226230
conditionAssertion: []conditionAssertion{{infrastructurev1beta2.InstanceReadyCondition, corev1.ConditionTrue, "", ""}},
227-
testSpecificSetup: func(machineScope *scope.MachineScope, computeClient *mock_compute.MockComputeClient, vcnClient *mock_vcn.MockClient, nlbclient *mock_nlb.MockNetworkLoadBalancerClient) {
231+
testSpecificSetup: func(t *test, machineScope *scope.MachineScope, computeClient *mock_compute.MockComputeClient, vcnClient *mock_vcn.MockClient, nlbclient *mock_nlb.MockNetworkLoadBalancerClient) {
232+
machineScope.OCIMachine.Status.Addresses = []clusterv1.MachineAddress{
233+
{
234+
Type: clusterv1.MachineInternalIP,
235+
Address: "1.1.1.1",
236+
},
237+
}
238+
computeClient.EXPECT().GetInstance(gomock.Any(), gomock.Eq(core.GetInstanceRequest{
239+
InstanceId: common.String("test"),
240+
})).
241+
Return(core.GetInstanceResponse{
242+
Instance: core.Instance{
243+
Id: common.String("test"),
244+
LifecycleState: core.InstanceLifecycleStateRunning,
245+
},
246+
}, nil)
247+
},
248+
},
249+
{
250+
name: "instance in running state, reconcile every 5 minutes",
251+
errorExpected: false,
252+
conditionAssertion: []conditionAssertion{{infrastructurev1beta2.InstanceReadyCondition, corev1.ConditionTrue, "", ""}},
253+
testSpecificSetup: func(t *test, machineScope *scope.MachineScope, computeClient *mock_compute.MockComputeClient, vcnClient *mock_vcn.MockClient, nlbclient *mock_nlb.MockNetworkLoadBalancerClient) {
254+
if machineScope.Machine.ObjectMeta.Annotations == nil {
255+
machineScope.Machine.ObjectMeta.Annotations = make(map[string]string)
256+
}
257+
machineScope.Machine.ObjectMeta.Annotations[infrastructurev1beta2.DeleteMachineOnInstanceTermination] = ""
258+
machineScope.OCIMachine.Status.Addresses = []clusterv1.MachineAddress{
259+
{
260+
Type: clusterv1.MachineInternalIP,
261+
Address: "1.1.1.1",
262+
},
263+
}
264+
computeClient.EXPECT().GetInstance(gomock.Any(), gomock.Eq(core.GetInstanceRequest{
265+
InstanceId: common.String("test"),
266+
})).
267+
Return(core.GetInstanceResponse{
268+
Instance: core.Instance{
269+
Id: common.String("test"),
270+
LifecycleState: core.InstanceLifecycleStateRunning,
271+
},
272+
}, nil)
273+
},
274+
},
275+
{
276+
name: "instance in running state, reconcile every 5 minutes",
277+
errorExpected: false,
278+
conditionAssertion: []conditionAssertion{{infrastructurev1beta2.InstanceReadyCondition, corev1.ConditionTrue, "", ""}},
279+
testSpecificSetup: func(t *test, machineScope *scope.MachineScope, computeClient *mock_compute.MockComputeClient, vcnClient *mock_vcn.MockClient, nlbclient *mock_nlb.MockNetworkLoadBalancerClient) {
280+
if machineScope.Machine.ObjectMeta.Annotations == nil {
281+
machineScope.Machine.ObjectMeta.Annotations = make(map[string]string)
282+
}
283+
machineScope.Machine.ObjectMeta.Annotations[infrastructurev1beta2.DeleteMachineOnInstanceTermination] = ""
228284
machineScope.OCIMachine.Status.Addresses = []clusterv1.MachineAddress{
229285
{
230286
Type: clusterv1.MachineInternalIP,
@@ -241,13 +297,16 @@ func TestNormalReconciliationFunction(t *testing.T) {
241297
},
242298
}, nil)
243299
},
300+
validate: func(g *WithT, t *test, result ctrl.Result) {
301+
g.Expect(result.RequeueAfter).To(Equal(300 * time.Second))
302+
},
244303
},
245304
{
246305
name: "instance in terminated state",
247306
errorExpected: true,
248307
expectedEvent: "invalid lifecycle state TERMINATED",
249308
conditionAssertion: []conditionAssertion{{infrastructurev1beta2.InstanceReadyCondition, corev1.ConditionFalse, clusterv1.ConditionSeverityError, infrastructurev1beta2.InstanceProvisionFailedReason}},
250-
testSpecificSetup: func(machineScope *scope.MachineScope, computeClient *mock_compute.MockComputeClient, vcnClient *mock_vcn.MockClient, nlbclient *mock_nlb.MockNetworkLoadBalancerClient) {
309+
testSpecificSetup: func(t *test, machineScope *scope.MachineScope, computeClient *mock_compute.MockComputeClient, vcnClient *mock_vcn.MockClient, nlbclient *mock_nlb.MockNetworkLoadBalancerClient) {
251310
computeClient.EXPECT().GetInstance(gomock.Any(), gomock.Eq(core.GetInstanceRequest{
252311
InstanceId: common.String("test"),
253312
})).
@@ -258,13 +317,16 @@ func TestNormalReconciliationFunction(t *testing.T) {
258317
},
259318
}, nil)
260319
},
320+
validate: func(g *WithT, t *test, result ctrl.Result) {
321+
g.Expect(result.RequeueAfter).To(Equal(0 * time.Second))
322+
},
261323
},
262324
{
263325
name: "control plane backend vnic attachments call error",
264326
errorExpected: true,
265327
expectedEvent: "server error",
266328
conditionAssertion: []conditionAssertion{{infrastructurev1beta2.InstanceReadyCondition, corev1.ConditionFalse, clusterv1.ConditionSeverityError, infrastructurev1beta2.InstanceIPAddressNotFound}},
267-
testSpecificSetup: func(machineScope *scope.MachineScope, computeClient *mock_compute.MockComputeClient, vcnClient *mock_vcn.MockClient, nlbclient *mock_nlb.MockNetworkLoadBalancerClient) {
329+
testSpecificSetup: func(t *test, machineScope *scope.MachineScope, computeClient *mock_compute.MockComputeClient, vcnClient *mock_vcn.MockClient, nlbclient *mock_nlb.MockNetworkLoadBalancerClient) {
268330
machineScope.Machine.ObjectMeta.Labels = make(map[string]string)
269331
machineScope.Machine.ObjectMeta.Labels[clusterv1.MachineControlPlaneLabel] = "true"
270332
computeClient.EXPECT().GetInstance(gomock.Any(), gomock.Eq(core.GetInstanceRequest{
@@ -287,7 +349,7 @@ func TestNormalReconciliationFunction(t *testing.T) {
287349
{
288350
name: "control plane backend backend exists",
289351
errorExpected: false,
290-
testSpecificSetup: func(machineScope *scope.MachineScope, computeClient *mock_compute.MockComputeClient, vcnClient *mock_vcn.MockClient, nlbClient *mock_nlb.MockNetworkLoadBalancerClient) {
352+
testSpecificSetup: func(t *test, machineScope *scope.MachineScope, computeClient *mock_compute.MockComputeClient, vcnClient *mock_vcn.MockClient, nlbClient *mock_nlb.MockNetworkLoadBalancerClient) {
291353
machineScope.Machine.ObjectMeta.Labels = make(map[string]string)
292354
machineScope.Machine.ObjectMeta.Labels[clusterv1.MachineControlPlaneLabel] = "true"
293355
computeClient.EXPECT().GetInstance(gomock.Any(), gomock.Eq(core.GetInstanceRequest{
@@ -344,7 +406,7 @@ func TestNormalReconciliationFunction(t *testing.T) {
344406
{
345407
name: "backend creation",
346408
errorExpected: false,
347-
testSpecificSetup: func(machineScope *scope.MachineScope, computeClient *mock_compute.MockComputeClient, vcnClient *mock_vcn.MockClient, nlbClient *mock_nlb.MockNetworkLoadBalancerClient) {
409+
testSpecificSetup: func(t *test, machineScope *scope.MachineScope, computeClient *mock_compute.MockComputeClient, vcnClient *mock_vcn.MockClient, nlbClient *mock_nlb.MockNetworkLoadBalancerClient) {
348410
machineScope.Machine.ObjectMeta.Labels = make(map[string]string)
349411
machineScope.Machine.ObjectMeta.Labels[clusterv1.MachineControlPlaneLabel] = "true"
350412
computeClient.EXPECT().GetInstance(gomock.Any(), gomock.Eq(core.GetInstanceRequest{
@@ -419,7 +481,7 @@ func TestNormalReconciliationFunction(t *testing.T) {
419481
{
420482
name: "ip address exists",
421483
errorExpected: false,
422-
testSpecificSetup: func(machineScope *scope.MachineScope, computeClient *mock_compute.MockComputeClient, vcnClient *mock_vcn.MockClient, nlbClient *mock_nlb.MockNetworkLoadBalancerClient) {
484+
testSpecificSetup: func(t *test, machineScope *scope.MachineScope, computeClient *mock_compute.MockComputeClient, vcnClient *mock_vcn.MockClient, nlbClient *mock_nlb.MockNetworkLoadBalancerClient) {
423485
machineScope.Machine.ObjectMeta.Labels = make(map[string]string)
424486
machineScope.OCIMachine.Status.Addresses = []clusterv1.MachineAddress{
425487
{
@@ -478,7 +540,7 @@ func TestNormalReconciliationFunction(t *testing.T) {
478540
{
479541
name: "backend creation fails",
480542
errorExpected: true,
481-
testSpecificSetup: func(machineScope *scope.MachineScope, computeClient *mock_compute.MockComputeClient, vcnClient *mock_vcn.MockClient, nlbClient *mock_nlb.MockNetworkLoadBalancerClient) {
543+
testSpecificSetup: func(t *test, machineScope *scope.MachineScope, computeClient *mock_compute.MockComputeClient, vcnClient *mock_vcn.MockClient, nlbClient *mock_nlb.MockNetworkLoadBalancerClient) {
482544
machineScope.Machine.ObjectMeta.Labels = make(map[string]string)
483545
machineScope.Machine.ObjectMeta.Labels[clusterv1.MachineControlPlaneLabel] = "true"
484546
computeClient.EXPECT().GetInstance(gomock.Any(), gomock.Eq(core.GetInstanceRequest{
@@ -557,9 +619,9 @@ func TestNormalReconciliationFunction(t *testing.T) {
557619
g := NewWithT(t)
558620
defer teardown(t, g)
559621
setup(t, g)
560-
tc.testSpecificSetup(ms, computeClient, vcnClient, nlbClient)
622+
tc.testSpecificSetup(&tc, ms, computeClient, vcnClient, nlbClient)
561623
ctx := context.Background()
562-
_, err := r.reconcileNormal(ctx, log.FromContext(ctx), ms)
624+
result, err := r.reconcileNormal(ctx, log.FromContext(ctx), ms)
563625
if len(tc.conditionAssertion) > 0 {
564626
expectConditions(g, ociMachine, tc.conditionAssertion)
565627
}
@@ -571,6 +633,9 @@ func TestNormalReconciliationFunction(t *testing.T) {
571633
if tc.expectedEvent != "" {
572634
g.Eventually(recorder.Events).Should(Receive(ContainSubstring(tc.expectedEvent)))
573635
}
636+
if tc.validate != nil {
637+
tc.validate(g, &tc, result)
638+
}
574639
})
575640
}
576641
}

0 commit comments

Comments
 (0)