| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-11-27 | |||
| 14:53:05 | efried | gibi: I think what I'm asking (and maybe dansmith also could weigh in on this) is: In general if it's possible simply to use the flavor/image-meta to construct the request groups, like what these guys do: | |
| 14:53:05 | efried | https://opendev.org/openstack/nova/src/branch/master/nova/scheduler/utils.py#L141-L143 | |
| 14:53:05 | efried | ...then would there be any other reason to use RequestSpec.requested_resources instead? | |
| 14:53:26 | dansmith | efried: that was supposed to be a hack | |
| 14:53:53 | efried | dansmith: which was? the thing I linked? | |
| 14:53:55 | dansmith | because the requested_resources stuff wasn't ready for ... something | |
| 14:53:59 | dansmith | efried: doing it via the flavor | |
| 14:54:53 | gibi | efried: how do you use flavor/image-meta to construct the bandwidth related request groups? | |
| 14:55:14 | efried | gibi: oh, you wouldn't necessarily for bw, I don't know. | |
| 14:55:21 | dansmith | efried: https://opendev.org/openstack/nova/src/branch/master/nova/scheduler/request_filter.py#L188 | |
| 14:55:27 | efried | presumably that one is more involved, requires going back to neutron API etc. | |
| 14:56:01 | efried | dansmith: that makes it sound like using the *request spec* is the hacky one. | |
| 14:56:32 | efried | i.e. if we want to make sure we don't persist the "interpreted request groups", they're better off being put in the ResourceRequest constructor. | |
| 14:56:48 | dansmith | efried: it's using the *flavor* in the request_spec, instead of requested_resources | |
| 14:57:08 | gibi | efried: there is a gap, an itch, something. We have flavor to describe the nova related resource requst, we have the neutron port that describes the networking related resource request. And we need a common, internal place where we collect these. And that should be the RequestSpec. But today we have different ways to collect the resource request there | |
| 14:57:30 | dansmith | efried: there was a big thing when we merged that code, something that couldn't be done with requested_resources because of traits in the unnumbered group or some such, so that flavor-based approach was the workaround until that got resolved | |
| 14:57:38 | dansmith | efried: I'm sure mriedem remembers, but he's out | |
| 14:57:43 | gibi | we store a flavor in the RequestSpec but also a list of RequestGroups | |
| 14:58:40 | gibi | dansmith, efried: at the moment the requested_resources list does not have _all_ the request groups represented as resources reqeusted throigh the flavor are not added to the requested_resources | |
| 14:59:00 | gibi | so there is no unnumbered group there either | |
| 14:59:14 | dansmith | gibi: I don't remember the details, I could try to find that review | |
| 14:59:18 | efried | Right -- no matter what, ResourceRequest.__init__ is going to parse the flavor. | |
| 14:59:26 | dansmith | gibi: but we couldn't request the trait via requested_resources | |
| 14:59:58 | gibi | dansmith: yes, as you would need to add that to the unnumbered group but there is no unnumbered group in the requested_resources | |
| 15:00:24 | dansmith | ah okay I see | |
| 15:00:32 | gibi | efried: yes, at the end ResourceRequest.__init__ uses the flavor,image and the requested_resources to construc the a_c query | |
| 15:01:06 | dansmith | https://review.opendev.org/#/c/656413/2/nova/scheduler/request_filter.py@105 | |
| 15:01:38 | dansmith | anyway, since the (ab)use of the flavor is only transient, not stored or exposed anywhere, it makes it fairly obscure what is going on, IMHO | |
| 15:01:55 | gibi | https://github.com/openstack/nova/blob/23995b4f9945aa242f6a9e7bd5bc700c27f31eb2/nova/objects/request_spec.py#L491 | |
| 15:02:05 | gibi | here is a TODO that is hard to fix ^^ | |
| 15:02:19 | gibi | as per my mail http://lists.openstack.org/pipermail/openstack-discuss/2019-May/005920.html | |
| 15:02:51 | openstackgerrit | Merged openstack/nova stable/stein: Add --dry-run option to heal_allocations CLI https://review.opendev.org/693199 | |
| 15:07:33 | efried | it looks like | |
| 15:07:33 | efried | --> in _provision_instances we populate the RequestSpec.requested_resources, but only with part of the info available | |
| 15:07:33 | efried | --> from select_destinations we call resources_from_request_spec which | |
| 15:07:33 | efried | ---> calls ResourceRequest.__init__ with the RequestSpec, (that ignores the requested_resources and only processes the flavor/image) | |
| 15:07:33 | efried | ---> *then* appends RequestSpec.requested_resources into the ResourceRequest | |
| 15:07:34 | efried | then we call GET /a_c | |
| 15:08:32 | gibi | yepp that sounds like it | |
| 15:08:53 | efried | _provision_instances happens on the conductor side? | |
| 15:09:28 | gibi | I think it happens at the nova-api side | |
| 15:09:52 | efried | sorry, yeah, this whole thing happens on the controller side, duh. | |
| 15:10:21 | gibi | it is called from nova.api.openstack.compute.servers.ServersController.create during boot | |
| 15:38:54 | efried | gibi, bauzas, stephenfin: do y'all want to have a meeting tomorrow or should I cancel? (US folk will be out) | |
| 15:39:07 | stephenfin | I don't have anything to bring up | |
| 15:39:15 | gibi | me neither but I can be present | |
| 15:39:38 | efried | let me see if anyone has put topics on the agenda. If not, I'll cancel. | |
| 15:40:55 | efried | it looks like shilpasd wants to talk about disk provider stuff. | |
| 15:41:08 | gibi | I reviewd the sharing disk spec today it looks good to me | |
| 15:41:19 | gibi | but somebody else should check at least the API impact part | |
| 15:41:20 | stephenfin | Because it's stuck? | |
| 15:41:29 | stephenfin | Maybe we can unstick it now | |
| 15:41:39 | efried | I don't know. I saw gibi's review but haven't dug in | |
| 15:41:39 | gibi | the API impact part needs eyes I would say | |
| 15:42:01 | gibi | bauzas was involved but he is busy downstream recently | |
| 15:42:08 | efried | If nothing else it gives her a scheduled time to talk to a couple of cores. So, gibi would you mind running that meeting? | |
| 15:42:19 | gibi | efried: I can do that, yes | |
| 15:42:26 | efried | thank you sir. | |
| 15:53:38 | bauzas | gibi: efried: thanks | |
| 15:53:47 | bauzas | sorry for being busy at the moment :( | |
| 15:53:52 | gibi | bauzas: no worries | |
| 15:54:14 | efried | gibi, dansmith: stephenfin: can you see any problem with putting the appending of RequestSpec.resource_requests [1] directly into ResourceRequest.__init__ [2]? | |
| 15:54:14 | efried | [1] https://opendev.org/openstack/nova/src/branch/master/nova/scheduler/utils.py#L546-L551 | |
| 15:54:14 | efried | [2] https://opendev.org/openstack/nova/src/branch/master/nova/scheduler/utils.py#L60 | |
| 15:54:43 | efried | Since we're passing in the RequestSpec anyway, it makes sense to me to consolidate that. | |
| 15:55:32 | gibi | efried: that refactoring make sense to me | |
| 15:56:07 | gibi | efried: is it part of a bigger surgery series? | |
| 15:56:28 | efried | gibi: not specifically, no. It's a standalone "make this shit make more sense to efried" patch. | |
| 15:56:40 | gibi | efried: ack. go for it! | |
| 15:57:07 | melwitt | gibi: ack thanks | |
| 15:57:08 | sean-k-mooney | efried: sorry i was not following but im going to rework my prefilter patches soon | |
| 15:57:19 | sean-k-mooney | what is the tldr of what is beign changed | |
| 15:57:44 | sean-k-mooney | are we ditching adding the stuff to the falvor temportaly and moveing to use requested_resouces | |
| 15:57:51 | sean-k-mooney | which i know was the long term plan | |
| 15:58:20 | stephenfin | efried: Looks sane. I'm not sure why I didn't do that when I refactored all that so might be worth giving git-blame a quick whirl | |
| 15:58:32 | stephenfin | but otherwise LGTM | |
| 15:59:21 | efried | stephenfin: it makes sense why you didn't do it when you refactored ResourceRequest, because (code stack wise) resources_from_request_spec is not actually "near" ResourceRequest.__init__. | |
| 15:59:26 | efried | they're sort of cousins | |
| 15:59:51 | stephenfin | ah, makes sense | |
| 16:00:24 | efried | and theoretically there would have been more extensive test fallout. Though practically I'm not sure there will be any test impact, as the places that use ResourceRequest.__init__ either don't care about RequestSpec.resource_requests, or *should* care and are currently ignoring it. | |
| 16:00:25 | sean-k-mooney | efried: is the requested_resources in the request spec still just resocue request and not traits | |
| 16:00:45 | efried | sean-k-mooney: It's a list of RequestGroup | |
| 16:00:49 | efried | RequestGroup includes traits | |
| 16:01:01 | efried | well, theoretically it does. | |
| 16:01:07 | efried | I don't know if it actually does for bw | |
| 16:01:10 | efried | but it will for accels | |
| 16:01:29 | sean-k-mooney | right but can i create a resouce group with no resouce requests and just traits | |
| 16:01:41 | efried | um | |
| 16:03:11 | efried | I don't see any reason code-wise that that shouldn't work | |
| 16:03:24 | sean-k-mooney | so in my prefilter changes i have this add_trait funciton https://review.opendev.org/#/c/665775/21/nova/scheduler/request_filter.py@52 | |
| 16:04:12 | sean-k-mooney | which when i wrote it i wanted to refactor all the exsiting prefilters to use | |
| 16:04:23 | efried | sean-k-mooney: ...except that I don't think nova is yet using the placement microversion that allows a resourceless request group. | |
| 16:04:55 | sean-k-mooney | but at least in the prefilter case for image type or the device modeles which im addeing supprot for there are not resouce requests | |
| 16:04:58 | sean-k-mooney | right | |
| 16:05:02 | efried | So: I thought there might be some places in the nova side where we check and forbid resourceless request groups, but I don't see them on a quick skim | |
| 16:05:02 | efried | But: placement is going to bounce it if you try. | |
| 16:05:51 | sean-k-mooney | right so i woudl either have to continue to modify the flavor for the traits case so they get included in the unnumbered group | |
| 16:06:06 | sean-k-mooney | or we woudl need to increase the microverion we use | |
| 16:06:22 | openstackgerrit | Alexandre arents proposed openstack/nova stable/train: Do not update root_device_name during guest config https://review.opendev.org/696339 | |
| 16:06:44 | efried | which would be nbd, and we're going to want to do it very soon anyway. | |