[simple-app] Fix the ServiceMonitor and improve the Ports configurati…

…on (#103)

I am working on using this chart to implement [YACE](https://github.com/nerdswords/yet-another-cloudwatch-exporter) - which is a service collects metrics from CloudWatch and exports so a ServiceMonitor can scrape them into Prometheus. I discovered a few bugs though with the way our `Service`, `ServiceMonitor` and `Deployment` resources were handling port configuration.

charts/simple-app/Chart.yaml

@@ -2,7 +2,7 @@ apiVersion: v2
 name: simple-app
 description: Default Microservice Helm Chart
 type: application
-version: 0.16.2
+version: 0.17.0
 appVersion: latest
 maintainers:
   - name: diranged

charts/simple-app/README.md

@@ -2,7 +2,7 @@
 
 Default Microservice Helm Chart
 
-![Version: 0.16.2](https://img.shields.io/badge/Version-0.16.2-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.17.0](https://img.shields.io/badge/Version-0.17.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)
 
 [deployments]: https://kubernetes.io/docs/concepts/workloads/controllers/deployment/
 [hpa]: https://kubernetes.io/docs/tasks/run-application/horizontal-pod-autoscale/
@@ -11,6 +11,23 @@ This chart provides a default deployment for a simple application that operates
 in a [Deployment][deployments]. The chart automatically configures various
 defaults for you like the Kubernetes [Horizontal Pod Autoscaler][hpa].
 
+## Upgrade Notes
+
+### 0.16.x -> 0.17.x
+
+**New Feature: Customize User-Facing Ports**
+
+You can now expose a custom port for your users (eg: `80`) while your service
+continues to listen on a private containerPort (eg: `5000`). In the maps in
+`.Values.ports` simply add a `port: <int>` key and the `Service` resource
+will be reconfigured to route that port to the backend container port.
+
+**Bug Fix: ServiceMonitor resources were broken**
+
+Previously the `ServiceMonitor` resources were pointing to the `Service` but
+the `Service` did not expose a `metrics` endpoint, which caused the resource to
+be invalid. This has been fixed.
+
 ## Monitoring
 
 This chart makes the assumption that you _do_ have a Prometheus-style
@@ -116,7 +133,7 @@ This feature is turned on by default if you set `Values.istio.enabled=true` and
 | podDisruptionBudget | object | `{}` | Set up a PodDisruptionBudget for the Deployment. See https://kubernetes.io/docs/tasks/run-application/configure-pdb/ for more details. |
 | podLabels | object | `{}` | (`Map`) List of Labels to be added to the PodSpec |
 | podSecurityContext | object | `{}` |  |
-| ports | list | `[{"containerPort":80,"name":"http","protocol":"TCP"},{"containerPort":443,"name":"https","protocol":"TCP"}]` | 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). The port list is also used to generate Network Policies that allow ingress into the pods. |
+| ports | list | `[{"containerPort":80,"name":"http","port":null,"protocol":"TCP"}]` | (`ContainerPort[]`) 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). The port list is also used to generate Network Policies that allow ingress into the pods. See https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.21/#containerport-v1-core for details. **Note: We have added an optional "port" field to this list that allows the user to override the Service Port (for example 80) that a client  connects to, without altering the Container Port (say, 8080) that is listening for connections. |
 | preStopCommand | list | `["/bin/sleep","10"]` | Before a pod gets terminated, Kubernetes sends a SIGTERM signal to every container and waits for period of time (10s by default) for all containers to exit gracefully. If your app doesn't handle the SIGTERM signal or if it doesn't exit within the grace period, Kubernetes will kill the container and any inflight requests that your app is processing will fail. Make sure you set this to SHORTER than the terminationGracePeriod (30s default) setting. https://docs.flagger.app/tutorials/zero-downtime-deployments#graceful-shutdown |
 | progressDeadlineSeconds | string | `nil` | https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#progress-deadline-seconds |
 | prometheusRules.CPUThrottlingHigh | object | `{"for":"15m","severity":"warning","threshold":65}` | Container is being throttled by the CGroup - needs more resources. |

charts/simple-app/README.md.gotmpl

@@ -10,6 +10,23 @@ This chart provides a default deployment for a simple application that operates
 in a [Deployment][deployments]. The chart automatically configures various
 defaults for you like the Kubernetes [Horizontal Pod Autoscaler][hpa].
 
+## Upgrade Notes
+
+### 0.16.x -> 0.17.x
+
+**New Feature: Customize User-Facing Ports**
+
+You can now expose a custom port for your users (eg: `80`) while your service
+continues to listen on a private containerPort (eg: `5000`). In the maps in
+`.Values.ports` simply add a `port: <int>` key and the `Service` resource
+will be reconfigured to route that port to the backend container port.
+
+**Bug Fix: ServiceMonitor resources were broken**
+
+Previously the `ServiceMonitor` resources were pointing to the `Service` but
+the `Service` did not expose a `metrics` endpoint, which caused the resource to
+be invalid. This has been fixed.
+
 ## Monitoring
 
 This chart makes the assumption that you _do_ have a Prometheus-style

charts/simple-app/templates/_helpers.tpl

@@ -118,3 +118,33 @@ setting.
 {{- .Values.proxySidecar.image.repository }}:{{ $tag }}
 {{- end }}
 {{- end }}
+
+{{/*
+Creates a Container "ports" map based on .Values.ports. We do this because we
+have customized the values that can be put into the list of "port" maps to
+simplify exposing a customer-facing port number (eg 80) while maintaining an
+internal application port-number (eg, 8080)
+*/}}
+{{- define "simple-app.containerPorts" -}}
+{{- range $p := index .Values.ports -}}
+- name: {{ required "Must set a port name" $p.name }}
+  containerPort: {{ required "Must set a containerPort" $p.containerPort }}
+  {{- with $p.protocol }}
+  protocol: {{ . }}
+  {{- end }}
+{{- end -}}
+{{- end -}}
+
+{{/*
+This is the Service-side of the Ports mapping - we take the .Values.ports map
+and turn it into a list of ports that are exposed by the Service resource.
+Again, we do not use all of the values, we only use the values that make sense.
+*/}}
+{{- define "simple-app.servicePorts" -}}
+{{- range $port := .Values.ports -}}
+- port: {{ default $port.containerPort $port.port }}
+  targetPort: {{ $port.name }}
+  protocol: {{ $port.protocol }}
+  name: {{ $port.name }}
+{{- end -}}
+{{- end -}}

charts/simple-app/templates/deployment.yaml

@@ -210,9 +210,9 @@ spec:
           volumeMounts:
             {{- toYaml . | nindent 12 }}
           {{- end }}
-          {{- with .Values.ports }}
+          {{- if .Values.ports }}
           ports:
-            {{- toYaml . | nindent 12 }}
+            {{- include "simple-app.containerPorts" . | nindent 12 }}
           {{- end }}
           livenessProbe:
             {{- toYaml .Values.livenessProbe | nindent 12 }}
@@ -303,11 +303,7 @@ spec:
           {{- if .Values.proxySidecar.enabled }}
           {{- else }}
           ports:
-            {{- if .Values.ports }}
-            {{- if gt (len .Values.ports) 0 }}
-            {{- toYaml .Values.ports | nindent 12 }}
-            {{- end }}
-            {{- end }}
+            {{- include "simple-app.containerPorts" . | nindent 12 }}
             {{- if .Values.monitor.enabled }}
             - name: {{ .Values.monitor.portName }}
               containerPort: {{ .Values.monitor.portNumber }}

charts/simple-app/templates/service.yaml

@@ -1,5 +1,9 @@
-{{- if .Values.ports }}
-{{- if gt (len .Values.ports) 0 }}
+{{/*
+The Service resource is  created any time the "ports" key has ports. It is
+also created any time the Values.monitor.enabled is set true - because we use
+a ServiceMonitor in this chart to monitor pods.
+*/}}
+{{- if or .Values.ports .Values.monitor.enabled }}
 apiVersion: v1
 kind: Service
 metadata:
@@ -9,13 +13,12 @@ metadata:
 spec:
   type: {{ .Values.service.type }}
   ports:
-    {{- range $port := .Values.ports }}
-    - port: {{ $port.containerPort }}
-      targetPort: {{ $port.name }}
-      protocol: {{ $port.protocol }}
-      name: {{ $port.name }}
+    {{- include "simple-app.servicePorts" . | nindent 4 }}
+    {{- if .Values.monitor.enabled }}
+    - port: {{ .Values.monitor.portNumber }}
+      targetPort: {{ .Values.monitor.portNumber }}
+      name: {{ .Values.monitor.portName }}
     {{- end }}
   selector:
     {{- include "simple-app.selectorLabels" . | nindent 4 }}
 {{- end }}
-{{- end }}

charts/simple-app/templates/servicemonitor.yaml

@@ -1,3 +1,8 @@
+{{/*
+If we're in an Istio mesh, we expect the Istio ServiceMonitors to collect the
+metrics from the Envoy Sidecar, and we let the Envoy sidecar collect the
+metrics from the application itself.
+*/}}
 {{- if and .Values.monitor.enabled (not .Values.istio.enabled) }}
 apiVersion: monitoring.coreos.com/v1
 kind: ServiceMonitor

charts/simple-app/values.yaml

@@ -116,17 +116,23 @@ readinessProbe:
     path: /
     port: http
 
-# -- 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).
-# The port list is also used to generate Network Policies that allow ingress
-# into the pods.
+# -- (`ContainerPort[]`) 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). The port list is also used to generate Network
+# Policies that allow ingress into the pods. See
+# https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.21/#containerport-v1-core
+# for details.
+#
+# **Note: We have added an optional "port" field to this list that allows the
+# user to override the Service Port (for example 80) that a client  connects
+# to, without altering the Container Port (say, 8080) that is listening for
+# connections.
 ports:
   - name: http
     containerPort: 80
     protocol: TCP
-  - name: https
-    containerPort: 443
-    protocol: TCP
+    # Optional flag to override the client-facing port for service requests.
+    port:
 
 # -- Supply a reference to a Secret that can be used by Kubernetes to pull down
 # the Docker image. This is only used in local development, in combination with