| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-10-13 | |||
| 14:29:23 | gibi | I guess I need to create a functional test for this case as well | |
| 14:29:53 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: Send Allocations to spawn https://review.openstack.org/511879 | |
| 14:29:57 | openstackgerrit | Dan Smith proposed openstack/nova master: Regenerate context during targeting, and sanity check some things https://review.openstack.org/511651 | |
| 14:29:58 | openstackgerrit | Dan Smith proposed openstack/nova master: WIP: Add cell retargeting warnings https://review.openstack.org/511864 | |
| 14:30:03 | mriedem | gibi: so i assume you're thinking about moving the RP allocation cleanup outside of that for loop and add a new for loop on the 'evacuations' instances right? | |
| 14:30:15 | gibi | mriedem: something like that | |
| 14:30:45 | gibi | mriedem: the driver.destroy still need to be only run for the instances that are known by the hypervisor but the resource cleanup should run on all the evacuated instances | |
| 14:31:00 | mriedem | gibi: yeah probably | |
| 14:31:55 | gibi | mriedem: I will do this change separatly from the current fix | |
| 14:32:26 | gibi | mriedem: as this one will need additional functional test tweaking | |
| 14:34:50 | openstackgerrit | Merged openstack/nova-specs master: Spec for flavor description https://review.openstack.org/501017 | |
| 14:47:02 | mriedem | whoa, we've been translated https://etherpad.openstack.org/p/nova-ptg-queens | |
| 14:54:43 | mdbooth | dansmith: We have to discuss the uuid migration thing in real time, because we're talking past each other in review :) You seem to be in violent agreement with me. | |
| 14:55:55 | openstackgerrit | garyk proposed openstack/nova master: Add debug information to metadata requests https://review.openstack.org/511895 | |
| 14:57:18 | superdan | mdbooth: we may agree, but your last comment says specific things I don't agree with | |
| 14:57:29 | superdan | mdbooth: but sure, +1h we can | |
| 15:08:44 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: fix cleaning up evacuated instances https://review.openstack.org/510938 | |
| 15:08:45 | alex_xu | fried_rice: this may help you a little http://specs.openstack.org/openstack/nova-specs/specs/newton/implemented/generic-resource-pools.html#scenario-1-shared-disk-storage-used-for-vm-disk-images | |
| 15:09:09 | fried_rice | alex_xu Roger that, thanks. | |
| 15:09:40 | alex_xu | fried_rice: np | |
| 15:10:45 | alex_xu | fried_rice: there are some functional test for shared rp also https://review.openstack.org/#/c/498737/2 | |
| 15:11:14 | fried_rice | cool. | |
| 15:12:38 | mriedem | sdague: are you happy with the rebuild + keypair spec now? https://review.openstack.org/#/c/375221/ | |
| 15:28:32 | sdague | mriedem: yes | |
| 15:35:42 | stephenfin | In jaypipes' absence, who could I ask to have a look at the PCI NUMA policies spec? https://review.openstack.org/#/c/361140/ | |
| 15:36:23 | stephenfin | superdan, perhaps? ^ | |
| 15:40:12 | fried_rice | stephenfin Trade ya for https://review.openstack.org/510244 | |
| 15:40:29 | stephenfin | fried_rice: Deal (y) | |
| 15:41:24 | stephenfin | Heh :) | |
| 15:50:47 | mriedem | alex_xu: you can drop the -2 on this now https://review.openstack.org/#/c/379128/ | |
| 15:50:50 | mriedem | the blueprint is approved | |
| 16:04:20 | fried_rice | stephenfin Done. | |
| 16:04:21 | openstackgerrit | Merged openstack/nova-specs master: Reset the instance keypair while rebuilding (spec) https://review.openstack.org/375221 | |
| 16:09:34 | openstackgerrit | Matthew Booth proposed openstack/nova-specs master: Add serial numbers for local disks https://review.openstack.org/511466 | |
| 16:15:34 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Regenerate context during targeting, and sanity check some things https://review.openstack.org/511651 | |
| 16:15:49 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Add cell retargeting warnings https://review.openstack.org/511864 | |
| 16:19:44 | superdan | mriedem: the title of the commit needs to lose the "and sanity check some things" bit | |
| 16:19:56 | superdan | I can edit in place if you want | |
| 16:20:47 | openstackgerrit | Dan Smith proposed openstack/nova master: Regenerate context during targeting https://review.openstack.org/511651 | |
| 16:20:52 | superdan | boom ^ | |
| 16:23:42 | openstackgerrit | Eric Fried proposed openstack/nova master: Send Allocations to spawn https://review.openstack.org/511879 | |
| 16:24:53 | johnthetubaguy | mriedem: I have one outstanding question in my head on the live-migration cinder patch, otherwise I am +2 | |
| 16:26:42 | openstackgerrit | Eric Fried proposed openstack/nova master: Send Allocations to spawn https://review.openstack.org/511879 | |
| 16:27:33 | fried_rice | superdan ^ with the objects outta there (and DRYing deferred to the future first patch that uses the new kwarg) | |
| 16:29:35 | friedrice_injera | Goofy length limits. Should be at least 255c, if not a TEXT field. | |
| 16:30:34 | superdan | friedrice_injera: ack, I'll look in a bit when I get back from something | |
| 16:36:41 | openstackgerrit | zhangyangyang proposed openstack/nova master: Fix bug of py27 job failing on testtools.matchers._impl.MismatchError https://review.openstack.org/511919 | |
| 16:39:53 | openstackgerrit | zhangyangyang proposed openstack/nova master: Fix bug of py27 job failing on testtools.matchers._impl.MismatchError https://review.openstack.org/511919 | |
| 16:49:01 | stephenfin | friedrice_injera: I got half way through that. Will finish bright n early Monday :) (it's nearly 6pm herE) | |
| 17:17:56 | mdbooth | dansmith: Ironic is a problem... | |
| 17:18:30 | mdbooth | dansmith: But it's also potentially a problem in other virt drivers, as they might have default serial number behaviour. | |
| 17:19:00 | mdbooth | I think we need to have the virt driver populate the serial number in instance device metadata. | |
| 17:19:16 | mdbooth | And we'd suggest that bdm.uuid is the default. | |
| 17:20:14 | mdbooth | Incidentally, it's weird to me that the driver creates instance.device_metadata, but I guess it makes sense. | |
| 17:20:26 | mdbooth | (i.e. that's the current behaviour) | |
| 17:21:57 | mdbooth | So ironic would query the disk's current serial number and populate it in device.serial | |
| 17:22:20 | mdbooth | Anyway, that's a much bigger change than I was anticipating this evening. I'll do that Monday. | |
| 17:34:04 | mriedem | superdan: boomshakalaka | |
| 17:34:51 | mriedem | johnthetubaguy: looking | |
| 17:35:42 | johnthetubaguy | mriedem: thanks, I think the current volume attach patch fixes the problem I found | |
| 17:35:52 | johnthetubaguy | mriedem: no sure what we should do about that, maybe just ignore it? | |
| 17:35:55 | mriedem | i think i might know what you're talking about | |
| 17:36:05 | mriedem | if i got tripped up on the same thing | |
| 17:38:28 | mriedem | superdan: some unrelated trickery here? https://review.openstack.org/#/c/506419/20/nova/compute/manager.py | |
| 17:42:23 | mriedem | johnthetubaguy: yeah you fell into the same trap that i did | |
| 17:42:28 | mriedem | finding where i pointed this out | |
| 17:48:04 | johnthetubaguy | mriedem: I am curious why its OK | |
| 17:48:47 | mriedem | why which what is ok? | |
| 17:49:13 | mriedem | so steve's live migration patch was relying on john's new style enablement patch to call attachment_update deep down in refresh_connection_info, | |
| 17:49:38 | mriedem | that was before i found out that attachment_update in refresh_conn_info puts the instance back into attaching status, and expects you to eventually call attachment_complete on it, | |
| 17:49:45 | mriedem | even if you aren't attaching a volume | |
| 17:50:08 | mriedem | so i think we have to change that in john's patch, and fix the now incorrect assertion/assumption in steve's patch | |
| 17:50:23 | mriedem | i left some replies in stvnoyes' change | |
| 17:51:22 | johnthetubaguy | mriedem: OK, thanks. | |
| 17:52:47 | mriedem | so we really have to keep in mind now that attachment_update != os-initialize_connection | |
| 17:53:02 | mriedem | because attachment_update changes the volume's attach status, where os-initialize_connection didn't | |
| 17:53:27 | mriedem | this does make me wonder about the ceph creds refresh thing we talked about at the ptg, | |
| 17:53:49 | mriedem | we were going to just always refresh the connection info to force cinder to give us new connection info in case anything has changed, | |
| 17:54:05 | mriedem | now if we're not initiating that refresh on the storage backend, i don't know what would be if your ceph ip or creds change | |
| 17:54:18 | johnthetubaguy | mriedem: yeah, it changes that case some, we spoke about needing a new attachment for the connector changed case, maybe... | |
| 17:54:34 | mriedem | but how do we know if the connector changed? | |
| 17:54:49 | mriedem | we don't - that's why we said at the ptg we'd just refresh when we have the chance, like during reboot | |
| 17:55:08 | mriedem | that was the alternative to adding a new admin-only API to force a refresh | |
| 17:55:42 | johnthetubaguy | mriedem: I am quite a fan of making that explicit, almost feels like volume migrate | |
| 17:55:49 | mriedem | i don't think we want to be randomly updating and completing attachments just to refresh the connection info | |
| 17:55:59 | johnthetubaguy | ++ | |
| 17:56:02 | mriedem | for a case that should rarely happen | |
| 17:56:19 | johnthetubaguy | yeah, its a whoops I broke my cloud, please help me case | |
| 17:56:26 | mriedem | we can still fix the bug for old style attachments like we talked about at the ptg, | |
| 17:56:33 | mriedem | but we'll likely have to think of something else for new style attachments | |
| 17:56:48 | johnthetubaguy | so I need to go and sort out food before sally looses the plot | |
| 17:57:02 | mriedem | yup, go go! | |
| 18:30:47 | superdan | mriedem: replied | |
| 18:30:51 | superdan | mriedem1: ^ | |
| 18:32:28 | mriedem | yeah it's just weird it's showing up in this change, | |
| 18:32:38 | mriedem | i.e. how did this pass the change that added this code in the py35 unit tests? | |
| 18:34:31 | superdan | oh jeez, yeah, this was in the wrong patch I see | |
| 18:34:49 | mriedem | https://www.youtube.com/watch?v=gGrNAB45CtY | |
| 18:34:56 | superdan | oh, I bet it's because we never get in here | |
| 18:35:01 | superdan | until this patch | |