Merge pull request #16 from aermakov-zalando/use-max-unavailable

Create PDBs with maxUnavailable: 1 instead of minAvailable: 1

Author: mikkeloscar

controller.go

@@ -131,6 +131,13 @@ func (n *PDBController) addPDBs(namespace *v1.Namespace) error {
 			continue
 		}
 
+		// remove PDBs that are no longer valid, they'll be recreated on the next iteration
+		for _, pdb := range ownedPDBs {
+			if !pdbSpecValid(pdb) {
+				removePDB = append(removePDB, pdb)
+			}
+		}
+
 		// if nonReadyTTL is enabled and not all replicas of the
 		// deployment is ready then we check when the pods were last
 		// ready. This is done to ensure we don't keep PDBs for broken
@@ -178,6 +185,14 @@ func (n *PDBController) addPDBs(namespace *v1.Namespace) error {
 		// owned PDBs to not shadow what's defined by users.
 		if len(ownedPDBs) != len(matchedPDBs) {
 			removePDB = append(removePDB, ownedPDBs...)
+			continue
+		}
+
+		// remove PDBs that are no longer valid, they'll be recreated on the next iteration
+		for _, pdb := range ownedPDBs {
+			if !pdbSpecValid(pdb) {
+				removePDB = append(removePDB, pdb)
+			}
 		}
 
 		// if nonReadyTTL is enabled and not all replicas of the
@@ -200,10 +215,10 @@ func (n *PDBController) addPDBs(namespace *v1.Namespace) error {
 
 	// add missing PDBs
 	for _, resource := range addPDB {
-		minAvailable := intstr.FromInt(1)
+		maxUnavailable := intstr.FromInt(1)
 		pdb := &pv1beta1.PodDisruptionBudget{
 			Spec: pv1beta1.PodDisruptionBudgetSpec{
-				MinAvailable: &minAvailable,
+				MaxUnavailable: &maxUnavailable,
 			},
 		}
 
@@ -316,6 +331,11 @@ func (n *PDBController) getPodsLastTransitionTime(namespace string, selector map
 	return lastTransitionTime, nil
 }
 
+// pdbSpecValid returns true if the PDB spec is up-to-date
+func pdbSpecValid(pdb pv1beta1.PodDisruptionBudget) bool {
+	return pdb.Spec.MinAvailable == nil
+}
+
 // getPDBs gets matching PodDisruptionBudgets.
 func getPDBs(labels map[string]string, pdbs []pv1beta1.PodDisruptionBudget, selector map[string]string) []pv1beta1.PodDisruptionBudget {
 	matchedPDBs := make([]pv1beta1.PodDisruptionBudget, 0)

controller_test.go

@@ -3,7 +3,9 @@ package main
 import (
 	"testing"
 
+	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/kubernetes/fake"
 	"k8s.io/client-go/pkg/api/v1"
@@ -82,6 +84,107 @@ func TestRun(t *testing.T) {
 	stopCh <- struct{}{}
 }
 
+func TestRemoveInvalidPDBs(t *testing.T) {
+	deplabels := map[string]string{"foo": "deployment"}
+	sslabels := map[string]string{"foo": "statefulset"}
+	replicas := int32(2)
+
+	one := intstr.FromInt(1)
+	pdbs := []*pv1beta1.PodDisruptionBudget{
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:   "pdb-1",
+				Labels: ownerLabels,
+			},
+			Spec: pv1beta1.PodDisruptionBudgetSpec{
+				Selector: &metav1.LabelSelector{
+					MatchLabels: deplabels,
+				},
+				MinAvailable: &one,
+			},
+		},
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:   "pdb-2",
+				Labels: ownerLabels,
+			},
+			Spec: pv1beta1.PodDisruptionBudgetSpec{
+				Selector: &metav1.LabelSelector{
+					MatchLabels: sslabels,
+				},
+				MinAvailable: &one,
+			},
+		},
+	}
+
+	deployments := []*v1beta1.Deployment{
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:   "deployment-1",
+				Labels: deplabels,
+			},
+			Spec: v1beta1.DeploymentSpec{
+				Replicas: &replicas,
+				Selector: &metav1.LabelSelector{
+					MatchLabels: deplabels,
+				},
+				Template: v1.PodTemplateSpec{
+					ObjectMeta: metav1.ObjectMeta{
+						Labels: deplabels,
+					},
+				},
+			},
+		},
+	}
+
+	statefulSets := []*v1beta1.StatefulSet{
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:   "stateful-set-1",
+				Labels: sslabels,
+			},
+			Spec: v1beta1.StatefulSetSpec{
+				Replicas: &replicas,
+				Selector: &metav1.LabelSelector{
+					MatchLabels: sslabels,
+				},
+				Template: v1.PodTemplateSpec{
+					ObjectMeta: metav1.ObjectMeta{
+						Labels: sslabels,
+					},
+				},
+			},
+		},
+	}
+
+	namespaces := []*v1.Namespace{
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "default",
+			},
+		},
+	}
+
+	controller := &PDBController{
+		Interface: setupMockKubernetes(t, pdbs, deployments, statefulSets, namespaces),
+	}
+
+	err := controller.addPDBs(namespaces[0])
+	if err != nil {
+		t.Error(err)
+	}
+
+	for _, pdb := range []string{"pdb-1", "pdb-2"} {
+		pdbResource, err := controller.Interface.PolicyV1beta1().PodDisruptionBudgets("default").Get(pdb, metav1.GetOptions{})
+		if err == nil {
+			t.Fatalf("unexpected pdb (%s) found: %v", pdb, pdbResource)
+		}
+		if !errors.IsNotFound(err) {
+			t.Fatalf("unexpected error: %s", err)
+		}
+	}
+}
+
 func TestAddPDBs(t *testing.T) {
 	labels := map[string]string{"foo": "bar"}
 	notFoundLabels := map[string]string{"bar": "foo"}