| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-06-11 | |||
| 17:29:23 | mriedem | https://github.com/openstack/cinder/blob/master/cinder/volume/manager.py#L4376 | |
| 17:29:43 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: Add InstanceNUMATopology to LibvirtLiveMigrateData https://review.openstack.org/566398 | |
| 17:29:44 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: WIP: LM: re-fit topology and claim resources on dest https://review.openstack.org/567242 | |
| 17:29:45 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: Service version check for NUMA live migration https://review.openstack.org/566723 | |
| 17:30:10 | mriedem | mdbooth: i planned on cleaning up the _translate_attachment_ref stuff in a follow up | |
| 17:30:15 | mriedem | since it's not necessary for the backport | |
| 17:31:34 | mdbooth | mriedem: I was thinking of the issue of whether ro attachments are really ro. | |
| 17:31:45 | mdbooth | Which they might be, but it's not at all obvious. | |
| 17:32:10 | mdbooth | Bizarrely, that code is adding attach_mode to connection_info, but I couldn't find any use of it. | |
| 17:32:10 | mriedem | if they aren't, that's a bug in cinder yes? | |
| 17:32:23 | mriedem | right, attach_mode doesn't need to be in the connection_info | |
| 17:32:27 | mdbooth | But as you pointed out, we *are* using access_mode in nova/os-brick. | |
| 17:32:29 | mriedem | since access_mode is there and that's what the volume driver uses | |
| 17:32:33 | mriedem | yes | |
| 17:32:47 | mriedem | in a follow up, i'll move attach_mode back top-level on the attachment record | |
| 17:33:07 | mdbooth | So, what's the relationship between access_mode and attach_mode, and why do we both putting the latter in connection_info if we don't seem to us it? | |
| 17:33:08 | mriedem | where it belongs given https://developer.openstack.org/api-ref/block-storage/v3/#show-attachment-details | |
| 17:33:29 | mriedem | that is an excellent question for our friends in the -cinder channel | |
| 17:33:44 | mdbooth | And if access_mode and attach_mode don't correlate 100%, is our assumption that attach_mode==ro means a readonly attachment a safe one? | |
| 17:33:45 | mriedem | i assume in part to eventually decouple from needing to stash stuff in connection_info | |
| 17:33:51 | mriedem | that is not host/backend specific | |
| 17:34:08 | mriedem | mdbooth: again, that would be a bug in cinder if they aren't | |
| 17:34:29 | artom | We could just be ruthless about it and block swap if attachments (of any kind) are > 1 | |
| 17:34:32 | mdbooth | mriedem: If that's true, that's good enough for me. | |
| 17:34:49 | mdbooth | mriedem: It was just weird enough for me to flag it. | |
| 17:34:55 | mriedem | i agree it's weird | |
| 17:35:22 | mriedem | there are some things missing from the attachments API, like the ability to actually create/update an attachment and set attach_mode on the record directly | |
| 17:35:30 | mriedem | rather than indirectly via connector['mode'] | |
| 17:36:05 | mriedem | AND | |
| 17:36:28 | mriedem | the volume admin metadata readonly stuff needs to be decoupled from the actual attachments, so attach mode is per-attachment | |
| 17:36:30 | mriedem | not per-volume | |
| 17:36:52 | mriedem | but i don't think jgriffith et al got that far when plumbing this into cinder, but the TODOs and NOTEs are in the code | |
| 17:37:04 | mriedem | e.g. https://github.com/openstack/cinder/blob/master/cinder/volume/manager.py#L4384 | |
| 17:37:29 | mdbooth | mriedem: Actually I did get far enough to read some of that | |
| 17:37:46 | mdbooth | It looks like attached_mode in admin_metadata is basically a temporary stash | |
| 17:38:07 | mdbooth | So it's admin_metadata on the volume, but it only lives during the creation of a single attachment. | |
| 17:38:08 | openstackgerrit | Matt Riedemann proposed openstack/nova master: placement: always create consumer records https://review.openstack.org/567678 | |
| 17:38:09 | openstackgerrit | Matt Riedemann proposed openstack/nova master: add consumers generation field https://review.openstack.org/557958 | |
| 17:38:10 | openstackgerrit | Matt Riedemann proposed openstack/nova master: placement: Allocation.consumer field https://review.openstack.org/565405 | |
| 17:38:11 | openstackgerrit | Matt Riedemann proposed openstack/nova master: rework allocation handler _allocations_dict() https://review.openstack.org/565407 | |
| 17:38:12 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add a microversion for consumer generation support https://review.openstack.org/565604 | |
| 17:38:13 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Fix nits from change Id609789ef6b4a4c745550cde80dd49cabe03869a https://review.openstack.org/574324 | |
| 17:38:23 | mriedem | efried: dansmith: ok rebased ^ | |
| 17:38:40 | mriedem | my find/replace 1.27 -> 1.28 fu in the actual microversion change might have missed something | |
| 17:38:44 | mriedem | but tests are happy | |
| 18:10:10 | openstackgerrit | Dan Smith proposed openstack/nova stable/pike: Change consecutive build failure limit to a weigher https://review.openstack.org/573248 | |
| 18:12:18 | openstackgerrit | Curt Moore proposed openstack/nova master: Add ability to download Glance images into the libvirt image cache via RBD https://review.openstack.org/574301 | |
| 18:14:05 | openstackgerrit | Brianna Poulos proposed openstack/nova master: Implement certificate_utils https://review.openstack.org/479949 | |
| 18:14:06 | openstackgerrit | Brianna Poulos proposed openstack/nova master: Plumb trusted_certs through libvirt driver image paths https://review.openstack.org/561262 | |
| 18:14:07 | openstackgerrit | Brianna Poulos proposed openstack/nova master: Add trusted_image_certificates to REST API https://review.openstack.org/486204 | |
| 18:14:08 | openstackgerrit | Brianna Poulos proposed openstack/nova master: Add notification support for trusted_certs https://review.openstack.org/563269 | |
| 18:14:09 | openstackgerrit | Brianna Poulos proposed openstack/nova master: Add certificate validation docs https://review.openstack.org/560158 | |
| 18:17:49 | artom | In situations like this I really wish Python had static typing | |
| 18:18:28 | artom | ... or that OpenStack was written in Java ;) | |
| 18:18:29 | mriedem | melwitt: reminder, you can release r-2 now https://launchpad.net/nova/+milestone/rocky-2 | |
| 18:18:40 | mriedem | based on https://review.openstack.org/#/c/573495/ it looks like it was june 8 | |
| 18:30:28 | openstackgerrit | Matt Riedemann proposed openstack/nova master: xenapi: drop deprecated vif_driver config option https://review.openstack.org/567877 | |
| 18:35:31 | openstackgerrit | Curt Moore proposed openstack/nova master: Add ability to download Glance images into the libvirt image cache via RBD https://review.openstack.org/574301 | |
| 18:49:07 | openstackgerrit | Merged openstack/nova stable/queens: Fix interpretation of max_attempts for scheduling alternates https://review.openstack.org/573254 | |
| 18:55:21 | mriedem | dansmith: i have beat your heart down on that patch as requested | |
| 18:56:36 | dansmith | aye | |
| 18:57:28 | melwitt | mriedem: what does it mean to release r-2 wrt to launchpad? oh, I see a "create release link" | |
| 18:58:15 | mriedem | just paperwork | |
| 18:58:34 | mriedem | but see the storyboard thread in the ML about why people care | |
| 18:59:21 | melwitt | okay, I just didn't know to/how to do it. my bad | |
| 19:01:43 | mriedem | artom: re https://review.openstack.org/#/c/572790/ i don't think we need your tempest patch | |
| 19:01:47 | mriedem | i'm not sure why you created it | |
| 19:01:57 | mriedem | because did you see that ^ depends on https://review.openstack.org/#/c/573025/ ? | |
| 19:02:12 | artom | mriedem, we don't? I mean, to actually test the "positive" case where we refuse the swap | |
| 19:02:29 | mriedem | you mean negative? | |
| 19:03:07 | artom | mriedem, I genuinely have no idea :/ | |
| 19:03:17 | artom | mriedem, the case where we assertRaises(400) | |
| 19:03:18 | artom | That one. | |
| 19:03:18 | mriedem | testing error conditions in tempest are called negative tests | |
| 19:03:26 | artom | OK, negative then :) | |
| 19:05:17 | melwitt | mriedem: okay, done. thank you for letting me know about it https://launchpad.net/nova/+milestone/rocky-2 | |
| 19:05:45 | melwitt | I removed the rocky-2 milestone target from https://blueprints.launchpad.net/nova/+spec/preemptible-instances also | |
| 19:08:02 | mriedem | artom: ok here is your 400 http://logs.openstack.org/09/573809/2/check/nova-multiattach/ebd4e2a/job-output.txt.gz#_2018-06-09_14_49_19_637410 | |
| 19:08:18 | mriedem | PUT http://104.130.136.141/compute/v2.1/servers/57ba4f52-8076-45e2-b6ac-14cf789b6926/os-volume_attachments/8361a693-e7c6-4a9b-8303-c303d5fbdad5 | |
| 19:08:23 | mriedem | Body: {"volumeAttachment": {"volumeId": "d9636fa2-78a3-468a-b56a-9e6e2d6f14f6"}} | |
| 19:08:48 | artom | mriedem, yep, which is why I posted that patch | |
| 19:08:56 | artom | mriedem, you don't have to merge it if you don't have to :) | |
| 19:09:07 | artom | Although the cleanup failure is interesting | |
| 19:09:11 | mriedem | i'm not QA core so i can't merge it.. | |
| 19:09:28 | artom | Either I'm really thick and doing something wrong, or refusing to swap a volume legit makes it undetachable | |
| 19:10:32 | mriedem | where is the cleanup code that is waiting for the volume to be available after the swap failure, | |
| 19:10:41 | mriedem | the code is doing the right thing, it leaves the old volume attached (in-use) | |
| 19:11:04 | mriedem | the trace in the tempest failure doesn't make sense to me | |
| 19:11:14 | artom | Same here, I can't understand it | |
| 19:11:31 | artom | I tried printing the full stacktrace manually, but got just a bunch of test runner stuff | |
| 19:11:46 | artom | So I can't even pinpoint where it's from - I *think* it's the cleanup phase | |
| 19:11:59 | mriedem | i'm pretty sure it's the cleanups from attach_volume | |
| 19:12:09 | mriedem | # is already detached. | |
| 19:12:09 | mriedem | # Ignore 404s on detach in case the server is deleted or the volume | |
| 19:12:09 | mriedem | self.volumes_client, volume['id'], 'available') | |
| 19:12:09 | mriedem | self.addCleanup(waiters.wait_for_volume_resource_status, | |
| 19:12:09 | mriedem | # teardown. | |
| 19:12:09 | mriedem | # is so we don't error out when trying to delete the volume during | |
| 19:12:09 | mriedem | # On teardown detach the volume and wait for it to be available. This | |
| 19:12:10 | mriedem | self.addCleanup(self._detach_volume, server, volume) | |