Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-03-21
21:35:45 mriedem so you'd get '{}'
21:35:51 mnaser yep, and that would fix it
21:36:13 mriedem right, hence me saying "anyway, it's probably not worth trying to figure out where in that pile of spaghetting the bdm.save is getting called"
21:36:20 mriedem "we shouldn't save 'null' in the connection_info"
21:36:25 sean-k-mooney {} is an empty json object which is technically not the same thing but it will fix this case
21:36:37 mnaser mriedem: ah because my original 'fix' didn't actually fix it -- https://review.openstack.org/#/c/645352/1/nova/virt/block_device.py
21:36:44 mnaser so i was referring to that, but, i will fix that now
21:36:48 mnaser and will try to have some unit test for _transform
21:37:04 mnaser to make sure it doesn't output connection_info = None
21:37:17 mriedem i would think this could be easily reproducible if we just hit overquota when booting from volume and then try to delete the server
21:38:02 mnaser probably, but not sure if i can write that level of functional tests from my skillset so far
21:38:20 mnaser unless you wanna a similar functional test that i can read and implement similar to
21:38:22 mriedem i could write one
21:38:44 mnaser sure, let me push up this with a unit test too
21:41:40 openstackgerrit Matt Riedemann proposed openstack/nova master: Pre-filter hosts based on multiattach volume support https://review.openstack.org/645316
21:44:16 mriedem efried: i went through and marked the deferred train blueprints that dont have specs re-approved as pending approval https://blueprints.launchpad.net/nova/train
21:44:22 mriedem i forgot to do that when deferring from stein
21:44:31 mriedem so we re-approve in LP once the spec is re-approved
21:44:59 efried mriedem: ack, thank you. I can see you saw I approved the quota one.
21:45:14 mriedem yeah, i thought about it because you said on the review:
21:45:15 mriedem "Blueprint in approved state [3] ✔"
21:45:26 mriedem technically the blueprint shouldn't have been approved before the spec
21:45:31 mriedem but i forgot to change those when deferring
21:47:40 openstackgerrit Ghanshyam Mann proposed openstack/nova master: Trivial: remove unused var from policies.base.py https://review.openstack.org/645380
21:49:29 mriedem dansmith: i've got this pretty much done, without using a pre-request filter https://review.openstack.org/#/c/645316/ but not sure if i need a blueprint or not, since i'm not adding a configurable filter, it's just going to always be required if you're booting from volume
21:50:01 mriedem cuz it's kind of dumb to even try to boot with a multiattach volume if the scheduler isn't going to filter out hosts that won't support them
21:51:03 efried does that mean they would break further on down the line?
21:51:13 efried I mean, would initial spawn succeed or fail?
21:51:23 mriedem it would fail,
21:51:25 mriedem it's arguably a bug fix
21:51:49 mriedem the bare minimum we do today is check if the computes are new enough, but that was minimum of queens
21:52:01 mriedem that doesn't say bfv wo'nt work for vmware for exapmle
21:52:17 mriedem if you're using vmware exclusively, you'd just have to never enable multiattach volumes on the cinder side via policy
21:52:30 mriedem that's a pain if you're doing multi-hypervisor though
21:52:55 mriedem you'd have to setup aggregates and filter using some flavor extra spec or something
21:55:38 mriedem anywho, idk, it's janky, i suppose you can't reasonably do bfv with vmware and libvirt mixed anyway because you have different volume types, and the libvirt driver isn't going to support vmware volumes and vice versa, so i'm not sure what if anything people do for mixed hypervisor - definitely at least aggregates/AZs
21:56:39 sean-k-mooney efried: ... this change https://github.com/openstack/nova/commit/6b844af57eebda6317b1ed3bf5a2f85dcba0bc13 does not work after i fix the lower constrating so im going to have to change it
21:57:39 efried mriedem: I say no bp, bug good, reno optional
21:58:33 efried sean-k-mooney: I'm on board with that. Long as you don't have to go changing the prod code, I say do what you gotta do to get this fixed.
21:59:38 sean-k-mooney im actully not sure why this is even working upstream honestly
22:00:34 sean-k-mooney i think it is depending on something having previously loaded messaging._drivers.impl_fake
22:00:50 sean-k-mooney *imported
22:02:35 sean-k-mooney ya it is
22:02:38 sean-k-mooney >>> import oslo_messaging as messaging
22:02:40 sean-k-mooney >>> messaging._drivers.impl_fake.FakeExchangeManager._exchanges
22:02:42 sean-k-mooney Traceback (most recent call last):
22:02:47 sean-k-mooney AttributeError: 'module' object has no attribute 'impl_fake'
22:02:47 sean-k-mooney File "<stdin>", line 1, in <module>
22:02:48 sean-k-mooney >>> import oslo_messaging._drivers.impl_fake
22:02:50 sean-k-mooney >>> messaging._drivers.impl_fake.FakeExchangeManager._exchanges
22:02:52 sean-k-mooney {}
22:03:40 sean-k-mooney so there is a race based on the execution order of the tests
22:16:14 openstackgerrit Sundar Nadathur proposed openstack/nova master: ksa auth conf and client for cyborg access https://review.openstack.org/631242
22:16:15 openstackgerrit Sundar Nadathur proposed openstack/nova master: WIP: Create and bind Cyborg ARQs. https://review.openstack.org/631244
22:16:15 openstackgerrit Sundar Nadathur proposed openstack/nova master: WIP: Add Cyborg device profile groups to request spec. https://review.openstack.org/631243
22:16:16 openstackgerrit Sundar Nadathur proposed openstack/nova master: WIP: Get resolved Cyborg ARQs and add PCI BDFs to VM's domain XML. https://review.openstack.org/631245
22:26:57 dansmith mriedem: why is that not in request_filter.py, just without a conf flag?
22:27:11 dansmith that way all these "check a thing about the request and add a trait requirement" things are in one place
22:27:17 sean-k-mooney efried: so just as an fyi we have other dodgy test too that are wrong upstream :(
22:27:29 efried sean-k-mooney: I am not surprised at all.
22:27:44 efried sean-k-mooney: I wonder if we need to file a bug for this.
22:27:51 sean-k-mooney oh we do
22:28:00 sean-k-mooney but that is not what im concerned about
22:28:20 sean-k-mooney the test issue im finding shoudl be broken in other jobs
22:28:32 sean-k-mooney https://github.com/openstack/nova/blob/d2dc7549ce9a0c979fd319cf0332a861b9cd2000/nova/tests/unit/api/openstack/compute/test_serversV21.py#L5129
22:28:43 sean-k-mooney self.stub_out('uuid.uuid4', lambda: FAKE_UUID)
22:29:02 sean-k-mooney that stubs out uuid.uuid4 to return FAKE_UUID
22:29:09 sean-k-mooney FAKE_UUID is a string
22:29:31 sean-k-mooney oslo messaging does unique_id = uuid.uuid4().hex'
22:29:44 sean-k-mooney .hex is not defiend on a string
22:29:49 sean-k-mooney so it explodes
22:30:06 sean-k-mooney oslo.messaging still does unique_id = uuid.uuid4().hex on master
22:30:06 efried why isn't it exploding now?
22:31:02 sean-k-mooney i think its another race in the tests
22:32:23 sean-k-mooney hum running that test on its own on the py36 target passes but files in lower constratits
22:34:39 efried are you running stop-on-first-failure or do you have a sense of how many tests are broken?
22:34:47 sean-k-mooney ok so my theory is that thing have moved and we are not mocking the right things when usign the older version of oslo.messaging
22:34:56 openstackgerrit Mohammed Naser proposed openstack/nova master: bdm: store empty object as connection_info by default https://review.openstack.org/645352
22:35:49 sean-k-mooney i was running all the test but while i was getting many of the test to pass the lower-constratits job was hannging after running for a bit
22:36:54 mnaser mriedem: ^ this worked locally running the unit tests
22:37:49 openstackgerrit Mohammed Naser proposed openstack/nova master: bdm: store empty object as connection_info by default https://review.openstack.org/645352
22:38:17 cfriesen mriedem: I assume the vTPM blueprint status change is because it didn't make it in for Stein?
22:39:09 sean-k-mooney cfriesen: mriedem mentioned he set the state back to pending approval for any blupritn that was approved last cycle but needs to have the spec reapproved
22:39:26 cfriesen thx, figured as much
22:40:31 mriedem cfriesen: right, re-propose the spec for train
22:40:38 mriedem i forgot to switch the bp status when deferring
22:40:55 mriedem mnaser: want to report a bug for this?
22:45:17 mnaser mriedem: oh dang i had one but i didnt add closes-bug
22:45:35 openstackgerrit Mohammed Naser proposed openstack/nova master: bdm: store empty object as connection_info by default https://review.openstack.org/645352
22:46:02 openstackgerrit sean mooney proposed openstack/nova master: make lower-constratints env use lower-constratins https://review.openstack.org/645392
22:46:19 mnaser ok just noticed your reviews too ill try to address them after foodz
22:46:26 sean-k-mooney im going to keep working on ^
22:46:55 efried sean-k-mooney: Thanks for doing that.
22:47:38 mriedem mnaser: ok i left some comments in PS3, not really worth -1ing over though
22:48:28 sean-k-mooney efried: i can fix the FAKE_UUID issue but i still suspect that our mocking is wrong in some of the tests and code that should not be run actully is being run
22:48:40 mriedem dansmith: it's not a request filter because the request spec doesn't store the bdms, and during server create the bdms won't have multiattach information in them anyway, so to figure it out, i'd have to, within the filter, pull the list of bdms for the instance and for all with a volume_id, query cinder to figure out if the volume is multiattach or not
22:48:55 sean-k-mooney if the lower constraitn job explodes or times out then that is likely why
22:49:26 mriedem if we actually had a BDM.multiattach field, that could maybe work - but we'd still have to get the bdms during scheduling...which might suck
22:56:13 dansmith mriedem_afk: okay, seemed like it was just examining the reqspec to determine what to do, which is exactly like the other filters, but I admittedly was skimming
22:56:18 dansmith can look again tomorrow

Earlier   Later