Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-01-31
22:43:18 mriedem melwitt: you know,
22:43:22 mriedem now that i think about it,
22:43:39 mriedem the tempest test that tests encrypted volumes is marked 'slow' so i don't think it actually runs in a job in our check queue...
22:43:47 mriedem i think there is a job that runs it in the experimental queue
22:44:36 mriedem this one https://github.com/openstack-infra/project-config/blob/master/zuul.d/projects.yaml#L10841
22:46:07 mriedem passes here though http://logs.openstack.org/60/539260/2/experimental/legacy-tempest-dsvm-neutron-scenario-multinode-lvm-multibackend/77a82c8/job-output.txt.gz#_2018-01-31_20_59_15_161538
22:48:01 melwitt hm, okay
22:48:06 openstackgerrit Matt Riedemann proposed openstack/nova master: Remove old flavor_access_get_by_flavor_id db api method https://review.openstack.org/539720
22:53:10 Roamer` mriedem, the StorPool driver does not generate any structures by itself, at your suggestion it inherits LibvirtVolumeDriver's get_config() method
22:53:52 Roamer` and from what I can see of the libvirt LUKS patch, the secret should be stored by LibvirtBaseVolumeDriver's get_config()
22:54:38 mriedem u'data': {u'device_path': u'/dev/storpool/os--volume-645fd643-89fc-4b3d-9ea5-59c764fc39a2', u'encrypted': True, u'volume': u'645fd643-89fc-4b3d-9ea5-59c764fc39a2', u'qos_specs': None, u'client_id': 12, u'access_mode': u'rw'}
22:54:50 mriedem https://review.openstack.org/#/c/523958/18/nova/virt/libvirt/driver.py@1342
22:55:04 mriedem looks like the libvirt driver is expecting a connection_info['data']['volume_id'] entry
22:56:02 melwitt there must be one, else we'd see a KeyError, right? is it None?
22:56:29 Roamer` hm, ISTR that there was a "volume_id" one level up, on the same level as "data", but let me look again
22:56:37 mriedem https://review.openstack.org/#/c/523958/18/nova/virt/libvirt/volume/volume.py
22:57:08 mriedem melwitt: the libvirt driver code is using .get() to avoid a keyerror
22:57:47 melwitt I see it now
22:58:59 melwitt so is volume_id supposed to always be set in connect_info['data'] or is it valid not to have it?
22:59:17 mriedem shrug,
22:59:31 mriedem connection_info is a wild west of whatever from the bazillion volume drivers in cinder
22:59:48 mriedem i do know that in nova, when we attach, we shove 'serial' into the connection_info if it's not already there
22:59:51 mriedem and serial == volume_id
22:59:56 melwitt sigh
23:00:10 smcginnis I don't think the serial is always the volume_id.
23:00:25 smcginnis RemoteFS based storage I think that's the case, but not others.
23:00:28 mriedem {u'status': u'reserved', u'instance': u'9a7f0101-917d-43f2-ac18-0b7d77c14fda', u'attached_at': u'', u'attach_mode': None, u'driver_volume_type': u'storpool', u'detached_at': u'', u'volume_id': u'645fd643-89fc-4b3d-9ea5-59c764fc39a2', u'serial': u'645fd643-89fc-4b3d-9ea5-59c764fc39a2', u'data': {u'device_path': u'/dev/storpool/os--volume-645fd643-89fc-4b3d-9ea5-59c764fc39a2', u'encrypted': True, u'volume': u'645fd643-89fc-4b3d
23:00:29 mriedem 5-59c764fc39a2', u'qos_specs': None, u'client_id': 12, u'access_mode': u'rw'}}
23:00:49 mriedem smcginnis: it's the volume_id if it's not already in the dict from cinder
23:01:02 smcginnis Ah, OK.
23:01:03 mriedem https://github.com/openstack/nova/blob/master/nova/virt/block_device.py#L425
23:01:07 mriedem b/c we shove it in there
23:01:09 mriedem shove it real good
23:01:29 smcginnis Nah, just not the same as push it. Nice try.
23:01:39 melwitt oh, heh. smcginnis came here. I just asked the question in #openstack-cinder if it's valid not to have 'volume_id' in connection_info['data']
23:01:43 mriedem so all of this new encryptor code from lyarwood should probably be falling back to looking for the serial in connection_info if connection_info['data'] doesn't have 'volume_id'
23:02:14 smcginnis I do not think it's safe to assume that.
23:02:23 melwitt mdbooth ^
23:05:28 openstackgerrit Eric Berglund proposed openstack/nova master: Use dict.get() when accessing capabilities dict https://review.openstack.org/539715
23:07:53 mriedem i don't really know how this is failing
23:08:01 mriedem if there is no secret, it shouldn't be set in the conf
23:09:03 melwitt oh, you mean find_secret should return None or something
23:09:23 mriedem idk, i get lost following the damn xml conversion crap in the libvirt driver
23:09:31 melwitt er, sorry it wouldn't even call find_secret
23:11:28 mriedem my guess is we don't get here https://review.openstack.org/#/c/464008/10/nova/virt/libvirt/config.py@1149
23:13:08 melwitt yeah, and we wouldn't because there's no secret in the config
23:13:29 melwitt if c.tag == 'secret': would be False
23:14:32 melwitt so since there was no volume_id, there's no secret in the config, so there's no secret to be parsed into the config object, then it blows up when it goes to format the domain later
23:15:39 mriedem http://logs.ci-openstack.storpool.com/18/539318/1/check/dsvm-tempest-storpool/c3daf58/logs/screen-n-cpu.txt.gz#_Jan_31_18_07_21_994385
23:15:56 mriedem that's the secret xml
23:15:59 mriedem sshhhh
23:16:17 melwitt lol
23:17:07 melwitt so there IS secret xml? sigh, I'm confused
23:17:38 melwitt back to not knowing how this is failing
23:18:14 mriedem right i have no idea
23:18:24 mriedem basically need to wait for lyarwood to wake up
23:18:38 melwitt yeah
23:33:22 openstackgerrit Matt Riedemann proposed openstack/nova master: Remove old flavor_destroy db api method https://review.openstack.org/539729
23:37:12 lyarwood *yawn*
23:37:20 lyarwood I really need to remove email from my phone
23:37:27 lyarwood <volume>None</volume>
23:37:31 lyarwood that's the issue btw
23:37:40 melwitt well HI
23:39:51 lyarwood ah, so you already found that above :)
23:40:15 openstackgerrit Eric Fried proposed openstack/nova master: Test case: new standard resource class unusable https://review.openstack.org/539732
23:40:44 efried mriedem (in the correct channel) FYI there's a test case demonstrating the aforementioned bug that has nothing to do with infra ^
23:41:14 melwitt lyarwood: we thought so but then got confused about how there's a <secret> element in the XML anyway
23:41:56 melwitt we were thinking the non-existent connection_info['data']['volume_id'] would result in *no* 'secret' element in the XML, then blow up later when it tried to parse <secret> back
23:42:29 melwitt but we see <secret> XML in the log. so then we knew we had to wait until you could look at it
23:47:11 Roamer` sorry I disappeared like that, had to take care of something at home
23:47:48 Roamer` but now it's a bit late here, so I guess I'll be off for the night; feel free to let me know if there's any way I can help with this secret problem
23:49:06 openstackgerrit Matt Riedemann proposed openstack/nova master: Remove old flavor_get_by_flavor_id db api method https://review.openstack.org/539733
23:49:37 mriedem lyarwood: seems odd that it would add encryption elements at all if there was no volume_id found in the connection_info, that's what i don't get
23:50:33 lyarwood mriedem: right that's the inital bug here - https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L1326
23:50:51 efried mriedem: Okay, slightly weirder question. I assume it's possible to upgrade the placement service without upgrading the compute service (cause that's a way to get to the condition I asked about before). Does compute get restarted if that happens?
23:50:52 lyarwood mriedem: that needs to change to handle the case where volume_id isn't in connection_info as you've already discussed
23:53:58 mriedem lyarwood: ah yeah
23:53:59 mriedem ok
23:54:01 mriedem easy peasy
23:54:20 mriedem efried: i have'nt read the bug, but yes placement gets upgraded before nova services
23:54:34 mriedem and computes can be n-1 for rolling upgrades
23:54:48 efried ight
#openstack-nova - 2018-02-01
00:00:03 Roamer` mriedem, lyarwood, if you guys think that it might be a good idea to modify our Cinder driver to put the volume_id in the connection info, we could do that too... but people in -cinder seemed to think that it's not necessarily expected
00:00:53 mriedem Roamer`: no it's a nova problem
00:00:58 mriedem i'll see if i can post something quick
00:02:17 lyarwood thanks mriedem, just trying to understand why we then fail attempting to format the encryption dom that shouldn't even be there in this case tbh
00:03:56 Roamer` mriedem, thanks. Unfortunately I'm really tired right now, but if you do manage to come up with something, I'll test it seven or eight hours from now
00:07:09 openstackgerrit Eric Fried proposed openstack/nova master: WIP: Ensure resource classes correctly https://review.openstack.org/539738
00:07:34 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Look all over hell for that damn volume_id https://review.openstack.org/539739
00:07:35 mriedem Roamer`: do we need a cinder patch to test that out with your CI? ^
00:16:57 efried mriedem: I'm out for the evening. In case I forget to mention it tomorrow, I need to leave the nova meeting like ten minutes in (I'm on the hook for school runs).
00:17:37 mriedem efried: ok, it's tomorrow afternoon though
00:17:43 efried yuh
00:47:41 Spazmotic Morning
00:50:11 melwitt mriedem: is this correct in the cinder fixtures? or should it be volume['attach_status'] being set? https://github.com/openstack/nova/blob/master/nova/tests/fixtures.py#L1394
00:58:54 melwitt I think it's supposed to be 'attach_status' if the values are 'attached' or 'detached'
01:00:45 mriedem melwitt: you're right i think, status would be 'in-use' if it's attached
01:00:53 mriedem and 'available' if it's not attached
01:01:10 mriedem https://developer.openstack.org/api-ref/block-storage/v3/#volumes-volumes
01:01:18 melwitt k, thanks for the sanity check. I can propose a fix for that. I'm meddling with my CellDatabases fixture improvements again

Earlier   Later