From 9f58185a6a8ed6d8611208987545ce8c4312ad85 Mon Sep 17 00:00:00 2001 From: Noah Kantrowitz Date: Tue, 11 Sep 2018 16:45:57 -0700 Subject: [PATCH 1/2] Don't create an impossible disruption budget for smaller clusters. --- pkg/cluster/k8sres.go | 5 ++ pkg/cluster/k8sres_test.go | 94 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+) diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 195d1c76d..574e26cde 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -1052,6 +1052,11 @@ func (c *Cluster) generateCloneEnvironment(description *acidv1.CloneDescription) func (c *Cluster) generatePodDisruptionBudget() *policybeta1.PodDisruptionBudget { minAvailable := intstr.FromInt(1) + // Avoid creating an unsatisfyable budget. + if c.Spec.NumberOfInstances <= 1 { + minAvailable = intstr.FromInt(0) + } + return &policybeta1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{ Name: c.podDisruptionBudgetName(), diff --git a/pkg/cluster/k8sres_test.go b/pkg/cluster/k8sres_test.go index 12e145c04..5c757af2f 100644 --- a/pkg/cluster/k8sres_test.go +++ b/pkg/cluster/k8sres_test.go @@ -4,7 +4,12 @@ import ( acidv1 "github.com/zalando-incubator/postgres-operator/pkg/apis/acid.zalan.do/v1" "github.com/zalando-incubator/postgres-operator/pkg/util/config" "github.com/zalando-incubator/postgres-operator/pkg/util/k8sutil" + "reflect" "testing" + + policyv1beta1 "k8s.io/api/policy/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" ) func True() *bool { @@ -17,6 +22,11 @@ func False() *bool { return &b } +func toIntStr(val int) *intstr.IntOrString { + b := intstr.FromInt(val) + return &b +} + func TestCreateLoadBalancerLogic(t *testing.T) { var cluster = New( Config{ @@ -75,3 +85,87 @@ func TestCreateLoadBalancerLogic(t *testing.T) { } } } + +func TestGeneratePodDisruptionBudget(t *testing.T) { + tests := []struct { + c *Cluster + out policyv1beta1.PodDisruptionBudget + }{ + // With multiple instances. + { + New( + Config{OpConfig: config.Config{Resources: config.Resources{ClusterNameLabel: "cluster-name", PodRoleLabel: "spilo-role"}, PDBNameFormat: "postgres-{cluster}-pdb"}}, + k8sutil.KubernetesClient{}, + acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{Name: "myapp-database", Namespace: "myapp"}, + Spec: acidv1.PostgresSpec{TeamID: "myapp", NumberOfInstances: 3}}, + logger), + policyv1beta1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: "postgres-myapp-database-pdb", + Namespace: "myapp", + Labels: map[string]string{"team": "myapp", "cluster-name": "myapp-database"}, + }, + Spec: policyv1beta1.PodDisruptionBudgetSpec{ + MinAvailable: toIntStr(1), + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"spilo-role": "master", "cluster-name": "myapp-database"}, + }, + }, + }, + }, + // With a single instance. + { + New( + Config{OpConfig: config.Config{Resources: config.Resources{ClusterNameLabel: "cluster-name", PodRoleLabel: "spilo-role"}, PDBNameFormat: "postgres-{cluster}-pdb"}}, + k8sutil.KubernetesClient{}, + acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{Name: "myapp-database", Namespace: "myapp"}, + Spec: acidv1.PostgresSpec{TeamID: "myapp", NumberOfInstances: 1}}, + logger), + policyv1beta1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: "postgres-myapp-database-pdb", + Namespace: "myapp", + Labels: map[string]string{"team": "myapp", "cluster-name": "myapp-database"}, + }, + Spec: policyv1beta1.PodDisruptionBudgetSpec{ + MinAvailable: toIntStr(0), + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"spilo-role": "master", "cluster-name": "myapp-database"}, + }, + }, + }, + }, + // With non-default PDBNameFormat. + { + New( + Config{OpConfig: config.Config{Resources: config.Resources{ClusterNameLabel: "cluster-name", PodRoleLabel: "spilo-role"}, PDBNameFormat: "postgres-{cluster}-databass-budget"}}, + k8sutil.KubernetesClient{}, + acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{Name: "myapp-database", Namespace: "myapp"}, + Spec: acidv1.PostgresSpec{TeamID: "myapp", NumberOfInstances: 3}}, + logger), + policyv1beta1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: "postgres-myapp-database-databass-budget", + Namespace: "myapp", + Labels: map[string]string{"team": "myapp", "cluster-name": "myapp-database"}, + }, + Spec: policyv1beta1.PodDisruptionBudgetSpec{ + MinAvailable: toIntStr(1), + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"spilo-role": "master", "cluster-name": "myapp-database"}, + }, + }, + }, + }, + } + + for _, tt := range tests { + result := tt.c.generatePodDisruptionBudget() + if !reflect.DeepEqual(*result, tt.out) { + t.Errorf("Expected PodDisruptionBudget: %#v, got %#v", tt.out, *result) + } + } +} From eea5fd589c6d643495a3610c01d087c19e9d802f Mon Sep 17 00:00:00 2001 From: Noah Kantrowitz Date: Sun, 16 Sep 2018 16:34:25 -0700 Subject: [PATCH 2/2] Move the setting to an explicit thing, or if there are zero nodes (which shouldn't matter anyway but be nicer to the scheduler). --- pkg/apis/acid.zalan.do/v1/postgresql_type.go | 19 +++++++------- pkg/cluster/k8sres.go | 4 +-- pkg/cluster/k8sres_test.go | 27 ++++++++++++++++++-- 3 files changed, 37 insertions(+), 13 deletions(-) diff --git a/pkg/apis/acid.zalan.do/v1/postgresql_type.go b/pkg/apis/acid.zalan.do/v1/postgresql_type.go index 3f6d34165..2c67f1b64 100644 --- a/pkg/apis/acid.zalan.do/v1/postgresql_type.go +++ b/pkg/apis/acid.zalan.do/v1/postgresql_type.go @@ -42,15 +42,16 @@ type PostgresSpec struct { // load balancers' source ranges are the same for master and replica services AllowedSourceRanges []string `json:"allowedSourceRanges"` - NumberOfInstances int32 `json:"numberOfInstances"` - Users map[string]UserFlags `json:"users"` - MaintenanceWindows []MaintenanceWindow `json:"maintenanceWindows,omitempty"` - Clone CloneDescription `json:"clone"` - ClusterName string `json:"-"` - Databases map[string]string `json:"databases,omitempty"` - Tolerations []v1.Toleration `json:"tolerations,omitempty"` - Sidecars []Sidecar `json:"sidecars,omitempty"` - PodPriorityClassName string `json:"pod_priority_class_name,omitempty"` + NumberOfInstances int32 `json:"numberOfInstances"` + Users map[string]UserFlags `json:"users"` + MaintenanceWindows []MaintenanceWindow `json:"maintenanceWindows,omitempty"` + Clone CloneDescription `json:"clone"` + ClusterName string `json:"-"` + Databases map[string]string `json:"databases,omitempty"` + Tolerations []v1.Toleration `json:"tolerations,omitempty"` + Sidecars []Sidecar `json:"sidecars,omitempty"` + PodPriorityClassName string `json:"pod_priority_class_name,omitempty"` + AllowMasterDisruption bool `json:"allow_master_disruption,omitempty"` } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 574e26cde..0d8328e5b 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -1052,8 +1052,8 @@ func (c *Cluster) generateCloneEnvironment(description *acidv1.CloneDescription) func (c *Cluster) generatePodDisruptionBudget() *policybeta1.PodDisruptionBudget { minAvailable := intstr.FromInt(1) - // Avoid creating an unsatisfyable budget. - if c.Spec.NumberOfInstances <= 1 { + // Is master disruption is enabled or if there is no master, set the budget to 0. + if c.Spec.AllowMasterDisruption || c.Spec.NumberOfInstances <= 0 { minAvailable = intstr.FromInt(0) } diff --git a/pkg/cluster/k8sres_test.go b/pkg/cluster/k8sres_test.go index 5c757af2f..6ea6b9732 100644 --- a/pkg/cluster/k8sres_test.go +++ b/pkg/cluster/k8sres_test.go @@ -114,14 +114,37 @@ func TestGeneratePodDisruptionBudget(t *testing.T) { }, }, }, - // With a single instance. + // With zero instances. { New( Config{OpConfig: config.Config{Resources: config.Resources{ClusterNameLabel: "cluster-name", PodRoleLabel: "spilo-role"}, PDBNameFormat: "postgres-{cluster}-pdb"}}, k8sutil.KubernetesClient{}, acidv1.Postgresql{ ObjectMeta: metav1.ObjectMeta{Name: "myapp-database", Namespace: "myapp"}, - Spec: acidv1.PostgresSpec{TeamID: "myapp", NumberOfInstances: 1}}, + Spec: acidv1.PostgresSpec{TeamID: "myapp", NumberOfInstances: 0}}, + logger), + policyv1beta1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: "postgres-myapp-database-pdb", + Namespace: "myapp", + Labels: map[string]string{"team": "myapp", "cluster-name": "myapp-database"}, + }, + Spec: policyv1beta1.PodDisruptionBudgetSpec{ + MinAvailable: toIntStr(0), + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"spilo-role": "master", "cluster-name": "myapp-database"}, + }, + }, + }, + }, + // With AllowMasterDisruption. + { + New( + Config{OpConfig: config.Config{Resources: config.Resources{ClusterNameLabel: "cluster-name", PodRoleLabel: "spilo-role"}, PDBNameFormat: "postgres-{cluster}-pdb"}}, + k8sutil.KubernetesClient{}, + acidv1.Postgresql{ + ObjectMeta: metav1.ObjectMeta{Name: "myapp-database", Namespace: "myapp"}, + Spec: acidv1.PostgresSpec{TeamID: "myapp", NumberOfInstances: 3, AllowMasterDisruption: true}}, logger), policyv1beta1.PodDisruptionBudget{ ObjectMeta: metav1.ObjectMeta{