Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Always show applied migration configuration in VMI status #7267

Merged
merged 5 commits into from
Mar 24, 2022
Merged

Always show applied migration configuration in VMI status #7267

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6df8485
Update VMI status' MigrationConfiguration on every migration
iholder101 Feb 23, 2022
ee9c010
Add functional tests
iholder101 Feb 23, 2022
ea70f87
Adjust unit tests
iholder101 Feb 23, 2022
a8c601a
getMigrationConfigPatch to support getting custom configs
iholder101 Feb 23, 2022
86a7e6a
Use DeepCopy() to copy migration configs
iholder101 Feb 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 17 additions & 4 deletions pkg/virt-controller/watch/migration.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -671,11 +671,16 @@ func (c *MigrationController) handleTargetPodHandoff(migration *virtv1.VirtualMa
}
}

err := c.matchMigrationPolicy(vmiCopy, c.clusterConfig.GetMigrationConfiguration())
clusterMigrationConfigs := c.clusterConfig.GetMigrationConfiguration().DeepCopy()
err := c.matchMigrationPolicy(vmiCopy, clusterMigrationConfigs)
if err != nil {
return fmt.Errorf("failed to match migration policy: %v", err)
}

if !c.isMigrationPolicyMatched(vmiCopy) {
vmiCopy.Status.MigrationState.MigrationConfiguration = clusterMigrationConfigs
}

err = c.patchVMI(vmi, vmiCopy)
if err != nil {
c.recorder.Eventf(migration, k8sv1.EventTypeWarning, FailedHandOverPodReason, fmt.Sprintf("Failed to set MigrationStat in VMI status. :%v", err))
Expand Down Expand Up @@ -1588,17 +1593,25 @@ func (c *MigrationController) matchMigrationPolicy(vmi *virtv1.VirtualMachineIns
return nil
}

migrationConfigCopy := *clusterMigrationConfiguration
isUpdated, err := matchedPolicy.GetMigrationConfByPolicy(&migrationConfigCopy)
isUpdated, err := matchedPolicy.GetMigrationConfByPolicy(clusterMigrationConfiguration)
if err != nil {
return err
}

if isUpdated {
vmi.Status.MigrationState.MigrationPolicyName = &matchedPolicy.Name
vmi.Status.MigrationState.MigrationConfiguration = &migrationConfigCopy
vmi.Status.MigrationState.MigrationConfiguration = clusterMigrationConfiguration
log.Log.Object(vmi).Infof("migration is updated by migration policy named %s.", matchedPolicy.Name)
}

return nil
}

func (c *MigrationController) isMigrationPolicyMatched(vmi *virtv1.VirtualMachineInstance) bool {
if vmi == nil {
return false
}

migrationPolicyName := vmi.Status.MigrationState.MigrationPolicyName
return migrationPolicyName != nil && *migrationPolicyName != ""
}
38 changes: 27 additions & 11 deletions pkg/virt-controller/watch/migration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,24 @@ var _ = Describe("Migration watcher", func() {
}
}

getMigrationConfigPatch := func(customConfigs ...*virtv1.MigrationConfiguration) string {
Expect(customConfigs).To(Or(BeEmpty(), HaveLen(1)))

var migrationConfiguration *virtv1.MigrationConfiguration

if len(customConfigs) > 0 && customConfigs[0] != nil {
migrationConfiguration = customConfigs[0]
} else {
migrationConfiguration = controller.clusterConfig.GetMigrationConfiguration()
Expect(migrationConfiguration).ToNot(BeNil())
}

marshalledConfigs, err := json.Marshal(migrationConfiguration)
Expect(err).ToNot(HaveOccurred())

return fmt.Sprintf(`"migrationConfiguration":%s`, string(marshalledConfigs))
}

