| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-08-01 | |||
| 13:25:05 | mriedem | stephenfin: are you going to update this? https://review.openstack.org/#/c/477497/ | |
| 13:25:16 | mriedem | stephenfin: i've identified some missing nova CLI guides from the admin guide that are missing in there | |
| 13:25:32 | openstackgerrit | Jay Pipes proposed openstack/nova master: Test resize with placement api https://review.openstack.org/487958 | |
| 13:25:32 | openstackgerrit | Jay Pipes proposed openstack/nova master: placement: remove existing allocs when set allocs https://review.openstack.org/489273 | |
| 13:25:33 | openstackgerrit | Jay Pipes proposed openstack/nova master: remove source provider allocs in confirm_resize() https://review.openstack.org/488510 | |
| 13:27:00 | jaypipes | mriedem, bauzas, gibi: I think you'll want to take a look at the changes I made to the test above. The tests now fail -- with no code changes -- indicating that the faulty assertions that supposedly were verifying the existing bad behaviour were incorrect. | |
| 13:28:57 | jaypipes | mriedem, bauzas, gibi: specifically, after the changes to the test I made, the following fails: | |
| 13:28:58 | mriedem | if you didn't change any function then why would they start failing? | |
| 13:29:00 | jaypipes | 1250 # NOTE(danms): This is bug 1707071 where we've lost the entire | |
| 13:29:00 | openstack | bug 1707071 in OpenStack Compute (nova) "Compute nodes will fight over allocations during migration" [Medium,In progress] https://launchpad.net/bugs/1707071 - Assigned to Jay Pipes (jaypipes) | |
| 13:29:00 | jaypipes | 1251 # allocation because one of the computes deleted it | |
| 13:29:00 | jaypipes | 1252 self.assertEqual(0, dest_usages['VCPU']) | |
| 13:29:14 | jaypipes | mriedem: because the test assertion was wrong. | |
| 13:29:22 | mriedem | then why wasn't it failing before? | |
| 13:30:03 | jaypipes | mriedem: I believe because the test was incorrectly passing the source rp UUID when it was supposed to pass the dest rp UUID | |
| 13:32:01 | mriedem | jaypipes: so you did change something functional | |
| 13:32:05 | mriedem | on which line? | |
| 13:32:47 | gibi | jaypipes: could you point out the faulty line in the test in the previous patchset, in 1252 seems worked the same way before than now | |
| 13:33:31 | openstackgerrit | Merged openstack/nova master: add a redirect for the old cells landing page https://review.openstack.org/487932 | |
| 13:33:53 | openstackgerrit | OpenStack Proposal Bot proposed openstack/nova master: Updated from global requirements https://review.openstack.org/489604 | |
| 13:37:39 | jaypipes | gibi: your guess is as good as mine. :( | |
| 13:38:02 | jaypipes | mriedem: I did not make any functional changes... :( | |
| 13:38:52 | cdent | jaypipes, gibi : you two didn’t collide on your rebasing/patching of the test did you? | |
| 13:38:56 | jaypipes | gibi, mriedem: the only thing I changes was not using this: | |
| 13:38:56 | jaypipes | server['OS-EXT-SRV-ATTR:host'] | |
| 13:38:57 | mriedem | jaypipes: ok, i don't know what you're saying then. you didn't make any changes, but things are failing, but they weren't before | |
| 13:39:13 | jaypipes | cdent: no, I rebased on a clean version from gibi | |
| 13:40:24 | jaypipes | mriedem, gibi: I believe the problem is the original test was relying on server['OS-EXT-SRV-ATTR:host'] to get the server's host and determine what was the "source" or "target". That's incorrect though because that value changes over the course of the migration | |
| 13:40:57 | jaypipes | so when I remove that and just use constants for the source and destination hostname (and rp UUIDs), the tests fails. | |
| 13:41:22 | mriedem | sdague: did you see my comments in here about versioning https://review.openstack.org/#/c/467699/ ? | |
| 13:41:22 | jaypipes | this is also why I think dansmith was seeing non-deterministic issues with ordering. | |
| 13:41:42 | jaypipes | because the server['OS-EXT-SRV-ATTR:host'] would change at different times in the migration sequence. | |
| 13:42:14 | dansmith | jaypipes: we got the host once at the beginning though | |
| 13:43:35 | jaypipes | dansmith: no... it was grabbed again on previous line 1215 in gibi's new _resize_and_check_allocations() | |
| 13:45:18 | jaypipes | dansmith: well, it wasn't "grabbed again"... just read again from the server dict. but the server dict is passed to the _wait_for_status() thing. perhaps that dict is modified? | |
| 13:45:30 | dansmith | jaypipes: that function wasn't there when Ilast pushed | |
| 13:45:44 | jaypipes | dansmith: I know, it was gibi's overnight refactor. | |
| 13:45:53 | jaypipes | but the basic premise remains. | |
| 13:46:18 | dansmith | jaypipes: I don't think it does, | |
| 13:46:36 | dansmith | because we pulled host, even saved it in original_host, | |
| 13:46:41 | dansmith | and used the provider uuids once | |
| 13:46:46 | dansmith | that was mriedem's early comment | |
| 13:46:55 | jaypipes | dansmith: no, I'm right. bingo. The server parameter to _wait_for_state_change() is modified **in-place** and replaced with the returned value from the GET call. | |
| 13:47:03 | jaypipes | dansmith: and that is why server['OS-EXT-SRV-ATTR:host'] changes value. | |
| 13:47:19 | jaypipes | while True: | |
| 13:47:19 | jaypipes | 225 server = admin_api.get_server(server['id']) | |
| 13:47:26 | dansmith | oh you're right, so I should just stop looking then eh? | |
| 13:47:43 | gibi | jaypipes: can it be that your refactring changed what host order we skip and what host order we test now? | |
| 13:47:47 | jaypipes | well, I need food and more caffeine. will tackle this further when returning. | |
| 13:48:23 | mriedem | dan's point was with the way the test was written about 16 hours ago, | |
| 13:48:29 | mriedem | everything was monolithic, | |
| 13:48:34 | mriedem | and we stored the source host right up front | |
| 13:48:43 | jaypipes | gibi: no, my refactoring removed the use of server['OS-EXT-SRV-ATTR:host'] to get the resource provider UUIDs. that server['OS-EXT-SRV-ATTR:host'] changes over the course of the migration resulting in the wrong compute host being returned | |
| 13:49:04 | dansmith | jaypipes: right and that change is wrong | |
| 13:49:20 | dansmith | assuming that the rp uuid of the host we asked for is where it actually is is assuming too much, IMHO | |
| 13:49:33 | mriedem | https://review.openstack.org/#/c/487958/14/nova/tests/functional/test_servers.py | |
| 13:49:34 | jaypipes | dansmith: huh? | |
| 13:49:39 | gibi | interestingly if I set dest_hostname to host1 in the confirm test it passes | |
| 13:49:53 | jaypipes | gibi: and that is incorrect. | |
| 13:49:58 | mriedem | https://review.openstack.org/#/c/487958/14/nova/tests/functional/test_servers.py@1168 | |
| 13:50:18 | openstackgerrit | Jacek Tomasiak proposed openstack/nova master: ironic: Use internal API endpoint https://review.openstack.org/489537 | |
| 13:50:19 | mriedem | as of last night (me and dan time), we got the source host once at the beginning | |
| 13:50:25 | dansmith | jaypipes: I left a comment. However, I'm wrong and you're right, so I'm getting coffee and a bagel | |
| 13:50:38 | jaypipes | dansmith: ditto. | |
| 13:51:10 | mriedem | don't make me quote rodney king | |
| 13:52:01 | sdague | mriedem: apparently not, I didn't see any -1s on it. | |
| 13:52:02 | cdent | I often, in cases like this, wish we had to write new commits each time we pushed to gerrit | |
| 13:54:59 | mriedem | bauzas: i'm not sure why this is pike-rc-potential https://bugs.launchpad.net/nova/+bug/1702454 | |
| 13:54:59 | openstack | Launchpad bug 1702454 in OpenStack Compute (nova) "Transforming the RequestSpec object into legacy dicts doesn't support the requested_destination field" [High,In progress] - Assigned to Sylvain Bauza (sylvain-bauza) | |
| 13:55:05 | mriedem | it's not a regression in master, it's a latent issue in stable right? | |
| 13:55:36 | bauzas | mriedem: yup, like I said "As a consequence, the feature to pass a destination for evacuation is not working in Newton and Ocata. " | |
| 13:55:56 | bauzas | mriedem: I'd love to see it merged for Pike so we could backport it 'til Newton | |
| 13:56:12 | bauzas | if not, it would be a bit difficult to backport it | |
| 13:56:20 | mriedem | why? | |
| 13:56:28 | mriedem | pike GA != newton phase 3 | |
| 13:56:51 | mriedem | bauzas: so how about writing a functional regression test for https://review.openstack.org/#/c/481116/ then ? | |
| 13:57:09 | mriedem | because anything involving the request spec getting passed around 3 different services should have a functional test | |
| 13:57:33 | bauzas | mriedem: for evacuating ? | |
| 13:57:43 | bauzas | mriedem: not sure it would work for a functional test | |
| 13:58:18 | mriedem | i've been wanting to write a functional test for evacuate for a long time, | |
| 13:58:22 | mriedem | i don't think it would be that hard | |
| 13:58:35 | bauzas | mriedem: about why Pike, because https://docs.openstack.org/project-team-guide/stable-branches.html#support-phases | |
| 13:58:38 | mriedem | you start 2 services, create server on one, force it down and evacuate | |
| 13:58:45 | bauzas | mriedem: I can try | |
| 13:59:04 | mriedem | https://releases.openstack.org/ | |
| 13:59:12 | mriedem | Phase III – Legacy release on 2017-10-09 | |
| 13:59:27 | mriedem | you have 5 weeks to make it happen :) | |
| 13:59:45 | bauzas | okay okay :) | |
| 14:00:08 | bauzas | mriedem: just remove the pike-potential tag and I'll try to provide a functional test | |
| 14:00:24 | bauzas | mriedem: but then, I'll harass you :p | |
| 14:00:38 | mriedem | except !jk | |
| 14:00:43 | gibi | bauzas: there is example evacuate test in the server_group functional test I think | |
| 14:01:10 | bauzas | gibi: maybe, I'll look :) | |
| 14:01:18 | mriedem | bauzas: keep in mind, the way we do the functional regression tests is a 2 patch process, | |
| 14:01:22 | gibi | bauzas: https://github.com/openstack/nova/blob/master/nova/tests/functional/test_server_group.py#L412 | |
| 14:01:22 | mriedem | the first writes the test recreating the bug, | |
| 14:01:26 | mriedem | asserting the failure, | |
| 14:01:29 | mriedem | the 2nd patch fixes the bug | |
| 14:01:34 | mriedem | and adjusts the test | |
| 14:01:34 | bauzas | the functional server groups tests just do a lot so I'm not remembering if it's also calling evacuate :p | |