Default extensions/v1beta1 Deployment's ProgressDeadlineSeconds to MaxInt32.

1. MaxInt32 has the same meaning as unset, for compatibility
2. Deployment controller treats MaxInt32 the same as unset (nil)
This commit is contained in:
Janet Kuo 2018-07-24 13:20:39 -07:00
parent 845a55dbbd
commit a4f85c8dd0
7 changed files with 52 additions and 17 deletions

View File

@ -17,6 +17,8 @@ limitations under the License.
package v1beta1
import (
"math"
"k8s.io/api/core/v1"
extensionsv1beta1 "k8s.io/api/extensions/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@ -110,6 +112,12 @@ func SetDefaults_Deployment(obj *extensionsv1beta1.Deployment) {
strategy.RollingUpdate.MaxSurge = &maxSurge
}
}
// Set extensionsv1beta1.DeploymentSpec.ProgressDeadlineSeconds to MaxInt,
// which has the same meaning as unset.
if obj.Spec.ProgressDeadlineSeconds == nil {
obj.Spec.ProgressDeadlineSeconds = new(int32)
*obj.Spec.ProgressDeadlineSeconds = math.MaxInt32
}
}
func SetDefaults_ReplicaSet(obj *extensionsv1beta1.ReplicaSet) {

View File

@ -17,6 +17,7 @@ limitations under the License.
package v1beta1_test
import (
"math"
"reflect"
"testing"
@ -188,7 +189,8 @@ func TestSetDefaultDeployment(t *testing.T) {
MaxUnavailable: &defaultIntOrString,
},
},
Template: defaultTemplate,
Template: defaultTemplate,
ProgressDeadlineSeconds: utilpointer.Int32Ptr(math.MaxInt32),
},
},
},
@ -213,7 +215,8 @@ func TestSetDefaultDeployment(t *testing.T) {
MaxUnavailable: &defaultIntOrString,
},
},
Template: defaultTemplate,
Template: defaultTemplate,
ProgressDeadlineSeconds: utilpointer.Int32Ptr(math.MaxInt32),
},
},
},
@ -237,7 +240,8 @@ func TestSetDefaultDeployment(t *testing.T) {
MaxUnavailable: &defaultIntOrString,
},
},
Template: defaultTemplate,
Template: defaultTemplate,
ProgressDeadlineSeconds: utilpointer.Int32Ptr(math.MaxInt32),
},
},
},
@ -256,7 +260,8 @@ func TestSetDefaultDeployment(t *testing.T) {
Strategy: extensionsv1beta1.DeploymentStrategy{
Type: extensionsv1beta1.RecreateDeploymentStrategyType,
},
Template: defaultTemplate,
Template: defaultTemplate,
ProgressDeadlineSeconds: utilpointer.Int32Ptr(math.MaxInt32),
},
},
},

View File

