From 2652ad975d0cd50d8936bdb809e8dd40c72d193d Mon Sep 17 00:00:00 2001 From: Miguel Duarte Barroso Date: Mon, 13 May 2024 18:31:14 +0200 Subject: [PATCH] controller: reconcile VMIs instead of VMs This way, we can handle live-migration of stateless VMIs. Signed-off-by: Miguel Duarte Barroso --- pkg/vmnetworkscontroller/controller.go | 49 ++++++++++---------- pkg/vmnetworkscontroller/controller_test.go | 51 +++++++++------------ 2 files changed, 46 insertions(+), 54 deletions(-) diff --git a/pkg/vmnetworkscontroller/controller.go b/pkg/vmnetworkscontroller/controller.go index 7ca5d117..221b5687 100644 --- a/pkg/vmnetworkscontroller/controller.go +++ b/pkg/vmnetworkscontroller/controller.go @@ -37,7 +37,7 @@ type VirtualMachineReconciler struct { func NewVMReconciler(manager controllerruntime.Manager) *VirtualMachineReconciler { return &VirtualMachineReconciler{ Client: manager.GetClient(), - Log: controllerruntime.Log.WithName("controllers").WithName("VirtualMachine"), + Log: controllerruntime.Log.WithName("controllers").WithName("VirtualMachineInstance"), Scheme: manager.GetScheme(), manager: manager, } @@ -47,25 +47,33 @@ func (r *VirtualMachineReconciler) Reconcile( ctx context.Context, request controllerruntime.Request, ) (controllerruntime.Result, error) { - vm := &virtv1.VirtualMachine{} + vmi := &virtv1.VirtualMachineInstance{} ctx, cancel := context.WithTimeout(ctx, time.Second) defer cancel() - err := r.Client.Get(ctx, request.NamespacedName, vm) + err := r.Client.Get(ctx, request.NamespacedName, vmi) if apierrors.IsNotFound(err) { - r.Log.Error(err, "Error retrieving VM") + r.Log.Error(err, "Error retrieving VMI") // Error reading the object - requeue the request. return controllerruntime.Result{}, err } - vmi := &virtv1.VirtualMachineInstance{} - if err := r.Client.Get(ctx, request.NamespacedName, vmi); apierrors.IsNotFound(err) { - r.Log.Error(err, "Error retrieving VMI") - // Error reading the object - requeue the request. - return controllerruntime.Result{}, err + var ownerInfo corev1.OwnerReference + vm := &virtv1.VirtualMachine{} + if err := r.Client.Get(ctx, request.NamespacedName, vm); apierrors.IsNotFound(err) { + r.Log.Info("Corresponding VM not found", "vm", request.NamespacedName) + ownerInfo = corev1.OwnerReference{APIVersion: vmi.APIVersion, Kind: vmi.Kind, Name: vmi.Name, UID: vmi.UID} + } else if err == nil { + ownerInfo = corev1.OwnerReference{APIVersion: vm.APIVersion, Kind: vm.Kind, Name: vm.Name, UID: vm.UID} + } else { + return controllerruntime.Result{}, fmt.Errorf( + "error to get VMI %q corresponding VM: %w", + request.NamespacedName, + err, + ) } - vmiSpec := vm.Spec.Template.Spec + vmiSpec := vmi.Spec for _, net := range vmiSpec.Networks { if net.Pod != nil { continue @@ -73,7 +81,7 @@ func (r *VirtualMachineReconciler) Reconcile( if net.Multus != nil { nadName := net.Multus.NetworkName - namespace := vm.Namespace + namespace := vmi.Namespace namespaceAndName := strings.Split(nadName, "/") if len(namespaceAndName) == 2 { namespace = namespaceAndName[0] @@ -100,19 +108,12 @@ func (r *VirtualMachineReconciler) Reconcile( } if nadConfig.AllowPersistentIPs { - claimKey := fmt.Sprintf("%s.%s", vm.Name, net.Name) + claimKey := fmt.Sprintf("%s.%s", vmi.Name, net.Name) ipamClaim := &ipamclaimsapi.IPAMClaim{ ObjectMeta: controllerruntime.ObjectMeta{ - Name: claimKey, - Namespace: vm.Namespace, - OwnerReferences: []corev1.OwnerReference{ - { - APIVersion: vm.APIVersion, - Kind: vm.Kind, - Name: vm.Name, - UID: vm.UID, - }, - }, + Name: claimKey, + Namespace: vmi.Namespace, + OwnerReferences: []corev1.OwnerReference{ownerInfo}, }, Spec: ipamclaimsapi.IPAMClaimSpec{ Network: nadConfig.Name, @@ -121,7 +122,7 @@ func (r *VirtualMachineReconciler) Reconcile( if err := r.Client.Create(ctx, ipamClaim, &client.CreateOptions{}); err != nil { if apierrors.IsAlreadyExists(err) { claimKey := apitypes.NamespacedName{ - Namespace: vm.Namespace, + Namespace: vmi.Namespace, Name: claimKey, } @@ -157,7 +158,7 @@ func (r *VirtualMachineReconciler) Reconcile( // Setup sets up the controller with the Manager passed in the constructor. func (r *VirtualMachineReconciler) Setup() error { return controllerruntime.NewControllerManagedBy(r.manager). - For(&virtv1.VirtualMachine{}). + For(&virtv1.VirtualMachineInstance{}). WithEventFilter(onVMPredicates()). Complete(r) } diff --git a/pkg/vmnetworkscontroller/controller_test.go b/pkg/vmnetworkscontroller/controller_test.go index 7025de59..a19c3219 100644 --- a/pkg/vmnetworkscontroller/controller_test.go +++ b/pkg/vmnetworkscontroller/controller_test.go @@ -75,16 +75,16 @@ var _ = Describe("vm IPAM controller", Serial, func() { DescribeTable("reconcile behavior is as expected", func(config testConfig) { var initialObjects []client.Object - var vmKey apitypes.NamespacedName if config.inputVM != nil { - vmKey = apitypes.NamespacedName{ - Namespace: config.inputVM.Namespace, - Name: config.inputVM.Name, - } initialObjects = append(initialObjects, config.inputVM) } + var vmiKey apitypes.NamespacedName if config.inputVMI != nil { + vmiKey = apitypes.NamespacedName{ + Namespace: config.inputVMI.Namespace, + Name: config.inputVMI.Name, + } initialObjects = append(initialObjects, config.inputVMI) } @@ -111,18 +111,18 @@ var _ = Describe("vm IPAM controller", Serial, func() { reconcileMachine := NewVMReconciler(mgr) if config.expectedError != nil { - _, err := reconcileMachine.Reconcile(context.Background(), controllerruntime.Request{NamespacedName: vmKey}) + _, err := reconcileMachine.Reconcile(context.Background(), controllerruntime.Request{NamespacedName: vmiKey}) Expect(err).To(MatchError(config.expectedError)) } else { Expect( - reconcileMachine.Reconcile(context.Background(), controllerruntime.Request{NamespacedName: vmKey}), + reconcileMachine.Reconcile(context.Background(), controllerruntime.Request{NamespacedName: vmiKey}), ).To(Equal(config.expectedResponse)) } if len(config.expectedIPAMClaims) > 0 { ipamClaimList := &ipamclaimsapi.IPAMClaimList{} Expect(mgr.GetClient().List(context.Background(), ipamClaimList, &client.ListOptions{ - Namespace: config.inputVM.Namespace, + Namespace: config.inputVMI.Namespace, })).To(Succeed()) Expect(ipamClaimsSpecExtractor(ipamClaimList.Items...)).To(ConsistOf(config.expectedIPAMClaims)) } @@ -144,23 +144,6 @@ var _ = Describe("vm IPAM controller", Serial, func() { inputNAD: dummyNADWrongFormat(nadName), expectedError: fmt.Errorf("failed to extract the relevant NAD information"), }), - Entry("the associated VMI is not yet found", testConfig{ - inputVM: dummyVM(nadName), - inputNAD: dummyNAD(nadName), - expectedError: &errors.StatusError{ - ErrStatus: metav1.Status{ - Status: "Failure", - Message: "virtualmachineinstances.kubevirt.io \"vm1\" not found", - Reason: "NotFound", - Details: &metav1.StatusDetails{ - Name: "vm1", - Group: "kubevirt.io", - Kind: "virtualmachineinstances", - }, - Code: 404, - }, - }, - }), Entry("the associated VMI exists but points to a NAD that doesn't exist", testConfig{ inputVM: dummyVM(nadName), inputVMI: dummyVMI(nadName), @@ -178,17 +161,16 @@ var _ = Describe("vm IPAM controller", Serial, func() { }, }, }), - - Entry("the VM does not exist on the datastore - it might have been deleted in the meantime", testConfig{ + Entry("the VMI does not exist on the datastore - it might have been deleted in the meantime", testConfig{ expectedError: &errors.StatusError{ ErrStatus: metav1.Status{ Status: "Failure", - Message: "virtualmachines.kubevirt.io \"\" not found", // we're not passing a VM, so it doesn't know its name + Message: "virtualmachineinstances.kubevirt.io \"\" not found", // we're not passing a VM, so it doesn't know its name Reason: "NotFound", Details: &metav1.StatusDetails{ Name: "", Group: "kubevirt.io", - Kind: "virtualmachines", + Kind: "virtualmachineinstances", }, Code: 404, }, @@ -207,7 +189,6 @@ var _ = Describe("vm IPAM controller", Serial, func() { }, expectedError: fmt.Errorf("failed since it found an existing IPAMClaim for \"vm1.randomnet\""), }), - Entry("found an existing IPAMClaim for the same VM", testConfig{ inputVM: decorateVMWithUID(dummyUID, dummyVM(nadName)), inputVMI: dummyVMI(nadName), @@ -234,6 +215,16 @@ var _ = Describe("vm IPAM controller", Serial, func() { }, }, }), + Entry("a lonesome VMI (with no corresponding VM) is a valid migration use-case", testConfig{ + inputVMI: dummyVMI(nadName), + inputNAD: dummyNAD(nadName), + expectedResponse: reconcile.Result{}, + expectedIPAMClaims: []ipamclaimsapi.IPAMClaimSpec{ + { + Network: "goodnet", + }, + }, + }), ) })