-
Notifications
You must be signed in to change notification settings - Fork 420
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
Status Conditions for EventListener #932
Labels
kind/api-change
Categorizes issue or PR as related to adding, removing, or otherwise changing an API
kind/feature
Categorizes issue or PR as related to a new feature.
Milestone
Comments
dibyom
added
kind/api-change
Categorizes issue or PR as related to adding, removing, or otherwise changing an API
kind/feature
Categorizes issue or PR as related to a new feature.
labels
Jan 27, 2021
4 tasks
/assign |
4 tasks
dibyom
added a commit
to dibyom/triggers
that referenced
this issue
May 10, 2021
The Ready Status is true when the other 4 status conditions - (Service, Deployment, Progessing, and Available) are all true. The idea is that in the future, Ready would be the one Condition users could use to see if an EventListener is ready to serve traffic or not. Fixes tektoncd#932 Signed-off-by: Dibyo Mukherjee <dibyo@google.com>
dibyom
added a commit
to dibyom/triggers
that referenced
this issue
May 10, 2021
The Ready Status is true when the other 4 status conditions - (Service, Deployment, Progessing, and Available) are all true. The idea is that in the future, Ready would be the one Condition users could use to see if an EventListener is ready to serve traffic or not. Fixes tektoncd#932 Signed-off-by: Dibyo Mukherjee <dibyo@google.com>
dibyom
added a commit
to dibyom/triggers
that referenced
this issue
May 11, 2021
The Ready Status is true when the other 4 status conditions - (Service, Deployment, Progessing, and Available) are all true. The idea is that in the future, Ready would be the one Condition users could use to see if an EventListener is ready to serve traffic or not. Fixes tektoncd#932 Signed-off-by: Dibyo Mukherjee <dibyo@google.com>
tekton-robot
pushed a commit
that referenced
this issue
May 12, 2021
The Ready Status is true when the other 4 status conditions - (Service, Deployment, Progessing, and Available) are all true. The idea is that in the future, Ready would be the one Condition users could use to see if an EventListener is ready to serve traffic or not. Fixes #932 Signed-off-by: Dibyo Mukherjee <dibyo@google.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
kind/api-change
Categorizes issue or PR as related to adding, removing, or otherwise changing an API
kind/feature
Categorizes issue or PR as related to a new feature.
Background
Currently, the EventListener's
status
field contains following 4 condition types -Available
,Deployment
,Progressing
,Service
.The
Deployment
andService
condition indicate whether the deployment or service exists.Available
andProgressing
are conditions propagated from the underlying Deployment's conditions.Problems
4 conditions are a lot and can be confusing -- which conditions is most relevant? Usually the Available one since that will tell you if the deployment is actually available or not. But in that case, why do we need the other 3?
Two of them are of type "Deployment", and "Service". DeploymentExists and ServiceExists are probably more explicit.
With TEP-008, we will support Knative services as an alternate deployment model for the EventListeners. With that the existing 4 conditions do not make as much sense.
Proposal
Add a new condition of type
Ready
. It will be true if both service/deployment are up and available. We can use thereason
andmessage
fields to add details on why the EL is not ready e.g. ServiceNotAvailable, DeploymentProgressing etc.Deprecate the 4 older conditions and remove them when we move to the v1beta1 APIversion.
References:
Knative Serving API spec says:
Each resource MUST have either a Ready condition (for ongoing systems)
K8s API conventions on conditions
The text was updated successfully, but these errors were encountered: