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,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

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 <str>` | `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/. |

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!
 

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

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 }}

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

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).