Skip to content

Commit

Permalink
[simple-app] Completely refactor the ingress/virtualService handling (#…
Browse files Browse the repository at this point in the history
…11)

**What did I want?**

The existing `Ingress` and `VirtualService` handling was confusing and inconsistent. I wanted to
simplify it - by making an opinionated decision about the "Standard" resource configuration that
we should use.

**What did I do?**

1. Refactored the `Ingress` template so that the HTTP rules were hard-coded rather than using
    big nested strings that are then run through `tpl`. The new opinionated framework more closely
    matches what we do in our normal ALBs. This also just makes the template _much_ easier to
    understand and document. If a user needs a custom Ingress, they can create it on their own.

2. Refactored the `VirtualService` template to also be highly opinionated and favor a very simple
    single-service routing model. Custom `VirtualService` resources can be defined by the user
    if they need one.

3. Renamed the `ingressGateway` top level values key to `virtualService` to more accurately
    reflect the resource that it is configuring.

4. Removed unnecessary `service.port` parameters because we use the `ports[]` list instead
    anyways. Even I forgot what that value was being used for (hint: it was just used for a test)
    so it was clearly confusing.

5. Improved the test-suite by adding in some handling of connection-refused errors - this is
    because the test-container starts up faster than the service container does, and we were
    seeing random race condition failures.
  • Loading branch information
diranged authored May 21, 2021
1 parent 755b7a6 commit 6a6f2ee
Show file tree
Hide file tree
Showing 10 changed files with 166 additions and 107 deletions.
2 changes: 1 addition & 1 deletion charts/simple-app/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ apiVersion: v2
name: simple-app
description: Default Microservice Helm Chart
type: application
version: 0.4.1
version: 0.5.0
appVersion: latest
maintainers:
- name: diranged
Expand Down
2 changes: 2 additions & 0 deletions charts/simple-app/Makefile
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
NAMESPACE := simple-app

include ../../contrib/Helm.mk
include ../../contrib/Testing.mk
45 changes: 23 additions & 22 deletions charts/simple-app/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

Default Microservice Helm Chart

