Update chronicle-agent examples, update CI

[AlexMapley]

Background

Closes #445

Summary

• Fixes chronicle-agent yaml example from Chronicle README Workbench example fixes #445 feedback
• Expands complex-values.yaml test files with the chronicle-agent and a busybox sidecar, which will run in our ci
• Fixes some, but not all issues in the connect/workbench complex values that will cause silent templating failures in our ci: https://github.com/rstudio/helm/actions/runs/7133027308/job/19424836827
• Updates the chronicle-agent image reference to point to ghcr.io/rstudio/chronicle-agent:2023.10.4, the last agent release
  • The plan is to eventually condense the server and agent releases into 1, for now they do diverge a bit though

Notes

We are currently failing 1 ci job: https://github.com/rstudio/helm/actions/runs/7132419728/job/19422921712, complaining that we haven't bumped up the chart version in this PR.
We haven't changed the contents of these charts though, just the docs and ci. Bumping up the chart version is unwarranted, so I'm going to ignore this complaint.

This is coming from https://github.com/rstudio/helm/blob/main/.github/workflows/chart-test.yaml#L33, and I don't think ct list-changes is flexible enough to ignore .md or .ci file changes:

% ct list-changed --help

Usage:
  ct list-changed [flags]

Flags:
      --chart-dirs strings        Directories containing Helm charts. May be specified multiple times
                                  or separate values with commas (default [charts])
      --config string             Config file
      --exclude-deprecated        Skip charts that are marked as deprecated
      --excluded-charts strings   Charts that should be skipped. May be specified multiple times
                                  or separate values with commas
      --github-groups             Change the delimiters for github to create collapsible groups
                                  for command output
  -h, --help                      help for list-changed
      --print-config              Prints the configuration to stderr (caution: setting this may
                                  expose sensitive data when helm-repo-extra-args contains passwords)
      --remote string             The name of the Git remote used to identify changed charts (default "origin")
      --since string              The Git reference used to identify changed charts (default "HEAD")
      --target-branch string      The name of the target branch used to identify changed charts (default "master")

hence why i'd rather just ignore this for now - I don't have a good fix in mind.

[AlexMapley]

see https://github.com/rstudio/helm/blob/main/charts/rstudio-connect/values.yaml#L57-L58, helm template was complaining that pod.nodeSelector had been rendered invalid

[AlexMapley]

We have some notes in connect 0.4.0 about this if you take a look at https://github.com/rstudio/helm/blob/main/charts/rstudio-connect/NEWS.md#040

[markrtucker]

I'm not sure I understand the purpose of this. Typically, a nodeSelector is used to force (or at least encourage) a pod to be scheduled on specific worker nodes. Does https://github.com/rstudio/helm/blob/main/charts/rstudio-connect/values.yaml#L57-L58 mean that we are required to put something here?

[AlexMapley]

Well this ci files was definitely out of date and referencing pod.nodeSelector instead of NodeSelector which was incorrect, so this change was needed.

Does https://github.com/rstudio/helm/blob/main/charts/rstudio-connect/values.yaml#L57-L58 mean that we are required to put something here?

If you take a look at https://github.com/rstudio/helm/blob/main/charts/rstudio-connect/templates/deployment.yaml#L43-L46 you'll see where this is referenced. nodeSelector will be an empty {} list by default, and thus won't even unwrap that template block:

      {{- with .Values.nodeSelector }}
      nodeSelector:
        {{- toYaml . | nindent 8 }}
      {{- end }}

So you aren't required to set a nodeSelector value, it's okay to leave this field blank and not invoke this code

[AlexMapley]

This change is admittedly a little bit of scope creep because it's just a connect ci fix, unrelated to chronicle. Still I think it's good to get our ci/cd pipelines working a bit better here.

[AlexMapley]

@shahmonanshi @tnederlof @colearendt I'm not who sure who the go to for the approval on this PR will be, but I was hoping some of you could take a look and give the okay on these changes. Thank you in advance!

[AlexMapley]

I was thinking we don't need to bump up the chart versions if we're not including any changes to our actual templates in this branch.

[tnederlof]

@AlexMapley sorry, I don't know much with regards to the CI, @colearendt will likely know more there.

[AlexMapley]

No worries @tnederlof, ty for taking a look anyways. With #457 we can probably close this branch too soon