| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-03-06 | |||
| 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, | |
| 22:54:41 | mriedem | i avoided the -2 to not blow up jay's beautiful stats | |
| 22:54:51 | efried | mriedem: ack | |
| 22:54:53 | sean-k-mooney | mikal: do you know that we somtimes import the fake_libvirt direver for the unit tests in too fucntional tests | |
| 22:54:59 | mriedem | efried: as for the one below that, i don't have an opinion | |
| 22:55:07 | efried | mikal: You mean mock.patch(restore original method)? | |
| 22:55:20 | efried | that's pretty fantastic | |
| 22:55:32 | mikal | efried: no, its even more useless. We do a mock.patch to a noop for a function THAT IS ALREADY A NOOP. | |
| 22:55:39 | efried | wow | |
| 22:56:17 | sean-k-mooney | mikal: well actully that is proably new test that were written without knowing that ere was a global mock | |
| 22:56:45 | sean-k-mooney | so those test are actully proably mocking just the things they need to mock | |
| 22:56:49 | sean-k-mooney | which is good | |
| 22:57:24 | sean-k-mooney | the fact it was already mocked is wasteful but it does not hurt anything | |
| 22:58:14 | sean-k-mooney | by the way is there a reason the fake libvirt dirver is not with the other virt dirvers like the the Fake virt diriver is? | |
| 22:59:10 | mikal | sean-k-mooney: please don't ask logical questions, it just makes use feel even more inadequate | |
| 23:08:22 | mikal | Herm. Turns out we apply the fake_libvirt_utils fixture in many places it isn't even used. | |
| 23:08:33 | mikal | This rathole is nice. I like it. It feels like home. | |
| 23:23:36 | mikal | Wait what? nova.virt.libvirt.utils.file_open is definitely my favourite part of all this. | |
| 23:25:13 | cfriesen | mikal: did you intentionally omit "mv" or "cp" from privsep? | |
| 23:27:14 | mikal | cfriesen: privsep only has things which had callers. So an absence either means that the user hasn't merged yet, or there is no user in nova until you came along. | |
| 23:28:03 | openstackgerrit | melanie witt proposed openstack/nova master: Add user_id field to InstanceMapping https://review.openstack.org/633350 | |
| 23:28:04 | openstackgerrit | melanie witt proposed openstack/nova master: Populate InstanceMapping.user_id during migrations and schedules https://review.openstack.org/638574 | |
| 23:28:04 | openstackgerrit | melanie witt proposed openstack/nova master: Add online data migration for populating user_id https://review.openstack.org/633351 | |
| 23:28:05 | openstackgerrit | melanie witt proposed openstack/nova master: Add get_counts() to InstanceMappingList https://review.openstack.org/638072 | |
| 23:28:05 | openstackgerrit | melanie witt proposed openstack/nova master: Count instances from mappings and cores/ram from placement https://review.openstack.org/638073 | |
| 23:28:06 | openstackgerrit | melanie witt proposed openstack/nova master: Use instance mappings to count server group members https://review.openstack.org/638324 | |
| 23:29:02 | cfriesen | mikal: I need to copy a subtree under /var/lib/libvirt/swtpm/<uuid> and recursively chmod or chown it. On the dest I need to do the opposite. | |
| 23:29:30 | cfriesen | I'm wondering whether from a security standpoint it'd make more sense to create these under libvirt, or as generic operations in path.py | |
| 23:30:49 | mikal | cfriesen: I think I'd put it under path, it seems generically useful. | |
| 23:31:03 | mikal | cfriesen: there's no security difference between either location, its more about who uses it. | |
| 23:37:01 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Remove mox in virt/test_block_device.py https://review.openstack.org/566153 | |
| 23:37:33 | openstackgerrit | Takashi NATSUME proposed openstack/nova stable/queens: Add description of custom resource classes https://review.openstack.org/619125 | |
| 23:45:53 | openstackgerrit | melanie witt proposed openstack/nova master: Add online data migration for populating user_id https://review.openstack.org/633351 | |
| 23:45:54 | openstackgerrit | melanie witt proposed openstack/nova master: Add get_counts() to InstanceMappingList https://review.openstack.org/638072 | |
| 23:45:54 | openstackgerrit | melanie witt proposed openstack/nova master: Count instances from mappings and cores/ram from placement https://review.openstack.org/638073 | |
| 23:45:55 | openstackgerrit | melanie witt proposed openstack/nova master: Use instance mappings to count server group members https://review.openstack.org/638324 | |
| 23:52:24 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add functional recreate test for bug 1818914 https://review.openstack.org/641521 | |
| 23:52:26 | openstack | 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] https://launchpad.net/bugs/1818914 | |
| 23:52:26 | mriedem | cfriesen: ^ recreate test at least | |
| 23:59:43 | melwitt | mriedem: I think I'm not going to be able to be around tomorrow... :( on that note, I'm not sure how helpful it'd be to land the migration without the user-impacting changes, | |
| #openstack-nova - 2019-03-07 | |||
| 00:00:14 | melwitt | there's an open question on the bottom patch about what table index(es) we should add, need jay's input for that | |
| 00:01:11 | mriedem | melwitt: ok...who is dealing with final client release and any tags we need (don't think we do milestone tags for nova anymore) | |
| 00:01:47 | mriedem | shit this reminds me that we need a python-novaclient bump for 2.72 if we want to supported bw provider ports with nova boot... | |
| 00:01:49 | mriedem | gibi: ^ | |
| 00:02:02 | melwitt | mriedem: I hoped to get https://review.openstack.org/640898 out in the final osc-placement release. I can propose the novaclient release tonight, same with osc-placement if you think the allocation ratio change won't make it | |
| 00:02:36 | mriedem | i haven't reviewed that allocation ratio thing, it's late for that imo | |
| 00:03:04 | mriedem | the instance mapping user_id is valuable in stein if we can get it because it means less data to migrate in train | |
| 00:03:20 | mriedem | i'm also assuming cern is going to be backporting this stuff... | |
| 00:03:26 | efried | cfriesen: still around? | |
| 00:03:28 | melwitt | ok. that's too bad. cdent and I have tested it locally a bit and it has a func test, but yeah | |
| 00:03:28 | mriedem | i.e. the counting quota stuff from train to stein | |
| 00:03:46 | mriedem | well, it was posted 2 days ago so... | |
| 00:04:20 | mriedem | anyway, i can do a novaclient release tomorrow since we need 2.72 for gibi's stuff | |
| 00:04:28 | melwitt | yeah, I spent a long time writing the func test for it. I guess I could have uploaded it in two stages | |
| 00:04:32 | efried | cfriesen: going once... okay, I'm going to update the commit message on https://review.openstack.org/#/c/641480/ and +2 it. | |
| 00:04:34 | mriedem | and i'd like to ge the instance mapping user_id stuff in at least for the 'less data to migrate' thing | |
| 00:04:52 | openstackgerrit | Eric Fried proposed openstack/nova master: Add missing libvirt exception during device detach https://review.openstack.org/641480 | |
| 00:05:00 | melwitt | ok. I'll propose the osc-placement release tonight then | |
| 00:05:14 | melwitt | feel free to make whatever changes are needed to the counting quotas migration stuff tomorrow | |
| 00:05:28 | melwitt | (anyone feel free) | |
| 00:06:16 | melwitt | I guess the osc-placement thing isn't so big a deal because could have another release soon-ish and not hard for people to upgrade | |
| 00:06:28 | melwitt | client stuff is a lot easier | |