Add helm starter chart for developing helm chart #967
Conversation
lianhao
force-pushed
the
helm-chart-starter
branch
from
a8ed171 to
bc37842
Compare
|
Let's hold this until we've got the helm charts directory layout figured out |
Helm starter chart pack can be used to create helm chart for new services. Signed-off-by: Lianhao Lu <[email protected]>
lianhao
force-pushed
the
helm-chart-starter
branch
from
bc37842 to
fe34c8c
Compare
There was a problem hiding this comment.
I think some instructions on using the template are also needed.
It could e.g. state which classes service needs to use, for it to have v1/health_check endpoint for the probes, and that services always need to handle SIGTERM, to avoid lost requests on service scale down.
| spec: | ||
| containers: | ||
| - name: curl | ||
| image: python:3.10.14 |
There was a problem hiding this comment.
Latest Python 3.10 bug/security-fix version is 3.10.16 (and current latest Python version is 3.13.1).
| # We usually recommend not to specify default resources and to leave this as a conscious | ||
| # choice for the user. This also increases chances charts run on environments with little | ||
| # resources, such as Minikube. If you do want to specify resources, uncomment the following | ||
| # lines, adjust them as necessary, and remove the curly braces after 'resources:'. |
There was a problem hiding this comment.
Update the comment to something more relevant.
| livenessProbe: | ||
| httpGet: | ||
| path: v1/health_check | ||
| port: <CHARTNAME> | ||
| initialDelaySeconds: 5 | ||
| periodSeconds: 5 | ||
| failureThreshold: 24 | ||
| readinessProbe: | ||
| httpGet: | ||
| path: v1/health_check | ||
| port: <CHARTNAME> | ||
| initialDelaySeconds: 5 | ||
| periodSeconds: 5 | ||
| startupProbe: | ||
| httpGet: | ||
| path: v1/health_check | ||
| port: <CHARTNAME> | ||
| initialDelaySeconds: 5 | ||
| periodSeconds: 5 | ||
| failureThreshold: 120 |
There was a problem hiding this comment.
IMHO better to comment out liveness probe. That's useful only if pod has habit of deadlocking and restarting the pod will fix that issue (instead of restart just making the service response worse).
| # cpu: 100m | ||
| # memory: 128Mi | ||
|
|
||
| # This is to setup the liveness and readiness probes more information can be found here: https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-startup-probes/ |
There was a problem hiding this comment.
| # This is to setup the liveness and readiness probes more information can be found here: https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-startup-probes/ | |
| # This is to setup the startup, liveness and readiness probes more information can be found here: https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-startup-probes/ |
Or:
| # This is to setup the liveness and readiness probes more information can be found here: https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-startup-probes/ | |
| # Set up pod health monitoring probes, see: https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-startup-probes/ |
| # This section is for setting up autoscaling more information can be found here: https://kubernetes.io/docs/concepts/workloads/autoscaling/ | ||
| # Enabling HPA will: | ||
| # - Ignore above replicaCount, as it will be controlled by HPA | ||
| # - Add example HPA scaling rules with thresholds suitable for Xeon deployments | ||
| # - Require custom metrics ConfigMap available in the main application chart |
There was a problem hiding this comment.
Fix comment (similarly to the other PR).
Because HPA can sometimes change replica counts up and down rather frequently, dev documentation (or this comment) should emphatize that service needs to handle SIGTERM and:
- stop accepting new requests
- handle all of its buffered requests
- terminate after tthose have been processed
To avoid k8s SIGKILL (sent to pod after specified grace period) causing request failures.
See: https://kubernetes.io/docs/concepts/workloads/pods/pod-lifecycle/#pod-termination
| # If set, it will overwrite serviceAccount.name. | ||
| # If set, and serviceAccount.create is false, it will assume this service account is already created by others. | ||
| sharedSAName: "" | ||
| # Install Prometheus serviceMonitor for service |
There was a problem hiding this comment.
| # Install Prometheus serviceMonitor for service | |
| # Install Prometheus serviceMonitor for service metrics |
| - type: Object | ||
| object: | ||
| describedObject: | ||
| apiVersion: v1 | ||
| # get metric for named object of given type (in same namespace) | ||
| kind: Service | ||
| name: {{ include "<CHARTNAME>.fullname" . }} | ||
| target: | ||
| type: AverageValue | ||
| averageValue: 15 | ||
| metric: | ||
| name: {{ include "<CHARTNAME>.metricPrefix" . }}_dummy_queue_size |
There was a problem hiding this comment.
Remove this part as this template does not set up any custom metrics for this application, and because there would be no point in doing that (custom metrics are by definition application specific).
| {{/* | ||
| Convert chart name to a string suitable as metric prefix | ||
| */}} | ||
| {{- define "<CHARTNAME>.metricPrefix" -}} | ||
| {{- include "<CHARTNAME>.fullname" . | replace "-" "_" | regexFind "[a-zA-Z_:][a-zA-Z0-9_:]*" }} |
There was a problem hiding this comment.
Remove this part as this template does not set up any custom metrics for this application.
| ## Values | ||
|
|
||
| | Key | Type | Default | Description | | ||
| | ------------------------------- | ------ | ------------------------------------ | ---------------------- | | ||
| | global.HUGGINGFACEHUB_API_TOKEN | string | `insert-your-huggingface-token-here` | Hugging Face API token | |
There was a problem hiding this comment.
I think this needs a link to a shared documentation explaining all the common keys in the values file (proxies etc). Such doc could be split to sections based on how commonly given keys need to be specified by the user.
| # targetCPUUtilizationPercentage: 80 | ||
| # targetMemoryUtilizationPercentage: 80 |
There was a problem hiding this comment.
Helm install fails if there are not targets. Because these do nothing unless autoscaling is enabled, I think they can be always enabled:
| # targetCPUUtilizationPercentage: 80 | |
| # targetMemoryUtilizationPercentage: 80 | |
| targetCPUPercentage: 80 | |
| targetMemoryPercentage: 80 |
Description
Helm starter chart pack can be used to create helm chart for new services.
Issues
n/a.Type of change
List the type of change like below. Please delete options that are not relevant.
Dependencies
List the newly introduced 3rd party dependency if exists.
Tests
Describe the tests that you ran to verify your changes.