Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-01-31
22:34:35 openstackgerrit Eric Fried proposed openstack/nova master: Avoid inventory DELETE API (no conflict detection) https://review.openstack.org/539712
22:34:48 efried jaypipes: How about them apples ^
22:35:12 efried Note approx -400LOC net.
22:35:13 openstackgerrit Matt Riedemann proposed openstack/nova master: Remove old flavor_access_add db api methods https://review.openstack.org/539714
22:35:30 melwitt Roamer`: thanks. we'll use this to investigate what's going wrong
22:35:38 openstackgerrit Eric Berglund proposed openstack/nova master: Use dict.get() when accessing capabilities dict https://review.openstack.org/539715
22:41:23 mriedem Roamer`: is the storpool volume driver passing a secret in it's connection_info dict when using an encrypted volume type
22:42:01 mriedem i'm not actually sure if that's required
22:42:27 mriedem should probably check lee's patches to see if he had to change anything in devstack for this stuff to work, like with the key manager across both nova and cinder
22:43:00 mriedem https://review.openstack.org/#/q/topic:bp/libvirt-qemu-native-luks+(status:open+OR+status:merged)
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? ^

Earlier   Later