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

OCPBUGS-13209: Revert "Set DNS DaemonSet's maxSurge value to 10%" #379

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
17 changes: 6 additions & 11 deletions pkg/manifests/assets/dns/daemonset.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ kind: DaemonSet
apiVersion: apps/v1
# name, namespace and labels are set at runtime
spec:
# minReadySeconds should be 3x the readiness probe's polling interval (i.e. periodSeconds).
minReadySeconds: 9
template:
metadata:
annotations:
Expand Down Expand Up @@ -35,7 +33,7 @@ spec:
port: 8181
scheme: HTTP
initialDelaySeconds: 10
periodSeconds: 3 # Update the daemonset's spec.minReadySeconds above if you change this value!
periodSeconds: 3
successThreshold: 1
failureThreshold: 3
timeoutSeconds: 3
Expand Down Expand Up @@ -87,11 +85,8 @@ spec:
updateStrategy:
type: RollingUpdate
rollingUpdate:
# Set maxSurge to a positive value so that each node that has a pod
# continues to have a local ready pod during a rolling update. This is
# important for topology-aware hints as well as for similar logic in
# openshift-sdn and ovn-kubernetes that prefers to use a local ready DNS
# pod whenever one exists.
maxSurge: 10%
# maxUnavailable must be zero when maxSurge is nonzero.
maxUnavailable: 0
# TODO: Consider setting maxSurge to a positive value.
maxSurge: 0
# Note: The daemon controller rounds the percentage up
# (unlike the deployment controller, which rounds down).
maxUnavailable: 10%
5 changes: 0 additions & 5 deletions pkg/operator/controller/controller_dns_daemonset.go
Original file line number Diff line number Diff line change
Expand Up @@ -337,11 +337,6 @@ func daemonsetConfigChanged(current, expected *appsv1.DaemonSet) (bool, *appsv1.
changed := false
updated := current.DeepCopy()

if current.Spec.MinReadySeconds != expected.Spec.MinReadySeconds {
updated.Spec.MinReadySeconds = expected.Spec.MinReadySeconds
changed = true
}

if !cmp.Equal(current.Spec.UpdateStrategy, expected.Spec.UpdateStrategy, cmpopts.EquateEmpty()) {
updated.Spec.UpdateStrategy = expected.Spec.UpdateStrategy
changed = true
Expand Down
35 changes: 7 additions & 28 deletions pkg/operator/controller/controller_dns_daemonset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,6 @@ func TestDesiredDNSDaemonset(t *testing.T) {
t.Errorf("invalid dns daemonset: %v", err)
} else {
// Validate the daemonset
pointerTo := func(ios intstr.IntOrString) *intstr.IntOrString { return &ios }
expectedUpdateStrategy := appsv1.DaemonSetUpdateStrategy{
Type: appsv1.RollingUpdateDaemonSetStrategyType,
RollingUpdate: &appsv1.RollingUpdateDaemonSet{
MaxSurge: pointerTo(intstr.FromString("10%")),
MaxUnavailable: pointerTo(intstr.FromInt(0)),
},
}
actualUpdateStrategy := ds.Spec.UpdateStrategy
if !reflect.DeepEqual(actualUpdateStrategy, expectedUpdateStrategy) {
t.Errorf("unexpected update strategy: expected %#v, got %#v", expectedUpdateStrategy, actualUpdateStrategy)
}
expectedPodAnnotations := map[string]string{
"cluster-autoscaler.kubernetes.io/enable-ds-eviction": "true",
"target.workload.openshift.io/management": "{\"effect\": \"PreferredDuringScheduling\"}",
Expand Down Expand Up @@ -481,23 +469,14 @@ func TestDaemonsetConfigChanged(t *testing.T) {
expect: true,
},
{
description: "if the update strategy's max unavailable parameter changes",
description: "if the update strategy changes",
mutate: func(daemonset *appsv1.DaemonSet) {
daemonset.Spec.UpdateStrategy.RollingUpdate.MaxUnavailable = pointerTo(intstr.FromString("10%"))
},
expect: true,
},
{
description: "if the update strategy's max surge parameter changes",
mutate: func(daemonset *appsv1.DaemonSet) {
daemonset.Spec.UpdateStrategy.RollingUpdate.MaxSurge = pointerTo(intstr.FromString("10%"))
},
expect: true,
},
{
description: "if spec.minReadySeconds changes",
mutate: func(daemonset *appsv1.DaemonSet) {
daemonset.Spec.MinReadySeconds = 9
daemonset.Spec.UpdateStrategy = appsv1.DaemonSetUpdateStrategy{
Type: appsv1.RollingUpdateDaemonSetStrategyType,
RollingUpdate: &appsv1.RollingUpdateDaemonSet{
MaxUnavailable: pointerTo(intstr.FromString("10%")),
},
}
},
expect: true,
},
Expand Down