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.

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

Fix vm state bug and deletion bug #340

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 7 additions & 2 deletions cloud/scope/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,8 +301,8 @@ func (m *MachineScope) getFreeFormTags() map[string]string {
}

// DeleteMachine terminates the instance using InstanceId from the OCIMachine spec and deletes the boot volume
func (m *MachineScope) DeleteMachine(ctx context.Context) error {
req := core.TerminateInstanceRequest{InstanceId: m.OCIMachine.Spec.InstanceId,
func (m *MachineScope) DeleteMachine(ctx context.Context, instance *core.Instance) error {
req := core.TerminateInstanceRequest{InstanceId: instance.Id,
PreserveBootVolume: common.Bool(false)}
_, err := m.ComputeClient.TerminateInstance(ctx, req)
return err
Expand Down Expand Up @@ -405,6 +405,11 @@ func (m *MachineScope) SetReady() {
m.OCIMachine.Status.Ready = true
}

// SetNotReady sets the OCIMachine Ready Status to false.
func (m *MachineScope) SetNotReady() {
m.OCIMachine.Status.Ready = false
}

// IsReady returns the ready status of the machine.
func (m *MachineScope) IsReady() bool {
return m.OCIMachine.Status.Ready
Expand Down
9 changes: 8 additions & 1 deletion cloud/scope/machine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2393,6 +2393,7 @@ func TestInstanceDeletion(t *testing.T) {
expectedEvent string
eventNotExpected string
matchError error
instance *core.Instance
errorSubStringMatch bool
testSpecificSetup func(machineScope *MachineScope, computeClient *mock_compute.MockComputeClient)
}{
Expand All @@ -2406,6 +2407,9 @@ func TestInstanceDeletion(t *testing.T) {
PreserveBootVolume: common.Bool(false),
})).Return(core.TerminateInstanceResponse{}, nil)
},
instance: &core.Instance{
Id: common.String("test"),
},
},
{
name: "delete instance error",
Expand All @@ -2418,6 +2422,9 @@ func TestInstanceDeletion(t *testing.T) {
PreserveBootVolume: common.Bool(false),
})).Return(core.TerminateInstanceResponse{}, errors.New("could not terminate instance"))
},
instance: &core.Instance{
Id: common.String("test"),
},
},
}

Expand All @@ -2427,7 +2434,7 @@ func TestInstanceDeletion(t *testing.T) {
defer teardown(t, g)
setup(t, g)
tc.testSpecificSetup(ms, computeClient)
err := ms.DeleteMachine(context.Background())
err := ms.DeleteMachine(context.Background(), tc.instance)
if tc.errorExpected {
g.Expect(err).To(Not(BeNil()))
if tc.errorSubStringMatch {
Expand Down
7 changes: 6 additions & 1 deletion controllers/ocimachine_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,10 @@ func (r *OCIMachineReconciler) reconcileNormal(ctx context.Context, logger logr.
machineScope.Info("Instance is pending")
conditions.MarkFalse(machineScope.OCIMachine, infrastructurev1beta2.InstanceReadyCondition, infrastructurev1beta2.InstanceNotReadyReason, clusterv1.ConditionSeverityInfo, "")
return reconcile.Result{RequeueAfter: 10 * time.Second}, nil
case core.InstanceLifecycleStateStopping, core.InstanceLifecycleStateStopped:
machineScope.SetNotReady()
shyamradhakrishnan marked this conversation as resolved.
Show resolved Hide resolved
conditions.MarkFalse(machineScope.OCIMachine, infrastructurev1beta2.InstanceReadyCondition, infrastructurev1beta2.InstanceNotReadyReason, clusterv1.ConditionSeverityInfo, "")
return reconcile.Result{}, nil
case core.InstanceLifecycleStateRunning:
machineScope.Info("Instance is active")
if machine.Status.Addresses == nil || len(machine.Status.Addresses) == 0 {
Expand Down Expand Up @@ -387,6 +391,7 @@ func (r *OCIMachineReconciler) reconcileNormal(ctx context.Context, logger logr.
}
fallthrough
default:
machineScope.SetNotReady()
conditions.MarkFalse(machineScope.OCIMachine, infrastructurev1beta2.InstanceReadyCondition, infrastructurev1beta2.InstanceProvisionFailedReason, clusterv1.ConditionSeverityError, "")
machineScope.SetFailureReason(capierrors.CreateMachineError)
machineScope.SetFailureMessage(errors.Errorf("Instance status %q is unexpected", instance.LifecycleState))
Expand Down Expand Up @@ -446,7 +451,7 @@ func (r *OCIMachineReconciler) reconcileDelete(ctx context.Context, machineScope
if err != nil {
return reconcile.Result{}, err
}
if err := machineScope.DeleteMachine(ctx); err != nil {
if err := machineScope.DeleteMachine(ctx, instance); err != nil {
machineScope.Error(err, "Error deleting Instance")
return ctrl.Result{}, errors.Wrapf(err, "error deleting instance %s", machineScope.Name())
}
Expand Down
32 changes: 32 additions & 0 deletions controllers/ocimachine_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,38 @@ func TestNormalReconciliationFunction(t *testing.T) {
g.Expect(result.RequeueAfter).To(Equal(300 * time.Second))
},
},
{
name: "instance in stopped state",
errorExpected: false,
conditionAssertion: []conditionAssertion{{infrastructurev1beta2.InstanceReadyCondition, corev1.ConditionFalse, clusterv1.ConditionSeverityInfo, infrastructurev1beta2.InstanceNotReadyReason}},
testSpecificSetup: func(t *test, machineScope *scope.MachineScope, computeClient *mock_compute.MockComputeClient, vcnClient *mock_vcn.MockClient, nlbclient *mock_nlb.MockNetworkLoadBalancerClient) {
computeClient.EXPECT().GetInstance(gomock.Any(), gomock.Eq(core.GetInstanceRequest{
InstanceId: common.String("test"),
})).
Return(core.GetInstanceResponse{
Instance: core.Instance{
Id: common.String("test"),
LifecycleState: core.InstanceLifecycleStateStopped,
},
}, nil)
},
},
{
name: "instance in stopping state",
errorExpected: false,
conditionAssertion: []conditionAssertion{{infrastructurev1beta2.InstanceReadyCondition, corev1.ConditionFalse, clusterv1.ConditionSeverityInfo, infrastructurev1beta2.InstanceNotReadyReason}},
testSpecificSetup: func(t *test, machineScope *scope.MachineScope, computeClient *mock_compute.MockComputeClient, vcnClient *mock_vcn.MockClient, nlbclient *mock_nlb.MockNetworkLoadBalancerClient) {
computeClient.EXPECT().GetInstance(gomock.Any(), gomock.Eq(core.GetInstanceRequest{
InstanceId: common.String("test"),
})).
Return(core.GetInstanceResponse{
Instance: core.Instance{
Id: common.String("test"),
LifecycleState: core.InstanceLifecycleStateStopping,
},
}, nil)
},
},
{
name: "instance in terminated state",
errorExpected: true,
Expand Down