Commit 629fea3e authored by Kubernetes Submit Queue's avatar Kubernetes Submit Queue Committed by GitHub

Merge pull request #52127 from kargakis/do-not-report-timeout

Automatic merge from submit-queue Fix deployment timeout reporting If the previous condition has been a successful rollout then we shouldn't try to estimate any progress. Scenario: * progressDeadlineSeconds is smaller than the difference between now and the time the last rollout finished in the past. * the creation of a new ReplicaSet triggers a resync of the Deployment prior to the cached copy of the Deployment getting updated with the status.condition that indicates the creation of the new ReplicaSet. The Deployment will be resynced and eventually its Progressing condition will catch up with the state of the world. Fixes https://github.com/kubernetes/kubernetes/issues/49637 I will also cherry-pick this back to 1.7. **Release note**: ```NONE ```
parents 36b3a0d7 af0f5dcc
...@@ -861,6 +861,20 @@ func DeploymentTimedOut(deployment *extensions.Deployment, newStatus *extensions ...@@ -861,6 +861,20 @@ func DeploymentTimedOut(deployment *extensions.Deployment, newStatus *extensions
if condition == nil { if condition == nil {
return false return false
} }
// If the previous condition has been a successful rollout then we shouldn't try to
// estimate any progress. Scenario:
//
// * progressDeadlineSeconds is smaller than the difference between now and the time
// the last rollout finished in the past.
// * the creation of a new ReplicaSet triggers a resync of the Deployment prior to the
// cached copy of the Deployment getting updated with the status.condition that indicates
// the creation of the new ReplicaSet.
//
// The Deployment will be resynced and eventually its Progressing condition will catch
// up with the state of the world.
if condition.Reason == NewRSAvailableReason {
return false
}
if condition.Reason == TimedOutReason { if condition.Reason == TimedOutReason {
return true return true
} }
......
...@@ -1127,7 +1127,7 @@ func TestDeploymentTimedOut(t *testing.T) { ...@@ -1127,7 +1127,7 @@ func TestDeploymentTimedOut(t *testing.T) {
timeFn := func(min, sec int) time.Time { timeFn := func(min, sec int) time.Time {
return time.Date(2016, 1, 1, 0, min, sec, 0, time.UTC) return time.Date(2016, 1, 1, 0, min, sec, 0, time.UTC)
} }
deployment := func(condType extensions.DeploymentConditionType, status v1.ConditionStatus, pds *int32, from time.Time) extensions.Deployment { deployment := func(condType extensions.DeploymentConditionType, status v1.ConditionStatus, reason string, pds *int32, from time.Time) extensions.Deployment {
return extensions.Deployment{ return extensions.Deployment{
Spec: extensions.DeploymentSpec{ Spec: extensions.DeploymentSpec{
ProgressDeadlineSeconds: pds, ProgressDeadlineSeconds: pds,
...@@ -1137,6 +1137,7 @@ func TestDeploymentTimedOut(t *testing.T) { ...@@ -1137,6 +1137,7 @@ func TestDeploymentTimedOut(t *testing.T) {
{ {
Type: condType, Type: condType,
Status: status, Status: status,
Reason: reason,
LastUpdateTime: metav1.Time{Time: from}, LastUpdateTime: metav1.Time{Time: from},
}, },
}, },
...@@ -1155,24 +1156,30 @@ func TestDeploymentTimedOut(t *testing.T) { ...@@ -1155,24 +1156,30 @@ func TestDeploymentTimedOut(t *testing.T) {
{ {
name: "no progressDeadlineSeconds specified - no timeout", name: "no progressDeadlineSeconds specified - no timeout",
d: deployment(extensions.DeploymentProgressing, v1.ConditionTrue, null, timeFn(1, 9)), d: deployment(extensions.DeploymentProgressing, v1.ConditionTrue, "", null, timeFn(1, 9)),
nowFn: func() time.Time { return timeFn(1, 20) }, nowFn: func() time.Time { return timeFn(1, 20) },
expected: false, expected: false,
}, },
{ {
name: "progressDeadlineSeconds: 10s, now - started => 00:01:20 - 00:01:09 => 11s", name: "progressDeadlineSeconds: 10s, now - started => 00:01:20 - 00:01:09 => 11s",
d: deployment(extensions.DeploymentProgressing, v1.ConditionTrue, &ten, timeFn(1, 9)), d: deployment(extensions.DeploymentProgressing, v1.ConditionTrue, "", &ten, timeFn(1, 9)),
nowFn: func() time.Time { return timeFn(1, 20) }, nowFn: func() time.Time { return timeFn(1, 20) },
expected: true, expected: true,
}, },
{ {
name: "progressDeadlineSeconds: 10s, now - started => 00:01:20 - 00:01:11 => 9s", name: "progressDeadlineSeconds: 10s, now - started => 00:01:20 - 00:01:11 => 9s",
d: deployment(extensions.DeploymentProgressing, v1.ConditionTrue, &ten, timeFn(1, 11)), d: deployment(extensions.DeploymentProgressing, v1.ConditionTrue, "", &ten, timeFn(1, 11)),
nowFn: func() time.Time { return timeFn(1, 20) }, nowFn: func() time.Time { return timeFn(1, 20) },
expected: false, expected: false,
}, },
{
name: "previous status was a complete deployment",
d: deployment(extensions.DeploymentProgressing, v1.ConditionTrue, NewRSAvailableReason, nil, time.Time{}),
expected: false,
},
} }
for _, test := range tests { for _, test := range tests {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment