| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-09-25 | |||
| 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 | |
| 17:48:50 | melwitt | sean-k-mooney: oh, nice | |
| 17:49:37 | AJaeger | nova stable team, could you review a simple zuul job change, please? https://review.openstack.org/602018 https://review.openstack.org/602019 https://review.openstack.org/602022 https://review.openstack.org/602023 | |
| 17:49:45 | AJaeger | mriedem: I answered your comment on https://review.openstack.org/#/c/602018/1/.zuul.yaml | |
| 17:49:54 | sean-k-mooney | melwitt: so my preference would be have result be a type that you can just do "if result: #handel sucess else : # handel error" | |
| 17:50:58 | melwitt | sean-k-mooney: we also have "did not respond" i.e. timeout waiting for result | |
| 17:51:02 | sean-k-mooney | melwitt: to do that you would have to override __bool__ in the wrapper class to see if the value was an exception type or not | |
| 17:51:19 | melwitt | which could be an error, but then you have to check the type of the error | |
| 17:51:34 | melwitt | but I guess we have to do that anyway if we're differentiating | |
| 17:52:46 | sean-k-mooney | melwitt: ya. if you stuff the exeption into the value field you can just rerais it and cactch it polymorifclly assuming the timeout is a different exception type | |
| 17:53:13 | melwitt | yeah, could do that | |
| 17:53:49 | sean-k-mooney | melwitt: so basically its the same as what you suggted but your hiding the type checking in a dedicated class that does it for you instead of doing it in multiple places | |
| 17:55:34 | sean-k-mooney | melwitt: python3's concurrent futures basically wrap this up nicly for you too but they are python 3 only https://docs.python.org/3/library/concurrent.futures.html#concurrent.futures.Future | |
| 17:56:09 | melwitt | sean-k-mooney: thanks | |
| 17:59:57 | sean-k-mooney | melwitt: want me to mock up an example of using what i described so you can see how it would work. im not sure its the correct solution in this case but its what i would try personally. | |
| 18:05:09 | melwitt | sean-k-mooney: an example that would work for python2 also? I would be interested in looking at it if you mock it up | |
| 18:05:57 | sean-k-mooney | melwitt: yes i can make it work for python too. ill mock something up and push it with some unit tests showing its usage later tonight | |
| 18:06:17 | sean-k-mooney | *python 2 | |
| 18:06:50 | melwitt | thanks | |
| 18:16:34 | openstackgerrit | Jack Ding proposed openstack/nova master: Handle missing marker during online data migration https://review.openstack.org/605164 | |
| 18:17:11 | jaypipes | man I fucking hate the scheduler unit tests. | |
| 18:17:15 | jaypipes | what a pile of shit. | |
| 18:17:59 | sean-k-mooney | jaypipes: let me guess asserting behavior 3 levles deep then just the local function behvior? | |
| 18:19:39 | sean-k-mooney | anyone know where the devstack log is stored by default? ther eused to be an xstack log in /opt/stack/logs but that seams to not be a thing anymore | |
| 18:20:10 | jaypipes | sean-k-mooney: more the endless hierarchy of copy/pasted test case classes. | |
| 18:20:39 | sean-k-mooney | jaypipes: yeah OOP | |
| 18:22:11 | sean-k-mooney | jaypipes: did i mention im playing with creating a programming language. it "object orianted" current it does not allow inheritence but you can have objects :P | |
| 18:26:30 | AJaeger | mriedem: could you review the other stable backports of that ceph change as well, please? | |
| 18:26:47 | mriedem | yeah | |
| 18:28:22 | AJaeger | thanks | |
| 18:31:58 | melwitt | what is going on with the gate lately? changes that have been in the queue for 26 hours | |
| 18:33:12 | AJaeger | melwitt: high failure rate plus one of our clouds down, see clark's email from a week ago or so | |
| 18:33:49 | melwitt | found it, thank you | |
| 18:33:52 | AJaeger | melwitt: http://lists.openstack.org/pipermail/openstack-dev/2018-September/134867.html | |
| 18:34:40 | AJaeger | melwitt: still applies regarding cloud down - clarkb is testing a fix right now | |
| 18:46:27 | openstackgerrit | Jonte Watford proposed openstack/nova master: This commit adds __str__ and __repr__ methods to objects InstanceNUMACell and NUMACell. These will generate more readable representations of the objects for use in logs. https://review.openstack.org/600269 | |
| 18:48:57 | mriedem | AJaeger: waiting to see the renamed job run on ocata https://review.openstack.org/#/c/602023/ | |