Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Readiness probe based on number of endpoints #590

Closed
markmandel opened this issue Sep 10, 2022 · 0 comments · Fixed by #591
Closed

Readiness probe based on number of endpoints #590

markmandel opened this issue Sep 10, 2022 · 0 comments · Fixed by #591
Labels
area/operations Installation, updating, metrics etc kind/feature New feature or request

Comments

@markmandel
Copy link
Member

Is your feature request related to a problem? Please describe.

Since we are putting Quilkin behind UDP Load Balancers in K8s, we should have a readiness endpoint, so that Services know if the proxy is ready to receive traffic or not.

This I feel is particularly true when scaling up a proxy that is connected to an xDS resource, but has yet to receive endpoint instructions yet.

Describe the solution you'd like

I think the easiest option is to have a /ready admin http probe that returns 200 when the proxy has 1 or more endpoints configured.

If the proxy has no endpoints, it will return StatusCode::INTERNAL_SERVER_ERROR.

That way new Proxies that have yet to receive their endpoints from the xDS server won't get sent data that won't go anywhere.

Also if no proxies have endpoints, there's no point for the Service to send data to a proxy anyway, since it's not going anywhere.

Describe alternatives you've considered

We could leave things as is, and rely on the eventual consistency, but I think that will potentially put unnecessary delays in UDP network streams / connections.

Additional context

quilkin/src/admin.rs

Lines 47 to 71 in 25d9360

fn handle_request(request: Request<Body>, config: Arc<Config>, health: Health) -> Response<Body> {
match (request.method(), request.uri().path()) {
(&Method::GET, "/metrics") => collect_metrics(),
(&Method::GET, "/live") => health.check_healthy(),
(&Method::GET, "/config") => match serde_json::to_string(&config) {
Ok(body) => Response::builder()
.status(StatusCode::OK)
.header(
"Content-Type",
hyper::header::HeaderValue::from_static("application/json"),
)
.body(Body::from(body))
.unwrap(),
Err(err) => Response::builder()
.status(StatusCode::INTERNAL_SERVER_ERROR)
.body(Body::from(format!("failed to create config dump: {err}")))
.unwrap(),
},
(_, _) => {
let mut response = Response::new(Body::empty());
*response.status_mut() = StatusCode::NOT_FOUND;
response
}
}
}

@markmandel markmandel added kind/feature New feature or request area/operations Installation, updating, metrics etc labels Sep 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/operations Installation, updating, metrics etc kind/feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant