Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve support for sidecar containers #58

Open
jawnsy opened this issue Jun 6, 2022 · 2 comments
Open

Improve support for sidecar containers #58

jawnsy opened this issue Jun 6, 2022 · 2 comments

Comments

@jawnsy
Copy link

jawnsy commented Jun 6, 2022

Overview

Support configuring sidecars as YAML rather than a YAML string.

Current behavior

The extraManifests setting allows us to define a list of manifests to deploy, in list format:

retool-helm/values.yaml

Lines 282 to 290 in 1e06cee

extraManifests: []
# extraManifests:
# - apiVersion: cloud.google.com/v1beta1
# kind: BackendConfig
# metadata:
# name: "{{ .Release.Name }}-testing"
# spec:
# securityPolicy:
# name: "my-gcp-cloud-armor-policy"

For example, my extraManifests looks like this:

extraManifests:
  - apiVersion: networking.gke.io/v1beta1
    kind: FrontendConfig
    metadata:
      name: "frontend"
    spec:
      sslPolicy: "restricted"
  - apiVersion: cloud.google.com/v1
    kind: BackendConfig
    metadata:
      name: "backend"
  - apiVersion: networking.gke.io/v1
    kind: ManagedCertificate
    metadata:
      name: managed-cert
    spec:
      domains:
        - retool.contoso.com

Given that extraContainers is defined as a list [] by default, a reasonable assumption would be that we can add sidecar containers in the same way:

extraContainers: []

However, the following configuration results in an error message:

extraContainers:
  - name: cloud-sql-proxy
    image: gcr.io/cloudsql-docker/gce-proxy:1.28.0
    command:
      - "/cloud_sql_proxy"
      - "-enable_iam_login"
      - "-log_debug_stdout"
      - "-instances=contoso-retool:us-east1:retool=tcp:5432"
    securityContext:
      runAsNonRoot: true
    resources:
      requests:
        memory: "256Mi"
        cpu: "250m"
      limits:
        memory: "1Gi"
        cpu: "1000m"

This results in an error:

$ helm upgrade retool retool/retool --install --namespace=retool --values=values.yaml
Error: UPGRADE FAILED: template: retool/templates/deployment_backend.yaml:195:7: executing "retool/templates/deployment_backend.yaml" at <.>: wrong type for value; expected string; got []interface {}

We can fix this by changing the extraContainers to be a string, by adding a |, like this:

extraContainers: |
  - name: cloud-sql-proxy
    image: gcr.io/cloudsql-docker/gce-proxy:1.28.0
    command:
      - "/cloud_sql_proxy"
      - "-enable_iam_login"
      - "-log_debug_stdout"
      - "-instances=prefect-retool:us-east1:retool-us-east1=tcp:5432"
    securityContext:
      runAsNonRoot: true
    resources:
      requests:
        memory: "256Mi"
        cpu: "250m"
      limits:
        memory: "1Gi"
        cpu: "1000m"

Unfortunately, this causes us to lose syntax highlighting support from IDEs, since it is interpreted as a simple string.

Desired behavior

Ideally, we should be able to specify containers using YAML rather than as a string. Compare this to how Bitnami charts allow users to configure sidecars: https://docs.bitnami.com/kubernetes/apps/wordpress/configuration/configure-sidecar-init-containers/

In the template, it is implemented with code like this: https://github.com/bitnami/charts/blob/0c32e83bd5ac05cd9f2f17e60b0e3f3aaa6d0c8f/bitnami/postgresql/templates/primary/statefulset.yaml#L561-L563

Notes

A benefit of the existing approach is that the extra containers can refer to other template values, which we would lose if we made it behave similarly to extraManifests.

@jawnsy
Copy link
Author

jawnsy commented Jun 6, 2022

Related: #40

@djMax
Copy link

djMax commented Dec 18, 2023

Also a benefit that value overrides in environments would work instead of having to cut/paste/modify

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants