Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-01-24
17:03:23 mriedem vladikr: i'm not so sure, because that's done separately in the libvirt driver https://github.com/openstack/nova/blob/45e79583cea3a87df931c7333bf3c82de2979105/nova/virt/libvirt/driver.py#L3180
17:03:23 vladikr mriedem, config drive uses InstanceMetadata to build
17:03:28 mriedem yes i know,
17:03:33 vladikr looking
17:03:44 mriedem but what you're changing in the metadata is the route from an API request to get the metadata
17:04:34 mriedem artom: have you noticed that we build the device metadata for an instance (which hits the db) even if we don't build the config drive? https://github.com/openstack/nova/blob/45e79583cea3a87df931c7333bf3c82de2979105/nova/virt/libvirt/driver.py#L3180
17:05:08 openstackgerrit Gábor Antal proposed openstack/nova: Remove mox in unit/cells/test_cells_manager.py https://review.openstack.org/399036
17:05:13 dansmith johnthetubaguy: see if those explanations make sense
17:05:31 dansmith johnthetubaguy: sorry for saying this was simple. I've factored out all the complexity of "why" in my head I guess :)
17:05:31 johnthetubaguy dansmith: will take a peak, was just looking at next one
17:05:49 johnthetubaguy dansmith: heh, no worries
17:06:17 johnthetubaguy dansmith: in that follow up, I saw a comment about 404 if nova-api service minimum version < 15, can't we avoid that?
17:07:01 dansmith johnthetubaguy: let me refresh my memory but I don't think we can
17:07:35 mriedem vladikr: ok i see how InstanceMetadata.metadata_for_config_drive is called from the config drive builder in the virt driver,
17:07:39 mriedem so it should work,
17:07:58 mriedem but i'm wondering why we have the explicit _build_device_metadata stuff in the libvirt driver then for device tags?
17:08:02 dansmith johnthetubaguy: oh that's in a piece that is already merged right?
17:08:18 openstackgerrit Andrey Kurilin proposed openstack/python-novaclient: Clarify description for tags https://review.openstack.org/424745
17:08:54 artom mriedem, we need it for the metadata API, but I agree that it's a weird place to put it
17:09:29 dansmith johnthetubaguy: actually, I'm not sure which of the version<15 checks you're talking about.. linky?
17:09:35 melwitt dansmith: I think he means this https://review.openstack.org/#/c/319379/73/nova/tests/functional/integrated_helpers.py@226
17:10:01 johnthetubaguy dansmith: yeah, https://review.openstack.org/#/c/319379/73/nova/tests/functional/integrated_helpers.py@228
17:10:05 melwitt that is, immediately after requesting a boot you could assume the instance exists before. but with the move to conductor, you can't
17:10:10 mriedem artom: vladikr: oh i see, so we build the device metadata in the virt driver and store that on instance.device_metadata, which the metadata API uses to build the md json for the config drive
17:10:35 johnthetubaguy I thought we would return it in the API list, because the uuid would be in the cell mapping table to avoid that
17:10:46 dansmith ohg that
17:10:58 melwitt yeah, I mean wouldn't we have the build request to create the instance from with the new stuff?
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 vladikr sfinucan, sure
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: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: Collected release notes for Ocata CellsV2 https://review.openstack.org/424737
17:36:37 openstackgerrit Dan Smith proposed openstack/nova: WIP: Remove duplicate BDM validation https://review.openstack.org/424719
17:36:38 openstackgerrit Dan Smith proposed openstack/nova: Make api_samples tests use simple cell environment https://review.openstack.org/410473
17:36:38 openstackgerrit Dan Smith proposed openstack/nova: Move instance creation to conductor https://review.openstack.org/319379
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

Earlier   Later