WIP: live migration to a named node

Follow-up and derived from:
kubevirt#10712
Implements:
kubevirt/community#320

TODO: add functional tests

Signed-off-by: zhonglin6666 <[email protected]>
Signed-off-by: Simone Tiraboschi <[email protected]>

api/openapi-spec/swagger.json

@@ -14960,6 +14960,14 @@
     "description": "MigrateOptions may be provided on migrate request.",
     "type": "object",
     "properties": {
+     "addedNodeSelector": {
+      "description": "AddedNodeSelector is an additional selector that can be used to complement a NodeSelector or NodeAffinity as set on the VM to restrict the set of allowed target nodes for a migration. In case of key collisions, values set on the VM objects are going to be preserved to ensure that addedNodeSelector can only restrict but not bypass constraints already set on the VM object.",
+      "type": "object",
+      "additionalProperties": {
+       "type": "string",
+       "default": ""
+      }
+     },
      "apiVersion": {
       "description": "APIVersion defines the versioned schema of this representation of an object. Servers should convert recognized schemas to the latest internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources",
       "type": "string"
@@ -16728,6 +16736,14 @@
    "v1.VirtualMachineInstanceMigrationSpec": {
     "type": "object",
     "properties": {
+     "addedNodeSelector": {
+      "description": "AddedNodeSelector is an additional selector that can be used to complement a NodeSelector or NodeAffinity as set on the VM to restrict the set of allowed target nodes for a migration. In case of key collisions, values set on the VM objects are going to be preserved to ensure that addedNodeSelector can only restrict but not bypass constraints already set on the VM object.",
+      "type": "object",
+      "additionalProperties": {
+       "type": "string",
+       "default": ""
+      }
+     },
      "vmiName": {
       "description": "The name of the VMI to perform the migration on. VMI must exist in the migration objects namespace",
       "type": "string"

pkg/virt-api/rest/subresource.go

@@ -301,7 +301,8 @@ func (app *SubresourceAPIApp) MigrateVMRequestHandler(request *restful.Request,
 				GenerateName: "kubevirt-migrate-vm-",
 			},
 			Spec: v1.VirtualMachineInstanceMigrationSpec{
-				VMIName: name,
+				VMIName:           name,
+				AddedNodeSelector: bodyStruct.AddedNodeSelector,
 			},
 		}, k8smetav1.CreateOptions{DryRun: bodyStruct.DryRun})
 		if err != nil {

pkg/virt-controller/watch/migration/migration.go

@@ -23,6 +23,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"maps"
 	"sort"
 	"strconv"
 	"strings"
@@ -740,6 +741,11 @@ func (c *Controller) createTargetPod(migration *virtv1.VirtualMachineInstanceMig
 		templatePod.Spec.Affinity.PodAntiAffinity.RequiredDuringSchedulingIgnoredDuringExecution = append(templatePod.Spec.Affinity.PodAntiAffinity.RequiredDuringSchedulingIgnoredDuringExecution, antiAffinityTerm)
 	}
 
+	nodeSelector := make(map[string]string)
+	maps.Copy(nodeSelector, migration.Spec.AddedNodeSelector)
+	maps.Copy(nodeSelector, templatePod.Spec.NodeSelector)
+	templatePod.Spec.NodeSelector = nodeSelector
+
 	templatePod.ObjectMeta.Labels[virtv1.MigrationJobLabel] = string(migration.UID)
 	templatePod.ObjectMeta.Annotations[virtv1.MigrationJobNameAnnotation] = migration.Name
 

pkg/virt-controller/watch/migration/migration_test.go

@@ -21,6 +21,7 @@ package migration
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"strings"
 	"time"
@@ -99,6 +100,19 @@ var _ = Describe("Migration watcher", func() {
 		}
 	}
 
+	getTargetPod := func(namespace string, uid types.UID, migrationUid types.UID) (*k8sv1.Pod, error) {
+		pods, err := kubeClient.CoreV1().Pods(namespace).List(context.Background(), metav1.ListOptions{
+			LabelSelector: fmt.Sprintf("%s=%s,%s=%s", virtv1.MigrationJobLabel, string(migrationUid), virtv1.CreatedByLabel, string(uid)),
+		})
+		if err != nil {
+			return nil, err
+		}
+		if len(pods.Items) == 1 {
+			return &pods.Items[0], nil
+		}
+		return nil, errors.New("Failed identifying target pod")
+	}
+
 	expectPodDoesNotExist := func(namespace, uid, migrationUid string) {
 		pods, err := kubeClient.CoreV1().Pods(namespace).List(context.Background(), metav1.ListOptions{
 			LabelSelector: fmt.Sprintf("%s=%s,%s=%s", virtv1.MigrationJobLabel, migrationUid, virtv1.CreatedByLabel, uid),
@@ -884,6 +898,86 @@ var _ = Describe("Migration watcher", func() {
 			expectPodCreation(vmi.Namespace, vmi.UID, migration.UID, 2, 1, 1)
 		})
 
+		It("should create target pod merging addedNodeSelector and preserving the labels in the existing NodeSelector and NodeAffinity", func() {
+			vmi := newVirtualMachine("testvmi", virtv1.Running)
+
+			vmiNodeSelector := map[string]string{
+				"topology.kubernetes.io/region": "us-east-1",
+				"vmiLabel1":                     "vmiValue1",
+				"vmiLabel2":                     "vmiValue2",
+			}
+			nodeAffinityRule := &k8sv1.NodeAffinity{
+				RequiredDuringSchedulingIgnoredDuringExecution: &k8sv1.NodeSelector{
+					NodeSelectorTerms: []k8sv1.NodeSelectorTerm{
+						{
+							MatchExpressions: []k8sv1.NodeSelectorRequirement{
+								{
+									Key:      k8sv1.LabelHostname,
+									Operator: k8sv1.NodeSelectorOpIn,
+									Values:   []string{"somenode"},
+								},
+							},
+						},
+						{
+							MatchExpressions: []k8sv1.NodeSelectorRequirement{
+								{
+									Key:      k8sv1.LabelHostname,
+									Operator: k8sv1.NodeSelectorOpIn,
+									Values:   []string{"anothernode-ORed"},
+								},
+							},
+						},
+					},
+				},
+			}
+
+			vmi.Spec.NodeSelector = vmiNodeSelector
+			vmi.Spec.Affinity = &k8sv1.Affinity{
+				NodeAffinity: nodeAffinityRule,
+			}
+
+			addedNodeSelector := map[string]string{
+				"topology.kubernetes.io/region": "us-west-1",
+				"additionaLabel1":               "additionalValue1",
+				"additionaLabel2":               "additionalValue2",
+			}
+
+			Expect(vmiNodeSelector).To(HaveKey("topology.kubernetes.io/region"))
+			Expect(addedNodeSelector).To(HaveKey("topology.kubernetes.io/region"))
+
+			migration := newMigrationWithAddedNodeSelector("testmigration", vmi.Name, virtv1.MigrationPending, addedNodeSelector)
+
+			addMigration(migration)
+			addVirtualMachineInstance(vmi)
+			addPod(newSourcePodForVirtualMachine(vmi))
+
+			controller.Execute()
+
+			testutils.ExpectEvent(recorder, virtcontroller.SuccessfulCreatePodReason)
+			expectPodCreation(vmi.Namespace, vmi.UID, migration.UID, 1, 0, 2)
+			targetPod, err := getTargetPod(vmi.Namespace, vmi.UID, migration.UID)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(targetPod).ToNot(BeNil())
+			Expect(targetPod.Spec.Affinity).ToNot(BeNil())
+			Expect(targetPod.Spec.Affinity.PodAntiAffinity).ToNot(BeNil())
+			Expect(targetPod.Spec.Affinity.NodeAffinity).ToNot(BeNil())
+			Expect(targetPod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution).ToNot(BeNil())
+
+			By("Expecting migration target pod to contain all the NodeSelector labels defined on the VM")
+			for k, v := range vmiNodeSelector {
+				Expect(targetPod.Spec.NodeSelector).To(HaveKeyWithValue(k, v))
+			}
+			for k, v := range addedNodeSelector {
+				vmiVal, ok := vmiNodeSelector[k]
+				if ok {
+					Expect(targetPod.Spec.NodeSelector).To(HaveKeyWithValue(k, vmiVal))
+				} else {
+					Expect(targetPod.Spec.NodeSelector).To(HaveKeyWithValue(k, v))
+				}
+			}
+
+		})
+
 		It("should place migration in scheduling state if pod exists", func() {
 			vmi := newVirtualMachine("testvmi", virtv1.Running)
 			migration := newMigration("testmigration", vmi.Name, virtv1.MigrationPending)
@@ -2276,6 +2370,12 @@ func newMigration(name string, vmiName string, phase virtv1.VirtualMachineInstan
 	return migration
 }
 
+func newMigrationWithAddedNodeSelector(name string, vmiName string, phase virtv1.VirtualMachineInstanceMigrationPhase, addedNodeSelector map[string]string) *virtv1.VirtualMachineInstanceMigration {
+	migration := newMigration(name, vmiName, phase)
+	migration.Spec.AddedNodeSelector = addedNodeSelector
+	return migration
+}
+
 func newVirtualMachine(name string, phase virtv1.VirtualMachineInstancePhase) *virtv1.VirtualMachineInstance {
 	vmi := api.NewMinimalVMI(name)
 	vmi.UID = types.UID(name)

pkg/virt-operator/resource/generate/components/validations_generated.go

@@ -13908,6 +13908,17 @@ var CRDsValidation map[string]string = map[string]string{
       type: object
     spec:
       properties:
+        addedNodeSelector:
+          additionalProperties:
+            type: string
+          description: |-
+            AddedNodeSelector is an additional selector that can be used to
+            complement a NodeSelector or NodeAffinity as set on the VM
+            to restrict the set of allowed target nodes for a migration.
+            In case of key collisions, values set on the VM objects
+            are going to be preserved to ensure that addedNodeSelector
+            can only restrict but not bypass constraints already set on the VM object.
+          type: object
         vmiName:
           description: The name of the VMI to perform the migration on. VMI must exist
             in the migration objects namespace

pkg/virtctl/vm/migrate.go

@@ -33,6 +33,8 @@ import (
 
 const COMMAND_MIGRATE = "migrate"
 
+var nodeName string
+
 func NewMigrateCommand() *cobra.Command {
 	c := Command{command: COMMAND_MIGRATE}
 	cmd := &cobra.Command{
@@ -42,6 +44,8 @@ func NewMigrateCommand() *cobra.Command {
 		Args:    cobra.ExactArgs(1),
 		RunE:    c.migrateRun,
 	}
+
+	cmd.Flags().StringVar(&nodeName, "nodeName", nodeName, "--nodeName=<nodeName>: Flag to migrate this VM to a specific node regardless of its affinity rules. If it's omitted, recommended, the scheduler becomes responsible for finding the best Node to migrate the VM to.")
 	cmd.Flags().BoolVar(&dryRun, dryRunArg, false, dryRunCommandUsage)
 	cmd.SetUsageTemplate(templates.UsageTemplate())
 	return cmd
@@ -57,7 +61,13 @@ func (o *Command) migrateRun(cmd *cobra.Command, args []string) error {
 
 	dryRunOption := setDryRunOption(dryRun)
 
-	err = virtClient.VirtualMachine(namespace).Migrate(context.Background(), vmiName, &v1.MigrateOptions{DryRun: dryRunOption})
+	options := &v1.MigrateOptions{DryRun: dryRunOption}
+
+	if nodeName != "" {
+		options.AddedNodeSelector = map[string]string{"kubernetes.io/hostname": nodeName}
+	}
+
+	err = virtClient.VirtualMachine(namespace).Migrate(context.Background(), vmiName, options)
 	if err != nil {
 		return fmt.Errorf("Error migrating VirtualMachine %v", err)
 	}

pkg/virtctl/vm/migrate_test.go

@@ -53,19 +53,35 @@ var _ = Describe("Migrate command", func() {
 		Expect(err).Should(MatchError("accepts 1 arg(s), received 0"))
 	})
 
-	DescribeTable("should migrate a vm according to options", func(migrateOptions *v1.MigrateOptions) {
+	DescribeTable("should migrate a vm according to options", func(expectedMigrateOptions *v1.MigrateOptions, extraArgs ...string) {
 		vm := kubecli.NewMinimalVM(vmName)
 
 		kubecli.MockKubevirtClientInstance.EXPECT().VirtualMachine(k8smetav1.NamespaceDefault).Return(vmInterface).Times(1)
-		vmInterface.EXPECT().Migrate(context.Background(), vm.Name, migrateOptions).Return(nil).Times(1)
+		vmInterface.EXPECT().Migrate(context.Background(), vm.Name, expectedMigrateOptions).Return(nil).Times(1)
 
 		args := []string{"migrate", vmName}
-		if len(migrateOptions.DryRun) > 0 {
-			args = append(args, "--dry-run")
-		}
+		args = append(args, extraArgs...)
 		Expect(testing.NewRepeatableVirtctlCommand(args...)()).To(Succeed())
 	},
-		Entry("with default", &v1.MigrateOptions{}),
-		Entry("with dry-run option", &v1.MigrateOptions{DryRun: []string{k8smetav1.DryRunAll}}),
+		Entry(
+			"with default",
+			&v1.MigrateOptions{}),
+		Entry(
+			"with dry-run option",
+			&v1.MigrateOptions{
+				DryRun: []string{k8smetav1.DryRunAll}},
+			"--dry-run"),
+		Entry(
+			"with nodeName option",
+			&v1.MigrateOptions{
+				AddedNodeSelector: map[string]string{"kubernetes.io/hostname": "test.example.com"}},
+			"--nodeName", "test.example.com"),
+		Entry(
+			"with dry-run and nodeName options",
+			&v1.MigrateOptions{
+				AddedNodeSelector: map[string]string{"kubernetes.io/hostname": "test.example.com"},
+				DryRun:            []string{k8smetav1.DryRunAll}},
+			"--dry-run", "--nodeName", "test.example.com"),
 	)
+
 })

staging/src/kubevirt.io/api/core/v1/types.go

@@ -1381,6 +1381,15 @@ type VirtualMachineInstanceMigrationList struct {
 type VirtualMachineInstanceMigrationSpec struct {
 	// The name of the VMI to perform the migration on. VMI must exist in the migration objects namespace
 	VMIName string `json:"vmiName,omitempty" valid:"required"`
+
+	// AddedNodeSelector is an additional selector that can be used to
+	// complement a NodeSelector or NodeAffinity as set on the VM
+	// to restrict the set of allowed target nodes for a migration.
+	// In case of key collisions, values set on the VM objects
+	// are going to be preserved to ensure that addedNodeSelector
+	// can only restrict but not bypass constraints already set on the VM object.
+	// +optional
+	AddedNodeSelector map[string]string `json:"addedNodeSelector,omitempty"`
 }
 
 // VirtualMachineInstanceMigrationPhaseTransitionTimestamp gives a timestamp in relation to when a phase is set on a vmi
@@ -2273,6 +2282,15 @@ type MigrateOptions struct {
 	// +optional
 	// +listType=atomic
 	DryRun []string `json:"dryRun,omitempty" protobuf:"bytes,1,rep,name=dryRun"`
+
+	// AddedNodeSelector is an additional selector that can be used to
+	// complement a NodeSelector or NodeAffinity as set on the VM
+	// to restrict the set of allowed target nodes for a migration.
+	// In case of key collisions, values set on the VM objects
+	// are going to be preserved to ensure that addedNodeSelector
+	// can only restrict but not bypass constraints already set on the VM object.
+	// +optional
+	AddedNodeSelector map[string]string `json:"addedNodeSelector,omitempty"`
 }
 
 // VirtualMachineInstanceGuestAgentInfo represents information from the installed guest agent