| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-01-24 | |||
| 17:11:00 | artom | mriedem, we have to build the device metadatain (or at least from) the virt driver because only it knows the hardware metadata like devices addresses | |
| 17:11:18 | artom | mriedem, but I don't think the metadata API builds anything for the config drive... | |
| 17:11:23 | vladikr | artom is typing much faster :) | |
| 17:11:30 | dansmith | johnthetubaguy: melwitt good question, that might've been an early attempt by avolkov to address the sync spawn thing, lemme check | |
| 17:11:40 | artom | When the config drive builds itself (for lack of a better work) it gets the instance metadata on its own | |
| 17:11:46 | artom | vladikr, jinx ;) | |
| 17:12:03 | melwitt | dansmith: yeah, I would think we don't need that change because we should be getting the same behavior as before via build request | |
| 17:12:13 | mriedem | artom: well it's used here v | |
| 17:12:15 | mriedem | https://github.com/openstack/nova/blob/45e79583cea3a87df931c7333bf3c82de2979105/nova/virt/configdrive.py#L69 | |
| 17:12:30 | dansmith | melwitt: yeah, I dunno, I'll try removing it | |
| 17:12:59 | mriedem | and i suppose we persist instance.device_metadata later in the compute manager when it calls instance.save() | |
| 17:13:15 | mriedem | so that later metadata API requests (when not using config drive) can retrieve the device tags | |
| 17:13:21 | dansmith | johnthetubaguy: I would think that if it's really an api thing we'd be failing tempest | |
| 17:13:45 | johnthetubaguy | dansmith: yeah, +1 | |
| 17:14:13 | artom | mriedem, ah, didn't know about metadata_for_configdrive | |
| 17:14:35 | artom | Then yeah, the metadata service (not API surely?) builds the metadata for the config drive | |
| 17:14:52 | sfinucan | vladikr, sahid: Could ye take a look at https://review.openstack.org/#/c/370374 at some point? I think it looks good but I'd like more opinions and moshele is offline | |
| 17:15:02 | artom | mriedem, the instance.save() part is I think just lucky, because when coding the tagged attached stuff I don't recall ever seeing it in the existing code | |
| 17:15:17 | openstackgerrit | Matthew Booth proposed openstack/nova: libvirt: Fix races in glusterfs mount/umount https://review.openstack.org/423402 | |
| 17:15:20 | artom | For attach I have to call it explicitly | |
| 17:15:27 | dansmith | johnthetubaguy: sfinucan: so for that flag set in the api_samples patch, all I can think of as a comment to help is: | |
| 17:15:27 | vladikr | sfinucan, sure | |
| 17:15:43 | openstackgerrit | Stephen Finucane proposed openstack/nova: pci: Validate behavior of empty devname https://review.openstack.org/396130 | |
| 17:15:45 | dansmith | johnthetubaguy: sfinucan: "Default our global database connection info to be the first cell for any non-cells-aware code" | |
| 17:15:48 | mriedem | artom: yeah after driver.spawn(), the compute manager changes the state to ACTIVE on the instance and calls instance.save() | |
| 17:15:51 | gibi | mriedem: I think I have solution for the randomly failing notification sample functional test https://review.openstack.org/#/c/424574/ | |
| 17:15:52 | dansmith | johnthetubaguy: sfinucan: would that help? | |
| 17:15:58 | mriedem | so anything the virt driver changed on the instance during spawn is going to get persisted with that | |
| 17:16:11 | sfinucan | vladikr: Cheers. No panic, mind - VLAN stuff take priority | |
| 17:16:17 | johnthetubaguy | dansmith: that would work for me I think | |
| 17:16:33 | dansmith | johnthetubaguy: ack | |
| 17:16:55 | artom | mriedem, cool - not sure if that was intentional on our part to not call save() from the virt drive to save on DB requests, but glad that it works :D | |
| 17:18:17 | artom | (toast)^2ing | |
| 17:22:14 | mdbooth | sfinucan: ^^^ TODO'd btw | |
| 17:23:43 | mdbooth | dansmith: I was trying to reference something the other day which says that reboot() has a secondary function of 'try to fix all the hypervisor things'. | |
| 17:23:50 | mdbooth | Are you aware of anything? | |
| 17:24:03 | dansmith | mdbooth: um, what? | |
| 17:24:17 | mdbooth | dansmith: I'm guessing that a no ;) | |
| 17:24:22 | dansmith | heh | |
| 17:24:33 | openstackgerrit | Balazs Gibizer proposed openstack/nova: Fix race condition in instance.update sample test https://review.openstack.org/424574 | |
| 17:24:39 | mdbooth | So, when you reboot an instance we rewrite the domain xml and recreate backing files which should already exist. | |
| 17:24:56 | mdbooth | cburgess: ^^^ | |
| 17:25:49 | mdbooth | Anyway, this is very messy and sahid proposed a patch to remove it. However, if this is operator-expected behaviour then we can't do that without replacement. | |
| 17:26:03 | gibi | mriedem: thanks for the quick review. I've fixed your comments | |
| 17:31:08 | openstackgerrit | Artom Lifshitz proposed openstack/nova: Fix tag attribute disappearing in 2.32 and 2.37 https://review.openstack.org/424759 | |
| 17:33:03 | dansmith | johnthetubaguy: melwitt: confirmed, that integrated helpers hack isn't needed | |
| 17:33:33 | melwitt | cool, good catch johnthetubaguy | |
| 17:33:57 | johnthetubaguy | it was jst that comment that made my eyes pop out a little | |
| 17:34:44 | melwitt | heh | |
| 17:34:58 | johnthetubaguy | dansmith: I am missing why we need to call _validate_bdm twice in there, are there more accurate instance details the second time around? | |
| 17:35:08 | sfinucan | mdbooth: and +2'd | |
| 17:35:14 | johnthetubaguy | dansmith: thinking about: https://review.openstack.org/#/c/319379/73/nova/compute/api.py@1227 | |
| 17:35:31 | dansmith | johnthetubaguy: see the comments in there. we previously figured out that we do need to do that, but we don't remember why. so I've stacked a patch on the end we can experiment with | |
| 17:35:43 | melwitt | johnthetubaguy: I pointed that out too, and I remember dansmith, mriedem and I all dug into once on a different review. but I can't remember what review it was so we can look at it | |
| 17:36:06 | johnthetubaguy | dansmith: melwitt: ah, gotcha, sorry, didn't check the history around that bit | |
| 17:36:37 | openstackgerrit | Dan Smith proposed openstack/nova: WIP: Remove duplicate BDM validation https://review.openstack.org/424719 | |
| 17:36:37 | openstackgerrit | Dan Smith proposed openstack/nova: WIP: Collected release notes for Ocata CellsV2 https://review.openstack.org/424737 | |
| 17:36:38 | openstackgerrit | Dan Smith proposed openstack/nova: Move instance creation to conductor https://review.openstack.org/319379 | |
| 17:36:38 | openstackgerrit | Dan Smith proposed openstack/nova: Make api_samples tests use simple cell environment https://review.openstack.org/410473 | |
| 17:36:43 | dansmith | johnthetubaguy: np, good eye :) | |
| 17:36:53 | mriedem | wasn't it the object copy thing? | |
| 17:36:57 | dansmith | melwitt: johnthetubaguy: comment added on the first, and that hack removed from the second ^ | |
| 17:37:04 | melwitt | mriedem to the rescue | |
| 17:37:09 | mriedem | it was avolkov's patch | |
| 17:37:44 | dansmith | mriedem: yeah, but we must have discussed on irc | |
| 17:38:05 | mriedem | i thought we had captured the conversation in the review too | |
| 17:38:45 | mriedem | i don't remember the context on the object copy thing now either | |
| 17:39:03 | johnthetubaguy | its odd how we update the BDM, after the initial validate, duno if thats related: https://github.com/openstack/nova/blob/065cd6a8d69c1ec862e5b402a3150131f35b2420/nova/compute/api.py#L1302 | |
| 17:39:14 | melwitt | I thought we discussed on the review too | |
| 17:40:04 | mriedem | https://review.openstack.org/#/c/374267/ is the patch i'm thinking of fwiw | |
| 17:40:33 | melwitt | yep that's the one | |
| 17:40:47 | melwitt | let's see if we recorded the detail of what's going on there | |
| 17:41:21 | openstackgerrit | Andrey Kurilin proposed openstack/python-novaclient: [CLI] Fix token auth type https://review.openstack.org/424256 | |
| 17:41:26 | mriedem | https://review.openstack.org/#/c/374267/21/nova/compute/api.py@1197 | |
| 17:43:47 | melwitt | nice | |
| 17:46:23 | sfinucan | dansmith: Yeah, that comment WFM too. Re-reviewing now | |
| 17:46:31 | dansmith | sfinucan: cool | |
| 17:47:02 | paw | Hi, how do you specify a microversion for nova/api? I'm trying to test some interoperability issue. Thanks. | |
| 17:49:01 | artom | paw, in the client? --os-compute-api-version=2.40 | |
| 17:49:39 | paw | as in the python-novaclient? | |
| 17:50:29 | paw | aries: confirmed. thanks | |
| 17:50:43 | artom | paw, that option I gave is for the client, yes - both novaclient and openstackclient accept I think | |
| 17:51:19 | paw | artom: thanks | |
| 17:51:39 | artom | paw, you're welcome :) | |
| 17:52:43 | openstackgerrit | Dan Smith proposed openstack/nova: WIP: Remove duplicate BDM validation https://review.openstack.org/424719 | |
| 17:52:43 | dansmith | johnthetubaguy: gdi. | |
| 17:52:43 | openstackgerrit | Dan Smith proposed openstack/nova: WIP: Collected release notes for Ocata CellsV2 https://review.openstack.org/424737 | |
| 17:52:44 | openstackgerrit | Dan Smith proposed openstack/nova: Move instance creation to conductor https://review.openstack.org/319379 | |
| 17:52:44 | openstackgerrit | Dan Smith proposed openstack/nova: Make api_samples tests use simple cell environment https://review.openstack.org/410473 | |
| 17:52:58 | dansmith | johnthetubaguy: I don't know what to do in a world where _you_ are finding _my_ typos :P | |
| 17:55:42 | johnthetubaguy | dansmith: its a very crazy day! | |
| 17:55:48 | dansmith | heh | |
| 17:56:01 | cburgess | mdbooth Yes correct. An in general do anything we can to make sure the VM comes up. | |
| 17:56:18 | cburgess | mdbooth Think of it as a check list of what can we do to attempt to fix *anything* that might be wrong. | |
| 17:57:32 | melwitt | from what I can tell, I don't see why we would need the second _validate_bdm call. it doesn't seem to do anything except check things | |
| 17:58:05 | melwitt | in that other patch where we discussed the double validation, it was more than that, it was two calls of the self._bdm_validate_set_size_and_instance function, which does more | |
| 18:00:30 | dansmith | melwitt: well, lets see if we get a full run of the wip patch | |
| 18:01:16 | mriedem | well i'm going to call it a morning and head out for awhile, i'm already in trouble | |
| 18:01:41 | melwitt | o/ | |
| 18:02:13 | dansmith | mriedem: thanks | |