| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-08-28 | |||
| 16:56:33 | mriedem | in this case it's hard-coded in code | |
| 16:56:46 | mriedem | https://github.com/openstack/nova/blob/16.0.0.0rc2/nova/compute/api.py#L2120 | |
| 16:56:56 | mriedem | https://github.com/openstack/nova/blob/16.0.0.0rc2/nova/compute/api.py#L198 | |
| 16:57:50 | mriedem | ssmith: https://review.openstack.org/#/c/492477/ is semi related | |
| 16:59:07 | ssmith | So remove " and not context.is_admin" from the code? | |
| 16:59:24 | mriedem | well, as i suggested in that patch, it could be a configurable policy check | |
| 16:59:59 | ssmith | I agree with the need for the patch | |
| 17:00:01 | mriedem | that patch is wrong in two ways: 1. it's not on the master branch and 2. it's config-driven API via nova.conf; configuring access to things in the API should be via policy rules, not nova.conf | |
| 17:00:29 | mriedem | i wasn't -2 on the thing it's trying to address, just the way it's triyng to do it | |
| 17:00:36 | mriedem | plus it's on stable/ocata which is wrong | |
| 17:00:56 | ssmith | We're running Newton | |
| 17:01:15 | mriedem | all changes start on master, so whatever the solution, it has to start on master | |
| 18:50:24 | openstackgerrit | Merged openstack/nova master: docs: Document the scheduler workflow https://review.openstack.org/475810 | |
| 18:50:49 | openstackgerrit | Merged openstack/nova master: Monkey patch the blockdiag extension https://review.openstack.org/476159 | |
| 18:51:25 | openstackgerrit | Merged openstack/nova master: Add formatting to scheduling activity diagram https://review.openstack.org/476204 | |
| 18:51:55 | openstackgerrit | Merged openstack/nova master: Update PCI passthrough doc for moved options https://review.openstack.org/498461 | |
| 18:52:18 | openstackgerrit | Merged openstack/nova master: VMware: Do not check if folder already exists in vCenter https://review.openstack.org/376387 | |
| 19:10:57 | edleafe | OK, nova objects experts: what sort of Field would I use to store a dict like this: http://paste.openstack.org/show/619670/ | |
| 19:12:50 | artom | edleafe, I think that depends on how you're going to be accessing it | |
| 19:13:44 | edleafe | artom: probably just obj.foo to get the dict | |
| 19:14:47 | cdent | edleafe: any of this help https://github.com/openstack/nova/blob/master/nova/objects/resource_provider.py#L2338-L2359 | |
| 19:14:56 | artom | edleafe, I would think it'd be just a Dict then | |
| 19:16:03 | cdent | edleafe: of https://github.com/openstack/nova/blob/master/nova/objects/resource_provider.py#L2437-L2455 | |
| 19:16:11 | cdent | basically reuse the objects use to create the data in the first place | |
| 19:16:34 | cdent | If people want structured objects... | |
| 19:16:51 | edleafe | artom: but I got the feeling that fields.Dict was not to be used anymore | |
| 19:16:54 | edleafe | artom: https://github.com/openstack/nova/blob/master/nova/objects/fields.py#L72 | |
| 19:17:27 | openstackgerrit | Merged openstack/nova master: VMware: Handle missing volume vmdk during detach https://review.openstack.org/484675 | |
| 19:17:30 | artom | edleafe, ah, true, didn't see the comment | |
| 19:17:35 | artom | dansmith's the expert | |
| 19:17:46 | edleafe | cdent: The object needs to have the hostname and the corresponding allocation | |
| 19:17:51 | edleafe | that's it | |
| 19:17:55 | openstackgerrit | Merged openstack/nova master: libvirt: Fix getting a wrong guest object https://review.openstack.org/496515 | |
| 19:17:55 | artom | cdent, I think the idea behind objects is a poor man's type system | |
| 19:18:16 | artom | So that we don't have random unversioned unknown undiscoverable dicts flying around | |
| 19:18:25 | openstackgerrit | Merged openstack/nova master: Tests: Add cleanup of 'instances' directory https://review.openstack.org/491589 | |
| 19:18:28 | cdent | artom: I know. Me and types have never felt all that friendly. | |
| 19:18:51 | artom | cdent, typist ;) | |
| 19:20:05 | edleafe | cdent: I was thinking about using AllocationList, but that has DB stuff. This should just be a blob to send back | |
| 19:20:43 | cdent | edleafe: the other thing to keep in mind is that we really don’t want to be using placement objects on the nova side of the equation if we can help it | |
| 19:20:53 | cdent | so a fresh object might be the way to go | |
| 19:20:55 | edleafe | cdent: yeah, that too | |
| 19:21:08 | mriedem | edleafe: https://github.com/openstack/nova/blob/master/nova/objects/fields.py#L72 doesn't mean you can't use them, | |
| 19:21:14 | mriedem | it means use the fields from ovo | |
| 19:21:15 | mriedem | rather than nova | |
| 19:21:16 | edleafe | but this allocations dict doesn't seem to fit any of the ovoo types | |
| 19:21:19 | mriedem | as we're moving stuff from nova to ova | |
| 19:21:21 | mriedem | *ovo | |
| 19:21:52 | mriedem | so you have an Allocation object | |
| 19:22:00 | mriedem | which has a resource_provider and resources field | |
| 19:22:02 | cdent | edleafe: so you’re idea at this point is an object that is effectively; ‘target’: String, ‘allocation’: Dict | |
| 19:22:29 | cdent | could be moultiple providers | |
| 19:22:45 | edleafe | cdent: pretty much | |
| 19:22:46 | cdent | is the idea to maintain the opacity of the allocation request bits? | |
| 19:23:16 | edleafe | IMO, the allocation dict is already too much placement internals to be passed around in Nova | |
| 19:23:24 | edleafe | But everyone else seems fine with it, so... | |
| 19:24:04 | edleafe | cdent: and the example I pasted contains multiple providers. It will only have one hostname, though | |
| 19:24:10 | edleafe | We're still in nova-land | |
| 19:24:21 | cdent | I was responding to mriedem on the rp thing | |
| 19:24:47 | cdent | I’m ambivalent about the allocation requests being passed around. sailed ship | |
| 19:25:17 | mriedem | passing allocation requests dicts around within the scheduler, as we are today, seems fine | |
| 19:25:33 | mriedem | if you want to pass something structured back over rpc to conductor to use for alternatives, you can define an object for that | |
| 19:25:43 | mriedem | to avoid some list of 2-item tuples | |
| 19:25:46 | edleafe | mriedem: that's what I'm trying to do | |
| 19:25:59 | edleafe | mriedem: my question was what field type to use for the set of allocations | |
| 19:26:18 | edleafe | We don't seem to have a matching type for that kind of nested dict structure | |
| 19:26:33 | mriedem | well, you don't want to end up with a ListOfDictOfLists field | |
| 19:27:02 | edleafe | DictOfListOfDicts? | |
| 19:27:30 | mriedem | for http://paste.openstack.org/show/619670/ we're not going to just pass a top-level dict with a single "allocations" key are we? | |
| 19:27:48 | mriedem | as the outermost object | |
| 19:27:55 | mriedem | pass a list of allocation objects | |
| 19:27:57 | edleafe | mriedem: yes. This is the body that is needed to claim/unclaim a host | |
| 19:27:58 | artom | Remove the top-level 'allocation' and just use a ListOfObjectsField? | |
| 19:28:07 | mriedem | artom: +1 | |
| 19:28:22 | artom | I still think it depends on what you'll be doing with that | |
| 19:28:26 | edleafe | So the object will have to have a method for creating the POST body | |
| 19:28:36 | artom | As in, if the provider uuids are important at all, it could be a dict keyed on those | |
| 19:28:55 | edleafe | artom: this should be opaque. It is the body needed to claim/unclaim a selected host and its resources | |
| 19:29:20 | mriedem | creating an actual allocation request is pretty trivial https://review.openstack.org/#/c/496031/3/nova/conductor/tasks/live_migrate.py@122 | |
| 19:29:22 | artom | Completely opaque? Just a string then? | |
| 19:29:34 | mriedem | we're doing that when bypassing the scheduler and forcing a host for live migration ^ | |
| 19:29:35 | cdent | edleafe: should we fix this first: https://bugs.launchpad.net/nova/+bug/1708204 | |
| 19:29:36 | openstack | Launchpad bug 1708204 in OpenStack Compute (nova) "placement allocation representation asymetric on PUT and GET" [Wishlist,Confirmed] | |
| 19:29:39 | edleafe | sure, we can chop this up any way that fits ovo, but then it has be able to put back together | |
| 19:30:18 | edleafe | mriedem: that won't be as easy with shared/nested providers | |
| 19:30:41 | mriedem | yeah i understand | |
| 19:31:00 | mriedem | and per the todo in that patch, i'd like to still call the scheduler even if we're forcing the host during live migration | |
| 19:31:06 | edleafe | the whole point of returning these allocation dicts was to handle those more complex situations | |
| 19:31:46 | mriedem | we already have an AllocationCandidate object today right? | |
| 19:32:12 | mriedem | sorry, it's called AllocationRequest | |
| 19:32:53 | mriedem | which has a list of AllocationRequestResource | |
| 19:33:09 | edleafe | mriedem: I guess I'll try following that pattern | |
| 19:33:20 | mriedem | why not just use the same exact objects? | |
| 19:33:30 | mriedem | if that's exactly what you want for the structure? | |
| 19:33:36 | edleafe | it's not | |
| 19:34:05 | mriedem | ok | |
| 19:34:11 | edleafe | I need a thing that is a hostname, and the blob needed to claim/unclaim the resources for the particular request | |
| 19:34:49 | mriedem | "(2:22:02 PM) cdent: edleafe: so you’re idea at this point is an object that is effectively; ‘target’: String, ‘allocation’: Dict" | |
| 19:35:04 | edleafe | mriedem: yep | |
| 19:35:08 | mriedem | and then return a list of those | |