dnskr commented on code in PR #5062:
URL: https://github.com/apache/kyuubi/pull/5062#discussion_r1268653838
##########
charts/kyuubi/values.yaml:
##########
@@ -22,6 +22,26 @@
# Kyuubi server numbers
replicaCount: 2
+# controls how Kyuubi server pods are created during initial scale up,
+# when replacing pods on nodes, or when scaling down.
+# The default policy is `OrderedReady`, alternative policy is `Parallel`.
+podManagementPolicy: OrderedReady
Review Comment:
`podManagementPolicy` declared, but not used in the templates
##########
charts/kyuubi/templates/kyuubi-statefulset.yaml:
##########
@@ -16,16 +16,22 @@
*/}}
apiVersion: apps/v1
-kind: Deployment
+kind: StatefulSet
metadata:
name: {{ .Release.Name }}
labels:
{{- include "kyuubi.labels" . | nindent 4 }}
spec:
- replicas: {{ .Values.replicaCount }}
selector:
matchLabels:
{{- include "kyuubi.selectorLabels" . | nindent 6 }}
+ serviceName: {{ $.Release.Name }}-headless
Review Comment:
Global variable `$` is not needed here, i.e. the following should be enough:
```
serviceName: {{ .Release.Name }}-headless
```
##########
charts/kyuubi/templates/kyuubi-service.yaml:
##########
@@ -15,6 +15,24 @@
limitations under the License.
*/}}
+apiVersion: v1
+kind: Service
+metadata:
+ name: {{ $.Release.Name }}-headless
+ labels:
+ {{- include "kyuubi.labels" $ | nindent 4 }}
+spec:
+ type: ClusterIP
+ clusterIP: None
+ ports:
+ {{- range $name, $frontend := .Values.server }}
+ - name: {{ $name | kebabcase }}
+ port: {{ tpl $frontend.service.port $ }}
+ targetPort: {{ $frontend.port }}
+ {{- end }}
+ selector:
+ {{- include "kyuubi.selectorLabels" $ | nindent 4 }}
+---
Review Comment:
Please move headless Service definition to separate file according to Helm
[Best Practices
Guide](https://helm.sh/docs/chart_best_practices/templates/#structure-of-templates)
##########
charts/kyuubi/templates/kyuubi-service.yaml:
##########
@@ -15,6 +15,24 @@
limitations under the License.
*/}}
+apiVersion: v1
+kind: Service
+metadata:
+ name: {{ $.Release.Name }}-headless
Review Comment:
Global variable `$` is not needed here, i.e. the following should be enough:
```
name: {{ .Release.Name }}-headless
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
