Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-01-24
16:40:30 andreykurilin since it is just validation step at API side
16:40:53 mriedem andreykurilin: at a minimum, the help text on the nova boot command will need to be updated
16:41:14 mriedem to say that bdm tags only work with 2.32, and nic tags only work with 2.32->2.36
16:41:59 mriedem otherwise someone will be wondering why they can't create a server with device tags and microversion 2.40, for example
16:42:02 andreykurilin mriedem: can we reserve next microversion for that bug-fix, so it will be possible to release novaclient with support of that microversion today and than merge patch to API side?
16:42:17 melwitt dansmith: I pointed out on PS71 but didn't notice it's still there (sorry). did you want this logging here? https://review.openstack.org/#/c/319379/73/nova/compute/api.py@2517
16:42:20 andreykurilin *next microversion number
16:42:26 mriedem andreykurilin: i'd rather not try to reserve a microversion
16:42:51 mriedem andreykurilin: since there are some patches still up for review for nova before FF that bump microversions
16:43:03 dansmith melwitt: yeah I was going to keep that and the related ones as it helped me debug and only runs once
16:43:12 melwitt okay, cool
16:43:12 openstackgerrit Roman Podoliaka proposed openstack/nova: neutron: re-raise 401 for a non-admin client https://review.openstack.org/422696
16:43:29 andreykurilin mriedem: but it sounds like only possible solution to release novaclient with fixed bug
16:44:00 mdbooth lyarwood: Have the test changes in that patch gotten mixed up with one of the other patches?
16:44:10 mriedem andreykurilin: it's already broken in older versions of novaclient,
16:44:15 mdbooth lyarwood: https://review.openstack.org/#/c/399735/18/nova/tests/unit/virt/libvirt/test_driver.py
16:44:21 mriedem andreykurilin: so i'm less worried about rushing a fix in on the client side for a new microversion,
16:44:39 mriedem andreykurilin: i think it's good to just get a change into novaclient for ocata that updates the help to note the limitation on usage
16:44:58 mriedem andreykurilin: we can get the novaclient fix with the new microversion released in pike
16:47:15 lyarwood mdbooth: I don't think so, it was easier to rewrite some of the snapshot tests in mock that have increased the size if that's an issue
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:

Earlier   Later