| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-01-31 | |||
| 18:53:54 | jaypipes | cdent: lol, nm | |
| 18:54:12 | cdent | jaypipes: what was it? | |
| 18:54:32 | jaypipes | cdent: I'd forgotten to do: | |
| 18:54:39 | jaypipes | self.rt.report_client = self.report_client | |
| 18:54:51 | jaypipes | where self.report_client is the test_report_client.NoAuthKeystoneClient thing | |
| 18:55:01 | cdent | ah | |
| 18:55:05 | jaypipes | boom. | |
| 18:55:08 | jaypipes | perfecto. | |
| 18:55:13 | mriedem | artom: melwitt: http://tinyurl.com/hkpuoel | |
| 18:55:45 | cdent | jaypipes: cool, I was worried urllib3 had gone off and changed something (as they do) and made wsgi-intercept much more complicated (it's already a hairball) | |
| 18:55:50 | jroll | dansmith: did you get your question answered? typical devstack registers nodes with ironic after starting ironic, and then we (re)start n-cpu. so yeah, thinking we should wedge it in here-ish https://github.com/openstack/ironic/blob/master/devstack/lib/ironic#L1556 | |
| 18:55:57 | jaypipes | cdent: nah | |
| 18:56:49 | artom | mriedem, hrmm, does keep popping up, doesn't it? | |
| 18:57:37 | artom | And it predate's the change melwitt was talking about, since there are some on the 24th... | |
| 18:58:03 | melwitt | yeah, looks like this has been happening for awhile, just not often | |
| 19:00:46 | mriedem | i'll open a bug | |
| 19:01:17 | openstackgerrit | Pushkar Umaranikar proposed openstack/nova: Adopts keystoneauth with glance client. https://review.openstack.org/412634 | |
| 19:03:05 | artom | mriedem, melwitt, https://github.com/openstack/tempest/blob/master/tempest/api/compute/admin/test_servers.py#L50 | |
| 19:03:20 | artom | That seems... obvious? Surely you'd want to wait for the server to enter error state at that point... | |
| 19:06:26 | mriedem | artom: well it could, | |
| 19:06:57 | artom | I checked the api-ref, it returns 202, not 200 | |
| 19:06:58 | mriedem | it fails in cells v1, i assume, because the child cell is reporting the vm_state change back up to the parent api cell so the instances in both dbs are in sync, and when tempest makes the GET request, the api cell doesn't have the updated vm_state yet | |
| 19:07:12 | artom | So it's async and might not be done by the time the next requests lists the servers in error state... | |
| 19:08:07 | mriedem | it's actually odd that it's a 202 | |
| 19:08:35 | mriedem | because all that api does is literally change the vm_state in the db https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/admin_actions.py#L82 | |
| 19:10:41 | mriedem | the upcall to the api cell is an rpc cast, | |
| 19:10:47 | mriedem | i wonder if that's why it's a 202, for cells v1 | |
| 19:13:40 | artom | Where does the cells call happen in that? | |
| 19:13:51 | mriedem | in nova.objects.Instance.save | |
| 19:14:35 | melwitt | I would think that would save it at the top and should be returned as 'error' as expected by the test. the save() would sync the vm_state change to the child. the get_all in compute/api is reading from child cells, so that's probably how it's happening | |
| 19:15:13 | melwitt | we've put in some conditionals "if cells v1, read from the top" but it looks like this was missed, for the get_all | |
| 19:15:20 | artom | Bah, vim ctags has failed me | |
| 19:15:34 | artom | It took me to agents.py save method for some reason | |
| 19:16:04 | openstack | Launchpad bug 1660747 in OpenStack Compute (nova) "test_list_servers_filter_by_error_status intermittently fails with MismatchError on no servers in response" [Medium,Confirmed] | |
| 19:16:04 | mriedem | here is the bug btw https://bugs.launchpad.net/nova/+bug/1660747 | |
| 19:16:25 | melwitt | thanks | |
| 19:16:38 | openstackgerrit | Lee Yarwood proposed openstack/nova: WIP - libvirt: Remove redundant bdm serial mangling during swap_volume https://review.openstack.org/427364 | |
| 19:17:15 | mriedem | sfinucan: do you know the ReST magic to find the root of a doc tree? | |
| 19:17:19 | mriedem | for a link | |
| 19:18:25 | artom | melwitt, you wanna propose a fix to Tempest? Shall I? | |
| 19:18:53 | dansmith | melwitt: I definitely had that in one of my patches at some point | |
| 19:19:03 | melwitt | artom: I'll propose a fix to nova, it's the get_all that needs a tweak I think | |
| 19:19:15 | dansmith | melwitt: for get_all, but I don't see it right now, so might've been the victim of a rebase, although I was failing the cells job regularly without it | |
| 19:19:34 | artom | melwitt, aha! | |
| 19:19:35 | melwitt | dansmith: yeah, I think it must have gotten lost in the mix | |
| 19:19:56 | artom | I still think Tempest should wait after a request that returns a 202, not just assume it's been completed... | |
| 19:20:04 | openstackgerrit | Eric Fried proposed openstack/nova: Introduce base PowerVM driver https://review.openstack.org/391288 | |
| 19:20:22 | openstackgerrit | Sarafraj Singh proposed openstack/nova-specs: Send service token along with user tokens https://review.openstack.org/426933 | |
| 19:20:26 | mriedem | melwitt: technically, we don't have to care about fixing latent bugs in cells v1 | |
| 19:21:08 | dansmith | mriedem: this isn't latent | |
| 19:21:10 | melwitt | mriedem: it's not a latent bug in cells v1. it should have been working fine until we added cells v2 instance listing from all cells | |
| 19:22:59 | openstackgerrit | Matt Riedemann proposed openstack/nova: doc: add upgrade notes to the placement devref https://review.openstack.org/427367 | |
| 19:23:01 | mriedem | cdent: bauzas: ^ | |
| 19:23:11 | mriedem | dansmith: melwitt: ok | |
| 19:24:02 | browne | mriedem: do the RPC versions need to be updated for Ocata? seemed so, so i put up this patch: https://review.openstack.org/#/c/425958/ | |
| 19:24:36 | lyarwood | dansmith: https://review.openstack.org/#/c/427267 should be ready for a review if you have time later today | |
| 19:25:06 | melwitt | artom: I think we've left the test alone since it helps catch our bugs, but technically you're right, a 202 means wait | |
| 19:26:03 | artom | melwitt, well, is the problem that the server state update takes too long, or doesn't happen at all? Because the latter isn't affected by waiting | |
| 19:26:22 | mriedem | it would have to be a race, | |
| 19:26:26 | mriedem | else this would be 100% failure | |
| 19:26:35 | melwitt | artom: the problem is our code is reading instances from the child cells instead of the parent cell, for cells v1 | |
| 19:26:40 | artom | It'll still catch the bugs, except it'll do it by timing out on the wait_for_server_status call | |
| 19:27:10 | melwitt | most of the time, the sync down gets to the child before our read does, and in that case, it will pass the test | |
| 19:27:23 | melwitt | no, it wouldn't catch this bug I don't think | |
| 19:27:45 | melwitt | this is like a split second difference | |
| 19:28:29 | artom | Ah, so the status ends up being synced in all cases | |
| 19:28:36 | melwitt | yes | |
| 19:28:39 | mriedem | lyarwood: i threw the rc-potential tag on that one since it's a regression | |
| 19:29:03 | lyarwood | mriedem: ack thanks | |
| 19:29:23 | artom | melwitt, wait, just to make this clear in my head: the write happens on the parent cell, the read from the child cells, so if you time your read really quickly after the write, you won't get the new state? | |
| 19:29:42 | artom | And the fix you have in mind is to read from the parent cell? | |
| 19:30:05 | melwitt | yes. cells v1 normally reads from the parent cell. it has instances table in the parent and the childs and it syncs them | |
| 19:31:32 | artom | Hrmpf, this is indeed a case of a test being technically wrong but actually useful | |
| 19:31:47 | dansmith | artom: no | |
| 19:31:54 | dansmith | artom: the test case can't tell where it's being read from | |
| 19:32:23 | melwitt | he means the 202 return code implies async so it shouldn't just assume the state change is available after the call returns | |
| 19:32:35 | artom | melwitt, words out of my mouth :) | |
| 19:32:44 | melwitt | we know that it should be, from the code. but the API doesn't tell that | |
| 19:32:51 | dansmith | is it not polling? | |
| 19:33:00 | melwitt | no, it's just immediately reading after | |
| 19:33:00 | artom | dansmith, nope | |
| 19:33:03 | dansmith | ah, okay | |
| 19:33:07 | dansmith | then yeah | |
| 19:33:30 | jaypipes | mriedem, cdent: yes, I'm still working on functional tests for the Ironic patch. Didn't want you to think I forgot or am slacking. | |
| 19:34:08 | mriedem | i'm pretty far removed from that patch by now, | |
| 19:34:12 | mriedem | once john manhandled it | |
| 19:34:44 | mriedem | like, before i start reviewing it again, someone is probably going to have to give me a recap of what happened | |
| 19:35:57 | mriedem | browne: probably... | |
| 19:36:26 | mriedem | i feel like russellb used to have a cheat sheet for this | |
| 19:42:08 | openstackgerrit | Matt Riedemann proposed openstack/nova: doc: add upgrade notes to the placement devref https://review.openstack.org/427367 | |
| 19:45:37 | mordred | mriedem: I've got this error coming up on nodepool patches: http://logs.openstack.org/06/425706/7/check/gate-dsvm-nodepool-src/7004c5b/logs/screen-nodepool.txt.gz#_2017-01-31_19_02_00_177 | |
| 19:45:57 | mordred | mriedem: which is reporting a 500 error from nova and <class 'sqlalchemy.exc.ProgrammingError'> as the error | |
| 19:46:33 | mriedem | nice http://logs.openstack.org/06/425706/7/check/gate-dsvm-nodepool-src/7004c5b/logs/screen-n-api.txt.gz?level=TRACE#_2017-01-31_19_02_00_169 | |
| 19:46:41 | mordred | yah | |
| 19:46:51 | mordred | isn't that pretty? (I was just about to paste that link next :) ) | |
| 19:46:53 | mriedem | Table 'nova_cell0.instance_extra' doesn't exist | |
| 19:47:00 | mriedem | mordred: you don't have your cells v2 stuff setup | |
| 19:47:02 | mriedem | in whatever that thing is | |
| 19:47:18 | mordred | well, it's nodepool's functional tests that run on top of a devstack node | |
| 19:47:36 | mordred | does devstack-gate need cellsv2 now but not set it up by default? :( | |