Earlier  
Posted Nick Remark
#openstack-nova - 2018-03-30
21:38:21 openstackgerrit Matt Riedemann proposed openstack/nova master: Add "bind_ports_to_host" neutron API method https://review.openstack.org/523604
21:38:22 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: libvirt: use dest host vif migrate details for live migration https://review.openstack.org/551370
21:38:23 openstackgerrit Matt Riedemann proposed openstack/nova master: Add "activate_port_binding" neutron API method https://review.openstack.org/555947
21:38:23 openstackgerrit Matt Riedemann proposed openstack/nova master: Add "delete_port_binding" network API method https://review.openstack.org/552170
21:38:24 openstackgerrit Matt Riedemann proposed openstack/nova master: Implement migrate_instance_start method for neutron https://review.openstack.org/556334
21:38:24 openstackgerrit Matt Riedemann proposed openstack/nova master: Delete port bindings in setup_networks_on_host if teardown=True https://review.openstack.org/556333
21:38:25 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Port binding based on events during live migration https://review.openstack.org/434870
21:38:25 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: compute: use port binding extended API during live migration https://review.openstack.org/551371
21:38:26 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Wait for network-vif-plugged before starting live migration https://review.openstack.org/558001
21:38:26 openstackgerrit Matt Riedemann proposed openstack/nova master: conductor: use port binding extended API in during live migrate https://review.openstack.org/522537
21:39:06 mriedem fried_bunny: imacdonn: idk, definitely not something i care to think about when i'm about 10 minutes from a 4 day break
21:39:26 mriedem i'm sure dansmith would like to make this is first thing monday morning
21:39:30 mriedem *his
21:40:33 imacdonn mriedem fried_bunny dansmith: it can wait (from my perspective) ... I was mostly treating it as "low-hanging fruit"
21:42:36 fried_bunny imacdonn: Are you interested in writing some code for this?
21:43:53 imacdonn fried_bunny: I was interested in a simple patch to remove the check and associated unit-test... beyond that, probably not really (have bigger fish to fry)
21:44:12 fried_bunny imacdonn: The fix wouldn't be much more than that.
21:44:22 imacdonn heheh ... or bunnies to fry, as the case may be ;)
21:44:54 fried_bunny raise exception.PlacementNotConfigured()
21:44:54 fried_bunny except:
21:44:54 fried_bunny self.reportclient.get('/')
21:44:54 fried_bunny try:
21:44:54 fried_bunny imacdonn: Instead of just removing the check, replace it with something like:
21:46:28 imacdonn I can give that a go ... I actually want to see what happens if placement is not configured and there's no check first, though
21:47:39 fried_bunny coolcool. Feel free to add me (efried) to the review if you do decide to spin something up.
21:48:33 imacdonn fried_bunny mriedem http://paste.openstack.org/show/718043/
21:48:55 imacdonn that's what I get if there's no [placement] section in my compute's nova.conf
21:49:21 imacdonn looks pretty obvious to me
21:49:56 fried_bunny imacdonn: ...and you removed that region_name check?
21:49:58 imacdonn although maybe with different scheduler config, it'd be less-so
21:50:06 imacdonn yeah, I commented out the check
21:52:08 fried_bunny raise...
21:52:08 fried_bunny if self.reportclient.get('/') is None:
21:52:08 fried_bunny imacdonn: I amend my suggested code change above. It should be more like:
21:53:05 fried_bunny Then you'll get that auth warning as well as the PlacementNotConfigured exception.
21:57:46 imacdonn hmm, doesn't seem to be working ... still experimenting
21:58:30 fried_bunny oh
21:59:12 fried_bunny imacdonn: You don't need auth to get the version document. So if placement is actually running and there's a service catalog entry for it, that will succeed.
21:59:32 imacdonn well, it's getting the MissingAuthPlugin, so it doesn't actually get to raise the PlacementNotConfigured
21:59:37 fried_bunny Right.
21:59:47 fried_bunny I assume you have the service running and there's an entry for it in the service catalog?
21:59:57 fried_bunny ...but no [placement] section in your conf.
22:00:00 imacdonn right
22:00:28 fried_bunny The version document doesn't require authentication, and we'll get the endpoint from the service catalog, so get('/') will actually work. And then you'll blow up later when you try to hit a real URI.
22:00:37 fried_bunny And I contend that's actually the behavior we want.
22:00:54 fried_bunny If you shut down your placement service and try again, I think you'll get the PlacementNotConfigured error.
22:01:45 imacdonn that's not really what this check was intended for, though
22:01:53 imacdonn (originally, at least)
22:02:48 fried_bunny well, it was to make sure you had "configured" placement. It didn't previously do anything to ensure you'd configured it *right*. Heck, it wasn't even checking a thing that you had to have, clearly.
22:03:27 imacdonn in my interpretation, it was to check that you had the compute service configured to use placement, not to check that placement is functional
22:04:09 fried_bunny okay, I can buy that interpretation. In which case maybe auth stuff not being empty is more appropriate to check for in here.
22:05:00 fried_bunny because correct me if I'm wrong, but you *can't* get by without e.g. [placement]auth_type ?
22:05:35 imacdonn I thought so, but Matt said something about different scheduler options, so I lost confidence for a bit
22:06:09 fried_bunny shit, if there's a scheduler option that doesn't use placement, then why are we enforcing that you have it set up when you're not using that scheduler option.
22:06:45 imacdonn "mriedem> but we do want the computes putting inventory information into placement so we can eventually migrate CachingScheduler users"
22:06:48 fried_bunny Whatever, this is probably a better thing to discuss on Monday or Tuesday when people who were around for the inception of this check are back from worshipping egg-laying rabbits.
22:07:26 imacdonn you mean chocolate-egg-laying rabbits, of course
22:12:11 imacdonn try:
22:12:12 imacdonn raise exception.PlacementNotConfigured()
22:12:12 imacdonn except keystone_exception.MissingAuthPlugin:
22:12:12 imacdonn 'some sort of exception here')
22:12:12 imacdonn log.error('placement is not working - I should raise '
22:12:12 imacdonn if self.reportclient.get('/') is None:
22:12:31 imacdonn (maybe - "thinking out loud")
22:16:31 fried_bunny imacdonn: MissingAuthPlugin will never happen there.
22:16:45 imacdonn it does, if the placement config is missing
22:16:48 fried_bunny It gets swallowed by @safe_connect
22:16:59 fried_bunny No, you get a warning about it, but the exception doesn't get raised.
22:17:11 imacdonn I tried it
22:17:38 imacdonn I mean - I tried the code above, with no placement config, and it did what I expected
22:17:40 fried_bunny that pastebin you showed me had the warning right above an unrelated exception. Did you see something different another way?
22:18:30 imacdonn I think that unrelated warning was caused by something that the scheduler happened to do that time ... I don't usually see that warning on startup
22:19:43 fried_bunny The warning was coming from the report client trying to bootstrap the compute node inventory.
22:19:46 fried_bunny through placement
22:20:24 fried_bunny which hits @safe_connect, which catches MissingAuthPlugin and prints that warning.... and then does nothing. Like, implicitly returns None. Which is why you got that NoneType blah blah error.
22:20:35 imacdonn http://paste.openstack.org/show/718047/
22:21:10 imacdonn that's with code pasted above, and missing config
22:22:36 fried_bunny imacdonn: Are you running on master?
22:22:46 imacdonn no, this is queens
22:23:20 fried_bunny if you curl the base placement URI, do you get the version document or a 401?
22:23:55 imacdonn {"versions": [{"min_version": "1.0", "max_version": "1.17", "id": "v1.0"}]}
22:24:44 fried_bunny oh - .get isn't wrapped by safe_connect. Let me see where that MissingAuthPlugin business is coming from.
22:25:10 fried_bunny though it would be easier for you to find out - by removing the try/except and seeing what .get raises all by itself.
22:25:30 imacdonn can do
22:25:57 fried_bunny is my guess.
22:25:57 fried_bunny ...something in ksa loading...
22:25:57 fried_bunny get_ksa_adapter
22:25:57 fried_bunny _create_client
22:26:22 fried_bunny load_auth_from_conf_options
22:26:27 imacdonn http://paste.openstack.org/show/718048/
22:29:10 fried_bunny oh, interesting - we actually let you load up the ksa adapter; and it fails on the request. But still, that's weird; you shouldn't need auth to get the version document.
22:30:16 fried_bunny What happens when you shut down the placement service?
22:30:59 imacdonn ConnectFailure
22:31:39 imacdonn from ksa trying to do a GET request
22:32:30 fried_bunny mm
22:33:20 fried_bunny Well, I'm not happy that you're getting MissingAuthPlugin for /. But it's what you'll get for anything else you try, so that's not the end of the world.
22:34:07 imacdonn I guess that the client doesn't know that auth is not required to get the version
22:34:26 fried_bunny mordred will not be happy about that. Or maybe it's my fault.
22:35:40 fried_bunny anyway, it's sounding like to cover bases we may want to do something like .get('/resource_providers?name=bogus'), which *should* require auth, and catch both MissingAuthPlugin and ConnectFailure and convert those to PlacementNotConfigured.
22:37:23 fried_bunny imacdonn: If that's more than you want to take on, or if you want to write part of it and then hand it off, put something somewhere and flag me on it.

Earlier   Later