Skip to content

Proposal: basic health check before declaring a Revision Ready #2674

New issue
New issue
Closed
Closed
Proposal: basic health check before declaring a Revision Ready#2674
Labels
area/APIAPI objects and controllerskind/featureWell-understood/specified features, ready for coding.
Milestone
Serving "v1" (ready for production)

Description

@steren
steren
opened on Dec 7, 2018

/area API
/kind feature

Problem statement

Today, if I deploy a container image that is not binding to the right port or that crashes at startup, the revision is successfully created and serving traffic (in case I am in runLatest mode). Of course, trying to access it results in a failure: "upstream request timeout".

Proposal

Before marking a new Revision with Ready=True (ready to serve traffic), we implement a basic health check by ensuring that is successfully binds to $PORT.

If, after a given timeout (e.g. 4 minutes), the container is not listening on $PORT, we consider the Revision unhealthy, we mark is as Ready=False and we do not route traffic to it if the service is in runLatest mode.

Activity

  1. knative-prow-robot
    added
    area/APIAPI objects and controllers
    kind/featureWell-understood/specified features, ready for coding.
    on Dec 7, 2018

  2. tcnghia commented on Dec 8, 2018

    @tcnghia
    tcnghia
    on Dec 8, 2018
    Contributor

    @steren do you have ReadinessProbe for your application? Something like this

        spec:
      container:
        image: your-image
        readinessProbe:
          httpGet:
            path: /

  3. steren commented on Dec 8, 2018

    @steren
    steren
    on Dec 8, 2018
    ContributorAuthor

    Thanks for pointing this out. But this proposal is more general than for my personal app:

    The idea is to provide some readiness check for users who do not implement a readiness probe.

  4. mattmoor commented on Feb 11, 2019

    @mattmoor
    mattmoor
    on Feb 11, 2019
    Contributor

    @tcnghia this is about creating a default tcpProbe of the user container. cc @dgerd (as I think the runtime contract touches on this).

  5. mattmoor
    added this to the Serving "v1" (ready for production) milestone on Feb 11, 2019

  6. tcnghia commented on Feb 11, 2019

    @tcnghia
    tcnghia
    on Feb 11, 2019
    Contributor

    This is fixed now in 0.4.0, since queue-proxy health check also probes the user-container. In addition to that we always configure health check for queue-proxy.

  7. mattmoor
    closed this as completedon Feb 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/APIAPI objects and controllerskind/featureWell-understood/specified features, ready for coding.

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions