| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-11-15 | |||
| 15:42:53 | artom | slaweq, there's your failure: https://zuul.opendev.org/t/openstack/build/3008cc3eeaea44369a2fa3db3a29ae67/log/controller/logs/libvirt/qemu/instance-00000011_log.txt.gz#4 | |
| 15:43:05 | artom | mriedem too if you care ^^ | |
| 15:43:10 | mriedem | artom: that's not a faliure | |
| 15:43:15 | mriedem | that shows up in like every guest log in thegate | |
| 15:43:28 | artom | Oh | |
| 15:43:51 | mriedem | e.g. random guest https://storage.gra1.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_300/678438/22/check/neutron-tempest-dvr-ha-multinode-full/3008cc3/controller/logs/libvirt/qemu/instance-00000023_log.txt.gz | |
| 15:44:14 | mriedem | my guess is the failure is due to, like many gate failures these days, overloaded nodes crapping out | |
| 15:44:22 | mriedem | though the rax nodes usually aren't one of them | |
| 15:46:37 | openstackgerrit | Balazs Gibizer proposed openstack/nova stable/rocky: Fix false ERROR message at compute restart https://review.opendev.org/694581 | |
| 15:50:02 | gibi | eandersson: ^^ | |
| 15:56:21 | openstackgerrit | Matt Riedemann proposed openstack/nova master: docs: Extract rescue from reboot https://review.opendev.org/694529 | |
| 16:00:48 | lyarwood | thanks for that mriedem | |
| 16:02:27 | openstackgerrit | Matt Riedemann proposed openstack/nova master: doc: mention that rescuing a volume-backed server is not supported https://review.opendev.org/694584 | |
| 16:02:39 | mriedem | np | |
| 16:03:03 | openstackgerrit | Matt Riedemann proposed openstack/nova master: doc: mention that rescuing a volume-backed server is not supported https://review.opendev.org/694584 | |
| 16:07:36 | gibi | bauzas: I hit a bug in https://review.opendev.org/#/c/670112/8/nova/cmd/manage.py@2867 | |
| 16:08:10 | mriedem | gibi: you were +2 on this before it was rebased a bit in earlier changes https://review.opendev.org/#/c/642591/ - can you hit that again? dansmith - that's also the one where you suggested re-doing the logic so it should be simple and familiar | |
| 16:08:21 | bauzas | gibi: graaah ok, thanks ! | |
| 16:08:44 | gibi | mriedem: on it. | |
| 16:08:46 | bauzas | gibi: just respinning, sec | |
| 16:09:02 | gibi | bauzas: sure, I will retry with the new ps | |
| 16:09:10 | mriedem | bauzas: why wouldn't your functional tests hit that? | |
| 16:09:33 | bauzas | mriedem: the CI is still in the weeds | |
| 16:09:37 | bauzas | no result yet | |
| 16:09:39 | bauzas | and I was lazy | |
| 16:09:44 | mriedem | bauzas: i meant locally... | |
| 16:09:45 | mriedem | yeah | |
| 16:10:01 | bauzas | but that's a good call, lemme check | |
| 16:10:08 | mriedem | this is why i said you don't really need unit testing on these types of commands, it should mostly, if not all, be functional test driven | |
| 16:10:30 | bauzas | that's what happens when you do three things at same time... | |
| 16:10:42 | bauzas | meeting, patching, discussing | |
| 16:11:14 | bauzas | gibi: I just run the functest now | |
| 16:11:28 | bauzas | hopefully it will get caught | |
| 16:12:00 | bauzas | oh, and FWIW, I was rushing on updating a new revision, I haven't used yet the COMPUTE_NODE trait or did the split | |
| 16:13:19 | openstackgerrit | Merged openstack/nova stable/train: Use admin neutron client to query ports for binding https://review.opendev.org/694013 | |
| 16:13:38 | bauzas | mriedem: I'm not opposed to have most of the logic being checked by functional tests, I just added a few unit tests for sanity :) | |
| 16:14:24 | gibi | bauzas: nova.tests.functional.test_nova_manage.TestNovaManagePlacementAudit.test_audit_orphaned_allocations_from_deleted_compute_evacuate failed for me locally now but with a different error than what I saw in the devstack | |
| 16:14:27 | mriedem | that's a slippery slope and bad habit to get into with commands like this | |
| 16:14:30 | mriedem | so just, don't do it | |
| 16:14:51 | gibi | bauzas: File "nova/cmd/manage.py", line 2927, in audit | |
| 16:14:51 | gibi | ctxt, placement, output, provider, delete) | |
| 16:14:51 | gibi | File "nova/cmd/manage.py", line 2781, in _check_orphaned_allocations_for_provider | |
| 16:14:55 | gibi | inst_uuids, mig_uuids = result | |
| 16:14:56 | bauzas | gibi: yeah, I just saw it | |
| 16:14:57 | gibi | TypeError: 'bool' object is not iterable | |
| 16:15:02 | bauzas | I'm on it | |
| 16:15:05 | gibi | bauzas: cool | |
| 16:15:35 | gibi | mriedem: do you mean sanity is slippery slope with nova manage cli ? :) | |
| 16:15:58 | bauzas | gibi: but for *some* reason, the functests should have caught the argument issue | |
| 16:16:04 | bauzas | (for the microversion) | |
| 16:16:11 | bauzas | I mean, the other issue is unrelateed | |
| 16:16:19 | bauzas | or a side effect | |
| 16:16:30 | bauzas | so, mriedem's point is legit | |
| 16:16:41 | bauzas | I'm leaking coverage here | |
| 16:17:30 | mriedem | gibi: heh, no, i just don't really trust unit tests in nova anymore for anything that involves more than one service or even more than a couple of methods interacting within the same service | |
| 16:17:38 | mriedem | it's too easy to mock things that become false positives | |
| 16:20:52 | bauzas | so the bug is fixed, but the argument issue isn't shown | |
| 16:26:27 | gibi | mriedem: I share your view regarding the unit tests in nova | |
| 16:27:16 | bauzas | 2019-11-15 17:26:12,996 INFO [placement.requestlog] 127.0.0.1 "GET /placement/resource_providers" status: 200 len: 840 microversion: 1.0 | |
| 16:27:21 | bauzas | magnifico ^ | |
| 16:27:39 | bauzas | the report client is blindly accepting the 'microversion' keyword | |
| 16:27:48 | bauzas | but just says "meh" to it | |
| 16:27:57 | bauzas | gibi: mriedem: ^ | |
| 16:28:40 | bauzas | I don't see how to catch this unless to mock and assert the call | |
| 16:28:50 | bauzas | which is nonsense | |
| 16:29:21 | gibi | bauzas: then something is wrong with the placement fixture we use in the functional test | |
| 16:29:45 | bauzas | and, with using the 'version' keyword : | |
| 16:29:52 | bauzas | 2019-11-15 17:29:16,087 INFO [placement.requestlog] 127.0.0.1 "GET /placement/resource_providers" status: 200 len: 1684 microversion: 1.14 | |
| 16:30:05 | bauzas | we probably kwargs | |
| 16:30:07 | mriedem | def get(self, url, **kwargs): is your problem | |
| 16:30:08 | bauzas | gibi: ^ | |
| 16:30:13 | bauzas | heh, this | |
| 16:30:13 | mriedem | like gibi said it's a bug in the fixture | |
| 16:30:41 | mriedem | SchedulerReportClient.get won't let you pass microversion= | |
| 16:30:42 | mriedem | def get(self, url, version=None, global_request_id=None): | |
| 16:30:43 | gibi | then it is a good time to fix the fixture | |
| 16:30:51 | gibi | :) | |
| 16:31:42 | bauzas | aaaaand I love to see a bug about some code with a few TODO(sbauza) around it... | |
| 16:33:02 | openstackgerrit | Stephen Finucane proposed openstack/nova master: functional: Change order of two classes https://review.opendev.org/689178 | |
| 16:33:03 | openstackgerrit | Stephen Finucane proposed openstack/nova master: functional: Rework '_delete_server' https://review.opendev.org/689179 | |
| 16:33:03 | openstackgerrit | Stephen Finucane proposed openstack/nova master: functional: Make '_wait_for_state_change' behave consistently https://review.opendev.org/689180 | |
| 16:33:04 | openstackgerrit | Stephen Finucane proposed openstack/nova master: functional: Unify '_wait_until_deleted' implementations https://review.opendev.org/689181 | |
| 16:33:04 | openstackgerrit | Stephen Finucane proposed openstack/nova master: functional: Make 'ServerTestBase' subclass 'InstanceHelperMixin' https://review.opendev.org/689182 | |
| 16:33:36 | mriedem | should just remove those todos in the placement fixture about passing a token, that's never going to happen if it hasn't happened by now | |
| 16:33:48 | mriedem | we should like, put a 3 year timer on todos in the code | |
| 16:34:05 | mriedem | "TODO(vishy): do this in grizzly" | |
| 16:34:28 | gibi | :) | |
| 16:36:06 | openstackgerrit | Sylvain Bauza proposed openstack/nova master: Add a placement audit command https://review.opendev.org/670112 | |
| 16:38:06 | openstackgerrit | Stephen Finucane proposed openstack/nova master: docs: Remove 'adv-config', 'system-admin' subdocs https://review.opendev.org/684402 | |
| 16:38:06 | openstackgerrit | Stephen Finucane proposed openstack/nova master: docs: Replacing underscores with dashes https://review.opendev.org/685929 | |
| 16:38:07 | openstackgerrit | Stephen Finucane proposed openstack/nova master: docs: Strip '.rst' suffix https://review.opendev.org/687264 | |
| 16:42:43 | stephenfin | melwitt: If you have time before the end of the day, think you could hit this quotas doc patch? https://review.opendev.org/#/c/667165/ | |
| 16:42:53 | stephenfin | It's been around for a few months. Would be nice to close it out | |
| 16:43:09 | KeithMnemonic1 | hello all any chance to get some more reviews and a possible WF+1 on this one ? https://review.opendev.org/#/c/683008/ | |
| 16:43:30 | melwitt | stephenfin: that's a biggun. I'll try to get to it | |
| 16:44:06 | stephenfin | melwitt: Sure thing. tbh, I'd just forget whatever was there previously and read it like a new doc. If nothing stands out as totally wrong, it's gonna be better than what we have | |
| 16:44:09 | stephenfin | IMO, obv | |
| 16:44:51 | sean-k-mooney | KeithMnemonic1: i think lyarwood should proably review that | |
| 16:45:07 | melwitt | stephenfin: ok, I'll approach it with that in mind | |
| 16:47:45 | stephenfin | ta | |