Compute v2: Add Live Migration Action #728

dstdfx · 2018-01-17T15:25:58Z

For #520

The same for Openstack CLI:

openstack server migrate --live <hostname> <server-name-or-id>

Links to the line numbers/files in the OpenStack source code that support the
code in this PR:

API doc:

https://developer.openstack.org/api-ref/compute/#live-migrate-server-os-migratelive-action

Source code:

https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/migrate_server.py#L77

coveralls · 2018-01-17T15:31:45Z

Changes Unknown when pulling fd197c6 on dstdfx:server-live-migration into ** on gophercloud:master**.

theopenlab-ci · 2018-01-17T16:31:22Z

Build succeeded.

gophercloud-unittest http://logs.openlabtesting.org/logs/28/728/fd197c6246ec18825a9b92d92aeae5604a7d018c/check/gophercloud-unittest/b413129/ : SUCCESS in 2m 12s
gophercloud-acceptance-test http://logs.openlabtesting.org/logs/28/728/fd197c6246ec18825a9b92d92aeae5604a7d018c/check/gophercloud-acceptance-test/3a4c280/ : SUCCESS in 1h 02m 53s

coveralls · 2018-01-19T14:19:10Z

Coverage decreased (-0.0003%) to 73.134% when pulling 36c6c90 on dstdfx:server-live-migration into 8ca2d18 on gophercloud:master.

coveralls · 2018-01-19T14:44:45Z

Coverage decreased (-0.0003%) to 73.134% when pulling b8706ed on dstdfx:server-live-migration into 8ca2d18 on gophercloud:master.

theopenlab-ci · 2018-01-19T17:12:26Z

Build failed.

gophercloud-unittest http://logs.openlabtesting.org/logs/28/728/b8706edf2e105c2413d47a2169261f1e7db3f385/check/gophercloud-unittest/0900395/ : SUCCESS in 2m 27s
gophercloud-acceptance-test http://logs.openlabtesting.org/logs/28/728/b8706edf2e105c2413d47a2169261f1e7db3f385/check/gophercloud-acceptance-test/92167a5/ : FAILURE in 2h 20m 50s

coveralls · 2018-01-19T17:25:04Z

Coverage decreased (-0.0003%) to 73.134% when pulling 4f5656e on dstdfx:server-live-migration into 8ca2d18 on gophercloud:master.

theopenlab-ci · 2018-01-19T18:08:39Z

Build succeeded.

gophercloud-unittest http://logs.openlabtesting.org/logs/28/728/4f5656e2e79dfd2e604f3b0d2a63b1dc2af38a35/check/gophercloud-unittest/279607d/ : SUCCESS in 2m 12s
gophercloud-acceptance-test http://logs.openlabtesting.org/logs/28/728/4f5656e2e79dfd2e604f3b0d2a63b1dc2af38a35/check/gophercloud-acceptance-test/985befc/ : SUCCESS in 41m 49s

dstdfx · 2018-01-19T18:18:30Z

@jtopjian This is ready for review, but I've got -0.0003% as a decreased coverage after some refactoring, is it possible to skip it ?

jtopjian · 2018-01-19T20:31:19Z

@dstdfx I think we can let that slide 😉

Thank you for working on this. I'll have time to do a full review in the next day or so.

jtopjian

@dstdfx Nice work. I have one nit and one questions.

Additionally, I won't be able to run the acceptance test for this since I don't have a multi-node test environment on-hand at the moment. I'll take your word that the acceptance test works 😄

But that brings up a good point: the OpenLab CI environment probably can't run this test, either, so we will want to use an environment variable to trigger when this can be run.

Can you add a field called LiveMigrate (bool) here.

Then set the value by checking if OS_LIVE_MIGRATE is set here. Don't error if it's not set.

Add the LiveMigrate field here, too.

Then in the acceptance test, check if choices.LiveMigrate is false. If so, do t.Skip().

You can obtain choices by doing this.

Let me know if you need help with this - I realize it is a unique request for this test.

note: The reason we don't do this for the other Migrate test is because Nova can migrate to the same host (if nova.conf is set accordingly).

jtopjian · 2018-01-20T04:20:34Z

openstack/compute/v2/extensions/migrate/requests.go

+
+	// The host to which to migrate the server.
+	// If this parameter is None, the scheduler chooses a host.
+	Host *string `json:"host"`


