fix(connect): propagate sharedStorage.subPath to launcher-managed job volumeMounts

[hyperbolist]

When launcher.enabled, and when sharedStorage.subPath is non-empty, launcher-managed job pod volumeMounts did not prepend that subPath to their job-specific volumeMounts, which had the effect of mounting them higher on the shared storage volume than intended, which caused the job pods to quit immediately when they could not find expected files.

This change ensures that when sharedStorage.subPath is non-empty, it is prepended to the job-specific subPaths of the launcher-managed job pod volumeMounts.

Tested by deploying the chart with clean shared storage volume and fresh database, and then adding a few gallery examples via the web UI, which we could then open and interact with.

fixes #759

Details:

The default values file now prepares an empty value at .Values.launcher.templateValues.sharesStorage.subPath.
https://github.com/FairwindsOps/rstudio-helm/blob/e8ea3daea13f5b1e6ab9ec579d997b1b0a9c2fac/charts/rstudio-connect/values.yaml#L381-L382

The configmap template now pushes .Values.sharedStorage.subPath into the $sessionTemplate for job.tpl to pick up.
https://github.com/FairwindsOps/rstudio-helm/blob/e8ea3daea13f5b1e6ab9ec579d997b1b0a9c2fac/charts/rstudio-connect/templates/configmap.yaml#L60-L63

The job template now looks for the subPath key of .Job.volumeMounts items and prepends $templateData.sharedStorage.subPath to its value if non-empty.
https://github.com/FairwindsOps/rstudio-helm/blob/e8ea3daea13f5b1e6ab9ec579d997b1b0a9c2fac/charts/rstudio-connect/files/job.tpl#L285-L293

[CLAassistant]

All committers have signed the CLA.

[dbkegley]

First of all, thank you for identifying this issue and submitting a fix! I think there's 1 edge case related to execution environment volume mounts that would need to be addressed but this looks like it's very much on the right track

https://docs.posit.co/connect/admin/appendix/off-host/execution-environment-volume-mounts/#volume-types

[dbkegley]

I think this implementation breaks when the execution environment contains any custom volume mounts. I believe this should only prepend the subPath only if sharedStorage.name != $volumeMount.name. Otherwise we'll end up prepending subPath to all volumes and not just the DataDirPVC volume.

[hyperbolist]

Ahh, I see. I will adjust the logic such that it can only apply to sharedStorage volume mounts.

[hyperbolist]

Adjusted such that this only applies to mounts of the sharedStorage volume.

[hyperbolist]

Adjusted the logic such that it can only prepend sharedStorage.subPath to mounts of the sharedStorage volume.

The configmap template now also pushes sharedStorage pvc name into the $sessionTemplate for job.tpl to pick up.
https://github.com/FairwindsOps/rstudio-helm/blob/8ebc46b0d447ac4744f54ec76f1296f547420f22/charts/rstudio-connect/templates/configmap.yaml#L60-L64

The job template now finds the volume whose pvc name matches the above, and only considers prepending for mounts of that volume.
https://github.com/FairwindsOps/rstudio-helm/blob/8ebc46b0d447ac4744f54ec76f1296f547420f22/charts/rstudio-connect/files/job.tpl#L280-L290

[dbkegley]

Ran this locally and confirmed that it works as expected. Thanks for this!

    Mounts:
      /connect/mnt/app from mount0 (rw,path="connect-data/apps/1/1")
      /connect/mnt/job from mount0 (rw,path="connect-data/jobs/1/sLUGS8Fgs1wZhAv3")
      /connect/mnt/python-environments from mount0 (rw,path="connect-data/python-environments")
      /var/run/secrets/kubernetes.io/serviceaccount from kube-api-access-tkjhx (ro)
Conditions:
  Type                        Status
  PodReadyToStartContainers   False
  Initialized                 True
  Ready                       False
  ContainersReady             False
  PodScheduled                True
Volumes:
  mount0:
    Type:       PersistentVolumeClaim (a reference to a PersistentVolumeClaim in the same namespace)
    ClaimName:  connect-dev-data-pvc
    ReadOnly:   false

Unfortunately we can't run CI for contributor PRs so I'll close this and open one so we can get this changed merged.