Open Stack

On Tue, Jul 27, 2021 at 12:59 PM Sean Mooney <smooney at redhat.com> wrote:
>

Hi Sean,

Thanks for your reply.

> On Tue, 2021-07-27 at 08:53 +0200, Radosław Piliszek wrote:
> > On Mon, Jul 26, 2021 at 10:36 PM melanie witt <melwittt at gmail.com> wrote:
> > >
> >
> > Hello Melanie,
> >
> > First of all, thank you for your response.
> >
> > > On Sun, 25 Jul 2021, 18:39 +0200, Radosław Piliszek
> > > <radoslaw.piliszek at gmail.com> wrote:
> > > > Sorry for the premature send. I include extra missed details below.
> > > >
> > > > On Sun, Jul 25, 2021 at 6:36 PM Radosław Piliszek
> > > > <radoslaw.piliszek at gmail.com> wrote:
> > > > >
> > > > > Dears,
> > > > >
> > > > > In Masakari we have a spec [0] to track the issued evacuations.
> > > > > It could greatly use having more insight into the migrations details
> > > > > that Nova saves in its database.
> > > > > It seems there are two issues here.
> > > > > One is quite cosmetic one, in that Nova API does not allow getting
> > > > > migrations by UUID but just ID [1] (while UUIDs seem to be called the
> > > > > future and the internal objects support them as well).
> > > > > I think this one can be easily tackled by making that endpoint [1]
> > > > > more flexible.
> > >
> > > Note that you would also have to change the API to allow showing a
> > > migration that is not of migration_type "live-migration". Currently it
> > > only allows showing an in-progress live migration.
> >
> > Ah, I see the description. But I am pretty sure I got it to retrieve
> > an arbitrary ID migration though. But the relevant listing was
> > filtered for sure. Could you confirm that?
> > Should we treat that as a bug then? I hope not because I wanted to
> > rely on it. :-)
>
> the /os-migrations endpoint allows listing migrations of any type (evacuation,live-migration,migration,resize)
> https://docs.openstack.org/api-ref/compute/?expanded=list-migrations-detail#list-migrations
>
> migration_type is both a query paramater and a responce value
>
> /servers/{server_id}/migrations on the other hand i belive is for running live migration only
> https://docs.openstack.org/api-ref/compute/?expanded=id312-detail#list-migrations
>
>
> so yes the fact that /servers/{server_id}/migrations/{migration_id} can retrun non live migraiton i belive is a bug.
> we assumed that you would only be able to see the migration_id form /servers/{server_id}/migrations but if you got
> a migration id form /os-migrations and use it with  /servers/{server_id}/migrations/{migration_id} then it would return the info i think which stricly speaking it should not.
>
> so right now i would not realy on this behavior we likely shoudl block it. have you looked at using /os-migrations instead.

Well, Melanie has confirmed I got it wrong in my earlier testing. The
server_id endpoint is for live migrations only. Not useful here.

And yeah, I looked at /os-migrations, it seems sensible to extend it
to support filtering by UUID then.

> >
> > > > > The other issue is a more complex one, but still not introducing a
> > > > > great deal of change to Nova, and it is that the Nova API returns
> > > > > neither migration ID nor UUID when asked to evacuate an instance [2]
> > > > > (to be more general, it does not do it for migrations and live
> > > > > migrations either).
> > > >
> > > > So my idea would be to extend the API response here to include the
> > > > migration UUID which is all we would need.
> > > > It seems the Migration object is created and saved on request so it's
> > > > only a matter of wiring it up.
> > >
> > > You are correct that the Migration object is written to the database
> > > before the API response occurs, so it would be technically possible to
> > > add UUID to the response in a new API microversion.
> >
> > Thank you for confirming.
> i dont think i would have any objection ot returning the migration uuid, its an async operation so returning
> a handel in this case the uuid to chaeck its status would make sense. unfortunelty as i noted above the only way to use that
> is really expliting a bug.
>
> /os-mirgations does not curetnly expose the migration uuid as a query parmater but it does have the uuid in the responce as of 2.59
>
> so if we were to return the uuid in the evaucate ectra action reponces we likely should add
> "/os-mirgations/${uuid}" or add the migration uuid as a query arg to the current list endpoint.
>

Roger that.

> >
> > > > > It seems, otherwise, we would have to call the
> > > > > listing API [3] and find the latest entry there but this is possibly
> > > > > error-prone and wasting resources.
> > >
> > > You wouldn't have to use/assume the latest entry because you can pull
> > > the evacuations for a server using:
> > >
> > > GET
> > > http://192.168.33.10/compute/v2.1/os-migrations?instance_uuid=f1aece4e-e7f7-436b-9faf-799129503dcc&migration_type=evacuation
> >
> > Yeah, I meant to use that endpoint. It is highly unlikely that we have
> > another evacuation in between but still it's less reliable than just
> > getting the UUID directly.
> >
> >
> > To sum up, since this is introducing a new API microversion, the next
> > step for me is to create a Nova spec, right?
> yes for the yoga cycle we already have the folder created so we can likely start the review
> pretty quickly but not sure when we will start merging spec for yoga. typicallly this has happened a few weeks before
> feature freeze so its a little elarly now but there is no rule against merging them early so personally im happy to review
> and i can bring this up in the next nova team meeting and see if anyone objects to opening the yoga specs review cycle early.

Thanks, I'll propose the spec.

> > I read https://docs.openstack.org/nova/latest/contributor/microversions.html
> > but it was not clear to me whether some changes could do a new API
> > microversion without speccing it all up upfront or not.
> in nova you cannot make any api chnage without a spec.

That's sensible. Only I could not find it written verbatim. Perhaps it
is, somewhere.


All in all, thanks for the discussion, Melanie and Sean.
The spec, as I see it now, would entail:
- adding UUID filtering capability to /os-migrations (simple)
- adding UUID to the response of evacuate API (simple)


-yoctozepto