![Version: 0.4.1](https://img.shields.io/badge/Version-0.4.1-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.5.0](https://img.shields.io/badge/Version-0.5.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/
Expand All @@ -26,52 +26,53 @@ defaults for you like the Kubernetes [Horizontal Pod Autoscaler][hpa].
| image.repository | string | `"nginx"` | (String) The Docker image name and repository for your application |
| image.tag | String | `nil` | Overrides the image tag whose default is the chart appVersion. |
| imagePullSecrets | list | `[]` | 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 our `kube_create_ecr_creds` function from dotfiles. |
| ingress.annotations."alb.ingress.kubernetes.io/actions.ssl-redirect" | string | `"{\n \"Type\": \"redirect\",\n \"RedirectConfig\": {\n \"Protocol\": \"HTTPS\",\n \"Port\": \"443\",\n \"StatusCode\": \"HTTP_301\"\n }\n}"` | |
| ingress.annotations | object | `{}` | Any annotations you wish to add to the ALB. See https://kubernetes-sigs.github.io/aws-load-balancer-controller/v2.2/guide/ingress/annotations/ for more details. |
| ingress.enabled | bool | `false` | |
| ingress.hosts[0].host | string | `"chart-example.local"` | |
| ingress.hosts[0].path | string | `"/"` | |
| ingress.hosts[0].pathType | string | `"Prefix"` | |
| ingress.hosts[0].sslRedirect | bool | `true` | |
| ingressGateway.annotations | object | `{}` | |
| ingressGateway.enabled | bool | `false` | (Boolean) Maps the Service to an Istio IngressGateway, exposing the service outside of the Kubernetes cluster. |
| ingressGateway.gateway | string | `"default-gateway"` | |
| ingressGateway.hosts | string | `"- {{ include \"simple-app.fullname\" . }}.{{ .Release.Namespace }}"` | |
| ingressGateway.http | string | `"- match:\n - uri:\n prefix: /\n route:\n - destination:\n host: {{ include \"simple-app.fullname\" . }}\n port:\n number: {{ .Values.ingressGateway.port }}"` | (String) VirtualService "http" blob in text-form. This is run through the tpl function so you may use template variables here. |
| ingressGateway.namespace | string | `"istio-system"` | |
| ingressGateway.port | int | `80` | |
| ingressGateway.tls | string | `""` | |
| ingress.host | string | `"{{ include \"simple-app.fullname\" . }}.{{ .Release.Namespace }}"` | This setting configures the ALB to listen specifically to requests for this hostname. It _also_ ties into the external-dns controller and automatically provisions DNS hostnames matching this value (presuming that they are allowed by the cluster settings). |
| ingress.path | string | `"/"` | See the `ingress.pathType` setting documentation. |
| ingress.pathType | string | `"Prefix"` | https://kubernetes.io/docs/concepts/services-networking/ingress/#path-types |
| ingress.port | string | `nil` | If set, this will override the `service.portName` parameter, and the `Service` object will point specifically to this port number on the backing Pods. |
| ingress.portName | string | `"http"` | This is the port "name" that the `Service` will point to on the backing Pods. This value must match one of the values of `.name` in the `Values.ports` configuration. |
| ingress.sslRedirect | bool | `true` | If `true`, then this will annotate the Ingress with a special AWS ALB Ingress Controller annotation that configures an SSL-redirect at the ALB level. |
| 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. |
| nameOverride | string | `""` | |
| nodeSelector | object | `{}` | |
| podAnnotations | object | `{}` | |
| podDisruptionBudget | object | `{}` | Set up a PodDisruptionBudget for the Deployment |
| podDisruptionBudget | object | `{}` | Set up a PodDisruptionBudget for the Deployment. See https://kubernetes.io/docs/tasks/run-application/configure-pdb/ for more details. |
| 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. |
| proxySidecar.enabled | bool | `false` | (Boolean) Enables injecting a pre-defined reverse proxy sidecar container into the Pod containers list. |
| proxySidecar.env | list | `[]` | Environment Variables for the primary container. These are all run through the tpl function (the key name and value), so you can dynamically name resources as you need. |
| proxySidecar.image.pullPolicy | string | `"Always"` | (String) Always, Never or IfNotPresent |
| proxySidecar.image.pullPolicy | string | `"IfNotPresent"` | (String) Always, Never or IfNotPresent |
| proxySidecar.image.repository | string | `"nginx"` | (String) The Docker image name and repository for the sidecar |
| proxySidecar.image.tag | string | `"latest"` | (String) The Docker tag for the sidecar |
| proxySidecar.name | string | `"proxy"` | (String) 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. |
| replicaCount | int | `1` | The number of Pods to start up by default |
| replicaCount | int | `1` | 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. |
| resources | object | `{}` | |
| securityContext | object | `{}` | |
| service.port | int | `80` | |
| service.type | string | `"ClusterIP"` | |
| serviceAccount.annotations | object | `{}` | |
| serviceAccount.create | bool | `true` | |
| serviceAccount.name | string | `""` | |
| tests.connection.args | list | `["{{ include \"simple-app.fullname\" . }}:{{ .Values.service.port }}"]` | A list of arguments passed into the command. These are run through the tpl function. |
| tests.connection.command | list | `["curl"]` | The command used to trigger the test. |
| tests.connection.args | list | `["{{ include \"simple-app.fullname\" . }}"]` | A list of arguments passed into the command. These are run through the tpl function. |
| tests.connection.command | list | `["curl","--retry-connrefused","--retry","5"]` | The command used to trigger the test. |
| tests.connection.image.repository | string | `nil` | Sets the image-name that will be used in the "connection" integration test. If this is left empty, then the .image.repository value will be used instead (and the .image.tag will also be used). |
| tests.connection.image.tag | string | `nil` | Sets the tag that will be used in the "connection" integration test. If this is left empty, the default is "latest" |
| tolerations | list | `[]` | |
| virtualService.annotations | object | `{}` | Any annotations you wish to add to the `VirtualService` resource. See https://istio.io/latest/docs/reference/config/annotations/ for more details. |
| virtualService.enabled | bool | `false` | (Boolean) Maps the Service to an Istio IngressGateway, exposing the service outside of the Kubernetes cluster. |
| virtualService.gateway | string | `"default-gateway"` | The name of the Istio `IngressGateway` object that this `VirtualService` will register with. The default here is a private internal gateway that is not internet-facing. |
| virtualService.hosts | list | `["{{ include \"simple-app.fullname\" . }}.{{ .Release.Namespace }}"]` | A list of destination hostnames that this VirtualService will accept traffic for. Multiple names can be listed here. See https://istio.io/latest/docs/reference/config/networking/virtual-service/#VirtualService for more details. |
| virtualService.namespace | string | `"istio-system"` | The namespace where the Istio services are operating. Do not change this. |
| virtualService.path | string | `"/"` | The default path prefix that the `VirtualService` will match requests against to pass to the default `Service` object in this deployment. |
| 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.tls | string | `""` | |
| volumeMounts | list | `[]` | List of VolumeMounts that are applied to the application container - these must refer to volumes set in the `Values.volumes` parameter. |
| volumes | string | `nil` | A list of 'volumes' that can be mounted into the Pod. |
| volumesString | string | `nil` | A stringified list of 'volumes' similar to the `Values.volumes` parameter - but this one gets run through the `tpl` function so that you can use templatized values if you need to. |
| volumes | list | `[]` | A list of 'volumes' that can be mounted into the Pod. See https://kubernetes.io/docs/concepts/storage/volumes/. |
| volumesString | string | `""` | A stringified list of 'volumes' similar to the `Values.volumes` parameter, but this one gets run through the `tpl` function so that you can use templatized values if you need to. See https://kubernetes.io/docs/concepts/storage/volumes/. |

----------------------------------------------
Autogenerated from chart metadata using [helm-docs v1.5.0](https://github.com/norwoodj/helm-docs/releases/v1.5.0)
2 changes: 1 addition & 1 deletion charts/simple-app/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ spec:
- name: {{ .Chart.Name }}
securityContext:
{{- toYaml .Values.securityContext | nindent 12 }}
image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}"
image: "{{ .Values.image.repository }}:{{ default .Chart.AppVersion .Values.image.tag }}"
imagePullPolicy: {{ .Values.image.pullPolicy }}
{{- with .Values.env }}
env:
Expand Down
54 changes: 29 additions & 25 deletions charts/simple-app/templates/ingress/ingress.yaml
Original file line number Diff line number Diff line change
@@ -1,45 +1,49 @@
{{- if .Values.ingress.enabled -}}
{{- $fullName := include "simple-app.fullname" . -}}
{{- $svcPort := .Values.service.port -}}
{{- $fullName := include "simple-app.fullname" . }}
apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
name: {{ $fullName }}
labels:
{{- include "simple-app.labels" . | nindent 4 }}
{{- with .Values.ingress.annotations }}
annotations:
{{- with .Values.ingress.annotations }}
{{- toYaml . | nindent 4 }}
{{- end }}
{{- end }}
{{- if .Values.ingress.sslRedirect }}
alb.ingress.kubernetes.io/actions.ssl-redirect: >-
{
"Type": "redirect",
"RedirectConfig": {
"Protocol": "HTTPS",
"Port": "443",
"StatusCode": "HTTP_301"
}
}
{{- end }}
spec:
rules:
{{- range .Values.ingress.hosts }}
- host: {{ .host | quote }}
- host: {{ tpl .Values.ingress.host . | quote }}
http:
paths:
{{- /*

The ssl-redirect action must be the first action in the list, if
we're going to use it. It is combined with the the
"alb.ingress.kubernetes.io/actions.ssl-redirect" annotation to do
ALB-level HTTP->HTTPS routing.

*/}}
{{- if .sslRedirect }}
- path: {{ .path }}
pathType: {{ .pathType }}
- path: {{ .Values.ingress.path }}
pathType: {{ .Values.ingress.pathType }}
backend:
service:
name: ssl-redirect
name: {{ $fullName }}
port:
name: use-annotation
{{- end }}
- path: {{ .path }}
pathType: {{ .pathType }}
{{- if .Values.ingress.port }}
number: {{ .Values.ingress.port }}
{{- else }}
name: {{ .Values.ingress.portName }}
{{- end }}
{{- if .Values.ingress.sslRedirect }}
- path: {{ .Values.ingress.path }}
pathType: {{ .Values.ingress.pathType }}
backend:
service:
name: {{ $fullName }}
name: ssl-redirect
port:
number: {{ $svcPort }}
{{- end }}
name: use-annotation
{{- end }}
{{- end }}
2 changes: 1 addition & 1 deletion charts/simple-app/templates/istio/networkpolicy.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
{{- if .Values.ingressGateway.enabled }}
{{- if .Values.virtualService.enabled }}
apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
Expand Down
31 changes: 21 additions & 10 deletions charts/simple-app/templates/istio/virtualservice.yaml
Original file line number Diff line number Diff line change
@@ -1,25 +1,36 @@
{{- if .Values.ingressGateway.enabled }}
{{- if .Values.virtualService.enabled }}
{{- $global := . }}
apiVersion: networking.istio.io/v1alpha3
kind: VirtualService
metadata:
name: {{ include "simple-app.fullname" . }}
labels:
{{- include "simple-app.labels" . | nindent 4 }}
{{- with .Values.ingressGateway.annotations }}
{{- with .Values.virtualService.annotations }}
annotations:
{{- toYaml . | nindent 4 }}
{{- end }}
spec:
hosts:
{{- tpl .Values.ingressGateway.hosts . | nindent 4 }}
gateways:
- {{ .Values.ingressGateway.namespace }}/{{ .Values.ingressGateway.gateway }}
{{- if .Values.ingressGateway.http }}
- {{ .Values.virtualService.namespace }}/{{ .Values.virtualService.gateway }}
hosts:
{{- range .Values.virtualService.hosts }}
- {{ tpl . $global | quote }}
{{- end }}
{{- /* https://istio.io/latest/docs/reference/config/networking/virtual-service/#HTTPRoute */}}
http:
{{- tpl .Values.ingressGateway.http . | nindent 4 }}
{{- end }}
{{- if .Values.ingressGateway.tls }}
- match:
{{- /* https://istio.io/latest/docs/reference/config/networking/virtual-service/#HTTPMatchRequest */}}
- uri:
prefix: {{ .Values.virtualService.path }}
{{- /* https://istio.io/latest/docs/reference/config/networking/virtual-service/#HTTPRouteDestination */}}
route:
- destination:
host: {{ include "simple-app.fullname" . }}
port:
number: {{ .Values.virtualService.port }}
{{- with .Values.virtualService.tls }}
tls:
{{- tpl .Values.ingressGateway.tls . | nindent 4 }}
{{- tpl . $global | nindent 4 }}
{{- end }}
{{- end }}
6 changes: 3 additions & 3 deletions charts/simple-app/templates/tests/test-connection.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ spec:
- name: test
image: >-
{{- if .Values.tests.connection.image.repository }}
{{ .Values.tests.connection.image.repository }}:{{ .Values.tests.connection.image.tag | default "latest" }}
{{ .Values.tests.connection.image.repository }}:{{ default "latest" .Values.tests.connection.image.tag }}
{{- else }}
{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}
{{ .Values.image.repository }}:{{ default .Chart.AppVersion .Values.image.tag }}
{{- end }}
command:
{{ toYaml .Values.tests.connection.command }}
{{- toYaml .Values.tests.connection.command | nindent 8 }}
args:
{{- $global := . }}
{{- range $arg := index .Values.tests.connection.args }}
Expand Down
9 changes: 9 additions & 0 deletions charts/simple-app/values.local.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# For local development, we turn on the Ingress controller and set up a simple
# local ingress.
ingress:
# -- Enable local ingress for local development.
enabled: true

# -- Disable the SSL-Redirect explicitly because it only applies to
# ALB-ingress controllers.
sslRedirect: false
Loading

0 comments on commit 6a6f2ee

Please sign in to comment.