| *** bhagyashris_ is now known as bhagyashris | 03:02 | |
| *** akekane_ is now known as abhishekk | 07:32 | |
| *** gibi_ is now known as gibi | 07:52 | |
| gibi | o/ morning nova | 08:02 |
|---|---|---|
| gibi | sean-k-mooney, bauzas, gmann: yesterday I forgot to read back on the instance_action proposal from Tuesday. Now I did | 08:03 |
| gibi | my view is that we can and therefore should decouple the API behavior from the DB representation | 08:03 |
| gibi | so if we agree that we change the deletion of instance_action in the DB that should not impact how our API behaves | 08:04 |
| gibi | so assuming we agree to make the instance action DB table rows soft deleted when the instance is soft deleted, we should change our query in the API to read soft deleted instance actions too | 08:05 |
| gibi | and then we are done. OVH gets the DB consistency they need, and nova keeps all the external behavior unchanged | 08:05 |
| gibi | win - win | 08:05 |
| pslestan1 | hello! | 08:17 |
| pslestan1 | gibi: agree with that and seems aligned with what sean-k-mooney suggested | 08:19 |
| gibi | pslestan1: o/ | 08:20 |
| bauzas | gibi: yup, but we discussed about the default for the new microversion | 08:20 |
| gibi | does anybody requested an API change on nova? :) | 08:20 |
| bauzas | gibi: your thought was what I said first | 08:21 |
| gibi | or rephrahsing that, why we want to change API behavior? | 08:21 |
| bauzas | gibi: I don't wanna change the API | 08:21 |
| gibi | cool, then we are on the same page | 08:21 |
| pslestan1 | gibi: about DB consistency I guess that it's more a community oriented choice than OVH only, I mean this is a good thing to get a consistency for everybody not only OVH | 08:22 |
| gibi | pslestan1: sure, sorry for pin it up on OVH, just needed a name for the set of people who need this change | 08:23 |
| pslestan1 | gibi: don't worry | 08:23 |
| gibi | :) | 08:23 |
| sean-k-mooney | gibi: yep i was orginaly suggesting that if we were ever going to change the api behaivor it made sense to cahnge it now but it looks like we dont want to expose this at the api at all | 09:57 |
| sean-k-mooney | gibi: chanig the api behavior would be just to have partity with the servers endpoint | 09:57 |
| gibi | I don't need the API behavior to change | 09:57 |
| gibi | I think it make sense that the history of a VM overlives the VM itself | 09:57 |
| gibi | outlive :D | 09:58 |
| sean-k-mooney | for server list you need to pass --deleted to see the soft deleted ones | 09:58 |
| sean-k-mooney | you dont have to do that for show | 09:58 |
| sean-k-mooney | as far asim aware jsut the list endpoint | 09:58 |
| sean-k-mooney | anyway its fine do we still need a spec if we are not changing the api now | 09:59 |
| sean-k-mooney | we will start soft deleting them in the db but we will change the api to ignore the deleted column | 09:59 |
| sean-k-mooney | so the behavior is the same. | 10:00 |
| gibi | do we need to add a data migration to fill the deleted column of existing actions? | 10:00 |
| gibi | maybe based on the the fact if the instance is deleted or not yet | 10:00 |
| sean-k-mooney | i dont think we do | 10:03 |
| sean-k-mooney | we can just remvoe the deleted column form the query | 10:03 |
| gibi | OK, then I think we don't need a spec, but I let bauzas to decide with his PTL hat | 10:03 |
| sean-k-mooney | we can have an optional nova-manage command to fill it in | 10:03 |
| sean-k-mooney | but i dont think it should be required | 10:04 |
| bauzas | gibi: my first comment was to say : OK for a specless BP *but* not for the last phrase | 10:05 |
| bauzas | https://meetings.opendev.org/meetings/nova/2021/nova.2021-12-07-16.00.log.html#l-203 | 10:06 |
| gibi | <bauzas> "We should also implement the possibility to retrieve deleted instance actions as we do for instances." seems like an API change, right? | 10:07 |
| gibi | but there our answer is to keep the API as is | 10:07 |
| gibi | that fullfills this requirement without any API behavior change | 10:07 |
| gibi | as the API always returns both deleted and not delete actions :) | 10:07 |
| gibi | if the requirement is to retrive deleted instance actions _only_ | 10:08 |
| gibi | then there is an API change | 10:08 |
| sean-k-mooney | gibi: ya i dont think there really is a usecase fo deleted _only_ | 10:10 |
| sean-k-mooney | so i think we are ok to not change the db | 10:11 |
| sean-k-mooney | *api | 10:11 |
| gibi | yepp, I'm on that side yesa | 10:11 |
| sean-k-mooney | so the cahnge is really to two things | 10:11 |
| sean-k-mooney | the server delete sql query/funciton will need to soft delete the instance actions | 10:12 |
| sean-k-mooney | and the instance actions quies will need to ignore the deleted field | 10:12 |
| sean-k-mooney | and that shoudl be sufficnet | 10:12 |
| pslestang | bauzas: I can change the BP to remove the possibility to retrieve deleted instance action as the solution proposed does not need such feature | 10:12 |
| sean-k-mooney | oh and server restore shoudl also restore the instance_actions | 10:12 |
| bauzas | pslestang: okay then | 10:13 |
| pslestang | bauzas: done | 10:14 |
| bauzas | gibi: sean-k-mooney: would we need to modify the DB if we accept to soft-delete the instance actions ? | 10:15 |
| bauzas | pslestang: maybe that's here we would need to discuss about upgrades | 10:16 |
| gibi | bauzas: we need to modify the DB yes, and need to modify the DB query the API does to ignore the new column | 10:16 |
| bauzas | that's my question | 10:16 |
| gibi | so if you are strict then there is an upgrade impact, as the db schema changes | 10:16 |
| bauzas | for upgrades | 10:16 |
| bauzas | I mean | 10:17 |
| bauzas | for upgrading there are two directioins | 10:17 |
| bauzas | 1/ we want to modify the DB for already existed instance actions | 10:17 |
| bauzas | 2/ we don't do it and we only soft-delete the new instance actions | 10:17 |
| bauzas | 1/ would mean that we would need to have a nova-manage command | 10:18 |
| bauzas | 2/ wouldn't | 10:18 |
| bauzas | for both, we need to discuss how the API query would do | 10:18 |
| bauzas | remember that we have cells v2 | 10:19 |
| bauzas | so we would need to know whether the cell DB is upgraded or not | 10:19 |
| bauzas | before calling the instance actions table | 10:19 |
| bauzas | (from the API I mean) | 10:19 |
| bauzas | anyway, I need to get my kid from school in a few mins | 10:20 |
| bauzas | but I guess we probably need to want to discuss about the upgrade questions in some... spec ? | 10:20 |
| gibi | bauzas: that is your call :) I'm happy to discuss this in a spec | 10:20 |
| bauzas | I don't wanna use my baton | 10:21 |
| bauzas | it's more a core question | 10:21 |
| bauzas | about upgrades | 10:21 |
| gibi | regarding nova-manage that feels optional to me, and probably pslestang can state if OVH needs it or not | 10:21 |
| gibi | so this is a requirement question | 10:21 |
| bauzas | yeah | 10:21 |
| pslestang | bauzas: by the way we already need to change the nova-manage purge behaviour which actually rely on update_at column for instance_action_* tables instead of deleted_at | 10:21 |
| bauzas | correct | 10:21 |
| bauzas | we also need to look at which methods look at instance actions | 10:22 |
| bauzas | we know the API for sure | 10:22 |
| bauzas | but I wonder whether we also look at the actions within nova directly | 10:22 |
| sean-k-mooney | bauzas: we dont need to modify the db but we do need to modify hte db queries | 10:22 |
| sean-k-mooney | the schema will remain the same | 10:22 |
| *** redrobot6 is now known as redrobot | 10:23 | |
| bauzas | sean-k-mooney: agreed this isn't a DB schema modifcatioin | 10:23 |
| gibi | sean-k-mooney: ooh the same already has deleted column, I missed that | 10:23 |
| bauzas | but this is about the values | 10:23 |
| sean-k-mooney | gibi: yep it already inherits form teh softdelete mixin | 10:23 |
| pslestang | gibi: at OVH we do not use nova-manage but it could be useful for someone | 10:23 |
| bauzas | for the moment, soft-deleted instances have not deleted actions | 10:23 |
| sean-k-mooney | gibi: we just currently dont soft delete it | 10:23 |
| bauzas | if we start to soft-delete actions when deleting instances, then I wonder what happens for existing instance related actions | 10:24 |
| sean-k-mooney | the remain not deleted | 10:24 |
| sean-k-mooney | but that wont mater | 10:24 |
| bauzas | sean-k-mooney: this is one direction | 10:24 |
| sean-k-mooney | since we will just start ignoring that field | 10:24 |
| bauzas | or one solution | 10:24 |
| sean-k-mooney | well setting a colume to the value it currently has in an update is valid | 10:25 |
| bauzas | anyway, needs to get my daughter, moving | 10:25 |
| sean-k-mooney | so the resotre can ignore it too and just always set it to 0 | 10:25 |
| sean-k-mooney | even if its already 0 becuase we did not migrate the existing ones to eb soft deleted | 10:25 |
| sean-k-mooney | pslestang: wehn you archive the deleted rows it will delete the instance actions | 10:26 |
| sean-k-mooney | so by the time you get to purge they shoudl already be gone form the main table | 10:26 |
| sean-k-mooney | archiveing the delete rows will remove the instance form the main table which would break the forien key constraitnt on the isntnace action table | 10:27 |
| sean-k-mooney | so the rows have to be removed to archive the instance to the shadow tables | 10:28 |
| pslestang | sean-k-mooney: from what I saw when you archive the deleted rows it does not delete the instance actions but copy them in shadow tables | 10:28 |
| sean-k-mooney | it shoudl delete them form the main table and copy them to the shadow table | 10:28 |
| sean-k-mooney | basically a move | 10:28 |
| pslestang | yes that's it | 10:28 |
| sean-k-mooney | yep that is the expect behavior | 10:29 |
| sean-k-mooney | and pruge should remove them for the shadow tabels | 10:29 |
| pslestang | and when we purge de archived rows, it deletes the instances based on deleted_at date, and deleted the insatnce_action bases on updated_at date | 10:29 |
| sean-k-mooney | if you are using --before yes | 10:30 |
| pslestang | yep | 10:30 |
| sean-k-mooney | we could likely adapt that to use delete_at if its popluatedand fall abck to updated at if not | 10:30 |
| sean-k-mooney | or just leave it as is | 10:31 |
| pslestang | I prefer to adapt the code to make it consistent with the soft-delete of instance_actions | 10:33 |
| sean-k-mooney | bauzas: im prety sure we dont look at the actions in nova they are write only | 10:33 |
| sean-k-mooney | pslestang: ok would you prefer to write this all down in a short spec which we can try an appove in the review day on the 14th | 10:34 |
| sean-k-mooney | or just proceed with this as a specless blueprint | 10:34 |
| pslestang | I can write a short spec | 10:34 |
| sean-k-mooney | i dont thinks we really need a spec but i think having one to document this would be good so im happy to review it if you write it | 10:34 |
| pslestang | sure | 10:35 |
| sean-k-mooney | cool feel free to add me and likely gibi/bauzas to it | 10:35 |
| pslestang | is it better to change the one I wrote or to create a new one? | 10:35 |
| sean-k-mooney | oh you already have one? | 10:35 |
| pslestang | I was talking about the BP | 10:35 |
| sean-k-mooney | oh reuse the blueprint you have now but file a spec with the same name | 10:36 |
| pslestang | ok understand, I will dot it asap | 10:36 |
| sean-k-mooney | cool | 10:37 |
| * bauzas is back, scrolling up | 10:45 | |
| bauzas | pslestang: thanks pslestang for writing up a quick spec | 10:46 |
| bauzas | you can see the template and the already existing specs too | 10:46 |
| bauzas | I'm not really asking about a paperwork, just want to be sure we don't miss any important issue | 10:47 |
| sean-k-mooney | yep for me the spec is more just documentaiton/todo list in this case rather then a detailed design requirement because its complex | 10:48 |
| sean-k-mooney | this is a relitively simple change we just dont want to miss any interop or upgrade impact | 10:49 |
| sean-k-mooney | i think we are all supportive of doing it so you dont really need to convice use just detail what needs to be done | 10:50 |
| gibi | bauzas: could you quickly mark this wontfix https://bugs.launchpad.net/nova/+bug/1953734 as it is talking about a deprecated API ? (I don't want to do it as I was asked downstream to open it :D) | 10:53 |
| bauzas | gibi: hehe sure :p | 10:54 |
| bauzas | do you want my bank account ? | 10:54 |
| bauzas | :D | 10:55 |
| gibi | I will pay in beer | 10:55 |
| gibi | :D | 10:55 |
| gibi | when we finally meet again | 10:55 |
| bauzas | well, if I was paying for a vine in 2019 for the next meeting, then it would be a very nice one :) | 10:55 |
| bauzas | after 3 years | 10:56 |
| sean-k-mooney | gibi: what is the invalid input in this case by the way | 10:56 |
| bauzas | hmmm, https://docs.openstack.org/api-ref/compute/?expanded=list-security-groups-by-server-detail#servers-security-groups-servers-os-security-groups | 10:57 |
| bauzas | unfortunately, we don't document the fact that the POST action is now deprecated | 10:57 |
| gibi | sean-k-mooney: security_group is expected to be a dict not a string | 10:57 |
| sean-k-mooney | ah ok | 10:57 |
| gibi | These APIs are proxy calls to the Network service. Nova has deprecated all the proxy APIs and users should use the native APIs instead. These will fail with a 404 starting from microversion 2.36. See: Relevant Network APIs. | 10:57 |
| bauzas | gibi: yeah I know | 10:58 |
| gibi | I think this is for the whole /os-security-groups resource | 10:58 |
| sean-k-mooney | yep the post action i tough twas only vailid for nova networks | 10:58 |
| bauzas | gibi: I'm just saying we don't document it in our API docs :p | 10:58 |
| bauzas | gibi: well, then the GET /os-security-groups action should be told to by deprecated in our API docs :) | 10:58 |
| gibi | I think the documentation is there in the header | 10:58 |
| bauzas | oh, did I miss something in the docs ? | 10:59 |
| sean-k-mooney | https://docs.openstack.org/api-ref/compute/?expanded=list-security-groups-by-server-detail#list-security-groups | 10:59 |
| sean-k-mooney | bauzas: its in the os-security-groups section | 10:59 |
| sean-k-mooney | These APIs are proxy calls to the Network service. Nova has deprecated all the proxy APIs and users should use the native APIs instead. These will fail with a 404 starting from microversion 2.36. See: Relevant Network APIs. | 10:59 |
| bauzas | I'm blind | 10:59 |
| sean-k-mooney | you were looking at the instance action | 10:59 |
| sean-k-mooney | that does not have the hearder | 11:00 |
| * gibi even started creating a screenshot for bauzas | 11:00 | |
| sean-k-mooney | but the os-security-groups endpoint section does | 11:00 |
| bauzas | what the f*** | 11:00 |
| sean-k-mooney | bauzas: https://docs.openstack.org/api-ref/compute/?expanded=list-security-groups-by-server-detail#delete-deallocate-floating-ip-address | 11:00 |
| sean-k-mooney | the big red box ^ | 11:00 |
| bauzas | yeah I get it now | 11:00 |
| sean-k-mooney | :) | 11:00 |
| bauzas | but... can't get why I wasn't able to see it in my own link | 11:01 |
| sean-k-mooney | bauzas: because you were not looking in the right section | 11:01 |
| sean-k-mooney | you were looking at /servers/uuid/os-secuirty-groups | 11:01 |
| sean-k-mooney | that was not deprecated | 11:01 |
| * bauzas facepalms | 11:01 | |
| bauzas | Ctrl+F | 11:01 |
| sean-k-mooney | but /os-security-groups and /os-security-group-rules were | 11:02 |
| bauzas | not the right endpoint | 11:02 |
| sean-k-mooney | yep easy mistake to make | 11:02 |
| sean-k-mooney | brb | 11:02 |
| bauzas | gibi: done, you owe me one :p | 11:07 |
| gibi | thanks, and noted :) | 11:07 |
| plibeau | lyarwood: To explain what append in my case: context, the image used during the boot of the instance is now private and the base image on the source compute don't exsit. | 13:40 |
| plibeau | during the resize on the instance in the code we go in "_try_fetch_image_cache" (nova/virt/libvirt/driver.py +10359) and go in except because image.cache can't found image (because now the image is private). In the except image.cache is call again without image in parameter so create_image in call also without image_id. generating is True because image_id not found in kwargs | 13:40 |
| plibeau | and the self.exists because it's a resize so the disk already exist. | 13:40 |
| plibeau | I don't change in image.cache method the line 280 (if os.path.exists(base) and size > self.get_disk_size(base):) because it's the same code for other type of type (qow2,etc) and it's not good idea IMPOV to change that but mayY | 13:40 |
| plibeau | https://review.opendev.org/c/openstack/nova/+/820531 | 13:40 |
| plibeau | *but maybe I'm wrong | 13:46 |
| opendevreview | sean mooney proposed openstack/nova master: This change adds generative envs https://review.opendev.org/c/openstack/nova/+/804292 | 14:02 |
| opendevreview | sean mooney proposed openstack/nova master: This change adds generative envs https://review.opendev.org/c/openstack/nova/+/804292 | 14:04 |
| opendevreview | sean mooney proposed openstack/nova master: [WIP] allow cpus to be externally managed https://review.opendev.org/c/openstack/nova/+/821228 | 14:41 |
| lyarwood | plibeau: Apologies was on calls downstream | 14:55 |
| * lyarwood reads back | 14:55 | |
| lyarwood | I think I see what you mean | 14:59 |
| lyarwood | and apologies I was mixing resize and rebuild | 15:00 |
| opendevreview | mitya-eremeev-2 proposed openstack/nova master: Delete bogus attachments. https://review.opendev.org/c/openstack/nova/+/820935 | 15:13 |
| plibeau1 | okay I can copy my detail in the change if you want? And you prefer another change for the refacto? | 15:24 |
| lyarwood | plibeau1: same change, new patchset would be fine I think? | 15:31 |
| plibeau1 | probably yes I will check to apply your proposal on unit test | 15:33 |
| lyarwood | yeah just amend the commit and git review again | 15:45 |
| opendevreview | Pierre Libeau proposed openstack/nova master: Nova resize don't extend disk in one specific case https://review.opendev.org/c/openstack/nova/+/820531 | 16:31 |
| gmann | gibi: sean-k-mooney bauzas pslestang : I am fine not to change API as no one asked it. but +1 on spec to get clear view/documentation | 16:38 |
| sean-k-mooney | gmann: ack | 17:27 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Log which instance event was timed out https://review.opendev.org/c/openstack/nova/+/819817 | 17:47 |
| *** weechat1 is now known as amorin | 17:54 | |
| *** weechat1 is now known as amorin | 18:00 | |
| mloza | /j #openstack-keystone | 19:24 |
| opendevreview | sean mooney proposed openstack/nova-specs master: add per process healthcheck spec https://review.opendev.org/c/openstack/nova-specs/+/821279 | 19:28 |
| opendevreview | melanie witt proposed openstack/nova master: Enable unified limits in the nova-next job https://review.opendev.org/c/openstack/nova/+/789963 | 21:17 |
| *** tbachman_ is now known as tbachman | 22:54 | |
| *** tbachman_ is now known as tbachman | 23:04 | |
| *** artom__ is now known as artom | 23:39 | |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!