| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-02-15 | |||
| 15:58:21 | cdent | yes, definitely custom code should not happen to placement | |
| 15:58:31 | dansmith | cdent: that's not what he's saying here htough | |
| 15:59:12 | bauzas | dansmith: so, modularity is good, provided we don't open the entry points | |
| 15:59:24 | dansmith | cdent: nobody is talking about touching placement at all, he's talking about in this code that we use to query placement | |
| 15:59:26 | bauzas | dansmith: a big fat comment in the code would be fine for me | |
| 15:59:48 | dansmith | bauzas: we don't comment code that isn't pluggable so I don't see why this would be any different :) | |
| 15:59:49 | bauzas | yeah, what dansmith said | |
| 15:59:52 | cdent | ah, sorry, I misinterpreted "I meant custom code modifying behaviour of placement" | |
| 16:00:16 | dansmith | bauzas: I will have to say, that scheduler filters and these pre-filters would be the _best_ place to introduce custom behavior if I was an operator | |
| 16:00:21 | bauzas | dansmith: you surely imagine the creativity of people using our code :) | |
| 16:00:36 | cdent | presumably the only way to modify placement is by sending different queries and creating various entities. in which case it's not "custom" is it? | |
| 16:00:39 | dansmith | because placement is a solid interface and all this is doing is augmenting how we call it | |
| 16:00:42 | cdent | it's what it's for | |
| 16:00:47 | dansmith | cdent: exactly my point | |
| 16:00:50 | bauzas | the scheduler base filter is an external interface | |
| 16:01:01 | cdent | yay, agreeing with dan is fun | |
| 16:01:27 | bauzas | what would be the base pre-filter isn't an external interface and I'd vote -2 on any change that would open that | |
| 16:01:57 | dansmith | bauzas: why do you think the scheduler filters are an external interface? | |
| 16:02:09 | bauzas | dansmith: because it's pluggable :) | |
| 16:02:28 | bauzas | shit, meeting | |
| 16:02:29 | dansmith | bauzas: we don't load them externally anymore right? | |
| 16:02:46 | bauzas | oh hell, we still do that alot | |
| 16:02:54 | bauzas | unless I missed the bus | |
| 16:03:09 | bauzas | and cutting that flexibility would be a huge thing | |
| 16:03:11 | dansmith | where is the loading code for that? | |
| 16:03:37 | bauzas | sec | |
| 16:04:13 | dansmith | either way, the spec says I'm not even adding a list of pre-filters, but making each one a semantic config toggle | |
| 16:04:14 | dansmith | so it should be pretty clear we're not adding a new external interface here :) | |
| 16:04:57 | bauzas | dansmith: https://docs.openstack.org/nova/latest/user/filter-scheduler.html?#writing-your-own-filter | |
| 16:05:39 | dansmith | ah, I thought we removed the classloading from available_filters | |
| 16:05:54 | bauzas | no, we removed the possibility to hook a custom scheduler *driver* | |
| 16:06:02 | dansmith | yeah I remember that | |
| 16:06:19 | mriedem | no we didn't | |
| 16:06:23 | mriedem | that was reverted | |
| 16:06:24 | bauzas | okay, I thought we never agreed on removing the custom filter | |
| 16:06:24 | dansmith | well, anyway, this is not that :) | |
| 16:06:38 | bauzas | mriedem: orly? | |
| 16:06:48 | mriedem | yes you were on the reno for it | |
| 16:06:58 | bauzas | man, did I slept 100 years like the Sleeping Beauty ? | |
| 16:07:31 | bauzas | mriedem: on reverting the removal of the classloading ? | |
| 16:07:44 | bauzas | if so, I apologize for my memory lack | |
| 16:07:49 | dansmith | bauzas: I think he's talking about the driver pluggability | |
| 16:07:59 | mriedem | you can't classload the driver, but you can add an entry point for the scheduler driver in setup.cfg | |
| 16:08:12 | bauzas | righrt | |
| 16:08:50 | mriedem | https://github.com/openstack/nova/commit/1e5c7b52a403e708dba5a069dd86b628a4cb952c | |
| 16:09:19 | bauzas | seriously, I'm sometimes afraid of my memory lacks :/ | |
| 16:09:25 | bauzas | I totally forgot that story | |
| 16:09:51 | bauzas | apologies for that | |
| 16:10:10 | bauzas | it's 6 months ago and I don't recall it | |
| 16:10:12 | bauzas | creepy | |
| 16:10:50 | dansmith | so, um, | |
| 16:11:00 | dansmith | are we okay on this non-pluggable modular pre-filter thing or what? | |
| 16:11:04 | cdent | bauzas: too much chaud-verte (or was it verte-chaud or whatever)? | |
| 16:11:17 | bauzas | cdent: no, Chartreuse | |
| 16:11:24 | bauzas | dansmith: oui | |
| 16:11:36 | bauzas | dansmith: I'm currently re-reviewing your spec | |
| 16:19:06 | bauzas | cdent: edleafe: I mostly see your concerns by having a spec approved in 1 day, and pre-PTG | |
| 16:19:17 | bauzas | cdent: edleafe: please note that I don't feel it's a problem | |
| 16:19:35 | bauzas | we can approve a spec and amend it later based on feedback that can come from the PTG or elsewhere | |
| 16:19:54 | edleafe | bauzas: the concern was that a series of related specs came out all at once | |
| 16:20:02 | bauzas | or we can at the end of the day have a spec that is approved but leading to a dead-end, that's not a problem to me | |
| 16:20:10 | edleafe | At first I thought I missed the discussions about them | |
| 16:20:13 | dansmith | I don't think the concern is over PTG discussion, | |
| 16:20:39 | dansmith | we can't tie spec approvals to ptg discussion globally.. not everyone can or will go, nor will we have enough time for that | |
| 16:20:54 | edleafe | dansmith: agreed. | |
| 16:20:57 | bauzas | edleafe: the relationship between specs is surely a thing to consider, but which shouldn't hold an approval if that's not blocking (heh, tautology) | |
| 16:21:11 | bauzas | dansmith: sure, hence my "or elsewhere" | |
| 16:21:48 | edleafe | bauzas: it was because there were a few unstated assumptions, such as the idea that we would be syncing nova aggs to placement aggs, that were surprising | |
| 16:22:12 | dansmith | edleafe: I dunno why that is surprising, I feel like we've covered that multiple times | |
| 16:22:28 | dansmith | but that said, I'm totally cool with letting the ink dry a bit on things before they go in so all the timezones have a chance to comment | |
| 16:22:32 | edleafe | dansmith: it's possible, but I must have missed those discussions | |
| 16:22:46 | dansmith | I actually thought we already were doing that syncing, | |
| 16:22:56 | dansmith | because of the method for doing so in the scheduler client, which is apparently unused as of yet | |
| 16:22:58 | edleafe | dansmith: I still repeated the mantra "placement aggs are not nova aggs" | |
| 16:23:14 | dansmith | ...which is still true :) | |
| 16:24:33 | edleafe | now it's "placement aggs *are* nova aggs, and then some" | |
| 16:24:35 | edleafe | :) | |
| 16:24:44 | mriedem | fwiw i never thought we were syncing nova aggs to placement aggs, or intended to do so, before yesterday | |
| 16:24:57 | mriedem | edleafe: but they aren't | |
| 16:25:02 | mriedem | placement aggs don't have metadata | |
| 16:25:07 | mriedem | they don't have to exist for anyone outside of nova | |
| 16:25:18 | edleafe | mriedem: I don't mean they are exactly the same | |
| 16:25:34 | mriedem | i believe jay said they'd be a superset of nova host aggregates, which i think is correct | |
| 16:25:43 | edleafe | mriedem: just that if a compute node is in a nova agg, it will also be a matching placement agg | |
| 16:25:48 | dansmith | right, a superset | |
| 16:25:51 | mriedem | because my compute node providers could be in an aggregate that mirror a nova host aggregate, and also in a shared storage provider aggregate with cinder | |
| 16:26:14 | dansmith | aggregates are a thing that placement provides for consumers to group resource providers, | |
| 16:26:17 | bauzas | mriedem: edleafe: if we only keep placement aggregates as a bag of a collection of hosts, I'm fine with sync'ing that to nova | |
| 16:26:24 | edleafe | again, I don't think that this is a bad approach | |
| 16:26:38 | edleafe | it was just a big surprise | |
| 16:26:40 | dansmith | nova is a user of placement and thus would use it to augment its grouping when placing instances, | |
| 16:26:52 | bauzas | what I'm not okay is if placement aggs begin to have metadata information that carries some superseding logic | |
| 16:26:58 | dansmith | as I would expect neutron to create aggregates to mirror is grouping constructs when it helps to place things, cinder the same | |
| 16:27:03 | bauzas | yeah that | |
| 16:27:06 | mriedem | bauzas: no one said they would, and i'm sure jaypipes' head would explode | |
| 16:27:08 | mriedem | if we said that | |
| 16:27:17 | bauzas | mriedem: just stating loud things | |
| 16:27:41 | mriedem | maybe i should update https://review.openstack.org/#/c/539033/ | |
| 16:28:11 | bauzas | mriedem: please, CC'ing that change | |
| 16:28:36 | bauzas | since my brain is untrustable, I need to star things | |