From bac234b5945a0cd6f3ca557d336bfe7ea6a892a0 Mon Sep 17 00:00:00 2001 From: Thorarinn Sigurdsson Date: Fri, 31 May 2024 12:07:38 +0200 Subject: [PATCH] improvement(k8s): allow volume mounts in runners (#6112) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If a `kubernetes-pod` Run or Test includes `volumeMounts` on the first container in its `podSpec`, we now include this in the runner pod. Previously, we omitted the `volumeMounts` field from the first container in the (optional) user-supplied `podSpec` field. This is because the relevant code assumed `container` Runs/Tests—it's now appropriate for `container` and `kubernetes-pod` Runs/Tests. --- core/src/plugins/kubernetes/run.ts | 6 +- core/test/integ/src/plugins/kubernetes/run.ts | 77 ++++++++++++++++--- 2 files changed, 68 insertions(+), 15 deletions(-) diff --git a/core/src/plugins/kubernetes/run.ts b/core/src/plugins/kubernetes/run.ts index 6a45526933..1a329ec864 100644 --- a/core/src/plugins/kubernetes/run.ts +++ b/core/src/plugins/kubernetes/run.ts @@ -303,8 +303,6 @@ export async function prepareRunPodSpec({ name: mainContainerName, image, env, - // TODO: consider supporting volume mounts in ad-hoc runs (would need specific logic and testing) - volumeMounts: [], }, ] @@ -317,7 +315,9 @@ export async function prepareRunPodSpec({ imagePullSecrets, } - if (volumes) { + // This logic is only relevant for `container` Runs and Tests, which need to support mounting `persistentvolumeclaim` + // and `configmap` actions (which are only supported for `container` actions, and are currently discouraged). + if (volumes && volumes.length && action.type === "container") { configureVolumes(action, preparedPodSpec, volumes) } diff --git a/core/test/integ/src/plugins/kubernetes/run.ts b/core/test/integ/src/plugins/kubernetes/run.ts index 7cdd579dac..df27450f7a 100644 --- a/core/test/integ/src/plugins/kubernetes/run.ts +++ b/core/test/integ/src/plugins/kubernetes/run.ts @@ -688,12 +688,10 @@ describe("kubernetes Pod runner functions", () => { value: helmAction.versionString(), }, ], - volumeMounts: [], command: ["echo", "foo"], }, ], imagePullSecrets: [], - volumes: [], }) }) @@ -743,12 +741,10 @@ describe("kubernetes Pod runner functions", () => { value: helmAction.versionString(), }, ], - volumeMounts: [], command: ["echo", "foo"], }, ], imagePullSecrets: [], - volumes: [], }) }) @@ -799,16 +795,79 @@ describe("kubernetes Pod runner functions", () => { value: helmAction.versionString(), }, ], - volumeMounts: [], command: ["echo", "foo"], }, ], imagePullSecrets: [], - volumes: [], shareProcessNamespace: true, }) }) + it("should include volume mounts for containers in the generated pod spec", async () => { + const volumeMounts = [ + { + name: "some-volume", + mountPath: "/some-volume", + }, + ] + const helmContainerWithVolumeMounts = { + ...helmContainer, + volumeMounts, + } + const generatedPodSpec = await prepareRunPodSpec({ + podSpec: undefined, + getArtifacts: false, + api: helmApi, + provider: helmProvider, + log: helmLog, + action: helmAction, + args: ["sh", "-c"], + command: ["echo", "foo"], + + envVars: {}, + resources, + description: "Helm module", + mainContainerName: "main", + image: "foo", + container: helmContainerWithVolumeMounts, + namespace: helmNamespace, + // Note: We're not passing the `volumes` param here, since that's for `container` Runs/Tests. + // This test case is intended for `kubernetes-pod` Runs and Tests. + }) + + expect(pruneEmpty(generatedPodSpec)).to.eql({ + containers: [ + { + name: "main", + image: "foo", + imagePullPolicy: "IfNotPresent", + args: ["sh", "-c"], + ports: [ + { + name: "http", + containerPort: 80, + protocol: "TCP", + }, + ], + resources: getResourceRequirements(resources), + env: [ + { + name: "GARDEN_ACTION_VERSION", + value: helmAction.versionString(), + }, + { + name: "GARDEN_MODULE_VERSION", + value: helmAction.versionString(), + }, + ], + volumeMounts, // <------ + command: ["echo", "foo"], + }, + ], + imagePullSecrets: [], + }) + }) + it("should apply security context fields to the main container when provided", async () => { const generatedPodSpec = await prepareRunPodSpec({ podSpec: undefined, @@ -858,7 +917,6 @@ describe("kubernetes Pod runner functions", () => { value: helmAction.versionString(), }, ], - volumeMounts: [], command: ["echo", "foo"], securityContext: { privileged: true, @@ -870,7 +928,6 @@ describe("kubernetes Pod runner functions", () => { }, ], imagePullSecrets: [], - volumes: [], }) }) @@ -944,12 +1001,10 @@ describe("kubernetes Pod runner functions", () => { value: helmAction.versionString(), }, ], - volumeMounts: [], command: ["echo", "foo"], }, ], imagePullSecrets: [], - volumes: [], }) }) @@ -1024,12 +1079,10 @@ describe("kubernetes Pod runner functions", () => { value: helmAction.versionString(), }, ], - volumeMounts: [], command: ["echo", "foo"], }, ], imagePullSecrets: [], - volumes: [], }) }) })