Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2017-09-21
15:52:33 mriedem stvnoyes: looks like it fails here https://review.openstack.org/#/c/487884/7/tempest/api/compute/admin/test_live_migration.py@185
15:52:38 dansmith edleafe: so, what follows is some rambling for improvements we can make later, but listen and nod (or not):
15:52:57 bauzas dansmith: see HostManager.consume_from_request() and you'll understand
15:53:21 dansmith edleafe: I think what you're doing here is going through the list of hosts like we do today, and picking the target and alternates for each one,
15:54:02 dansmith edleafe: which may mean your first alternate for instance 2 may be the the target for instance 3 and so on, such that all the alternates for early instances are almost definitely going to be burned by the later instances for any num_instances>max_retries
15:54:03 dansmith right?
15:54:29 dansmith so, hmm, yeah if that's right, that seems like a problem
15:54:31 edleafe dansmith: yes. Selecting an alternate doesn't consume anything
15:54:52 dansmith what we probably need to do is go through and pick primaries, and then pick alternates for each after that
15:55:03 dansmith the alternates can overlap, but they shouldn't overlap with any primaries
15:55:21 edleafe dansmith: ok, that could be done
15:55:48 dansmith edleafe: I'm just thinking that we're basically never going to be able to reschedule those early instances if we do it in the order you have, which is probably bad :)
15:56:04 dansmith edleafe: I shall add comments about this
15:56:06 edleafe I think in an earlier version of this we didn't think this would be a significant issue
15:56:54 dansmith yeah, I think we discussed it before, indeed
15:56:55 dansmith I think i was focusing on the overlapping of the alternates,
15:56:57 edleafe since a) alternates should rarely be used and b) hosts might still fit an additional instance
15:56:59 dansmith and not considering the overlapping of primaries
15:57:01 dansmith yeah
15:57:26 stvnoyes mriedem: ok, Ill take a look
15:58:41 gibi dansmith: I think I found why the target_cell context manager eats our MarkerNotFoundException, see my comment in https://review.openstack.org/#/c/504986/6/nova/compute/instance_list.py@85
15:59:21 dansmith gibi: ahh, that makes more sense.. I forgot we were using the figure's target_cell here
15:59:40 dansmith gibi: I wrote that fixture, so I'll fix that up and make this change at the end of this series
16:00:00 dansmith gibi: now, please, go find bugs in someone else's code :)
16:00:34 efried sdague mordred Design point about bp/use-ksa-adapter-for-endpoints: When setting up for a service where we have the ability to get auth from context, what should we do about auth in conf? Options: a) Always use the context auth, don't even register auth options in the conf; b) Register auth conf options and allow them to override the context auth.
16:00:45 efried cdent ^ you may also have an opinion
16:01:03 openstackgerrit Merged openstack/nova-specs master: Spec: Use keystoneauth1 Adapter for endpoints https://review.openstack.org/500190
16:01:36 sdague efried: it depends on what the subcall is doing
16:01:48 sdague if it is acting on behalf of the user, it should use the context auth
16:02:02 gibi dansmith: OK, I will review that follow up too. But instead of looking at others code I will just stop looking at any code for today
16:02:04 sdague optionally wrapped in a service token so that it doesn't expire
16:02:23 dansmith gibi: as long as it's not finding bugs in _my_ code I'm fine with whatever :P
16:03:15 gibi dansmith: :)
16:03:30 efried sdague I guess there's also c) Don't use the context auth; require the conf auth.
16:03:38 sdague efried: yeh
16:03:39 efried sdague I hear you saying it's gonna be case by case.
16:03:41 dansmith gibi: I'll add you to the review of the fixture cleanup when I have it
16:03:45 sdague it's going to be case by case
16:03:59 efried sdague I'll likely need some help identifying which is which.
16:04:12 sdague efried: so, I think the answer ends up being roughly
16:04:23 sdague glance, cinder, barbican, keystone always act as user
16:04:41 sdague ironic always from conf, because nova is the ironic multi tenancy solution
16:04:51 sdague and neutron, it depends on the operation
16:05:31 efried sdague Okay. For that first set: You already pushed for glance to be able to get the auth from context, so not c). Should we b) allow conf override or just a) always use context?
16:05:59 sdague efried: for glance, I don't think so
16:06:13 sdague I can't think of a case where we should "sudo" on image things
16:06:27 sdague or that image things have to happen outside of a user context
16:07:08 sdague we should, whenever possible, really operate in the user context of the user token we got, because it ensures we don't have an unintended priv escalation
16:07:26 sdague ironic is a special case, ironic doesn't have regular users
16:07:31 dansmith edleafe: sanity check my tome of words on that review?
16:07:38 efried (It's worth noting that part of the mission statement of this bp was consistency. Sigh.)
16:08:09 mordred efried, sdague: "optionally wrapped in a service token" doesn't need nova-specific creds does it?
16:08:21 edleafe dansmith: on a call and an IRC meeting, so it'll be later
16:08:29 dansmith edleafe: sure
16:08:54 efried mordred It's just service_auth.get_auth_plugin(context)
16:09:00 sdague mordred: i foreget exactly what it needs on disk
16:09:08 sdague mordred: it does need some creds
16:09:20 efried oh
16:10:03 efried mordred sdague https://github.com/openstack/nova/blob/master/nova/service_auth.py
16:10:28 efried So it loads up the auth from the [service_user] group.
16:10:51 efried and we get two auths in the thing.
16:10:53 efried yeesh.
16:11:35 mordred so - you need at least some of the ksa settings for each service no matter what
16:11:51 efried Only if CONF.service_user.send_service_user_token is set
16:11:56 mordred like you need the adapter options and probably the session options - so it's really just a question of whether the authoptions are included right?
16:13:22 efried mordred Yeah, that's what we're discussing - whether we should even register the auth options in groups where that service can be contacted using the user context auth.
16:16:02 mordred nod. well - consistency notwithstanding, I'd vote for not registering conf options if we're not ever going to use them - otherwise someone is going to configure them and then be confused why they're not used
16:17:16 mordred but I defer to smarter nova humans
16:17:46 sdague mordred: ++ lets keep things trimmed down
16:18:03 sdague efried: it's probably going to be worth writing a doc section on configuring nova with other services as well
16:18:11 sdague because I agree there is confusion here
16:20:11 efried sdague Cool. edmondsw brought this up in the spec review, and I addressed it with a brief parenthetical, but it has become a bigger thing quite suddenly. Worth a delta to the spec, you think? (It just merged.)
16:20:48 sdague efried: I'm ok if it's just admin docs as part of the work
16:20:53 efried rgr
16:23:08 sdague mordred: on https://review.openstack.org/#/c/488137/17/nova/utils.py@1309
16:23:30 sdague is the service types library not encoding this in python?
16:23:47 sdague like, I'd kind of expect that to be happening all behind the scenes
16:25:14 efried sdague Without a session param, it's pretty lightweight: https://github.com/openstack/os-service-types/blob/master/os_service_types/service_types.py#L51
16:25:15 mordred sdague: yes- os-service-types is - os-service-types didn't make it in to the keystoneauth release for pike, so we need to add that now that queens is open
16:25:37 efried Almost all of the work is done at import time, actually.
16:25:46 efried https://github.com/openstack/os-service-types/blob/master/os_service_types/service_types.py#L22
16:26:06 mordred efried: yah, I think it's actually fine for this cycle - the contents really do not change frequently - and CERTAINLY not for the services that nova cares about
16:26:19 efried The race would be harmless. I can take the lock out.
16:26:20 mordred so we can get live-update landed as a follow on
16:26:24 sdague um... https://github.com/openstack/os-service-types/blob/master/os_service_types/service_types.py#L59 ? closet network call?
16:26:53 edmondsw efried sdague mordred to be clear, long-term I think we'll need auth conf options for everything. But we don't today
16:26:56 sdague that seems like something people should have to opt into
16:26:58 mordred sdague: yah - it's there - but the keystoneauth consumption of the library at least at first will not pass a session
16:27:01 mordred sdague: yup
16:27:36 efried sdague The opt-in is by passing a `session` param.
16:27:43 mordred sdague: so when we land the next patch to ksa to consume that, nova can just switch to always passing the correct/official type to keystoneauth and keystoneauth will dtrt
16:28:16 mordred we'll make sure subsequent turning on of remote access/ network calls is appropriately opt-in when we add it
16:28:30 sdague I'm actually not super clear why the remote part is there
16:28:48 efried to get a fresh copy of the service-types-authority data
16:29:05 efried os-service-types ships with a cached copy
16:29:18 openstackgerrit Merged openstack/os-vif master: Update reno for stable/pike https://review.openstack.org/488671
16:29:19 sdague I thought the point of this was a no requirements version which we rev every time there is a service types update
16:29:29 sdague so people just replace it with the new one
16:29:42 efried Yup. Unless they don't.
16:29:50 sdague if they don't, then they don't

Earlier   Later