From 0d5a6ead6e2d09feb91784f5d04edbada71ff80d Mon Sep 17 00:00:00 2001 From: "Joshua C. Forest" Date: Wed, 4 Dec 2024 10:03:19 -0500 Subject: [PATCH 1/8] Add tests for both styles of jobJsonOverridesFiles --- .../tests/deployment_test.yaml | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/ci/rstudio-workbench/tests/deployment_test.yaml b/ci/rstudio-workbench/tests/deployment_test.yaml index 7e4961d9..1764fb3a 100644 --- a/ci/rstudio-workbench/tests/deployment_test.yaml +++ b/ci/rstudio-workbench/tests/deployment_test.yaml @@ -416,3 +416,79 @@ tests: path: 'spec.template.spec.containers[0].volumeMounts[?(@.name=="rstudio-pam")]' - notExists: path: 'spec.template.spec.volumes[?(@.name=="rstudio-pam")]' + - it: should specify a volumeMount and a volume for the old style jobJsonOverridesFiles if jobJsonOverridesFiles is defined and not empty + template: deployment.yaml + set: + config: + defaultMode: + jobJsonOverrides: 0644 + jobJsonOverridesFiles: + - name: "test" + asserts: + - equal: + path: 'spec.template.spec.containers[0].volumeMounts[?(@.name=="rstudio-job-overrides-old")].mountPath' + value: "/mnt/job-json-overrides" + - equal: + path: 'spec.template.spec.containers[0].volumeMounts[?(@.name=="rstudio-job-overrides-old")].name' + value: "rstudio-job-overrides-old" + - equal: + path: 'spec.template.spec.volumes[?(@.name=="rstudio-job-overrides-old")].name' + value: "rstudio-job-overrides-old" + - equal: + path: 'spec.template.spec.volumes[?(@.name=="rstudio-job-overrides-old")].configMap.name' + value: "RELEASE-NAME-rstudio-workbench-overrides-old" + - equal: + path: 'spec.template.spec.volumes[?(@.name=="rstudio-job-overrides-old")].configMap.defaultMode' + value: 0644 + - it: should not specify a volumeMount and a volume for the old style jobJsonOverridesFiles if jobJsonOverridesFiles is not defined + template: deployment.yaml + set: + config: + defaultMode: + jobJsonOverrides: 0644 + jobJsonOverridesFiles: {} + asserts: + - notExists: + path: 'spec.template.spec.containers[0].volumeMounts[?(@.name=="rstudio-job-overrides-old")]' + - notExists: + path: 'spec.template.spec.volumes[?(@.name=="rstudio-job-overrides-old")]' + +# $useNewOverrides is defined as {- $useNewerOverrides := and (not (hasKey .Values.config.server "launcher.kubernetes.profiles.conf")) (not .Values.launcher.useTemplates) }} + - it: should specify a volumeMount and a volume for the new style jobJsonOverridesFiles if $useNewerOverrides is true + template: deployment.yaml + set: + config: + defaultMode: + jobJsonOverrides: 0644 + launcher: + useTemplates: false + asserts: + - equal: + path: 'spec.template.spec.containers[0].volumeMounts[?(@.name=="rstudio-job-overrides-new")].mountPath' + value: "/mnt/job-json-overrides-new" + - equal: + path: 'spec.template.spec.containers[0].volumeMounts[?(@.name=="rstudio-job-overrides-new")].name' + value: "rstudio-job-overrides-new" + - equal: + path: 'spec.template.spec.volumes[?(@.name=="rstudio-job-overrides-new")].name' + value: "rstudio-job-overrides-new" + - equal: + path: 'spec.template.spec.volumes[?(@.name=="rstudio-job-overrides-new")].configMap.name' + value: "RELEASE-NAME-rstudio-workbench-overrides-new" + - equal: + path: 'spec.template.spec.volumes[?(@.name=="rstudio-job-overrides-new")].configMap.defaultMode' + value: 0644 + - it: should not specify a volumeMount and a volume for the new style jobJsonOverridesFiles if $useNewerOverrides is false + template: deployment.yaml + set: + config: + server: + launcher.kubernetes.profiles.conf: + thing: "test" + launcher: + useTemplates: true + asserts: + - notExists: + path: 'spec.template.spec.containers[0].volumeMounts[?(@.name=="rstudio-job-overrides-new")]' + - notExists: + path: 'spec.template.spec.volumes[?(@.name=="rstudio-job-overrides-new")]' From 9478cb84513ca67b1a5686ec899e7353eea8deeb Mon Sep 17 00:00:00 2001 From: "Joshua C. Forest" Date: Wed, 4 Dec 2024 11:08:12 -0500 Subject: [PATCH 2/8] Add tests for launcher using templates --- .../tests/deployment_test.yaml | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/ci/rstudio-workbench/tests/deployment_test.yaml b/ci/rstudio-workbench/tests/deployment_test.yaml index 1764fb3a..a63bff9e 100644 --- a/ci/rstudio-workbench/tests/deployment_test.yaml +++ b/ci/rstudio-workbench/tests/deployment_test.yaml @@ -492,3 +492,73 @@ tests: path: 'spec.template.spec.containers[0].volumeMounts[?(@.name=="rstudio-job-overrides-new")]' - notExists: path: 'spec.template.spec.volumes[?(@.name=="rstudio-job-overrides-new")]' + - it: should specify 3 volumeMounts and a volume if Values.launcher.useTemplates is true + template: deployment.yaml + set: + config: + defaultMode: + server: 0644 + launcher: + useTemplates: true + asserts: + - contains: + path: 'spec.template.spec.containers[0].volumeMounts' + content: + name: "session-templates" + mountPath: "/var/lib/rstudio-launcher/Kubernetes/rstudio-library-templates-data.tpl" + subPath: "rstudio-library-templates-data.tpl" + any: true + - contains: + path: 'spec.template.spec.containers[0].volumeMounts' + content: + name: "session-templates" + mountPath: "/var/lib/rstudio-launcher/Kubernetes/job.tpl" + subPath: "job.tpl" + any: true + - contains: + path: 'spec.template.spec.containers[0].volumeMounts' + content: + name: "session-templates" + mountPath: "/var/lib/rstudio-launcher/Kubernetes/service.tpl" + subPath: "service.tpl" + any: true + - equal: + path: 'spec.template.spec.volumes[?(@.name=="session-templates")].name' + value: "session-templates" + - equal: + path: 'spec.template.spec.volumes[?(@.name=="session-templates")].configMap.name' + value: "RELEASE-NAME-rstudio-workbench-templates" + - equal: + path: 'spec.template.spec.volumes[?(@.name=="session-templates")].configMap.defaultMode' + value: 0644 + - it: should not specify any volumeMounts or a volume if Values.launcher.useTemplates is false + template: deployment.yaml + set: + launcher: + useTemplates: false + asserts: + - notContains: + path: 'spec.template.spec.containers[0].volumeMounts' + content: + name: "session-templates" + mountPath: "/var/lib/rstudio-launcher/Kubernetes/rstudio-library-templates-data.tpl" + subPath: "rstudio-library-templates-data.tpl" + any: true + - notContains: + path: 'spec.template.spec.containers[0].volumeMounts' + content: + name: "session-templates" + mountPath: "/var/lib/rstudio-launcher/Kubernetes/job.tpl" + subPath: "job.tpl" + any: true + - notContains: + path: 'spec.template.spec.containers[0].volumeMounts' + content: + name: "session-templates" + mountPath: "/var/lib/rstudio-launcher/Kubernetes/service.tpl" + subPath: "service.tpl" + any: true + - notExists: + path: 'spec.template.spec.volumes[?(@.name=="session-templates")]' + - notExists: + path: 'spec.template.spec.volumeMounts[?(@.name=="session-templates")]' From a01531c8e2cff1ff642e1ba86f0e3d66322b5eac Mon Sep 17 00:00:00 2001 From: "Joshua C. Forest" Date: Wed, 4 Dec 2024 11:29:02 -0500 Subject: [PATCH 3/8] Add tests for user defined volumemounts --- .../tests/deployment_test.yaml | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/ci/rstudio-workbench/tests/deployment_test.yaml b/ci/rstudio-workbench/tests/deployment_test.yaml index a63bff9e..0cf42921 100644 --- a/ci/rstudio-workbench/tests/deployment_test.yaml +++ b/ci/rstudio-workbench/tests/deployment_test.yaml @@ -562,3 +562,25 @@ tests: path: 'spec.template.spec.volumes[?(@.name=="session-templates")]' - notExists: path: 'spec.template.spec.volumeMounts[?(@.name=="session-templates")]' + - it: should have the pod volumemounts defined by the user + template: deployment.yaml + set: + pod: + volumeMounts: + - name: "test" + mountPath: "/mnt/test" + - name: "secondTest" + mountPath: "/mnt/secondTest" + asserts: + - contains: + path: 'spec.template.spec.containers[0].volumeMounts' + content: + name: "test" + mountPath: "/mnt/test" + any: true + - contains: + path: 'spec.template.spec.containers[0].volumeMounts' + content: + name: "secondTest" + mountPath: "/mnt/secondTest" + any: true From b59281f6de7fbb117076418c8730329dbf05000d Mon Sep 17 00:00:00 2001 From: "Joshua C. Forest" Date: Wed, 4 Dec 2024 11:43:18 -0500 Subject: [PATCH 4/8] Add resource limit and requests unit tests. Fix a logic bug where the limit key is set even when the resource limits are not disabled --- charts/rstudio-workbench/Chart.yaml | 2 +- charts/rstudio-workbench/NEWS.md | 4 +++ .../rstudio-workbench/templates/_helpers.tpl | 2 +- .../tests/deployment_test.yaml | 34 +++++++++++++++++++ 4 files changed, 40 insertions(+), 2 deletions(-) diff --git a/charts/rstudio-workbench/Chart.yaml b/charts/rstudio-workbench/Chart.yaml index c66cd382..98c874ac 100644 --- a/charts/rstudio-workbench/Chart.yaml +++ b/charts/rstudio-workbench/Chart.yaml @@ -1,6 +1,6 @@ name: rstudio-workbench description: Official Helm chart for Posit Workbench -version: 0.8.8 +version: 0.8.9 apiVersion: v2 appVersion: 2024.09.1 icon: https://rstudio.com/wp-content/uploads/2018/10/RStudio-Logo-Flat.png diff --git a/charts/rstudio-workbench/NEWS.md b/charts/rstudio-workbench/NEWS.md index adb6e4b4..d32aa42b 100644 --- a/charts/rstudio-workbench/NEWS.md +++ b/charts/rstudio-workbench/NEWS.md @@ -1,5 +1,9 @@ # Changelog +## 0.8.9 + +- Fix a logic bug where the resource limit key was set even if `resources.limits.enabled` is false + ## 0.8.8 - Bump Chronicle Agent to version 2024.11.0 diff --git a/charts/rstudio-workbench/templates/_helpers.tpl b/charts/rstudio-workbench/templates/_helpers.tpl index 29f9ffdd..57f3cf72 100644 --- a/charts/rstudio-workbench/templates/_helpers.tpl +++ b/charts/rstudio-workbench/templates/_helpers.tpl @@ -183,8 +183,8 @@ containers: cpu: "{{ .Values.resources.requests.cpu }}" ephemeral-storage: "{{ .Values.resources.requests.ephemeralStorage }}" {{- end }} - limits: {{- if .Values.resources.limits.enabled }} + limits: memory: "{{ .Values.resources.limits.memory }}" cpu: "{{ .Values.resources.limits.cpu }}" ephemeral-storage: "{{ .Values.resources.limits.ephemeralStorage }}" diff --git a/ci/rstudio-workbench/tests/deployment_test.yaml b/ci/rstudio-workbench/tests/deployment_test.yaml index 0cf42921..f4b2ef02 100644 --- a/ci/rstudio-workbench/tests/deployment_test.yaml +++ b/ci/rstudio-workbench/tests/deployment_test.yaml @@ -584,3 +584,37 @@ tests: name: "secondTest" mountPath: "/mnt/secondTest" any: true + - it: should set the resource requests if enabled + template: deployment.yaml + set: + resources: + requests: + enabled: true + cpu: "100m" + memory: "128Mi" + asserts: + - equal: + path: 'spec.template.spec.containers[0].resources.requests.cpu' + value: "100m" + - equal: + path: 'spec.template.spec.containers[0].resources.requests.memory' + value: "128Mi" + - notExists: + path: 'spec.template.spec.containers[0].resources.limits' + - it: should set the resource limits if enabled + template: deployment.yaml + set: + resources: + limits: + enabled: true + cpu: "1000m" + memory: "1024Mi" + asserts: + - equal: + path: 'spec.template.spec.containers[0].resources.limits.cpu' + value: "1000m" + - equal: + path: 'spec.template.spec.containers[0].resources.limits.memory' + value: "1024Mi" + - notExists: + path: 'spec.template.spec.containers[0].resources.requests' From 3ba60f5dde69a30e1f87ab4648301478b874aa29 Mon Sep 17 00:00:00 2001 From: GitHub Actions Date: Wed, 4 Dec 2024 16:52:02 +0000 Subject: [PATCH 5/8] Update helm-docs and README.md --- charts/rstudio-workbench/README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/charts/rstudio-workbench/README.md b/charts/rstudio-workbench/README.md index 4cbf7f3e..70385830 100644 --- a/charts/rstudio-workbench/README.md +++ b/charts/rstudio-workbench/README.md @@ -1,6 +1,6 @@ # Posit Workbench -![Version: 0.8.8](https://img.shields.io/badge/Version-0.8.8-informational?style=flat-square) ![AppVersion: 2024.09.1](https://img.shields.io/badge/AppVersion-2024.09.1-informational?style=flat-square) +![Version: 0.8.9](https://img.shields.io/badge/Version-0.8.9-informational?style=flat-square) ![AppVersion: 2024.09.1](https://img.shields.io/badge/AppVersion-2024.09.1-informational?style=flat-square) #### _Official Helm chart for Posit Workbench_ @@ -24,11 +24,11 @@ To ensure a stable production deployment: ## Installing the chart -To install the chart with the release name `my-release` at version 0.8.8: +To install the chart with the release name `my-release` at version 0.8.9: ```{.bash} helm repo add rstudio https://helm.rstudio.com -helm upgrade --install my-release rstudio/rstudio-workbench --version=0.8.8 +helm upgrade --install my-release rstudio/rstudio-workbench --version=0.8.9 ``` To explore other chart versions, look at: From 6bae13006f113e4cc4561b27ca6139da0441d111 Mon Sep 17 00:00:00 2001 From: "Joshua C. Forest" Date: Wed, 4 Dec 2024 12:54:54 -0500 Subject: [PATCH 6/8] add tests for the startup/liveness/readiness probes --- .../tests/deployment_test.yaml | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/ci/rstudio-workbench/tests/deployment_test.yaml b/ci/rstudio-workbench/tests/deployment_test.yaml index f4b2ef02..6cdce6af 100644 --- a/ci/rstudio-workbench/tests/deployment_test.yaml +++ b/ci/rstudio-workbench/tests/deployment_test.yaml @@ -618,3 +618,75 @@ tests: value: "1024Mi" - notExists: path: 'spec.template.spec.containers[0].resources.requests' + - it: should configure the livenessProbe if values.livenessProbe.enabled is true + template: deployment.yaml + set: + livenessProbe: + enabled: true + initialDelaySeconds: 10 + periodSeconds: 20 + asserts: + - equal: + path: 'spec.template.spec.containers[0].livenessProbe.initialDelaySeconds' + value: 10 + - equal: + path: 'spec.template.spec.containers[0].livenessProbe.periodSeconds' + value: 20 + - it: should not configure the livenessProbe if values.livenessProbe.enabled is false + template: deployment.yaml + set: + livenessProbe: + enabled: false + initialDelaySeconds: 10 + periodSeconds: 20 + asserts: + - notExists: + path: 'spec.template.spec.containers[0].livenessProbe' + - it: should configure the startupProbe if values.startupsProbe.enabled is true + template: deployment.yaml + set: + startupProbe: + enabled: true + initialDelaySeconds: 10 + periodSeconds: 20 + asserts: + - equal: + path: 'spec.template.spec.containers[0].startupProbe.initialDelaySeconds' + value: 10 + - equal: + path: 'spec.template.spec.containers[0].startupProbe.periodSeconds' + value: 20 + - it: should not configure the startupProbe if values.startupProbe.enabled is false + template: deployment.yaml + set: + startupProbe: + enabled: false + initialDelaySeconds: 10 + periodSeconds: 20 + asserts: + - notExists: + path: 'spec.template.spec.containers[0].startupProbe' + - it: should configure the readinessProbe if values.readinessProbe.enabled is true + template: deployment.yaml + set: + readinessProbe: + enabled: true + initialDelaySeconds: 10 + periodSeconds: 20 + asserts: + - equal: + path: 'spec.template.spec.containers[0].readinessProbe.initialDelaySeconds' + value: 10 + - equal: + path: 'spec.template.spec.containers[0].readinessProbe.periodSeconds' + value: 20 + - it: should not configure the readinessProbe if values.readinessProbe.enabled is false + template: deployment.yaml + set: + readinessProbe: + enabled: false + initialDelaySeconds: 10 + periodSeconds: 20 + asserts: + - notExists: + path: 'spec.template.spec.containers[0].readinessProbe' From dd3a2facedb3fe4c48940ea4a30b105519bf8013 Mon Sep 17 00:00:00 2001 From: "Joshua C. Forest" Date: Wed, 4 Dec 2024 13:00:43 -0500 Subject: [PATCH 7/8] Add test for sidecar definitiions --- ci/rstudio-workbench/tests/deployment_test.yaml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/ci/rstudio-workbench/tests/deployment_test.yaml b/ci/rstudio-workbench/tests/deployment_test.yaml index 6cdce6af..b7435ec3 100644 --- a/ci/rstudio-workbench/tests/deployment_test.yaml +++ b/ci/rstudio-workbench/tests/deployment_test.yaml @@ -690,3 +690,13 @@ tests: asserts: - notExists: path: 'spec.template.spec.containers[0].readinessProbe' + - it: should create a sidecar container if pod.sidecar is defined + template: deployment.yaml + set: + pod: + sidecar: + - name: "sidecarTest" + image: "test" + asserts: + - exists: + path: 'spec.template.spec.containers[?(@.name=="sidecarTest")]' From 6ba0a1a5fe5345f5b3a70e84b1bf5bf8b4e8805d Mon Sep 17 00:00:00 2001 From: "Joshua C. Forest" Date: Wed, 4 Dec 2024 16:24:20 -0500 Subject: [PATCH 8/8] somehow missed launcher.enabled way back, adding tests for that --- .../tests/deployment_test.yaml | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/ci/rstudio-workbench/tests/deployment_test.yaml b/ci/rstudio-workbench/tests/deployment_test.yaml index b7435ec3..c0c9b984 100644 --- a/ci/rstudio-workbench/tests/deployment_test.yaml +++ b/ci/rstudio-workbench/tests/deployment_test.yaml @@ -299,6 +299,34 @@ tests: path: 'spec.template.spec.containers[0].volumeMounts[?(@.name=="rstudio-user")]' - notExists: path: 'spec.template.spec.volumes[?(@.name=="rstudio-user")]' + - it: should specify a volumeMount and a volume for launcher if launcher.enabled is true + template: deployment.yaml + set: + config: + defaultMode: + launcher: 0755 + launcher: + enabled: true + asserts: + - equal: + path: 'spec.template.spec.containers[0].volumeMounts[?(@.name=="rstudio-launcher-startup")].mountPath' + value: "/startup/launcher" + - equal: + path: 'spec.template.spec.volumes[?(@.name=="rstudio-launcher-startup")].configMap.name' + value: "RELEASE-NAME-rstudio-workbench-start-launcher" + - equal: + path: 'spec.template.spec.volumes[?(@.name=="rstudio-launcher-startup")].configMap.defaultMode' + value: 0755 + - it: should not specify a volumeMount and a volume for launcher if launcher.enabled is false + template: deployment.yaml + set: + launcher: + enabled: false + asserts: + - notExists: + path: 'spec.template.spec.containers[0].volumeMounts[?(@.name=="rstudio-launcher-startup")]' + - notExists: + path: 'spec.template.spec.volumes[?(@.name=="rstudio-launcher-startup")]' - it: should specify a volumeMount and a volume for startupUserProvisioning if config.startupUserProvisioning is defined and not empty template: deployment.yaml set: