| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-05-07 | |||
| 18:03:28 | bnemec | Healthcheck is pluggable. | |
| 18:03:33 | sean-k-mooney | then override process_request https://opendev.org/openstack/oslo.middleware/src/branch/master/oslo_middleware/healthcheck/__init__.py#L544 | |
| 18:04:02 | sean-k-mooney | bnemec: yes nova does not use the middelware at all by default | |
| 18:04:37 | sean-k-mooney | bnemec: but if we start enabling it we would like something more advanced that check if critical serivce like the scheduler and condcutoer were alive | |
| 18:04:45 | sean-k-mooney | rather then jsut saying yes the api is alive | |
| 18:05:03 | bnemec | Right, I'm saying you don't need to reimplement it, just write a plugin for those checks. | |
| 18:05:34 | sean-k-mooney | bnemec: right and we woudl do that by inheritinbg and then overriding process_request? | |
| 18:05:45 | sean-k-mooney | bnemec: do you have an example we could follow? | |
| 18:06:05 | bnemec | No, it's a stevedore-based plugin system. You just register your plugin in the appropriate namespace. | |
| 18:06:27 | bnemec | https://github.com/openstack/oslo.middleware/blob/master/oslo_middleware/healthcheck/__init__.py#L388 | |
| 18:06:32 | sean-k-mooney | bnemec: sure but we dont kwno what the plugin should do | |
| 18:06:40 | sean-k-mooney | e.g. what i shoudl retrun | |
| 18:06:51 | sean-k-mooney | what the api for this is as a user | |
| 18:07:10 | bnemec | https://github.com/openstack/oslo.middleware/blob/master/oslo_middleware/healthcheck/pluginbase.py | |
| 18:07:34 | sean-k-mooney | ah perfect | |
| 18:07:42 | sean-k-mooney | so we have to retrun a HealthcheckResult | |
| 18:08:18 | bnemec | Right | |
| 18:08:37 | sean-k-mooney | there are some examples of the respocnes here too https://opendev.org/openstack/oslo.middleware/src/branch/master/oslo_middleware/healthcheck/__init__.py#L68-L163 | |
| 18:10:55 | sean-k-mooney | bnemec: anyway im sure gmann_afk will figure it out or be in touch | |
| 18:11:32 | sean-k-mooney | bnemec: what i was more interested was if there was a standard way to report erros and infomation when it not healty | |
| 18:12:25 | sean-k-mooney | if not and we ant it to be consumable in an automated fashion then we would have too treat it like a version notification or other api resonce and managen and document it in our api ref | |
| 18:13:31 | sean-k-mooney | https://opendev.org/openstack/oslo.middleware/src/branch/master/oslo_middleware/healthcheck/disable_by_file.py i guess are the example plugins | |
| 18:13:35 | bnemec | Yep, versioning these checks is definitely something we should work out on the spec. | |
| 18:14:11 | bnemec | Looks like it, yeah. | |
| 18:15:20 | sean-k-mooney | yeah so other then establishing conventiosn this look like its not much code to add on our side to have a funtioning plugin | |
| 18:15:41 | sean-k-mooney | then we can just make the healt check more advanced and imporve the info we return iteritivly | |
| 18:19:20 | dansmith | I'm surprised that nobody thinks that exposing python version, gcc version, distro, version, kernel version, etc to everyone unauthenticated is not a huge problem | |
| 18:20:02 | dansmith | because all those things are included in the healthcheck response without any recourse, as far as I can tell | |
| 18:20:18 | artom | dansmith, only if you configure detailed=True in the paste file | |
| 18:20:26 | artom | Which nobody should ever do | |
| 18:20:45 | dansmith | ah, okay, I thought that was a query param | |
| 18:21:03 | artom | It isn't - at least, based on what I saw | |
| 18:22:07 | mriedem | if your healthcheck api would give me some credentials i could do some health checking of my own, thanks | |
| 18:22:10 | dansmith | so the details of the healthchecks aren't either, other than the reason thing | |
| 18:25:41 | dansmith | okay, so in that case, the config causes us to return different data types | |
| 18:25:50 | artom | My own personal healthcheck api is to down a pint of beer. Still awak = 200 OK, pass out = 503 service unavailable | |
| 18:26:04 | dansmith | which is unfortunate.. if nobody should ever turn on detailed=true, then hopefully nothing will ever see the conflicting structure, but.. | |
| 18:26:38 | artom | dansmith, I suppose you could if you're absolutely certain your Nova API endpoints aren't accessible from any unsafe network | |
| 18:26:48 | artom | Like, you trust your firewalls 110% | |
| 18:27:04 | dansmith | hmm, I don't see the if detailed switch in the html version tough | |
| 18:28:09 | dansmith | so the html version is always detailed? | |
| 18:28:43 | dansmith | or maybe the default template doesn't include those things? | |
| 18:29:39 | dansmith | oh I see, conditionals in the template, although I can't eye-parse what that all looks like by default | |
| 18:30:30 | dansmith | ah, I see the if detailed at the top | |
| 18:33:24 | sean-k-mooney | dansmith: so yes detailed is a vaild security consern. im not sure if we are required to exose any of the detailed info you listed | |
| 18:33:46 | sean-k-mooney | dansmith: e.g. if we can retrun different detailed infor per service that is less scare form a security point of view | |
| 18:33:56 | sean-k-mooney | liek the equivalent of a service list | |
| 18:34:03 | dansmith | sean-k-mooney: no, it looks like that's all off unless show_details, but it's one switch to turn on details which then exposes a ton of internals, which may surprise someone | |
| 18:35:14 | sean-k-mooney | ok so other then the last hour or so i have never bother looking at this in detail other then being aware it was configurable via api-paste.ini | |
| 18:35:43 | sean-k-mooney | so im ok with the idea of a healt check api endpoint. but im not sure it we want to use teh default implemantion | |
| 18:35:48 | dansmith | right, well, the fact that it is configurable with api-paste means this is not an emergency :) | |
| 18:35:51 | sean-k-mooney | them more i lean about it the mor ei think no | |
| 18:36:02 | sean-k-mooney | yep | |
| 18:36:19 | sean-k-mooney | operatoer can add it today if they want. if the should is a different matter | |
| 18:36:37 | dansmith | so, it's totally fine to have a spec, get people on the same page here, and make sure we have a plan for how this happens now, soon, and later | |
| 18:36:39 | sean-k-mooney | i thikn with out detailed=true its ok but does not tell you much | |
| 18:36:40 | dansmith | right | |
| 18:37:01 | dansmith | sean-k-mooney: it tells you basically nothing about heath.. other than that, it's great! :) | |
| 18:37:08 | dansmith | *health | |
| 18:37:43 | sean-k-mooney | ya it tells you apache is running or well the wsgi app | |
| 18:37:56 | sean-k-mooney | it does not tell you if nova is working | |
| 18:38:15 | sean-k-mooney | which is where wrighing our own plugin would come in | |
| 18:40:51 | sean-k-mooney | dansmith: since your about can you take a look at https://review.opendev.org/#/c/724188/4 and the follow up | |
| 18:41:10 | sean-k-mooney | its jsut silencing the amqp heartbeat warning | |
| 18:43:16 | dansmith | sean-k-mooney: omg, tell me you were the one that wrote "heartbeart" originally | |
| 18:43:34 | sean-k-mooney | hehe for one i think not | |
| 18:43:45 | dansmith | damn. | |
| 18:44:04 | sean-k-mooney | but i mean i would not put it past me that is a very sean thing to do | |
| 18:45:34 | dansmith | sean-k-mooney: jeez, I missed all the typos in the first commit message | |
| 18:46:09 | dansmith | sean-k-mooney: can we please get those fixed since you'regoing to backport I assume? | |
| 18:46:11 | sean-k-mooney | do you want to comment on them | |
| 18:46:17 | dansmith | lemme just fix inline | |
| 18:46:17 | sean-k-mooney | i can can fix them up with the other one | |
| 18:46:46 | sean-k-mooney | dansmith: ya we have a downstream bug for osp 13 so im going to back prot this upstream as far as people are ok with | |
| 18:47:25 | sean-k-mooney | dansmith: want to remvoe the +w on the first patch and ill fix all typos and resubmit shortly | |
| 18:47:36 | openstackgerrit | Dan Smith proposed openstack/nova master: Silence amqp heartbeat warning https://review.opendev.org/724188 | |
| 18:47:46 | dansmith | sean-k-mooney: I just edited the bottom one to kick it out | |
| 18:47:53 | sean-k-mooney | ah ok cool | |
| 18:47:54 | dansmith | sean-k-mooney: I can do the top if you want or you can | |
| 18:48:02 | dansmith | sean-k-mooney: look over that edit I made and then I'll +W | |
| 18:48:56 | sean-k-mooney | damb it i misspelled heartbeats lol | |
| 18:49:17 | dansmith | you also misspelled "damn" :P | |
| 18:50:28 | dansmith | sean-k-mooney: okay re +Wd the bottom one | |
| 18:50:45 | sean-k-mooney | i am honestly surpised i write code that works sometimes. if it wasnt for the fact im consitet in adjcent lines it totally would not work | |
| 18:51:05 | dansmith | I honestly have no idea how you do, so.. there's that at least :) | |
| 18:51:20 | dansmith | keep working on that compiler for English and you'll be set :) | |
| 18:51:36 | dansmith | (and rich) | |
| 18:51:44 | sean-k-mooney | :) | |
| 18:56:13 | openstackgerrit | sean mooney proposed openstack/nova master: remove support of oslo.messaging 9.8.0 warning message https://review.opendev.org/724725 | |
| 18:56:32 | sean-k-mooney | dansmith: stephenfin ^ ok that should have the typos fixed | |
| 18:56:49 | dansmith | no need to wake him, I'll get it | |
| 18:57:53 | sean-k-mooney | are we still holding backport to stable/ussuri until the upstream release by the way | |
| 18:58:33 | sean-k-mooney | this obviosly isnt a RC candiate but ill start preparing the backports tomorow and we can merge them whenever | |
| 18:59:12 | melwitt | yeah there shouldn't be non RC related backports to stable/ussuri until after GA IIRC | |
| 18:59:21 | melwitt | merged I mean | |
| 18:59:36 | sean-k-mooney | cool ill mark the backport as workflow -1 untill after then | |
| 18:59:59 | melwitt | +1 | |
| 19:00:42 | sean-k-mooney | ok im going to get something to eat ttyl | |
| 20:21:29 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Moving functional jobs to Victoria testing runtime https://review.opendev.org/725372 | |
| 21:50:33 | openstackgerrit | Merged openstack/nova stable/rocky: Add retry to cinder API calls related to volume detach https://review.opendev.org/725272 | |