| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-02-27 | |||
| 19:21:07 | jaypipes | mriedem: https://review.openstack.org/#/c/538498/ is showing it needs a rebase anyway. | |
| 19:21:39 | efried | artom: Anything that's a capability-based trait should indeed go into os-traits. Is that what you're saying? | |
| 19:21:45 | artom | efried, yeah | |
| 19:21:52 | efried | random driver-owned CUSTOM_ stuff notsomuch. | |
| 19:21:56 | mriedem | jaypipes: i didn't ask you about that | |
| 19:22:00 | artom | efried, exactl | |
| 19:22:01 | artom | y | |
| 19:22:16 | artom | It's a weird message we're sending to admins | |
| 19:22:24 | efried | jaypipes: that was me. aspiers is working on the rebase (and adding a test case). | |
| 19:22:36 | artom | "The driver owns this stuff, but also this other differently-named stuff that shares a namespace with the stuff you can set" | |
| 19:23:01 | efried | artom: Well, the namespace is COMPUTE_, not CUSTOM_ | |
| 19:23:12 | jaypipes | mriedem: sorry, meant efried :) | |
| 19:23:27 | efried | CUSTOM_ just means it's not in os-traits (yet) | |
| 19:24:03 | efried | artom: Where are you seeing a driver capability-based trait that's not in os-traits? | |
| 19:24:12 | efried | Or are you just concerned about the fact that we're testing that scenario? | |
| 19:24:28 | artom | efried, IMAGECACHE? | |
| 19:25:30 | efried | mm. mriedem said "optionally" at https://review.openstack.org/#/c/538498/19/nova/virt/driver.py@143 -- whyzat? | |
| 19:26:36 | efried | Trying to think of a scenario (heterogeneous levels of nova-compute / os-traits in a cloud, upgrade, something) where we would *need* to support a custom version of a driver capability trait. | |
| 19:26:48 | mriedem | let me remember something from over a year ago... | |
| 19:27:19 | efried | iow could we just make L1033-5 an exception? | |
| 19:27:40 | mriedem | not all capabilities would have traits, | |
| 19:27:43 | mriedem | like has_imagecache | |
| 19:27:53 | efried | okay, but why does has_imagecache not have a trait? | |
| 19:27:57 | mriedem | but your question is if it's in CAPABILITY_TRAITS_MAP should it also be a standard trait | |
| 19:28:02 | efried | right | |
| 19:28:04 | efried | always | |
| 19:28:07 | mriedem | we don't schedule based on has_imagecache | |
| 19:28:32 | efried | then why are we bothering to add it as a trait? | |
| 19:28:45 | efried | and, who says we wouldn't schedule based on has_imagecache? | |
| 19:28:56 | mriedem | i am | |
| 19:29:00 | mriedem | i am saying that right now | |
| 19:29:10 | efried | Maybe I have a flavor that favors nodes with imagecache | |
| 19:29:16 | mriedem | the day we schedule based on has_imagecache is the day i go work at walgreens | |
| 19:29:34 | efried | then wy are we bothering to add it as a trait? | |
| 19:29:38 | efried | on the compute provider | |
| 19:30:04 | mriedem | you're talking about L1034 right? | |
| 19:30:11 | efried | yes | |
| 19:30:17 | mriedem | gdi, did i write that code? | |
| 19:31:20 | artom | That should be, like, our motto. | |
| 19:31:21 | efried | yes, it is present in PS7 (though with normalize_name done manually, tsk) | |
| 19:32:08 | efried | as well as the corresponding comment on L1023 | |
| 19:32:08 | mriedem | looking at https://review.openstack.org/#/c/538498/7/nova/virt/driver.py | |
| 19:32:26 | mriedem | i was probably doing it because supports_trusted_certs wasn't in os-traits at the time | |
| 19:32:33 | mriedem | and didn't think about how that would generate CUSTOM_HAS_IMAGECACHE | |
| 19:32:43 | mriedem | and i think https://review.openstack.org/#/c/538498/7/nova/virt/driver.py@145 | |
| 19:32:53 | mriedem | relates to something i asked jaypipes when he started adding these to os-traits | |
| 19:33:02 | mriedem | so let me dig that up and i can redirect you to jaypipes | |
| 19:33:16 | efried | buck about to be passed | |
| 19:33:51 | mriedem | https://review.openstack.org/#/c/546713/ | |
| 19:33:51 | jaypipes | damn | |
| 19:33:53 | efried | I'm not really concerned about whodunit; what's the right thing here? Should we make it a rule that compute driver capability traits must be standard, and anything that's not will be ignored? | |
| 19:33:56 | mriedem | "I don't seehas_imagecachesupports_recreatesupports_migrate_to_same_host" | |
| 19:34:01 | mriedem | "None of those things are capabilities that a flavor or image would require." | |
| 19:34:33 | efried | that conversation sounds familiar | |
| 19:34:43 | efried | I was apparently convinced enough to +2 the thing. | |
| 19:34:52 | mriedem | "Eric Fried Mar 29, 2018 Patch Set 1: Code-Review+1Okay, I'm convinced we don't need the missing capabilities as traits. Thanks for your patience, all." | |
| 19:35:01 | efried | artom signed off on it too. | |
| 19:35:42 | efried | so, fine, we decided we didn't need them to be standard, and the reason was because we didn't think they would need to be scheduled to. So the question remains: do we need to put them onto the compute node as traits? (Given ^ they would be CUSTOM_ fo sho) | |
| 19:35:42 | mriedem | i think i would agree that if a compute driver capability is going to be reported as a trait, it should probably be a standard trait | |
| 19:35:49 | artom | efried, yeah, but that's like saying a kid ate some candy ;) | |
| 19:36:07 | artom | And in my defense, 'twas me who started this whole kerfuffle we're in now | |
| 19:36:27 | efried | yup, so it's only fair that the blame ended up back atcha. | |
| 19:36:54 | artom | But... was it a *useful* kerfuffle? | |
| 19:36:56 | mriedem | especially because if we added a new driver capability, like supports_trusted_certs, and before it's standard we report CUSTOM_SUPPORTS_TRUSTED_CERTS and then we have a standard trait and it becomes COMPUTE_TRUSTED_CERTS, that's going to be a weird situation for anyone building flavors requiring the former | |
| 19:37:07 | artom | Or did we just re-hash a past discussion to arrive at the same conclusion? | |
| 19:37:22 | efried | artom: No, this is a slightly new discussion and a decision that needs to be made. | |
| 19:37:29 | efried | but we did have to rehash the old discussion as a prerequisite :) | |
| 19:37:34 | artom | \o/ I'm useful! | |
| 19:37:38 | mriedem | i kind of also wish we weren't discussing the design of this ~1 week from feature freeze | |
| 19:37:49 | artom | Sorta | |
| 19:37:51 | efried | mriedem: Yes, there should be no reason we need to introduce a standard trait but wait to use it in nova. | |
| 19:37:54 | mriedem | because i'm going to say "sure whatever you want idk anymore" | |
| 19:38:02 | mriedem | and in a year we'll be saying "wtf did we do this?" | |
| 19:39:17 | efried | jaypipes: Do you agree with mriedem? To paraphrase: If we're going to tag the compute RP with a driver capability trait, it should be a standard trait; and conversely we should not tag the compute RP with a driver capability trait that is not standard; and therefore we should never tag the compute RP with a CUSTOM_* capability trait. | |
| 19:39:36 | jaypipes | reading... | |
| 19:40:06 | efried | (note use of 'capability' throughout - obviously the compute RP can get CUSTOM_* traits from elsewhere - admin, update_provider_tree, etc.) | |
| 19:42:10 | jaypipes | efried, mriedem: I feel the same as I did back then. If the thing is a capability of the compute node that an end user could feasibly want to influence scheduling, it should be a standardized os-trait. if it's just an internal implementation of the virt driver that isn't something an end-user would ever care about but is important for the virt driver infrastructure, just make it a simple attribute of the virt driver class. | |
| 19:43:01 | efried | okay, I'll mark up the review for aspiers | |
| 19:43:28 | mriedem | jaypipes: yeah i agree | |
| 19:43:45 | mriedem | as for why the patch was written how it was when i did a year ago, i can only guess (and already did) | |
| 19:43:49 | jaypipes | in the case of trusted certs, if this is something an end user will say "get me on a host that supports trusted certs", then it should be in os-traits as COMPUTE_TRUSTED_CERTS or whatever. | |
| 19:44:00 | mriedem | i believe it is | |
| 19:44:10 | jaypipes | and it sounds like end users do want that (via a flavor of course) so yeah. | |
| 19:44:20 | mriedem | https://github.com/openstack/os-traits/commit/3e0a6ea4f5045e0bdfe5a2e8097a184ba2c93c5a#diff-98123dc3728cad793e21fa497cf05723 | |
| 19:44:23 | mriedem | it's not really the end user | |
| 19:44:28 | mriedem | it's not something on a flavor or image | |
| 19:44:43 | mriedem | it's that the user says 'i want you to honor my request' and we send you to some compute that has no f'ing clue what that thing is | |
| 19:45:09 | mriedem | i.e. placement request filters | |
| 19:45:16 | mriedem | same for multiattach volumes and device tags | |
| 19:45:21 | jaypipes | that's scheduling though. :) | |
| 19:45:31 | jaypipes | but yes. | |
| 19:46:02 | mriedem | my point was just trusted certs are not a flavor/image thing | |
| 19:46:11 | mriedem | you can't build aggregates around them | |
| 19:47:13 | jaypipes | ack | |
| 19:48:41 | mriedem | efried: aspiers: ok documented https://review.openstack.org/#/c/538498/19/nova/virt/driver.py@1034 | |
| 19:49:23 | efried | mriedem: nice, race condition, correctly assumed "furiously" | |
| 19:52:33 | melwitt | mriedem, dansmith: thanks for the comments on the InstanceMapping.user_id patch. I just replied to mriedem. dansmith, build request doesn't have a user_id field, and request spec has had the user_id field only since rocky. do you think that's ok to rely on? I had been thinking we have to account for request specs older than rocky, but maybe that doesn't make sense because we're trying to account for old instance mappings that are in | |
| 19:52:33 | melwitt | the middle of being scheduled (and something older than rocky should be still in the middle of being scheduled) | |
| 19:52:57 | melwitt | *shouldn't | |
| 19:53:22 | dansmith | melwitt: right, it seems only worse to rely on something that has only existed since stein, but I thought it was on reqspec from well before that | |