| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-05-05 | |||
| 15:26:13 | zigo | bauzas: I know how to do it, but if you have time, that'd be great. I'm a bit swamped testing the fix for [censored-because-embargoed]. | |
| 15:26:23 | bauzas | kk | |
| 16:34:23 | openstackgerrit | Sylvain Bauza proposed openstack/nova master: Add a /healthcheck URL https://review.opendev.org/724684 | |
| 16:34:53 | bauzas | zigo: been there, done that ^ | |
| 17:49:10 | artom | dansmith, o/ Could you look over the NUMA LM fup patch: https://review.opendev.org/#/c/724903/ ? | |
| 17:50:17 | dansmith | yup in a bit | |
| 17:56:56 | gmann | bauzas: zigo +1 on idea of healthcheck but there are few things we should discuss. like adding nova specific backends like DB checks, compute service checks etc. how about adding a spec for that so that we do not miss the things. | |
| 18:00:02 | gmann | added in PTG etherpad also. | |
| 18:32:48 | gmann | cores- this is ready. making py38 functional job on gate - https://review.opendev.org/#/c/725372/ | |
| 18:52:19 | dansmith | artom: I really hate the assertTrue($not_a_bool) and I thought we even had hacking rules to prevent tat | |
| 18:52:29 | dansmith | artom: wouldn't assert_is_instance be better? | |
| 18:53:02 | artom | dansmith, it would | |
| 18:53:29 | dansmith | artom: wanna change it real quick and I'll fast approve assuming stephenfin would be fine with it given it still fits the pattern he wanted? | |
| 18:53:55 | artom | dansmith, on it | |
| 18:54:02 | dansmith | thanks | |
| 19:02:34 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: Follow-up for NUMA live migration functional tests https://review.opendev.org/724903 | |
| 19:02:43 | artom | dansmith, ^^ there we go | |
| 19:03:21 | artom | Hol'up, I did a stupid | |
| 19:04:51 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: Follow-up for NUMA live migration functional tests https://review.opendev.org/724903 | |
| 19:05:14 | artom | OK, stupid un-did | |
| 19:05:21 | dansmith | didn't you miss a bunch? | |
| 19:05:58 | artom | Of assertTrues? | |
| 19:06:25 | artom | Did I? There are some left, but they're actually checking booleans | |
| 19:06:57 | dansmith | artom: but.. assertFalse should be assertIsNone, no? | |
| 19:08:45 | artom | That implies I'm capable of thinking for myself about the larger picture | |
| 19:09:24 | dansmith | aren't you just a small search-and-replace lamba function in the cloud somewhere? | |
| 19:10:02 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: Follow-up for NUMA live migration functional tests https://review.opendev.org/724903 | |
| 19:10:32 | artom | :( | |
| 19:11:16 | artom | My inputs are beer and hugs, and my output is sadness | |
| 19:11:24 | dansmith | lol | |
| 19:12:25 | dansmith | artom: before I whack this, you agree that this jives with stephenfin's ask right? I don't want to be reverting his vote | |
| 19:12:49 | artom | Yeah, I don't see stephenfin disputing this, whack away | |
| 19:13:12 | artom | You have our collective permission to do so | |
| 19:13:20 | dansmith | doneski | |
| 19:14:17 | artom | \o/ | |
| 19:27:56 | openstackgerrit | Jiri Suchomel proposed openstack/nova master: Add ability to download Glance images into the libvirt image cache via RBD https://review.opendev.org/574301 | |
| 19:45:42 | zigo | gmann: I very much agree that all you say "would be nice", though the situation *before* this patch is even worse. There's nowhere where Haproxy can ask the API "are you up". | |
| 19:46:29 | zigo | Operators have the choice of either configuring nothing, and then haproxy just opens a connection to the API port, and then closes it. This is very ugly, and makes lots of very bad entries in the logs. | |
| 19:46:58 | zigo | Or operators can query "/" which gives a json with versions in return, and is not designed for all of this. | |
| 19:47:33 | zigo | So yeah, I agree with you that it'd be nice to check many things in the /healthcheck and improve it, but *after* this patch is merged. | |
| 19:48:18 | zigo | BTW, we can actually disable the /healthcheck, it's enabled by default. | |
| 19:48:31 | zigo | It'd be just by creating a file, as per disable_by_file_path = /var/run/nova/healthcheck_disable in the patch. | |
| 19:48:39 | zigo | Which by the way is at the wrong location, IMO... | |
| 19:48:43 | zigo | It should be in /etc/nova. | |
| 19:48:47 | zigo | I'll change the path ... | |
| 19:48:51 | zigo | (right away now) | |
| 19:51:41 | openstackgerrit | Thomas Goirand proposed openstack/nova master: Add a /healthcheck URL https://review.opendev.org/724684 | |
| 19:55:21 | artom | gmann, zigo, FWIW I'm in favor of the /healthcheck URL (and I feel zigo's bandwidth as an operator). My concern would be around 1. documentation 2. openstack-wide uniformity | |
| 19:55:36 | artom | So for instance, keystone has it as well, but IIUC you can't disable there via a file | |
| 19:56:22 | zigo | artom: I intend to patch other projects. Neutron has already approved my patch, and got some +1 on other projects. | |
| 19:56:35 | zigo | I can add more /etc/<FOO>/healthcheck_disable | |
| 19:57:04 | zigo | As for documentation, well, maybe this goes into oslo.middleware ? | |
| 19:57:36 | zigo | Do you have any other place more OpenStack wide to suggest ? | |
| 19:57:40 | gmann | zigo: artom yeah, and there are way to add detail in response via conf, all those we can document. | |
| 19:57:50 | artom | zigo, well... Maybe? The admins will be reading the project guides, not oslo.middleware docs... | |
| 19:58:19 | gmann | zigo: but how about API is there but no DB connection? i mean DB check can be generic and can be done on oslo side | |
| 19:58:34 | zigo | I don't feel like we should document this on each and every project *IF* we have consistency across projects. | |
| 19:59:03 | zigo | gmann: That's a call for adding more code in oslo.middleware then, no ? | |
| 19:59:05 | artom | zigo, fair point | |
| 19:59:27 | bnemec | gmann: That's the sort of thing mugsie suggested a while back. | |
| 19:59:29 | artom | Is the docs SIG/WG still a thing? The would know... | |
| 19:59:31 | gmann | zigo: yeah on oslo.middleware side. | |
| 19:59:37 | artom | *They | |
| 19:59:50 | bnemec | Basically the ability to do more than just HTTP API checks, say db, messaging, maybe libvirt for Nova. | |
| 19:59:51 | gmann | artom: doc SIG is there. | |
| 20:00:23 | artom | gmann, so they'd be the ones coming up with where to put this doc, if we get openstack-wide uniformity | |
| 20:00:35 | gmann | bnemec: yeah, libvirt specific can be done as nova defined backend. but yeah good to have db, messaging things on oslo.middleware | |
| 20:01:05 | artom | gmann, but... which DB? If the deployment is multicell, just the nova_api one? | |
| 20:01:21 | artom | It's a rathole that I'm not sure we want to go down into, for something this simple | |
| 20:01:30 | artom | /healthcheck is better than nothing, even if it's not perfect | |
| 20:01:38 | gmann | artom: is it technical writing SIG not doc sig - https://governance.openstack.org/sigs/ | |
| 20:01:48 | bnemec | artom: That was our conclusion too last time we discussed it. | |
| 20:02:11 | bnemec | If we try to boil the ocean and healthcheck all the things this will never get done at all. | |
| 20:02:51 | gmann | yeah deployment model is one challenge for such checks | |
| 20:03:16 | artom | gmann, and presumably we can improve this in the future, no? | |
| 20:03:34 | artom | So maybe just version is (somehow) for now, and leave is as the simple middleware thing | |
| 20:03:48 | artom | And eventually add /healthcheck/v2 or whatever, which can be smarter | |
| 20:04:01 | artom | *version it | |
| 20:04:22 | gmann | url can be same and configured backends by operator can enhance the checks. | |
| 20:07:50 | gmann | zigo: i think we need separate new config on nova side if we want to disable it right? bnemec can correct me. | |
| 20:08:09 | gmann | otherwise if middleware is added then it is enabled by defualt | |
| 20:08:12 | gmann | default | |
| 20:10:01 | bnemec | If we want to make healthcheck runtime configurable then it would be preferrable if we could do it in oslo. Otherwise you end up with a half dozen differen opts in different projects designed to do the same thing. | |
| 20:11:08 | gmann | bnemec: yeah. i am worried to have enable by default because it is unauthenticated endpoint. | |
| 20:12:21 | artom | gmann, what's the harm though? | |
| 20:13:08 | artom | Like, OK, anyone can spam that, but so can anyone spam any other API and get a flood of 403s back | |
| 20:13:33 | artom | The disk of DOS (if that's your concern) is greater with the second one, because keystone has to actually do work | |
| 20:13:58 | artom | *risk of | |
| 20:14:42 | bnemec | My understanding was the unauthenticated healthchecks are generally okay. Most of the stuff calling them doesn't have auth capability anyway, so you don't have much choice. | |
| 20:15:07 | gmann | artom: well it is still security risk to know services running on that node | |
| 20:15:28 | artom | gmann, security by obscurity is Real Bad (TM) ;) | |
| 20:15:49 | gmann | yeah i am not suggesting to make it auth but disable by default and let operator to enable it by knowing this is unauth things. | |
| 20:15:50 | artom | If knowing that nova-api is running somewhere is a risk, we're doing something horribly wrong | |
| 20:16:48 | bnemec | How would you prevent them from knowing the service is there? A simple port scan would tell them that. | |
| 20:17:07 | artom | gmann, off by default works for me. But that only reinforces the doc problem - admins have to know how to enable it, and more fundamentally that it's available | |
| 20:18:18 | gmann | artom: doc anyways we might need to tell at least it is there now in nova and refer oslo doc for all details and config options. | |
| 20:19:56 | gmann | and there is no harm of doing those doc of 2-3 line in all projects start implementing that and point to oslo doc for details. because we do not have any central doc place now for openstack operators/admin etc as general | |
| 20:21:36 | artom | gmann, fair point | |
| 20:21:58 | gmann | knowing service can be hacked by multiple way. but is it fine to have this unatuh endpoint for operator without no way to disable it ? may be zigo can tell. | |
| 20:22:36 | gmann | i felt giving the enable/disable option for them does not harm. disable by default or enable that can be disucssed whatever is more common. | |