Add queue readiness probe package #4668
Conversation
This PR builds the adapter which converts a user-defined probe to a probe that will be executed by queue proxy against user container. Co-authored-by: Shash Reddy <[email protected]>
|
So there's good news and bad news. 👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there. 😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request. Note to project maintainer: This is a terminal state, meaning the ℹ️ Googlers: Go here for more info. |
knative-prow-robot
added
the
size/XL
There was a problem hiding this comment.
@joshrider: 1 warning.
In response to this:
Co-authored-by: Shash Reddy [email protected]
Supports #4014
Proposed Changes
- Package to support addition of user-defined ReadinessProbes executed by the queue-proxy against the user-container
- Will be used in PRs to follow
- See [WIP] Use user-defined readiness probes through queue-proxy #4600 for reference
Release Note
NONE
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
joshrider
changed the title
joshrider
changed the title
|
/assign @mattmoor |
Co-authored-by: Shash Reddy <[email protected]>
knative-prow-robot
added
the
do-not-merge/hold
joshrider
force-pushed
the
probe-readiness
branch
from
14bf105 to
98c88a1
Compare
knative-prow-robot
removed
the
do-not-merge/hold
|
|
||
| const ( | ||
| aggressiveProbeTimeout = 100 * time.Millisecond | ||
| // PollTimeout is set equal to the queue-proxy's ExecProbe timeout to take |
There was a problem hiding this comment.
are those used outside the package?
Co-authored-by: Shash Reddy <[email protected]>
|
The following is the coverage report on pkg/.
|
| @@ -116,7 +116,7 @@ func TestHTTPsSchemeProbeSuccess(t *testing.T) { | |||
| } | |||
| // Connecting to the server should work | |||
| if err := HTTPProbe(config); err != nil { | |||
| t.Errorf("Expected probe to succeed but it failed with %v", err) | |||
| t.Errorf("Expected probe to succeed but failed with error %v", err) | |||
There was a problem hiding this comment.
well, I guess this one as well 😄
|
/test pull-knative-serving-integration-tests |
|
/test pull-knative-serving-upgrade-tests |
knative-prow-robot
added
lgtm
mattmoor
added
cla: yes
|
A Googler has manually verified that the CLAs look good. (Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.) ℹ️ Googlers: Go here for more info. |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: joshrider, mattmoor The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
knative-prow-robot
added
the
approved
Co-authored-by: Shash Reddy [email protected]
Supports #4014
Proposed Changes
Release Note