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

api: Allow MaxSurge to be set on DaemonSets during update #96375

Merged
merged 4 commits into from Dec 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion api/openapi-spec/swagger.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 4 additions & 2 deletions pkg/apis/apps/fuzzer/fuzzer.go
Expand Up @@ -118,11 +118,13 @@ var Funcs = func(codecs runtimeserializer.CodecFactory) []interface{} {
rollingUpdate := apps.RollingUpdateDaemonSet{}
if c.RandBool() {
if c.RandBool() {
rollingUpdate.MaxUnavailable = intstr.FromInt(1 + int(c.Rand.Int31()))
rollingUpdate.MaxUnavailable = intstr.FromInt(int(c.Rand.Int31()))
rollingUpdate.MaxSurge = intstr.FromInt(int(c.Rand.Int31()))
} else {
rollingUpdate.MaxUnavailable = intstr.FromString(fmt.Sprintf("%d%%", 1+c.Rand.Int31()))
rollingUpdate.MaxSurge = intstr.FromString(fmt.Sprintf("%d%%", c.Rand.Int31()))
}
}

j.RollingUpdate = &rollingUpdate
}
},
Expand Down
38 changes: 30 additions & 8 deletions pkg/apis/apps/types.go
Expand Up @@ -532,19 +532,41 @@ type RollingUpdateDaemonSet struct {
// The maximum number of DaemonSet pods that can be unavailable during the
// update. Value can be an absolute number (ex: 5) or a percentage of total
// number of DaemonSet pods at the start of the update (ex: 10%). Absolute
// number is calculated from percentage by rounding up.
// This cannot be 0.
// number is calculated from percentage by rounding down to a minimum of one.
// This cannot be 0 if MaxSurge is 0
// Default value is 1.
// Example: when this is set to 30%, at most 30% of the total number of nodes
// that should be running the daemon pod (i.e. status.desiredNumberScheduled)
// can have their pods stopped for an update at any given
// time. The update starts by stopping at most 30% of those DaemonSet pods
// and then brings up new DaemonSet pods in their place. Once the new pods
// are available, it then proceeds onto other DaemonSet pods, thus ensuring
// that at least 70% of original number of DaemonSet pods are available at
// all times during the update.
// can have their pods stopped for an update at any given time. The update
// starts by stopping at most 30% of those DaemonSet pods and then brings
// up new DaemonSet pods in their place. Once the new pods are available,
// it then proceeds onto other DaemonSet pods, thus ensuring that at least
// 70% of original number of DaemonSet pods are available at all times during
// the update.
// +optional
MaxUnavailable intstr.IntOrString

// The maximum number of nodes with an existing available DaemonSet pod that
// can have an updated DaemonSet pod during during an update.
// Value can be an absolute number (ex: 5) or a percentage of desired pods (ex: 10%).
// This can not be 0 if MaxUnavailable is 0.
// Absolute number is calculated from percentage by rounding up to a minimum of 1.
// Default value is 0.
// Example: when this is set to 30%, at most 30% of the total number of nodes
// that should be running the daemon pod (i.e. status.desiredNumberScheduled)
// can have their a new pod created before the old pod is marked as deleted.
// The update starts by launching new pods on 30% of nodes. Once an updated
// pod is available (Ready for at least minReadySeconds) the old DaemonSet pod
// on that node is marked deleted. If the old pod becomes unavailable for any
// reason (Ready transitions to false, is evicted, or is drained) an updated
// pod is immediatedly created on that node without considering surge limits.
// Allowing surge implies the possibility that the resources consumed by the
// daemonset on any given node can double if the readiness check fails, and
// so resource intensive daemonsets should take into account that they may
// cause evictions during disruption.
// This is an alpha field and requires enabling DaemonSetUpdateSurge feature gate.
// +optional
MaxSurge intstr.IntOrString
}

