From e5f886d74a495fd29b241a04ff318d3995c920f8 Mon Sep 17 00:00:00 2001 From: Matt Wise <768067+diranged@users.noreply.github.com> Date: Tue, 28 Nov 2023 19:51:14 -0800 Subject: [PATCH] fix(charts/stateful-app): make the readinessprobe required (#247) * fix(charts/stateful-app): make the readinessprobe required * add support --- charts/stateful-app/Chart.yaml | 2 +- charts/stateful-app/README.md | 13 ++++++++++--- charts/stateful-app/README.md.gotmpl | 10 ++++++++-- charts/stateful-app/ci/ci-values.yaml | 5 +++++ charts/stateful-app/templates/statefulset.yaml | 17 +++++++++++------ charts/stateful-app/values.local.yaml | 5 +++++ charts/stateful-app/values.yaml | 16 +++++++--------- 7 files changed, 47 insertions(+), 21 deletions(-) diff --git a/charts/stateful-app/Chart.yaml b/charts/stateful-app/Chart.yaml index b98dccdb..f2bce433 100644 --- a/charts/stateful-app/Chart.yaml +++ b/charts/stateful-app/Chart.yaml @@ -2,7 +2,7 @@ apiVersion: v2 name: stateful-app description: Default StatefulSet Helm Chart type: application -version: 0.14.0 +version: 0.15.0 appVersion: latest maintainers: - name: diranged diff --git a/charts/stateful-app/README.md b/charts/stateful-app/README.md index de9b4f5c..615acb88 100644 --- a/charts/stateful-app/README.md +++ b/charts/stateful-app/README.md @@ -2,7 +2,7 @@ Default StatefulSet Helm Chart -![Version: 0.14.0](https://img.shields.io/badge/Version-0.14.0-informational?style=flat-square) ![Type: application](https://img.shields.io/badge/Type-application-informational?style=flat-square) ![AppVersion: latest](https://img.shields.io/badge/AppVersion-latest-informational?style=flat-square) +![Version: 0.15.0](https://img.shields.io/badge/Version-0.15.0-informational?style=flat-square) ![Type: application](https://img.shields.io/badge/Type-application-informational?style=flat-square) ![AppVersion: latest](https://img.shields.io/badge/AppVersion-latest-informational?style=flat-square) [statefulsets]: https://kubernetes.io/docs/concepts/workloads/controllers/statefulset/ [hpa]: https://kubernetes.io/docs/tasks/run-application/horizontal-pod-autoscale/ @@ -13,6 +13,12 @@ ServiceAccounts, Services, etc. ## Upgrade Notes +### 0.14.x -> 0.15.x + +The `livenessProbe` and `readinessProbe` changes made in +https://github.com/Nextdoor/k8s-charts/pull/212 were invalid. Going forward +`livenessProbe` is optional, but `readinessProbe` is a required field. + ### 0.11.x -> 0.12.x **BREAKING: Istio Alerts have changed** @@ -300,7 +306,7 @@ kmsSecretsRegion: us-west-2 (AWS region where the KMS key is located) | istio.metricsMerging | `bool` | `false` | If set to "True", then the Istio Metrics Merging system will be turned on and Envoy will attempt to scrape metrics from the application pod and merge them with its own. This defaults to False beacuse in most environments we want to explicitly split up the metrics and collect Istio metrics separate from Application metrics. | | istio.preStopCommand | `list ` | `nil` | If supplied, this is the command that will be passed into the `istio-proxy` sidecar container as a pre-stop function. This is used to delay the shutdown of the istio-proxy sidecar in some way or another. Our own default behavior is applied if this value is not set - which is that the sidecar will wait until it does not see the application container listening on any TCP ports, and then it will shut down. eg: preStopCommand: [ /bin/sleep, "30" ] | | kmsSecretsRegion | String | `nil` | AWS region where the KMS key is located | -| livenessProbe | object | `{"httpGet":{"path":"/","port":"http"}}` | A PodSpec container "livenessProbe" configuration object. Note that this livenessProbe will be applied to the proxySidecar container instead if that is enabled. | +| livenessProbe | string | `nil` | A PodSpec container "livenessProbe" configuration object. Note that this livenessProbe will be applied to the proxySidecar container instead if that is enabled. | | monitor.annotations | `map` | `{}` | ServiceMonitor annotations. | | monitor.enabled | `bool` | `true` | If enabled, ServiceMonitor resources for Prometheus Operator are created or if `Values.istio.enabled` is `True`, then the appropriate Pod Annotations will be added for the istio-proxy sidecar container to scrape the metrics. | | monitor.interval | string | `nil` | ServiceMonitor scrape interval | @@ -343,7 +349,7 @@ kmsSecretsRegion: us-west-2 (AWS region where the KMS key is located) | proxySidecar.name | String | `"proxy"` | The name of the proxy sidecar container | | proxySidecar.resources | object | `{}` | A PodSpec "Resources" object for the proxy container | | proxySidecar.volumeMounts | list | `[]` | List of VolumeMounts that are applied to the proxySidecar container - these must refer to volumes set in the `Values.volumes` parameter. | -| readinessProbe | object | `{"httpGet":{"path":"/","port":"http"}}` | A PodSpec container "readinessProbe" configuration object. Note that this readinessProbe will be applied to the proxySidecar container instead if that is enabled. | +| readinessProbe | string | `nil` | A PodSpec container "readinessProbe" configuration object. Note that this readinessProbe will be applied to the proxySidecar container instead if that is enabled. *This is required* | | replicaCount | `int` | `2` | The number of Pods to start up by default. If the `autoscaling.enabled` parameter is set, then this serves as the "start scale" for an application. Setting this to `null` prevents the setting from being applied at all in the PodSpec, leaving it to Kubernetes to use the default value (1). https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#replicas | | resources | object | `{}` | | | revisionHistoryLimit | `int` | `3` | The default revisionHistoryLimit in Kubernetes is 10 - which is just really noisy. Set our default to 3. https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#clean-up-policy | @@ -381,6 +387,7 @@ kmsSecretsRegion: us-west-2 (AWS region where the KMS key is located) | virtualService.port | int | `80` | This is the backing Pod port _number_ to route traffic to. This must match a `containerPort` in the `Values.ports` list. | | virtualService.retries | `map` | `{}` | Pass in an optional [`HTTPRetry`](https://istio.io/latest/docs/reference/config/networking/virtual-service/#HTTPRetry) configuration here to control how services retry their failed requests to the backend service. The default behavior is to retry 2 times if a 503 is returned. | | virtualService.tls | string | `""` | | +| volumeClaimRetentionPolicy | `map` | `nil` | : https://kubernetes.io/docs/concepts/workloads/controllers/statefulset/#persistentvolumeclaim-retention | | volumeClaimTemplates | `PersistentVolumeClaim[]` | `nil` | volumeClaimTemplates is a list of claims that pods are allowed to reference. The StatefulSet controller is responsible for mapping network identities to claims in a way that maintains the identity of a pod. Every claim in this list must have at least one matching (by name) volumeMount in one container in the template. A claim in this list takes precedence over any volumes in the template, with the same name. https://v1-18.docs.kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#persistentvolumeclaim-v1-core | | volumeMounts | list | `[]` | List of VolumeMounts that are applied to the application container - these must refer to volumes set in the `Values.volumes` parameter. | | volumes | list | `[]` | A list of 'volumes' that can be mounted into the Pod. See https://kubernetes.io/docs/concepts/storage/volumes/. | diff --git a/charts/stateful-app/README.md.gotmpl b/charts/stateful-app/README.md.gotmpl index 5a6f2994..507d2113 100644 --- a/charts/stateful-app/README.md.gotmpl +++ b/charts/stateful-app/README.md.gotmpl @@ -12,6 +12,12 @@ ServiceAccounts, Services, etc. ## Upgrade Notes +### 0.14.x -> 0.15.x + +The `livenessProbe` and `readinessProbe` changes made in +https://github.com/Nextdoor/k8s-charts/pull/212 were invalid. Going forward +`livenessProbe` is optional, but `readinessProbe` is a required field. + ### 0.11.x -> 0.12.x **BREAKING: Istio Alerts have changed** @@ -119,7 +125,7 @@ _not_ running within the mesh, then you must set the value to `false`. **BREAKING: ServiceMonitor has been replaced with PodMonitor** -We have replaced the behavior of creating a `ServiceMonitor` resource with a +We have replaced the behavior of creating a `ServiceMonitor` resource with a `PodMonitor` resource. This is done because not all applications will use a `Service` (in fact, the creation of the `Service` resource is optional), and that can cause the monitoring to fail. `PodMonitor` resources will configure @@ -128,7 +134,7 @@ Service. **BREAKING: All .Values.serviceMonitor parameters moved to .Values.monitor** -We have condensed the Values a bit, so the entire `.Values.serviceMonitor` key +We have condensed the Values a bit, so the entire `.Values.serviceMonitor` key has been removed, and all of the parameters have been moved into `.Values.monitor`. Make sure to update your values appropriately! diff --git a/charts/stateful-app/ci/ci-values.yaml b/charts/stateful-app/ci/ci-values.yaml index 7925bfeb..01a4411e 100644 --- a/charts/stateful-app/ci/ci-values.yaml +++ b/charts/stateful-app/ci/ci-values.yaml @@ -8,6 +8,11 @@ ingress: # ALB-ingress controllers. sslRedirect: false +readinessProbe: + httpGet: + path: / + port: http + replicaCount: 2 minReadySeconds: 2 progressDeadlineSeconds: 90 diff --git a/charts/stateful-app/templates/statefulset.yaml b/charts/stateful-app/templates/statefulset.yaml index 41898e6e..16837e3b 100644 --- a/charts/stateful-app/templates/statefulset.yaml +++ b/charts/stateful-app/templates/statefulset.yaml @@ -1,3 +1,8 @@ +{{- /* +Verify that some required inputs are supplied +*/}} +{{- $readinessProbe := required "readinessProbe is required" .Values.readinessProbe }} + apiVersion: apps/v1 kind: StatefulSet metadata: @@ -26,6 +31,10 @@ spec: volumeClaimTemplates: {{- toYaml . | nindent 4 }} {{- end }} + {{- with .Values.volumeClaimRetentionPolicy }} + persistentVolumeClaimRetentionPolicy: + {{- toYaml . | nindent 4 }} + {{- end }} template: metadata: annotations: @@ -138,10 +147,8 @@ spec: livenessProbe: {{- toYaml . | nindent 12 }} {{- end }} - {{- with .Values.readinessProbe }} readinessProbe: - {{- toYaml . | nindent 12 }} - {{- end }} + {{- tpl (toYaml $.Values.readinessProbe) $ | nindent 12 }} resources: {{- toYaml .Values.proxySidecar.resources | nindent 12 }} {{- end }} @@ -223,7 +230,5 @@ spec: livenessProbe: {{- toYaml . | nindent 12 }} {{- end }} - {{- with .Values.readinessProbe }} readinessProbe: - {{- toYaml . | nindent 12 }} - {{- end }} + {{- tpl (toYaml $.Values.readinessProbe) $ | nindent 12 }} diff --git a/charts/stateful-app/values.local.yaml b/charts/stateful-app/values.local.yaml index d7cbf124..fab2e317 100644 --- a/charts/stateful-app/values.local.yaml +++ b/charts/stateful-app/values.local.yaml @@ -8,6 +8,11 @@ ingress: # ALB-ingress controllers. sslRedirect: false +readinessProbe: + httpGet: + path: / + port: http + topologyKey: kubernetes.io/hostname enableTopologySpread: true terminationGracePeriodSeconds: 30 diff --git a/charts/stateful-app/values.yaml b/charts/stateful-app/values.yaml index e8500871..4af77707 100644 --- a/charts/stateful-app/values.yaml +++ b/charts/stateful-app/values.yaml @@ -36,6 +36,10 @@ updateStrategy: null # https://v1-18.docs.kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#persistentvolumeclaim-v1-core volumeClaimTemplates: null +# -- (`map`): +# https://kubernetes.io/docs/concepts/workloads/controllers/statefulset/#persistentvolumeclaim-retention +volumeClaimRetentionPolicy: null + # -- Set up a PodDisruptionBudget for the Deployment. See # https://kubernetes.io/docs/tasks/run-application/configure-pdb/ for more # details. @@ -136,18 +140,12 @@ startupProbe: # -- A PodSpec container "livenessProbe" configuration object. Note that this # livenessProbe will be applied to the proxySidecar container instead if that # is enabled. -livenessProbe: - httpGet: - path: / - port: http +livenessProbe: null # -- A PodSpec container "readinessProbe" configuration object. Note that this # readinessProbe will be applied to the proxySidecar container instead if that -# is enabled. -readinessProbe: - httpGet: - path: / - port: http +# is enabled. *This is required* +readinessProbe: null # -- A list of Port objects that are exposed by the service. These ports are # applied to the main container, or the proxySidecar container (if enabled).