diff --git a/hack/test-local.sh b/hack/test-local.sh index ca732d3c..d7e585a1 100755 --- a/hack/test-local.sh +++ b/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 ]]] " diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 1dcbd136..173fb6e4 100644 --- a/pkg/controller/controller.go +++ b/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) diff --git a/pkg/controller/poddisruptionbudget_control.go b/pkg/controller/poddisruptionbudget_control.go index 55c10aab..82d6a07d 100644 --- a/pkg/controller/poddisruptionbudget_control.go +++ b/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 } diff --git a/pkg/controller/poddisruptionbudget_control_test.go b/pkg/controller/poddisruptionbudget_control_test.go new file mode 100644 index 00000000..968c6187 --- /dev/null +++ b/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) + } + }) + } +} diff --git a/test/e2e/framework/redis_cluster_handlers.go b/test/e2e/framework/redis_cluster_handlers.go index 0b85d959..f9070723 100644 --- a/test/e2e/framework/redis_cluster_handlers.go +++ b/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 } } diff --git a/test/e2e/operator.go b/test/e2e/operator.go index 1ee11318..c98e846b 100644 --- a/test/e2e/operator.go +++ b/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() {