Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-03-06
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
22:06:43 dansmith melwitt: sean-k-mooney: that's whyfor we have careful review and don't just slam everything in, IMHO
22:07:55 sean-k-mooney change is scary? or so that we can all check our understading of the change and its wider effect so that changin a thing does not change other tings unless we ment too
22:10:04 dansmith sean-k-mooney: no, the fear of weird side effects part.. that's why review from a heterogenous pool of experienced reviewers is important
22:10:14 mriedem cfriesen: you mean this? "drop_move_claim looks up the new_flavor to compare it with the flavor that was used to track the migration, but on the source node it's certain to be the old_flavor. Thus as it stands now drop_move_claim (only ran on source nodes) doesn't do anything. Not a big deal, but we should probably fix it."
22:10:25 cfriesen mriedem: yeah
22:10:47 cfriesen I haven't checked whether it still applies
22:10:50 mriedem i don't think that applies anymore, i'm not really sure what it's talking about
22:11:13 mriedem when confirm_resize calls drop_move_claim, it's from the source and dropping usage for the old_flavor
22:11:18 sean-k-mooney dansmith: yes because we all have different experice and approce solutions in different ways so we tend to see different things.
22:11:51 mriedem when revert_resize (on the dest) calls rt.drop_move_claim, it's dropping the new_flavor usage from the dest host
22:13:20 mriedem anyway, the bug i reported is something i hit in my cross-cell resize functional tests for confirm resize and should be really easy to recreate in general
22:13:39 mriedem auto-confirm the resize and set the update_available_resource interval to a high number, like 10 minutes
22:16:31 cfriesen mriedem: I think you're right and it doesn't apply anymore.
22:17:20 sean-k-mooney dansmith: by the way the horrible code in https://review.openstack.org/#/c/624592/9/nova/network/linux_net.py
22:17:36 sean-k-mooney we have the same code more or less in os-vif https://github.com/openstack/os-vif/blob/master/vif_plug_linux_bridge/linux_net.py#L115-L153
22:20:42 sean-k-mooney we also have iptables code we inheritied form nova networks in the linux bridge plugin.
22:23:00 sean-k-mooney i wouldnt be too surpisidn if we could replace alot of this code wide a call to plug on the linux bridge driver
22:23:16 sean-k-mooney *surprised
22:27:16 mikal efried: I effectively did this sort of thing with the nova-net privsep thing. I'd much rather end up with mock decorators on the methods which actually need them. Otherwise its super hard to untangle what calls what when something fails. I can take a look at this later, but only if people say one nice thing about me each day. Today's topic can be how pleasing my hairstyling is. Go.
22:28:18 efried I saw a shampoo commercial the other day and thought of you.
22:29:42 efried mikal: Did you see conversation above where I said: this is stage 1, so we don't have to have "passthrough mocks" in the fake module. Stage 2-N might be to remove the remaining mocks one at a time, see which tests break, and explicitly mock the removed thingy in those tests.
22:29:58 mikal No not really. I was trying to remember how jeans work.
22:30:17 mikal I didn't read all the scrollback, it looked long and boring.
22:31:45 efried it was
22:31:51 sean-k-mooney mikal: fyi dansmith reorderd some of you privsep removal code so the 4 patchs with +w can merge without being blocked by https://review.openstack.org/#/c/624592
22:32:01 efried mikal: that's why I extracted the salient bit of it for you. So you wouldn't have to mess up your hair.
22:32:05 efried or... something.
22:32:30 mikal sean-k-mooney: careful. Some of us said regrettable things when we noticed that but hadn't yet had our valium for the day.
22:38:00 sean-k-mooney mikal: i assumed you would be happy that 4 more of them are currently makeing there way through the gate
22:38:34 sean-k-mooney mikal: but on a releated note we have similar code in os-vif which i kindof want to kill at some point once i figure out why we do it
22:38:35 sean-k-mooney https://github.com/openstack/os-vif/blob/master/vif_plug_linux_bridge/linux_net.py#L115-L153
22:49:55 efried mriedem: Are we waiting for train for https://review.openstack.org/#/c/633855/ series? (SEV stuff)
22:51:55 mriedem efried: yes
22:52:34 efried mriedem: procedural hold so I don't keep looking at it and trying to decide whether to approve it?
22:53:41 mikal Oh wow, this is terrible. Did you know we sometimes mock away the fake for that fake_libvirt_utils thing? Why? Who did this?!?
22:54:28 mriedem efried: i put a -W on the 2nd patch in the series which adds the capability which will now auto-generate the trait on the compute node resource provider,

Earlier   Later