Issue 132: Shorten the service names generated by Bookkeeper Operator (…

…#139) * Provide support to configure headless service name Signed-off-by: anishakj <[email protected]> * Added documentation and e2e tests Signed-off-by: anishakj <[email protected]> * Fixed indentation Signed-off-by: anishakj <[email protected]> * Fixed typo Signed-off-by: anishakj <[email protected]>
pravega · May 11, 2021 · a0f1a00 · a0f1a00
1 parent 5a98c04
commit a0f1a00
Show file tree

Hide file tree

Showing 15 changed files with 82 additions and 19 deletions.
diff --git a/charts/bookkeeper-operator/templates/bookkeeper.pravega.io_bookkeeperclusters_crd.yaml b/charts/bookkeeper-operator/templates/bookkeeper.pravega.io_bookkeeperclusters_crd.yaml
@@ -67,6 +67,10 @@ spec:
                 to provide additional key-value pairs that need to be configured into
                 the bookie pods as environmental variables
               type: string
+            headlessSvcNameSuffix:
+              description: This is used as suffix for bookkeeper headless service
+                name
+              type: string
             image:
               description: Image defines the BookKeeper Docker image to use. By default,
                 "pravega/bookkeeper" will be used.

diff --git a/charts/bookkeeper/README.md b/charts/bookkeeper/README.md
@@ -152,6 +152,7 @@ The following table lists the configurable parameters of the Bookkeeper chart an
 | `autoRecovery`| Enable bookkeeper auto-recovery | `true` |
 | `blockOwnerDeletion`| Enable blockOwnerDeletion | `true` |
 | `affinity` | Specifies scheduling constraints on bookie pods | `{}` |
+| `headlessSvcNameSuffix` | suffix for bookkeeper headless service name | `bookie-headless` |
 | `probes.readiness.initialDelaySeconds` | Number of seconds after the container has started before readiness probe is initiated | `20` |
 | `probes.readiness.periodSeconds` | Number of seconds in which readiness probe will be performed | `10` |
 | `probes.readiness.failureThreshold` | Number of seconds after which the readiness probe times out | `9` |

diff --git a/charts/bookkeeper/templates/bookkeeper.yaml b/charts/bookkeeper/templates/bookkeeper.yaml
@@ -31,6 +31,9 @@ spec:
 {{ toYaml .Values.labels | indent 4 }}
   annotations:
 {{ toYaml .Values.annotations | indent 4 }}
+  {{- if .Values.headlessSvcNameSuffix }}
+  headlessSvcNameSuffix: {{ .Values.headlessSvcNameSuffix }}
+  {{- end }}
   {{- if .Values.probes }}
   probes:
     {{- if .Values.probes.readiness }}

diff --git a/charts/bookkeeper/values.yaml b/charts/bookkeeper/values.yaml
@@ -23,6 +23,7 @@ blockOwnerDeletion: true
 affinity: {}
 labels: {}
 annotations: {}
+headlessSvcNameSuffix:
 probes:
   readiness:
     initialDelaySeconds: 20

diff --git a/deploy/crds/bookkeeper.pravega.io_bookkeeperclusters_crd.yaml b/deploy/crds/bookkeeper.pravega.io_bookkeeperclusters_crd.yaml
@@ -66,6 +66,10 @@ spec:
                 to provide additional key-value pairs that need to be configured into
                 the bookie pods as environmental variables
               type: string
+            headlessSvcNameSuffix:
+              description: This is used as suffix for bookkeeper headless service
+                name
+              type: string
             image:
               description: Image defines the BookKeeper Docker image to use. By default,
                 "pravega/bookkeeper" will be used.
@@ -119,7 +123,7 @@ spec:
               description: MaxUnavailableBookkeeperReplicas defines the MaxUnavailable
                 Bookkeeper Replicas Default is 1.
               format: int32
-              type: integer              
+              type: integer
             options:
               additionalProperties:
                 type: string

diff --git a/doc/configuration.md b/doc/configuration.md
@@ -1,9 +1,10 @@
 ## Configuration
 
-This document explains how to configure Pravega
+This document explains how to configure Bookkeeper
 
 * [RBAC](rbac.md)
 * [Use non-default service accounts](rbac.md#use-non-default-service-accounts)
 * [Installing on a Custom Namespace with RBAC enabled](rbac.md#installing-on-a-custom-namespace-with-rbac-enabled)
 * [Tune Bookkeeper Configuration](bookkeeper-options.md)
 * [Enable admission webhook](webhook.md)
+* [Configuring Service Name](service-configuration.md)
diff --git a/doc/service-configuration.md b/doc/service-configuration.md
@@ -0,0 +1,19 @@
+# Configuring Bookkeeper Headless Service Name
+
+By default bookkeeper headless service name is configured as  `[CLUSTER_NAME]-bookie-headless`.
+
+```
+bookkeeper-bookie-headless              ClusterIP   None             <none>        3181/TCP       4d15h
+```
+But we can configure the headless service name as follows:
+
+```
+helm install bookkeeper pravega/bookkeeper --set headlessSvcNameSuffix="headless"
+```
+
+After installation services can be listed using `kubectl get svc` command.
+
+
+```
+bookkeeper-headless         ClusterIP   None             <none>        3181/TCP       4d15h
+```
diff --git a/pkg/apis/bookkeeper/v1alpha1/bookkeepercluster_types.go b/pkg/apis/bookkeeper/v1alpha1/bookkeepercluster_types.go
@@ -255,6 +255,10 @@ type BookkeeperClusterSpec struct {
 	// Annotations to be added to the bookie pods
 	// +optional
 	Annotations map[string]string `json:"annotations"`
+
+	// This is used as suffix for bookkeeper headless service name
+	// +optional
+	HeadlessSvcNameSuffix string `json:"headlessSvcNameSuffix,omitempty"`
 }
 
 // BookkeeperImageSpec defines the fields needed for a BookKeeper Docker image
@@ -520,6 +524,10 @@ func (s *BookkeeperClusterSpec) withDefaults(bk *BookkeeperCluster) (changed boo
 		changed = true
 		s.Annotations = map[string]string{}
 	}
+	if s.HeadlessSvcNameSuffix == "" {
+		changed = true
+		s.HeadlessSvcNameSuffix = "bookie-headless"
+	}
 
 	return changed
 }
@@ -723,6 +731,10 @@ func (bk *BookkeeperCluster) BookkeeperTargetImage() (string, error) {
 	return fmt.Sprintf("%s:%s", bk.Spec.Image.Repository, bk.Status.TargetVersion), nil
 }
 
+func (bk *BookkeeperCluster) HeadlessServiceNameForBookie() string {
+	return fmt.Sprintf("%s-%s", bk.Name, bk.Spec.HeadlessSvcNameSuffix)
+}
+
 // Wait for pods in cluster to be terminated
 func (bk *BookkeeperCluster) WaitForClusterToTerminate(kubeClient client.Client) (err error) {
 	listOptions := &client.ListOptions{

diff --git a/pkg/apis/bookkeeper/v1alpha1/bookkeepercluster_types_test.go b/pkg/apis/bookkeeper/v1alpha1/bookkeepercluster_types_test.go
@@ -440,4 +440,15 @@ var _ = Describe("BookkeeperCluster Types Spec", func() {
 			os.Remove("filename")
 		})
 	})
+	Context("HeadlessServiceNameForBookie", func() {
+		var str1 string
+		BeforeEach(func() {
+			bk.WithDefaults()
+			str1 = bk.HeadlessServiceNameForBookie()
+		})
+		It("should return headless service name", func() {
+			Ω(str1).To(Equal("default-bookie-headless"))
+		})
+	})
+
 })
diff --git a/pkg/controller/bookkeepercluster/bookie.go b/pkg/controller/bookkeepercluster/bookie.go
@@ -37,7 +37,7 @@ func MakeBookieHeadlessService(bk *v1alpha1.BookkeeperCluster) *corev1.Service {
 			APIVersion: "v1",
 		},
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      util.HeadlessServiceNameForBookie(bk.Name),
+			Name:      bk.HeadlessServiceNameForBookie(),
 			Namespace: bk.Namespace,
 			Labels:    bk.LabelsForBookie(),
 		},
@@ -67,7 +67,7 @@ func MakeBookieStatefulSet(bk *v1alpha1.BookkeeperCluster) *appsv1.StatefulSet {
 			Annotations: bk.AnnotationsForBookie(),
 		},
 		Spec: appsv1.StatefulSetSpec{
-			ServiceName:         util.HeadlessServiceNameForBookie(bk.Name),
+			ServiceName:         bk.HeadlessServiceNameForBookie(),
 			Replicas:            &bk.Spec.Replicas,
 			PodManagementPolicy: appsv1.ParallelPodManagement,
 			UpdateStrategy: appsv1.StatefulSetUpdateStrategy{

diff --git a/pkg/controller/bookkeepercluster/bookie_test.go b/pkg/controller/bookkeepercluster/bookie_test.go
@@ -106,7 +106,7 @@ var _ = Describe("Bookie", func() {
 			Context("Bookkeeper", func() {
 				It("should create a headless service", func() {
 					headlessservice := bookkeepercluster.MakeBookieHeadlessService(bk)
-					Ω(headlessservice.Name).Should(Equal(util.HeadlessServiceNameForBookie(bk.Name)))
+					Ω(headlessservice.Name).Should(Equal(bk.HeadlessServiceNameForBookie()))
 				})
 
 				It("should create a pod disruption budget", func() {
@@ -226,7 +226,7 @@ var _ = Describe("Bookie", func() {
 			Context("Bookkeeper", func() {
 				It("should create a headless service", func() {
 					headlessService := bookkeepercluster.MakeBookieHeadlessService(bk)
-					Ω(headlessService.Name).Should(Equal(util.HeadlessServiceNameForBookie(bk.Name)))
+					Ω(headlessService.Name).Should(Equal(bk.HeadlessServiceNameForBookie()))
 				})
 
 				It("should create a pod disruption budget", func() {

diff --git a/pkg/test/e2e/e2eutil/bkcluster_util.go b/pkg/test/e2e/e2eutil/bkcluster_util.go
@@ -252,6 +252,15 @@ func CheckConfigMap(t *testing.T, f *framework.Framework, ctx *framework.TestCtx
 	return fmt.Errorf("Configmap does not contain the expected value")
 }
 
+func CheckServiceExists(t *testing.T, f *framework.Framework, ctx *framework.TestCtx, b *bkapi.BookkeeperCluster, svcName string) error {
+	svc := &corev1.Service{}
+	err := f.Client.Get(goctx.TODO(), types.NamespacedName{Namespace: b.Namespace, Name: svcName}, svc)
+	if err != nil {
+		return fmt.Errorf("service doesnt exist: %v", err)
+	}
+	return nil
+}
+
 // WaitForBookkeeperClusterToBecomeReady will wait until all Bookkeeper cluster pods are ready
 func WaitForBookkeeperClusterToBecomeReady(t *testing.T, f *framework.Framework, ctx *framework.TestCtx, b *bkapi.BookkeeperCluster) error {
 	t.Logf("waiting for cluster pods to become ready: %s", b.Name)

diff --git a/pkg/util/bookkeepercluster.go b/pkg/util/bookkeepercluster.go
@@ -46,10 +46,6 @@ func StatefulSetNameForBookie(clusterName string) string {
 	return fmt.Sprintf("%s-bookie", clusterName)
 }
 
-func HeadlessServiceNameForBookie(clusterName string) string {
-	return fmt.Sprintf("%s-bookie-headless", clusterName)
-}
-
 func IsOrphan(k8sObjectName string, replicas int32) bool {
 	index := strings.LastIndexAny(k8sObjectName, "-")
 	if index == -1 {

diff --git a/pkg/util/bookkeepercluster_test.go b/pkg/util/bookkeepercluster_test.go
@@ -54,15 +54,7 @@ var _ = Describe("bookkeepercluster", func() {
 			Ω(str1).To(Equal("bk-bookie"))
 		})
 	})
-	Context("HeadlessServiceNameForBookie", func() {
-		var str1 string
-		BeforeEach(func() {
-			str1 = HeadlessServiceNameForBookie("bk")
-		})
-		It("should return headless service name", func() {
-			Ω(str1).To(Equal("bk-bookie-headless"))
-		})
-	})
+
 	Context("IsOrphan", func() {
 		var result1, result2, result3, result4 bool
 		BeforeEach(func() {

diff --git a/test/e2e/basic_test.go b/test/e2e/basic_test.go
@@ -11,6 +11,7 @@
 package e2e
 
 import (
+	"fmt"
 	"testing"
 
 	. "github.com/onsi/gomega"
@@ -35,13 +36,18 @@ func testCreateRecreateCluster(t *testing.T) {
 
 	defaultCluster := bookkeeper_e2eutil.NewDefaultCluster(namespace)
 	defaultCluster.WithDefaults()
+	defaultCluster.Spec.HeadlessSvcNameSuffix = "headlesssvc"
 
 	bookkeeper, err := bookkeeper_e2eutil.CreateBKCluster(t, f, ctx, defaultCluster)
 	g.Expect(err).NotTo(HaveOccurred())
 
 	err = bookkeeper_e2eutil.WaitForBookkeeperClusterToBecomeReady(t, f, ctx, bookkeeper)
 	g.Expect(err).NotTo(HaveOccurred())
 
+	svcName := fmt.Sprintf("%s-headlesssvc", bookkeeper.Name)
+	err = bookkeeper_e2eutil.CheckServiceExists(t, f, ctx, bookkeeper, svcName)
+	g.Expect(err).NotTo(HaveOccurred())
+
 	err = bookkeeper_e2eutil.DeleteBKCluster(t, f, ctx, bookkeeper)
 	g.Expect(err).NotTo(HaveOccurred())
 
@@ -57,6 +63,10 @@ func testCreateRecreateCluster(t *testing.T) {
 	err = bookkeeper_e2eutil.WaitForBookkeeperClusterToBecomeReady(t, f, ctx, bookkeeper)
 	g.Expect(err).NotTo(HaveOccurred())
 
+	svcName = fmt.Sprintf("%s-bookie-headless", bookkeeper.Name)
+	err = bookkeeper_e2eutil.CheckServiceExists(t, f, ctx, bookkeeper, svcName)
+	g.Expect(err).NotTo(HaveOccurred())
+
 	err = bookkeeper_e2eutil.DeleteBKCluster(t, f, ctx, bookkeeper)
 	g.Expect(err).NotTo(HaveOccurred())