mirror of
https://github.com/k3s-io/kubernetes.git
synced 2025-07-23 11:50:44 +00:00
Merge pull request #66581 from janetkuo/deploy-progress
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. Default extensions/v1beta1 Deployment's ProgressDeadlineSeconds to MaxInt32 **What this PR does / why we need it**: Default values should be set in all API versions, because defaulting happens whenever a serialized version is read. When we switched to `apps/v1` as the storage version in `1.10` (#58854), `extensions/v1beta1` `DeploymentSpec.ProgressDeadlineSeconds` gets `apps/v1` default value (`600`) instead of being unset. **Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*: Fixes #66135 **Special notes for your reviewer**: We need to cherrypick this fix to 1.10 and 1.11. Note that this fix will only help people who haven't upgraded to 1.10 or 1.11 when the storage version is changed. @kubernetes/sig-apps-bugs **Release note**: ```release-note NONE ```
This commit is contained in:
commit
259e0743f1
2
api/openapi-spec/swagger.json
generated
2
api/openapi-spec/swagger.json
generated
@ -83779,7 +83779,7 @@
|
||||
"type": "boolean"
|
||||
},
|
||||
"progressDeadlineSeconds": {
|
||||
"description": "The maximum time in seconds for a deployment to make progress before it is considered to be failed. The deployment controller will continue to process failed deployments and a condition with a ProgressDeadlineExceeded reason will be surfaced in the deployment status. Note that progress will not be estimated during the time a deployment is paused. This is not set by default.",
|
||||
"description": "The maximum time in seconds for a deployment to make progress before it is considered to be failed. The deployment controller will continue to process failed deployments and a condition with a ProgressDeadlineExceeded reason will be surfaced in the deployment status. Note that progress will not be estimated during the time a deployment is paused. This is set to the max value of int32 (i.e. 2147483647) by default, which means \"no deadline\".",
|
||||
"type": "integer",
|
||||
"format": "int32"
|
||||
},
|
||||
|
2
api/swagger-spec/extensions_v1beta1.json
generated
2
api/swagger-spec/extensions_v1beta1.json
generated
@ -9756,7 +9756,7 @@
|
||||
"progressDeadlineSeconds": {
|
||||
"type": "integer",
|
||||
"format": "int32",
|
||||
"description": "The maximum time in seconds for a deployment to make progress before it is considered to be failed. The deployment controller will continue to process failed deployments and a condition with a ProgressDeadlineExceeded reason will be surfaced in the deployment status. Note that progress will not be estimated during the time a deployment is paused. This is not set by default."
|
||||
"description": "The maximum time in seconds for a deployment to make progress before it is considered to be failed. The deployment controller will continue to process failed deployments and a condition with a ProgressDeadlineExceeded reason will be surfaced in the deployment status. Note that progress will not be estimated during the time a deployment is paused. This is set to the max value of int32 (i.e. 2147483647) by default, which means \"no deadline\"."
|
||||
}
|
||||
}
|
||||
},
|
||||
|
@ -4337,7 +4337,7 @@ When an object is created, the system will populate this list with the current s
|
||||
</tr>
|
||||
<tr>
|
||||
<td class="tableblock halign-left valign-top"><p class="tableblock">progressDeadlineSeconds</p></td>
|
||||
<td class="tableblock halign-left valign-top"><p class="tableblock">The maximum time in seconds for a deployment to make progress before it is considered to be failed. The deployment controller will continue to process failed deployments and a condition with a ProgressDeadlineExceeded reason will be surfaced in the deployment status. Note that progress will not be estimated during the time a deployment is paused. This is not set by default.</p></td>
|
||||
<td class="tableblock halign-left valign-top"><p class="tableblock">The maximum time in seconds for a deployment to make progress before it is considered to be failed. The deployment controller will continue to process failed deployments and a condition with a ProgressDeadlineExceeded reason will be surfaced in the deployment status. Note that progress will not be estimated during the time a deployment is paused. This is set to the max value of int32 (i.e. 2147483647) by default, which means "no deadline".</p></td>
|
||||
<td class="tableblock halign-left valign-top"><p class="tableblock">false</p></td>
|
||||
<td class="tableblock halign-left valign-top"><p class="tableblock">integer (int32)</p></td>
|
||||
<td class="tableblock halign-left valign-top"></td>
|
||||
|
@ -127,8 +127,8 @@ type DeploymentSpec struct {
|
||||
// is considered to be failed. The deployment controller will continue to
|
||||
// process failed deployments and a condition with a ProgressDeadlineExceeded
|
||||
// reason will be surfaced in the deployment status. Note that progress will
|
||||
// not be estimated during the time a deployment is paused. This is not set
|
||||
// by default.
|
||||
// not be estimated during the time a deployment is paused. This is set to
|
||||
// the max value of int32 (i.e. 2147483647) by default, which means "no deadline".
|
||||
// +optional
|
||||
ProgressDeadlineSeconds *int32
|
||||
}
|
||||
|
@ -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) {
|
||||
|
@ -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),
|
||||
},
|
||||
},
|
||||
},
|
||||
|
@ -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.
|
||||
|
@ -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:
|
||||
|
@ -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)
|
||||
|
@ -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
|
||||
}
|
||||
|
@ -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",
|
||||
|
||||
|
@ -353,8 +353,8 @@ message DeploymentSpec {
|
||||
// is considered to be failed. The deployment controller will continue to
|
||||
// process failed deployments and a condition with a ProgressDeadlineExceeded
|
||||
// reason will be surfaced in the deployment status. Note that progress will
|
||||
// not be estimated during the time a deployment is paused. This is not set
|
||||
// by default.
|
||||
// not be estimated during the time a deployment is paused. This is set to
|
||||
// the max value of int32 (i.e. 2147483647) by default, which means "no deadline".
|
||||
// +optional
|
||||
optional int32 progressDeadlineSeconds = 9;
|
||||
}
|
||||
|
@ -168,8 +168,8 @@ type DeploymentSpec struct {
|
||||
// is considered to be failed. The deployment controller will continue to
|
||||
// process failed deployments and a condition with a ProgressDeadlineExceeded
|
||||
// reason will be surfaced in the deployment status. Note that progress will
|
||||
// not be estimated during the time a deployment is paused. This is not set
|
||||
// by default.
|
||||
// not be estimated during the time a deployment is paused. This is set to
|
||||
// the max value of int32 (i.e. 2147483647) by default, which means "no deadline".
|
||||
// +optional
|
||||
ProgressDeadlineSeconds *int32 `json:"progressDeadlineSeconds,omitempty" protobuf:"varint,9,opt,name=progressDeadlineSeconds"`
|
||||
}
|
||||
|
@ -196,7 +196,7 @@ var map_DeploymentSpec = map[string]string{
|
||||
"revisionHistoryLimit": "The number of old ReplicaSets to retain to allow rollback. This is a pointer to distinguish between explicit zero and not specified.",
|
||||
"paused": "Indicates that the deployment is paused and will not be processed by the deployment controller.",
|
||||
"rollbackTo": "DEPRECATED. The config this deployment is rolling back to. Will be cleared after rollback is done.",
|
||||
"progressDeadlineSeconds": "The maximum time in seconds for a deployment to make progress before it is considered to be failed. The deployment controller will continue to process failed deployments and a condition with a ProgressDeadlineExceeded reason will be surfaced in the deployment status. Note that progress will not be estimated during the time a deployment is paused. This is not set by default.",
|
||||
"progressDeadlineSeconds": "The maximum time in seconds for a deployment to make progress before it is considered to be failed. The deployment controller will continue to process failed deployments and a condition with a ProgressDeadlineExceeded reason will be surfaced in the deployment status. Note that progress will not be estimated during the time a deployment is paused. This is set to the max value of int32 (i.e. 2147483647) by default, which means \"no deadline\".",
|
||||
}
|
||||
|
||||
func (DeploymentSpec) SwaggerDoc() map[string]string {
|
||||
|
Loading…
Reference in New Issue
Block a user