Skip to content
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

Add lastTransitionTime to revisionCondition #655

Closed
akyyy opened this issue Apr 13, 2018 · 0 comments
Closed

Add lastTransitionTime to revisionCondition #655

akyyy opened this issue Apr 13, 2018 · 0 comments
Assignees
Labels
area/API API objects and controllers kind/feature Well-understood/specified features, ready for coding.

Comments

@akyyy
Copy link
Contributor

akyyy commented Apr 13, 2018

Expected Behavior

With the addition of activator, the revision k8s resources could be created multiple times. We need to know the starting time of creating k8s resources, instead of using revision creation time.

Actual Behavior

https://github.com/elafros/elafros/blob/02b084e3f56745c917b545dad202e83bb32d4834/pkg/controller/revision/revision.go#L567
Currently we're calculating time diff between now and the revision creating time to decide if it's timeout.

Additional Info

@grantr suggested we could use LastTransitionTime
https://github.com/kubernetes/kubernetes/blob/cb5f1ad9f7b45843567fd22befab31a80065ba4c/pkg/apis/extensions/types.go#L280-L281

@akyyy akyyy self-assigned this Apr 13, 2018
@mattmoor mattmoor added kind/feature Well-understood/specified features, ready for coding. area/API API objects and controllers labels Apr 13, 2018
@mattmoor mattmoor added this to the M4 milestone Apr 13, 2018
@akyyy akyyy closed this as completed Apr 14, 2018
ReToCode pushed a commit to ReToCode/serving that referenced this issue Mar 13, 2024
…native#14864) (knative#655)

* Surface Replica failures over Progressing failures

When transforming the deployment status to the revision
we want to bubble up the more severe condition to Ready.

Since Replica failures will include a more actionable error
message this condition is preferred

* Stop always marking the revision healthy when the PA is Ready

This isn't accurate when the Revision has failed to rollout
an update to it's deployment

* Various updates to the revision reconciler

1. PA Reachability now depends on the status of the Deployment

If we have available replicas we don't mark the revision as
unreachable. This allows ongoing requests to be handled

2. Always propagate the K8s Deployment Status to the Revision.

We don't need to gate this depending on whether the Revision
required activation. Since the only two conditions we propagate
from the Deployment is Progressing and ReplicaSetFailure=False

3. Mark Revision as Deploying if the PA's service name isn't set

* test deployment failures don't drop traffic on upgrade

* fix boilerplate check

---------

Co-authored-by: Knative Prow Robot <[email protected]>
Co-authored-by: dprotaso <[email protected]>
Co-authored-by: John Doe <johndoe@localhost>
ReToCode pushed a commit to ReToCode/serving that referenced this issue Mar 27, 2024
…native#14864) (knative#655)

* Surface Replica failures over Progressing failures

When transforming the deployment status to the revision
we want to bubble up the more severe condition to Ready.

Since Replica failures will include a more actionable error
message this condition is preferred

* Stop always marking the revision healthy when the PA is Ready

This isn't accurate when the Revision has failed to rollout
an update to it's deployment

* Various updates to the revision reconciler

1. PA Reachability now depends on the status of the Deployment

If we have available replicas we don't mark the revision as
unreachable. This allows ongoing requests to be handled

2. Always propagate the K8s Deployment Status to the Revision.

We don't need to gate this depending on whether the Revision
required activation. Since the only two conditions we propagate
from the Deployment is Progressing and ReplicaSetFailure=False

3. Mark Revision as Deploying if the PA's service name isn't set

* test deployment failures don't drop traffic on upgrade

* fix boilerplate check

---------

Co-authored-by: Knative Prow Robot <[email protected]>
Co-authored-by: dprotaso <[email protected]>
Co-authored-by: John Doe <johndoe@localhost>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/API API objects and controllers kind/feature Well-understood/specified features, ready for coding.
Projects
None yet
Development

No branches or pull requests

2 participants