rstudio / helm

Helm Resources for RStudio Products
MIT License
36 stars 28 forks source link

Add hostAliases to the main Connect pod and content pods #545

Closed samcofer closed 2 months ago

samcofer commented 2 months ago

A customer asked to have hostAlias exposed because of some custom hostAliases that they need to resolve in Kubernetes, but don't have an internal DNS service to used. Seems like an innocuous request and something we should support anyway.

samcofer commented 2 months ago

I haven't updated the NEWS because this feels like something that we might not even want a version bump for, and because it's possible this change should just be merged with the next version upgrade.

dbkegley commented 2 months ago

LGTM. I did not try installing but it looks like these new entries make it into the rendered output as expected for both the connect pod and the content pod templates.

$ helm template --set launcher.enabled=true --set sharedStorage.create=true ./ | grep hostAlias -A 5
    {"job":{"annotations":{},"labels":{}},"pod":{"affinity":{},"annotations":{},"command":[],"containerSecurityContext":{},"defaultSecurityContext":{},"env":[],"extraContainers":[],"hostAliases":[{"asdfqwer":"asdfqwer.com"}],"imagePullPolicy":"","imagePullSecrets":[],"initContainers":[{"image":"ghcr.io/rstudio/rstudio-connect-content-init:ubuntu2204-2024.08.0","imagePullPolicy":"IfNotPresent","name":"init","securityContext":{},"volumeMounts":[{"mountPath":"/mnt/rstudio-connect-runtime/","name":"rsc-volume"}]}],"labels":{},"nodeSelector":{},"priorityClassName":"","securityContext":{},"serviceAccountName":"","tolerations":[],"volumeMounts":[{"mountPath":"/opt/rstudio-connect","name":"rsc-volume"}],"volumes":[{"emptydir":{},"name":"rsc-volume"}]},"service":{"annotations":{},"labels":{},"type":"ClusterIP"}}
    {{- end }}
  job.tpl: |
    # Version: 2.3.1
    # DO NOT MODIFY the "Version: " key
    # Helm Version: v3
--
          {{- with $templateData.pod.hostAliases }}
          hostAliases:
            {{- toYaml . | nindent 8 }}
          {{- end }}
          shareProcessNamespace: {{ .Job.shareProcessNamespace }}
          {{- if or (ne (len .Job.volumes) 0) (ne (len $templateData.pod.volumes) 0) }}
          volumes:
--
      hostAliases:
        - asdf: asdf.com

      serviceAccountName: release-name-rstudio-connect
      containers:

I haven't updated the NEWS because this feels like something that we might not even want a version bump for, and because it's possible this change should just be merged with the next version upgrade.

It would be good to pull in both this PR and #541 before the next chart release. I'd prefer to do this before the next Connect release because that's still a few weeks away. We're also sorting out some issues with the current chart version so we may do another chart release soon anyway.

samcofer commented 2 months ago

@atheriel @colearendt

I'm not super clear on the helm release/merge progress. I also know that you're working on the release process today, so feel free to review/approve and merge when it makes sense.

dbkegley commented 2 months ago

@samcofer okay if I use this branch? I can pull #541 in here and then do a release later today.

samcofer commented 2 months ago

Work for me!

dbkegley commented 2 months ago

Running with launcher.templateValues.defaultInitContainer.resources

k get pod -o json  python-environment-restore-2hvmv-f5ktc | jq '.spec.initContainers[0].resources'
{
  "limits": {
    "cpu": "512m",
    "memory": "512Mi"
  },
  "requests": {
    "cpu": "128m",
    "memory": "128Mi"
  }
}

And with pod.hostAliases and launcher.templateValues.pod.hostAliases

k get pod -o json connect-rstudio-connect-85c9f557b5-5rlfm | jq '.spec.hostAliases' 
[
  {
    "hostnames": [
      "foo.local",
      "bar.local"
    ],
    "ip": "127.0.0.1"
  }
]
k get pod -o json  python-environment-restore-2hvmv-f5ktc | jq '.spec.hostAliases'
[
  {
    "hostnames": [
      "foo.local",
      "bar.local"
    ],
    "ip": "127.0.0.1"
  }
]