Update PodDisruptionBudget in RedisCluster reconcil

hack/test-local.sh

@@ -9,15 +9,6 @@ minikube start
 echo "Create the missing rolebinding for k8s dashboard"
 kubectl create clusterrolebinding add-on-cluster-admin --clusterrole=cluster-admin --serviceaccount=kube-system:default
 
-echo "Init the helm tiller"
-kubectl -n kube-system create sa tiller
-kubectl create clusterrolebinding tiller --clusterrole cluster-admin --serviceaccount=kube-system:tiller
-helm init --service-account tiller
-
-printf "Waiting for tiller deployment to complete."
-until [ $(kubectl get deployment -n kube-system tiller-deploy -ojsonpath="{.status.conditions[?(@.type=='Available')].status}") == "True" ] > /dev/null 2>&1; do sleep 1; printf "."; done
-echo
-
 eval $(minikube docker-env)
 echo "Install the redis-cluster operator"
 
@@ -31,11 +22,11 @@ echo "create RBAC for rediscluster"
 #kubectl create -f $GIT_ROOT/examples/RedisCluster_RBAC.yaml
 
 printf  "create and install the redis operator in a dedicate namespace"
-until helm install -n operator --set image.tag=$TAG chart/redis-operator; do sleep 1; printf "."; done
+until helm install operator --set image.tag=$TAG charts/operator-for-redis; do sleep 1; printf "."; done
 echo
 
 printf "Waiting for redis-operator deployment to complete."
-until [ $(kubectl get deployment operator-redis-operator -ojsonpath="{.status.conditions[?(@.type=='Available')].status}") == "True" ] > /dev/null 2>&1; do sleep 1; printf "."; done
+until [ $(kubectl get deployment operator-operator-for-redis -ojsonpath="{.status.conditions[?(@.type=='Available')].status}") == "True" ] > /dev/null 2>&1; do sleep 1; printf "."; done
 echo
 
 echo "[[[ Run End2end test ]]] "

pkg/controller/controller.go

@@ -234,6 +234,17 @@ func (c *Controller) syncCluster(ctx context.Context, redisCluster *rapi.RedisCl
 			glog.Errorf("RedisCluster-Operator.Reconcile unable to create podDisruptionBudget associated with RedisCluster: %s/%s", redisCluster.Namespace, redisCluster.Name)
 			return result, err
 		}
+	} else {
+		needUpdatePDB, err := c.podDisruptionBudgetControl.NeedUpdateRedisClusterPodDisruptionBudget(redisCluster, redisClusterPodDisruptionBudget)
+		if err != nil {
+			return result, err
+		}
+		if needUpdatePDB {
+			if _, err = c.podDisruptionBudgetControl.UpdateRedisClusterPodDisruptionBudget(redisCluster, redisClusterPodDisruptionBudget); err != nil {
+				glog.Errorf("RedisCluster-Operator.Reconcile unable to update podDisruptionBudget associated with RedisCluster: %s/%s", redisCluster.Namespace, redisCluster.Name)
+				return result, err
+			}
+		}
 	}
 
 	redisPods, err := c.podControl.GetRedisClusterPods(redisCluster)

pkg/controller/poddisruptionbudget_control.go

