Resolve: PR reviews

Fix: e2e test cases

Fix: e2e test cases

Author: NishanthNalluri

Makefile

@@ -112,7 +112,7 @@ test/cluster/down: bin/k3d
 
 test/e2e/%: PKG=$*
 test/e2e/%: bin/cockroach bin/kubectl bin/helm build/self-signer test/cluster ## run e2e tests for package (e.g. install or rotate)
-	@PATH="$(PWD)/bin:${PATH}" go test -timeout 30m -v ./tests/e2e/... || EXIT_CODE=$$?; \
+	@PATH="$(PWD)/bin:${PATH}" go test -timeout 30m -v ./tests/e2e/${PKG}/... || EXIT_CODE=$$?; \
 	$(MAKE) test/cluster/down; \
 	exit $${EXIT_CODE:-0}
 

build/templates/values.yaml

@@ -712,11 +712,11 @@ iap:
   # clientSecret:
 
 # Set the GODEBUG env var for the Go runtime. By default, disable Transparent Huge Pages,
-# which cause memory inefficiency for CRDB.
+# which cause memory inefficiency for CockroachDB.
 godebug:
   disablethp: "1"
 
-# Use the CRDB Operator to manage the CRDB clusters
+# Use the CockroachDB Operator to manage the CockroachDB clusters.
 operator:
   enabled: true
   # Default values for the cluster chart.
@@ -729,11 +729,11 @@ operator:
   nameOverride: ""
   fullnameOverride: ""
 
-  # A map of CRDB cluster settings.
+  # A map of CockroachDB cluster settings.
   # See https://www.cockroachlabs.com/docs/stable/cluster-settings.html
   clusterSettings: ~
 
-    # Regions controls the number of CRDB nodes that are deployed per region.
+    # Regions controls the number of CockroachDB nodes that are deployed per region.
     # regions: ~
     # - code: us-central1
       # nodes: 3
@@ -758,7 +758,7 @@ operator:
       # cpu: 100m
       # memory: 128Mi
 
-  # dataStore specifies the disk configuration for the CRDB Node.
+  # dataStore specifies the disk configuration for the CockroachDB Node.
   dataStore:
     volumeClaimTemplate:
       metadata: {}
@@ -779,7 +779,7 @@ operator:
     # the number of days generated certs are valid for
     # validForDays: 3650
 
-    # External certificates for the CRDB cluster.
+    # External certificates for the CockroachDB cluster.
     externalCertificates:
       clientCaConfigMapName: ""
       nodeCaConfigMapName: ""
@@ -788,7 +788,7 @@ operator:
       nodeSecretName: ""
       rootSqlClientSecretName: ""
 
-  # RBAC settings for CRDB nodes
+  # RBAC settings for CockroachDB nodes
   rbac:
     # By default the service account will be the resource name. It will
     # be created during the installation along with a namespaced role and
@@ -822,29 +822,56 @@ operator:
       cloudProvider: k3d
       namespace: default
 
-  # PodLabels are the labels that should be applied to the underlying CRDB pod
+  # PodLabels are the labels that should be applied to the underlying CockroachDB pod
   podLabels:
     app.kubernetes.io/component: cockroachdb
 
-  # Flags passed to the cockroachdb container.
+  # Flags passed to the CockroachDB container.
   flags:
     # Disable backup/restore to local disk by default.
     --external-io-dir: disabled
 
-  # Environment variables set on cockroachdb pods.
+  # Environment variables set on CockroachDB pods.
   env: []
 
-  # Delay between cockroachdb pod restarts. Wait 3m by default to avoid
+  # Delay between CockroachDB pod restarts. Wait 3m by default to avoid
   # unavailability during restarts.
   rollingRestartDelay: 3m0s
 
-  # Topology spread constraints set on cockroachdb pods. Spread cockroachdb
+  # Topology spread constraints set on CockroachDB pods. Spread CockroachDB
   # pods across zones by default.
   topologySpreadConstraints:
     - maxSkew: 1
       topologyKey: topology.kubernetes.io/zone
       whenUnsatisfiable: DoNotSchedule
 
+  # Service Ports for configuring non-standard ports
+  # for grpc, sql, http services.
+  service:
+    ports:
+      # You can set a different external and internal gRPC ports and their name.
+      grpc:
+        external:
+          port: 26258
+          name: grpc
+        # If the port number is different than `external.port`, then it will be
+        # named as `internal.name` in Service.
+        internal:
+          # CockroachDB's port to listen to inter-communications and client connections.
+          port: 26257
+          # If using Istio set it to `cockroach`.
+          name: grpc-internal
+      http:
+        # CockroachDB's port to listen to HTTP requests.
+        port: 8080
+        name: http
+
+    # This Service is meant to be used by clients of the database.
+    # It exposes a ClusterIP that will automatically load balance connections
+    # to the different database Pods.
+    public:
+      type: ClusterIP
+
   extras:
     # Add a container with dnsutils (nslookup, dig, ping, etc.) installed.
     dnsutils: false

cockroachdb/templates/_helpers.tpl

