| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-06 | |||
| 21:16:08 | efried | for some values of "PJs". | |
| 21:16:10 | dansmith | and for all we know, australian PJs might be like american birthday suits | |
| 21:16:18 | efried | swhat I'm sayin | |
| 21:16:30 | efried | ...with a pouch | |
| 21:17:35 | mikal | Look. I've only told one of you to fuck off so far today. I'm really not being _unusually_ offensive. | |
| 21:18:00 | mriedem | i was going to say "as is his wont" but figured that might offend him, but i see it wouldn't | |
| 21:19:28 | efried | We can hug it out, mikal, but let's get one thing perfectly clear: | |
| 21:19:31 | efried | No pants, no hug. | |
| 21:19:52 | mriedem | i'll hug a nude man | |
| 21:19:55 | mriedem | i don't care | |
| 21:20:08 | mriedem | i'm told it will be cold on the receiving end though | |
| 21:20:15 | dansmith | last week mriedem told me he prefers sweaty guys | |
| 21:20:27 | dansmith | I don't really like where this is going | |
| 21:20:59 | mriedem | i'll go back to the fire i just started in the tc channel | |
| 21:21:09 | dansmith | to warm your icy embrace? | |
| 21:28:44 | melwitt | dansmith: going through the change to add a test case for an instance mapping that has no instance (manual cleanup or purge) and was thinking, what should we do in that case? if we don't migrate the user_id, the record would keep getting found. should we migrate it to some sentinel value? or bother trying to get it from request spec? or other? | |
| 21:31:50 | cfriesen | are there docs on how to add new privsep commands in nova? | |
| 21:32:06 | efried | cfriesen: You are right on time, mikal just put on pants | |
| 21:32:18 | efried | if anyone has that answer, it's him. | |
| 21:32:36 | mikal | Well, I actually _left_ to put on pants, but you get the idea. | |
| 21:32:55 | mikal | cfriesen: do you want a pre-pants possibly grumpy answer, or a post pants probably still grumpy answer? | |
| 21:33:14 | cfriesen | :) | |
| 21:34:11 | mikal | cfriesen: https://www.madebymikal.com/adding-oslo-privsep-to-a-new-project-a-worked-example/ is an overly complete answer with way more detail than you care about. | |
| 21:34:11 | cfriesen | I'm easy | |
| 21:34:38 | mikal | cfriesen: but the short answer is its pretty trivial. Decide where in nova/privsep to put the method. Add the method with the decorator. Call the method from other places. Profit. | |
| 21:35:13 | mikal | cfriesen: the only real warts are around style (don't pass in command lines, pass in args that are used to generate a command line); and unit testing hoops if efried notices that test coverage has dropped. | |
| 21:35:30 | cfriesen | sweet, thanks. | |
| 21:35:45 | mikal | cfriesen: I'd be happy to do a worked example in a blog post, but you're the first person to ever ask because no one loves me and my life is meaningless. | |
| 21:36:20 | cfriesen | (I'm trying to resolve a problem with emulated TPM and cold migration...the software TPM daemon runs as root but nova doesn't, and I need to copy the root-owned file to the dest node.) | |
| 21:36:27 | mikal | cfriesen: although when I think of it, https://review.openstack.org/#/c/624593/ is probably close enough to documentation for now. Copy that. | |
| 21:36:54 | mikal | Oh, so you just want to read a file with escalated permissions? | |
| 21:36:58 | mikal | That's already implemented IIRC. | |
| 21:37:36 | mikal | cfriesen: http://git.openstack.org/cgit/openstack/nova/tree/nova/privsep/path.py#n27 is what you want to read a file as root. | |
| 21:37:48 | cfriesen | I suspect I need to modify SshDriver.copy_file() and RsyncDriver.copy_file() to optionally run with root privileges. | |
| 21:38:00 | mikal | cfriesen: with maybe a http://git.openstack.org/cgit/openstack/nova/tree/nova/privsep/path.py#n73 thrown in for the destination file. | |
| 21:39:03 | mikal | Ummmm. Why not just change the permissions on the file, sync, then change the permissions at the destination? | |
| 21:39:13 | mikal | Instead of running the sync as root? | |
| 21:40:17 | cfriesen | might work. It's tricky because root might not even be set up for passwordless ssh, and the resize code path doesn't do RPCs to set things up the way live migration does. | |
| 21:40:35 | cfriesen | still studying the flow to figure out how to make it work | |
| 21:41:47 | mikal | Ok, well I'm going to put on pants and stuff, but ping me if you need more random pointers. | |
| 21:50:21 | openstackgerrit | Eric Fried proposed openstack/nova master: WIP: Make fake_libvirt_utils DIAF https://review.openstack.org/641489 | |
| 21:50:31 | efried | mikal, mriedem: let's see how that goes ^ | |
| 21:50:45 | efried | it's stage 1 of ? btw | |
| 21:51:10 | efried | Probably better to explicitly mock only what's necessary to mock in only the places it's necessary. | |
| 21:51:17 | efried | But I probably *am* too lazy for that. | |
| 21:53:04 | dansmith | melwitt: I think we should include it in found, not in done, so it's clear that it's not migrated.. a sentinel doesn't make sense, IMHO, and it really should be cleaned up via archive/purge I think | |
| 21:53:20 | dansmith | melwitt: this is exactly why I wanted a test for it, so we make some decision and commit to it :) | |
| 21:53:47 | sean-k-mooney | so instead of mocking nova.virt.libvirt.driver.libvirt_utils you are now mocking a buch of functions in that module | |
| 21:53:54 | sean-k-mooney | efried: ^ | |
| 21:54:22 | efried | sean-k-mooney: right, instead of mocking a whole module with a whole module, I'm mocking the individual methods that are actually mocked. | |
| 21:54:31 | sean-k-mooney | most of which were just pass.... | |
| 21:54:40 | efried | which is fine | |
| 21:54:53 | sean-k-mooney | ya | |
| 21:55:11 | efried | sean-k-mooney: what pissed me off enough to do this was having to add new passthrough utils in fake_libvirt_utils at the same time as adding them to utils - see the predecessor patch. | |
| 21:55:22 | efried | this makes it so you don't have to do that. | |
| 21:55:27 | efried | which is a win. | |
| 21:55:42 | sean-k-mooney | why not mock them in the testfucntion where they are actuly needed | |
| 21:55:46 | efried | i.e. by default anything you write in utils is not mocked - but also won't break the world if you try to use it. | |
| 21:55:59 | sean-k-mooney | e.g. instaad of a blanket mock | |
| 21:56:04 | efried | Yeah, sean-k-mooney agree with that. Not having looked, that sounds like a huge effort. | |
| 21:56:10 | efried | I assume the test surface of libvirt is enormous | |
| 21:56:24 | efried | though from the little I've seen, we're nowhere near majority coverage. | |
| 21:56:33 | efried | I guess we have a 'cover' job to tell us that, huh | |
| 21:56:43 | sean-k-mooney | efried: you should assume that but it is not nessicaialy true. | |
| 21:57:11 | efried | in any case, yes, stage 2-N might be to remove the mocks one at a time, see which tests break, and explicitly mock the removed thingy in those tests. | |
| 21:57:13 | sean-k-mooney | well i guess we could do that first then localise the mocs | |
| 21:57:21 | efried | just so. | |
| 21:58:19 | mriedem | cfriesen: wrote this up based on what i pinged you about last night https://bugs.launchpad.net/nova/+bug/1818914 | |
| 21:58:20 | openstack | Launchpad bug 1818914 in OpenStack Compute (nova) "Hypervisor resource usage on source still shows old flavor usage after resize confirm until update_available_resource periodic runs" [Low,Triaged] | |
| 21:58:42 | melwitt | dansmith: oh, gotcha. consider it part of a "not completed" migration and operator must clean up to finish migration. yep, thank you | |
| 21:59:21 | sean-k-mooney | efried: also fun fact https://github.com/openstack/nova/search?q=assert_called_once&unscoped_q=assert_called_once | |
| 21:59:28 | melwitt | dansmith, mriedem: something that sucks that I realize now is that if we don't land the counting part of the series, we need to restore the policy change behavior thing to account for what to do with server creates in the face of down cells... | |
| 21:59:33 | dansmith | melwitt: yeah, you could also make a nova-status check for those I guess, and/or just do a warn-once on the migrate to say "yo dawg, this might never end if you don't do something" | |
| 21:59:38 | sean-k-mooney | i leaned yesterday that assert_called_once was added in py36 | |
| 21:59:44 | cfriesen | mriedem: oh yeah, meant to bring that up. | |
| 22:00:00 | mriedem | melwitt: i don't know that we *need* to | |
| 22:00:10 | mriedem | it's a latent issue since multi-cell support in pike right? | |
| 22:00:12 | dansmith | melwitt: we already changed that? but it's no different than if we land none of this right? | |
| 22:00:16 | mriedem | and counting quotas since pike | |
| 22:00:18 | efried | sean-k-mooney: Yes, in base python, BUT we use mock-the-lib, which had it added/backported to much earlier. | |
| 22:00:22 | melwitt | dansmith: ++ appreciate the ideas, as my brain is about fried | |
| 22:00:24 | dansmith | right, nothing has changed since _that_ point right? | |
| 22:00:40 | melwitt | mriedem: ok. I wasn't sure if it's need or not. ok | |
| 22:00:41 | efried | sean-k-mooney: I remember learning that a couple years ago when I freaked out about same. | |
| 22:00:42 | sean-k-mooney | efried: oh ok | |
| 22:01:24 | sean-k-mooney | efried: ya so i freaked out a bit yesterday when i found that neutron have a hacking check that prevent you from using ig | |
| 22:01:32 | sean-k-mooney | *it | |
| 22:01:36 | melwitt | dansmith: yeah... yeah, that's true, current behavior is to ignore cell timeouts and such | |
| 22:01:45 | melwitt | so it's no worse than today | |
| 22:01:56 | dansmith | yeah, I thought you were saying we had landed something very out of order | |
| 22:02:00 | dansmith | from where we are today, | |
| 22:02:08 | dansmith | landing the data bit won't change anything I don't think | |
| 22:02:13 | melwitt | no, just... I don't know, I was thinking did down cell support change something about that. but I don't think it did | |
| 22:02:23 | dansmith | I surely hope not | |
| 22:03:01 | melwitt | haha. just paranoid about things changing things | |
| 22:03:27 | sean-k-mooney | change is scary | |
| 22:04:27 | melwitt | it is :) especially in nova where you can change something and some weird side effect pops up somewhere else because of it | |
| 22:05:22 | cfriesen | mriedem: might want to check whether the last paragraph in https://bugs.launchpad.net/nova/+bug/1498126 is still applicable | |
| 22:05:23 | openstack | Launchpad bug 1498126 in OpenStack Compute (nova) "Inconsistencies with resource tracking in the case of resize operation." [Medium,Confirmed] | |
| 22:06:00 | cfriesen | mriedem: that bug is old enough that the code may be fixed by now | |