@@ -3,6 +3,7 @@ package controller
 import (
 	"context"
 
+	"k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -21,6 +22,10 @@ import (
 type PodDisruptionBudgetsControlInterface interface {
 	// CreateRedisClusterPodDisruptionBudget used to create the Kubernetes PodDisruptionBudget needed to access the Redis Cluster
 	CreateRedisClusterPodDisruptionBudget(redisCluster *rapi.RedisCluster) (*policyv1.PodDisruptionBudget, error)
+	// UpdateRedisClusterPodDisruptionBudget used to update the Kubernetes PodDisruptionBudget needed to access the Redis Cluster
+	UpdateRedisClusterPodDisruptionBudget(redisCluster *rapi.RedisCluster, existingPDB *policyv1.PodDisruptionBudget) (*policyv1.PodDisruptionBudget, error)
+	// NeedUpdateRedisClusterPodDisruptionBudget used to check if the Kubernetes PodDisruptionBudget needed to access the Redis cluster needs to be updated
+	NeedUpdateRedisClusterPodDisruptionBudget(redisCluster *rapi.RedisCluster, existingPDB *policyv1.PodDisruptionBudget) (bool, error)
 	// DeleteRedisClusterPodDisruptionBudget used to delete the Kubernetes PodDisruptionBudget linked to the Redis Cluster
 	DeleteRedisClusterPodDisruptionBudget(redisCluster *rapi.RedisCluster) error
 	// GetRedisClusterPodDisruptionBudget used to retrieve the Kubernetes PodDisruptionBudget associated to the RedisCluster
@@ -72,6 +77,56 @@ func (s *PodDisruptionBudgetsControl) DeleteRedisClusterPodDisruptionBudget(redi
 
 // CreateRedisClusterPodDisruptionBudget used to create the Kubernetes PodDisruptionBudget needed to access the Redis Cluster
 func (s *PodDisruptionBudgetsControl) CreateRedisClusterPodDisruptionBudget(redisCluster *rapi.RedisCluster) (*policyv1.PodDisruptionBudget, error) {
+	desiredPodDisruptionBudget, err := s.desiredRedisClusterPodDisruptionBudget(redisCluster)
+	if err != nil {
+		return nil, err
+	}
+
+	err = s.KubeClient.Create(context.Background(), desiredPodDisruptionBudget)
+	if err != nil {
+		return nil, err
+	}
+
+	return desiredPodDisruptionBudget, nil
+}
+
+// NeedUpdateRedisClusterPodDisruptionBudget used to check if the Kubernetes PodDisruptionBudget needed to access the Redis cluster needs to be updated
+func (s *PodDisruptionBudgetsControl) NeedUpdateRedisClusterPodDisruptionBudget(redisCluster *rapi.RedisCluster, existingPDB *policyv1.PodDisruptionBudget) (bool, error) {
+	desiredPodDisruptionBudget, err := s.desiredRedisClusterPodDisruptionBudget(redisCluster)
+	if err != nil {
+		return false, err
+	}
+
+	if !equality.Semantic.DeepEqual(existingPDB.Labels, desiredPodDisruptionBudget.Labels) ||
+		!equality.Semantic.DeepEqual(existingPDB.Annotations, desiredPodDisruptionBudget.Annotations) ||
+		!equality.Semantic.DeepEqual(existingPDB.Spec, desiredPodDisruptionBudget.Spec) {
+		return true, nil
+	}
+
+	return false, nil
+}
+
+// UpdateRedisClusterPodDisruptionBudget used to update the Kubernetes PodDisruptionBudget needed to access the Redis Cluster
+func (s *PodDisruptionBudgetsControl) UpdateRedisClusterPodDisruptionBudget(redisCluster *rapi.RedisCluster, existingPDB *policyv1.PodDisruptionBudget) (*policyv1.PodDisruptionBudget, error) {
+	desiredPodDisruptionBudget, err := s.desiredRedisClusterPodDisruptionBudget(redisCluster)
+	if err != nil {
+		return nil, err
+	}
+
+	newPodDisruptionBudget := existingPDB.DeepCopy()
+	newPodDisruptionBudget.ObjectMeta.Labels = desiredPodDisruptionBudget.Labels
+	newPodDisruptionBudget.ObjectMeta.Annotations = desiredPodDisruptionBudget.Annotations
+	newPodDisruptionBudget.Spec = desiredPodDisruptionBudget.Spec
+
+	err = s.KubeClient.Update(context.Background(), newPodDisruptionBudget)
+	if err != nil {
+		return nil, err
+	}
+
+	return newPodDisruptionBudget, nil
+}
+
+func (s *PodDisruptionBudgetsControl) desiredRedisClusterPodDisruptionBudget(redisCluster *rapi.RedisCluster) (*policyv1.PodDisruptionBudget, error) {
 	PodDisruptionBudgetName := redisCluster.Name
 	desiredLabels, err := pod.GetLabelsSet(redisCluster)
 	if err != nil {
@@ -102,10 +157,5 @@ func (s *PodDisruptionBudgetsControl) CreateRedisClusterPodDisruptionBudget(redi
 			Selector:     &labelSelector,
 		},
 	}
-	err = s.KubeClient.Create(context.Background(), newPodDisruptionBudget)
-	if err != nil {
-		return nil, err
-	}
-
 	return newPodDisruptionBudget, nil
 }

pkg/controller/poddisruptionbudget_control_test.go

@@ -0,0 +1,134 @@
+package controller
+
+import (
+	"testing"
+
+	rapi "github.com/IBM/operator-for-redis-cluster/api/v1alpha1"
+	"github.com/gogo/protobuf/proto"
+	policyv1 "k8s.io/api/policy/v1"
+	"k8s.io/apimachinery/pkg/api/equality"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
+	"k8s.io/client-go/tools/record"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+func TestPodDisruptionBudgetsControl_desiredRedisClusterPodDisruptionBudget(t *testing.T) {
+	boolPtr := func(value bool) *bool {
+		return &value
+	}
+
+	type fields struct {
+		KubeClient client.Client
+		Recorder   record.EventRecorder
+	}
+	type args struct {
+		redisCluster *rapi.RedisCluster
+	}
+	tests := []struct {
+		name    string
+		fields  fields
+		args    args
+		want    *policyv1.PodDisruptionBudget
+		wantErr bool
+	}{
+		{
+			name: "3 primary, replica=1",
+			args: args{
+				redisCluster: &rapi.RedisCluster{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "rediscluster",
+					},
+					Spec: rapi.RedisClusterSpec{
+						NumberOfPrimaries: proto.Int32(3),
+						ReplicationFactor: proto.Int32(1),
+					},
+				},
+			},
+			want: &policyv1.PodDisruptionBudget{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "rediscluster",
+					Labels: map[string]string{
+						rapi.ClusterNameLabelKey: "rediscluster",
+					},
+					Annotations: map[string]string{},
+					OwnerReferences: []metav1.OwnerReference{
+						{
+							Name:       "rediscluster",
+							APIVersion: rapi.GroupVersion.String(),
+							Kind:       rapi.ResourceKind,
+							Controller: boolPtr(true),
+						},
+					},
+				},
+				Spec: policyv1.PodDisruptionBudgetSpec{
+					MinAvailable: &intstr.IntOrString{
+						IntVal: 5,
+					},
+					Selector: &metav1.LabelSelector{
+						MatchLabels: map[string]string{
+							rapi.ClusterNameLabelKey: "rediscluster",
+						},
+					},
+				},
+			},
+		},
+		{
+			name: "2 primary, replica=1",
+			args: args{
+				redisCluster: &rapi.RedisCluster{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "rediscluster",
+					},
+					Spec: rapi.RedisClusterSpec{
+						NumberOfPrimaries: proto.Int32(2),
+						ReplicationFactor: proto.Int32(1),
+					},
+				},
+			},
+			want: &policyv1.PodDisruptionBudget{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "rediscluster",
+					Labels: map[string]string{
+						rapi.ClusterNameLabelKey: "rediscluster",
+					},
+					Annotations: map[string]string{},
+					OwnerReferences: []metav1.OwnerReference{
+						{
+							Name:       "rediscluster",
+							APIVersion: rapi.GroupVersion.String(),
+							Kind:       rapi.ResourceKind,
+							Controller: boolPtr(true),
+						},
+					},
+				},
+				Spec: policyv1.PodDisruptionBudgetSpec{
+					MinAvailable: &intstr.IntOrString{
+						IntVal: 3,
+					},
+					Selector: &metav1.LabelSelector{
+						MatchLabels: map[string]string{
+							rapi.ClusterNameLabelKey: "rediscluster",
+						},
+					},
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			s := &PodDisruptionBudgetsControl{
+				KubeClient: tt.fields.KubeClient,
+				Recorder:   tt.fields.Recorder,
+			}
+			got, err := s.desiredRedisClusterPodDisruptionBudget(tt.args.redisCluster)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("PodDisruptionBudgetsControl.desiredRedisClusterPodDisruptionBudget() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !equality.Semantic.DeepEqual(got, tt.want) {
+				t.Errorf("PodDisruptionBudgetsControl.desiredRedisClusterPodDisruptionBudget() = %+v, want %+v", got, tt.want)
+			}
+		})
+	}
+}

test/e2e/framework/redis_cluster_handlers.go

@@ -477,14 +477,19 @@ func CreateRedisNodeServiceAccountFunc(kubeClient kclient.Client, redisCluster *
 
 // IsPodDisruptionBudgetCreatedFunc returns the func that checks if the PodDisruptionBudget
 // associated with the RedisCluster has been created properly.
-func IsPodDisruptionBudgetCreatedFunc(kubeClient kclient.Client, redisCluster *rapi.RedisCluster) func() error {
+func IsPodDisruptionBudgetCreatedFunc(kubeClient kclient.Client, redisCluster *rapi.RedisCluster, minAvailable int32) func() error {
 	return func() error {
+		pdb := &policyv1.PodDisruptionBudget{}
 		pdbName := types.NamespacedName{Namespace: redisCluster.Namespace, Name: redisCluster.Name}
-		err := kubeClient.Get(context.Background(), pdbName, &policyv1.PodDisruptionBudget{})
+		err := kubeClient.Get(context.Background(), pdbName, pdb)
 		if err != nil {
 			Logf("Cannot get PodDisruptionBudget associated to the redisCluster:%s/%s, err:%v", redisCluster.Namespace, redisCluster.Name, err)
 			return err
 		}
+
+		if minAvailable != pdb.Spec.MinAvailable.IntVal {
+			return LogAndReturnErrorf("PodDisruptionBudget minAvailable mismatch: %v, but got: %v", minAvailable, pdb.Spec.MinAvailable.IntVal)
+		}
 		return nil
 	}
 }

test/e2e/operator.go

@@ -45,7 +45,7 @@ var _ = Describe("RedisCluster CRUD operations", func() {
 
 		Eventually(framework.CreateRedisClusterConfigMapFunc(kubeClient, cluster), "5s", "1s").ShouldNot(HaveOccurred())
 
-		Eventually(framework.IsPodDisruptionBudgetCreatedFunc(kubeClient, cluster), "5s", "1s").ShouldNot(HaveOccurred())
+		Eventually(framework.IsPodDisruptionBudgetCreatedFunc(kubeClient, cluster, int32(5)), "5s", "1s").ShouldNot(HaveOccurred())
 
 		Eventually(framework.IsRedisClusterStartedFunc(kubeClient, cluster), "8m", "5s").ShouldNot(HaveOccurred())
 
@@ -87,6 +87,8 @@ lazyfree-lazy-expire yes`,
 
 			Eventually(framework.IsRedisClusterStartedFunc(kubeClient, cluster), "5m", "5s").ShouldNot(HaveOccurred())
 
+			Eventually(framework.IsPodDisruptionBudgetCreatedFunc(kubeClient, cluster, int32(7)), "5s", "1s").ShouldNot(HaveOccurred())
+
 			Eventually(framework.ZonesBalancedFunc(kubeClient, cluster), "10s", "1s").ShouldNot(HaveOccurred())
 		})
 		Context("a RedisCluster is running", func() {