knative / serving

Kubernetes-based, scale-to-zero, request-driven compute
https://knative.dev/docs/serving/
Apache License 2.0
5.52k stars 1.15k forks source link

Allow liveness probes via queue-proxy when listening on localhost #12940

Open mbrancato opened 2 years ago

mbrancato commented 2 years ago

Describe the feature

Currently, all liveness probes are sent to the service container, which is expected to be listening on 0.0.0.0 on the container port. There is no reason that the service container needs to listen on all interfaces if all traffic is to be proxied via queue-proxy anyway. It would be good to support services that listen on localhost and to which queue-proxy will proxy the requests.

Today, the following config will send all liveness probes to the container, and readiness probes seem to be ignore or handled by the queue proxy (not sure, might be a separate issue).

          ports:
            - containerPort: 80
          livenessProbe:
            httpGet:
              path: /healthz
          readinessProbe:
            httpGet:
              path: /readyz
Containers:
  my-app:
    Liveness:  http-get http://:80/healthz delay=0s timeout=1s period=10s #success=1 #failure=3
  queue-proxy:
    Readiness:  http-get http://:8012/ delay=0s timeout=1s period=1s #success=1 #failure=3

It would be nice if we didn't have to expose the service on the pod network for this to work by allowing the queue-proxy to handle both liveness and readiness.

e.g.

Containers:
  my-app:
    Liveness:  http-get http://:8012/healthz delay=0s timeout=1s period=10s #success=1 #failure=3
  queue-proxy:
    Readiness:  http-get http://:8012/ delay=0s timeout=1s period=1s #success=1 #failure=3
github-actions[bot] commented 2 years ago

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.

psschwei commented 2 years ago

/lifecycle frozen

mbrancato commented 11 months ago

Adding to this, I identified that the previous revisions will fail to scale down completely if listening on localhost.