Conversation
| {{- end }} | ||
| spec: | ||
| gatewayClassName: {{ $gateway.className }} | ||
| {{- if $gateway.addresses }} |
There was a problem hiding this comment.
addresses is part of the standard Gateway API spec https://gateway-api.sigs.k8s.io/reference/spec/#gatewayspecaddress but it is optional. This is not a provider specific change rather it does not render addresses if none are provided.
| - {{ $cert }} | ||
| {{- end }} | ||
| type: certmap | ||
| type: {{ $gatewayConfig.tlsType }} |
There was a problem hiding this comment.
certmap is GKE specific
| component: gateway | ||
| type: {{ $gatewayConfig.type }} | ||
| className: {{ $gatewayConfig.className }} | ||
| addresses: |
There was a problem hiding this comment.
I wonder if we want to do the same addresses optional if here.
| {{- $labels.labels | toYaml | nindent 4 }} | ||
| {{- $params = dict "type" "serviceAccount" }} | ||
| {{- $annotations := include "mozcloud.annotations.argo" $params | fromYaml }} | ||
| {{- if or (eq (index (index .Values "cloud" | default dict) "provider" | default "gke") "gke") $annotations }} |
There was a problem hiding this comment.
Not sure if this is correct. We likely still want annotations here. Just maybe different annotations.
There was a problem hiding this comment.
Leaving the non-GKE annotations is good. We'll need those for Argo.
On that note, we'll probably need to make Argo optional as well. That can be a future PR though.
2b88553 to
9507293
Compare
Currently we produce annotations, negs, and other items specifically tied to GKE. This PR is a first pass at adding conditionals around these GKE specific implementations. As our next target is likely to be an `on-prem` we introduce another values file to render the charts and test them without these specific implementations. As we gather more information about what we need to support for `on-prem` or other `providers` we can add specific cases for those options.
9507293 to
9513a40
Compare
| */ -}} | ||
| {{- define "mozcloud.gateway.backends" -}} | ||
| {{- $workloads := .workloads -}} | ||
| {{- $provider := index (index .Values "cloud" | default dict) "provider" | default "gke" -}} |
There was a problem hiding this comment.
| {{- $provider := index (index .Values "cloud" | default dict) "provider" | default "gke" -}} | |
| {{- $provider := dig "cloud" "provider" "gke" .Values }} |
I think you could simplify this using something like this.
| */}} | ||
| {{- if not (include "mozcloud.preview.usePreviewHttpRoute" .) }} | ||
| {{- $params := mergeOverwrite $context (dict "globals" .Values.global.mozcloud "workloads" $workloads) }} | ||
| {{- $provider := index (index .Values "cloud" | default dict) "provider" | default "gke" }} |
There was a problem hiding this comment.
| {{- $provider := index (index .Values "cloud" | default dict) "provider" | default "gke" }} | |
| {{- $provider := dig "cloud" "provider" "gke" .Values }} |
Same simplification here.
| GKE-specific gateway backend resources: GCPBackendPolicy, HealthCheckPolicy. | ||
| Only rendered when cloud.provider is "gke". | ||
| */ -}} | ||
| {{- $provider := index (index .Values "cloud" | default dict) "provider" | default "gke" }} |
There was a problem hiding this comment.
| {{- $provider := index (index .Values "cloud" | default dict) "provider" | default "gke" }} | |
| {{- $provider := dig "cloud" "provider" "gke" .Values }} |
| redirectToHttps: | ||
| enabled: true | ||
| responseCodeName: MOVED_PERMANENTLY_DEFAULT | ||
| sslPolicy: mozilla-intermediate |
There was a problem hiding this comment.
I believe this is unique to our specific GCP projects. We may want to make this configurable when cloud.provider == gke.
| {{- end -}} | ||
|
|
||
| {{- /* We do not use ingress resources for preview environments, and FrontendConfig is GKE-only */}} | ||
| {{- if and .Values.enabled (not (include "mozcloud.preview.enabled" .)) (eq (index (index .Values "cloud" | default dict) "provider" | default "gke") "gke") }} |
There was a problem hiding this comment.
| {{- if and .Values.enabled (not (include "mozcloud.preview.enabled" .)) (eq (index (index .Values "cloud" | default dict) "provider" | default "gke") "gke") }} | |
| {{- if and .Values.enabled (not (include "mozcloud.preview.enabled" .)) (eq (dig "cloud" "provider" "gke" .Values) "gke") }} |
| @@ -1,10 +1,12 @@ | |||
| {{- $provider := index (index .Values "cloud" | default dict) "provider" | default "gke" }} | |||
There was a problem hiding this comment.
| {{- $provider := index (index .Values "cloud" | default dict) "provider" | default "gke" }} | |
| {{- $provider := dig "cloud" "provider" "gke" .Values }} |
| GKE-specific gateway resources: GCPGatewayPolicy. | ||
| Only rendered when cloud.provider is "gke". | ||
| */ -}} | ||
| {{- $provider := index (index .Values "cloud" | default dict) "provider" | default "gke" }} |
There was a problem hiding this comment.
| {{- $provider := index (index .Values "cloud" | default dict) "provider" | default "gke" }} | |
| {{- $provider := dig "cloud" "provider" "gke" .Values }} |
| {{- $provider := index (index .Values "cloud" | default dict) "provider" | default "gke" }} | ||
| {{- if and (eq $provider "gke") (not (include "mozcloud.preview.usePreviewHttpRoute" .)) }} | ||
| {{- $chartMetadata := dict "Chart" .Chart "Release" .Release }} | ||
| {{- $context := deepCopy . }} | ||
| {{- $labelParams := mergeOverwrite $chartMetadata (include "mozcloud.labelParams" . | fromYaml) }} | ||
| {{- $previewConfig := mergeOverwrite (default dict .Values.global.preview) (default dict .Values.preview) }} | ||
| {{- $formatterParams := dict "api" "gateway" "workloads" (.Values.workloads | deepCopy) "preview" $previewConfig }} | ||
| {{- $workloads := include "mozcloud.formatter.workloads" $formatterParams | fromYaml -}} | ||
| {{- $hosts := list }} | ||
| {{- range $workloadName, $workloadConfig := $workloads }} | ||
| {{- range $hostName, $hostConfig := $workloadConfig.hosts }} | ||
| {{- $hosts = append $hosts $hostName }} | ||
| {{- end }} | ||
| {{- end }} |
There was a problem hiding this comment.
Since we keep doing some form of this across the Gateway and Ingress templates, I think it may make sense to create a helper function for this that we can call to get workloads and hosts.
| GKE-specific ingress resources: BackendConfig, ManagedCertificate. | ||
| Only rendered when cloud.provider is "gke". | ||
| */ -}} | ||
| {{- $provider := index (index .Values "cloud" | default dict) "provider" | default "gke" }} |
There was a problem hiding this comment.
| {{- $provider := index (index .Values "cloud" | default dict) "provider" | default "gke" }} | |
| {{- $provider := dig "cloud" "provider" "gke" .Values }} |
| {{- if or (eq (index (index .Values "cloud" | default dict) "provider" | default "gke") "gke") $annotations }} | ||
| annotations: | ||
| {{- if eq (index (index .Values "cloud" | default dict) "provider" | default "gke") "gke" }} |
There was a problem hiding this comment.
| {{- if or (eq (index (index .Values "cloud" | default dict) "provider" | default "gke") "gke") $annotations }} | |
| annotations: | |
| {{- if eq (index (index .Values "cloud" | default dict) "provider" | default "gke") "gke" }} | |
| {{- $provider := dig "cloud" "provider" "gke" .Values }} | |
| {{- if or (eq $provider "gke") $annotations }} | |
| annotations: | |
| {{- if eq $provider "gke" }} |
| # On-premises / non-GKE values profile for the mozcloud chart. | ||
| # | ||
| # Usage: | ||
| # helm template my-release . -f values.yaml -f values-onprem.yaml |
There was a problem hiding this comment.
| # helm template my-release . -f values.yaml -f values-onprem.yaml | |
| # helm template my-release . -f values-onprem.yaml |
values.yaml will always be rendered by default, even when specifying an alternate values file.
| "type": "string", | ||
| "description": "Cloud provider. Use \"gke\" for Google Kubernetes Engine (default), or \"on-prem\" for self hosted clusters.", | ||
| "default": "gke" |
There was a problem hiding this comment.
Do we know which providers we will be supporting yet? Is it boudned? If so, we may want to use an enum for this.
| apiVersion: networking.gke.io/v1 | ||
| kind: GCPBackendPolicy | ||
| metadata: | ||
| labels: | ||
| app.kubernetes.io/component: api | ||
| app.kubernetes.io/managed-by: argocd | ||
| app.kubernetes.io/name: mozcloud-test | ||
| app_code: mozcloud-test | ||
| component_code: api | ||
| env_code: dev | ||
| helm.sh/chart: test-chart | ||
| mozcloud_chart: mozcloud | ||
| mozcloud_chart_version: 1.0.0 | ||
| preview_pr: "789" | ||
| realm: nonprod | ||
| name: pr789-api-service | ||
| spec: | ||
| default: | ||
| logging: | ||
| enabled: true | ||
| sampleRate: 100000 | ||
| targetRef: | ||
| group: "" | ||
| kind: Service | ||
| name: pr789-api-service | ||
| 6: | | ||
| apiVersion: networking.gke.io/v1 | ||
| kind: HealthCheckPolicy | ||
| metadata: | ||
| labels: | ||
| app.kubernetes.io/component: api | ||
| app.kubernetes.io/managed-by: argocd | ||
| app.kubernetes.io/name: mozcloud-test | ||
| app_code: mozcloud-test | ||
| component_code: api | ||
| env_code: dev | ||
| helm.sh/chart: test-chart | ||
| mozcloud_chart: mozcloud | ||
| mozcloud_chart_version: 1.0.0 | ||
| preview_pr: "789" | ||
| realm: nonprod | ||
| name: pr789-api-service | ||
| spec: | ||
| default: | ||
| config: | ||
| httpHealthCheck: | ||
| requestPath: /__lbheartbeat__ | ||
| type: HTTP | ||
| targetRef: | ||
| group: "" | ||
| kind: Service | ||
| name: pr789-api-service |
There was a problem hiding this comment.
It seems like this shouldn't be getting deleted outright. Was this intentional?
emaydeck-mozilla
left a comment
There was a problem hiding this comment.
Left several comments above with some suggestions and thoughts.
Description
Currently we produce annotations, negs, and other items specifically tied to GKE. This PR is a first pass at adding conditionals around these GKE specific implementations.
As our next target is likely to be an
on-premwe introduce another values file to render the charts and test them without these specific implementations.As we gather more information about what we need to support for
on-premor otherproviderswe can add specific cases for those options.References