Skip to content

Commit 00ed464

Browse files
committed
fix(helm): address chart review feedback
1 parent 6ddce9a commit 00ed464

9 files changed

Lines changed: 56 additions & 15 deletions

File tree

.github/workflows/helm-chart-ci.yml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ jobs:
3232
runs-on: ubuntu-latest
3333
steps:
3434
- uses: actions/checkout@v4
35+
with:
36+
persist-credentials: false
3537

3638
- uses: azure/setup-helm@v4
3739
with:
@@ -55,6 +57,12 @@ jobs:
5557
! helm template ci helm/hugegraph --set pd.replicas=100 2>/dev/null
5658
! helm template ci helm/hugegraph --set pd.pdb.minAvailable=3 2>/dev/null
5759
! helm template ci helm/hugegraph --set server.hpa.enabled=true 2>/dev/null
60+
! helm template ci helm/hugegraph \
61+
--set server.hpa.enabled=true \
62+
--set server.hpa.minReplicas=2 \
63+
--set server.resources.requests.cpu=100m \
64+
--set server.pdb.enabled=true \
65+
--set server.pdb.minAvailable=2 2>/dev/null
5866
! helm template ci helm/hugegraph --set server.auth.enabled=true 2>/dev/null
5967
6068
- name: kubeconform

helm/hugegraph/README.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ default values.
220220
| `server.podSecurityContext` | Pod-level securityContext, rendered only when set | `{}` |
221221
| `server.securityContext` | Container-level securityContext. Hardened by default; `runAsNonRoot` is not set because the published images run as root | `allowPrivilegeEscalation: false`, `capabilities.drop: [ALL]`, `seccompProfile: RuntimeDefault` |
222222
| `server.pdb.enabled` | Create a PodDisruptionBudget for Server. Off by default: Server holds no quorum | `false` |
223-
| `server.pdb.minAvailable` | Must be strictly less than `server.replicas` | `2` |
223+
| `server.pdb.minAvailable` | Must be less than `server.hpa.minReplicas` when HPA is enabled, otherwise less than `server.replicas` | `2` |
224224
| `server.antiAffinity` | One of `required`, `preferred`, `disabled`. Defaults to `preferred` rather than `required` because HPA may scale Server past the node count; set `required` when replicas always stay below it | `preferred` |
225225
| `server.nodeSelector` | Node selector for server Pods | `{}` |
226226
| `server.tolerations` | Tolerations for server Pods | `[]` |
@@ -236,6 +236,7 @@ default values.
236236
| `server.serviceAccount.annotations` | Annotations on the created ServiceAccount | `{}` |
237237
| `server.serviceAccount.automountServiceAccountToken` | Mount an API token. The chart makes no API calls | `false` |
238238
| `server.waitImage` | Image for the Helm test hook | `curlimages/curl:8.5.0` |
239+
| `server.testResources` | Resources for the Helm test hook container | `{}` |
239240
| `server.restServer.minFreeMemory` | Empty preserves the image default | `""` |
240241
| `server.restServer.batchMaxWriteThreads` | Empty preserves the image default | `""` |
241242
| `server.initStoreEnabled` | Must remain `false` for distributed HStore | `false` |
@@ -371,7 +372,7 @@ kubectl describe pod <pod> | grep -A5 "Last State"
371372
Helm itself rejects release names longer than 53 characters, before this chart
372373
renders anything:
373374

374-
```
375+
```text
375376
invalid release name ... the length must not be longer than 53
376377
```
377378

helm/hugegraph/templates/NOTES.txt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,12 @@ Verify the release:
3434
Reach the Server API:
3535

3636
kubectl port-forward -n {{ .Release.Namespace }} svc/{{ include "hugegraph.server.name" . }} {{ .Values.server.port }}:{{ .Values.server.port }}
37+
{{- if .Values.server.auth.enabled }}
38+
PASSWORD="$(kubectl get secret -n {{ .Release.Namespace }} {{ .Values.server.auth.existingSecret }} -o jsonpath='{.data.password}' | base64 --decode)"
39+
curl --user "admin:${PASSWORD}" http://127.0.0.1:{{ .Values.server.port }}/versions
40+
{{- else }}
3741
curl http://127.0.0.1:{{ .Values.server.port }}/versions
42+
{{- end }}
3843
{{- if not .Values.server.auth.enabled }}
3944

4045
Authentication is disabled. Do not expose this release to untrusted networks.

helm/hugegraph/templates/_helpers.tpl

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,17 @@ default
208208
{{- end -}}
209209
{{- end }}
210210

211+
{{/*
212+
The minimum Server replica count that a PDB must remain valid against.
213+
*/}}
214+
{{- define "hugegraph.server.replicaFloor" -}}
215+
{{- if .Values.server.hpa.enabled -}}
216+
{{- .Values.server.hpa.minReplicas -}}
217+
{{- else -}}
218+
{{- .Values.server.replicas -}}
219+
{{- end -}}
220+
{{- end }}
221+
211222
{{/*
212223
Cross-field validation that JSON Schema draft-07 cannot express.
213224
*/}}
@@ -246,8 +257,9 @@ and must not be failed for a value that has no effect.
246257
{{- fail "server.service.nodePort requires server.service.type to be NodePort or LoadBalancer" -}}
247258
{{- end -}}
248259
{{- $serverPdb := get .Values.server "pdb" | default dict -}}
249-
{{- if and (get $serverPdb "enabled" | default false) (gt (int .Values.server.replicas) 1) (ge (int (get $serverPdb "minAvailable" | default 1)) (int .Values.server.replicas)) -}}
250-
{{- fail "server.pdb.minAvailable must be less than server.replicas, otherwise the PDB permanently blocks voluntary disruptions such as node drains" -}}
260+
{{- $serverReplicaFloor := include "hugegraph.server.replicaFloor" . | int -}}
261+
{{- if and (get $serverPdb "enabled" | default false) (gt $serverReplicaFloor 1) (ge (int (get $serverPdb "minAvailable" | default 1)) $serverReplicaFloor) -}}
262+
{{- fail "server.pdb.minAvailable must be less than the active Server replica floor (server.hpa.minReplicas when HPA is enabled, otherwise server.replicas), otherwise the PDB permanently blocks voluntary disruptions such as node drains" -}}
251263
{{- end -}}
252264
{{- end }}
253265

helm/hugegraph/templates/server-ingress.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ spec:
4141
http:
4242
paths:
4343
{{- range .paths }}
44-
- path: {{ .path }}
44+
- path: {{ .path | quote }}
4545
pathType: {{ .pathType }}
4646
backend:
4747
service:

helm/hugegraph/templates/server-pdb.yaml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@
1616
#
1717

1818
{{- $pdb := get .Values.server "pdb" | default dict }}
19-
{{- if and (get $pdb "enabled" | default false) (gt (int .Values.server.replicas) 1) }}
19+
{{- $replicaFloor := include "hugegraph.server.replicaFloor" . | int }}
20+
{{- if and (get $pdb "enabled" | default false) (gt $replicaFloor 1) }}
2021
apiVersion: policy/v1
2122
kind: PodDisruptionBudget
2223
metadata:

helm/hugegraph/templates/tests/test-connection.yaml

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,26 @@ metadata:
2828
"helm.sh/hook": test
2929
"helm.sh/hook-delete-policy": before-hook-creation,hook-succeeded
3030
spec:
31+
automountServiceAccountToken: false
3132
restartPolicy: Never
33+
securityContext:
34+
seccompProfile:
35+
type: RuntimeDefault
3236
containers:
3337
- name: curl
3438
image: {{ .Values.server.waitImage | quote }}
39+
securityContext:
40+
allowPrivilegeEscalation: false
41+
capabilities:
42+
drop: ["ALL"]
43+
readOnlyRootFilesystem: true
44+
runAsGroup: 101
45+
runAsNonRoot: true
46+
runAsUser: 100
47+
{{- with .Values.server.testResources }}
48+
resources:
49+
{{- toYaml . | nindent 8 }}
50+
{{- end }}
3551
{{- if .Values.server.auth.enabled }}
3652
env:
3753
- name: PASSWORD

helm/hugegraph/values.schema.json

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,6 @@
3131
},
3232
"global": {
3333
"type": "object"
34-
},
35-
"networkPolicy": {
36-
"type": "object",
37-
"properties": {
38-
"enabled": {
39-
"type": "boolean"
40-
}
41-
}
4234
}
4335
},
4436
"definitions": {
@@ -576,6 +568,7 @@
576568
},
577569
"ingress": {
578570
"type": "object",
571+
"additionalProperties": false,
579572
"required": [
580573
"enabled",
581574
"className",
@@ -734,6 +727,9 @@
734727
}
735728
}
736729
},
730+
"testResources": {
731+
"type": "object"
732+
},
737733
"service": {
738734
"type": "object",
739735
"additionalProperties": false,

helm/hugegraph/values.yaml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ server:
185185
podLabels: {}
186186
# Extra environment variables appended to the server container.
187187
extraEnv: []
188-
# Gives a JVM with an on-disk store time to shut down cleanly on drain.
188+
# Allows in-flight requests to drain before the Server is stopped.
189189
terminationGracePeriodSeconds: 60
190190
serviceAccount:
191191
create: true
@@ -206,6 +206,8 @@ server:
206206
minAvailable: 2
207207
# Image used by the Helm test hook.
208208
waitImage: curlimages/curl:8.5.0
209+
# Optional resources for the Helm test hook container.
210+
testResources: {}
209211
restServer:
210212
# Empty preserves the image's restserver.min_free_memory default.
211213
minFreeMemory: ""

0 commit comments

Comments
 (0)