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

Need a plan of the KIngress prober for HTTPRoute #18

Closed
nak3 opened this issue Mar 28, 2021 · 14 comments · Fixed by #672 or #693
Closed

Need a plan of the KIngress prober for HTTPRoute #18

nak3 opened this issue Mar 28, 2021 · 14 comments · Fixed by #672 or #693
Assignees
Labels
lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.
Milestone

Comments

@nak3
Copy link
Contributor

nak3 commented Mar 28, 2021

KIngress has prober function but HTTPRoute does not have it.
Additionally, the HTTPRoute's "admitted" status will not work as it is different from "Ready" status. Please see - https://kubernetes.slack.com/archives/CR0H13KGA/p1611865803004000

@markusthoemmes
Copy link
Contributor

This is super important for the semantics of our Ingress, but I honestly have no clue on how to generically solve this.

We kind of have a same'ish problem in Openshift where we have a separate routing plane that gets configured for every Knative ingress. The networking setup of these routes can change from cluster to cluster and there is no guarantee that the knative-serving namespace can even reach those routers. There's also no generic way of listing all possible routers which might even be worse with IngressV2, seeing as there will be different implementations on it.

Ideas super welcome 😅

@howardjohn
Copy link

Previously all of the net-xyz controllers did their own probing, right? So the closest match to this would be to have all IngressV2 implementors do the same, by pushing for a true "Ready" status field in the API?

That seems like the most pure way to implement it - however, I have some pretty major concerns about implementors actually implementing it correctly for knative semantics. AFAIK no Envoy based implementations would be able to do this without a lot of efforts, which accounts for ~half of the currently implementors I think.

@nak3
Copy link
Contributor Author

nak3 commented Mar 30, 2021

Previously all of the net-xyz controllers did their own probing, right?

Yes but strictly speaking, Knative has a common library for probing as:

https://github.com/knative/networking/blob/45fe1f5dd35eaebaf6f8ce03ab0983ede9497369/pkg/status/status.go#L135

and net-xyz controllers just calls it like:

(e.g. net-istio)
https://github.com/knative-sandbox/net-istio/blob/c4e24bc7750441150ef56e8ece9c937661993c9d/pkg/reconciler/ingress/controller.go#L131-L140
https://github.com/knative-sandbox/net-istio/blob/c4e24bc7750441150ef56e8ece9c937661993c9d/pkg/reconciler/ingress/ingress.go#L230-L233

So the closest match to this would be to have all IngressV2 implementors do the same, by pushing for a true "Ready" status field in the API?

Yes, that's correct. The perfect solution is that Ingress v2 supports Ready status in their API but I think they will not accept it due to the reason you mentioned.

@markusthoemmes
Copy link
Contributor

@howardjohn I guess IngressV2 implementors could do the same probing we do in Knative to achieve the same effect. I'm aware that Envoy doesn't give you the "I"m actually ready to serve this config" signal via its config, which is a bummer. The difference is that the IngressV2 implementor intimitaly knows the router while Knative only knows the IngressV2 plane, which makes probing on our end brittle.

@howardjohn
Copy link

I'm aware that Envoy doesn't give you the "I"m actually ready to serve this config" signal via its config, which is a bummer

It does give you an ACK. In theory, this means it is ready to serve - I have not put the theory to test though.

I think it may be challenging for IngressV2 implementations to do probing since they need to handle arbitrary configs, whereas Knative has full control and a bounded set of possible routes defined, etc. I suppose you could probably always add some route not defined by the user with some special header or other match clause that is a NOP perhaps... but even then, there is no guarantee for all implementations that the control plane can even reach the data plane.

@markusthoemmes
Copy link
Contributor

Right, exactly. That "not guaranteed to reach the data plane" is the problem we'll have in Knative too 😢

@github-actions
Copy link
Contributor

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 30, 2021
@nak3 nak3 removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 13, 2021
@github-actions
Copy link
Contributor

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 12, 2021
@nak3 nak3 added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Oct 12, 2021
@nak3 nak3 added this to the v1.4.0 milestone Feb 21, 2022
@nak3 nak3 modified the milestones: v1.4.0, v1.6.0 Jun 10, 2022
@carlisia carlisia modified the milestones: v1.6.0, v1.7.0, Icebox Aug 5, 2022
@carlisia carlisia moved this to Icebox in Gateway API Roadmap Aug 10, 2022
@dprotaso
Copy link
Contributor

dprotaso commented Sep 26, 2023

Note this GEP will allow us to include a label for in cluster gateway deployments. If we can lookup the service by the label then we have endpoints of the proxies we can probe.

kubernetes-sigs/gateway-api#1757

@dprotaso
Copy link
Contributor

^ the above in my mind is a fallback

@dprotaso
Copy link
Contributor

/assign @dprotaso

@dprotaso dprotaso modified the milestones: Icebox, v1.14.0 Feb 21, 2024
@dprotaso
Copy link
Contributor

It might also be possible to do probing without requests hitting the activator/queue proxy

eg. match on the probe header and do a redirect
https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io%2fv1.HTTPRequestRedirectFilter

@dprotaso
Copy link
Contributor

Though we would lose an assurance that there's an activator/queue proxy in the data path.

@dprotaso
Copy link
Contributor

dprotaso commented Apr 3, 2024

I've written a proposal here on how we can include endpoint probing - 2024 - Gateway API Endpoint Probing

It's more or less the same approach as we took in Contour - though trying to accomplish this with a single resource

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.
Projects
No open projects
Status: Icebox
5 participants