Issue-497: Added changes for configuring PDB (#499)

* added changes for configuringpdb Signed-off-by: prabhaker24 <[email protected]> * changed test crd Signed-off-by: prabhaker24 <[email protected]> * chart changes added Signed-off-by: prabhaker24 <[email protected]> * added changes for e2e and changed logic for updating pdb Signed-off-by: prabhaker24 <[email protected]> * added comment and ut Signed-off-by: prabhaker24 <[email protected]> Co-authored-by: prabhaker24 <[email protected]>
pravega · Jan 30, 2021 · 03fca50 · 03fca50
1 parent 9d15bf1
commit 03fca50
Show file tree

Hide file tree

Showing 12 changed files with 129 additions and 4 deletions.
diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml
@@ -111,6 +111,7 @@ jobs:
         ssh -o StrictHostKeyChecking=no root@$CLUSTER_IP "kubectl create -f /root/pravega-operator/deploy/certificate.yaml"
         ssh -o StrictHostKeyChecking=no root@$CLUSTER_IP "kubectl create -f /root/pravega-operator/deploy/webhook.yaml"
         ssh -o StrictHostKeyChecking=no root@$CLUSTER_IP "kubectl create -f /root/pravega-operator/test/e2e/resources/crd.yaml"
+        ssh -o StrictHostKeyChecking=no root@$CLUSTER_IP "kubectl -n default create secret docker-registry regcred --docker-server=https://index.docker.io/v1/ --docker-username=testpravegaop --docker-password=c155d654-3c09-48aa-a62d-2d5178454784 [email protected]"
         ssh -o StrictHostKeyChecking=no root@$CLUSTER_IP "curl -Lo operator-sdk https://github.com/operator-framework/operator-sdk/releases/download/$OPERATOR_SDK_VERSION/operator-sdk-$OPERATOR_SDK_VERSION-x86_64-linux-gnu && chmod +x operator-sdk && sudo mv operator-sdk /usr/local/bin/"
         ssh -o StrictHostKeyChecking=no root@$CLUSTER_IP "bash < <(curl -s -S -L https://raw.githubusercontent.com/moovweb/gvm/master/binscripts/gvm-installer);source /root/.gvm/scripts/gvm;gvm install go1.13.8 --binary;gvm use go1.13.8 --default"
 

diff --git a/charts/pravega-operator/templates/pravega.pravega.io_pravegaclusters_crd.yaml b/charts/pravega-operator/templates/pravega.pravega.io_pravegaclusters_crd.yaml
@@ -1134,6 +1134,16 @@ spec:
                             type: string
                         type: object
                     type: object
+                  maxUnavailableControllerReplicas:
+                    description: MaxUnavailableControllerReplicas defines the MaxUnavailable
+                      Controller Replicas Default is 1.
+                    format: int32
+                    type: integer
+                  maxUnavailableSegmentStoreReplicas:
+                    description: MaxUnavailableSegmentStoreReplicas defines the MaxUnavailable
+                      SegmentStore Replicas Default is 1.
+                    format: int32
+                    type: integer
                   options:
                     additionalProperties:
                       type: string

diff --git a/charts/pravega/README.md b/charts/pravega/README.md
@@ -75,6 +75,7 @@ The following table lists the configurable parameters of the pravega chart and t
 | `debugLogging` | Enable debug logging | `false` |
 | `serviceAccount.name` | Service account to be used | `pravega-components` |
 | `controller.replicas` | Number of controller replicas | `1` |
+| `controller.maxUnavailableControllerReplicas` | Number of maxUnavailableControllerReplicas possible for controller pdb | `1` |
 | `controller.resources.requests.cpu` | CPU requests for controller | `500m` |
 | `controller.resources.requests.memory` | Memory requests for controller | `1Gi` |
 | `controller.resources.limits.cpu` | CPU limits for controller | `1000m` |
@@ -85,6 +86,7 @@ The following table lists the configurable parameters of the pravega chart and t
 | `controller.service.annotations` | Annotations to add to the controller service, if external access is enabled | `{}` |
 | `controller.jvmOptions` | JVM Options for controller | `["-Xmx2g", "-XX:MaxDirectMemorySize=2g"]` |
 | `segmentStore.replicas` | Number of segmentStore replicas | `1` |
+| `segmentStore.maxUnavailableSegmentStoreReplicas` | Number of maxUnavailableSegmentStoreReplicas possible for segmentstore pdb | `1` |
 | `segmentStore.secret` | Secret configuration for the segmentStore | `{}` |
 | `segmentStore.env` | Name of configmap containing environment variables to be added to the segmentStore | |
 | `segmentStore.resources.requests.cpu` | CPU requests for segmentStore | `1000m` |

diff --git a/charts/pravega/templates/pravega.yaml b/charts/pravega/templates/pravega.yaml
@@ -84,11 +84,13 @@ spec:
       repository: {{ .Values.image.repository }}
       pullPolicy: {{ .Values.image.pullPolicy }}
     controllerReplicas: {{ .Values.controller.replicas }}
+    maxUnavailableControllerReplicas: {{ .Values.controller.maxUnavailableControllerReplicas }}
     {{- if .Values.controller.resources }}
     controllerResources:
 {{ toYaml .Values.controller.resources | indent 6 }}
     {{- end }}
     segmentStoreReplicas: {{ .Values.segmentStore.replicas }}
+    maxUnavailableSegmentStoreReplicas: {{ .Values.segmentStore.maxUnavailableSegmentStoreReplicas }}
     {{- if .Values.segmentStore.resources }}
     segmentStoreResources:
 {{ toYaml .Values.segmentStore.resources | indent 6 }}

diff --git a/charts/pravega/values.yaml b/charts/pravega/values.yaml
@@ -45,6 +45,7 @@ serviceAccount:
 
 controller:
   replicas: 1
+  maxUnavailableControllerReplicas: 1
   resources:
     requests:
       cpu: 500m
@@ -64,6 +65,7 @@ controller:
 
 segmentStore:
   replicas: 1
+  maxUnavailableSegmentStoreReplicas: 1
   secret: {}
     # name:
     # path:

diff --git a/deploy/crds/pravega.pravega.io_pravegaclusters_crd.yaml b/deploy/crds/pravega.pravega.io_pravegaclusters_crd.yaml
@@ -1122,6 +1122,16 @@ spec:
                             type: string
                         type: object
                     type: object
+                  maxUnavailableControllerReplicas:
+                    description: MaxUnavailableControllerReplicas defines the MaxUnavailable
+                      Controller Replicas Default is 1.
+                    format: int32
+                    type: integer
+                  maxUnavailableSegmentStoreReplicas:
+                    description: MaxUnavailableSegmentStoreReplicas defines the MaxUnavailable
+                      SegmentStore Replicas Default is 1.
+                    format: int32
+                    type: integer
                   options:
                     additionalProperties:
                       type: string

diff --git a/pkg/apis/pravega/v1beta1/pravega.go b/pkg/apis/pravega/v1beta1/pravega.go
@@ -81,6 +81,18 @@ type PravegaSpec struct {
 	// +optional
 	SegmentStoreReplicas int32 `json:"segmentStoreReplicas"`
 
+	// MaxUnavailableSegmentStoreReplicas defines the
+	// MaxUnavailable SegmentStore Replicas
+	// Default is 1.
+	// +optional
+	MaxUnavailableSegmentStoreReplicas int32 `json:"maxUnavailableSegmentStoreReplicas"`
+
+	// MaxUnavailableControllerReplicas defines the
+	// MaxUnavailable Controller Replicas
+	// Default is 1.
+	// +optional
+	MaxUnavailableControllerReplicas int32 `json:"maxUnavailableControllerReplicas"`
+
 	// DebugLogging indicates whether or not debug level logging is enabled.
 	// Defaults to false.
 	// +optional
@@ -194,6 +206,16 @@ func (s *PravegaSpec) withDefaults() (changed bool) {
 		s.SegmentStoreReplicas = 1
 	}
 
+	if !config.TestMode && s.MaxUnavailableControllerReplicas < 1 {
+		changed = true
+		s.MaxUnavailableControllerReplicas = 1
+	}
+
+	if !config.TestMode && s.MaxUnavailableSegmentStoreReplicas < 1 {
+		changed = true
+		s.MaxUnavailableSegmentStoreReplicas = 1
+	}
+
 	if s.Image == nil {
 		changed = true
 		s.Image = &ImageSpec{}

diff --git a/pkg/controller/pravega/pravega_controller.go b/pkg/controller/pravega/pravega_controller.go
@@ -296,7 +296,7 @@ func MakeControllerService(p *api.PravegaCluster) *corev1.Service {
 }
 
 func MakeControllerPodDisruptionBudget(p *api.PravegaCluster) *policyv1beta1.PodDisruptionBudget {
-	minAvailable := intstr.FromInt(1)
+	minAvailable := intstr.FromInt(int(p.Spec.Pravega.MaxUnavailableControllerReplicas))
 	return &policyv1beta1.PodDisruptionBudget{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       "PodDisruptionBudget",

diff --git a/pkg/controller/pravega/pravega_segmentstore.go b/pkg/controller/pravega/pravega_segmentstore.go
@@ -536,7 +536,7 @@ func MakeSegmentstorePodDisruptionBudget(p *api.PravegaCluster) *policyv1beta1.P
 	if p.Spec.Pravega.SegmentStoreReplicas == int32(1) {
 		maxUnavailable = intstr.FromInt(0)
 	} else {
-		maxUnavailable = intstr.FromInt(1)
+		maxUnavailable = intstr.FromInt(int(p.Spec.Pravega.MaxUnavailableSegmentStoreReplicas))
 	}
 
 	return &policyv1beta1.PodDisruptionBudget{

diff --git a/pkg/controller/pravegacluster/pravegacluster_controller.go b/pkg/controller/pravegacluster/pravegacluster_controller.go
@@ -20,9 +20,9 @@ import (
 	pravegav1beta1 "github.com/pravega/pravega-operator/pkg/apis/pravega/v1beta1"
 	"github.com/pravega/pravega-operator/pkg/controller/pravega"
 	"github.com/pravega/pravega-operator/pkg/util"
-
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
+	policyv1beta1 "k8s.io/api/policy/v1beta1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
@@ -304,19 +304,45 @@ func (r *ReconcilePravegaCluster) reconcileControllerPdb(p *pravegav1beta1.Prave
 	pdb := pravega.MakeControllerPodDisruptionBudget(p)
 	controllerutil.SetControllerReference(p, pdb, r.scheme)
 	err = r.client.Create(context.TODO(), pdb)
+
 	if err != nil && !errors.IsAlreadyExists(err) {
 		return err
 	}
-	return nil
+
+	currentPdb := &policyv1beta1.PodDisruptionBudget{}
+	err = r.client.Get(context.TODO(), types.NamespacedName{Name: pdb.Name, Namespace: p.Namespace}, currentPdb)
+	if err != nil {
+		return err
+	}
+	return r.updatePdb(currentPdb, pdb)
 }
 
 func (r *ReconcilePravegaCluster) reconcileSegmentStorePdb(p *pravegav1beta1.PravegaCluster) (err error) {
 	pdb := pravega.MakeSegmentstorePodDisruptionBudget(p)
 	controllerutil.SetControllerReference(p, pdb, r.scheme)
 	err = r.client.Create(context.TODO(), pdb)
+
 	if err != nil && !errors.IsAlreadyExists(err) {
 		return err
 	}
+
+	currentPdb := &policyv1beta1.PodDisruptionBudget{}
+	err = r.client.Get(context.TODO(), types.NamespacedName{Name: pdb.Name, Namespace: p.Namespace}, currentPdb)
+	if err != nil {
+		return err
+	}
+	return r.updatePdb(currentPdb, pdb)
+}
+
+func (r *ReconcilePravegaCluster) updatePdb(currentPdb *policyv1beta1.PodDisruptionBudget, newPdb *policyv1beta1.PodDisruptionBudget) (err error) {
+
+	if !reflect.DeepEqual(currentPdb.Spec.MaxUnavailable, newPdb.Spec.MaxUnavailable) {
+		currentPdb.Spec.MaxUnavailable = newPdb.Spec.MaxUnavailable
+		err = r.client.Update(context.TODO(), currentPdb)
+		if err != nil {
+			return fmt.Errorf("failed to update pdb (%s): %v", currentPdb.Name, err)
+		}
+	}
 	return nil
 }
 

diff --git a/pkg/controller/pravegacluster/pravegacluster_controller_test.go b/pkg/controller/pravegacluster/pravegacluster_controller_test.go
@@ -17,6 +17,9 @@ import (
 	"testing"
 
 	"k8s.io/apimachinery/pkg/api/resource"
+	"k8s.io/apimachinery/pkg/util/intstr"
+
+	policyv1beta1 "k8s.io/api/policy/v1beta1"
 
 	"github.com/pravega/pravega-operator/pkg/apis/pravega/v1beta1"
 	"github.com/pravega/pravega-operator/pkg/controller/pravega"
@@ -353,6 +356,43 @@ var _ = Describe("PravegaCluster Controller", func() {
 					})
 				})
 
+				Context("checking updatePDB", func() {
+					var (
+						err1 error
+						str1 string
+					)
+					BeforeEach(func() {
+						res, err = r.Reconcile(req)
+						currentpdb := &policyv1beta1.PodDisruptionBudget{}
+						r.client.Get(context.TODO(), types.NamespacedName{Name: p.PdbNameForSegmentstore(), Namespace: p.Namespace}, currentpdb)
+						maxUnavailable := intstr.FromInt(3)
+						newpdb := &policyv1beta1.PodDisruptionBudget{
+							TypeMeta: metav1.TypeMeta{
+								Kind:       "PodDisruptionBudget",
+								APIVersion: "policy/v1beta1",
+							},
+							ObjectMeta: metav1.ObjectMeta{
+								Name:      "test-name",
+								Namespace: p.Namespace,
+							},
+							Spec: policyv1beta1.PodDisruptionBudgetSpec{
+								MaxUnavailable: &maxUnavailable,
+								Selector: &metav1.LabelSelector{
+									MatchLabels: p.LabelsForController(),
+								},
+							},
+						}
+						err1 = r.updatePdb(currentpdb, newpdb)
+						str1 = fmt.Sprintf("%s", currentpdb.Spec.MaxUnavailable)
+					})
+					It("should not give error", func() {
+						Ω(err1).Should(BeNil())
+					})
+					It("unavailable replicas should change to 3", func() {
+						Ω(str1).To(Equal("3"))
+					})
+				})
+
 				Context("reconcileFinalizers", func() {
 					BeforeEach(func() {
 						p.WithDefaults()

diff --git a/test/e2e/resources/crd.yaml b/test/e2e/resources/crd.yaml
@@ -1124,6 +1124,16 @@ spec:
                             type: string
                         type: object
                     type: object
+                  maxUnavailableControllerReplicas:
+                    description: MaxUnavailableControllerReplicas defines the MaxUnavailable
+                      Controller Replicas Default is 1.
+                    format: int32
+                    type: integer
+                  maxUnavailableSegmentStoreReplicas:
+                    description: MaxUnavailableSegmentStoreReplicas defines the MaxUnavailable
+                      SegmentStore Replicas Default is 1.
+                    format: int32
+                    type: integer
                   options:
                     additionalProperties:
                       type: string