Skip to content

Commit c1e805a

Browse files
committed
OCPBUGS-57444: set appropriate rolling update settings
Signed-off-by: Thomas Jungblut <[email protected]>
1 parent 8e30dc7 commit c1e805a

File tree

2 files changed

+18
-7
lines changed

2 files changed

+18
-7
lines changed

bindata/oauth-openshift/deployment.yaml

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@ spec:
99
strategy:
1010
type: RollingUpdate
1111
rollingUpdate:
12-
maxUnavailable: 1
12+
# those are being adjusted for each control plane size by the deployment controller
13+
maxUnavailable: 0
1314
maxSurge: 0
1415
selector:
1516
matchLabels:
@@ -136,8 +137,8 @@ spec:
136137
preStop:
137138
exec:
138139
command:
139-
- sleep
140-
- "25"
140+
- sleep
141+
- "25"
141142
terminationMessagePolicy: FallbackToLogsOnError
142143
resources:
143144
requests:

pkg/controllers/deployment/deployment_controller.go

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"k8s.io/apimachinery/pkg/api/errors"
1212
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1313
"k8s.io/apimachinery/pkg/labels"
14+
"k8s.io/apimachinery/pkg/util/intstr"
1415
"k8s.io/client-go/informers"
1516
coreinformers "k8s.io/client-go/informers/core/v1"
1617
"k8s.io/client-go/kubernetes"
@@ -253,12 +254,21 @@ func (c *oauthServerDeploymentSyncer) Sync(ctx context.Context, syncContext fact
253254
return nil, false, append(errs, fmt.Errorf("unable to ensure at most one pod per node: %v", err))
254255
}
255256

256-
// Set the replica count to the number of master nodes.
257-
masterNodeCount, err := c.countNodes(expectedDeployment.Spec.Template.Spec.NodeSelector)
257+
// Set the replica count to the number of control plane nodes.
258+
controlPlaneCount, err := c.countNodes(expectedDeployment.Spec.Template.Spec.NodeSelector)
258259
if err != nil {
259-
return nil, false, append(errs, fmt.Errorf("failed to determine number of master nodes: %v", err))
260+
return nil, false, append(errs, fmt.Errorf("failed to determine number of control plane nodes: %v", err))
260261
}
261-
expectedDeployment.Spec.Replicas = masterNodeCount
262+
expectedDeployment.Spec.Replicas = controlPlaneCount
263+
264+
// Given the control plane sizes, we adjust the max unavailable and max surge values to mimic "MinAvailable".
265+
// We always ensure it is controlPlaneCount - 1, as this allows us to keep have at least a single replica running.
266+
// We also set MaxSurge to always be exactly the control plane count, as this allows us to more quickly replace failing
267+
// deployments with a new replica set. This does not clash with the pod anti affinity set above.
268+
maxUnavailable := intstr.FromInt32(max(*controlPlaneCount-1, 1))
269+
maxSurge := intstr.FromInt32(*controlPlaneCount)
270+
expectedDeployment.Spec.Strategy.RollingUpdate.MaxUnavailable = &maxUnavailable
271+
expectedDeployment.Spec.Strategy.RollingUpdate.MaxSurge = &maxSurge
262272

263273
deployment, _, err := resourceapply.ApplyDeployment(ctx, c.deployments,
264274
syncContext.Recorder(),

0 commit comments

Comments
 (0)