@@ -362,7 +362,6 @@ Construct the GODEBUG env var value (looks like: GODEBUG="foo=bar,baz=quux"; def
 {{- join "," $godebugList -}}
 {{- end }}
 
-
 {{/* Common labels that are applied to all managed objects. */}}
 {{- define "cluster.labels" -}}
 helm.sh/chart: {{ include "cockroachdb.chart" . }}
@@ -375,7 +374,7 @@ app.kubernetes.io/managed-by: {{ .Release.Service }}
 
 {{/*
   Selector labels defines the set of labels that can be used as selectors for
-  crdb nodes.
+  CockroachDB nodes.
 */}}
 {{- define "cluster.selectorLabels" -}}
 app.kubernetes.io/name: {{ include "cockroachdb.clusterfullname" . }}

cockroachdb/templates/crdb.yaml

@@ -61,8 +61,14 @@ spec:
       {{- if .Values.operator.loggingConf }}
       loggingConfigMapName: {{ .Release.Name }}-logging
       {{- end }}
-  # All properties below are solely to pass validation. They aren't used by the
-  # betaclusterctrl controller so the values don't matter so long as they're
-  # valid.
+      {{- if .Values.operator.service.ports.grpc.external.port }}
+      gRPCPort: {{ .Values.operator.service.ports.grpc.external.port }}
+      {{- end }}
+      {{- if .Values.operator.service.ports.grpc.internal.port }}
+      sQLPort: {{ .Values.operator.service.ports.grpc.internal.port }}
+      {{- end }}
+      {{- if .Values.operator.service.ports.http.port }}
+      hTTPPort: {{ .Values.operator.service.ports.http.port }}
+      {{- end }}
   dataStore: {}
 {{- end }}

cockroachdb/templates/job-certSelfSigner.yaml

@@ -9,7 +9,7 @@ metadata:
     # job is considered part of the release.
     "helm.sh/hook": pre-install,pre-upgrade
     "helm.sh/hook-weight": "4"
-    "helm.sh/hook-delete-policy": hook-succeeded,hook-failed
+    "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded,hook-failed
   labels:
     helm.sh/chart: {{ template "cockroachdb.chart" . }}
     app.kubernetes.io/name: {{ template "cockroachdb.name" . }}

cockroachdb/templates/job-cleaner.yaml

@@ -8,7 +8,7 @@ metadata:
     # This is what defines this resource as a hook. Without this line, the
     # job is considered part of the release.
     "helm.sh/hook": pre-delete
-    "helm.sh/hook-delete-policy": hook-succeeded,hook-failed
+    "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded,hook-failed
   labels:
     helm.sh/chart: {{ template "cockroachdb.chart" . }}
     app.kubernetes.io/name: {{ template "cockroachdb.name" . }}

cockroachdb/templates/role-certSelfSigner.yaml

@@ -9,7 +9,7 @@ metadata:
     # job is considered part of the release.
     "helm.sh/hook": pre-install,pre-upgrade
     "helm.sh/hook-weight": "2"
-    "helm.sh/hook-delete-policy": hook-succeeded,hook-failed
+    "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded,hook-failed
   labels:
     helm.sh/chart: {{ template "cockroachdb.chart" . }}
     app.kubernetes.io/name: {{ template "cockroachdb.name" . }}

cockroachdb/templates/rolebinding-certSelfSigner.yaml

@@ -9,7 +9,7 @@ metadata:
     # job is considered part of the release.
     "helm.sh/hook": pre-install,pre-upgrade
     "helm.sh/hook-weight": "3"
-    "helm.sh/hook-delete-policy": hook-succeeded,hook-failed
+    "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded,hook-failed
   labels:
     helm.sh/chart: {{ template "cockroachdb.chart" . }}
     app.kubernetes.io/name: {{ template "cockroachdb.name" . }}

cockroachdb/templates/service.public.yaml

@@ -39,20 +39,23 @@ spec:
   {{- end }}
   # Cockroach Cloud operator doesn't provide a way to change the sql,grpc and http ports for the pods. Hence, making these
   # ports static here.
-  type: ClusterIP
+  type: {{ .Values.operator.service.public.type | quote }}
   ports:
-  - name: sql
-    port: 26257
-    protocol: TCP
-    targetPort: 26257
-  - name: grpc
-    port: 26258
-    protocol: TCP
-    targetPort: 26258
-  - name: http
-    port: 8080
-    protocol: TCP
-    targetPort: 8080
+  {{- $ports := .Values.operator.service.ports }}
+    # The main port, served by gRPC, serves Postgres-flavor SQL, inter-node
+    # traffic and the CLI.
+    - name: {{ $ports.grpc.external.name | quote }}
+      port: {{ $ports.grpc.external.port | int64 }}
+      targetPort: grpc
+  {{- if ne ($ports.grpc.internal.port | int64) ($ports.grpc.external.port | int64) }}
+    - name: {{ $ports.grpc.internal.name | quote }}
+      port: {{ $ports.grpc.internal.port | int64 }}
+      targetPort: grpc
+  {{- end }}
+    # The secondary port serves the UI as well as health and debug endpoints.
+    - name: {{ $ports.http.name | quote }}
+      port: {{ $ports.http.port | int64 }}
+      targetPort: http
 {{- else }}
   {{- with .Values.statefulset.labels }}
     {{- toYaml . | nindent 4 }}

cockroachdb/templates/serviceaccount-certSelfSigner.yaml

@@ -10,7 +10,7 @@ metadata:
     # job is considered part of the release.
     "helm.sh/hook": pre-install,pre-upgrade
     "helm.sh/hook-weight": "1"
-    "helm.sh/hook-delete-policy": hook-succeeded,hook-failed
+    "helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded,hook-failed
     {{- with .Values.tls.certs.selfSigner.svcAccountAnnotations }}
       {{- toYaml . | nindent 4 }}
     {{- end }}