Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-09-25
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 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: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: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: Add regression test for bug 1550919 https://review.openstack.org/591733
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: Don't delete disks on shared storage during evacuate https://review.openstack.org/578846
17:34:04 tssurya melwitt: yea we would have to tweak the return part I guess
17:34:56 melwitt tssurya: I suppose we could just change things to include the exception object instead of the sentinel, then you just have to check isinstance(x, Exception) instead of checking "if sentinel"
17:35:37 melwitt to know whether you've got a good result vs an exception
17:37:03 tssurya so you mean at this point https://github.com/openstack/nova/blob/ebab3adb2849c17ebb0249277da89d0d8111e591/nova/context.py#L445 ?
17:38:55 melwitt yeah, one way would be to just store the exception object in 'result' instead of the sentinel. then obviously anywhere in the code that checks "if raised_exception_sentinel" would have to be updated to check isinstance(x, Exception) instead
17:40:56 sean-k-mooney melwitt: i was not following but is that not an anti pattern
17:42:16 melwitt sean-k-mooney: is it? I said earlier if we wanted to keep the sentinels, we'd have to do something to change the format of the value side to be able to include optional additional data (where the exception object could go)
17:42:33 sean-k-mooney melwitt: oh your talking about scatter_gather_cells
17:42:39 melwitt yes
17:43:40 sean-k-mooney the antipattern is the explcit type check in python e.g. isinstance checks but this is not a standard case
17:45:07 melwitt oh, I see. yeah, if results = {cell_uuid: (result or sentinel, optional data)}, then the optional data type could be assumed based on the sentinel
17:45:25 sean-k-mooney melwitt: if the was c++ or pyton3 only i would lean towrods makeing result a concurrent future or std::expected type
17:46:32 melwitt sean-k-mooney: I don't know much about that but the expected type will be different depending on whether the cell returned a result or an exception
17:47:04 tssurya melwitt: for me the requirement came because of wanting to differentiate between InstanceNotFound valid exception versus an actuall cell down exception here: https://review.openstack.org/#/c/591658/7/nova/compute/api.py@2327 , feel free to leave a comment and we can also raise it at tomorrow's cells meeting; need to leave office now
17:47:58 sean-k-mooney in c++ std::expect models a type that either pacages the expected result or and error type. when you use it you can test if it it has an error or not with an if expect ...
17:48:22 melwitt tssurya: ok, will look there. I was thinking we would need to be able to differentiate exceptions but didn't see a concrete example till now

Earlier   Later