| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-10-13 | |||
| 14:23:38 | bauwser | mriedem: thanks for having taken time to summarize the outcome,+2d | |
| 14:23:46 | mriedem | bauwser: thanks | |
| 14:24:16 | mriedem | sdague: happy with the changes here? https://review.openstack.org/#/c/501017/ | |
| 14:24:32 | gibi | Why does the compute manager only want to clean up an evacuated instance at startup if the virt driver still report such instance? https://github.com/openstack/nova/blob/d9212edb8fe8376c218cc40e0bee1540083b25e6/nova/compute/manager.py#L655 | |
| 14:25:01 | openstackgerrit | Ed Leafe proposed openstack/nova master: Return Selection objects from the scheduler driver https://review.openstack.org/495854 | |
| 14:25:23 | gibi | I could imagine situations when a compute is recovered in a way that the hypervisor does not see the instance any more but the allocations are still not cleaned up | |
| 14:25:28 | sdague | mriedem: sold | |
| 14:27:19 | mriedem | gibi: hmm, probably just legacy behavior because the RT only cared about things running on it at that time | |
| 14:27:45 | mriedem | the RT will pull instances based on host, and if the instance was evacuated to another host, the old recovered source RT won't find those instances and track them anymore | |
| 14:28:37 | gibi | but as the rt periodic task will not clean up these instances any more we have to clean up evacuated instance even if they are not visible to the hypervisor any more | |
| 14:28:50 | gibi | at startup explicitly | |
| 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, | |