From 0ce6061c41aa8d61c5c3d6cceecf12813df0346f Mon Sep 17 00:00:00 2001 From: Oren Cohen Date: Fri, 12 Jul 2024 14:46:07 +0300 Subject: [PATCH] Add LiveMigratable condition to KubevirtMachine 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 --- Dockerfile | 2 +- api/v1alpha1/condition_consts.go | 3 + api/v1alpha1/kubevirtclustertemplate_types.go | 4 +- .../cmd/credentials/credentials_test.go | 1 + controllers/kubevirtmachine_controller.go | 14 ++++ .../kubevirtmachine_controller_test.go | 84 +++++++++++++++++-- pkg/kubevirt/machine.go | 20 +++++ pkg/kubevirt/machine_factory.go | 2 + .../mock/machine_factory_generated.go | 27 ++++-- .../mock/workloadcluster_generated.go | 3 +- 10 files changed, 143 insertions(+), 17 deletions(-) diff --git a/Dockerfile b/Dockerfile index f0105d11..5258e290 100644 --- a/Dockerfile +++ b/Dockerfile @@ -16,7 +16,7 @@ # Build the manager binary # Run this with docker build --build-arg builder_image= -ARG builder_image=golang:1.22 +ARG builder_image=docker.io/golang:1.22 FROM ${builder_image} as builder WORKDIR /workspace diff --git a/api/v1alpha1/condition_consts.go b/api/v1alpha1/condition_consts.go index 07f225c5..7f671d8e 100644 --- a/api/v1alpha1/condition_consts.go +++ b/api/v1alpha1/condition_consts.go @@ -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 ( diff --git a/api/v1alpha1/kubevirtclustertemplate_types.go b/api/v1alpha1/kubevirtclustertemplate_types.go index 422c8cc1..bf0b9c70 100644 --- a/api/v1alpha1/kubevirtclustertemplate_types.go +++ b/api/v1alpha1/kubevirtclustertemplate_types.go @@ -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. @@ -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 diff --git a/clusterkubevirtadm/cmd/credentials/credentials_test.go b/clusterkubevirtadm/cmd/credentials/credentials_test.go index f9c2b213..63302375 100644 --- a/clusterkubevirtadm/cmd/credentials/credentials_test.go +++ b/clusterkubevirtadm/cmd/credentials/credentials_test.go @@ -2,6 +2,7 @@ package credentials import ( "context" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" diff --git a/controllers/kubevirtmachine_controller.go b/controllers/kubevirtmachine_controller.go index 932bf762..4a99c8cf 100644 --- a/controllers/kubevirtmachine_controller.go +++ b/controllers/kubevirtmachine_controller.go @@ -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 } diff --git a/controllers/kubevirtmachine_controller_test.go b/controllers/kubevirtmachine_controller_test.go index 0d26781f..0bb20171 100644 --- a/controllers/kubevirtmachine_controller_test.go +++ b/controllers/kubevirtmachine_controller_test.go @@ -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" @@ -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) @@ -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()) }) @@ -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, @@ -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{ @@ -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) @@ -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{ @@ -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) @@ -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() { diff --git a/pkg/kubevirt/machine.go b/pkg/kubevirt/machine.go index bb86c110..38a839dd 100644 --- a/pkg/kubevirt/machine.go +++ b/pkg/kubevirt/machine.go @@ -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" diff --git a/pkg/kubevirt/machine_factory.go b/pkg/kubevirt/machine_factory.go index 38180115..d5724dfa 100644 --- a/pkg/kubevirt/machine_factory.go +++ b/pkg/kubevirt/machine_factory.go @@ -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 diff --git a/pkg/kubevirt/mock/machine_factory_generated.go b/pkg/kubevirt/mock/machine_factory_generated.go index 870dc28d..14313629 100644 --- a/pkg/kubevirt/mock/machine_factory_generated.go +++ b/pkg/kubevirt/mock/machine_factory_generated.go @@ -10,7 +10,6 @@ import ( time "time" gomock "github.com/golang/mock/gomock" - context0 "sigs.k8s.io/cluster-api-provider-kubevirt/pkg/context" kubevirt "sigs.k8s.io/cluster-api-provider-kubevirt/pkg/kubevirt" ssh "sigs.k8s.io/cluster-api-provider-kubevirt/pkg/ssh" @@ -127,6 +126,11 @@ func (mr *MockMachineInterfaceMockRecorder) GenerateProviderID() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GenerateProviderID", reflect.TypeOf((*MockMachineInterface)(nil).GenerateProviderID)) } +// GetVMNotReadyReason mocks base method. +func (m *MockMachineInterface) GetVMNotReadyReason() (string, string) { + return "", "" +} + // IsBootstrapped mocks base method. func (m *MockMachineInterface) IsBootstrapped() bool { m.ctrl.T.Helper() @@ -141,6 +145,23 @@ func (mr *MockMachineInterfaceMockRecorder) IsBootstrapped() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsBootstrapped", reflect.TypeOf((*MockMachineInterface)(nil).IsBootstrapped)) } +// IsLiveMigratable mocks base method. +func (m *MockMachineInterface) IsLiveMigratable() (bool, string, string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IsLiveMigratable") + ret0, _ := ret[0].(bool) + ret1, _ := ret[1].(string) + ret2, _ := ret[2].(string) + ret3, _ := ret[3].(error) + return ret0, ret1, ret2, ret3 +} + +// IsLiveMigratable indicates an expected call of IsLiveMigratable. +func (mr *MockMachineInterfaceMockRecorder) IsLiveMigratable() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsLiveMigratable", reflect.TypeOf((*MockMachineInterface)(nil).IsLiveMigratable)) +} + // IsReady mocks base method. func (m *MockMachineInterface) IsReady() bool { m.ctrl.T.Helper() @@ -149,10 +170,6 @@ func (m *MockMachineInterface) IsReady() bool { return ret0 } -func (m *MockMachineInterface) GetVMNotReadyReason() (string, string) { - return "", "" -} - // IsReady indicates an expected call of IsReady. func (mr *MockMachineInterfaceMockRecorder) IsReady() *gomock.Call { mr.mock.ctrl.T.Helper() diff --git a/pkg/workloadcluster/mock/workloadcluster_generated.go b/pkg/workloadcluster/mock/workloadcluster_generated.go index 3a7bc134..c7b42e97 100644 --- a/pkg/workloadcluster/mock/workloadcluster_generated.go +++ b/pkg/workloadcluster/mock/workloadcluster_generated.go @@ -9,8 +9,9 @@ import ( gomock "github.com/golang/mock/gomock" kubernetes "k8s.io/client-go/kubernetes" - context "sigs.k8s.io/cluster-api-provider-kubevirt/pkg/context" client "sigs.k8s.io/controller-runtime/pkg/client" + + context "sigs.k8s.io/cluster-api-provider-kubevirt/pkg/context" ) // MockWorkloadCluster is a mock of WorkloadCluster interface.