Skip to content

Commit

Permalink
controller: reconcile VMIs instead of VMs
Browse files Browse the repository at this point in the history
This way, we can handle live-migration of stateless VMIs.

Signed-off-by: Miguel Duarte Barroso <[email protected]>
  • Loading branch information
maiqueb committed May 13, 2024
1 parent f648b76 commit 2652ad9
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 54 deletions.
49 changes: 25 additions & 24 deletions pkg/vmnetworkscontroller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand All @@ -47,33 +47,41 @@ 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
}

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]
Expand All @@ -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,
Expand All @@ -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,
}

Expand Down Expand Up @@ -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)
}
Expand Down
51 changes: 21 additions & 30 deletions pkg/vmnetworkscontroller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand All @@ -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))
}
Expand All @@ -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),
Expand All @@ -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,
},
Expand All @@ -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),
Expand All @@ -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",
},
},
}),
)
})

Expand Down

0 comments on commit 2652ad9

Please sign in to comment.