Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-01-24
16:47:46 mdbooth lyarwood: Couldn't really see what was changing, tbh.
16:48:22 mdbooth However, if it's just my lack of understanding...
16:48:55 mdbooth lyarwood: Personally I'd have called that out in the commit message, or ideally do the test rewriting in the patch before.
16:48:59 lyarwood mdbooth: so we shouldn't really change anything aside from mocking out the additional XMLDesc calls
16:49:26 openstackgerrit Andrey Kurilin proposed openstack/python-novaclient: Clarify description for tags https://review.openstack.org/424745
16:49:45 andreykurilin mriedem: do you mean something like this - https://review.openstack.org/424745 ?
16:49:46 mdbooth It's just a surprising amount of churn. If it's necessary then ok, I'll just look harder.
16:50:19 lyarwood mdbooth: yeah, maybe I'm missing a smaller change we can make to the original mox tests tbh
16:50:34 lyarwood mdbooth: and if I am please feel free to call it out and I'll go over this again
16:51:20 mdbooth lyarwood: I'll keep looking. I thought you'd just made a mistake so I didn't look all that hard.
16:53:59 sfinucan vladikr: I don't know why mriedem removed his -2 on this https://review.openstack.org/#/c/405951 - doesn't it still require a version bump?
16:54:20 vladikr sfinucan, looking
16:54:23 sfinucan Actually, wait - you did bump the version. nvm :) What about a reno?
16:54:53 mriedem sfinucan: i had a -2 on it when the series was much longer and i wanted to make sure the tempest testing was done and testing that API change first
16:55:12 mriedem vladikr: you should link to the tempest change in the commit message
16:55:37 sfinucan mriedem: Gotcha
16:55:49 sfinucan mriedem: Also, aren't you supposed to be drinking cocktails on a beach somewhere? :)
16:56:14 vladikr mriedem, ok, got it. so I need a new tempest test written for this right?
16:56:31 mriedem vladikr: i thought you already had one started
16:56:44 vladikr mriedem, no, but will do now
16:57:19 vladikr sfinucan, do I need a reno for that change? I thought we wanted to update the user guide
16:57:56 openstackgerrit Roman Podoliaka proposed openstack/nova: block_device: fix propagation of instance AZ to Cinder https://review.openstack.org/366724
16:57:58 mriedem i'd have a feature reno for it and the metadata API version change
16:58:03 sfinucan vladikr: if it introduces a new, user-visible change then I would assume so?
16:58:23 vladikr sfinucan, mriedem, will add now
16:59:10 sfinucan (y)
16:59:31 johnthetubaguy melwitt: I had a sudden memory of how the flavor stuff avoid duplicates earlier, I added a comment on that quota patch
17:00:22 melwitt johnthetubaguy: I saw and replied. thanks :) I will go that route. working through all of the changes on the series now
17:00:38 johnthetubaguy melwitt: cool
17:01:27 mriedem vladikr: will this change work for config drive too?
17:02:44 vladikr mriedem, yes, sure
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: ^^^

Earlier   Later