@ -36,7 +36,7 @@ func (dc *DeploymentController) syncRolloutStatus(allRSs []*apps.ReplicaSet, new
newStatus := calculateStatus(allRSs, newRS, d)
// If there is no progressDeadlineSeconds set, remove any Progressing condition.
if d.Spec.ProgressDeadlineSeconds == nil {
if !util.HasProgressDeadline(d) {
util.RemoveDeploymentCondition(&newStatus, apps.DeploymentProgressing)
}
@ -47,7 +47,7 @@ func (dc *DeploymentController) syncRolloutStatus(allRSs []*apps.ReplicaSet, new
isCompleteDeployment := newStatus.Replicas == newStatus.UpdatedReplicas && currentCond != nil && currentCond.Reason == util.NewRSAvailableReason
// Check for progress only if there is a progress deadline set and the latest rollout
// hasn't completed yet.
if d.Spec.ProgressDeadlineSeconds != nil && !isCompleteDeployment {
if util.HasProgressDeadline(d) && !isCompleteDeployment {
switch {
case util.DeploymentComplete(d, &newStatus):
// Update the deployment conditions with a message for the new replica set that
@ -159,7 +159,7 @@ var nowFn = func() time.Time { return time.Now() }
func (dc *DeploymentController) requeueStuckDeployment(d *apps.Deployment, newStatus apps.DeploymentStatus) time.Duration {
currentCond := util.GetDeploymentCondition(d.Status, apps.DeploymentProgressing)
// Can't estimate progress if there is no deadline in the spec or progressing condition in the current status.
if d.Spec.ProgressDeadlineSeconds == nil || currentCond == nil {
if !util.HasProgressDeadline(d) || currentCond == nil {
return time.Duration(-1)
}
// No need to estimate progress if the rollout is complete or already timed out.

View File

@ -17,6 +17,7 @@ limitations under the License.
package deployment
import (
"math"
"testing"
"time"
@ -67,6 +68,7 @@ func newRSWithAvailable(name string, specReplicas, statusReplicas, availableRepl
func TestRequeueStuckDeployment(t *testing.T) {
pds := int32(60)
infinite := int32(math.MaxInt32)
failed := []apps.DeploymentCondition{
{
Type: apps.DeploymentProgressing,
@ -90,11 +92,17 @@ func TestRequeueStuckDeployment(t *testing.T) {
expected time.Duration
}{
{
name: "no progressDeadlineSeconds specified",
name: "nil progressDeadlineSeconds specified",
d: currentDeployment(nil, 4, 3, 3, 2, nil),
status: newDeploymentStatus(3, 3, 2),
expected: time.Duration(-1),
},
{
name: "infinite progressDeadlineSeconds specified",
d: currentDeployment(&infinite, 4, 3, 3, 2, nil),
status: newDeploymentStatus(3, 3, 2),
expected: time.Duration(-1),
},
{
name: "no progressing condition found",
d: currentDeployment(&pds, 4, 3, 3, 2, nil),
@ -330,7 +338,7 @@ func TestSyncRolloutStatus(t *testing.T) {
newCond := util.GetDeploymentCondition(test.d.Status, test.conditionType)
switch {
case newCond == nil:
if test.d.Spec.ProgressDeadlineSeconds != nil {
if test.d.Spec.ProgressDeadlineSeconds != nil && *test.d.Spec.ProgressDeadlineSeconds != math.MaxInt32 {
t.Errorf("%s: expected deployment condition: %s", test.name, test.conditionType)
}
case newCond.Status != test.conditionStatus || newCond.Reason != test.conditionReason:

View File

@ -71,7 +71,7 @@ func (dc *DeploymentController) sync(d *apps.Deployment, rsList []*apps.ReplicaS
// These conditions are needed so that we won't accidentally report lack of progress for resumed deployments
// that were paused for longer than progressDeadlineSeconds.
func (dc *DeploymentController) checkPausedConditions(d *apps.Deployment) error {
if d.Spec.ProgressDeadlineSeconds == nil {
if !deploymentutil.HasProgressDeadline(d) {
return nil
}
cond := deploymentutil.GetDeploymentCondition(d.Status, apps.DeploymentProgressing)
@ -158,7 +158,7 @@ func (dc *DeploymentController) getNewReplicaSet(d *apps.Deployment, rsList, old
// of this deployment then it is likely that old users started caring about progress. In that
// case we need to take into account the first time we noticed their new replica set.
cond := deploymentutil.GetDeploymentCondition(d.Status, apps.DeploymentProgressing)
if d.Spec.ProgressDeadlineSeconds != nil && cond == nil {
if deploymentutil.HasProgressDeadline(d) && cond == nil {
msg := fmt.Sprintf("Found new replica set %q", rsCopy.Name)
condition := deploymentutil.NewDeploymentCondition(apps.DeploymentProgressing, v1.ConditionTrue, deploymentutil.FoundNewRSReason, msg)
deploymentutil.SetDeploymentCondition(&d.Status, *condition)
@ -253,7 +253,7 @@ func (dc *DeploymentController) getNewReplicaSet(d *apps.Deployment, rsList, old
return nil, err
case err != nil:
msg := fmt.Sprintf("Failed to create new replica set %q: %v", newRS.Name, err)
if d.Spec.ProgressDeadlineSeconds != nil {
if deploymentutil.HasProgressDeadline(d) {
cond := deploymentutil.NewDeploymentCondition(apps.DeploymentProgressing, v1.ConditionFalse, deploymentutil.FailedRSCreateReason, msg)
deploymentutil.SetDeploymentCondition(&d.Status, *cond)
// We don't really care about this error at this point, since we have a bigger issue to report.
@ -269,7 +269,7 @@ func (dc *DeploymentController) getNewReplicaSet(d *apps.Deployment, rsList, old
}
needsUpdate := deploymentutil.SetDeploymentRevision(d, newRevision)
if !alreadyExists && d.Spec.ProgressDeadlineSeconds != nil {
if !alreadyExists && deploymentutil.HasProgressDeadline(d) {
msg := fmt.Sprintf("Created new replica set %q", createdRS.Name)
condition := deploymentutil.NewDeploymentCondition(apps.DeploymentProgressing, v1.ConditionTrue, deploymentutil.NewReplicaSetReason, msg)
deploymentutil.SetDeploymentCondition(&d.Status, *condition)

View File

@ -18,6 +18,7 @@ package util
import (
"fmt"
"math"
"sort"
"strconv"
"strings"
@ -773,7 +774,7 @@ var nowFn = func() time.Time { return time.Now() }
// is older than progressDeadlineSeconds or a Progressing condition with a TimedOutReason reason already
// exists.
func DeploymentTimedOut(deployment *apps.Deployment, newStatus *apps.DeploymentStatus) bool {
if deployment.Spec.ProgressDeadlineSeconds == nil {
if !HasProgressDeadline(deployment) {
return false
}
@ -918,3 +919,7 @@ func ResolveFenceposts(maxSurge, maxUnavailable *intstrutil.IntOrString, desired
return int32(surge), int32(unavailable), nil
}
func HasProgressDeadline(d *apps.Deployment) bool {
return d.Spec.ProgressDeadlineSeconds != nil && *d.Spec.ProgressDeadlineSeconds != math.MaxInt32
}

View File

@ -18,6 +18,7 @@ package util
import (
"fmt"
"math"
"math/rand"
"reflect"
"sort"
@ -1068,8 +1069,9 @@ func TestDeploymentProgressing(t *testing.T) {
func TestDeploymentTimedOut(t *testing.T) {
var (
null *int32
ten = int32(10)
null *int32
ten = int32(10)
infinite = int32(math.MaxInt32)
)
timeFn := func(min, sec int) time.Time {
@ -1102,12 +1104,19 @@ func TestDeploymentTimedOut(t *testing.T) {
expected bool
}{
{
name: "no progressDeadlineSeconds specified - no timeout",
name: "nil progressDeadlineSeconds specified - no timeout",
d: deployment(apps.DeploymentProgressing, v1.ConditionTrue, "", null, timeFn(1, 9)),
nowFn: func() time.Time { return timeFn(1, 20) },
expected: false,
},
{
name: "infinite progressDeadlineSeconds specified - no timeout",
d: deployment(apps.DeploymentProgressing, v1.ConditionTrue, "", &infinite, timeFn(1, 9)),
nowFn: func() time.Time { return timeFn(1, 20) },
expected: false,
},
{
name: "progressDeadlineSeconds: 10s, now - started => 00:01:20 - 00:01:09 => 11s",