From 08c9724e9dc1751a8b113f636fa602829a3af397 Mon Sep 17 00:00:00 2001 From: Vladik Romanovsky Date: Tue, 5 Feb 2019 10:11:40 -0500 Subject: [PATCH 01/16] Add libvirt GetJobStats and AbortJob to track and cancel live migration Signed-off-by: Vladik Romanovsky --- .../virtwrap/cli/generated_mock_libvirt.go | 21 +++++++++++++++++++ pkg/virt-launcher/virtwrap/cli/libvirt.go | 2 ++ 2 files changed, 23 insertions(+) diff --git a/pkg/virt-launcher/virtwrap/cli/generated_mock_libvirt.go b/pkg/virt-launcher/virtwrap/cli/generated_mock_libvirt.go index cae0f8d22d04..5cf7df250867 100644 --- a/pkg/virt-launcher/virtwrap/cli/generated_mock_libvirt.go +++ b/pkg/virt-launcher/virtwrap/cli/generated_mock_libvirt.go @@ -368,6 +368,27 @@ func (_mr *_MockVirDomainRecorder) MemoryStats(arg0, arg1 interface{}) *gomock.C return _mr.mock.ctrl.RecordCall(_mr.mock, "MemoryStats", arg0, arg1) } +func (_m *MockVirDomain) GetJobStats(flags libvirt_go.DomainGetJobStatsFlags) (*libvirt_go.DomainJobInfo, error) { + ret := _m.ctrl.Call(_m, "GetJobStats", flags) + ret0, _ := ret[0].(*libvirt_go.DomainJobInfo) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +func (_mr *_MockVirDomainRecorder) GetJobStats(arg0 interface{}) *gomock.Call { + return _mr.mock.ctrl.RecordCall(_mr.mock, "GetJobStats", arg0) +} + +func (_m *MockVirDomain) AbortJob() error { + ret := _m.ctrl.Call(_m, "AbortJob") + ret0, _ := ret[0].(error) + return ret0 +} + +func (_mr *_MockVirDomainRecorder) AbortJob() *gomock.Call { + return _mr.mock.ctrl.RecordCall(_mr.mock, "AbortJob") +} + func (_m *MockVirDomain) Free() error { ret := _m.ctrl.Call(_m, "Free") ret0, _ := ret[0].(error) diff --git a/pkg/virt-launcher/virtwrap/cli/libvirt.go b/pkg/virt-launcher/virtwrap/cli/libvirt.go index 057ea772ae16..330314f7a17d 100644 --- a/pkg/virt-launcher/virtwrap/cli/libvirt.go +++ b/pkg/virt-launcher/virtwrap/cli/libvirt.go @@ -330,6 +330,8 @@ type VirDomain interface { OpenConsole(devname string, stream *libvirt.Stream, flags libvirt.DomainConsoleFlags) error MigrateToURI3(string, *libvirt.DomainMigrateParameters, libvirt.DomainMigrateFlags) error MemoryStats(nrStats uint32, flags uint32) ([]libvirt.DomainMemoryStat, error) + GetJobStats(flags libvirt.DomainGetJobStatsFlags) (*libvirt.DomainJobInfo, error) + AbortJob() error Free() error } From 7a85d5314d95586d1631a247ee7bf84579844fa9 Mon Sep 17 00:00:00 2001 From: Vladik Romanovsky Date: Thu, 13 Dec 2018 23:50:39 -0500 Subject: [PATCH 02/16] track live migration process and cancel stale migrations Signed-off-by: Vladik Romanovsky --- pkg/virt-launcher/virtwrap/manager.go | 108 ++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/pkg/virt-launcher/virtwrap/manager.go b/pkg/virt-launcher/virtwrap/manager.go index f33419194e68..51bf01a0e01c 100644 --- a/pkg/virt-launcher/virtwrap/manager.go +++ b/pkg/virt-launcher/virtwrap/manager.go @@ -29,6 +29,7 @@ import ( "encoding/xml" "fmt" "os" + "path/filepath" "strings" "sync" "time" @@ -36,6 +37,8 @@ import ( eventsclient "kubevirt.io/kubevirt/pkg/virt-launcher/notify-client" libvirt "github.com/libvirt/libvirt-go" + k8sv1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilwait "k8s.io/apimachinery/pkg/util/wait" @@ -365,6 +368,111 @@ func (l *LibvirtDomainManager) asyncMigrate(vmi *v1.VirtualMachineInstance) { }(l, vmi) } +func getVMIEphemeralDisksTotalSize() *resource.Quantity { + var baseDir = "/var/run/libvirt/kubevirt-ephemeral-disk" + totalSize := int64(0) + err := filepath.Walk(baseDir, func(path string, f os.FileInfo, err error) error { + if !f.IsDir() { + totalSize += f.Size() + } + return err + }) + if err != nil { + log.Log.Reason(err).Warning("failed to get VMI ephemeral disks size") + return &resource.Quantity{Format: resource.BinarySI} + } + + return resource.NewScaledQuantity(totalSize, 0) +} + +func getVMIMigrationDataSize(vmi *v1.VirtualMachineInstance) int64 { + var memory resource.Quantity + + // Take memory from the requested memory + if v, ok := vmi.Spec.Domain.Resources.Requests[k8sv1.ResourceMemory]; ok { + memory = v + } + // In case that guest memory is explicitly set, override it + if vmi.Spec.Domain.Memory != nil && vmi.Spec.Domain.Memory.Guest != nil { + memory = *vmi.Spec.Domain.Memory.Guest + } + + //get total data Size + if vmi.Status.MigrationMethod == v1.BlockMigration { + disksSize := getVMIEphemeralDisksTotalSize() + memory.Add(*disksSize) + } + return memory.ScaledValue(resource.Giga) +} + +func liveMigrationMonitor(vmi *v1.VirtualMachineInstance, dom cli.VirDomain) { + logger := log.Log.Object(vmi) + start := time.Now().UTC().Unix() + lastProgressUpdate := start + progressWatermark := int64(0) + // TODO:(vladikr) move to configMap + progressTimeout := int64(150) + + // TODO:(vladikr) move to configMap + completionTimeoutPerGiB := int64(800) + acceptableCompletionTime := completionTimeoutPerGiB * getVMIMigrationDataSize(vmi) + for { + stats, err := dom.GetJobStats(0) + if err != nil { + logger.Reason(err).Error("failed to get domain job info") + break + } + remainingData := int64(stats.DataRemaining) + switch stats.Type { + case libvirt.DOMAIN_JOB_UNBOUNDED: + // Migration is running + now := time.Now().UTC().Unix() + elapsed := now - start + + if (progressWatermark == 0) || + (progressWatermark > remainingData) { + progressWatermark = remainingData + lastProgressUpdate = now + } + // check if the migration is progressing + progressDelay := now - lastProgressUpdate + if progressTimeout != 0 && + progressDelay > progressTimeout { + logger.Warningf("Live migration stuck for %d sec", progressDelay) + err := dom.AbortJob() + if err != nil { + logger.Reason(err).Error("failed to abort migration") + } + } + + // check the overall migration time + if acceptableCompletionTime != 0 && + elapsed > acceptableCompletionTime { + logger.Warningf("Live migration not completed after %d sec", + acceptableCompletionTime) + err := dom.AbortJob() + if err != nil { + logger.Reason(err).Error("failed to abort migration") + } + } + + case libvirt.DOMAIN_JOB_NONE: + logger.Info("Migration job didn't start yet") + case libvirt.DOMAIN_JOB_COMPLETED: + logger.Info("Migration has beem completed") + break + case libvirt.DOMAIN_JOB_FAILED: + logger.Info("Migration job failed") + // migration failed + break + case libvirt.DOMAIN_JOB_CANCELLED: + logger.Info("Migration was canceled") + break + } + time.Sleep(500 * time.Millisecond) + } +} + func (l *LibvirtDomainManager) MigrateVMI(vmi *v1.VirtualMachineInstance) error { if vmi.Status.MigrationState == nil { From 89266080705d2bbd21b37eb1d4b83f75823fa4a6 Mon Sep 17 00:00:00 2001 From: Vladik Romanovsky Date: Thu, 13 Dec 2018 23:57:15 -0500 Subject: [PATCH 03/16] start using the live migration tracking Signed-off-by: Vladik Romanovsky --- pkg/virt-launcher/virtwrap/manager.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/virt-launcher/virtwrap/manager.go b/pkg/virt-launcher/virtwrap/manager.go index 51bf01a0e01c..7db656848947 100644 --- a/pkg/virt-launcher/virtwrap/manager.go +++ b/pkg/virt-launcher/virtwrap/manager.go @@ -355,6 +355,8 @@ func (l *LibvirtDomainManager) asyncMigrate(vmi *v1.VirtualMachineInstance) { params.MigrateDisks = copyDisks params.MigrateDisksSet = true } + // start live migration tracking + go liveMigrationMonitor(vmi, dom) err = dom.MigrateToURI3(dstUri, params, migrateFlags) if err != nil { @@ -448,7 +450,7 @@ func liveMigrationMonitor(vmi *v1.VirtualMachineInstance, dom cli.VirDomain) { // check the overall migration time if acceptableCompletionTime != 0 && elapsed > acceptableCompletionTime { - logger.Warningf("Live migration not completed after %d sec", + logger.Warningf("Live migration is not completed after %d sec", acceptableCompletionTime) err := dom.AbortJob() if err != nil { From 705fa3c6da475338b45f3c11c343b04f6edefcb7 Mon Sep 17 00:00:00 2001 From: Vladik Romanovsky Date: Tue, 5 Feb 2019 22:50:29 -0500 Subject: [PATCH 04/16] use correct path to calculate ephemeral disks size Signed-off-by: Vladik Romanovsky --- pkg/virt-launcher/virtwrap/manager.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/virt-launcher/virtwrap/manager.go b/pkg/virt-launcher/virtwrap/manager.go index 7db656848947..e65fd9b9ccc1 100644 --- a/pkg/virt-launcher/virtwrap/manager.go +++ b/pkg/virt-launcher/virtwrap/manager.go @@ -371,7 +371,7 @@ func (l *LibvirtDomainManager) asyncMigrate(vmi *v1.VirtualMachineInstance) { } func getVMIEphemeralDisksTotalSize() *resource.Quantity { - var baseDir = "/var/run/libvirt/kubevirt-ephemeral-disk" + var baseDir = "/var/run/kubevirt-ephemeral-disks/" totalSize := int64(0) err := filepath.Walk(baseDir, func(path string, f os.FileInfo, err error) error { if !f.IsDir() { From 927f7a02ab03e1de341f378c1a42491c5f547226 Mon Sep 17 00:00:00 2001 From: Vladik Romanovsky Date: Tue, 5 Feb 2019 22:51:23 -0500 Subject: [PATCH 05/16] use libvirt getJobInfo for live migration tracking Signed-off-by: Vladik Romanovsky --- .../virtwrap/cli/generated_mock_libvirt.go | 11 +++++++++++ pkg/virt-launcher/virtwrap/cli/libvirt.go | 1 + pkg/virt-launcher/virtwrap/manager.go | 2 +- 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/pkg/virt-launcher/virtwrap/cli/generated_mock_libvirt.go b/pkg/virt-launcher/virtwrap/cli/generated_mock_libvirt.go index 5cf7df250867..2d1a944581b9 100644 --- a/pkg/virt-launcher/virtwrap/cli/generated_mock_libvirt.go +++ b/pkg/virt-launcher/virtwrap/cli/generated_mock_libvirt.go @@ -379,6 +379,17 @@ func (_mr *_MockVirDomainRecorder) GetJobStats(arg0 interface{}) *gomock.Call { return _mr.mock.ctrl.RecordCall(_mr.mock, "GetJobStats", arg0) } +func (_m *MockVirDomain) GetJobInfo() (*libvirt_go.DomainJobInfo, error) { + ret := _m.ctrl.Call(_m, "GetJobInfo") + ret0, _ := ret[0].(*libvirt_go.DomainJobInfo) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +func (_mr *_MockVirDomainRecorder) GetJobInfo() *gomock.Call { + return _mr.mock.ctrl.RecordCall(_mr.mock, "GetJobInfo") +} + func (_m *MockVirDomain) AbortJob() error { ret := _m.ctrl.Call(_m, "AbortJob") ret0, _ := ret[0].(error) diff --git a/pkg/virt-launcher/virtwrap/cli/libvirt.go b/pkg/virt-launcher/virtwrap/cli/libvirt.go index 330314f7a17d..d7c69a2de678 100644 --- a/pkg/virt-launcher/virtwrap/cli/libvirt.go +++ b/pkg/virt-launcher/virtwrap/cli/libvirt.go @@ -331,6 +331,7 @@ type VirDomain interface { MigrateToURI3(string, *libvirt.DomainMigrateParameters, libvirt.DomainMigrateFlags) error MemoryStats(nrStats uint32, flags uint32) ([]libvirt.DomainMemoryStat, error) GetJobStats(flags libvirt.DomainGetJobStatsFlags) (*libvirt.DomainJobInfo, error) + GetJobInfo() (*libvirt.DomainJobInfo, error) AbortJob() error Free() error } diff --git a/pkg/virt-launcher/virtwrap/manager.go b/pkg/virt-launcher/virtwrap/manager.go index e65fd9b9ccc1..df23172a09b3 100644 --- a/pkg/virt-launcher/virtwrap/manager.go +++ b/pkg/virt-launcher/virtwrap/manager.go @@ -419,7 +419,7 @@ func liveMigrationMonitor(vmi *v1.VirtualMachineInstance, dom cli.VirDomain) { completionTimeoutPerGiB := int64(800) acceptableCompletionTime := completionTimeoutPerGiB * getVMIMigrationDataSize(vmi) for { - stats, err := dom.GetJobStats(0) + stats, err := dom.GetJobInfo() if err != nil { logger.Reason(err).Error("failed to get domain job info") break From 8f0e86b6eeb3402d63fb0614b26252b6b633753f Mon Sep 17 00:00:00 2001 From: Vladik Romanovsky Date: Wed, 6 Feb 2019 11:25:57 -0500 Subject: [PATCH 06/16] report when live migration is being aborted Signed-off-by: Vladik Romanovsky --- pkg/virt-launcher/virtwrap/manager.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/virt-launcher/virtwrap/manager.go b/pkg/virt-launcher/virtwrap/manager.go index df23172a09b3..e6d6b79ecc29 100644 --- a/pkg/virt-launcher/virtwrap/manager.go +++ b/pkg/virt-launcher/virtwrap/manager.go @@ -356,7 +356,7 @@ func (l *LibvirtDomainManager) asyncMigrate(vmi *v1.VirtualMachineInstance) { params.MigrateDisksSet = true } // start live migration tracking - go liveMigrationMonitor(vmi, dom) + go liveMigrationMonitor(vmi, dom, l) err = dom.MigrateToURI3(dstUri, params, migrateFlags) if err != nil { @@ -407,7 +407,7 @@ func getVMIMigrationDataSize(vmi *v1.VirtualMachineInstance) int64 { return memory.ScaledValue(resource.Giga) } -func liveMigrationMonitor(vmi *v1.VirtualMachineInstance, dom cli.VirDomain) { +func liveMigrationMonitor(vmi *v1.VirtualMachineInstance, dom cli.VirDomain, l *LibvirtDomainManager) { logger := log.Log.Object(vmi) start := time.Now().UTC().Unix() lastProgressUpdate := start @@ -445,6 +445,8 @@ func liveMigrationMonitor(vmi *v1.VirtualMachineInstance, dom cli.VirDomain) { if err != nil { logger.Reason(err).Error("failed to abort migration") } + l.setMigrationResult(vmi, true, fmt.Sprintf("Live migration stuck for %d sec and has been aborted", progressDelay)) + break } // check the overall migration time @@ -456,6 +458,8 @@ func liveMigrationMonitor(vmi *v1.VirtualMachineInstance, dom cli.VirDomain) { if err != nil { logger.Reason(err).Error("failed to abort migration") } + l.setMigrationResult(vmi, true, fmt.Sprintf("Live migration is not completed after %d sec and has been aborted", acceptableCompletionTime)) + break } case libvirt.DOMAIN_JOB_NONE: From a2893a12222e02bb19bcc42fba316f058a1f1d61 Mon Sep 17 00:00:00 2001 From: Vladik Romanovsky Date: Tue, 19 Feb 2019 14:50:58 -0500 Subject: [PATCH 07/16] add a struct for a migration config Signed-off-by: Vladik Romanovsky --- pkg/api/v1/deepcopy_generated.go | 16 ++++++++++++++++ pkg/api/v1/types.go | 7 +++++++ pkg/api/v1/types_swagger_generated.go | 7 +++++++ 3 files changed, 30 insertions(+) diff --git a/pkg/api/v1/deepcopy_generated.go b/pkg/api/v1/deepcopy_generated.go index 055e1bced6f4..7ec64873e707 100644 --- a/pkg/api/v1/deepcopy_generated.go +++ b/pkg/api/v1/deepcopy_generated.go @@ -1512,6 +1512,22 @@ func (in *Memory) DeepCopy() *Memory { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *MigrationConfig) DeepCopyInto(out *MigrationConfig) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MigrationConfig. +func (in *MigrationConfig) DeepCopy() *MigrationConfig { + if in == nil { + return nil + } + out := new(MigrationConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Network) DeepCopyInto(out *Network) { *out = *in diff --git a/pkg/api/v1/types.go b/pkg/api/v1/types.go index 090614c6eb12..f62b8e3b98de 100644 --- a/pkg/api/v1/types.go +++ b/pkg/api/v1/types.go @@ -365,6 +365,13 @@ type VirtualMachineInstanceMigrationState struct { MigrationUID types.UID `json:"migrationUid,omitempty"` } +type MigrationConfig struct { + // The time for GiB of data to wait for the migration to be completed before aborting it + CompletionTimeoutPerGiB int64 `json:"completionTimeoutPerGiB,omitempty"` + // The time to wait for live migration to make progress in transferring data. + ProgressTimeout int64 `json:"progressTimeout,omitempty"` +} + // --- // +k8s:openapi-gen=true type VirtualMachineInstanceMigrationMethod string diff --git a/pkg/api/v1/types_swagger_generated.go b/pkg/api/v1/types_swagger_generated.go index fb27ef93dbed..111850c7666e 100644 --- a/pkg/api/v1/types_swagger_generated.go +++ b/pkg/api/v1/types_swagger_generated.go @@ -78,6 +78,13 @@ func (VirtualMachineInstanceMigrationState) SwaggerDoc() map[string]string { } } +func (MigrationConfig) SwaggerDoc() map[string]string { + return map[string]string{ + "completionTimeoutPerGiB": "The time for GiB of data to wait for the migration to be completed before aborting it", + "progressTimeout": "The time to wait for live migration to make progress in transferring data.", + } +} + func (VMISelector) SwaggerDoc() map[string]string { return map[string]string{ "name": "Name of the VirtualMachineInstance to migrate", From 5704f7312a3100751fcacc96fa2cfea0f8596c74 Mon Sep 17 00:00:00 2001 From: Vladik Romanovsky Date: Wed, 20 Feb 2019 01:47:47 -0500 Subject: [PATCH 08/16] add migration config to the migration spec Signed-off-by: Vladik Romanovsky --- api/openapi-spec/swagger.json | 17 +++++++++++++++++ pkg/api/v1/deepcopy_generated.go | 11 ++++++++++- pkg/api/v1/openapi_generated.go | 8 +++++++- pkg/api/v1/types.go | 3 ++- 4 files changed, 36 insertions(+), 3 deletions(-) diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json index f7e6f17f054e..a4d9fcbcd279 100644 --- a/api/openapi-spec/swagger.json +++ b/api/openapi-spec/swagger.json @@ -4941,6 +4941,20 @@ } } }, + "v1.MigrationConfig": { + "properties": { + "completionTimeoutPerGiB": { + "description": "The time for GiB of data to wait for the migration to be completed before aborting it", + "type": "integer", + "format": "int64" + }, + "progressTimeout": { + "description": "The time to wait for live migration to make progress in transferring data.", + "type": "integer", + "format": "int64" + } + } + }, "v1.Network": { "description": "Network represents a network type and a resource that should be connected to the vm.", "required": [ @@ -5870,6 +5884,9 @@ }, "v1.VirtualMachineInstanceMigrationSpec": { "properties": { + "configuration": { + "$ref": "#/definitions/v1.MigrationConfig" + }, "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/api/v1/deepcopy_generated.go b/pkg/api/v1/deepcopy_generated.go index 7ec64873e707..106d9ba62b15 100644 --- a/pkg/api/v1/deepcopy_generated.go +++ b/pkg/api/v1/deepcopy_generated.go @@ -1981,7 +1981,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) out.Status = in.Status return } @@ -2040,6 +2040,15 @@ func (in *VirtualMachineInstanceMigrationList) DeepCopyObject() runtime.Object { // 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.Config != nil { + in, out := &in.Config, &out.Config + if *in == nil { + *out = nil + } else { + *out = new(MigrationConfig) + **out = **in + } + } return } diff --git a/pkg/api/v1/openapi_generated.go b/pkg/api/v1/openapi_generated.go index 4fececaebcc8..eecbe0626ed6 100644 --- a/pkg/api/v1/openapi_generated.go +++ b/pkg/api/v1/openapi_generated.go @@ -2476,10 +2476,16 @@ func schema_kubevirt_pkg_api_v1_VirtualMachineInstanceMigrationSpec(ref common.R Format: "", }, }, + "configuration": { + SchemaProps: spec.SchemaProps{ + Ref: ref("kubevirt.io/kubevirt/pkg/api/v1.MigrationConfig"), + }, + }, }, }, }, - Dependencies: []string{}, + Dependencies: []string{ + "kubevirt.io/kubevirt/pkg/api/v1.MigrationConfig"}, } } diff --git a/pkg/api/v1/types.go b/pkg/api/v1/types.go index f62b8e3b98de..0d124be6f621 100644 --- a/pkg/api/v1/types.go +++ b/pkg/api/v1/types.go @@ -743,7 +743,8 @@ func (vl *VirtualMachineInstanceMigrationList) GetListMeta() meta.List { // +k8s:openapi-gen=true 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"` + VMIName string `json:"vmiName,omitempty" valid:"required"` + Config *MigrationConfig `json:"configuration,omitempty"` } // VirtualMachineInstanceMigration reprents information pertaining to a VMI's migration. From 05873880d091461db91405acf418ff4663ce4e04 Mon Sep 17 00:00:00 2001 From: Vladik Romanovsky Date: Wed, 20 Feb 2019 15:26:00 -0500 Subject: [PATCH 09/16] verify that live migration monitor cancels the migration when needed Signed-off-by: Vladik Romanovsky --- pkg/virt-launcher/virtwrap/manager.go | 11 ++-- pkg/virt-launcher/virtwrap/manager_test.go | 77 ++++++++++++++++++++++ 2 files changed, 83 insertions(+), 5 deletions(-) diff --git a/pkg/virt-launcher/virtwrap/manager.go b/pkg/virt-launcher/virtwrap/manager.go index e6d6b79ecc29..1f6fad8eb106 100644 --- a/pkg/virt-launcher/virtwrap/manager.go +++ b/pkg/virt-launcher/virtwrap/manager.go @@ -418,6 +418,7 @@ func liveMigrationMonitor(vmi *v1.VirtualMachineInstance, dom cli.VirDomain, l * // TODO:(vladikr) move to configMap completionTimeoutPerGiB := int64(800) acceptableCompletionTime := completionTimeoutPerGiB * getVMIMigrationDataSize(vmi) +monitorLoop: for { stats, err := dom.GetJobInfo() if err != nil { @@ -446,7 +447,7 @@ func liveMigrationMonitor(vmi *v1.VirtualMachineInstance, dom cli.VirDomain, l * logger.Reason(err).Error("failed to abort migration") } l.setMigrationResult(vmi, true, fmt.Sprintf("Live migration stuck for %d sec and has been aborted", progressDelay)) - break + break monitorLoop } // check the overall migration time @@ -459,21 +460,21 @@ func liveMigrationMonitor(vmi *v1.VirtualMachineInstance, dom cli.VirDomain, l * logger.Reason(err).Error("failed to abort migration") } l.setMigrationResult(vmi, true, fmt.Sprintf("Live migration is not completed after %d sec and has been aborted", acceptableCompletionTime)) - break + break monitorLoop } case libvirt.DOMAIN_JOB_NONE: logger.Info("Migration job didn't start yet") case libvirt.DOMAIN_JOB_COMPLETED: logger.Info("Migration has beem completed") - break + break monitorLoop case libvirt.DOMAIN_JOB_FAILED: logger.Info("Migration job failed") // migration failed - break + break monitorLoop case libvirt.DOMAIN_JOB_CANCELLED: logger.Info("Migration was canceled") - break + break monitorLoop } time.Sleep(500 * time.Millisecond) } diff --git a/pkg/virt-launcher/virtwrap/manager_test.go b/pkg/virt-launcher/virtwrap/manager_test.go index 5a8c2bf2f9c8..758f984ae6e9 100644 --- a/pkg/virt-launcher/virtwrap/manager_test.go +++ b/pkg/virt-launcher/virtwrap/manager_test.go @@ -210,6 +210,83 @@ var _ = Describe("Manager", func() { Expect(newspec).ToNot(BeNil()) }) }) + Context("test migration monitor", func() { + It("migration should be canceled if it's not progressing", func() { + // Make sure that we always free the domain after use + mockDomain.EXPECT().Free().AnyTimes() + fake_jobinfo := &libvirt.DomainJobInfo{ + Type: libvirt.DOMAIN_JOB_UNBOUNDED, + DataRemaining: 32479827394, + } + + vmi := newVMI(testNamespace, testVmName) + vmi.Status.MigrationState = &v1.VirtualMachineInstanceMigrationState{ + MigrationUID: "111222333", + } + migrationConfig := &v1.MigrationConfig{ + CompletionTimeoutPerGiB: 800, + ProgressTimeout: 2, + } + + domainSpec := expectIsolationDetectionForVMI(vmi) + xml, err := xml.Marshal(domainSpec) + Expect(err).To(BeNil()) + manager := &LibvirtDomainManager{ + virConn: mockConn, + virtShareDir: "fake", + notifier: nil, + lessPVCSpaceToleration: 0, + } + mockDomain.EXPECT().GetState().Return(libvirt.DOMAIN_RUNNING, 1, nil) + mockConn.EXPECT().LookupDomainByName(testDomainName).Return(mockDomain, nil) + mockDomain.EXPECT().GetJobInfo().AnyTimes().Return(fake_jobinfo, nil) + mockDomain.EXPECT().AbortJob() + mockDomain.EXPECT().GetXMLDesc(gomock.Eq(libvirt.DOMAIN_XML_MIGRATABLE)).Return(string(xml), nil) + mockDomain.EXPECT().GetXMLDesc(gomock.Eq(libvirt.DOMAIN_XML_INACTIVE)).Return(string(xml), nil) + + liveMigrationMonitor(vmi, mockDomain, manager, migrationConfig) + }) + It("migration should be canceled if timeout has been reached", func() { + // Make sure that we always free the domain after use + var migrationData = 32479827394 + mockDomain.EXPECT().Free().AnyTimes() + fake_jobinfo := func() *libvirt.DomainJobInfo { + migrationData -= 125 + return &libvirt.DomainJobInfo{ + Type: libvirt.DOMAIN_JOB_UNBOUNDED, + DataRemaining: uint64(migrationData), + } + }() + + vmi := newVMI(testNamespace, testVmName) + vmi.Status.MigrationState = &v1.VirtualMachineInstanceMigrationState{ + MigrationUID: "111222333", + } + migrationConfig := &v1.MigrationConfig{ + CompletionTimeoutPerGiB: 3, + ProgressTimeout: 150, + } + + domainSpec := expectIsolationDetectionForVMI(vmi) + xml, err := xml.Marshal(domainSpec) + Expect(err).To(BeNil()) + manager := &LibvirtDomainManager{ + virConn: mockConn, + virtShareDir: "fake", + notifier: nil, + lessPVCSpaceToleration: 0, + } + mockDomain.EXPECT().GetState().Return(libvirt.DOMAIN_RUNNING, 1, nil) + mockConn.EXPECT().LookupDomainByName(testDomainName).Return(mockDomain, nil) + mockDomain.EXPECT().GetJobInfo().AnyTimes().Return(fake_jobinfo, nil) + mockDomain.EXPECT().AbortJob() + mockDomain.EXPECT().GetXMLDesc(gomock.Eq(libvirt.DOMAIN_XML_MIGRATABLE)).Return(string(xml), nil) + mockDomain.EXPECT().GetXMLDesc(gomock.Eq(libvirt.DOMAIN_XML_INACTIVE)).Return(string(xml), nil) + + liveMigrationMonitor(vmi, mockDomain, manager, migrationConfig) + }) + + }) Context("on successful VirtualMachineInstance migrate", func() { It("should prepare the target pod", func() { From 082672e887f2f4d2c245da14a94284c9f09df66c Mon Sep 17 00:00:00 2001 From: Vladik Romanovsky Date: Wed, 20 Feb 2019 15:36:03 -0500 Subject: [PATCH 10/16] update bazel Signed-off-by: Vladik Romanovsky --- pkg/virt-launcher/virtwrap/BUILD.bazel | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/virt-launcher/virtwrap/BUILD.bazel b/pkg/virt-launcher/virtwrap/BUILD.bazel index d205371cbf56..c1c9e45d608b 100644 --- a/pkg/virt-launcher/virtwrap/BUILD.bazel +++ b/pkg/virt-launcher/virtwrap/BUILD.bazel @@ -30,6 +30,8 @@ go_library( "//pkg/virt-launcher/virtwrap/util:go_default_library", "//vendor/github.com/golang/mock/gomock:go_default_library", "//vendor/github.com/libvirt/libvirt-go:go_default_library", + "//vendor/k8s.io/api/core/v1:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/api/resource:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/util/wait:go_default_library", ], From fd5fe85a33e1503deda1561415cf236c4db757e3 Mon Sep 17 00:00:00 2001 From: Vladik Romanovsky Date: Wed, 27 Feb 2019 13:17:13 -0500 Subject: [PATCH 11/16] add migration timeouts to vmi migration status Signed-off-by: Vladik Romanovsky --- api/openapi-spec/swagger.json | 10 ++++++++++ pkg/api/v1/types.go | 4 ++++ pkg/api/v1/types_swagger_generated.go | 2 ++ 3 files changed, 16 insertions(+) diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json index a4d9fcbcd279..2ceffeeb3c77 100644 --- a/api/openapi-spec/swagger.json +++ b/api/openapi-spec/swagger.json @@ -5899,6 +5899,11 @@ "description": "Indicates the migration completed", "type": "boolean" }, + "completionTimeoutPerGiB": { + "description": "The time for GiB of data to wait for the migration to be completed before aborting it", + "type": "integer", + "format": "int64" + }, "endTimestamp": { "description": "The time the migration action ended", "type": "string" @@ -5911,6 +5916,11 @@ "description": "The VirtualMachineInstanceMigration object associated with this migration", "type": "string" }, + "progressTimeout": { + "description": "The time to wait for live migration to make progress in transferring data.", + "type": "integer", + "format": "int64" + }, "sourceNode": { "description": "The source node that the VMI originated on", "type": "string" diff --git a/pkg/api/v1/types.go b/pkg/api/v1/types.go index 0d124be6f621..96cd833541ca 100644 --- a/pkg/api/v1/types.go +++ b/pkg/api/v1/types.go @@ -363,6 +363,10 @@ type VirtualMachineInstanceMigrationState struct { Failed bool `json:"failed,omitempty"` // The VirtualMachineInstanceMigration object associated with this migration MigrationUID types.UID `json:"migrationUid,omitempty"` + // The time for GiB of data to wait for the migration to be completed before aborting it + CompletionTimeoutPerGiB int64 `json:"completionTimeoutPerGiB,omitempty"` + // The time to wait for live migration to make progress in transferring data. + ProgressTimeout int64 `json:"progressTimeout,omitempty"` } type MigrationConfig struct { diff --git a/pkg/api/v1/types_swagger_generated.go b/pkg/api/v1/types_swagger_generated.go index 111850c7666e..fcc6ef73e22f 100644 --- a/pkg/api/v1/types_swagger_generated.go +++ b/pkg/api/v1/types_swagger_generated.go @@ -75,6 +75,8 @@ func (VirtualMachineInstanceMigrationState) SwaggerDoc() map[string]string { "completed": "Indicates the migration completed", "failed": "Indicates that the migration failed", "migrationUid": "The VirtualMachineInstanceMigration object associated with this migration", + "completionTimeoutPerGiB": "The time for GiB of data to wait for the migration to be completed before aborting it", + "progressTimeout": "The time to wait for live migration to make progress in transferring data.", } } From da12ac34da6e64b0b66633d3d890637258c91501 Mon Sep 17 00:00:00 2001 From: Vladik Romanovsky Date: Wed, 27 Feb 2019 13:47:07 -0500 Subject: [PATCH 12/16] copy the migration config to vmi migration status Signed-off-by: Vladik Romanovsky --- pkg/virt-controller/watch/migration.go | 10 ++++++ pkg/virt-controller/watch/migration_test.go | 34 +++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/pkg/virt-controller/watch/migration.go b/pkg/virt-controller/watch/migration.go index 9cf1b3d1d2e6..53d99cf78501 100644 --- a/pkg/virt-controller/watch/migration.go +++ b/pkg/virt-controller/watch/migration.go @@ -433,6 +433,16 @@ func (c *MigrationController) sync(migration *virtv1.VirtualMachineInstanceMigra SourceNode: vmi.Status.NodeName, TargetPod: pod.Name, } + if migration.Spec.Config != nil { + completionTimeout := migration.Spec.Config.CompletionTimeoutPerGiB + progressTimeout := migration.Spec.Config.ProgressTimeout + if completionTimeout != 0 { + vmiCopy.Status.MigrationState.CompletionTimeoutPerGiB = completionTimeout + } + if progressTimeout != 0 { + vmiCopy.Status.MigrationState.ProgressTimeout = progressTimeout + } + } // By setting this label, virt-handler on the target node will receive // the vmi and prepare the local environment for the migration diff --git a/pkg/virt-controller/watch/migration_test.go b/pkg/virt-controller/watch/migration_test.go index 51ad806ff5e6..58f4945ff697 100644 --- a/pkg/virt-controller/watch/migration_test.go +++ b/pkg/virt-controller/watch/migration_test.go @@ -136,6 +136,18 @@ var _ = Describe("Migration watcher", func() { }).Return(vmi, nil) } + shouldExpectVirtualMachineHandoffWithConfig := func(vmi *v1.VirtualMachineInstance, migrationUid types.UID, targetNode string, conf *v1.MigrationConfig) { + vmiInterface.EXPECT().Update(gomock.Any()).Do(func(arg interface{}) { + Expect(arg.(*v1.VirtualMachineInstance).Status.MigrationState).ToNot(BeNil()) + Expect(arg.(*v1.VirtualMachineInstance).Status.MigrationState.MigrationUID).To(Equal(migrationUid)) + + Expect(arg.(*v1.VirtualMachineInstance).Status.MigrationState.SourceNode).To(Equal(vmi.Status.NodeName)) + Expect(arg.(*v1.VirtualMachineInstance).Status.MigrationState.TargetNode).To(Equal(targetNode)) + Expect(arg.(*v1.VirtualMachineInstance).Labels[v1.MigrationTargetNodeNameLabel]).To(Equal(targetNode)) + Expect(arg.(*v1.VirtualMachineInstance).Status.MigrationState.CompletionTimeoutPerGiB).To(Equal(conf.CompletionTimeoutPerGiB)) + Expect(arg.(*v1.VirtualMachineInstance).Status.MigrationState.ProgressTimeout).To(Equal(conf.ProgressTimeout)) + }).Return(vmi, nil) + } syncCaches := func(stop chan struct{}) { go vmiInformer.Run(stop) @@ -398,6 +410,28 @@ var _ = Describe("Migration watcher", func() { controller.Execute() testutils.ExpectEvent(recorder, SuccessfulHandOverPodReason) }) + It("should hand pod over to target virt-handler with migration config", func() { + vmi := newVirtualMachine("testvmi", v1.Running) + vmi.Status.NodeName = "node02" + migration := newMigration("testmigration", vmi.Name, v1.MigrationScheduled) + migrationConfig := &v1.MigrationConfig{ + CompletionTimeoutPerGiB: 300, + ProgressTimeout: 100, + } + migration.Spec.Config = migrationConfig + + pod := newTargetPodForVirtualMachine(vmi, migration, k8sv1.PodPending) + pod.Spec.NodeName = "node01" + + addMigration(migration) + addVirtualMachine(vmi) + podFeeder.Add(pod) + + shouldExpectVirtualMachineHandoffWithConfig(vmi, migration.UID, "node01", migrationConfig) + + controller.Execute() + testutils.ExpectEvent(recorder, SuccessfulHandOverPodReason) + }) It("should hand pod over to target virt-handler overriding previous state", func() { vmi := newVirtualMachine("testvmi", v1.Running) From 69a22e9c6588cd43c5080f6ea27478029fff4aff Mon Sep 17 00:00:00 2001 From: Vladik Romanovsky Date: Wed, 27 Feb 2019 14:57:25 -0500 Subject: [PATCH 13/16] use the migration config in migration monitor Signed-off-by: Vladik Romanovsky --- pkg/virt-launcher/virtwrap/manager.go | 16 +++++++++++++--- pkg/virt-launcher/virtwrap/manager_test.go | 18 +++++++----------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/pkg/virt-launcher/virtwrap/manager.go b/pkg/virt-launcher/virtwrap/manager.go index 1f6fad8eb106..efdaab0f3210 100644 --- a/pkg/virt-launcher/virtwrap/manager.go +++ b/pkg/virt-launcher/virtwrap/manager.go @@ -412,11 +412,21 @@ func liveMigrationMonitor(vmi *v1.VirtualMachineInstance, dom cli.VirDomain, l * start := time.Now().UTC().Unix() lastProgressUpdate := start progressWatermark := int64(0) - // TODO:(vladikr) move to configMap - progressTimeout := int64(150) - // TODO:(vladikr) move to configMap + progressTimeout := int64(150) completionTimeoutPerGiB := int64(800) + + // update timeouts from migration config + conf := vmi.Status.MigrationState + if conf != nil { + if conf.CompletionTimeoutPerGiB != 0 { + completionTimeoutPerGiB = conf.CompletionTimeoutPerGiB + } + if conf.ProgressTimeout != 0 { + progressTimeout = conf.ProgressTimeout + } + } + acceptableCompletionTime := completionTimeoutPerGiB * getVMIMigrationDataSize(vmi) monitorLoop: for { diff --git a/pkg/virt-launcher/virtwrap/manager_test.go b/pkg/virt-launcher/virtwrap/manager_test.go index 758f984ae6e9..4d79b7c8c25d 100644 --- a/pkg/virt-launcher/virtwrap/manager_test.go +++ b/pkg/virt-launcher/virtwrap/manager_test.go @@ -221,11 +221,9 @@ var _ = Describe("Manager", func() { vmi := newVMI(testNamespace, testVmName) vmi.Status.MigrationState = &v1.VirtualMachineInstanceMigrationState{ - MigrationUID: "111222333", - } - migrationConfig := &v1.MigrationConfig{ - CompletionTimeoutPerGiB: 800, + MigrationUID: "111222333", ProgressTimeout: 2, + CompletionTimeoutPerGiB: 300, } domainSpec := expectIsolationDetectionForVMI(vmi) @@ -244,7 +242,7 @@ var _ = Describe("Manager", func() { mockDomain.EXPECT().GetXMLDesc(gomock.Eq(libvirt.DOMAIN_XML_MIGRATABLE)).Return(string(xml), nil) mockDomain.EXPECT().GetXMLDesc(gomock.Eq(libvirt.DOMAIN_XML_INACTIVE)).Return(string(xml), nil) - liveMigrationMonitor(vmi, mockDomain, manager, migrationConfig) + liveMigrationMonitor(vmi, mockDomain, manager) }) It("migration should be canceled if timeout has been reached", func() { // Make sure that we always free the domain after use @@ -260,11 +258,9 @@ var _ = Describe("Manager", func() { vmi := newVMI(testNamespace, testVmName) vmi.Status.MigrationState = &v1.VirtualMachineInstanceMigrationState{ - MigrationUID: "111222333", - } - migrationConfig := &v1.MigrationConfig{ - CompletionTimeoutPerGiB: 3, - ProgressTimeout: 150, + MigrationUID: "111222333", + ProgressTimeout: 3, + CompletionTimeoutPerGiB: 150, } domainSpec := expectIsolationDetectionForVMI(vmi) @@ -283,7 +279,7 @@ var _ = Describe("Manager", func() { mockDomain.EXPECT().GetXMLDesc(gomock.Eq(libvirt.DOMAIN_XML_MIGRATABLE)).Return(string(xml), nil) mockDomain.EXPECT().GetXMLDesc(gomock.Eq(libvirt.DOMAIN_XML_INACTIVE)).Return(string(xml), nil) - liveMigrationMonitor(vmi, mockDomain, manager, migrationConfig) + liveMigrationMonitor(vmi, mockDomain, manager) }) }) From cfe16de74baa8af677bb2ba1b6460142a935bf35 Mon Sep 17 00:00:00 2001 From: Vladik Romanovsky Date: Wed, 27 Feb 2019 15:16:35 -0500 Subject: [PATCH 14/16] migration should be set to completed even for failed migrations Signed-off-by: Vladik Romanovsky --- pkg/virt-launcher/virtwrap/manager.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/virt-launcher/virtwrap/manager.go b/pkg/virt-launcher/virtwrap/manager.go index efdaab0f3210..a97862d931ef 100644 --- a/pkg/virt-launcher/virtwrap/manager.go +++ b/pkg/virt-launcher/virtwrap/manager.go @@ -199,9 +199,8 @@ func (l *LibvirtDomainManager) setMigrationResultHelper(vmi *v1.VirtualMachineIn if failed { domainSpec.Metadata.KubeVirt.Migration.Failed = true domainSpec.Metadata.KubeVirt.Migration.FailureReason = reason - } else { - domainSpec.Metadata.KubeVirt.Migration.Completed = true } + domainSpec.Metadata.KubeVirt.Migration.Completed = true domainSpec.Metadata.KubeVirt.Migration.EndTimestamp = &now _, err = l.setDomainSpecWithHooks(vmi, domainSpec) From d0d5d2d68148976edcbdb3e03a6178fb49475745 Mon Sep 17 00:00:00 2001 From: Vladik Romanovsky Date: Mon, 25 Feb 2019 18:32:08 -0500 Subject: [PATCH 15/16] verify that the migration is aborted when timeout is reached Signed-off-by: Vladik Romanovsky --- tests/migration_test.go | 107 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/tests/migration_test.go b/tests/migration_test.go index 42eb1c9d656f..cbe1f1996c3a 100644 --- a/tests/migration_test.go +++ b/tests/migration_test.go @@ -24,8 +24,11 @@ import ( "fmt" "time" + expect "github.com/google/goexpect" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" + k8sv1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/rand" @@ -87,6 +90,25 @@ var _ = Describe("Migrations", func() { Expect(vmi.Status.Phase).To(Equal(v1.Running)) } + confirmVMIPostMigrationFailed := func(vmi *v1.VirtualMachineInstance, migrationUID string) { + By("Retrieving the VMI post migration") + vmi, err = virtClient.VirtualMachineInstance(vmi.Namespace).Get(vmi.Name, &metav1.GetOptions{}) + Expect(err).To(BeNil()) + + By("Verifying the VMI's migration state") + Expect(vmi.Status.MigrationState).ToNot(BeNil()) + Expect(vmi.Status.MigrationState.StartTimestamp).ToNot(BeNil()) + Expect(vmi.Status.MigrationState.EndTimestamp).ToNot(BeNil()) + Expect(vmi.Status.MigrationState.SourceNode).To(Equal(vmi.Status.NodeName)) + Expect(vmi.Status.MigrationState.TargetNode).ToNot(Equal(vmi.Status.MigrationState.SourceNode)) + Expect(vmi.Status.MigrationState.Completed).To(Equal(true)) + Expect(vmi.Status.MigrationState.Failed).To(Equal(true)) + Expect(vmi.Status.MigrationState.TargetNodeAddress).ToNot(Equal("")) + Expect(string(vmi.Status.MigrationState.MigrationUID)).To(Equal(migrationUID)) + + By("Verifying the VMI's is in the running state") + Expect(vmi.Status.Phase).To(Equal(v1.Running)) + } runMigrationAndExpectCompletion := func(migration *v1.VirtualMachineInstanceMigration, timeout int) string { By("Starting a Migration") Eventually(func() error { @@ -112,6 +134,31 @@ var _ = Describe("Migrations", func() { return uid } + runMigrationAndExpectFailure := func(migration *v1.VirtualMachineInstanceMigration, timeout int) string { + By("Starting a Migration") + Eventually(func() error { + _, err := virtClient.VirtualMachineInstanceMigration(migration.Namespace).Create(migration) + return err + }, timeout, 1*time.Second).ShouldNot(HaveOccurred()) + By("Waiting until the Migration Completes") + + uid := "" + Eventually(func() bool { + migration, err := virtClient.VirtualMachineInstanceMigration(migration.Namespace).Get(migration.Name, &metav1.GetOptions{}) + Expect(err).To(BeNil()) + + Expect(migration.Status.Phase).NotTo(Equal(v1.MigrationSucceeded)) + + uid = string(migration.UID) + if migration.Status.Phase == v1.MigrationFailed { + return true + } + return false + + }, timeout, 1*time.Second).Should(Equal(true)) + return uid + } + Describe("Starting a VirtualMachineInstance ", func() { Context("with a Cirros disk", func() { It("should be successfully migrated multiple times with cloud-init disk", func() { @@ -269,6 +316,66 @@ var _ = Describe("Migrations", func() { tests.WaitForVirtualMachineToDisappearWithTimeout(vmi, 120) }) }) + Context("migration monitor", func() { + It("should abort a vmi migration without progress", func() { + + vmi := tests.NewRandomVMIWithEphemeralDisk(tests.ContainerDiskFor(tests.ContainerDiskFedora)) + vmi.Spec.Domain.Resources.Requests[k8sv1.ResourceMemory] = resource.MustParse("1Gi") + tests.AddUserData(vmi, "cloud-init", fmt.Sprintf(`#!/bin/bash + echo "fedora" |passwd fedora --stdin + yum install -y stress qemu-guest-agent + systemctl start qemu-guest-agent`)) + + By("Starting the VirtualMachineInstance") + vmi = runVMIAndExpectLaunch(vmi, 240) + + getOptions := &metav1.GetOptions{} + var updatedVmi *v1.VirtualMachineInstance + By("Checking that the VirtualMachineInstance console has expected output") + expecter, expecterErr := tests.LoggedInFedoraExpecter(vmi) + Expect(expecterErr).To(BeNil()) + defer expecter.Close() + + // Need to wait for cloud init to finnish and start the agent inside the vmi. + Eventually(func() bool { + updatedVmi, err = virtClient.VirtualMachineInstance(tests.NamespaceTestDefault).Get(vmi.Name, getOptions) + Expect(err).ToNot(HaveOccurred()) + for _, condition := range updatedVmi.Status.Conditions { + if condition.Type == "AgentConnected" && condition.Status == "True" { + return true + } + } + return false + }, 420*time.Second, 2).Should(BeTrue(), "Should have agent connected condition") + + By("Run a stress test") + _, err = expecter.ExpectBatch([]expect.Batcher{ + &expect.BSnd{S: "stress --vm 1 --vm-bytes 600M --vm-keep --timeout 1600s&\n"}, + }, 15*time.Second) + Expect(err).ToNot(HaveOccurred(), "should run a stress test") + + // execute a migration, wait for finalized state + By("Starting the Migration") + migration := tests.NewRandomMigration(vmi.Name, vmi.Namespace) + migration.Spec.Config = &v1.MigrationConfig{ + CompletionTimeoutPerGiB: 5, + ProgressTimeout: 50, + } + migrationUID := runMigrationAndExpectFailure(migration, 180) + + // check VMI, confirm migration state + confirmVMIPostMigrationFailed(vmi, migrationUID) + + // delete VMI + By("Deleting the VMI") + err = virtClient.VirtualMachineInstance(vmi.Namespace).Delete(vmi.Name, &metav1.DeleteOptions{}) + Expect(err).To(BeNil()) + + By("Waiting for VMI to disappear") + tests.WaitForVirtualMachineToDisappearWithTimeout(vmi, 240) + + }) + }) }) }) From ab60e2288c76ed7105d28efdf5176f321614a6bd Mon Sep 17 00:00:00 2001 From: Vladik Romanovsky Date: Thu, 28 Feb 2019 23:33:02 -0500 Subject: [PATCH 16/16] store the migration config in VirtualMachineInstanceMigrationState Signed-off-by: Vladik Romanovsky --- api/openapi-spec/swagger.json | 14 ++++---------- pkg/api/v1/deepcopy_generated.go | 9 +++++++++ pkg/api/v1/types.go | 6 ++---- pkg/api/v1/types_swagger_generated.go | 3 +-- pkg/virt-controller/watch/migration.go | 9 +-------- pkg/virt-controller/watch/migration_test.go | 5 +++-- pkg/virt-launcher/virtwrap/manager.go | 5 +++-- pkg/virt-launcher/virtwrap/manager_test.go | 19 +++++++++++++------ 8 files changed, 36 insertions(+), 34 deletions(-) diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json index 2ceffeeb3c77..8caed203dcef 100644 --- a/api/openapi-spec/swagger.json +++ b/api/openapi-spec/swagger.json @@ -5899,11 +5899,6 @@ "description": "Indicates the migration completed", "type": "boolean" }, - "completionTimeoutPerGiB": { - "description": "The time for GiB of data to wait for the migration to be completed before aborting it", - "type": "integer", - "format": "int64" - }, "endTimestamp": { "description": "The time the migration action ended", "type": "string" @@ -5912,15 +5907,14 @@ "description": "Indicates that the migration failed", "type": "boolean" }, + "migrationConfig": { + "description": "Config contains migration configuration options", + "$ref": "#/definitions/v1.MigrationConfig" + }, "migrationUid": { "description": "The VirtualMachineInstanceMigration object associated with this migration", "type": "string" }, - "progressTimeout": { - "description": "The time to wait for live migration to make progress in transferring data.", - "type": "integer", - "format": "int64" - }, "sourceNode": { "description": "The source node that the VMI originated on", "type": "string" diff --git a/pkg/api/v1/deepcopy_generated.go b/pkg/api/v1/deepcopy_generated.go index 106d9ba62b15..9959afff58f0 100644 --- a/pkg/api/v1/deepcopy_generated.go +++ b/pkg/api/v1/deepcopy_generated.go @@ -2088,6 +2088,15 @@ func (in *VirtualMachineInstanceMigrationState) DeepCopyInto(out *VirtualMachine (*out)[key] = val } } + if in.Config != nil { + in, out := &in.Config, &out.Config + if *in == nil { + *out = nil + } else { + *out = new(MigrationConfig) + **out = **in + } + } return } diff --git a/pkg/api/v1/types.go b/pkg/api/v1/types.go index 96cd833541ca..928ef11419f9 100644 --- a/pkg/api/v1/types.go +++ b/pkg/api/v1/types.go @@ -363,10 +363,8 @@ type VirtualMachineInstanceMigrationState struct { Failed bool `json:"failed,omitempty"` // The VirtualMachineInstanceMigration object associated with this migration MigrationUID types.UID `json:"migrationUid,omitempty"` - // The time for GiB of data to wait for the migration to be completed before aborting it - CompletionTimeoutPerGiB int64 `json:"completionTimeoutPerGiB,omitempty"` - // The time to wait for live migration to make progress in transferring data. - ProgressTimeout int64 `json:"progressTimeout,omitempty"` + // Config contains migration configuration options + Config *MigrationConfig `json:"migrationConfig,omitempty"` } type MigrationConfig struct { diff --git a/pkg/api/v1/types_swagger_generated.go b/pkg/api/v1/types_swagger_generated.go index fcc6ef73e22f..108ba7d585c6 100644 --- a/pkg/api/v1/types_swagger_generated.go +++ b/pkg/api/v1/types_swagger_generated.go @@ -75,8 +75,7 @@ func (VirtualMachineInstanceMigrationState) SwaggerDoc() map[string]string { "completed": "Indicates the migration completed", "failed": "Indicates that the migration failed", "migrationUid": "The VirtualMachineInstanceMigration object associated with this migration", - "completionTimeoutPerGiB": "The time for GiB of data to wait for the migration to be completed before aborting it", - "progressTimeout": "The time to wait for live migration to make progress in transferring data.", + "migrationConfig": "Config contains migration configuration options", } } diff --git a/pkg/virt-controller/watch/migration.go b/pkg/virt-controller/watch/migration.go index 53d99cf78501..3343f03e8eae 100644 --- a/pkg/virt-controller/watch/migration.go +++ b/pkg/virt-controller/watch/migration.go @@ -434,14 +434,7 @@ func (c *MigrationController) sync(migration *virtv1.VirtualMachineInstanceMigra TargetPod: pod.Name, } if migration.Spec.Config != nil { - completionTimeout := migration.Spec.Config.CompletionTimeoutPerGiB - progressTimeout := migration.Spec.Config.ProgressTimeout - if completionTimeout != 0 { - vmiCopy.Status.MigrationState.CompletionTimeoutPerGiB = completionTimeout - } - if progressTimeout != 0 { - vmiCopy.Status.MigrationState.ProgressTimeout = progressTimeout - } + vmiCopy.Status.MigrationState.Config = migration.Spec.Config.DeepCopy() } // By setting this label, virt-handler on the target node will receive diff --git a/pkg/virt-controller/watch/migration_test.go b/pkg/virt-controller/watch/migration_test.go index 58f4945ff697..32380a7b2ffe 100644 --- a/pkg/virt-controller/watch/migration_test.go +++ b/pkg/virt-controller/watch/migration_test.go @@ -144,8 +144,9 @@ var _ = Describe("Migration watcher", func() { Expect(arg.(*v1.VirtualMachineInstance).Status.MigrationState.SourceNode).To(Equal(vmi.Status.NodeName)) Expect(arg.(*v1.VirtualMachineInstance).Status.MigrationState.TargetNode).To(Equal(targetNode)) Expect(arg.(*v1.VirtualMachineInstance).Labels[v1.MigrationTargetNodeNameLabel]).To(Equal(targetNode)) - Expect(arg.(*v1.VirtualMachineInstance).Status.MigrationState.CompletionTimeoutPerGiB).To(Equal(conf.CompletionTimeoutPerGiB)) - Expect(arg.(*v1.VirtualMachineInstance).Status.MigrationState.ProgressTimeout).To(Equal(conf.ProgressTimeout)) + Expect(arg.(*v1.VirtualMachineInstance).Status.MigrationState.Config).ToNot(BeNil()) + Expect(arg.(*v1.VirtualMachineInstance).Status.MigrationState.Config.CompletionTimeoutPerGiB).To(Equal(conf.CompletionTimeoutPerGiB)) + Expect(arg.(*v1.VirtualMachineInstance).Status.MigrationState.Config.ProgressTimeout).To(Equal(conf.ProgressTimeout)) }).Return(vmi, nil) } diff --git a/pkg/virt-launcher/virtwrap/manager.go b/pkg/virt-launcher/virtwrap/manager.go index a97862d931ef..9c6727038e5c 100644 --- a/pkg/virt-launcher/virtwrap/manager.go +++ b/pkg/virt-launcher/virtwrap/manager.go @@ -416,8 +416,9 @@ func liveMigrationMonitor(vmi *v1.VirtualMachineInstance, dom cli.VirDomain, l * completionTimeoutPerGiB := int64(800) // update timeouts from migration config - conf := vmi.Status.MigrationState - if conf != nil { + + if vmi.Status.MigrationState != nil && vmi.Status.MigrationState.Config != nil { + conf := vmi.Status.MigrationState.Config if conf.CompletionTimeoutPerGiB != 0 { completionTimeoutPerGiB = conf.CompletionTimeoutPerGiB } diff --git a/pkg/virt-launcher/virtwrap/manager_test.go b/pkg/virt-launcher/virtwrap/manager_test.go index 4d79b7c8c25d..3075723c5bef 100644 --- a/pkg/virt-launcher/virtwrap/manager_test.go +++ b/pkg/virt-launcher/virtwrap/manager_test.go @@ -219,13 +219,17 @@ var _ = Describe("Manager", func() { DataRemaining: 32479827394, } - vmi := newVMI(testNamespace, testVmName) - vmi.Status.MigrationState = &v1.VirtualMachineInstanceMigrationState{ - MigrationUID: "111222333", + mConfig := &v1.MigrationConfig{ ProgressTimeout: 2, CompletionTimeoutPerGiB: 300, } + vmi := newVMI(testNamespace, testVmName) + vmi.Status.MigrationState = &v1.VirtualMachineInstanceMigrationState{ + MigrationUID: "111222333", + Config: mConfig, + } + domainSpec := expectIsolationDetectionForVMI(vmi) xml, err := xml.Marshal(domainSpec) Expect(err).To(BeNil()) @@ -256,12 +260,15 @@ var _ = Describe("Manager", func() { } }() - vmi := newVMI(testNamespace, testVmName) - vmi.Status.MigrationState = &v1.VirtualMachineInstanceMigrationState{ - MigrationUID: "111222333", + mConfig := &v1.MigrationConfig{ ProgressTimeout: 3, CompletionTimeoutPerGiB: 150, } + vmi := newVMI(testNamespace, testVmName) + vmi.Status.MigrationState = &v1.VirtualMachineInstanceMigrationState{ + MigrationUID: "111222333", + Config: mConfig, + } domainSpec := expectIsolationDetectionForVMI(vmi) xml, err := xml.Marshal(domainSpec)