Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-01-08
13:57:29 openstackgerrit Stephen Finucane proposed openstack/nova master: nova-net: Remove remaining nova-network quotas https://review.opendev.org/697152
13:57:30 openstackgerrit Stephen Finucane proposed openstack/nova master: nova-net: Remove use of legacy 'SecurityGroup' object https://review.opendev.org/697155
13:57:30 openstackgerrit Stephen Finucane proposed openstack/nova master: nova-net: Remove use of legacy 'Network' object https://review.opendev.org/697154
13:57:31 openstackgerrit Stephen Finucane proposed openstack/nova master: nova-net: Remove unused nova-network objects https://review.opendev.org/697156
14:32:54 shilpasd mriedem: thanks for review
14:44:29 stephenfin gibi: when you've a chance, could you hit https://review.opendev.org/#/c/700511/ again? Just addressed two of efried's nits
14:47:12 gibi stephenfin: looking...
14:48:42 gibi stephenfin: done
14:49:04 gibi stephenfin: I will try to look into the next set of patches in the series during today or tomorrow
14:57:46 bauzas gibi: can I ask you a question ?
14:58:01 bauzas gibi: I'm looking at your comments on https://review.opendev.org/#/c/670112/11/nova/tests/functional/test_nova_manage.py
14:58:27 bauzas gibi: I honestly prefer to leave castascall rather than having more than one method call per contextmanager
14:58:28 gibi bauzas: sure
14:58:55 gibi bauzas: I mean sure, lets look at my comment
14:59:06 bauzas just because I think it's more understanding why we need to mock
14:59:45 bauzas gibi: okay, so you would still be OK if I don't modify it ?
15:00:38 gibi bauzas: unfortunately having only the post under the mock is a lie as such post starts an async task and the post returns before the function you are mocking really runs
15:01:06 gibi in a unit test I'm ok with castascall but not in a functional test
15:01:09 gibi sorry
15:01:52 efried stephenfin: dahh, I guess we do have access to the old image meta, don't we? So yeah, sean-k-mooney is right, I can add a check for that.Do you want the spec updated now or fup'd?
15:01:54 gibi in a functional test we want to express (and test) the async behavior of our API we are calling
15:02:15 sean-k-mooney efried: i think we can just do it in the implemnation
15:02:28 sean-k-mooney i dont think we need a spec update but a fup would be fine if you want
15:02:29 bauzas gibi: mmmm not sure I understand you, sorry :(
15:02:38 stephenfin efried: If you can get it today, I'm happy to fast approve and carry gibi's +2
15:03:00 efried really in that spot I was just trying to give examples of how things could go wrong. I got one of the wrong things wrong, but it's the *thought* that counts :P
15:03:06 efried stephenfin: Roger that, will do it right now...
15:03:07 bauzas gibi: ah-ha I think I understand your concern
15:03:40 gibi bauzas: in my eyes the nova internal in functional test shoudl behave exactly as it behaves in real life (except virt driver as it is hard, and except external dependencies like neutron) so I would like to keep the nova API async behaviour in func test
15:03:58 bauzas gibi: honestly given we call directly by castascall, it's no longer async
15:04:17 bauzas (and that's also why it works
15:04:28 gibi bauzas: yes, and I'm against such behavior change in func test
15:04:49 bauzas gibi: okay, fair enough :-)
15:05:33 bauzas gibi: no worries, I'll then fix this
15:05:50 gibi bauzas: we tend to allow to use some mocks in func test to inject faults as that is hard without the API but other than that I'm also against mocks in func test too
15:06:11 gibi s/without the API/ with only the API/
15:06:19 gibi bauzas: thanks
15:12:19 stephenfin efried: Sweet. Ta. When you're done, I have done as commanded on https://review.opendev.org/700511 and it's just _itching_ to get through that gate
15:15:54 efried stephenfin: firewall pt. 3 +A
15:16:58 efried stephenfin: regarding the ImageMetaProps updates... I'm not sure.
15:18:20 efried I always thought of image meta props the same as flavor extra specs -- a bag of random key/values that we can "enhance" without microversion changes -- except for some reason (that I never understood, never asked about) we actually have OVOs for image meta props, which need to be updated.
15:19:31 efried but like extra specs, I would have expected them to show up in a REST payload like the others
15:20:07 efried and be settable like anything else
15:21:49 openstackgerrit Eric Fried proposed openstack/nova-specs master: Spec: Ussuri: Encrypted Emulated Virtual TPM https://review.opendev.org/686804
15:21:57 sean-k-mooney efried: yes the can be extended without microverion changes
15:22:07 efried stephenfin, gibi, sean-k-mooney: with just that update about the API check --^
15:22:12 sean-k-mooney but the image metadata object is an ovo
15:22:22 sean-k-mooney so it will alter the object version
15:22:52 efried right; stephenfin is asking whether the new fields will be visible/settable via the API, and I don't see why not?
15:23:10 sean-k-mooney we do not allow arbiary keys to be set in the image metadata
15:23:16 sean-k-mooney but it should be visable yes
15:23:24 sean-k-mooney we dont allow you to downgrade the ovo
15:23:39 sean-k-mooney to strip out new keys and get an older version
15:24:11 efried by "arbitrary" you mean keys that we haven't explicitly registered, which is why we register them. But anything we've registered, you can set
15:24:28 sean-k-mooney yes
15:24:38 sean-k-mooney although this is set in glance not nova obviously
15:28:10 efried right, and then we pull in a copy of the meta and store it with the instance record, which is why this object exists in nova in the first place.
15:28:33 efried not sure why we needed it for image meta but not flavor extra specs tho
15:28:43 sean-k-mooney yes more or less
15:28:53 efried or rather, why we couldn't leave it out for image meta like we left it out for extra specs.
15:28:58 sean-k-mooney we did not have it for the image metadata orgithily
15:29:04 sean-k-mooney it was just a dict of strings
15:29:11 dansmith efried: you can use flavor extra specs for other things though right?
15:29:16 dansmith and schedule on them with things like the json filter?
15:29:35 sean-k-mooney but we stadarised it wehn we made the nova image an objec t in juno i think
15:29:54 sean-k-mooney dansmith: you used to be able to do the same with the image metadata
15:29:57 dansmith I think the problem with image props came because people were using them as a covert conduit between the user (which can set them) and the virt driver and bypassing nova
15:29:59 efried I'm not sure I understand a distinction dansmith. We've been using image meta props for scheduling decisions as long as I've been involved
15:30:18 efried okay, right, I was going to ask whether it had something to do with admin/flavor vs user/image.
15:30:39 dansmith efried: also, the problem is that image properties live a long time and we had a huge problem with virt drivers using different props for the same thing.. flavor extra specs are admin-only and we can force people to fix them during an upgrade
15:31:39 efried Okay. If there's a reason, so be it. I don't super care to challenge the status quo, since it's easy to add properties. Really just curious why they were different.
15:32:11 efried but back to stephenfin's question, which seems odd to me as worded
15:32:22 sean-k-mooney well the topic of makeing the extra specs versioned like the image meta has come up before
15:32:40 dansmith I think the "images live forever and are controlled by users" is the reason
15:32:42 sean-k-mooney but then we dont have a place to do the current non standdard thngs
15:33:10 efried Right, we've always punted on it because of the nonstandard things, things we don't know about etc.
15:33:22 efried which is one of the challenges stephenfin is facing with the validation effort.
15:33:54 dansmith yeah, it's also why I have a hard time getting excited about that
15:34:13 sean-k-mooney ya so we could always add a seperate sandardsed metadata filed that was versioned and leve extra specs for the unversioned stuff but its never really been urgent enough
15:34:17 dansmith I know we need to do it,
15:34:18 dansmith but it's always been a big bag of stuff
16:04:11 efried gibi: Could I ask you to review this please https://review.opendev.org/#/c/699050/ ? It bumps us to using placement 1.35, sets a request_level_params next to requested_resources in the RequestSpec, and lets us unwind the request filter hacks where we were transiently modifying the flavor extra specs.
16:04:40 efried ...and Dan is +2.
16:11:21 sean-k-mooney bauzas: https://review.opendev.org/#/c/700529/
16:11:55 bauzas sean-k-mooney: that's a different bug but okay
16:12:48 sean-k-mooney yes its a similar cause the _copy_resources function does create a copy and instead modifyes the stats
16:12:49 openstackgerrit Victor Morales proposed openstack/nova master: Update Testing NUMA documentation https://review.opendev.org/701301
16:43:14 stephenfin I'm seeing a lot of failures in the nova-next job, all due to 'Multiple possible networks found, use a Network ID to be more specific.'
16:43:33 stephenfin I feel we have a bug open against this. Anyone know of one?
16:44:03 artom stephenfin, it does ring a bell...
16:50:57 efried stephenfin: Yeah, this cropped up again recently and was fixed again. I could dig, but probably asking in -neutron would be the fastest way to get to the bottom.
16:53:53 stephenfin efried: Yeah, I'll do that
16:56:03 openstackgerrit Merged openstack/nova master: Update Testing NUMA documentation https://review.opendev.org/701301
17:03:27 openstackgerrit Sylvain Bauza proposed openstack/nova master: Avoid PlacementFixture silently swallowing kwargs https://review.opendev.org/695180
17:03:27 openstackgerrit Sylvain Bauza proposed openstack/nova master: Add a placement audit command https://review.opendev.org/670112
17:21:57 gibi efried: added https://review.opendev.org/#/c/699050/ to my queue
17:22:09 gibi efried: I will look at it tomorrow morning
17:22:28 efried thanks gibi!
17:28:59 mriedem efried: since i reviewed that before, i've +2ed the latest since it lgtm, but will let someone else approve
17:29:28 efried mriedem: okay, cool, thank you.

Earlier   Later