| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-01-17 | |||
| 14:31:22 | jroll | efried: when that happens you're supposed to call it a snow day and drink hot cocoa and watch movies :P | |
| 14:31:43 | bauzas | efried: if that's ski-able, that's not a problem | |
| 14:31:48 | openstackgerrit | Merged openstack/os-vif master: adds iptools driver for ip commands https://review.openstack.org/534307 | |
| 14:31:50 | cdent | jroll++ | |
| 14:33:38 | efried | bauzas I wish. Even if there was snow (there isn’t) there’s nothing approaching a skiable slope. | |
| 14:34:04 | efried | No, it’s only 15F, but we are NOT set up to handle that. | |
| 14:34:07 | jroll | excuses. https://www.youtube.com/watch?v=waPRZu9iBJw | |
| 14:36:28 | edleafe | Not set to handle cold: http://www.mysanantonio.com/news/local/article/TxDOT-All-San-Antonio-area-roadways-open-no-12500663.php | |
| 14:37:12 | ameeda | jaypipes: please check this out "http://paste.openstack.org/show/646514/" | |
| 14:38:50 | ameeda | jaypipes: I think that image properties don't appears on nova side, on cli I have to execute "glance image-show <image_uuid> to see the new key | |
| 14:41:25 | efried | jaypipes Mainly looking for a comeback on my... uhh, dissertation on the ProviderTree.snapshot patch today https://review.openstack.org/#/c/533244/ | |
| 14:42:20 | cdent | my weather adventure is that sometime in the night the various official bags and bins that are used for setting out the recycling were gone this morning. Just plain gone. As winds were gusting into the 60s and 70s overnight it is entirely possible they are in france now. | |
| 14:42:27 | efried | That’s a tall stack of pancakes; want to get it moving | |
| 14:47:38 | jaypipes | ameeda: and what I'm telling you is that that code path (doing self.image_api.get()) is *not* going to hit the code path you are modifying in the patch (around the utils.py module). | |
| 14:48:42 | jaypipes | ameeda: so you may need to just either rely on unit tests or convert your functional tests to validate the patch's behaviour changes by manually checking the length of the instance_metadata DB record's values. | |
| 14:48:59 | jaypipes | efried: ya, getting there... | |
| 14:49:07 | efried | Thx | |
| 14:50:39 | edleafe | As long as we're begging for reviews, it would be nice to get the last alternate hosts patch merged: https://review.openstack.org/#/c/526436/ | |
| 14:50:47 | ameeda | jaypipes: so If I remove my functional test. you would accept that and merge ? | |
| 14:51:04 | jaypipes | ameeda: not without a change to unit tests. | |
| 14:51:10 | edleafe | Matt was good with it before he left for holiday - just wanted some more eyes on it | |
| 14:51:30 | ameeda | jaypipes: what kind of change you want on unit tests ? | |
| 14:51:55 | jaypipes | ameeda: at least something that demonstrates the code changes in your patch work as expected. | |
| 14:52:39 | ameeda | jaypipes: so If I want to checking the length of the instance_metadata DB record's values, what do you think | |
| 14:54:01 | ameeda | jaypipes: what about this "images = self._image_api.get_all(self.admin_api)" , it will get the info from instance_metadata DB | |
| 14:54:11 | jaypipes | no. | |
| 14:54:19 | jaypipes | the image api doesn't touch the nova db. | |
| 14:54:44 | ameeda | jaypipes: how you can help me to check record manually ? | |
| 14:54:48 | jaypipes | ameeda: https://review.openstack.org/#/c/531512/6/nova/tests/functional/db/test_allocation_candidates.py | |
| 14:54:58 | jaypipes | ameeda: check out the utility method on line 1453 there | |
| 14:55:13 | jaypipes | ameeda: that shows an example of how to query the nova DB from a functional test manually. | |
| 14:55:35 | jaypipes | ameeda: you would need to do something like that. only instead of hitting the API DB, you'd want to hit the nova cell DB | |
| 14:55:47 | jaypipes | ameeda: so something like this: | |
| 14:55:57 | jaypipes | with self.db.get_engine().connect() as conn: | |
| 14:56:39 | jaypipes | return conn.execute("SELECT key, value FROM instance_metadata WHERE instance_uuid = '%s'" % instance.uuid) | |
| 14:56:54 | jaypipes | ameeda: I will leave it up to you to figure it out from here. | |
| 14:57:28 | ameeda | jaypipes: thank you very much, I appropriate that from you, Thank you for your time | |
| 14:57:46 | jaypipes | np | |
| 14:58:16 | ameeda | jaypipes: this case must be in unit test, so I should remove functional test ? | |
| 14:58:33 | jaypipes | ameeda: no, you can keep the functional test. | |
| 14:59:04 | jaypipes | ameeda: and instead of asserting the return value from self.images_api.get(), you would instead assert the length of the DB record | |
| 15:00:00 | ameeda | jaypipes: well, so I will add the code to check the record in functional test ? | |
| 15:00:29 | jaypipes | ameeda: yes | |
| 15:00:38 | ameeda | great ! | |
| 15:01:05 | ameeda | jaypipes: again thank you very much, I was feeling bad before, I have to leave now, I will complete the work tomorrow. | |
| 15:01:24 | jaypipes | ameeda: np | |
| 15:06:11 | sean-k-mooney | is mriedem around this week? | |
| 15:11:13 | edleafe | sean-k-mooney: on holiday without computer | |
| 15:12:26 | bauzas | sean-k-mooney: needing a os-vif release ? | |
| 15:12:34 | bauzas | I can do that | |
| 15:13:02 | bauzas | in theory, I have powers for that, heh | |
| 15:13:14 | sean-k-mooney | bauzas: ya https://review.openstack.org/#/c/534807/ | |
| 15:13:55 | bauzas | sean-k-mooney: I guess you only need a +1 ? | |
| 15:14:04 | sean-k-mooney | bauzas: oh ya your in the nova-stable branch group | |
| 15:14:05 | bauzas | because neither matt nor I have +2s | |
| 15:14:30 | bauzas | on the releases repo | |
| 15:14:30 | sean-k-mooney | bauzas: ya i need a +1 form a nova core and then infra need to +2 it | |
| 15:14:44 | bauzas | sean-k-mooney: ok, briefly looking | |
| 15:14:56 | bauzas | in theory, it's a +1 from the PTL or the Release Management liaison | |
| 15:15:03 | sean-k-mooney | bauzas: technically yes | |
| 15:15:08 | bauzas | lucky for you, I'm the nove liaison :) | |
| 15:15:15 | bauzas | nova* (of course) | |
| 15:16:23 | sean-k-mooney | edleafe: ah that explains why i have not seen him this week. he mentioned he would be away but was not sure if it was all week | |
| 15:18:41 | bauzas | sean-k-mooney: +1d FWIW, I leave you chase the release folks | |
| 15:20:01 | sean-k-mooney | bauzas: no worries. thanks | |
| 15:22:06 | openstackgerrit | Tetsuro Nakamura proposed openstack/nova master: Fix comments in get_all_with_shared() https://review.openstack.org/533195 | |
| 15:23:41 | gibi | ildikov: I have only question in the multiattach api patch, https://review.openstack.org/#/c/271047/55/nova/compute/api.py@3700 | |
| 15:24:40 | openstackgerrit | Tetsuro Nakamura proposed openstack/nova master: Fix allocation_candidates not to ignore shared RPs https://review.openstack.org/533396 | |
| 15:26:43 | stephenfin | sean-k-mooney: You might do a follow up patch to 'openstack/requirements' to blacklist os-vif 1.8.0 too | |
| 15:27:29 | ildikov | gibi: hmm, I'm not sure I fully understand the question | |
| 15:27:51 | sean-k-mooney | stephenfin: oh good point i guess we should wait for the auto generated one for 1.8.1 to be submitted then i can modify it to blacklist 1.8.0 | |
| 15:27:53 | openstackgerrit | Lee Yarwood proposed openstack/nova master: WIP libvirt: QEMU native LUKS decryption for volumes https://review.openstack.org/523958 | |
| 15:29:28 | gibi | ildikov: so in the _check_attach_and_reserve_volume we differentiate between two cases | |
| 15:29:38 | sean-k-mooney | stephenfin: i think it will just updat this one https://review.openstack.org/#/c/532987/ but ill wait for it to be submitted. | |
| 15:30:08 | gibi | ildikov: and if this is the case when the user attaches a new volume to a running instance then we don't check the multiattach capabilities | |
| 15:30:11 | sean-k-mooney | stephenfin: by the way can you take a look at https://review.openstack.org/#/c/533766/ this is the nova unit test change that is required for os-vif 1.8.x | |
| 15:30:44 | gibi | ildikov: at least based on the code comment I commented on | |
| 15:31:06 | stephenfin | sean-k-mooney: Stuck in meetings til EOD (Intel flashbacks, heh) but I'll try squeeze it in. If not, I'll tackle it tomorrow (y) | |
| 15:31:07 | ildikov | gibi: we always check this first: if volume['multiattach'] and not supports_multiattach: | |
| 15:31:38 | gibi | ildikov: true, but the compute version is not checked in the if branch | |
| 15:31:39 | stephenfin | sean-k-mooney: Oh, wait. It's tiny. Done | |
| 15:31:41 | sean-k-mooney | stephenfin: cool no problem, its pretty trivial | |
| 15:31:55 | sean-k-mooney | ya just missing the datapath field. | |
| 15:32:07 | gibi | ildikov: it is only checked in the else branch | |
| 15:32:49 | ildikov | gibi: ah ok, I see now | |
| 15:33:46 | gibi | ildikov: it could be that we check the compute version is some other point in the call stack for the if case | |
| 15:36:32 | ildikov | gibi: we do fail in reserve_block_device_name here: https://review.openstack.org/#/c/531244/9/nova/compute/rpcapi.py | |
| 15:37:48 | tetsuro | bauzas: Hi, if you have time, could you have a look at https://review.openstack.org/#/c/534807/ ? | |
| 15:40:47 | ildikov | gibi: which might not be satisfying enough for that 'if' | |
| 15:41:33 | ildikov | gibi: but that's my guess for now :) | |
| 15:42:18 | bauzas | sean-k-mooney: saw https://review.openstack.org/#/c/534807/ ? | |
| 15:42:22 | bauzas | just respin | |
| 15:42:26 | gibi | ildikov: thanks I think I see now. compute.api._attach_volume() calls _create_volume_bdm that will call the rpc | |
| 15:42:40 | bauzas | tetsuro: you pointed me sean-k-mooney's patch, was that intentional ? | |
| 15:42:43 | gibi | ildikov: in case of attaching a volume to an existing instance | |
| 15:43:07 | ildikov | gibi: yep, that flow sounds correct | |
| 15:43:28 | gibi | ildikov: so it fails before _check_attach_and_reserve_volume is called in _attach_volume | |
| 15:43:29 | tetsuro | bauzas: sorry, just a mistake | |
| 15:43:44 | gibi | ildikov: this is why we don't need an extra check in _check_attach_and_reserve_volume for the attach volume case | |
| 15:44:21 | tetsuro | bauzas: could you have a look at https://review.openstack.org/#/c/531347/ ? | |