| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-05-07 | |||
| 18:24:26 | dansmith | they already have an agent that sits on our bus and pings services, and unsurprisingly, have found that the version matters and things break if you don't have that right | |
| 18:25:11 | dansmith | and I just want to make sure we're not getting into a situation where we bless that kind of activity, and/or start trying to worry about external consumers of our RPC APIs and hold versions around because of what might be out in the wild | |
| 18:25:30 | dansmith | https://review.opendev.org/#/c/651140/ | |
| 18:28:16 | artom | I mean, we could be evil and just remove ping() | |
| 18:28:38 | dansmith | artom: we need it for our own use | |
| 18:28:51 | dansmith | and note that that error dump was on the *nova* server side, as a result of pinging with an old version, | |
| 18:29:01 | dansmith | so that tool potentially generates bug reports for us | |
| 18:29:29 | dansmith | artom: compute pings conductor at startup and waits until it replies, in case it gets started earlier, since it'll fail without conductor being up | |
| 18:29:51 | dansmith | I told them about ping because they wanted to add their own ping-like thing to our rpcapi that they would use to do that, | |
| 18:30:07 | dansmith | so I was like "we already have this, so please don't add another one, but also... about that approach..." | |
| 18:31:11 | artom | Wait, ping() is in the conductor rpcapi, right? | |
| 18:31:19 | artom | So... how are they going to check for Nova API health? | |
| 18:31:32 | artom | Which I assume is what they most care about... | |
| 18:31:53 | openstackgerrit | Chris Dent proposed openstack/nova master: Make all functional tests reusable by other projects https://review.opendev.org/657659 | |
| 18:33:27 | artom | Ah, it's in the base class | |
| 18:33:31 | artom | So all our services have it | |
| 18:34:18 | dansmith | they can't check api that way anyway | |
| 18:34:22 | dansmith | because api doesn't listen | |
| 18:34:32 | dansmith | but yeah, it's baked in for the services that do | |
| 18:35:24 | artom | So that's a good argument: you can only use ping() on services that listen to the message queue, which API doesn't. | |
| 18:35:32 | artom | And API is kinda important ;) | |
| 18:36:04 | dansmith | one could argue that you can test nova-api externally already ... via the api | |
| 18:36:21 | dansmith | but yeah, it's an argument.. the others I have are more important to me though :) | |
| 18:36:39 | artom | Right, I'm trying to think of what's important to *them* :) | |
| 18:36:47 | artom | Since you know, it's how you actually convince people ;) | |
| 18:39:01 | artom | Reading aspiers's reply on https://review.opendev.org/#/c/653707/2/specs/rpc-health-checks.rst@43, I can see where the self-healing SIG people are coming from. You want a think that tells you directly, with no proxy, whether a single service is healthy or not. | |
| 18:39:33 | artom | You then have your own intelligence on what to do. But you don't need further analysis to see where the actual failure is - well, less of it, anyways | |
| 18:39:49 | artom | Doing it through the RPC back door is one less layer to think about | |
| 18:40:24 | dansmith | it's dependent on the network and rabbit working though | |
| 18:40:45 | dansmith | and hides the visibility of what might be wrong on the network | |
| 18:41:06 | dansmith | like, if you get no route, you know the computer is down. if you get refused, you know the service is down | |
| 18:41:26 | dansmith | via rpc, you'll just not hear back, which could mean pretty much anything, including "the bus is overloaded right now" | |
| 18:42:03 | artom | Also true. | |
| 18:42:04 | dansmith | regardless, I don't want them to be on our bus as a matter of principle, aside from the fact that I think they'll get bad data from doing so | |
| 18:42:27 | artom | Anyways, I need to step out of this philosophical discussion, things need doing :) | |
| 18:42:31 | artom | Was fun thinking about, though | |
| 18:43:08 | openstackgerrit | Surya Seetharaman proposed openstack/nova master: Microversion 2.73: Support adding the reason behind a server lock https://review.opendev.org/648662 | |
| 18:44:58 | tssurya | mriedem: thanks again for the detailed review ^. I totally appreciate it. | |
| 18:49:51 | aspiers | dansmith: thanks for the quick and helpful reply. I think I'm totally fine with your suggestion of opening up new dedicated HTTP endpoints for health-checking RPC-only services, and the advantages you cite make sense | |
| 18:53:22 | aspiers | dansmith, artom: FWIW, the problem the OSH folks encountered was not due to pinging with an old version - it was due to not knowing ping() exists and therefore deliberately invoking a non-existent RPC call, which (unsurprisingly) caused errors in the server logs | |
| 18:53:43 | aspiers | The errors were actually masked for a while, but surfaced when something in oslo logging changed | |
| 18:54:10 | artom | aspiers, ah, yeah that makes more sense with "Attempted method: pod_health_probe_method_ignore_errors" | |
| 18:54:13 | dansmith | aspiers: it's the same thing that will happen if you do use ping, but with the wrong version | |
| 18:54:19 | artom | That's definitely not a method we have ') | |
| 18:54:20 | dansmith | which was my point | |
| 18:54:24 | aspiers | dansmith: gotcha | |
| 18:55:07 | artom | It does add an external thing to the upgrade process. Nova internally can support N/N+1 | |
| 18:55:36 | artom | With the proposed health check, suddenly this new external thing has be to upgraded at the same time as part of the Nova upgrade | |
| 18:55:41 | dansmith | artom: we actually only support N/N+1 internally for major versions on bridge releases | |
| 18:56:06 | dansmith | we support N.0-N.x on regular releases | |
| 18:56:22 | aspiers | artom: You lost me - what has to be upgraded? | |
| 18:56:29 | dansmith | this ^ :) | |
| 18:56:30 | artom | aspiers, and dansmith lost me :) | |
| 18:57:02 | artom | What's a bridge release? | |
| 18:57:25 | aspiers | Are you talking about a scenario where the ping() interface changes in future versions? | |
| 18:57:45 | dansmith | we haven't had one in a while, but when we bump the major, we support the old and new majors on the server side | |
| 18:57:53 | dansmith | it's a whole complex thing | |
| 18:58:35 | dansmith | aspiers: ping is unlikely to change, but it could, but even if it doesn't, you have to track the supported major in your tool else you'll generate errors like the one in the commit message when we bump, even though ping hasn't changed | |
| 18:59:13 | dansmith | and during a major bump release, you'd have to know which computes are upgraded to send the new major | |
| 18:59:13 | aspiers | dansmith: you mean because it won't support calls from (much) older versions? | |
| 18:59:19 | dansmith | and conductors don't support multiple majors | |
| 18:59:21 | artom | Ah, so my confusion stemmed from thinking release = major bump, which isn't the case | |
| 18:59:26 | dansmith | artom: right | |
| 18:59:38 | dansmith | aspiers: from anything other than major.0 right | |
| 18:59:49 | aspiers | OK got it (I think) | |
| 18:59:57 | artom | So technically queens computes can talk to stein conductors, because both are major 5 | |
| 19:00:50 | aspiers | even though I suspect the liveness probe would be packaged within the same container as the service it's checking | |
| 19:01:22 | artom | It would still have to be code-updated at the same time with the new major | |
| 19:01:26 | aspiers | right | |
| 19:01:31 | artom | Which is harder when it's not the same repo :) | |
| 19:01:35 | dansmith | this is why you shouldn't do this :) | |
| 19:01:52 | aspiers | whereas going through oslo.middleware (say) wouldn't have any of these problems | |
| 19:03:06 | dansmith | and we'd have a defined health check schema we agree on and support | |
| 19:03:11 | dansmith | i.e. list of tests | |
| 19:04:42 | aspiers | Hmm, re-reading the scrollback - did you say nova already exposes some health data on an existing endpoint? | |
| 19:05:09 | aspiers | I wasn't aware of that ... /me checks the API docs | |
| 19:05:18 | dansmith | no | |
| 19:05:19 | aspiers | Maybe it's just using the existing oslo.middleware mechanism | |
| 19:05:29 | dansmith | no, I'm saying we *should* do that | |
| 19:05:45 | dansmith | none of our services (other than API) even have http interfaces, so we need to do that thing | |
| 19:05:46 | aspiers | Ah, wrong interpretation of "this" --> http://eavesdrop.openstack.org/irclogs/%23openstack-nova/%23openstack-nova.2019-05-07.log.html#t2019-05-07T17:53:25 | |
| 19:05:51 | dansmith | I'm saying this is what we should be doing | |
| 19:06:15 | aspiers | Yeah OK, that's what I thought. Just got momentarily confused by the scrollback. | |
| 19:06:39 | aspiers | Sorry :) | |
| 19:09:14 | artom | I have to say, opening up internal services with an HTTP endpoint seems dangerous, security wise. | |
| 19:09:35 | artom | Depends on how it's implemented, I guess | |
| 19:09:44 | artom | But being able to DDoS the conductor doesn't seem fun | |
| 19:10:16 | aspiers | artom: I doubt OS-Helm has any intention of carrying out DDoS attacks ;-) | |
| 19:10:16 | artom | Tough I guess any self-respecting operator will firewall the crap out of those | |
| 19:10:34 | aspiers | Exactly. They probably don't even need to listen outside localhost | |
| 19:10:53 | aspiers | Remember this is k8s monitoring from the container host | |
| 19:10:56 | aspiers | or similar | |
| 19:11:04 | artom | aspiers, I don't doubt Helm's intentions, it's others I'm paranoid about | |
| 19:11:14 | aspiers | It won't be open to others | |
| 19:11:33 | aspiers | ... unless the operator wants it to be. We're on the same page :) | |
| 19:12:00 | dansmith | artom: it's read-only, has no real info, and you can make it depth-1, so literally only one request at a time | |
| 19:12:11 | aspiers | Right | |
| 19:12:46 | artom | dansmith, what, you mean dumping a detailed 1MB JSON blob of stats isn't a good idea? ;) | |
| 19:13:00 | aspiers | It actually links up quite nicely with dirk's idea of exposing more internal metrics for consumption by e.g. Prometheus | |
| 19:14:18 | aspiers | Anyway, gotta go. Back tomorrow. | |