From 87854cf1cad56e59b7448c42caf0f1288e9753d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=A0=E5=BF=A0=E7=90=B3?= Date: Fri, 10 Nov 2023 15:05:29 +0800 Subject: [PATCH] WIP: live migration to a named node Follow-up and derived from: https://github.com/kubevirt/kubevirt/pull/10712 Implements: https://github.com/kubevirt/community/pull/320 TODO: add functional tests Signed-off-by: zhonglin6666 Signed-off-by: Simone Tiraboschi --- api/openapi-spec/swagger.json | 16 +++ pkg/virt-api/rest/subresource.go | 3 +- .../watch/migration/migration.go | 6 ++ .../watch/migration/migration_test.go | 100 ++++++++++++++++++ .../components/validations_generated.go | 11 ++ pkg/virtctl/vm/migrate.go | 12 ++- pkg/virtctl/vm/migrate_test.go | 30 ++++-- .../api/core/v1/deepcopy_generated.go | 16 ++- staging/src/kubevirt.io/api/core/v1/types.go | 18 ++++ .../api/core/v1/types_swagger_generated.go | 8 +- .../client-go/api/openapi_generated.go | 32 ++++++ 11 files changed, 239 insertions(+), 13 deletions(-) diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json index db20fc5c5d78..2428f4a9cecc 100644 --- a/api/openapi-spec/swagger.json +++ b/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" diff --git a/pkg/virt-api/rest/subresource.go b/pkg/virt-api/rest/subresource.go index 973b267683b6..6a4105c62cdc 100644 --- a/pkg/virt-api/rest/subresource.go +++ b/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 { diff --git a/pkg/virt-controller/watch/migration/migration.go b/pkg/virt-controller/watch/migration/migration.go index dc6f75cf7dfe..e7a5e72dbdb6 100644 --- a/pkg/virt-controller/watch/migration/migration.go +++ b/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 diff --git a/pkg/virt-controller/watch/migration/migration_test.go b/pkg/virt-controller/watch/migration/migration_test.go index 725114a2dcb1..4fbb3f1bbdb6 100644 --- a/pkg/virt-controller/watch/migration/migration_test.go +++ b/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) diff --git a/pkg/virt-operator/resource/generate/components/validations_generated.go b/pkg/virt-operator/resource/generate/components/validations_generated.go index bc9953a0cf0b..f5cfa26cf9b2 100644 --- a/pkg/virt-operator/resource/generate/components/validations_generated.go +++ b/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 diff --git a/pkg/virtctl/vm/migrate.go b/pkg/virtctl/vm/migrate.go index b72ddec735bd..ec476aaab915 100644 --- a/pkg/virtctl/vm/migrate.go +++ b/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=: Flag to migrate this VM to a specific node (according to label \"kubernetes.io/hostname\"). If 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) } diff --git a/pkg/virtctl/vm/migrate_test.go b/pkg/virtctl/vm/migrate_test.go index 2f9a75af1da9..ab7b9714dae7 100644 --- a/pkg/virtctl/vm/migrate_test.go +++ b/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"), ) + }) diff --git a/staging/src/kubevirt.io/api/core/v1/deepcopy_generated.go b/staging/src/kubevirt.io/api/core/v1/deepcopy_generated.go index 1e45d6b30a7b..1f7bfd8ff074 100644 --- a/staging/src/kubevirt.io/api/core/v1/deepcopy_generated.go +++ b/staging/src/kubevirt.io/api/core/v1/deepcopy_generated.go @@ -3114,6 +3114,13 @@ func (in *MigrateOptions) DeepCopyInto(out *MigrateOptions) { *out = make([]string, len(*in)) copy(*out, *in) } + if in.AddedNodeSelector != nil { + in, out := &in.AddedNodeSelector, &out.AddedNodeSelector + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } return } @@ -5064,7 +5071,7 @@ func (in *VirtualMachineInstanceMigration) DeepCopyInto(out *VirtualMachineInsta *out = *in out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - out.Spec = in.Spec + in.Spec.DeepCopyInto(&out.Spec) in.Status.DeepCopyInto(&out.Status) return } @@ -5158,6 +5165,13 @@ func (in *VirtualMachineInstanceMigrationPhaseTransitionTimestamp) DeepCopy() *V // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *VirtualMachineInstanceMigrationSpec) DeepCopyInto(out *VirtualMachineInstanceMigrationSpec) { *out = *in + if in.AddedNodeSelector != nil { + in, out := &in.AddedNodeSelector, &out.AddedNodeSelector + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } return } diff --git a/staging/src/kubevirt.io/api/core/v1/types.go b/staging/src/kubevirt.io/api/core/v1/types.go index 3e17c95fe85c..9c3dc7cca8d1 100644 --- a/staging/src/kubevirt.io/api/core/v1/types.go +++ b/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 diff --git a/staging/src/kubevirt.io/api/core/v1/types_swagger_generated.go b/staging/src/kubevirt.io/api/core/v1/types_swagger_generated.go index 5b48775a7353..48b86805a159 100644 --- a/staging/src/kubevirt.io/api/core/v1/types_swagger_generated.go +++ b/staging/src/kubevirt.io/api/core/v1/types_swagger_generated.go @@ -316,7 +316,8 @@ func (VirtualMachineInstanceMigrationList) SwaggerDoc() map[string]string { func (VirtualMachineInstanceMigrationSpec) SwaggerDoc() map[string]string { return map[string]string{ - "vmiName": "The name of the VMI to perform the migration on. VMI must exist in the migration objects namespace", + "vmiName": "The name of the VMI to perform the migration on. VMI must exist in the migration objects namespace", + "addedNodeSelector": "AddedNodeSelector is an additional selector that can be used to\ncomplement a NodeSelector or NodeAffinity as set on the VM\nto restrict the set of allowed target nodes for a migration.\nIn case of key collisions, values set on the VM objects\nare going to be preserved to ensure that addedNodeSelector\ncan only restrict but not bypass constraints already set on the VM object.\n+optional", } } @@ -624,8 +625,9 @@ func (StopOptions) SwaggerDoc() map[string]string { func (MigrateOptions) SwaggerDoc() map[string]string { return map[string]string{ - "": "MigrateOptions may be provided on migrate request.", - "dryRun": "When present, indicates that modifications should not be\npersisted. An invalid or unrecognized dryRun directive will\nresult in an error response and no further processing of the\nrequest. Valid values are:\n- All: all dry run stages will be processed\n+optional\n+listType=atomic", + "": "MigrateOptions may be provided on migrate request.", + "dryRun": "When present, indicates that modifications should not be\npersisted. An invalid or unrecognized dryRun directive will\nresult in an error response and no further processing of the\nrequest. Valid values are:\n- All: all dry run stages will be processed\n+optional\n+listType=atomic", + "addedNodeSelector": "AddedNodeSelector is an additional selector that can be used to\ncomplement a NodeSelector or NodeAffinity as set on the VM\nto restrict the set of allowed target nodes for a migration.\nIn case of key collisions, values set on the VM objects\nare going to be preserved to ensure that addedNodeSelector\ncan only restrict but not bypass constraints already set on the VM object.\n+optional", } } diff --git a/staging/src/kubevirt.io/client-go/api/openapi_generated.go b/staging/src/kubevirt.io/client-go/api/openapi_generated.go index a5b51b9936c4..2d5ca506e115 100644 --- a/staging/src/kubevirt.io/client-go/api/openapi_generated.go +++ b/staging/src/kubevirt.io/client-go/api/openapi_generated.go @@ -22254,6 +22254,22 @@ func schema_kubevirtio_api_core_v1_MigrateOptions(ref common.ReferenceCallback) }, }, }, + "addedNodeSelector": { + SchemaProps: spec.SchemaProps{ + 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: []string{"object"}, + AdditionalProperties: &spec.SchemaOrBool{ + Allows: true, + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Default: "", + Type: []string{"string"}, + Format: "", + }, + }, + }, + }, + }, }, }, }, @@ -25567,6 +25583,22 @@ func schema_kubevirtio_api_core_v1_VirtualMachineInstanceMigrationSpec(ref commo Format: "", }, }, + "addedNodeSelector": { + SchemaProps: spec.SchemaProps{ + 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: []string{"object"}, + AdditionalProperties: &spec.SchemaOrBool{ + Allows: true, + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Default: "", + Type: []string{"string"}, + Format: "", + }, + }, + }, + }, + }, }, }, },