| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-11-27 | |||
| 15:54:14 | efried | [1] https://opendev.org/openstack/nova/src/branch/master/nova/scheduler/utils.py#L546-L551 | |
| 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: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 | But: placement is going to bounce it if you try. | |
| 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: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. | |
| 16:06:55 | efried | but hm, I don't see which microversion added support for resourceless request groups. | |
| 16:07:18 | sean-k-mooney | nbd? | |
| 16:07:27 | efried | (no big deal) | |
| 16:07:33 | sean-k-mooney | oh ok cool | |
| 16:07:41 | sean-k-mooney | also i should be in a meeting so brb | |
| 16:07:44 | efried | 1.35 supports root_required, which is actually the place we should be doing traits like that. | |
| 16:08:06 | sean-k-mooney | ok ill read up on that | |
| 16:08:29 | sean-k-mooney | i just want to make sure i can refactor my stuff to work with what you are doining when i start on it again | |
| 16:09:43 | efried | so yeah, this appears to have happened https://docs.openstack.org/placement/latest/specs/train/implemented/2005575-nested-magic-1.html#resourceless-request-groups but I can't figure out which microversion we did it in. | |
| 16:10:10 | efried | ah, looks like it's part of the same_subtree microversion. | |
| 16:10:31 | efried | which is 1.36 | |
| 16:10:40 | efried | but for your purposes, root_required might be better | |
| 16:11:01 | efried | in fact, for many things where we're currently just using the unnumbered request group, we should be using root_required instead. | |
| 16:11:16 | efried | that may even let gibi resolve that TODO he mentioned earlier... | |
| 16:16:02 | gibi | efried: interesting. It would remove the motivation (e.g. the pain) to try to do the bigger surgery discussed in the ML post linked above | |
| 16:16:17 | efried | pretty sure that's why we did it in the first place. | |
| 16:18:13 | gibi | I'am totally not against using root_required | |
| 16:18:16 | gibi | for the prefilters | |
| 16:24:36 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Mask the token used to allow access to consoles https://review.opendev.org/220622 | |
| 16:24:37 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Extend NeutronFixture to handle multiple bindings https://review.opendev.org/696246 | |
| 16:26:01 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Do not mock setup net and migrate inst in NeutronFixture https://review.opendev.org/696247 | |
| 16:30:25 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Support live migration with qos ports https://review.opendev.org/695905 | |
| 16:30:37 | efried | gibi: do bw requests have traits today? | |
| 16:31:12 | gibi | efried: yes they have | |
| 16:31:17 | gibi | custom traits | |
| 16:31:53 | efried | cause there's a few paths that were using ResourceRequest.__init__, without processing RequestSpec.requested_resources, in order to accumulate all the traits for a request. | |
| 16:32:10 | efried | That seems like a hole, but one that probably doesn't matter; I need to look closer at what those traits were being used for. | |
| 16:32:17 | gibi | exampl of a port with resource request http://paste.openstack.org/show/786810/ | |
| 16:33:18 | gibi | if you see a possible bug then link me to the holes and I will try to help | |
| 16:34:06 | efried | okay, example, isolate_aggregates collects all the traits | |
| 16:34:32 | efried | which means if you wanted to isolate based on a trait in a bw request, it wouldn't work before (but will once I push this "refactor") | |
| 16:36:33 | gibi | ohh | |
| 16:36:41 | gibi | what an interesting feature interaction | |
| 16:38:01 | efried | I guess I should cover that in test, huh. | |
| 16:39:03 | gibi | logically it is a bug but from end user perspective I don't see why the admin would want to isolate aggregates based on physnet or vnic type | |
| 16:39:13 | efried | right. | |
| 16:39:17 | gibi | as those are the two traits that could come today from ports | |
| 16:39:29 | efried | the only other example is libvirt driver accumulating cpu flags, which *should* never exist on a bw request | |
| 16:39:51 | efried | or an accel request?? maybe?? | |
| 16:40:27 | gibi | yeah a bw request will not have cpu flag traits | |
| 16:43:01 | openstackgerrit | Alexandre arents proposed openstack/nova stable/stein: Do not update root_device_name during guest config https://review.opendev.org/696351 | |
| 16:44:42 | openstackgerrit | Alexandre arents proposed openstack/nova stable/rocky: Do not update root_device_name during guest config https://review.opendev.org/696353 | |
| 16:46:25 | openstackgerrit | Eric Fried proposed openstack/nova master: Process requested_resources in ResourceRequest init https://review.opendev.org/696354 | |
| 16:46:38 | efried | gibi, stephenfin: ^ | |
| 16:49:37 | gibi | efried: ack | |
| 17:03:31 | stephenfin | efried: Care to send this docs patch on its way? https://review.opendev.org/#/c/686817 | |
| 17:03:37 | efried | ... | |
| 17:04:57 | stephenfin | I chose you by lucky dip, if that makes you feel better | |
| 17:05:11 | stephenfin | I imagine it does | |
| 17:20:45 | efried | got it. And I was folded funny, so I got picked. | |
| 17:21:04 | efried | btw, '...' is "on it, please stand by". | |
| 17:21:11 | efried | dunno why so many people are confused by that. | |
| 17:40:55 | efried | stephenfin: +2, couple of typos, I'll fast approve if you want to respin for those. | |
| 17:41:26 | efried | afaict no conflicts on the subsequent patches, so no need to rebase them | |
| 17:43:45 | efried | did I miss you? :( | |
| 17:48:29 | sean-k-mooney | maybe ill ping him on watsap an tell him to check irc but he normally heads home around now | |
| 18:11:36 | efried | dansmith: do we need RPC version bump when adding a method to an OVO? | |
| 18:11:46 | dansmith | no | |
| 18:12:00 | efried | cool, thx | |
| 18:12:07 | dansmith | efried: if it's remotable, you need a version bump on the object, but neither affect a regular rpc interface | |
| 18:12:22 | efried | if the *method* is remotable? | |
| 18:12:36 | dansmith | only methods are remotable, so.. yes | |