Add migrations list support in compute service #3244
Conversation
There was a problem hiding this comment.
Thank you for submitting your first PR! Be sure that we will be looking at it but keep in mind
this sometimes takes a while.
Please let the maintainers know if your PR has not got enough attention after a few days.
If any doubt, please consult our PR tutorial.
| @@ -0,0 +1,22 @@ | ||
| package osmigrations |
There was a problem hiding this comment.
Can we simply call this migrations? The os- prefix is from the days where this was an extension, but extensions are no longer a thing (nor have they been for some time now).
| // The source/destination compute node of migration to filter | ||
| Host *string `q:"host"` | ||
| // The uuid of the instance that migration is operated on to filter | ||
| InstanceUuid *string `q:"instance_uuid"` |
There was a problem hiding this comment.
| InstanceUuid *string `q:"instance_uuid"` | |
| InstanceID *string `q:"instance_uuid"` |
Or InstanceUUID, though IMO the ID/UUID distinction isn't useful: both ProjectID and UserID are UUIDs also.
rename InstanceUuid to InstanceID
| // The ID of the server migration | ||
| Id int64 `json:"id"` | ||
| // The UUID of the server | ||
| InstanceUuid string `json:"instance_uuid"` |
|
Failed to compute_task_migrate_server: No valid host was found. There are not enough hosts available. |
| allMigrations, err := migrations.ExtractMigrations(allPages) | ||
| th.AssertNoErr(t, err) | ||
|
|
||
| th.AssertIntGreaterOrEqual(t, len(allMigrations), 1) |
There was a problem hiding this comment.
migrate_test.go:39: Failure in migrate_test.go, line 39: The first value "0" is lesser than the second value "1"
I can't immediately spot what the issue is, the code looks fine, and this should be running as admin.
There was a problem hiding this comment.
You found your issue already :)
We'll need to add another compute for the nova job to test migrations. @stephenfin do you have an idea if that's easily feasible?
@Patzifist in case you want to give it a shot, the job is configured in https://github.com/gophercloud/gophercloud/blob/master/.github/workflows/functional-compute.yaml.
There was a problem hiding this comment.
I suggest disabling the test and merging the request. what do you think about it?
There was a problem hiding this comment.
Sorry for the lag anwering your question.
Rather than deleting a test that is otherwise good, could we instead skip it if there's less than 2 computes?
For #3243
Links to the line numbers/files in the OpenStack source code that support the
code in this PR:
Document
https://docs.openstack.org/api-ref/compute/#list-migrations
Code
https://github.com/openstack/nova/blob/stable/2024.2/nova/api/openstack/compute/migrations.py