Skip to content

Commit

Permalink
Add LiveMigratable condition to KubevirtMachine
Browse files Browse the repository at this point in the history
Virtual Machines serving as guest cluster nodes can be live migrated, this is beneficial in case of infra cluster nodes maintainance or mulfunction,
requiring the Virtual Machines to be migrated to another node.
This PR introduces the "VMLiveMigratable" condition to KubevirtMachine, and it takes that value as-is from the underlying VirtualMachine instance.
Then, consumers of cluster-api-provider-kubevirt could make use of it and buble that information up to the end user.

Signed-off-by: Oren Cohen <[email protected]>
  • Loading branch information
orenc1 committed Jul 17, 2024
1 parent 1cb0101 commit 0ce6061
Show file tree
Hide file tree
Showing 10 changed files with 143 additions and 17 deletions.
2 changes: 1 addition & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

# Build the manager binary
# Run this with docker build --build-arg builder_image=<golang:x.y.z>
ARG builder_image=golang:1.22
ARG builder_image=docker.io/golang:1.22
FROM ${builder_image} as builder
WORKDIR /workspace

Expand Down
3 changes: 3 additions & 0 deletions api/v1alpha1/condition_consts.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ const (
// VMCreateFailed (Severity=Error) documents a KubevirtMachine that is unable to create the
// corresponding VM object.
VMCreateFailedReason = "VMCreateFailed"

// VMLiveMigratableCondition documents whether the VM is live-migratable or not
VMLiveMigratableCondition clusterv1.ConditionType = "VMLiveMigratable"
)

const (
Expand Down
4 changes: 2 additions & 2 deletions api/v1alpha1/kubevirtclustertemplate_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
// KubevirtClusterTemplateResource describes the data needed to create a KubevirtCluster from a template.
type KubevirtClusterTemplateResource struct {
ObjectMeta clusterv1.ObjectMeta `json:"metadata,omitempty"`
Spec KubevirtClusterSpec `json:"spec"`
Spec KubevirtClusterSpec `json:"spec"`
}

// KubevirtClusterTemplateSpec defines the desired state of KubevirtClusterTemplate.
Expand All @@ -40,7 +40,7 @@ type KubevirtClusterTemplateSpec struct {
type KubevirtClusterTemplate struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`
Spec KubevirtClusterTemplateSpec `json:"spec,omitempty"`
Spec KubevirtClusterTemplateSpec `json:"spec,omitempty"`
}

// +kubebuilder:object:root=true
Expand Down
1 change: 1 addition & 0 deletions clusterkubevirtadm/cmd/credentials/credentials_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package credentials

import (
"context"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
corev1 "k8s.io/api/core/v1"
Expand Down
14 changes: 14 additions & 0 deletions controllers/kubevirtmachine_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,20 @@ func (r *KubevirtMachineReconciler) reconcileNormal(ctx *context.MachineContext)
ctx.KubevirtMachine.Status.Ready = false
}

liveMigratable, reason, message, err := externalMachine.IsLiveMigratable()
if err != nil {
ctx.Logger.Error(err, fmt.Sprintf("failed to get the %s condition of %s machine",
infrav1.VMLiveMigratableCondition, ctx.KubevirtMachine.Name))
return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
}
if liveMigratable {
// Mark VMLiveMigratableCondition to indicate whether the VM can be live migrated or not
conditions.MarkTrue(ctx.KubevirtMachine, infrav1.VMLiveMigratableCondition)
} else {
conditions.MarkFalse(ctx.KubevirtMachine, infrav1.VMLiveMigratableCondition, reason, clusterv1.ConditionSeverityInfo,
fmt.Sprintf("%s is not a live migratable machine: %s", ctx.KubevirtMachine.Name, message))
}

return ctrl.Result{}, nil
}

Expand Down
84 changes: 76 additions & 8 deletions controllers/kubevirtmachine_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
kubevirtv1 "kubevirt.io/api/core/v1"

"sigs.k8s.io/cluster-api-provider-kubevirt/pkg/kubevirt"

clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
Expand Down Expand Up @@ -424,7 +425,7 @@ var _ = Describe("reconcile a kubevirt machine", func() {
Expect(err).ShouldNot(HaveOccurred())
Expect(out).To(Equal(ctrl.Result{Requeue: false, RequeueAfter: 0}))

//Check bootstrapData secret is deleted
// Check bootstrapData secret is deleted
machineBootstrapSecretReferenceName := machineContext.Machine.Spec.Bootstrap.DataSecretName
machineBootstrapSecretReferenceKey := client.ObjectKey{Namespace: machineContext.Machine.GetNamespace(), Name: *machineBootstrapSecretReferenceName + "-userdata"}
infraClusterClient, _, err := infraClusterMock.GenerateInfraClusterClient(kubevirtMachine.Spec.InfraClusterSecretRef, kubevirtMachine.Namespace, machineContext.Context)
Expand All @@ -433,7 +434,7 @@ var _ = Describe("reconcile a kubevirt machine", func() {
err = infraClusterClient.Get(gocontext.Background(), machineBootstrapSecretReferenceKey, bootstrapDataSecret)
Expect(apierrors.IsNotFound(err)).To(BeTrue())

//Check finalizer is removed from machine
// Check finalizer is removed from machine
Expect(machineContext.Machine.ObjectMeta.Finalizers).To(BeEmpty())
})

Expand All @@ -456,14 +457,14 @@ var _ = Describe("reconcile a kubevirt machine", func() {
Expect(err).ShouldNot(HaveOccurred())
Expect(out).To(Equal(ctrl.Result{Requeue: false, RequeueAfter: 0}))

//Check finalizer is removed from machine
// Check finalizer is removed from machine
Expect(machineContext.Machine.ObjectMeta.Finalizers).To(BeEmpty())
})

It("should update userdata correctly at KubevirtMachine reconcile", func() {
//Get Machine
//Get userdata secret name from machine
//Get userdata secret and assert equality to original secret
// Get Machine
// Get userdata secret name from machine
// Get userdata secret and assert equality to original secret
objects := []client.Object{
cluster,
kubevirtCluster,
Expand Down Expand Up @@ -631,6 +632,10 @@ var _ = Describe("reconcile a kubevirt machine", func() {
Type: kubevirtv1.VirtualMachineInstanceReady,
Status: corev1.ConditionTrue,
},
{
Type: kubevirtv1.VirtualMachineInstanceIsMigratable,
Status: corev1.ConditionTrue,
},
}
vmi.Status.Interfaces = []kubevirtv1.VirtualMachineInstanceNetworkInterface{

Expand Down Expand Up @@ -951,6 +956,8 @@ var _ = Describe("reconcile a kubevirt machine", func() {
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)
machineMock.EXPECT().IsLiveMigratable().Return(false, "", "", nil).Times(1)

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 All @@ -959,8 +966,10 @@ var _ = Describe("reconcile a kubevirt machine", func() {
Expect(err).ShouldNot(HaveOccurred())

conditions := machineContext.KubevirtMachine.GetConditions()
Expect(conditions[0].Type).To(Equal(infrav1.VMProvisionedCondition))
Expect(conditions[0].Status).To(Equal(corev1.ConditionTrue))
Expect(conditions[0].Type).To(Equal(infrav1.VMLiveMigratableCondition))
Expect(conditions[0].Status).To(Equal(corev1.ConditionFalse))
Expect(conditions[1].Type).To(Equal(infrav1.VMProvisionedCondition))
Expect(conditions[1].Status).To(Equal(corev1.ConditionTrue))
})
It("adds a failed BootstrapExecSucceededCondition with reason BootstrapFailedReason when bootstraping is possible and failed", func() {
vmiReadyCondition := kubevirtv1.VirtualMachineInstanceCondition{
Expand Down Expand Up @@ -1047,6 +1056,63 @@ var _ = Describe("reconcile a kubevirt machine", func() {
machineMock.EXPECT().SupportsCheckingIsBootstrapped().Return(true)
machineMock.EXPECT().IsBootstrapped().Return(true)
machineMock.EXPECT().DrainNodeIfNeeded(gomock.Any()).Return(time.Duration(0), nil)
machineMock.EXPECT().IsLiveMigratable().Return(false, "", "", 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)

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

conditions := machineContext.KubevirtMachine.GetConditions()

Expect(conditions[0].Type).To(Equal(infrav1.BootstrapExecSucceededCondition))
Expect(conditions[0].Status).To(Equal(corev1.ConditionTrue))
})

It("adds a succeeded VMLiveMigratableCondition", func() {
vmiReadyCondition := kubevirtv1.VirtualMachineInstanceCondition{
Type: kubevirtv1.VirtualMachineInstanceReady,
Status: corev1.ConditionTrue,
}
vmiLiveMigratableCondition := kubevirtv1.VirtualMachineInstanceCondition{
Type: kubevirtv1.VirtualMachineInstanceIsMigratable,
Status: corev1.ConditionTrue,
}
vmi.Status.Conditions = append(vmi.Status.Conditions, vmiReadyCondition)
vmi.Status.Conditions = append(vmi.Status.Conditions, vmiLiveMigratableCondition)
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,
}

machineMock.EXPECT().IsTerminal().Return(false, "", nil).Times(1)
machineMock.EXPECT().Exists().Return(true).Times(1)
machineMock.EXPECT().IsReady().Return(true).Times(2)
machineMock.EXPECT().Address().Return("1.1.1.1").Times(1)
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)
machineMock.EXPECT().IsLiveMigratable().Return(true, "", "", nil).Times(1)

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

Expand All @@ -1061,6 +1127,8 @@ var _ = Describe("reconcile a kubevirt machine", func() {

Expect(conditions[0].Type).To(Equal(infrav1.BootstrapExecSucceededCondition))
Expect(conditions[0].Status).To(Equal(corev1.ConditionTrue))
Expect(conditions[1].Type).To(Equal(infrav1.VMLiveMigratableCondition))
Expect(conditions[1].Status).To(Equal(corev1.ConditionTrue))
})

It("should requeue on node draining", func() {
Expand Down
20 changes: 20 additions & 0 deletions pkg/kubevirt/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,26 @@ func (m *Machine) IsReady() bool {
return m.hasReadyCondition()
}

// IsLiveMigratable reports back the live-migratability state of the VM: Status, Reason and Message
func (m *Machine) IsLiveMigratable() (bool, string, string, error) {
if m.vmiInstance == nil {
return false, "", "", fmt.Errorf("VMI is nil")
}

for _, cond := range m.vmiInstance.Status.Conditions {
if cond.Type == kubevirtv1.VirtualMachineInstanceIsMigratable {
if cond.Status == corev1.ConditionTrue {
return true, "", "", nil
} else {
return false, cond.Reason, cond.Message, nil
}
}
}

return false, "", "", fmt.Errorf("%s VMI does not have a %s condition",
m.vmiInstance.Status.Phase, kubevirtv1.VirtualMachineInstanceIsMigratable)
}

const (
defaultCondReason = "VMNotReady"
defaultCondMessage = "VM is not ready"
Expand Down
2 changes: 2 additions & 0 deletions pkg/kubevirt/machine_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ type MachineInterface interface {
Exists() bool
// IsReady checks if the VM is ready
IsReady() bool
// IsLiveMigratable reports back the live-migratability state of the VM: Status, Reason and Message
IsLiveMigratable() (bool, string, string, error)
// Address returns the IP address of the VM.
Address() string
// SupportsCheckingIsBootstrapped checks if we have a method of checking
Expand Down
27 changes: 22 additions & 5 deletions pkg/kubevirt/mock/machine_factory_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion pkg/workloadcluster/mock/workloadcluster_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 0ce6061

Please sign in to comment.