fix(k8s): omit probes in runner pod spec

We now no longer include liveness or readiness probes on the pod spec
used when running e.g. tests or tasks for `kubernetes` or `helm`
modules.

In certain situations, these probes could result in test pods being
evicted before the test suite was able to finish. This is fixed here.

core/src/commands/base.ts

@@ -82,9 +82,9 @@ export abstract class Command<T extends Parameters = {}, U extends Parameters =
   hidden: boolean = false
   noProject: boolean = false
   protected: boolean = false
-  workflows: boolean = false // Set to true to whitelist for executing in workflow steps
-  streamEvents: boolean = false // Set to true to whitelist for streaming events
-  streamLogEntries: boolean = false // Set to true to whitelist for streaming log entries
+  workflows: boolean = false // Set to true to allow the command in workflow steps
+  streamEvents: boolean = false // Set to true to stream events for the command
+  streamLogEntries: boolean = false // Set to true to stream log entries for the command
   server: GardenServer | undefined = undefined
 
   constructor(private parent?: CommandGroup) {

core/src/config/workflow.ts

@@ -417,7 +417,7 @@ export function resolveWorkflowConfig(garden: Garden, config: WorkflowConfig) {
 }
 
 /**
- * Get all commands whitelisted for workflows
+ * Get all commands that are allowed in workflows
  */
 function getStepCommands() {
   return getCoreCommands()

core/src/plugins/kubernetes/config.ts

@@ -36,7 +36,7 @@ import { baseTaskSpecSchema, BaseTaskSpec, cacheResultSchema } from "../../confi
 import { baseTestSpecSchema, BaseTestSpec } from "../../config/test"
 import { ArtifactSpec } from "../../config/validation"
 import { V1Toleration } from "@kubernetes/client-node"
-import { runPodSpecWhitelist } from "./run"
+import { runPodSpecIncludeFields } from "./run"
 
 export const DEFAULT_KANIKO_IMAGE = "gcr.io/kaniko-project/executor:v1.6.0-debug"
 export interface ProviderSecretRef {
@@ -708,7 +708,7 @@ export const hotReloadArgsSchema = () =>
     .description("If specified, overrides the arguments for the main container when running in hot-reload mode.")
     .example(["nodemon", "my-server.js"])
 
-const runPodSpecWhitelistDescription = runPodSpecWhitelist.map((f) => `* \`${f}\``).join("\n")
+const runPodSpecWhitelistDescription = runPodSpecIncludeFields.map((f) => `* \`${f}\``).join("\n")
 
 export const kubernetesTaskSchema = () =>
   baseTaskSpecSchema()

core/src/plugins/kubernetes/helm/config.ts

@@ -35,7 +35,7 @@ import {
   hotReloadArgsSchema,
 } from "../config"
 import { posix } from "path"
-import { runPodSpecWhitelist } from "../run"
+import { runPodSpecIncludeFields } from "../run"
 import { omit } from "lodash"
 import { kubernetesDevModeSchema, KubernetesDevModeSpec } from "../dev-mode"
 
@@ -99,7 +99,7 @@ const helmServiceResourceSchema = () =>
     hotReloadArgs: hotReloadArgsSchema(),
   })
 
-const runPodSpecWhitelistDescription = runPodSpecWhitelist.map((f) => `* \`${f}\``).join("\n")
+const runPodSpecWhitelistDescription = runPodSpecIncludeFields.map((f) => `* \`${f}\``).join("\n")
 
 const helmTaskSchema = () =>
   kubernetesTaskSchema().keys({

core/src/plugins/kubernetes/run.ts

@@ -9,6 +9,7 @@
 import { resolve } from "path"
 import tar from "tar"
 import tmp from "tmp-promise"
+import { cloneDeep, omit, pick } from "lodash"
 import { V1PodSpec, V1Pod, V1Container } from "@kubernetes/client-node"
 import { RunResult } from "../../types/plugin/base"
 import { GardenModule } from "../../types/module"
@@ -38,7 +39,6 @@ import { prepareImagePullSecrets } from "./secrets"
 import { configureVolumes } from "./container/deployment"
 import { PluginContext } from "../../plugin-context"
 import { waitForResources, ResourceStatus } from "./status/status"
-import { cloneDeep, pick } from "lodash"
 import { RuntimeContext } from "../../runtime-context"
 import { getResourceRequirements } from "./container/util"
 
@@ -49,10 +49,13 @@ const defaultTimeout = 600
  * When a `podSpec` is passed to `runAndCopy`, only these fields will be used for the runner's pod spec
  * (and, in some cases, overridden/populated in `runAndCopy`).
  *
+ * Additionally, the keys in `runContainerExcludeFields` below will be omitted from the container used in the
+ * runner's pod spec.
+ *
  * See: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.19/#podspec-v1-core
  */
-export const runPodSpecWhitelist: (keyof V1PodSpec)[] = [
-  // "activeDeadlineSeconds", // <-- for clarity, we leave the non-whitelisted fields here commented out.
+export const runPodSpecIncludeFields: (keyof V1PodSpec)[] = [
+  // "activeDeadlineSeconds", // <-- for clarity, we leave the excluded fields here commented out.
   "affinity",
   "automountServiceAccountToken",
   "containers",
@@ -88,6 +91,8 @@ export const runPodSpecWhitelist: (keyof V1PodSpec)[] = [
   "volumes",
 ]
 
+export const runContainerExcludeFields: (keyof V1Container)[] = ["readinessProbe", "livenessProbe"]
+
 export async function runAndCopy({
   ctx,
   log,
@@ -249,7 +254,7 @@ export async function prepareRunPodSpec({
 
   const containers: V1Container[] = [
     {
-      ...(container || {}),
+      ...omit(container || {}, runContainerExcludeFields),
       ...resourceRequirements,
       // We always override the following attributes
       name: mainContainerName,
@@ -263,7 +268,7 @@ export async function prepareRunPodSpec({
   const imagePullSecrets = await prepareImagePullSecrets({ api, provider, namespace, log })
 
   const preparedPodSpec = {
-    ...pick(podSpec || {}, runPodSpecWhitelist),
+    ...pick(podSpec || {}, runPodSpecIncludeFields),
     containers,
     imagePullSecrets,
   }

core/test/integ/src/plugins/kubernetes/run.ts

@@ -43,7 +43,7 @@ import { buildHelmModules, getHelmTestGarden } from "./helm/common"
 import { getBaseModule, getChartResources } from "../../../../../src/plugins/kubernetes/helm/common"
 import { getModuleNamespace } from "../../../../../src/plugins/kubernetes/namespace"
 import { GardenModule } from "../../../../../src/types/module"
-import { V1Container, V1DaemonSet, V1Deployment, V1Pod, V1StatefulSet } from "@kubernetes/client-node"
+import { V1Container, V1DaemonSet, V1Deployment, V1Pod, V1PodSpec, V1StatefulSet } from "@kubernetes/client-node"
 import { getResourceRequirements } from "../../../../../src/plugins/kubernetes/container/util"
 import { ContainerResourcesSpec } from "../../../../../src/plugins/container/config"
 
@@ -784,10 +784,10 @@ describe("kubernetes Pod runner functions", () => {
       })
     })
 
-    it("should include only whitelisted pod spec fields in the generated pod spec", async () => {
+    it("should include only the right pod spec fields in the generated pod spec", async () => {
       const podSpec = getResourcePodSpec(helmTarget)
       expect(podSpec).to.eql({
-        // This field is *not* whitelisted in `runPodSpecWhitelist`, so it shouldn't appear in the
+        // This field is *not* included in `runPodSpecIncludeFields`, so it shouldn't appear in the
         // generated pod spec below.
         terminationGracePeriodSeconds: 60,
         containers: [
@@ -806,7 +806,7 @@ describe("kubernetes Pod runner functions", () => {
             resources: {},
           },
         ],
-        // This field is whitelisted in `runPodSpecWhitelist`, so it *should* appear in the generated
+        // This field is included in `runPodSpecIncludeFields`, so it *should* appear in the generated
         // pod spec below.
         shareProcessNamespace: true,
       })
@@ -831,9 +831,9 @@ describe("kubernetes Pod runner functions", () => {
       })
 
       expect(generatedPodSpec).to.eql({
-        // `shareProcessNamespace` is not blacklisted, so it should be propagated to here.
+        // `shareProcessNamespace` is not excluded, so it should be propagated to here.
         shareProcessNamespace: true,
-        // `terminationGracePeriodSeconds` *is* blacklisted, so it should not appear here.
+        // `terminationGracePeriodSeconds` *is* excluded, so it should not appear here.
         containers: [
           {
             name: "main",
@@ -857,6 +857,78 @@ describe("kubernetes Pod runner functions", () => {
         volumes: [],
       })
     })
+
+    it("should omit excluded container fields from the generated pod spec", async () => {
+      const podSpec = getResourcePodSpec(helmTarget)
+      const probe = {
+        initialDelaySeconds: 90,
+        periodSeconds: 10,
+        timeoutSeconds: 3,
+        successThreshold: 1,
+        failureThreshold: 30,
+        exec: {
+          command: ["echo", "ok"],
+        },
+      }
+      const podSpecWithProbes: V1PodSpec = {
+        ...podSpec,
+        containers: [
+          {
+            ...podSpec!.containers[0]!,
+            // These two fields are excluded, so they should be omitted from the generated pod spec.
+            livenessProbe: probe,
+            readinessProbe: probe,
+          },
+        ],
+      }
+      const generatedPodSpec = await prepareRunPodSpec({
+        podSpec: podSpecWithProbes, // <------
+        getArtifacts: false,
+        api: helmApi,
+        provider: helmProvider,
+        log: helmLog,
+        module: helmModule,
+        args: ["sh", "-c"],
+        command: ["echo", "foo"],
+        runtimeContext: { envVars: {}, dependencies: [] },
+        envVars: {},
+        description: "Helm module",
+        errorMetadata: {},
+        mainContainerName: "main",
+        image: "foo",
+        container: helmContainer,
+        namespace: helmNamespace,
+        volumes: [],
+      })
+
+      expect(generatedPodSpec).to.eql({
+        // `shareProcessNamespace` is not excluded, so it should be propagated to here.
+        shareProcessNamespace: true,
+        // `terminationGracePeriodSeconds` *is* excluded, so it should not appear here.
+        containers: [
+          {
+            name: "main",
+            image: "foo",
+            imagePullPolicy: "IfNotPresent",
+            args: ["sh", "-c"],
+            ports: [
+              {
+                name: "http",
+                containerPort: 80,
+                protocol: "TCP",
+              },
+            ],
+            // We expect `livenessProbe` and `readinessProbe` to be omitted here.
+            resources: {},
+            env: [],
+            volumeMounts: [],
+            command: ["echo", "foo"],
+          },
+        ],
+        imagePullSecrets: [],
+        volumes: [],
+      })
+    })
   })
 
   describe("runAndCopy", () => {