Context("Migration with hotplug volumes", func() {
var (
vmi *virtv1.VirtualMachineInstance
Expand Down Expand Up @@ -400,7 +418,7 @@ var _ = Describe("Migration watcher", func() {
podFeeder.Add(targetPod)
podFeeder.Add(attachmentPod)

patch := fmt.Sprintf(`[{ "op": "add", "path": "/status/migrationState", "value": {"targetNode":"node01","targetPod":"%s","targetAttachmentPodUID":"%s","sourceNode":"node02","migrationUid":"testmigration"} }, { "op": "test", "path": "/metadata/labels", "value": {} }, { "op": "replace", "path": "/metadata/labels", "value": {"kubevirt.io/migrationTargetNodeName":"node01"} }]`, targetPod.Name, attachmentPod.UID)
patch := fmt.Sprintf(`[{ "op": "add", "path": "/status/migrationState", "value": {"targetNode":"node01","targetPod":"%s","targetAttachmentPodUID":"%s","sourceNode":"node02","migrationUid":"testmigration",%s} }, { "op": "test", "path": "/metadata/labels", "value": {} }, { "op": "replace", "path": "/metadata/labels", "value": {"kubevirt.io/migrationTargetNodeName":"node01"} }]`, targetPod.Name, attachmentPod.UID, getMigrationConfigPatch())

shouldExpectVirtualMachineInstancePatch(vmi, patch)

Expand Down Expand Up @@ -916,7 +934,7 @@ var _ = Describe("Migration watcher", func() {
addVirtualMachineInstance(vmi)
podFeeder.Add(pod)

patch := fmt.Sprintf(`[{ "op": "add", "path": "/status/migrationState", "value": {"targetNode":"node01","targetPod":"%s","sourceNode":"node02","migrationUid":"testmigration"} }, { "op": "test", "path": "/metadata/labels", "value": {} }, { "op": "replace", "path": "/metadata/labels", "value": {"kubevirt.io/migrationTargetNodeName":"node01"} }]`, pod.Name)
patch := fmt.Sprintf(`[{ "op": "add", "path": "/status/migrationState", "value": {"targetNode":"node01","targetPod":"%s","sourceNode":"node02","migrationUid":"testmigration",%s} }, { "op": "test", "path": "/metadata/labels", "value": {} }, { "op": "replace", "path": "/metadata/labels", "value": {"kubevirt.io/migrationTargetNodeName":"node01"} }]`, pod.Name, getMigrationConfigPatch())

shouldExpectVirtualMachineInstancePatch(vmi, patch)

Expand Down Expand Up @@ -972,7 +990,7 @@ var _ = Describe("Migration watcher", func() {
addVirtualMachineInstance(vmi)
podFeeder.Add(pod)

patch := fmt.Sprintf(`[{ "op": "add", "path": "/status/migrationState", "value": {"targetNode":"node01","targetPod":"%s","sourceNode":"node02","migrationUid":"testmigration"} }, { "op": "test", "path": "/metadata/labels", "value": {} }, { "op": "replace", "path": "/metadata/labels", "value": {"kubevirt.io/migrationTargetNodeName":"node01"} }]`, pod.Name)
patch := fmt.Sprintf(`[{ "op": "add", "path": "/status/migrationState", "value": {"targetNode":"node01","targetPod":"%s","sourceNode":"node02","migrationUid":"testmigration",%s} }, { "op": "test", "path": "/metadata/labels", "value": {} }, { "op": "replace", "path": "/metadata/labels", "value": {"kubevirt.io/migrationTargetNodeName":"node01"} }]`, pod.Name, getMigrationConfigPatch())
shouldExpectVirtualMachineInstancePatch(vmi, patch)

controller.Execute()
Expand All @@ -996,7 +1014,7 @@ var _ = Describe("Migration watcher", func() {
addVirtualMachineInstance(vmi)
podFeeder.Add(pod)

patch := fmt.Sprintf(`[{ "op": "test", "path": "/status/migrationState", "value": {"migrationUid":"1111-2222-3333-4444"} }, { "op": "replace", "path": "/status/migrationState", "value": {"targetNode":"node01","targetPod":"%s","sourceNode":"node02","migrationUid":"testmigration"} }, { "op": "test", "path": "/metadata/labels", "value": {} }, { "op": "replace", "path": "/metadata/labels", "value": {"kubevirt.io/migrationTargetNodeName":"node01"} }]`, pod.Name)
patch := fmt.Sprintf(`[{ "op": "test", "path": "/status/migrationState", "value": {"migrationUid":"1111-2222-3333-4444"} }, { "op": "replace", "path": "/status/migrationState", "value": {"targetNode":"node01","targetPod":"%s","sourceNode":"node02","migrationUid":"testmigration",%s} }, { "op": "test", "path": "/metadata/labels", "value": {} }, { "op": "replace", "path": "/metadata/labels", "value": {"kubevirt.io/migrationTargetNodeName":"node01"} }]`, pod.Name, getMigrationConfigPatch())

shouldExpectVirtualMachineInstancePatch(vmi, patch)

Expand Down Expand Up @@ -1343,18 +1361,16 @@ var _ = Describe("Migration watcher", func() {
var pod *k8sv1.Pod

getExpectedVmiPatch := func(expectConfigUpdate bool, expectedConfigs *virtv1.MigrationConfiguration, migrationPolicy *migrationsv1.MigrationPolicy) string {
var migrationPolicyPatch string
if expectConfigUpdate {
marshalledConfigs, err := json.Marshal(*expectedConfigs)
Expect(err).ShouldNot(HaveOccurred())
var migrationPolicyNamePatch string

migrationPolicyPatch = fmt.Sprintf(`,"migrationPolicyName":"%s","migrationConfiguration":%s`, migrationPolicy.Name, string(marshalledConfigs))
if expectConfigUpdate {
migrationPolicyNamePatch = fmt.Sprintf(`,"migrationPolicyName":"%s"`, migrationPolicy.Name)
}

patch := fmt.Sprintf(`[{ "op": "add", "path": "/status/migrationState", `+
`"value": {"targetNode":"node01","targetPod":"%s","sourceNode":"tefwegwrerg","migrationUid":"testmigration"%s} }, `+
`"value": {"targetNode":"node01","targetPod":"%s","sourceNode":"tefwegwrerg","migrationUid":"testmigration"%s,%s} }, `+
`{ "op": "test", "path": "/metadata/labels", "value": {"mp-key-0":"mp-value-0"} }, `+
`{ "op": "replace", "path": "/metadata/labels", "value": {"kubevirt.io/migrationTargetNodeName":"node01","mp-key-0":"mp-value-0"} }]`, pod.Name, migrationPolicyPatch)
`{ "op": "replace", "path": "/metadata/labels", "value": {"kubevirt.io/migrationTargetNodeName":"node01","mp-key-0":"mp-value-0"} }]`, pod.Name, migrationPolicyNamePatch, getMigrationConfigPatch(expectedConfigs))

return patch
}
Expand Down
7 changes: 7 additions & 0 deletions pkg/virt-handler/vm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2347,6 +2347,13 @@ func (d *VirtualMachineController) vmUpdateHelperMigrationSource(origVMI *v1.Vir
AllowPostCopy: *migrationConfiguration.AllowPostCopy,
}

marshalledOptions, err := json.Marshal(options)
if err != nil {
log.Log.Object(vmi).Warning("failed to marshall matched migration options")
} else {
log.Log.Object(vmi).Infof("migration options matched for vmi %s: %s", vmi.Name, string(marshalledOptions))
}

err = client.MigrateVirtualMachine(vmi, options)
if err != nil {
return err
Expand Down
33 changes: 29 additions & 4 deletions tests/migration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2785,10 +2785,6 @@ var _ = Describe("[rfe_id:393][crit:high][vendor:cnv-qe@redhat.com][level:system
Context("[Serial] with migration policies", func() {

confirmMigrationPolicyName := func(vmi *v1.VirtualMachineInstance, expectedName *string) {
By("Retrieving the VMI post migration")
vmi, err = virtClient.VirtualMachineInstance(vmi.Namespace).Get(vmi.Name, &metav1.GetOptions{})
Expect(err).ToNot(HaveOccurred())

By("Verifying the VMI's configuration source")
if expectedName == nil {
Expect(vmi.Status.MigrationState.MigrationPolicyName).To(BeNil())
Expand Down Expand Up @@ -2834,6 +2830,12 @@ var _ = Describe("[rfe_id:393][crit:high][vendor:cnv-qe@redhat.com][level:system

// check VMI, confirm migration state
tests.ConfirmVMIPostMigration(virtClient, vmi, migrationUID)

By("Retrieving the VMI post migration")
vmi, err = virtClient.VirtualMachineInstance(vmi.Namespace).Get(vmi.Name, &metav1.GetOptions{})
Expect(err).ToNot(HaveOccurred())

Expect(vmi.Status.MigrationState.MigrationConfiguration).ToNot(BeNil())
confirmMigrationPolicyName(vmi, expectedPolicyName)
},
table.Entry("should override cluster-wide policy if defined", true),
Expand Down Expand Up @@ -3865,6 +3867,29 @@ var _ = Describe("[rfe_id:393][crit:high][vendor:cnv-qe@redhat.com][level:system
tests.WaitForVirtualMachineToDisappearWithTimeout(vmi, 240)
})
})

It("should update MigrationState's MigrationConfiguration of VMI status", func() {
By("Starting a VMI")
vmi := tests.NewRandomVMIWithEphemeralDisk(cd.ContainerDiskFor(cd.ContainerDiskAlpine))
vmi = runVMIAndExpectLaunch(vmi, 240)

By("Starting a Migration")
migration := tests.NewRandomMigration(vmi.Name, vmi.Namespace)
migrationUID := tests.RunMigrationAndExpectCompletion(virtClient, migration, 180)
tests.ConfirmVMIPostMigration(virtClient, vmi, migrationUID)

By("Ensuring MigrationConfiguration is updated")
vmi, err = virtClient.VirtualMachineInstance(vmi.Namespace).Get(vmi.Name, &metav1.GetOptions{})
Expect(err).ToNot(HaveOccurred())
Expect(vmi.Status.MigrationState).ToNot(BeNil())
Expect(vmi.Status.MigrationState.MigrationConfiguration).ToNot(BeNil())

By("Deleting the VMI")
Expect(virtClient.VirtualMachineInstance(vmi.Namespace).Delete(vmi.Name, &metav1.DeleteOptions{})).To(Succeed())

By("Waiting for VMI to disappear")
tests.WaitForVirtualMachineToDisappearWithTimeout(vmi, 120)
})
})

func fedoraVMIWithEvictionStrategy() *v1.VirtualMachineInstance {
Expand Down