| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-03-24 | |||
| 15:00:23 | bauwser | johnthetubaguy: that's bad to what ? :) | |
| 15:00:29 | johnthetubaguy | bauwser: we need to update the group with the latest, but we don't want to do the boot time only checks | |
| 15:00:46 | bauwser | johnthetubaguy: we don't need to update the group info | |
| 15:00:56 | johnthetubaguy | bauwser: because of lazy loading? | |
| 15:00:58 | bauwser | johnthetubaguy: because the group info is made of 3 things | |
| 15:01:10 | bauwser | 2 can't be modified and the 3rd is lazy-loadable | |
| 15:01:11 | bauwser | right | |
| 15:01:22 | johnthetubaguy | but its saved into the json blob in the DB? | |
| 15:01:45 | bauwser | johnthetubaguy: if you see the code https://github.com/openstack/nova/blob/master/nova/conductor/tasks/live_migrate.py#L162-L163 you will see that I don't update the Spec with that updated group already | |
| 15:01:54 | johnthetubaguy | I thought we had a bug where that list needed to be updated, because we persisted and out of date list | |
| 15:02:23 | johnthetubaguy | s/and/an/ | |
| 15:03:00 | bauwser | johnthetubaguy: that was fixed by https://github.com/openstack/nova/blob/master/nova/objects/instance_group.py#L282-L289 | |
| 15:03:03 | johnthetubaguy | I don't think we need the boot checks, I just through the filter params would need to be populated correctly, I guess I would have to go read more | |
| 15:03:33 | bauwser | I'm going to cut that call, and you could be reviewing it | |
| 15:03:36 | johnthetubaguy | bauwser: I am not convinced, but I guess you want some functional tests for that stuff really, then we can answer whats correct | |
| 15:04:59 | bauwser | johnthetubaguy: https://github.com/openstack/nova/commit/c48c1098cbcd7a9c7980d5fbe4668c38b16767f5 was missing a functional test but fixed the problem you mentioned | |
| 15:05:29 | mriedem | yeah seriously i'd write functional tests for anything we're removing | |
| 15:05:32 | mriedem | before removing it | |
| 15:05:40 | mriedem | because the unit tests just don't cut it for conductor <> scheduler interactions | |
| 15:05:41 | johnthetubaguy | what mriedem said, its safest | |
| 15:05:49 | mriedem | as we've been seeing lately with a lot of regressions | |
| 15:06:23 | mriedem | i'm still working on one for another regression | |
| 15:07:18 | mriedem | ooo and i think my regression test stumbled on another bug | |
| 15:08:42 | mriedem | ha, the fake virt driver doesn't set the image size on snapshot | |
| 15:08:54 | mriedem | something like that | |
| 15:09:23 | bauwser | mriedem: okay fair enough | |
| 15:10:04 | mriedem | oh it's the api code, not the fake virt driver | |
| 15:10:05 | mriedem | wtf | |
| 15:10:48 | mriedem | https://github.com/openstack/nova/blob/master/nova/compute/api.py#L3355 | |
| 15:11:02 | mriedem | we don't set the size attribute when creating the image for a shelving operation? | |
| 15:11:06 | bauwser | mriedem: johnthetubaguy: which specific functional tests would you like to see covered ? | |
| 15:11:23 | bauwser | mriedem: johnthetubaguy: the fact that we get a fresh list of hosts every time? | |
| 15:11:24 | mriedem | bauwser: anything related to whatever you're removing, | |
| 15:11:32 | mriedem | which i guess is a resize/migrate with an instance in a server group? | |
| 15:11:42 | johnthetubaguy | yeah, that ^ | |
| 15:11:53 | cdent | leakypipes: I responded on your os-traits stuff. I hope that new comment makes what I mean a bit clear. Your response suggests that my initial comment missed you by a mile. | |
| 15:11:54 | bauwser | mriedem: that test is already in tree | |
| 15:11:58 | mriedem | bauwser: where? | |
| 15:12:09 | bauwser | functional/test_server_group out of mind | |
| 15:12:50 | bauwser | mriedem: but it's not covering the situation that I want to remove, ie. the fact that live-migration can fail if the filter is now disabled | |
| 15:13:22 | bauwser | mriedem: johnthetubaguy: https://github.com/openstack/nova/blob/master/nova/tests/functional/test_server_group.py#L367 | |
| 15:14:34 | bauwser | mriedem: johnthetubaguy: I could be writing a test basically identical to https://github.com/openstack/nova/blob/master/nova/tests/functional/test_server_group.py#L367 that would disable the anti-affinity filter and show that the the move would fail ugly | |
| 15:14:54 | bauwser | mriedem: johnthetubaguy: so that I could remove that specific test in the cleanup patch I'm doing | |
| 15:14:56 | bauwser | wfy ? | |
| 15:15:53 | johnthetubaguy | just want something to prove the migrate works, and picks the correct host, or fails correctly, with and without your patch | |
| 15:16:15 | johnthetubaguy | maybe we have it already, thats cool | |
| 15:16:37 | johnthetubaguy | I guess we should have a test that fails with your bug, and we see it being fixed, ideally | |
| 15:16:42 | bauwser | johnthetubaguy: that's covering the migrate action, not the live-migrate action | |
| 15:16:55 | johnthetubaguy | bauwser: so we should have coverage for both right? | |
| 15:17:06 | bauwser | johnthetubaguy: yeah | |
| 15:17:13 | bauwser | okay, lemme see what I can do | |
| 15:25:50 | mriedem | hrm, what sets image size when we create a snapshot? does glance do it? | |
| 15:26:00 | mriedem | i can't see during a snapshot flow where we actually pass the size value to glacne | |
| 15:26:01 | mriedem | *glance | |
| 15:27:43 | mriedem | ah it is glance https://developer.openstack.org/api-ref/image/v2/index.html?expanded=create-an-image-detail#images | |
| 15:28:01 | andreykurilin | mriedem: sorry, it looks like my IRC bouncer had crashed and stopped sending me any messages, so I missed your messages yesterday. | |
| 15:28:27 | mriedem | andreykurilin: that's ok, we just disabled the trigger crash dump functional tests in novaclient, | |
| 15:28:33 | mriedem | because they were timing out and causing the jobs to hang | |
| 15:28:50 | mriedem | andreykurilin: i feel like you had found or fixed something in those tests like that not too long ago, but can't recall | |
| 15:29:56 | andreykurilin | mriedem: I remember that we had similar problems before, but I can recall what was done to fix it | |
| 15:33:45 | openstackgerrit | Anthony PERARD proposed openstack/nova master: Add exclusion list for tempest for a libvirt+xen job https://review.openstack.org/449687 | |
| 15:38:19 | mriedem | anthonyper: some of that is old news | |
| 15:38:22 | mriedem | anthonyper: see d571e03b5b67a401e7774988a84df3bef16273ad | |
| 15:38:28 | mriedem | https://github.com/openstack/tempest/commit/d571e03b5b67a401e7774988a84df3bef16273ad | |
| 15:38:38 | mriedem | tempest doesn't hardcode vda anymore in test_volume_boot_pattern | |
| 15:39:31 | anthonyper | mriedem: Thanks, I'll tests that and change the patch. | |
| 15:40:31 | openstackgerrit | Sylvain Bauza proposed openstack/nova master: Fix live-migrate usage of RequestSpec https://review.openstack.org/449690 | |
| 15:41:35 | openstackgerrit | Anthony PERARD proposed openstack/nova master: Add exclusion list for tempest for a libvirt+xen job https://review.openstack.org/449687 | |
| 15:43:18 | mriedem | johnthetubaguy: just fyi i added L137 in here https://etherpad.openstack.org/p/BOS-TC-vm-baremetal-platform | |
| 15:43:38 | johnthetubaguy | mriedem: ah, cool | |
| 15:52:50 | cdent | alex_xu: I assume you're not around anymore today, but wanted to check before I do some rebasing of our combined stacks of code | |
| 15:54:57 | mriedem | johnthetubaguy: leakypipes: http://forumtopics.openstack.org/cfp/details/28 | |
| 15:55:04 | mriedem | that might need to be two separate sessions | |
| 15:55:10 | mriedem | jgriffith: ildikov: Kevin_Zheng: ^ | |
| 15:56:26 | sdague | johnthetubaguy: ok, I looked again at the new spec, I still thing you want to take it back even one step further. Let me throw a patch on top of it with an idea of tone. | |
| 15:56:55 | ildikov | mriedem: thanks | |
| 15:57:14 | mriedem | johnthetubaguy: sdague: since you guys are going to be decided forum sessions, let me know if i should split http://forumtopics.openstack.org/cfp/details/28 into 2 sessions | |
| 15:57:19 | mriedem | *deciding | |
| 15:57:29 | mriedem | if they are 40 minute blocks, 20 minutes per topic is not enough | |
| 15:57:44 | ildikov | mriedem: the plan is 40 minutes long sessions | |
| 15:57:55 | ildikov | mriedem: I would say let's split that to two | |
| 15:58:01 | sdague | mriedem: yeh, I'd suggest splitting | |
| 15:58:09 | mriedem | ok | |
| 15:58:29 | ildikov | mriedem: johnthetubaguy: jgriffith: I got pointed to a multi-attach related forum session proposal: http://forumtopics.openstack.org/cfp/details/17 | |
| 15:58:58 | mriedem | ildikov: yeah i already commented on it | |
| 15:59:00 | johnthetubaguy | mriedem: I don't get a vote in that, its some other sub team of the TC looking at that, I believe | |
| 15:59:36 | ildikov | mriedem: thanks | |
| 16:00:00 | johnthetubaguy | but yeah, better to split to start, and combine later if needed, I guess | |
| 16:00:28 | leakypipes | mriedem: agreed. | |
| 16:00:55 | mriedem | ok done http://forumtopics.openstack.org/ | |
| 16:02:06 | mriedem | huh, apparently if you rename it's a new topic | |
| 16:02:10 | mriedem | and you can't delete a topic | |
| 16:02:12 | mriedem | because now i have 3 | |
| 16:02:29 | mriedem | EmilienM: ttx: do you know how to delete http://forumtopics.openstack.org/cfp/details/28 ? | |
| 16:02:54 | EmilienM | mriedem: Edit and Delete | |
| 16:03:08 | mriedem | thanks | |
| 16:03:08 | mriedem | ah nice | |
| 16:03:48 | mriedem | leakypipes: ah i see you created http://forumtopics.openstack.org/cfp/details/26 so that probably covers what i was going to submit | |
| 16:03:51 | leakypipes | cdent: I'm sorry, I still don't understand what you're asking for. | |
| 16:04:08 | leakypipes | mriedem: ya | |