Skip to content

Commit

Permalink
improvement(k8s): allow volume mounts in runners
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
thsig committed May 31, 2024
1 parent e57dbd1 commit e639ca4
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 15 deletions.
6 changes: 3 additions & 3 deletions core/src/plugins/kubernetes/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: [],
},
]

Expand All @@ -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)
}

Expand Down
77 changes: 65 additions & 12 deletions core/test/integ/src/plugins/kubernetes/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -688,12 +688,10 @@ describe("kubernetes Pod runner functions", () => {
value: helmAction.versionString(),
},
],
volumeMounts: [],
command: ["echo", "foo"],
},
],
imagePullSecrets: [],
volumes: [],
})
})

Expand Down Expand Up @@ -743,12 +741,10 @@ describe("kubernetes Pod runner functions", () => {
value: helmAction.versionString(),
},
],
volumeMounts: [],
command: ["echo", "foo"],
},
],
imagePullSecrets: [],
volumes: [],
})
})

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -858,7 +917,6 @@ describe("kubernetes Pod runner functions", () => {
value: helmAction.versionString(),
},
],
volumeMounts: [],
command: ["echo", "foo"],
securityContext: {
privileged: true,
Expand All @@ -870,7 +928,6 @@ describe("kubernetes Pod runner functions", () => {
},
],
imagePullSecrets: [],
volumes: [],
})
})

Expand Down Expand Up @@ -944,12 +1001,10 @@ describe("kubernetes Pod runner functions", () => {
value: helmAction.versionString(),
},
],
volumeMounts: [],
command: ["echo", "foo"],
},
],
imagePullSecrets: [],
volumes: [],
})
})

Expand Down Expand Up @@ -1024,12 +1079,10 @@ describe("kubernetes Pod runner functions", () => {
value: helmAction.versionString(),
},
],
volumeMounts: [],
command: ["echo", "foo"],
},
],
imagePullSecrets: [],
volumes: [],
})
})
})
Expand Down

0 comments on commit e639ca4

Please sign in to comment.