Skip to content

Commit

Permalink
Allow cluster daemonsets to use maxSurge
Browse files Browse the repository at this point in the history
Change the "[sig-arch] Managed cluster should only include cluster
daemonsets that have maxUnavailable update of 10 or 33 percent" test to
allow daemonsets to specify maxUnavailable 0 with maxSurge 10%.

The intention of this test is to verify that cluster components have
parameters configured for rolling updates to ensure a reasonable balance
between rapid rollout and minimum availability during upgrades.  At the
time this test was written, the only way to accomplish that goal was to set
an appropriate maxUnavailable value.  Since then, the maxSurge parameter
has been added to the available parameters for daemonsets (see
<https://github.com/kubernetes/enhancements/tree/master/keps/sig-apps/1591-daemonset-surge>).
Now, a daemonset can ensure rapid rollout and minimum availability using
maxSurge.  However, when maxSurge is used, maxUnavailable must be zero.
Thus this commit changes the test to allow maxUnavailable to be zero if
maxSurge is set to 10%.

* test/extended/operators/daemon_set.go: Don't mark a daemonset as invalid
if it has maxSurge set to 10%.  Include the maxSurge value in the violation
message when neither maxUnavailable nor maxSurge is set appropriately.
* cmd/openshift-tests/minimal.go:
* pkg/test/ginkgo/testNames.txt:
* test/extended/testdata/bindata.go:
* test/extended/util/annotate/generated/zz_generated.annotations.go: Update
test name.
  • Loading branch information
Miciah committed Apr 3, 2023
1 parent 4e43d18 commit 35bf447
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 8 deletions.
2 changes: 1 addition & 1 deletion cmd/openshift-tests/minimal.go
Original file line number Diff line number Diff line change
Expand Up @@ -2326,7 +2326,7 @@ var (
"[sig-api-machinery][Feature:ServerSideApply] Server-Side Apply should work for oauth.openshift.io/v1, Resource=oauthclients [Suite:openshift/conformance/parallel]": {},
"[sig-api-machinery][Feature:ServerSideApply] Server-Side Apply should work for apps.openshift.io/v1, Resource=deploymentconfigs [Suite:openshift/conformance/parallel]": {},
"[sig-api-machinery][Feature:ResourceQuota] Object count should properly count the number of imagestreams resources [Suite:openshift/conformance/parallel]": {},
"[sig-arch] Managed cluster should only include cluster daemonsets that have maxUnavailable update of 10 or 33 percent [Suite:openshift/conformance/parallel]": {},
"[sig-arch] Managed cluster should only include cluster daemonsets that have maxUnavailable or maxSurge update of 10 or 33 percent [Suite:openshift/conformance/parallel]": {},
"[sig-node] Events should be sent by kubelets and the scheduler about pods scheduling and running [Conformance] [Suite:openshift/conformance/parallel/minimal] [Suite:k8s]": {},
"[sig-apps] StatefulSet Basic StatefulSet functionality [StatefulSetBasic] should not deadlock when a pod's predecessor fails [Suite:openshift/conformance/parallel] [Suite:k8s]": {},
"[sig-imageregistry] Image registry remain available": {},
Expand Down
2 changes: 1 addition & 1 deletion pkg/test/ginkgo/testNames.txt
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@
"[sig-arch] Managed cluster should ensure platform components have system-* priority class associated [Suite:openshift/conformance/parallel]"
"[sig-arch] Managed cluster should ensure pods use downstream images from our release image with proper ImagePullPolicy [Suite:openshift/conformance/parallel]"
"[sig-arch] Managed cluster should have operators on the cluster version [apigroup:config.openshift.io] [Suite:openshift/conformance/parallel]"
"[sig-arch] Managed cluster should only include cluster daemonsets that have maxUnavailable update of 10 or 33 percent [apigroup:config.openshift.io] [Suite:openshift/conformance/parallel]"
"[sig-arch] Managed cluster should only include cluster daemonsets that have maxUnavailable or maxSurge update of 10 or 33 percent [apigroup:config.openshift.io] [Suite:openshift/conformance/parallel]"
"[sig-arch] Managed cluster should set requests but not limits [Suite:openshift/conformance/parallel]"
"[sig-arch] [Conformance] FIPS TestFIPS [apigroup:config.openshift.io] [Suite:openshift/conformance/parallel/minimal]"
"[sig-arch] [Conformance] sysctl pod should not start for sysctl not on whitelist kernel.msgmax [Suite:openshift/conformance/parallel/minimal]"
Expand Down
11 changes: 6 additions & 5 deletions test/extended/operators/daemon_set.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,12 @@ var _ = g.Describe("[sig-arch] Managed cluster", func() {
// would be impacted by a bug and at best the new code would roll out that much faster in very large
// spot instance machine sets.
//
// Use 10% maxUnavailable in all other cases, most especially if you have ANY impact on user
// workloads. This limits the additional load placed on the cluster to a more reasonable degree
// Use 10% maxUnavailable or maxSurge in all other cases, most especially if you have ANY impact on
// user workloads. This limits the additional load placed on the cluster to a more reasonable degree
// during an upgrade as new pods start and then establish connections.
//
// Currently only applies to daemonsets that don't explicitly target the control plane.
g.It("should only include cluster daemonsets that have maxUnavailable update of 10 or 33 percent", func() {
g.It("should only include cluster daemonsets that have maxUnavailable or maxSurge update of 10 or 33 percent", func() {
// iterate over the references to find valid images
daemonSets, err := oc.KubeFramework().ClientSet.AppsV1().DaemonSets("").List(context.Background(), metav1.ListOptions{})
if err != nil {
Expand Down Expand Up @@ -87,8 +87,9 @@ var _ = g.Describe("[sig-arch] Managed cluster", func() {
invalidDaemonSets = append(invalidDaemonSets, violation)
debug = append(debug, violation)
case ds.Spec.UpdateStrategy.RollingUpdate.MaxUnavailable.StrVal != "10%" &&
ds.Spec.UpdateStrategy.RollingUpdate.MaxUnavailable.StrVal != "33%":
violation := fmt.Sprintf("expected daemonset %s to have maxUnavailable 10%% or 33%% (see comment) instead of %s", key, ds.Spec.UpdateStrategy.RollingUpdate.MaxUnavailable.String())
ds.Spec.UpdateStrategy.RollingUpdate.MaxUnavailable.StrVal != "33%" &&
ds.Spec.UpdateStrategy.RollingUpdate.MaxSurge.StrVal != "10%":
violation := fmt.Sprintf("expected daemonset %s to have maxUnavailable 10%% or 33%% (see comment) instead of %s, or maxSurge 10%% instead of %s", key, ds.Spec.UpdateStrategy.RollingUpdate.MaxUnavailable.String(), ds.Spec.UpdateStrategy.RollingUpdate.MaxSurge.String())
invalidDaemonSets = append(invalidDaemonSets, violation)
debug = append(debug, violation)
default:
Expand Down

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

0 comments on commit 35bf447

Please sign in to comment.