| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-03-24 | |||
| 16:04:52 | bauzas | sorry | |
| 16:04:54 | bauzas | indeed | |
| 16:05:04 | bauzas | we would only pin the change to 6.0 | |
| 16:05:25 | bauzas | but yeah | |
| 16:05:51 | dansmith | for now, but then we have to fix that immediately before the next RPC minor or we'll start sending the 5.x version for 6.1, so I think we need to be more careful | |
| 16:05:58 | lyarwood | gibi: I can do that in a fup but I think that would be a microversion bump right? | |
| 16:06:09 | openstackgerrit | Stephen Finucane proposed openstack/nova master: WIP: Convert client also https://review.opendev.org/c/openstack/nova/+/782810 | |
| 16:06:19 | gibi | lyarwood: if it is already provided and we are not rejecting it then it is a bug in the API ref | |
| 16:06:19 | stephenfin | dansmith: like that? (need tests obv) ^ | |
| 16:06:32 | bauzas | dansmith: right, and 7.0 bump could also just support 6.y where y>1 | |
| 16:06:35 | gibi | I guess we are not really enforcing schema on that api | |
| 16:06:52 | gibi | lyarwood: it is totally OK to fix the API ref separately | |
| 16:06:54 | gibi | later | |
| 16:06:58 | lyarwood | kk | |
| 16:07:02 | openstackgerrit | Lee Yarwood proposed openstack/nova master: compute: Reject requests to commit intermediary snapshot of an inactive instance https://review.opendev.org/c/openstack/nova/+/781138 | |
| 16:07:16 | bauzas | stephenfin: no, for the reason dansmith mentioned | |
| 16:07:25 | bauzas | we can't only accept 6.0 | |
| 16:07:50 | dansmith | bauzas: well, I might be wrong about that in this case, I was kinda thinking of it differently | |
| 16:08:24 | dansmith | I guess this works because can_send_version(6.0) will be true even once we move to 6.1 | |
| 16:08:43 | stephenfin | well then weren't we doing that already? | |
| 16:08:50 | stephenfin | currently we're passing version=5.1 | |
| 16:08:53 | bauzas | dansmith: not if we bumped 7.0 and we only support 6.5 and later (per say) | |
| 16:09:13 | dansmith | bauzas: yeah, but this will all be removed before we'd bump to 7, | |
| 16:09:25 | dansmith | I was worried about the version calculus for 6.x where x>0 not 7 | |
| 16:09:30 | stephenfin | so we'd have to change that if we added a 6.x that affected that method | |
| 16:10:01 | bauzas | stephenfin: no, we pass either 6.0, 5.1 or 5.0 depending on the server | |
| 16:10:15 | dansmith | *depending on the pin | |
| 16:10:19 | bauzas | right | |
| 16:10:37 | stephenfin | cctxt = client.prepare(server=host, version=version) | |
| 16:10:48 | bauzas | that's the client version | |
| 16:10:58 | stephenfin | oh, I '_ver' is doing the magic | |
| 16:11:04 | bauzas | not the service version we discover based on the pin | |
| 16:11:09 | stephenfin | *the '_ver' method | |
| 16:11:40 | bauzas | the _ver method is temporary | |
| 16:12:00 | bauzas | just to avoid reproducing dumb checks | |
| 16:12:38 | bauzas | that's the pattern we used in the past bumps and I shamelessly reproduced it | |
| 16:13:08 | dansmith | patterns are good :) | |
| 16:13:16 | bauzas | provided people understand them. | |
| 16:14:06 | dansmith | stephenfin: you also want to change the client signature to use flavor too, I'm sure | |
| 16:14:20 | stephenfin | I don't think so. Not yet | |
| 16:14:26 | stephenfin | Keep it small | |
| 16:14:27 | dansmith | why? | |
| 16:14:42 | stephenfin | That's v. easy fix in a follow-up | |
| 16:15:02 | stephenfin | I mean I can but it's more test damage | |
| 16:15:07 | bauzas | right | |
| 16:15:10 | dansmith | well, that's the point here | |
| 16:15:15 | bauzas | some methods pass kwargs | |
| 16:15:27 | bauzas | then, you're dommed | |
| 16:15:29 | bauzas | doomed* | |
| 16:15:32 | dansmith | either we're going for consistency or we aren't right? | |
| 16:18:12 | stephenfin | *eventual consistency | |
| 16:18:22 | stephenfin | I can change the other things whenever. I can only change this right now | |
| 16:18:42 | stephenfin | and I have the follow-up ready. I just don't think it's wise to merge it right now since it's huge | |
| 16:21:07 | dansmith | well, my opinion is that this is all just naming and not useful for paying down any real debt (like deprecated parameters) so I'd vote to just punt on it anyway | |
| 16:24:21 | bauzas | the other problem I see is with the gate | |
| 16:24:37 | bauzas | getting at least a grenade-multinode result | |
| 16:24:57 | dansmith | for sure we need to see and scrutinize a grenade-multinode result at least | |
| 16:25:13 | bauzas | the good thing is that both prep_resize and resize_instance() are tested IIRC | |
| 16:31:45 | bauzas | I have to stop now as I need to taxi my daughter to some doctor's appointment | |
| 16:31:53 | bauzas | but I can be back later on in the evening | |
| 16:36:08 | openstackgerrit | Stephen Finucane proposed openstack/nova master: DNM: Rename 'instance_type' parameter https://review.opendev.org/c/openstack/nova/+/782804 | |
| 16:48:17 | openstackgerrit | Stephen Finucane proposed openstack/nova master: rpc: Rename 'instance_type' client argument https://review.opendev.org/c/openstack/nova/+/782825 | |
| 16:48:49 | stephenfin | dansmith: A'ight, there's the client change ^ Pretty simple after all. I think I'd still keep it separate but we could squash it in if we wanted to | |
| 16:49:40 | stephenfin | Also, for the previous patch, there doesn't appear to have been a test for backleveling of 'prep_resize' so we might want to adding that as a follow-up even if we don't do this (it seems to be working fine, thankfully, based on the test) | |
| 16:50:55 | stephenfin | I realize it doesn't ultimately matter, but when it's this easy I really do think we should be including this. It'd mean we're just left with some DB/o.vo references to instance types. I could live with those | |
| 17:57:55 | stephenfin | oh, lovely http://paste.openstack.org/show/803882/ | |
| 17:58:11 | stephenfin | creating a resource provider on placement with a duplicate name results in a HTTP 5xx | |
| 17:59:27 | melwitt | amazed that hasn't been hit until now | |
| 17:59:52 | stephenfin | You and me both. That's super dumb | |
| 18:00:21 | melwitt | hehehe | |
| 18:00:58 | stephenfin | I won't be able to work on it today, but if anyone else has time the simple reproducer is to use microversion 1.20 and create a resource provider specifying only the name | |
| 18:01:24 | stephenfin | nope, don't even need the microversion | |
| 18:01:38 | stephenfin | $ openstack resource provider create foo; openstack resource provider create foo | |
| 18:01:46 | stephenfin | delightful | |
| 18:01:52 | melwitt | :) | |
| 18:02:31 | melwitt | that'd be easy to add to the gabbits I think | |
| 18:05:11 | stephenfin | for sure | |
| 18:05:16 | stephenfin | I'll take a look tomorrow, no doubt | |
| 18:05:24 | melwitt | kewl | |
| 18:55:27 | sean-k-mooney | we defintly have hit duplicte RPs before | |
| 18:55:41 | sean-k-mooney | but i tough that gave you a 4XX error | |
| 18:56:09 | sean-k-mooney | you get this if you rename a compute node and change its hostname or you delete the compute service and its created with a different uuid | |
| 18:56:34 | sean-k-mooney | sorry only the later case has the name conflcit | |
| 18:56:55 | sean-k-mooney | where you keep the same compute node hostname but delete the compute service and let it get recteated | |
| 18:57:10 | sean-k-mooney | the compute service uuid change but the RP name will be the same | |
| 18:58:54 | sean-k-mooney | stephenfin: melwitt we have test for that in placment already i think https://github.com/openstack/placement/blob/master/placement/tests/functional/gabbits/resource-provider-duplication.yaml | |
| 18:59:11 | sean-k-mooney | https://github.com/openstack/placement/blob/master/placement/tests/functional/gabbits/resource-provider-duplication.yaml#L30-L37 | |
| 18:59:19 | sean-k-mooney | same name different uuid | |
| 18:59:32 | sean-k-mooney | stephenfin: it shoudl return a 409 ^ | |
| 18:59:49 | sean-k-mooney | stephenfin: those test are 4 years old too | |
| 19:00:06 | sean-k-mooney | so something else is going on | |
| 19:00:45 | sean-k-mooney | https://bugs.launchpad.net/nova/+bug/1693349 | |
| 19:00:49 | openstack | Launchpad bug 1693349 in OpenStack Compute (nova) "trying to create a resource provider with an already in use uuid causes misleading error" [Low,Fix released] - Assigned to Chris Dent (cdent) | |
| 19:02:17 | sean-k-mooney | stephenfin: fixed by https://review.opendev.org/c/openstack/nova/+/468928/ | |
| 19:02:31 | sean-k-mooney | stephenfin: where did you get that failure form ? | |
| 19:03:22 | sean-k-mooney | it was fixxed in nova in queens before placment was split out | |
| 20:41:05 | openstackgerrit | Erik Olof Gunnar Andersson proposed openstack/nova stable/rocky: Use subqueryload() instead of joinedload() for (system_)metadata https://review.opendev.org/c/openstack/nova/+/761813 | |
| 21:26:47 | melwitt | lyarwood: can't remember if I've linked you this gate bug I opened semi recently so if I didn't, fyi https://bugs.launchpad.net/openstack-gate/+bug/1919920 | |
| 21:26:49 | openstack | Launchpad bug 1919920 in OpenStack-Gate "volume related tests sometimes fail with "Could not open '/dev/sdb': Permission denied" on stable/(ussuri|train|stein)" [Undecided,New] | |
| 21:27:10 | melwitt | seeing it on stable | |