Is this so you can specify "" as a valid host value? Is that different than if host was omitted from the request body?

@jtopjian It will be "valid", but I reckon that "host" has to be always in the request body according to this line.
Moreover there's no clear mention as for migrate-server-migrate-action

If you specify null or don’t specify this parameter, the scheduler chooses a host.

I might be wrong please correct me.

jtopjian · 2018-01-20T04:21:04Z

openstack/compute/v2/extensions/migrate/requests.go

+
+// LiveMigrateOpts specifies parameters of live migrate action.
+type LiveMigrateOpts struct {
+


nit: can you remove this line?

coveralls · 2018-01-20T09:10:45Z

Coverage decreased (-0.0003%) to 73.134% when pulling 1d12f0d on dstdfx:server-live-migration into 8ca2d18 on gophercloud:master.

theopenlab-ci · 2018-01-20T09:41:48Z

Build succeeded.

gophercloud-unittest http://logs.openlabtesting.org/logs/28/728/1d12f0dab2963ec79346351ce68bd6605a1c3cc2/check/gophercloud-unittest/94ff5c9/ : SUCCESS in 1m 17s
gophercloud-acceptance-test http://logs.openlabtesting.org/logs/28/728/1d12f0dab2963ec79346351ce68bd6605a1c3cc2/check/gophercloud-acceptance-test/6c63641/ : SUCCESS in 33m 06s

jtopjian

@dstdfx Looks good! Thank you for implementing this.

Two small changes.

With regard to the Host setting, are you able to test whether or not Host needs to be "" or if it can be omitted?

jtopjian · 2018-01-20T15:39:57Z

acceptance/clients/clients.go

@@ -56,6 +60,7 @@ func AcceptanceTestChoicesFromEnv() (*AcceptanceTestChoices, error) {
 	shareNetworkID := os.Getenv("OS_SHARE_NETWORK_ID")
 	dbDatastoreType := os.Getenv("OS_DB_DATASTORE_TYPE")
 	dbDatastoreVersion := os.Getenv("OS_DB_DATASTORE_VERSION")
+	liveMigrate := os.Getenv("OS_LIVE_MIGRATE")


It might be easier to do this:

var liveMigrate bool if v := os.Getenv("OS_LIVE_MIGRATE"); v != "" { liveMigrate = true }

@jtopjian ~~Unfortunately I can't test it on my environment~~

jtopjian · 2018-01-20T15:40:20Z

acceptance/openstack/compute/v2/migrate_test.go

+	}
+
+	if !choices.LiveMigrate {
+		t.Skip()


How about

t.Skip("Testing of live migration is disabled")

coveralls · 2018-01-20T16:46:29Z

Coverage decreased (-0.0003%) to 73.134% when pulling 729daa9 on dstdfx:server-live-migration into 8ca2d18 on gophercloud:master.

theopenlab-ci · 2018-01-20T17:20:19Z

Build succeeded.

gophercloud-unittest http://logs.openlabtesting.org/logs/28/728/729daa91565fd8df912961dc2182e9ca7fcad3f2/check/gophercloud-unittest/e90e071/ : SUCCESS in 1m 21s
gophercloud-acceptance-test http://logs.openlabtesting.org/logs/28/728/729daa91565fd8df912961dc2182e9ca7fcad3f2/check/gophercloud-acceptance-test/e79caa6/ : SUCCESS in 32m 58s

jtopjian · 2018-01-20T19:17:32Z

@dstdfx

Unfortunately I can't test it on my environment

Do you mean you can't test this PR at all?

dstdfx · 2018-01-20T21:52:59Z

@jtopjian Sorry for balls-up! Finally I've tested cases with omitted host in request body, empty string and nil, so the results from nova-api.log:

DEBUG nova.api.openstack.wsgi [req-e3ee5fc5-8bc2-4be1-b457-06742cdac9a7 b6ba9c75146a49829a7427a3e8cc3c10 192796e61c174f718d6147b129f3f2ff] Action: 'action', calling method: <bound method MigrateServerController._migrate_live of <nova.api.openstack.compute.migrate_server.MigrateServerController object at 0x7f0d409eadd0>>, body: {"os-migrateLive":{"block_migration":false,"disk_over_commit":false}} _process_stack /usr/lib/python2.7/dist-packages/nova/api/openstack/wsgi.py:633 2018-01-21 00:44:51,425.425 28797 DEBUG nova.api.openstack.wsgi [req-e3ee5fc5-8bc2-4be1-b457-06742cdac9a7 b6ba9c75146a49829a7427a3e8cc3c10 192796e61c174f718d6147b129f3f2ff] Returning 400 to user: Invalid input for field/attribute os-migrateLive. Value: {u'disk_over_commit': False, u'block_migration': False}. 'host' is a required property __call__ /usr/lib/python2.7/dist-packages/nova/api/openstack/wsgi.py:1044
DEBUG nova.api.openstack.wsgi [req-cb1e7a08-10c7-4e5b-8c3e-8bfc788be0ec b6ba9c75146a49829a7427a3e8cc3c10 192796e61c174f718d6147b129f3f2ff] Action: 'action', calling method: <bound method MigrateServerController._migrate_live of <nova.api.openstack.compute.migrate_server.MigrateServerController object at 0x7f0d409eadd0>>, body: {"os-migrateLive":{"block_migration":false,"disk_over_commit":false,"host":""}} _process_stack /usr/lib/python2.7/dist-packages/nova/api/openstack/wsgi.py:633 2018-01-20 23:56:27,170.170 28792 DEBUG nova.api.openstack.wsgi [req-cb1e7a08-10c7-4e5b-8c3e-8bfc788be0ec b6ba9c75146a49829a7427a3e8cc3c10 192796e61c174f718d6147b129f3f2ff] Returning 400 to user: Invalid input for field/attribute host. Value: . u'' is too short __call__ /usr/lib/python2.7/dist-packages/nova/api/openstack/wsgi.py:1044
DEBUG nova.api.openstack.wsgi [req-07bcd3e0-4e85-4ed2-9b1b-7766fe8841f5 b6ba9c75146a49829a7427a3e8cc3c10 192796e61c174f718d6147b129f3f2ff] Action: 'action', calling method: <bound method MigrateServerController._migrate_live of <nova.api.openstack.compute.migrate_server.MigrateServerController object at 0x7f0d409eadd0>>, body: {"os-migrateLive":{"block_migration":false,"disk_over_commit":false,"host":null}} _process_stack /usr/lib/python2.7/dist-packages/nova/api/openstack/wsgi.py:633 2018-01-21 00:15:54,153.153 28793 DEBUG nova.compute.api [req-07bcd3e0-4e85-4ed2-9b1b-7766fe8841f5 b6ba9c75146a49829a7427a3e8cc3c10 192796e61c174f718d6147b129f3f2ff] [instance: b4811f78-8fa0-4c90-a67f-b2cacf34cc49] Fetching instance by UUID get /usr/lib/python2.7/dist-packages/nova/compute/api.py:2224

jtopjian · 2018-01-20T22:13:36Z

@dstdfx Thank you for getting that information :)

I realize this PR is more difficult to test, so if you weren't able to test, it's understandable. I would have picked it up at some point in the future. But since you can test it, that's great.

So it looks like Nova does not like when host is omitted and when host is "".

What about

Host string `json:"host"`

That should put "host": null in the request body.

dstdfx · 2018-01-20T22:25:29Z

@jtopjian Are you sure? As far as I know we will get empty string if we don't specify Host field. Just as pointer to string will give us nil -> unmarshalled null. https://play.golang.org/p/etRTk0Hf2cm

jtopjian · 2018-01-20T22:38:06Z

I apologize. I misunderstood that *string wasn't working when in fact it is.

So we've confirmed that all options are correct, then, right?

dstdfx · 2018-01-20T22:51:38Z

@jtopjian Right

jtopjian · 2018-01-21T02:18:37Z

In that case, this looks good to me. Thank you for your help and patience with this one -- it's really appreciated.

dstdfx added 3 commits January 19, 2018 14:49

Add support for server live-migration

6490524

Add unit test

8e53312

Fix 'Host' type string -> *string to be able to set it to nil

36c6c90

dstdfx force-pushed the server-live-migration branch from fd197c6 to 36c6c90 Compare January 19, 2018 14:12

Add acceptance test

4f5656e

dstdfx force-pushed the server-live-migration branch from b8706ed to 4f5656e Compare January 19, 2018 17:19

jtopjian requested changes Jan 20, 2018

View reviewed changes

Add ability to turn-on/off acceptance test for server live-migration

1d12f0d

jtopjian requested changes Jan 20, 2018

View reviewed changes

Get rid of 'strconv', add description when skip migrate test

729daa9

jtopjian approved these changes Jan 21, 2018

View reviewed changes

jtopjian merged commit a13ccee into gophercloud:master Jan 21, 2018

dstdfx deleted the server-live-migration branch January 21, 2018 09:50

jtopjian mentioned this pull request Feb 21, 2018

Compute v2: Add Live Migration Action #520

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Compute v2: Add Live Migration Action #728

Compute v2: Add Live Migration Action #728

dstdfx commented Jan 17, 2018 •

edited

coveralls commented Jan 17, 2018 •

edited

theopenlab-ci bot commented Jan 17, 2018

coveralls commented Jan 19, 2018 •

edited

coveralls commented Jan 19, 2018 •

edited

theopenlab-ci bot commented Jan 19, 2018

coveralls commented Jan 19, 2018 •

edited

theopenlab-ci bot commented Jan 19, 2018

dstdfx commented Jan 19, 2018

jtopjian commented Jan 19, 2018

jtopjian left a comment •

edited

jtopjian Jan 20, 2018 •

edited

dstdfx Jan 20, 2018

jtopjian Jan 20, 2018

coveralls commented Jan 20, 2018 •

edited

theopenlab-ci bot commented Jan 20, 2018

jtopjian left a comment

jtopjian Jan 20, 2018

dstdfx Jan 20, 2018 •

edited

jtopjian Jan 20, 2018

coveralls commented Jan 20, 2018 •

edited

theopenlab-ci bot commented Jan 20, 2018

jtopjian commented Jan 20, 2018

dstdfx commented Jan 20, 2018

jtopjian commented Jan 20, 2018

dstdfx commented Jan 20, 2018

jtopjian commented Jan 20, 2018

dstdfx commented Jan 20, 2018

jtopjian commented Jan 21, 2018


		// LiveMigrateOpts specifies parameters of live migrate action.
		type LiveMigrateOpts struct {

Compute v2: Add Live Migration Action #728

Compute v2: Add Live Migration Action #728

Conversation

dstdfx commented Jan 17, 2018 • edited

coveralls commented Jan 17, 2018 • edited

theopenlab-ci bot commented Jan 17, 2018

coveralls commented Jan 19, 2018 • edited

coveralls commented Jan 19, 2018 • edited

theopenlab-ci bot commented Jan 19, 2018

coveralls commented Jan 19, 2018 • edited

theopenlab-ci bot commented Jan 19, 2018

dstdfx commented Jan 19, 2018

jtopjian commented Jan 19, 2018

jtopjian left a comment • edited

Choose a reason for hiding this comment

jtopjian Jan 20, 2018 • edited

Choose a reason for hiding this comment

dstdfx Jan 20, 2018

Choose a reason for hiding this comment

jtopjian Jan 20, 2018

Choose a reason for hiding this comment

coveralls commented Jan 20, 2018 • edited

theopenlab-ci bot commented Jan 20, 2018

jtopjian left a comment

Choose a reason for hiding this comment

jtopjian Jan 20, 2018

Choose a reason for hiding this comment

dstdfx Jan 20, 2018 • edited

Choose a reason for hiding this comment

jtopjian Jan 20, 2018

Choose a reason for hiding this comment

coveralls commented Jan 20, 2018 • edited

theopenlab-ci bot commented Jan 20, 2018

jtopjian commented Jan 20, 2018

dstdfx commented Jan 20, 2018

jtopjian commented Jan 20, 2018

dstdfx commented Jan 20, 2018

jtopjian commented Jan 20, 2018

dstdfx commented Jan 20, 2018

jtopjian commented Jan 21, 2018

dstdfx commented Jan 17, 2018 •

edited

coveralls commented Jan 17, 2018 •

edited

coveralls commented Jan 19, 2018 •

edited

coveralls commented Jan 19, 2018 •

edited

coveralls commented Jan 19, 2018 •

edited

jtopjian left a comment •

edited

jtopjian Jan 20, 2018 •

edited

coveralls commented Jan 20, 2018 •

edited

dstdfx Jan 20, 2018 •

edited

coveralls commented Jan 20, 2018 •

edited