| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-09-25 | |||
| 13:39:48 | efried | okay, never mind what I said about the existing ones failing. They're not. | |
| 13:47:32 | openstackgerrit | Matthew Booth proposed openstack/nova master: Raise error on timeout in wait_for_versioned_notifications https://review.openstack.org/604859 | |
| 13:49:14 | mdbooth | efried: Incidentally, the other possibility is that in some of these tests the notification has never been emitted | |
| 13:49:29 | mdbooth | Because that would previously have been silently ignored | |
| 13:51:14 | efried | yup | |
| 13:51:35 | mdbooth | I've just thrown that patch back into the gate with fixes, but it's going to fail again | |
| 13:51:50 | efried | mdbooth: But IMO if we want that to be okay, we should explicitly try/except+ignore the wait_for_notifications call in the test case. | |
| 13:52:02 | mdbooth | efried: ack, for sure | |
| 13:52:33 | efried | so ++ to your change, at least in principle :) | |
| 14:08:09 | mdbooth | efried gibi: In the case of nova.tests.functional.regressions.test_bug_1735407.TestParallelEvacuationWithServerGroup.test_parallel_evacuate_with_server_group the test appears to be incorrect | |
| 14:08:38 | mdbooth | It's calling evacuate on 2 instances, one of which is going to fail due to anti-affinity | |
| 14:08:57 | mdbooth | It's then asserting it got 2 instances of instance.rebuild.start | |
| 14:09:14 | mdbooth | Problem is that request validation happens before that notification is sent | |
| 14:09:26 | mdbooth | So it has never received those notifications | |
| 14:09:56 | mdbooth | However, it does sound wrong that the notification isn't emitted first | |
| 14:09:57 | gibi | mdbooth: doesn't this test want to assert that the late validate of the server groups catches the parallel evacuation? | |
| 14:10:29 | mdbooth | I believe so, yes | |
| 14:11:02 | mdbooth | However, I think it's expecting the start notification to be emitted before the failure occurs | |
| 14:11:06 | mdbooth | Which imho isn't unreasonable | |
| 14:11:15 | mdbooth | But that's not the reality | |
| 14:12:33 | mdbooth | See ComputeManager._do_rebuild_instance | |
| 14:12:43 | mdbooth | We call _validate_instance_group way at the top | |
| 14:12:58 | mdbooth | Notification is emitted below that | |
| 14:15:34 | gibi | mdbooth: ohh I see now. So we can expect one rebuild.start but the second evac will never reach that point | |
| 14:16:36 | mdbooth | gibi: ack | |
| 14:17:06 | gibi | mdbooth: I thin it is OK to change that to wait only for a single notification | |
| 14:17:38 | gibi | mdbooth: as the two self._wait_for_migration_status(server1, ['done', 'failed']) calls will make sure that the test waits for the evac to finish/fail | |
| 14:28:24 | mnaser | https://review.openstack.org/#/q/I811e84af46d678c3fdbf94ee400eabe659fc3d4e if anyone wants to continue with the backporting +2s | |
| 14:31:15 | mriedem | done | |
| 14:31:58 | mdbooth | mriedem: I messed with your script again, btw. Still waiting to see how it gets on in the gate. | |
| 14:33:59 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: consumer gen: move_allocations https://review.openstack.org/591810 | |
| 14:34:21 | gibi | mriedem: hi, do you have anything for the notification meeting? | |
| 14:34:22 | mriedem | mdbooth: ok, there is a bug in there | |
| 14:34:24 | mriedem | comment inline | |
| 14:34:27 | mriedem | gibi: nope | |
| 14:34:40 | mdbooth | mriedem: Not unexpected :) | |
| 14:34:44 | gibi | mriedem: then there will be no meeting today | |
| 14:34:50 | mdbooth | mriedem: Are you able to run this locally, btw? | |
| 14:35:00 | mriedem | mdbooth: i don't have a setup for it atm so no | |
| 14:35:09 | mriedem | i could, but don't right now | |
| 14:38:21 | gibi | mriedem, jaypipes: I think https://review.openstack.org/#/c/591597 is good to go now | |
| 14:40:40 | mriedem | looking | |
| 14:46:47 | mriedem | +W | |
| 14:48:03 | gibi | mriedem: thanks | |
| 14:48:29 | gibi | mriedem: the next 3 patches also ready from my perspective | |
| 14:50:00 | gibi | mriedem: the rest is still in my queue | |
| 14:50:58 | openstackgerrit | caoyuan proposed openstack/nova master: Option "scheduler_default_filters" is deprecated. https://review.openstack.org/604148 | |
| 14:51:11 | mriedem | ok | |
| 14:54:13 | openstackgerrit | Matthew Booth proposed openstack/nova master: Add volume-backed evacuate test https://review.openstack.org/604397 | |
| 14:55:17 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Option "scheduler_default_filters" is deprecated. https://review.openstack.org/604148 | |
| 15:00:07 | openstackgerrit | Merged openstack/nova master: api-ref: Fix wrong bold decoration https://review.openstack.org/604986 | |
| 15:05:16 | mdbooth | efried: All the failures I've fixed so far have been deterministic, btw. | |
| 15:05:54 | efried | That's goodness. | |
| 15:06:16 | mdbooth | Either asserting a notification which doesn't happen, or asserting a notification without having first mocked fake_notifier | |
| 15:07:10 | mdbooth | race -> deterministic failure == goodness | |
| 15:15:29 | sean-k-mooney | mdbooth: i mean its now race -> deterministic sucsses but definetly a step in the right direction. | |
| 15:16:36 | mdbooth | sean-k-mooney: Nah, they're failing. They were previously asserting things which weren't true, but were silently ignored. | |
| 15:17:52 | sean-k-mooney | ya i have fixed incorerectly mocked thest in the past that always assered things that were incorrect. they always make me sad when i find them | |
| 15:33:55 | mriedem | gibi: some comments on https://review.openstack.org/#/c/591647/ | |
| 15:35:53 | imacdonn | kashyap: I already responded on the ops list: http://lists.openstack.org/pipermail/openstack-operators/2018-September/015931.html | |
| 15:36:13 | cfriesen | mdbooth: what's the story on the functional test failures for https://review.openstack.org/#/c/578846/ ? | |
| 15:36:19 | kashyap | imacdonn: Most excellent. I missed to notice it | |
| 15:37:01 | mdbooth | cfriesen: It's in a parent commit. Working on it. | |
| 15:37:16 | mdbooth | cfriesen: I'm getting sucked into fixing all the things again :/ | |
| 15:39:30 | mdbooth | gibi: In ComputeManager and ComputeAPI, is self.notifier always a legacy notifier? | |
| 15:40:29 | kashyap | imacdonn: So, no problem with the chosen future versions. Thanks for confirming! | |
| 15:43:11 | imacdonn | kashyap: np | |
| 15:43:50 | mdbooth | gibi: Oh, this one looks like it might be an actual bug | |
| 15:44:33 | openstack | Launchpad bug 1732199 in OpenStack Compute (nova) "test_extend_attached_volume fails with Unexpected compute_extend_volume result 'Error'" [Medium,Confirmed] | |
| 15:44:33 | imacdonn | mriedem: not sure if you're following the bug or not - (I believe) I figured out what's causing that volume extend intermittent failure - https://bugs.launchpad.net/nova/+bug/1732199 | |
| 15:44:56 | mdbooth | gibi: If an instance is deleted locally by compute api, it only sends an unversioned delete.end, whereas a proper delete by compute manager sends both | |
| 15:45:08 | imacdonn | mriedem: prob should discuss in #openstack-cinder I suppose | |
| 15:46:56 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/queens: Skip ServerActionsTestJSON.test_rebuild_server for cells v1 job https://review.openstack.org/605115 | |
| 15:54:35 | openstackgerrit | sean mooney proposed openstack/nova-specs master: Add spec for sriov live migration https://review.openstack.org/605116 | |
| 15:56:49 | openstackgerrit | Eric Fried proposed openstack/nova master: consumer gen: move_allocations https://review.openstack.org/591810 | |
| 16:18:44 | openstackgerrit | Matthew Booth proposed openstack/nova master: Raise error on timeout in wait_for_versioned_notifications https://review.openstack.org/604859 | |
| 16:21:00 | mdbooth | gibi: Please could you have a look at ^^^. It contains a fix to non-test code. The problem is a call to _delete_and_check_allocations() in a test. | |
| 16:21:20 | mdbooth | The problem is it results in a local delete, which only currently emits a legacy notification, which I suspect is a real bug. | |
| 16:22:10 | mdbooth | I'd vastly prefer not to fix that in this change. | |
| 16:22:20 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/rocky: Explicitly fail if trying to attach SR-IOV port https://review.openstack.org/605118 | |
| 16:43:08 | openstackgerrit | Merged openstack/nova master: Consumer gen support for delete instance allocations https://review.openstack.org/591597 | |
| 16:43:19 | melwitt | . | |
| 16:44:02 | mriedem | blarg, you can't instance.create() from one cell to another b/c _from_db_object resets the fields and instance.create() only saves what fields are changed | |
| 17:08:44 | openstackgerrit | Matthew Booth proposed openstack/nova master: Raise error on timeout in wait_for_versioned_notifications https://review.openstack.org/604859 | |
| 17:08:46 | mdbooth | gibi: nm, took it out ^^^ | |
| 17:09:00 | mdbooth | Just have to wait on an unversioned notification instead | |
| 17:15:59 | openstackgerrit | melanie witt proposed openstack/nova master: Restore nova-consoleauth to install docs https://review.openstack.org/605154 | |
| 17:19:40 | melwitt | mriedem: in the lp bug, you also mentioned adding a warning about the deprecation of nova-consoleauth. did you have a place for the warning in mind in the install guides? ^ | |
| 17:20:55 | mdbooth | Is it a bug that the api node only emits a legacy notification on local delete, but a full delete emits both? | |
| 17:21:09 | mdbooth | Sounds like a bug, but notifications aren't my thing. | |
| 17:21:18 | mdbooth | If it's a bug, I'll file it and reference it in a comment. | |
| 17:30:38 | tssurya | dansmith: if we use scatter-gather-cells function is there a way to differentiate legit exceptions upon which we should fail versus the ones we want to handle ? | |
| 17:31:06 | tssurya | at this point, all just raise the "raised_exception_sentinel" | |
| 17:31:07 | openstackgerrit | Matthew Booth proposed openstack/nova master: Raise error on timeout in wait_for_versioned_notifications https://review.openstack.org/604859 | |
| 17:32:12 | mdbooth | The check queue is toast | |
| 17:33:20 | melwitt | tssurya: currently there isn't a way, as you point out. I've been thinking about that too and it would be nice if we could include the actual exception raised per cell. I think we still want the sentinel though, so we'd need a new format for the value side of the results dict | |
| 17:33:37 | openstackgerrit | Matthew Booth proposed openstack/nova master: Don't delete disks on shared storage during evacuate https://review.openstack.org/578846 | |
| 17:33:37 | openstack | bug 1550919 in OpenStack Compute (nova) "[Libvirt]Evacuate fail may cause disk image be deleted" [Medium,In progress] https://launchpad.net/bugs/1550919 - Assigned to Matthew Booth (mbooth-9) | |
| 17:33:37 | openstackgerrit | Matthew Booth proposed openstack/nova master: Add regression test for bug 1550919 https://review.openstack.org/591733 | |
| 17:34:04 | tssurya | melwitt: yea we would have to tweak the return part I guess | |