| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-02-19 | |||
| 13:47:16 | aspiers | cdent: so this? https://pasteboard.co/I1Tbzp2.png | |
| 13:48:00 | openstackgerrit | Silvan Kaiser proposed openstack/nova master: Exec systemd-run without --user flag in Quobyte driver https://review.openstack.org/554195 | |
| 13:48:40 | tssurya | mriedem, dansmith: under which section of the admin docs would you prefer the down cell to be in ? or should I just start w new one? | |
| 13:49:09 | kaisers | stephenfin: Hi! FYI: Followed up on your comments on https://review.openstack.org/#/c/522245/ and https://review.openstack.org/#/c/554195/ | |
| 14:17:06 | openstackgerrit | Hamdy Khader proposed openstack/os-vif master: Add create_port field in VIFPortProfileOpenVSwitch profile https://review.openstack.org/636061 | |
| 14:17:08 | stephenfin | kaisers: Sweet. I've left comments on the systemd one. Might be as easy discuss here if you disagree | |
| 14:17:41 | stephenfin | kaisers: I'm well aware of how many revisions it's gone through too. Sorry :( | |
| 14:22:39 | kaisers | stephenfin: mmh, driver changes have outgrown the tests, I'm going through those... | |
| 14:32:43 | cdent | aspiers: was on a walk. parsing that image... | |
| 14:36:50 | mriedem | melwitt: so on https://review.openstack.org/#/c/633351/ you're picking up a marker instance record from an earlier data migration | |
| 14:36:55 | mriedem | and that marker instance doesn't have a user_id set | |
| 14:36:58 | mriedem | hence the infinite loop | |
| 14:37:04 | stephenfin | kaisers: Addressed the other one too https://review.openstack.org/#/c/522245/ | |
| 14:38:03 | kaisers | stephenfin: thnx! | |
| 14:38:30 | mriedem | melwitt: this is the migration that creates the marker instance with the 0'ed uuid and no user_id https://github.com/openstack/nova/blob/ab5a9bba31664e74c1e866c620b0d284d1d4f4cb/nova/objects/virtual_interface.py#L301 | |
| 14:39:45 | mriedem | there must also be a data migration that creates a marker instance mapping with the same 00000000-0000-0000-0000-000000000000 uuid | |
| 14:44:35 | aspiers | mriedem: please can you sanity check my understanding too? https://pasteboard.co/I1Tbzp2.png | |
| 14:45:45 | mriedem | the capabilities patch being the one you're trying to rebase? | |
| 14:45:50 | mriedem | then yes i think so | |
| 14:46:00 | aspiers | yes, that one | |
| 14:46:13 | aspiers | well, I've succeeded in rebasing it ;-) the struggle now is finishing off the tests | |
| 14:46:15 | mriedem | the compute "owns" the capabilities it reports, and if something external removes it then it will get put back on the next periodic task run | |
| 14:46:40 | mriedem | if an admin wants to schedule around hosts with a given capability, they use forbidden traits in their flavor | |
| 14:47:01 | aspiers | but the driver can still provide other traits outside this new capabilities-to-traits mechanism, right? like cpu flags etc. | |
| 14:47:14 | mriedem | correct | |
| 14:47:29 | aspiers | but it can also provide CUSTOM_* stuff which isn't capabilities? | |
| 14:47:41 | mriedem | the libvirt driver would be reporting traits that are both cpu flags and capabilities, like COMPUTE_SUPPORTS_MULTIATTACH or whatever | |
| 14:47:56 | mriedem | no i don't think so | |
| 14:48:21 | aspiers | is that COMPUTE_SUPPORTS_MULTIATTACH or CUSTOM_COMPUTE_SUPPORTS_MULTIATTACH? | |
| 14:48:33 | mriedem | not custom | |
| 14:48:38 | mriedem | it's a standard trait | |
| 14:48:42 | aspiers | ah OK | |
| 14:48:47 | mriedem | https://github.com/openstack/os-traits/tree/master/os_traits/compute | |
| 14:48:58 | openstackgerrit | Jan Gutter proposed openstack/os-vif master: Clean up versioned object backlevelling code https://review.openstack.org/630976 | |
| 14:49:00 | mriedem | https://github.com/openstack/os-traits/blob/master/os_traits/compute/volume.py#L24 | |
| 14:49:34 | mriedem | COMPUTE_TRUSTED_CERTS is another https://github.com/openstack/os-traits/blob/master/os_traits/compute/__init__.py#L19 | |
| 14:49:50 | aspiers | got it | |
| 14:50:07 | mriedem | the *only* driver i'm not sure about is ironic since i think nova proxies the traits from the ironic node to placement | |
| 14:50:13 | mriedem | i'm not sure if those are made CUSTOM_ | |
| 14:50:51 | mriedem | that happens here https://github.com/openstack/nova/blob/master/nova/virt/ironic/driver.py#L831 | |
| 14:50:59 | mriedem | looks like the traits on the node are just passed through as-is | |
| 14:51:02 | mriedem | so they can be standard or custom | |
| 14:51:07 | mriedem | jroll: ^ is that accurate? | |
| 14:51:30 | mriedem | https://developer.openstack.org/api-ref/baremetal/?expanded=show-node-details-detail#node-traits-nodes | |
| 14:51:39 | mriedem | "Traits can be either standard or custom." | |
| 14:52:46 | aspiers | mriedem: so the puzzle I'm currently facing with tests is this: I want to remove a CUSTOM_ trait in order to test that the periodic update replaces it correctly. But if I delete using the placement API fixtures then the trait is still in the provider tree, so it doesn't recognise that the traits are out of sync due to https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L1046 | |
| 14:53:22 | aspiers | if I comment out that return then it works, but I'm not sure if that will break something else | |
| 14:54:23 | jroll | mriedem: they can only be custom: https://developer.openstack.org/api-ref/baremetal/#node-traits-nodes | |
| 14:54:27 | jroll | er wait | |
| 14:54:32 | mriedem | so set_traits_for_provider is being called with the traits from the provider tree in the compute driver rather than the traits from the placement API? | |
| 14:54:34 | jroll | no, they can be both, you're right | |
| 14:55:34 | aspiers | mriedem: correct | |
| 14:55:51 | aspiers | (I think anyway; my head is beginning to hurt a bit at this stage) | |
| 14:56:07 | mriedem | aspiers: that sounds like an efried question | |
| 14:56:11 | aspiers | OK | |
| 14:56:12 | mriedem | and i'm about to jump on a call | |
| 14:56:38 | aspiers | OK np, thanks for the help | |
| 14:56:40 | mriedem | eric should be well rested after his presidents day celebrations | |
| 14:56:43 | efried | aspiers: gimme a sec | |
| 14:56:46 | aspiers | haha | |
| 14:56:59 | efried | yeah, it wasn't what you would call a relaxing day. | |
| 14:57:15 | mriedem | was someone out protesting the border wall again | |
| 14:57:49 | aspiers | %-/ | |
| 14:58:02 | efried | aspiers: should I read back or you wanna summarize? | |
| 14:58:13 | aspiers | I'll try to summarise | |
| 14:58:17 | efried | or that | |
| 14:58:26 | stephenfin | kashyap: What Georg Hoesch has pointed out here looks correct https://review.openstack.org/#/c/632507/12/nova/virt/libvirt/driver.py | |
| 14:58:34 | aspiers | it looks to me like set_traits_for_provider() is currently designed as a push from the provider tree to placement, rather than a bidirectional sync | |
| 14:58:35 | stephenfin | kashyap: If you agree, I'm happy to +2 as-is | |
| 14:58:44 | stephenfin | (Don't know how far you got in splitting up that patch) | |
| 14:59:07 | kashyap | stephenfin: Yeah, just read that. In the last 2 hours I came to the conclusion that we should untangle it | |
| 14:59:27 | kashyap | stephenfin: Phew. I was just about to admit myself to the "sanity ward" | |
| 14:59:29 | aspiers | :) | |
| 14:59:31 | efried | aspiers: true story. Up to now, virt drivers have been fully responsible for knowing what traits compute "owns". | |
| 14:59:42 | kashyap | stephenfin: That consoles stuff is mind-warping (to my weak brain, at least) | |
| 14:59:55 | aspiers | efried: so the puzzle I'm currently facing with tests is this: I want to remove a CUSTOM_ trait in test_servers.py in order to test that the periodic update replaces it correctly. But if I delete using the placement API fixtures then the trait is still in the provider tree, so it doesn't recognise that the traits are out of sync due to | |
| 14:59:56 | stephenfin | kashyap: It's a maze. So many conditionals | |
| 14:59:57 | aspiers | https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L1046 | |
| 14:59:58 | kashyap | And I'm glad I Cced Georg to also double-check. He writes thorough comments | |
| 15:00:03 | efried | aspiers: This will be (I think) the first time code outside of the virt driver, but inside n-cpu, will assert "ownership" of traits. | |
| 15:00:16 | stephenfin | kashyap: So, happy for me to go ahead and approve this as-is? | |
| 15:00:21 | kashyap | stephenfin: Yeah, did you see the reworked table? It was a cathartic relief to turn the tri-state to binary | |
| 15:00:24 | kashyap | stephenfin: Yes, please | |
| 15:00:50 | stephenfin | done and done | |
| 15:00:54 | efried | aspiers: okay, I understand the problem. Give us a tick to suss it out... | |
| 15:01:12 | kashyap | stephenfin: Excellent. | |
| 15:02:10 | kashyap | stephenfin: Unrelated: on the top-most patch, on that nuking of 'if' conditional from _create_pty_device() -- I noticed it, had a draft comment to myself, before I saw yours. | |
| 15:02:14 | kashyap | Will address that, too | |
| 15:02:48 | stephenfin | kashyap: ack | |
| 15:03:49 | aspiers | efried: this probably doesn't help, but this is how things work with the capabilities patch from mriedem / me: https://goo.gl/haARAc | |
| 15:04:14 | aspiers | I didn't get as far as modelling the sync with placement | |
| 15:04:27 | aspiers | because I didn't expect to need to change that | |
| 15:04:40 | efried | aspiers: Okay, so this is pretty tricky. We implemented a SIGHUP trap to trigger the report client to flush its cache and re-pull everything from placement. The operator is *supposed* to SIGHUP n-cpu any time they make an "out-of-band" change like the ones you're simulating here. | |
| 15:05:02 | efried | aspiers: So technically what you need to do is call ComputeManager.reset() before you hit the periodic and your test will work. | |
| 15:05:18 | aspiers | so that reset is more heavyweight than a periodic update? | |
| 15:05:22 | efried | aspiers: But it points to an interesting real-world issue, which is: what if the operator doesn't do that? | |
| 15:05:27 | aspiers | right | |
| 15:05:45 | efried | aspiers: Yes, it triggers the report client to blow away its provider tree cache completely, so on the periodic it will go re-retrieve everything. | |
| 15:05:58 | aspiers | do we really need that have_traits_changed() check? is it more than an optimisation? | |
| 15:06:37 | efried | it is exactly an optimiz^Hsation, so that we don't have to query placement every periodic as to whether things have changed. | |