-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Allow cluster daemonsets to use maxSurge #27819
Allow cluster daemonsets to use maxSurge #27819
Conversation
3b27bb7
to
1ffc9d7
Compare
1ffc9d7
to
35bf447
Compare
Rebased for changes in #27714. |
Lots of jobs failed with |
@mfojtik, could you approve this change? This PR is related to openshift/cluster-dns-operator#358, which is intended to fix a 4.13 release blocker. I'll want to backport both this origin PR and the cluster-dns-operator PR to their respective release-4.13 branches. |
@bparees, could you approve this change? (Maybe I should just avoid changing the test name so I wouldn't need to ping approvers from multiple |
/lgtm |
/approve |
35bf447
to
1219706
Compare
https://github.com/openshift/origin/compare/35bf4473e6cae0f8ef4748fe6216967411c93462..1219706cf49d08e6bf707e2274ef144b9b4d92b8 updates the test name per comments on Slack. |
Agree with the update. |
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.
1219706
to
b28b814
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bparees, gcs278, Miciah The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
e2e-gcp-ovn-rt-upgrade failed because one of the kube-apiserver pods didn't come up. As far as I can tell, the installer pod for master-1 didn't get created; |
e2e-aws-ovn-serial failed because |
@Miciah: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
/cherry-pick release-4.13 |
@Miciah: new pull request created: #27859 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
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 withmaxSurge
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, themaxSurge
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 usingmaxSurge
. However, whenmaxSurge
is used,maxUnavailable
must be zero. Thus this PR changes the test to allowmaxUnavailable
to be zero ifmaxSurge
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 themaxSurge
value in the violation message when neithermaxUnavailable
normaxSurge
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.