Skip to content

Commit

Permalink
fix(pdb): taking 90% of min available to allow recycling nodes
Browse files Browse the repository at this point in the history
  • Loading branch information
@nettoclaudio
nettoclaudio committed Jan 27, 2022
1 parent cf15a48 commit 4ae3829
Show file tree
Hide file tree
Showing 5 changed files with 159 additions and 22 deletions.
10 changes: 6 additions & 4 deletions api/v1alpha1/rpaasinstance_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,12 @@ type RpaasInstanceSpec struct {
// to Nginx or not. Defaults to disabled.
//
// If enabled, PDB's min available is calculated as:
// - rpaasinstance.spec.autoscale.minReplicas (if set and less than maxReplicas);
// - rpaasinstance.spec.autoscale.maxReplicas - 1 (if set);
// - rpaasinstance.spec.replicas - 1 (if set);
// - zero, otherwise.
// minAvailable = floor(N * 90%), where
// N:
// - rpaasinstance.spec.autoscale.minReplicas (if set and less than maxReplicas);
// - rpaasinstance.spec.autoscale.maxReplicas (if set);
// - rpaasinstance.spec.replicas (if set);
// - zero, otherwise.
//
// +optional
EnablePodDisruptionBudget *bool `json:"enablePodDisruptionBudget,omitempty"`
Expand Down
49 changes: 45 additions & 4 deletions config/crd/bases/extensions.tsuru.io_rpaasflavors.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,10 +226,11 @@ spec:
enablePodDisruptionBudget:
description: "EnablePodDisruptionBudget defines whether a PodDisruptionBudget
should be attached to Nginx or not. Defaults to disabled. \n
If enabled, PDB's min available is calculated as: - rpaasinstance.spec.autoscale.minReplicas
(if set and less than maxReplicas); - rpaasinstance.spec.autoscale.maxReplicas
- 1 (if set); - rpaasinstance.spec.replicas - 1 (if set); -
zero, otherwise."
If enabled, PDB's min available is calculated as: minAvailable
= floor(N * 90%), where N: - rpaasinstance.spec.autoscale.minReplicas
(if set and less than maxReplicas); - rpaasinstance.spec.autoscale.maxReplicas
(if set); - rpaasinstance.spec.replicas (if set); - zero,
otherwise."
type: boolean
extraFiles:
description: ExtraFiles points to a ConfigMap where the files
Expand Down Expand Up @@ -2724,6 +2725,46 @@ spec:
- containerPort
type: object
type: array
rollingUpdate:
description: RollingUpdate defines params to control the desired
behavior of rolling update.
properties:
maxSurge:
anyOf:
- type: integer
- type: string
description: 'The maximum number of pods that can be scheduled
above the desired number of pods. Value can be an absolute
number (ex: 5) or a percentage of desired pods (ex:
10%). This can not be 0 if MaxUnavailable is 0. Absolute
number is calculated from percentage by rounding up.
Defaults to 25%. Example: when this is set to 30%, the
new ReplicaSet can be scaled up immediately when the
rolling update starts, such that the total number of
old and new pods do not exceed 130% of desired pods.
Once old pods have been killed, new ReplicaSet can be
scaled up further, ensuring that total number of pods
running at any time during the update is at most 130%
of desired pods.'
x-kubernetes-int-or-string: true
maxUnavailable:
anyOf:
- type: integer
- type: string
description: 'The maximum number of pods that can be unavailable
during the update. Value can be an absolute number (ex:
5) or a percentage of desired pods (ex: 10%). Absolute
number is calculated from percentage by rounding down.
This can not be 0 if MaxSurge is 0. Defaults to 25%.
Example: when this is set to 30%, the old ReplicaSet
can be scaled down to 70% of desired pods immediately
when the rolling update starts. Once new pods are ready,
old ReplicaSet can be scaled down further, followed
by scaling up the new ReplicaSet, ensuring that the
total number of pods available at all times during the
update is at least 70% of desired pods.'
x-kubernetes-int-or-string: true
type: object
securityContext:
description: SecurityContext configures security attributes
for the nginx pod.
Expand Down
46 changes: 42 additions & 4 deletions config/crd/bases/extensions.tsuru.io_rpaasinstances.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,10 +207,10 @@ spec:
enablePodDisruptionBudget:
description: "EnablePodDisruptionBudget defines whether a PodDisruptionBudget
should be attached to Nginx or not. Defaults to disabled. \n If
enabled, PDB's min available is calculated as: - rpaasinstance.spec.autoscale.minReplicas
(if set and less than maxReplicas); - rpaasinstance.spec.autoscale.maxReplicas
- 1 (if set); - rpaasinstance.spec.replicas - 1 (if set); - zero,
otherwise."
enabled, PDB's min available is calculated as: minAvailable = floor(N
* 90%), where N: - rpaasinstance.spec.autoscale.minReplicas (if
set and less than maxReplicas); - rpaasinstance.spec.autoscale.maxReplicas
(if set); - rpaasinstance.spec.replicas (if set); - zero, otherwise."
type: boolean
extraFiles:
description: ExtraFiles points to a ConfigMap where the files are
Expand Down Expand Up @@ -2616,6 +2616,44 @@ spec:
- containerPort
type: object
type: array
rollingUpdate:
description: RollingUpdate defines params to control the desired
behavior of rolling update.
properties:
maxSurge:
anyOf:
- type: integer
- type: string
description: 'The maximum number of pods that can be scheduled
above the desired number of pods. Value can be an absolute
number (ex: 5) or a percentage of desired pods (ex: 10%).
This can not be 0 if MaxUnavailable is 0. Absolute number
is calculated from percentage by rounding up. Defaults to
25%. Example: when this is set to 30%, the new ReplicaSet
can be scaled up immediately when the rolling update starts,
such that the total number of old and new pods do not exceed
130% of desired pods. Once old pods have been killed, new
ReplicaSet can be scaled up further, ensuring that total
number of pods running at any time during the update is
at most 130% of desired pods.'
x-kubernetes-int-or-string: true
maxUnavailable:
anyOf:
- type: integer
- type: string
description: 'The maximum number of pods that can be unavailable
during the update. Value can be an absolute number (ex:
5) or a percentage of desired pods (ex: 10%). Absolute number
is calculated from percentage by rounding down. This can
not be 0 if MaxSurge is 0. Defaults to 25%. Example: when
this is set to 30%, the old ReplicaSet can be scaled down
to 70% of desired pods immediately when the rolling update
starts. Once new pods are ready, old ReplicaSet can be scaled
down further, followed by scaling up the new ReplicaSet,
ensuring that the total number of pods available at all
times during the update is at least 70% of desired pods.'
x-kubernetes-int-or-string: true
type: object
securityContext:
description: SecurityContext configures security attributes for
the nginx pod.
Expand Down
13 changes: 6 additions & 7 deletions controllers/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"encoding/base32"
"encoding/json"
"fmt"
"math"
"reflect"
"sort"
"strconv"
Expand Down Expand Up @@ -665,22 +666,20 @@ func newPDB(instance *v1alpha1.RpaasInstance, nginx *nginxv1alpha1.Nginx) (*poli

var minAvailable intstr.IntOrString
if replicas := instance.Spec.Replicas; replicas != nil {
// NOTE: reducing by one to allow operational tasks as draining nodes in the cluster.
minAvailable = intstr.FromInt(int(*replicas) - 1)
minAvailable = intstr.FromInt(int(*replicas))
}

if autoscale := instance.Spec.Autoscale; autoscale != nil {
// NOTE: reducing by one to allow operational tasks as draining nodes in the cluster.
minAvailable = intstr.FromInt(int(autoscale.MaxReplicas) - 1)
minAvailable = intstr.FromInt(int(autoscale.MaxReplicas))

if min := instance.Spec.Autoscale.MinReplicas; min != nil && *min < autoscale.MaxReplicas {
minAvailable = intstr.FromInt(int(*min))
}
}

if minAvailable.IntValue() < 0 {
minAvailable = intstr.FromInt(0)
}
// NOTE: taking 90% of the real min available to support operational tasks
// in the cluster, e.g scaling up/down nodes from Cluster Autoscaler.
minAvailable = intstr.FromInt(int(math.Floor(float64(minAvailable.IntValue()) * 0.9)))

return &policyv1beta1.PodDisruptionBudget{
TypeMeta: metav1.TypeMeta{
Expand Down
63 changes: 60 additions & 3 deletions controllers/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1014,6 +1014,63 @@ func Test_reconcilePDB(t *testing.T) {
nginx *nginxv1alpha1.Nginx
assert func(t *testing.T, c client.Client)
}{
"creating PDB, instance with 1 replicas": {
instance: &v1alpha1.RpaasInstance{
ObjectMeta: metav1.ObjectMeta{
Name: "my-instance",
Namespace: "rpaasv2",
},
Spec: v1alpha1.RpaasInstanceSpec{
EnablePodDisruptionBudget: func(b bool) *bool { return &b }(true),
Replicas: func(n int32) *int32 { return &n }(1),
},
},
nginx: &nginxv1alpha1.Nginx{
ObjectMeta: metav1.ObjectMeta{
Name: "my-instance",
Namespace: "rpaasv2",
},
Status: nginxv1alpha1.NginxStatus{
PodSelector: "nginx.tsuru.io/resource-name=my-instance",
},
},
assert: func(t *testing.T, c client.Client) {
var pdb policyv1beta1.PodDisruptionBudget
err := c.Get(context.TODO(), client.ObjectKey{Name: "my-instance", Namespace: "rpaasv2"}, &pdb)
require.NoError(t, err)
assert.Equal(t, policyv1beta1.PodDisruptionBudget{
TypeMeta: metav1.TypeMeta{
APIVersion: "policy/v1beta1",
Kind: "PodDisruptionBudget",
},
ObjectMeta: metav1.ObjectMeta{
Name: "my-instance",
Namespace: "rpaasv2",
Labels: map[string]string{
"rpaas.extensions.tsuru.io/instance-name": "my-instance",
"rpaas.extensions.tsuru.io/plan-name": "",
},
ResourceVersion: "1",
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: "extensions.tsuru.io/v1alpha1",
Kind: "RpaasInstance",
Name: "my-instance",
Controller: func(b bool) *bool { return &b }(true),
BlockOwnerDeletion: func(b bool) *bool { return &b }(true),
},
},
},
Spec: policyv1beta1.PodDisruptionBudgetSpec{
MinAvailable: func(n intstr.IntOrString) *intstr.IntOrString { return &n }(intstr.FromInt(0)),
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{"nginx.tsuru.io/resource-name": "my-instance"},
},
},
}, pdb)
},
},

"creating PDB, instance with 10 replicas": {
instance: &v1alpha1.RpaasInstance{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -1122,7 +1179,7 @@ func Test_reconcilePDB(t *testing.T) {
},
},
Spec: policyv1beta1.PodDisruptionBudgetSpec{
MinAvailable: func(n intstr.IntOrString) *intstr.IntOrString { return &n }(intstr.FromInt(99)),
MinAvailable: func(n intstr.IntOrString) *intstr.IntOrString { return &n }(intstr.FromInt(90)),
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{"nginx.tsuru.io/resource-name": "my-instance"},
},
Expand Down Expand Up @@ -1183,7 +1240,7 @@ func Test_reconcilePDB(t *testing.T) {
},
},
Spec: policyv1beta1.PodDisruptionBudgetSpec{
MinAvailable: func(n intstr.IntOrString) *intstr.IntOrString { return &n }(intstr.FromInt(50)),
MinAvailable: func(n intstr.IntOrString) *intstr.IntOrString { return &n }(intstr.FromInt(45)),
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{"nginx.tsuru.io/resource-name": "my-instance"},
},
Expand Down Expand Up @@ -1244,7 +1301,7 @@ func Test_reconcilePDB(t *testing.T) {
},
},
Spec: policyv1beta1.PodDisruptionBudgetSpec{
MinAvailable: func(n intstr.IntOrString) *intstr.IntOrString { return &n }(intstr.FromInt(50)),
MinAvailable: func(n intstr.IntOrString) *intstr.IntOrString { return &n }(intstr.FromInt(45)),
Selector: &metav1.LabelSelector{
MatchLabels: map[string]string{"nginx.tsuru.io/resource-name": "another-instance"},
},
Expand Down

0 comments on commit 4ae3829

Please sign in to comment.