| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-04-10 | |||
| 13:29:10 | mriedem | i don't see any links to ML discussion in the patches though | |
| 13:31:31 | mriedem | this reminds me, i think it's very weird that the use_neutron config option is deprecated https://docs.openstack.org/nova/latest/configuration/config.html#DEFAULT.use_neutron even though it's in our install guide and is required while we still have nova-network around | |
| 13:31:50 | mriedem | if anyone is going through their logs and sees a deprecation warning for using use_neutron, there isn't anything they can do about it | |
| 13:32:20 | mriedem | i think oslo.service or one of the oslo libraries even has a flag where you can force services to not start if they are using deprecated options, so you can flush those out in pre-prod | |
| 13:33:20 | mriedem | in other words, wouldn't it make more sense to *not* deprecate options required to run nova with neutron, until at least we've removed nova-network? | |
| 13:33:38 | mriedem | stephenfin: thoughts? ^ | |
| 13:35:07 | stephenfin | mriedem: You can filter out those warnings if you want. The intention is "this warning currently exists but is going away soon". The reason it's going away is given in the message | |
| 13:35:32 | stephenfin | *this option currently exists | |
| 13:36:15 | openstackgerrit | Raoul Hidalgo Charman proposed openstack/nova master: Expose shutdown retry interval as config setting https://review.openstack.org/552483 | |
| 13:37:00 | efried | claudiub: Yeah, I recognize that; what I'm asking for in this case is a way to signal that I want to get around the decorator and autospec the actual method underneath it. | |
| 13:38:58 | cdent | efried: not really possible, your method has been redefined | |
| 13:39:24 | efried | cdent: Yeah, hence "black magic" | |
| 13:39:24 | cdent | the original form is sort of gone | |
| 13:39:41 | cdent | that would be darker than black | |
| 13:41:07 | lpetrut | efried: you may be able to retrieve the decorated methods, we do it in a few cases to avoid lock decorators within unit tests: https://github.com/openstack/os-win/blob/e0d7032dfb042f56fd02a52796b186cd8d67d240/os_win/_utils.py#L82 | |
| 13:41:16 | mriedem | madhaviy: here is that ML thread http://lists.openstack.org/pipermail/openstack-dev/2017-September/121762.html | |
| 13:41:38 | efried | lpetrut: ooooo | |
| 13:42:50 | cdent | efried, lpetrut: too dark | |
| 13:43:03 | efried | ima try it anyway | |
| 13:43:08 | efried | cdent: hold my soul | |
| 13:43:23 | cdent | efried: wouldn't it be better to extract the thing you want to test to an undecorated thing? | |
| 13:44:05 | efried | cdent: I tried that first. Because it's already extracted thusly. But the decorator itself makes the test suite freak out. (It's the privsep entrypoint) | |
| 13:44:21 | cdent | it's all a bit smelly to me (not your soul (but maybe?)) | |
| 13:44:36 | cdent | but we already know how I feel about complexity in tests... | |
| 13:44:44 | efried | I'm trying to soften the blows I keep on dishing out to mikal | |
| 13:45:06 | efried | cdent: FYI: https://review.openstack.org/#/c/552242/ | |
| 13:47:40 | mriedem | madhaviy: there is also a thread in the operators ML with some other options | |
| 13:51:28 | claudiub | efried: yeah, as lpetrut said, we're getting the undecorated method is some unit tests in os-win, but that would only be needed for decorators which has some sort of special behaviour (adds / injects new arguments). even if we do autospec the undecorated methods, there are still plenty of cases in which the methods expects some sort of key-value argument, something like: | |
| 13:51:32 | madhaviy | mriedem: thanks. But I do not see any conclusion out of this discussion. Can we get back dhcp_domain conf option (not to deprecate) | |
| 13:51:33 | claudiub | if kwargs.get("something"): then do something | |
| 13:51:47 | claudiub | I've seen this in some oslo libs. | |
| 13:52:57 | mriedem | madhaviy: it definitely sounds like the option shouldn't be deprecated | |
| 13:53:09 | mriedem | at least until we know what we're going to do with it | |
| 13:53:52 | mriedem | i thought there was general agreement to use the dns_domain from the neutron network if there was one, and fallback to using the nova dhcp_domain option otherwise, but someone in the ops list punched some holes in that idea, but i'm not sure how valid those holes are, or if they are specific to that person's deployment | |
| 13:53:57 | efried | claudiub: Yeah, I understand that. That's not the case here, though - there are no arbitrary args. In this case, in the course of the review (over many iterations) I saw with my eyeballs that the invocation was wrong, and wanted a way to enforce it via autospec. But the decorator kills doing that directly. Now I'm hacking at it trying to figure out how I would actually use the undecorated method to do what I want. | |
| 13:54:28 | mriedem | madhaviy: if there were going to be a nova/neutron forum session at the summit, i'd put that in the agenda since ops would be in the room | |
| 13:54:32 | mriedem | melwitt: ^ | |
| 13:55:05 | madhaviy | mriedem: sure. | |
| 13:55:15 | claudiub | efried: well, instead of autospec=True, you can pass-in a function that has the signature you want to enforce? | |
| 13:55:50 | efried | claudiub: As a side_effect to the mock, you mean? | |
| 13:56:11 | claudiub | something like: @mock.patch('something', autospec=some_function_with_exactly_what_I_want) | |
| 13:56:38 | claudiub | I dunno if that works though. | |
| 13:56:50 | efried | That sort of defeats the purpose, which is to vet whether my (actual) method is being called with the args I expect. If my test is wrong (in that it uses the wrong args) and then I make some_function_with_exactly_what_I_want have those same (wrong) args, then I'm just as effed. | |
| 13:59:22 | claudiub | yeah, a bit, but at least you'll have tests that don't have autospecs with *args / **kwargs in their method signatures. | |
| 14:00:29 | mriedem | madhaviy: melwitt: added https://etherpad.openstack.org/p/YVR-nova-brainstorming | |
| 14:00:31 | mriedem | mlavalle: ^ | |
| 14:01:51 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/queens: libvirt: Allow to specify granular CPU feature flags https://review.openstack.org/559700 | |
| 14:02:04 | mlavalle | mriedem: cool, thanks! | |
| 14:02:41 | mriedem | dansmith: ^ rev'ed kashyap's backport if you want to hit that | |
| 14:03:07 | kashyap | mriedem: Ah, thanks. Was duking around fixing the compat code | |
| 14:03:13 | kashyap | (After removing the version stuff) | |
| 14:03:45 | kashyap | s/removing/bumping/ | |
| 14:03:46 | dansmith | mriedem: done thanks | |
| 14:04:15 | madhaviy | mriedem: thanks! | |
| 14:05:29 | kashyap | mriedem: For the 'pike' change, abandoning it, and doing a fresh cherry-pick from 'queens' is the correct way? | |
| 14:05:42 | mriedem | kashyap: the change id is the same so you don't need to abandon | |
| 14:05:50 | mriedem | just re-do the cherry pick locally | |
| 14:05:53 | kashyap | Ah, right | |
| 14:07:13 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/queens: libvirt: Allow to specify granular CPU feature flags https://review.openstack.org/559700 | |
| 14:12:39 | openstackgerrit | Eric Berglund proposed openstack/nova master: PowerVM: Add proc_units_factor conf option https://review.openstack.org/554688 | |
| 14:12:50 | openstackgerrit | Sylvain Bauza proposed openstack/nova-specs master: Proposes NUMA topology with RPs https://review.openstack.org/552924 | |
| 14:15:32 | mriedem | seems the messages here should just say 'placement is required' right? https://github.com/openstack/nova/blob/6d4958b5c225a602d22069b920cbf1a37b652c58/nova/scheduler/client/report.py#L87 | |
| 14:16:33 | jaypipes | mriedem: ack | |
| 14:19:42 | kashyap | Hmm, unrelated: "suddenly" I start seeing this import error when running unit tests: http://paste.openstack.org/show/718823/ | |
| 14:19:43 | mriedem | ok i'll make that change. was looking at https://review.openstack.org/#/c/554920/ which will add a requirement to configure placement for nova-api, | |
| 14:19:57 | mriedem | so jaypipes - ^ will likely add the placement requirement before the mirroring patch does anyway | |
| 14:20:13 | mriedem | kashyap: tox -r -e | |
| 14:20:21 | kashyap | Darn, thanks | |
| 14:21:20 | efried | kashyap: If you want to not wait forever, you can start upgrading packages manually in your venv... | |
| 14:21:31 | efried | ...but -r is certainly safest. | |
| 14:22:10 | jaypipes | mriedem: ack | |
| 14:22:29 | kashyap | efried: Yeah, I hear ya. The manual imports might result in "find the next dep" rabbit hole. So just went with the "nuclear" | |
| 14:22:38 | efried | just so | |
| 14:34:07 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Update wording in @safe_connect placement warnings https://review.openstack.org/560039 | |
| 14:35:28 | bauzas | efried: jaypipes: stephenfin: sean-k-mooney: cfriesen_: thanks all for the reviews of https://review.openstack.org/#/c/552924/6/specs/rocky/approved/numa-topology-with-rps.rst | |
| 14:36:00 | bauzas | efried: jaypipes: stephenfin: sean-k-mooney: cfriesen_: now we have a new revision based on your comments https://review.openstack.org/#/c/552924/ | |
| 14:36:04 | openstackgerrit | Merged openstack/nova master: Remove mox in unit/api/*/test_instance_actions.py https://review.openstack.org/559269 | |
| 14:38:35 | jaypipes | bauzas: k, reviewing now | |
| 14:39:41 | openstackgerrit | Surya Seetharaman proposed openstack/nova master: [WIP] Delete orphaned req_spec/inst_mapp records of archived instances https://review.openstack.org/560042 | |
| 14:55:58 | openstackgerrit | Chris Dent proposed openstack/nova master: Use nova.db.api directly https://review.openstack.org/543262 | |
| 15:01:34 | openstackgerrit | Merged openstack/nova master: Remove mox in test_user_data.py https://review.openstack.org/559264 | |
| 15:01:51 | openstackgerrit | Merged openstack/nova master: Remove mox in unit/api/*/test_server_metadata.py https://review.openstack.org/559673 | |
| 15:02:02 | openstackgerrit | Merged openstack/nova master: Remove mox in unit/api/*/test_server_password.py https://review.openstack.org/559649 | |
| 15:04:31 | efried | Spec cores (jaypipes dansmith because this spec is near and dear to your hearts) can we get https://review.openstack.org/#/c/556971/ approved now? Code is shaping up. | |
| 15:04:57 | dansmith | efried: wake me when jaypipes is +2 on it | |
| 15:05:02 | efried | ack | |
| 15:08:28 | efried | edleafe: You gonna rebase the rest of the consumer generation series to pick up that fix? | |
| 15:09:16 | edleafe | efried: already done (locally). Need to figure out one last bit before I push a new rev | |
| 15:09:22 | efried | coo | |
| 15:10:40 | openstackgerrit | Chris Dent proposed openstack/nova master: Use nova.db.api directly https://review.openstack.org/543262 | |
| 15:18:50 | jaypipes | dansmith: I'm +2 on the spec now. | |
| 15:19:36 | dansmith | well, that was a short nap | |
| 15:20:03 | mriedem | lyarwood: at the ptg we talked about adding an online data migration to look for and remove duplicate bdm entries in the db, to eventually clear the way to adding a unique constraint on instance uuid and volume id in the bdm table - is that still on your radar, or something i can start hacking on? | |
| 15:20:29 | mriedem | L575 https://etherpad.openstack.org/p/nova-ptg-rocky | |
| 15:21:46 | lyarwood | mriedem: yeah, only just got around to starting yesterday when I hit https://bugs.launchpad.net/cinder/+bug/1762687 | |
| 15:21:46 | openstack | Launchpad bug 1762687 in OpenStack Compute (nova) "Concurrent requests to attach the same non-multiattach volume to multiple instances can succeed" [Undecided,New] - Assigned to Lee Yarwood (lyarwood) | |
| 15:21:51 | mriedem | or was it a unique constraint on device name? now i'm confused | |
| 15:22:21 | lyarwood | mriedem: https://blueprints.launchpad.net/nova/+spec/remove-and-block-duplicate-bdms created that a while ago, wanted to catch up with melwitt or you about getting it approved etc this week | |
| 15:22:27 | dansmith | efried: the first bullet under "if there is no record" mentions stuff about user_id and project_id, but I'm missing why that's related | |
| 15:22:44 | mriedem | lyarwood: we don't need a blueprint for a bug fix | |