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.

Sign up for GitHub

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

Support VMI eviction also for external infra clusters #242

Merged
merged 1 commit into from
Jun 26, 2023
Merged

Support VMI eviction also for external infra clusters #242

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion api/v1alpha1/kubevirtcluster_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ const ( //labels
)

const ( // annotations
VmiDeletionGraceTime = "capk.cluster.x-k8s.io/vmi-deletion-grace-time"
VmiDeletionGraceTime = "capk.cluster.x-k8s.io/vmi-deletion-grace-time"
VmiDeletionGraceTimeEscape = "capk.cluster.x-k8s.io~1vmi-deletion-grace-time"
)

// KubevirtClusterSpec defines the desired state of KubevirtCluster.
Expand Down
2 changes: 1 addition & 1 deletion clusterkubevirtadm/cmd/get/kubeconfig/kubeconfig_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ var _ = Describe("test kubeconfig function", func() {
Expect(found).ShouldNot(BeNil())
Expect(found.Secrets).ToNot(BeEmpty())

Eventually(doneUpdatingSa)
Eventually(doneUpdatingSa).Should(BeClosed())
})

It("should fail after 10 seconds if the secret was not created", func() {
Expand Down
6 changes: 0 additions & 6 deletions config/rbac/role.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,6 @@ rules:
verbs:
- delete
- get
- list
- patch
- update
- watch
Comment on lines -104 to -107
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also remove list and watch from the virtualmachines entry that's slightly below this one

- apiGroups:
- kubevirt.io
resources:
Expand All @@ -113,10 +109,8 @@ rules:
- create
- delete
- get
- list
- patch
- update
- watch
- apiGroups:
- rbac.authorization.k8s.io
resources:
Expand Down
12 changes: 10 additions & 2 deletions controllers/kubevirtmachine_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ type KubevirtMachineReconciler struct {
// +kubebuilder:rbac:groups=infrastructure.cluster.x-k8s.io,resources=kubevirtmachines/status,verbs=get;update;patch
// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=clusters;machines,verbs=get;list;watch
// +kubebuilder:rbac:groups="",resources=secrets;,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=kubevirt.io,resources=virtualmachines;,verbs=get;list;watch;create;update;patch;delete
// +kubebuilder:rbac:groups=kubevirt.io,resources=virtualmachineinstances;,verbs=get;list;watch
// +kubebuilder:rbac:groups=kubevirt.io,resources=virtualmachines;,verbs=get;create;update;patch;delete
// +kubebuilder:rbac:groups=kubevirt.io,resources=virtualmachineinstances;,verbs=get;delete

// Reconcile handles KubevirtMachine events.
func (r *KubevirtMachineReconciler) Reconcile(goctx gocontext.Context, req ctrl.Request) (_ ctrl.Result, rerr error) {
Expand Down Expand Up @@ -289,6 +289,14 @@ func (r *KubevirtMachineReconciler) reconcileNormal(ctx *context.MachineContext)
return ctrl.Result{RequeueAfter: 20 * time.Second}, nil
}

retryDuration, err := externalMachine.DrainNodeIfNeeded(r.WorkloadCluster)
if err != nil {
return ctrl.Result{RequeueAfter: retryDuration}, errors.Wrap(err, "failed to drain node")
}
if retryDuration > 0 {
return ctrl.Result{RequeueAfter: retryDuration}, nil
}

if externalMachine.SupportsCheckingIsBootstrapped() && !conditions.IsTrue(ctx.KubevirtMachine, infrav1.BootstrapExecSucceededCondition) {
if !externalMachine.IsBootstrapped() {
ctx.Logger.Info("Waiting for underlying VM to bootstrap...")
Expand Down
97 changes: 97 additions & 0 deletions controllers/kubevirtmachine_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package controllers

import (
gocontext "context"
"fmt"
"time"

"github.com/golang/mock/gomock"
Expand Down Expand Up @@ -395,6 +396,8 @@ var _ = Describe("reconcile a kubevirt machine", func() {
machineMock.EXPECT().Address().Return("1.1.1.1").AnyTimes()
machineMock.EXPECT().SupportsCheckingIsBootstrapped().Return(false).AnyTimes()
machineMock.EXPECT().GenerateProviderID().Return("abc", nil).AnyTimes()
machineMock.EXPECT().GenerateProviderID().Return("abc", nil).AnyTimes()
machineMock.EXPECT().DrainNodeIfNeeded(gomock.Any()).Return(time.Duration(0), nil).AnyTimes()
machineFactoryMock.EXPECT().NewMachine(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(machineMock, nil).Times(1)

infraClusterMock.EXPECT().GenerateInfraClusterClient(kubevirtMachine.Spec.InfraClusterSecretRef, kubevirtMachine.Namespace, machineContext.Context).Return(fakeClient, kubevirtMachine.Namespace, nil).Times(3)
Expand Down Expand Up @@ -898,6 +901,7 @@ var _ = Describe("reconcile a kubevirt machine", func() {
machineMock.EXPECT().Exists().Return(true).Times(1)
machineMock.EXPECT().Address().Return("1.1.1.1").Times(1)
machineMock.EXPECT().SupportsCheckingIsBootstrapped().Return(false).Times(1)
machineMock.EXPECT().DrainNodeIfNeeded(gomock.Any()).Return(time.Duration(0), nil)
machineFactoryMock.EXPECT().NewMachine(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(machineMock, nil).Times(1)

infraClusterMock.EXPECT().GenerateInfraClusterClient(kubevirtMachine.Spec.InfraClusterSecretRef, kubevirtMachine.Namespace, machineContext.Context).Return(fakeClient, kubevirtMachine.Namespace, nil)
Expand Down Expand Up @@ -943,6 +947,7 @@ var _ = Describe("reconcile a kubevirt machine", func() {
machineMock.EXPECT().GenerateProviderID().Return("abc", nil).AnyTimes()
machineMock.EXPECT().SupportsCheckingIsBootstrapped().Return(true)
machineMock.EXPECT().IsBootstrapped().Return(false)
machineMock.EXPECT().DrainNodeIfNeeded(gomock.Any()).Return(time.Duration(0), nil)

machineFactoryMock.EXPECT().NewMachine(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(machineMock, nil).Times(1)

Expand Down Expand Up @@ -992,6 +997,7 @@ var _ = Describe("reconcile a kubevirt machine", func() {
machineMock.EXPECT().GenerateProviderID().Return("abc", nil).Times(1)
machineMock.EXPECT().SupportsCheckingIsBootstrapped().Return(true)
machineMock.EXPECT().IsBootstrapped().Return(true)
machineMock.EXPECT().DrainNodeIfNeeded(gomock.Any()).Return(time.Duration(0), nil)

machineFactoryMock.EXPECT().NewMachine(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(machineMock, nil).Times(1)

Expand All @@ -1007,6 +1013,97 @@ var _ = Describe("reconcile a kubevirt machine", func() {
Expect(conditions[0].Type).To(Equal(infrav1.BootstrapExecSucceededCondition))
Expect(conditions[0].Status).To(Equal(corev1.ConditionTrue))
})

It("should requeue on node draining", func() {
vmiReadyCondition := kubevirtv1.VirtualMachineInstanceCondition{
Type: kubevirtv1.VirtualMachineInstanceReady,
Status: corev1.ConditionTrue,
}
vmi.Status.Conditions = append(vmi.Status.Conditions, vmiReadyCondition)
vmi.Status.Interfaces = []kubevirtv1.VirtualMachineInstanceNetworkInterface{

{
IP: "1.1.1.1",
},
}
sshKeySecret.Data["pub"] = []byte("shell")

objects := []client.Object{
cluster,
kubevirtCluster,
machine,
kubevirtMachine,
bootstrapSecret,
bootstrapUserDataSecret,
sshKeySecret,
vm,
vmi,
}

const requeueDurationSeconds = 3
machineMock.EXPECT().IsTerminal().Return(false, "", nil).Times(1)
machineMock.EXPECT().Exists().Return(true).Times(1)
machineMock.EXPECT().IsReady().Return(true).Times(1)
machineMock.EXPECT().Address().Return("1.1.1.1").Times(1)
machineMock.EXPECT().DrainNodeIfNeeded(gomock.Any()).Return(time.Second*requeueDurationSeconds, nil).Times(1)

machineFactoryMock.EXPECT().NewMachine(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(machineMock, nil).Times(1)

setupClient(machineFactoryMock, objects)

infraClusterMock.EXPECT().GenerateInfraClusterClient(kubevirtMachine.Spec.InfraClusterSecretRef, kubevirtMachine.Namespace, machineContext.Context).Return(fakeClient, kubevirtMachine.Namespace, nil)

res, err := kubevirtMachineReconciler.reconcileNormal(machineContext)
Expect(err).ShouldNot(HaveOccurred())

Expect(res.RequeueAfter).To(Equal(time.Second * requeueDurationSeconds))
})

It("should requeue on node draining error + requeue duration", func() {
vmiReadyCondition := kubevirtv1.VirtualMachineInstanceCondition{
Type: kubevirtv1.VirtualMachineInstanceReady,
Status: corev1.ConditionTrue,
}
vmi.Status.Conditions = append(vmi.Status.Conditions, vmiReadyCondition)
vmi.Status.Interfaces = []kubevirtv1.VirtualMachineInstanceNetworkInterface{

{
IP: "1.1.1.1",
},
}
sshKeySecret.Data["pub"] = []byte("shell")

objects := []client.Object{
cluster,
kubevirtCluster,
machine,
kubevirtMachine,
bootstrapSecret,
bootstrapUserDataSecret,
sshKeySecret,
vm,
vmi,
}

const requeueDurationSeconds = 3
machineMock.EXPECT().IsTerminal().Return(false, "", nil).Times(1)
machineMock.EXPECT().Exists().Return(true).Times(1)
machineMock.EXPECT().IsReady().Return(true).Times(1)
machineMock.EXPECT().Address().Return("1.1.1.1").Times(1)
machineMock.EXPECT().DrainNodeIfNeeded(gomock.Any()).Return(time.Second*requeueDurationSeconds, fmt.Errorf("mock error")).Times(1)

machineFactoryMock.EXPECT().NewMachine(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(machineMock, nil).Times(1)

setupClient(machineFactoryMock, objects)

infraClusterMock.EXPECT().GenerateInfraClusterClient(kubevirtMachine.Spec.InfraClusterSecretRef, kubevirtMachine.Namespace, machineContext.Context).Return(fakeClient, kubevirtMachine.Namespace, nil)

res, err := kubevirtMachineReconciler.reconcileNormal(machineContext)
Expect(err).Should(HaveOccurred())
Expect(errors.Unwrap(err).Error()).Should(ContainSubstring("failed to drain node: mock error"))

Expect(res.RequeueAfter).To(Equal(time.Second * requeueDurationSeconds))
})
})
})
It("should detect when a previous Ready KubeVirtMachine is no longer ready due to vmi ready condition being false", func() {
Expand Down
Loading