Skip to content
This repository has been archived by the owner on Jun 19, 2022. It is now read-only.

Ingress Readiness and Liveness check stop working after an upstream change #1973

Closed
grac3gao-zz opened this issue Dec 3, 2020 · 3 comments
Closed
Assignees
Labels
area/broker kind/bug Something isn't working priority/1 Blocks current release defined by release/* label or blocks current milestone release/2
Milestone

Comments

@grac3gao-zz
Copy link
Contributor

grac3gao-zz commented Dec 3, 2020

Describe the bug
Ingress readiness and liveness (using kncloudevents library from eventing) check stop working after an upstream change: knative/eventing#3917

Expected behavior
A clear and concise description of what you expected to happen.

To Reproduce
change the following line to return response.WriteHeader(nethttp.StatusMethodNotAllowed)

if request.URL.Path == heathCheckPath {
response.WriteHeader(nethttp.StatusOK)
return

It should fail the readiness and liveness check for the ingress pod, however, ingress pod will continue work.

Revert the upstream change, then return response.WriteHeader(nethttp.StatusMethodNotAllowed) , readiness and liveness check will work.

Knative-GCP release version

Additional context
Add any other context about the problem here such as proposed priority

@grac3gao-zz grac3gao-zz added kind/bug Something isn't working area/broker labels Dec 3, 2020
@grac3gao-zz grac3gao-zz changed the title Broker Readiness and Liveness check stop working after an upstream change Ingress Readiness and Liveness check stop working after an upstream change Dec 7, 2020
@grac3gao-zz grac3gao-zz self-assigned this Dec 7, 2020
@grantr grantr added the priority/1 Blocks current release defined by release/* label or blocks current milestone label Dec 8, 2020
@grantr grantr added this to the V0.20.0-M3 milestone Dec 8, 2020
@grac3gao-zz
Copy link
Contributor Author

kncloudevents are using drainer in pkg to run liveness and readiness check now (https://github.com/knative/pkg/blob/d5c09d2aef18eaabbb9ce3ac71a487288ca542aa/network/handlers/drain.go#L78), all inbound request will firstly send through drainer then to the handler we configured. Drainer will take every request from k8s probe and truncated it instead of continuing sending it to our handler, which makes all our customized configuration for liveness and readiness useless as no request from k8s probe will be send into handler.
There are two ways to fix it:

  1. In the upstream drainer library, adding an interface for a customizedCheck function. Drainer will need to run customizedCheck for probe requests instead of directly returning 200. Then, change kncloudevents to also have a field for customizedCheck to initialize drainer.
  2. Have our own message_receive which is mostly same as kncloudevents except wrapping the drainer to run some customized check.

@grac3gao-zz
Copy link
Contributor Author

knative-pkg PR merged: knative/pkg#1977

@grac3gao-zz
Copy link
Contributor Author

Knative-eventing PR merged: knative/eventing#4707

Now we are OK to add customized liveness and readiness check

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/broker kind/bug Something isn't working priority/1 Blocks current release defined by release/* label or blocks current milestone release/2
Projects
None yet
Development

No branches or pull requests

2 participants