Skip to content

Commit

Permalink
Merge pull request #291 from orenc1/add_vm_live_migratable_condition
Browse files Browse the repository at this point in the history
Add VM LiveMigratable Condition to KubevirtMachine
  • Loading branch information
k8s-ci-robot authored Jul 21, 2024
2 parents 1cb0101 + 0ce6061 commit 8c78f30
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 8c78f30

Please sign in to comment.