From b28b8148fb6d7ca23b2b6ad106303794587d873b Mon Sep 17 00:00:00 2001 From: Miciah Masters Date: Fri, 24 Mar 2023 10:15:02 -0400 Subject: [PATCH] Allow cluster daemonsets to use maxSurge 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 ). 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. --- cmd/openshift-tests/minimal.go | 2 +- pkg/test/ginkgo/testNames.txt | 2 +- test/extended/operators/daemon_set.go | 11 ++++++----- .../annotate/generated/zz_generated.annotations.go | 2 +- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/cmd/openshift-tests/minimal.go b/cmd/openshift-tests/minimal.go index 21667293d3a9..1ccfd9f48d01 100644 --- a/cmd/openshift-tests/minimal.go +++ b/cmd/openshift-tests/minimal.go @@ -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 percent or maxUnavailable of 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": {}, diff --git a/pkg/test/ginkgo/testNames.txt b/pkg/test/ginkgo/testNames.txt index 09a51d1b9165..a1bb729a2088 100644 --- a/pkg/test/ginkgo/testNames.txt +++ b/pkg/test/ginkgo/testNames.txt @@ -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 percent or maxUnavailable of 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]" diff --git a/test/extended/operators/daemon_set.go b/test/extended/operators/daemon_set.go index f6ed034f022f..ddfeda8d7ebe 100644 --- a/test/extended/operators/daemon_set.go +++ b/test/extended/operators/daemon_set.go @@ -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 percent or maxUnavailable of 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 { @@ -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: diff --git a/test/extended/util/annotate/generated/zz_generated.annotations.go b/test/extended/util/annotate/generated/zz_generated.annotations.go index df9602e98b04..58fb874ec058 100644 --- a/test/extended/util/annotate/generated/zz_generated.annotations.go +++ b/test/extended/util/annotate/generated/zz_generated.annotations.go @@ -641,7 +641,7 @@ var Annotations = map[string]string{ "[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": " [Suite:openshift/conformance/parallel]", + "[sig-arch] Managed cluster should only include cluster daemonsets that have maxUnavailable or maxSurge update of 10 percent or maxUnavailable of 33 percent": " [Suite:openshift/conformance/parallel]", "[sig-arch] Managed cluster should recover when operator-owned objects are deleted [Disruptive][apigroup:config.openshift.io]": " [Serial]",