// DaemonSetSpec is the specification of a daemon set.
Expand Down
7 changes: 4 additions & 3 deletions pkg/apis/apps/v1/conversion_test.go
Expand Up @@ -20,7 +20,7 @@ import (
"testing"

appsv1 "k8s.io/api/apps/v1"
"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
apiequality "k8s.io/apimachinery/pkg/api/equality"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/intstr"
Expand Down Expand Up @@ -226,13 +226,14 @@ func TestV1StatefulSetUpdateStrategyConversion(t *testing.T) {

func TestV1RollingUpdateDaemonSetConversion(t *testing.T) {
intorstr := intstr.FromInt(1)
maxSurge := intstr.FromInt(0)
testcases := map[string]struct {
rollingUpdateDs1 *apps.RollingUpdateDaemonSet
rollingUpdateDs2 *appsv1.RollingUpdateDaemonSet
}{
"RollingUpdateDaemonSet Conversion 2": {
rollingUpdateDs1: &apps.RollingUpdateDaemonSet{MaxUnavailable: intorstr},
rollingUpdateDs2: &appsv1.RollingUpdateDaemonSet{MaxUnavailable: &intorstr},
rollingUpdateDs1: &apps.RollingUpdateDaemonSet{MaxUnavailable: intorstr, MaxSurge: maxSurge},
rollingUpdateDs2: &appsv1.RollingUpdateDaemonSet{MaxUnavailable: &intorstr, MaxSurge: &maxSurge},
},
}

Expand Down
5 changes: 5 additions & 0 deletions pkg/apis/apps/v1/defaults.go
Expand Up @@ -84,6 +84,11 @@ func SetDefaults_DaemonSet(obj *appsv1.DaemonSet) {
maxUnavailable := intstr.FromInt(1)
updateStrategy.RollingUpdate.MaxUnavailable = &maxUnavailable
}
if updateStrategy.RollingUpdate.MaxSurge == nil {
// Set default MaxSurge as 0 by default.
maxSurge := intstr.FromInt(0)
updateStrategy.RollingUpdate.MaxSurge = &maxSurge
}
}
if obj.Spec.RevisionHistoryLimit == nil {
obj.Spec.RevisionHistoryLimit = new(int32)
Expand Down
6 changes: 5 additions & 1 deletion pkg/apis/apps/v1/defaults_test.go
Expand Up @@ -21,7 +21,7 @@ import (
"testing"

appsv1 "k8s.io/api/apps/v1"
"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
apiequality "k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -38,6 +38,7 @@ import (
func TestSetDefaultDaemonSetSpec(t *testing.T) {
defaultLabels := map[string]string{"foo": "bar"}
maxUnavailable := intstr.FromInt(1)
maxSurge := intstr.FromInt(0)
period := int64(v1.DefaultTerminationGracePeriodSeconds)
defaultTemplate := v1.PodTemplateSpec{
Spec: v1.PodSpec{
Expand Down Expand Up @@ -80,6 +81,7 @@ func TestSetDefaultDaemonSetSpec(t *testing.T) {
Type: appsv1.RollingUpdateDaemonSetStrategyType,
RollingUpdate: &appsv1.RollingUpdateDaemonSet{
MaxUnavailable: &maxUnavailable,
MaxSurge: &maxSurge,
},
},
RevisionHistoryLimit: utilpointer.Int32Ptr(10),
Expand Down Expand Up @@ -110,6 +112,7 @@ func TestSetDefaultDaemonSetSpec(t *testing.T) {
Type: appsv1.RollingUpdateDaemonSetStrategyType,
RollingUpdate: &appsv1.RollingUpdateDaemonSet{
MaxUnavailable: &maxUnavailable,
MaxSurge: &maxSurge,
},
},
RevisionHistoryLimit: utilpointer.Int32Ptr(1),
Expand Down Expand Up @@ -146,6 +149,7 @@ func TestSetDefaultDaemonSetSpec(t *testing.T) {
Type: appsv1.RollingUpdateDaemonSetStrategyType,
RollingUpdate: &appsv1.RollingUpdateDaemonSet{
MaxUnavailable: &maxUnavailable,
MaxSurge: &maxSurge,
},
},
RevisionHistoryLimit: utilpointer.Int32Ptr(10),
Expand Down
6 changes: 6 additions & 0 deletions pkg/apis/apps/v1/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 4 additions & 3 deletions pkg/apis/apps/v1beta2/conversion_test.go
Expand Up @@ -20,7 +20,7 @@ import (
"testing"

"k8s.io/api/apps/v1beta2"
"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/kubernetes/pkg/api/legacyscheme"
"k8s.io/kubernetes/pkg/apis/apps"
Expand Down Expand Up @@ -153,13 +153,14 @@ func TestV1beta2StatefulSetUpdateStrategyConversion(t *testing.T) {

func TestV1beta2RollingUpdateDaemonSetConversion(t *testing.T) {
intorstr := intstr.FromInt(1)
maxSurge := intstr.FromInt(0)
testcases := map[string]struct {
rollingUpdateDs1 *apps.RollingUpdateDaemonSet
rollingUpdateDs2 *v1beta2.RollingUpdateDaemonSet
}{
"RollingUpdateDaemonSet Conversion 2": {
rollingUpdateDs1: &apps.RollingUpdateDaemonSet{MaxUnavailable: intorstr},
rollingUpdateDs2: &v1beta2.RollingUpdateDaemonSet{MaxUnavailable: &intorstr},
rollingUpdateDs1: &apps.RollingUpdateDaemonSet{MaxUnavailable: intorstr, MaxSurge: maxSurge},
rollingUpdateDs2: &v1beta2.RollingUpdateDaemonSet{MaxUnavailable: &intorstr, MaxSurge: &maxSurge},
},
}

Expand Down
5 changes: 5 additions & 0 deletions pkg/apis/apps/v1beta2/defaults.go
Expand Up @@ -41,6 +41,11 @@ func SetDefaults_DaemonSet(obj *appsv1beta2.DaemonSet) {
maxUnavailable := intstr.FromInt(1)
updateStrategy.RollingUpdate.MaxUnavailable = &maxUnavailable
}
if updateStrategy.RollingUpdate.MaxSurge == nil {
// Set default MaxSurge as 0 by default.
maxSurge := intstr.FromInt(0)
updateStrategy.RollingUpdate.MaxSurge = &maxSurge
}
}
if obj.Spec.RevisionHistoryLimit == nil {
obj.Spec.RevisionHistoryLimit = new(int32)
Expand Down
6 changes: 5 additions & 1 deletion pkg/apis/apps/v1beta2/defaults_test.go
Expand Up @@ -21,7 +21,7 @@ import (
"testing"

appsv1beta2 "k8s.io/api/apps/v1beta2"
"k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
apiequality "k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -38,6 +38,7 @@ import (
func TestSetDefaultDaemonSetSpec(t *testing.T) {
defaultLabels := map[string]string{"foo": "bar"}
maxUnavailable := intstr.FromInt(1)
maxSurge := intstr.FromInt(0)
period := int64(v1.DefaultTerminationGracePeriodSeconds)
defaultTemplate := v1.PodTemplateSpec{
Spec: v1.PodSpec{
Expand Down Expand Up @@ -80,6 +81,7 @@ func TestSetDefaultDaemonSetSpec(t *testing.T) {
Type: appsv1beta2.RollingUpdateDaemonSetStrategyType,
RollingUpdate: &appsv1beta2.RollingUpdateDaemonSet{
MaxUnavailable: &maxUnavailable,
MaxSurge: &maxSurge,
},
},
RevisionHistoryLimit: utilpointer.Int32Ptr(10),
Expand Down Expand Up @@ -110,6 +112,7 @@ func TestSetDefaultDaemonSetSpec(t *testing.T) {
Type: appsv1beta2.RollingUpdateDaemonSetStrategyType,
RollingUpdate: &appsv1beta2.RollingUpdateDaemonSet{
MaxUnavailable: &maxUnavailable,
MaxSurge: &maxSurge,
},
},
RevisionHistoryLimit: utilpointer.Int32Ptr(1),
Expand Down Expand Up @@ -146,6 +149,7 @@ func TestSetDefaultDaemonSetSpec(t *testing.T) {
Type: appsv1beta2.RollingUpdateDaemonSetStrategyType,
RollingUpdate: &appsv1beta2.RollingUpdateDaemonSet{
MaxUnavailable: &maxUnavailable,
MaxSurge: &maxSurge,
},
},
RevisionHistoryLimit: utilpointer.Int32Ptr(10),
Expand Down
6 changes: 6 additions & 0 deletions pkg/apis/apps/v1beta2/zz_generated.conversion.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions pkg/apis/apps/validation/BUILD
Expand Up @@ -14,6 +14,7 @@ go_library(
"//pkg/apis/apps:go_default_library",
"//pkg/apis/core:go_default_library",
"//pkg/apis/core/validation:go_default_library",
"//pkg/features:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/api/equality:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/api/validation:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
Expand All @@ -22,6 +23,7 @@ go_library(
"//staging/src/k8s.io/apimachinery/pkg/util/intstr:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/validation:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
],
)

Expand Down
37 changes: 30 additions & 7 deletions pkg/apis/apps/validation/validation.go
Expand Up @@ -28,9 +28,11 @@ import (
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/validation/field"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/kubernetes/pkg/apis/apps"
api "k8s.io/kubernetes/pkg/apis/core"
apivalidation "k8s.io/kubernetes/pkg/apis/core/validation"
"k8s.io/kubernetes/pkg/features"
)

// ValidateStatefulSetName can be used to check whether the given StatefulSet name is valid.
Expand Down Expand Up @@ -343,14 +345,35 @@ func ValidateDaemonSetSpec(spec *apps.DaemonSetSpec, fldPath *field.Path, opts a

// ValidateRollingUpdateDaemonSet validates a given RollingUpdateDaemonSet.
func ValidateRollingUpdateDaemonSet(rollingUpdate *apps.RollingUpdateDaemonSet, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
allErrs = append(allErrs, ValidatePositiveIntOrPercent(rollingUpdate.MaxUnavailable, fldPath.Child("maxUnavailable"))...)
if getIntOrPercentValue(rollingUpdate.MaxUnavailable) == 0 {
// MaxUnavailable cannot be 0.
allErrs = append(allErrs, field.Invalid(fldPath.Child("maxUnavailable"), rollingUpdate.MaxUnavailable, "cannot be 0"))
var allErrs field.ErrorList
if utilfeature.DefaultFeatureGate.Enabled(features.DaemonSetUpdateSurge) {
// Validate both fields are positive ints or have a percentage value
allErrs = append(allErrs, ValidatePositiveIntOrPercent(rollingUpdate.MaxUnavailable, fldPath.Child("maxUnavailable"))...)
allErrs = append(allErrs, ValidatePositiveIntOrPercent(rollingUpdate.MaxSurge, fldPath.Child("maxSurge"))...)

// Validate that MaxUnavailable and MaxSurge are not more than 100%.
allErrs = append(allErrs, IsNotMoreThan100Percent(rollingUpdate.MaxUnavailable, fldPath.Child("maxUnavailable"))...)
allErrs = append(allErrs, IsNotMoreThan100Percent(rollingUpdate.MaxSurge, fldPath.Child("maxSurge"))...)

// Validate exactly one of MaxSurge or MaxUnavailable is non-zero
hasUnavailable := getIntOrPercentValue(rollingUpdate.MaxUnavailable) != 0
hasSurge := getIntOrPercentValue(rollingUpdate.MaxSurge) != 0
switch {
case hasUnavailable && hasSurge:
allErrs = append(allErrs, field.Invalid(fldPath.Child("maxSurge"), rollingUpdate.MaxSurge, "may not be set when maxUnavailable is non-zero"))
case !hasUnavailable && !hasSurge:
allErrs = append(allErrs, field.Required(fldPath.Child("maxUnavailable"), "cannot be 0 when maxSurge is 0"))
}

} else {
allErrs = append(allErrs, ValidatePositiveIntOrPercent(rollingUpdate.MaxUnavailable, fldPath.Child("maxUnavailable"))...)
if getIntOrPercentValue(rollingUpdate.MaxUnavailable) == 0 {
// MaxUnavailable cannot be 0.
allErrs = append(allErrs, field.Invalid(fldPath.Child("maxUnavailable"), rollingUpdate.MaxUnavailable, "cannot be 0"))
}
// Validate that MaxUnavailable is not more than 100%.
allErrs = append(allErrs, IsNotMoreThan100Percent(rollingUpdate.MaxUnavailable, fldPath.Child("maxUnavailable"))...)
}
// Validate that MaxUnavailable is not more than 100%.
allErrs = append(allErrs, IsNotMoreThan100Percent(rollingUpdate.MaxUnavailable, fldPath.Child("maxUnavailable"))...)
return allErrs
}

Expand Down