fix(core): fix test dependencies for dev command

This fixes a minor regression introduced when we fixed the task graph's
batching algorithm (see commit `5625e79d`).

Here, we also add an optional `devMode` field to service statuses
(currently implemented/used by `container`, `kubernetes` and `helm`
services), which will eventually enable us to start `garden dev`
or `garden deploy --dev` without rebuilding dev-mode modules.

This is done in preparation for planned changes/improvements in how the
task graph works (which will introduce more dynamic dependency
resolution, enabling us to skip unnecessary tasks in many situations).

core/src/commands/dev.ts

@@ -300,7 +300,7 @@ export async function getDevCommandWatchTasks({
   })
 
   if (!skipTests) {
-    const testModules: GardenModule[] = await updatedGraph.withDependantModules([module])
+    const testModules: GardenModule[] = updatedGraph.withDependantModules([module])
     tasks.push(
       ...flatten(
         await Bluebird.map(testModules, (m) =>
@@ -310,6 +310,7 @@ export async function getDevCommandWatchTasks({
             module: m,
             graph: updatedGraph,
             filterNames: testNames,
+            fromWatch: true,
             devModeServiceNames,
             hotReloadServiceNames,
           })

core/src/commands/test.ts

@@ -171,7 +171,7 @@ export class TestCommand extends Command<Args, Opts> {
       initialTasks,
       watch: opts.watch,
       changeHandler: async (updatedGraph, module) => {
-        const modulesToProcess = await updatedGraph.withDependantModules([module])
+        const modulesToProcess = updatedGraph.withDependantModules([module])
         return flatten(
           await Bluebird.map(modulesToProcess, (m) =>
             getTestTasks({
@@ -182,6 +182,7 @@ export class TestCommand extends Command<Args, Opts> {
               filterNames,
               force,
               forceBuild,
+              fromWatch: true,
               devModeServiceNames: [],
               hotReloadServiceNames: [],
             })

core/src/plugins/kubernetes/container/status.ts

@@ -55,7 +55,13 @@ export async function getContainerServiceStatus({
     enableHotReload: hotReload,
     blueGreen: provider.config.deploymentStrategy === "blue-green",
   })
-  const { state, remoteResources } = await compareDeployedResources(k8sCtx, api, namespace, manifests, log)
+  const { state, remoteResources, deployedWithDevMode, deployedWithHotReloading } = await compareDeployedResources(
+    k8sCtx,
+    api,
+    namespace,
+    manifests,
+    log
+  )
   const ingresses = await getIngresses(service, api, provider)
 
   const forwardablePorts: ForwardablePort[] = service.spec.ports
@@ -78,6 +84,7 @@ export async function getContainerServiceStatus({
     namespaceStatuses: [namespaceStatus],
     version: state === "ready" ? service.version : undefined,
     detail: { remoteResources, workload },
+    devMode: deployedWithDevMode || deployedWithHotReloading,
   }
 
   if (state === "ready" && devMode) {

core/src/plugins/kubernetes/dev-mode.ts

@@ -34,6 +34,7 @@ import {
 } from "./mutagen"
 import { joi, joiIdentifier } from "../../config/common"
 import { KubernetesPluginContext, KubernetesProvider } from "./config"
+import { isConfiguredForDevMode } from "./status/status"
 
 const syncUtilImageName = "gardendev/k8s-sync:0.1.1"
 
@@ -188,7 +189,7 @@ export async function startDevModeSync({
 
   return mutagenConfigLock.acquire("start-sync", async () => {
     // Validate the target
-    if (target.metadata.annotations?.[gardenAnnotationKey("dev-mode")] !== "true") {
+    if (!isConfiguredForDevMode(target)) {
       throw new ConfigurationError(`Resource ${resourceName} is not deployed in dev mode`, {
         target,
       })

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

@@ -18,7 +18,7 @@ import { KubernetesServerResource } from "../types"
 import { getModuleNamespace, getModuleNamespaceStatus } from "../namespace"
 import { getServiceResource, getServiceResourceSpec } from "../util"
 import { startDevModeSync } from "../dev-mode"
-import { gardenAnnotationKey } from "../../../util/string"
+import { isConfiguredForDevMode } from "../status/status"
 
 const helmStatusMap: { [status: string]: ServiceState } = {
   unknown: "unknown",
@@ -48,6 +48,7 @@ export async function getServiceStatus({
 
   const detail: HelmStatusDetail = {}
   let state: ServiceState
+  let helmStatus: ServiceStatus
 
   const namespaceStatus = await getModuleNamespaceStatus({
     ctx: k8sCtx,
@@ -56,9 +57,12 @@ export async function getServiceStatus({
     provider: k8sCtx.provider,
   })
 
+  let deployedWithDevModeOrHotReloading: boolean | undefined
+
   try {
-    const helmStatus = await getReleaseStatus({ ctx: k8sCtx, service, releaseName, log, devMode, hotReload })
+    helmStatus = await getReleaseStatus({ ctx: k8sCtx, service, releaseName, log, devMode, hotReload })
     state = helmStatus.state
+    deployedWithDevModeOrHotReloading = helmStatus.devMode
   } catch (err) {
     state = "missing"
   }
@@ -84,7 +88,7 @@ export async function getServiceStatus({
 
       // Make sure we don't fail if the service isn't actually properly configured (we don't want to throw in the
       // status handler, generally)
-      if (target.metadata.annotations?.[gardenAnnotationKey("dev-mode")] === "true") {
+      if (isConfiguredForDevMode(target)) {
         const namespace =
           target.metadata.namespace ||
           (await getModuleNamespace({
@@ -115,6 +119,7 @@ export async function getServiceStatus({
     state,
     version: state === "ready" ? service.version : undefined,
     detail,
+    devMode: deployedWithDevModeOrHotReloading,
     namespaceStatuses: [namespaceStatus],
   }
 }
@@ -176,6 +181,9 @@ export async function getReleaseStatus({
     let state = helmStatusMap[res.info.status] || "unknown"
     let values = {}
 
+    let devModeEnabled = false
+    let hotReloadEnabled = false
+
     if (state === "ready") {
       // Make sure the right version is deployed
       values = JSON.parse(
@@ -187,8 +195,8 @@ export async function getReleaseStatus({
         })
       )
       const deployedVersion = values[".garden"] && values[".garden"].version
-      const devModeEnabled = values[".garden"] && values[".garden"].devMode === true
-      const hotReloadEnabled = values[".garden"] && values[".garden"].hotReload === true
+      devModeEnabled = values[".garden"] && values[".garden"].devMode === true
+      hotReloadEnabled = values[".garden"] && values[".garden"].hotReload === true
 
       if (
         (devMode && !devModeEnabled) ||
@@ -203,6 +211,7 @@ export async function getReleaseStatus({
     return {
       state,
       detail: { ...res, values },
+      devMode: devModeEnabled || hotReloadEnabled,
     }
   } catch (err) {
     if (err.message.includes("release: not found")) {

core/src/plugins/kubernetes/hot-reload/hot-reload.ts

@@ -25,6 +25,7 @@ import { getManifests } from "../kubernetes-module/common"
 import { KubernetesModule, KubernetesService } from "../kubernetes-module/config"
 import { getHotReloadSpec, syncToService } from "./helpers"
 import { GardenModule } from "../../../types/module"
+import { isConfiguredForHotReloading } from "../status/status"
 
 export type HotReloadableResource = KubernetesWorkload | KubernetesPod
 export type HotReloadableKind = "Deployment" | "DaemonSet" | "StatefulSet"
@@ -142,7 +143,7 @@ export async function hotReloadContainer({
     },
   })
 
-  const list = res.items.filter((r) => r.metadata.annotations![gardenAnnotationKey("hot-reload")] === "true")
+  const list = res.items.filter((r) => isConfiguredForHotReloading(r))
 
   if (list.length === 0) {
     throw new RuntimeError(`Unable to find deployed instance of service ${service.name} with hot-reloading enabled`, {

core/src/plugins/kubernetes/kubernetes-module/handlers.ts

@@ -27,7 +27,7 @@ import { apply, deleteObjectsBySelector, KUBECTL_DEFAULT_TIMEOUT } from "../kube
 import { streamK8sLogs } from "../logs"
 import { getModuleNamespace, getModuleNamespaceStatus } from "../namespace"
 import { getForwardablePorts, getPortForwardHandler, killPortForwards } from "../port-forward"
-import { compareDeployedResources, waitForResources } from "../status/status"
+import { compareDeployedResources, isConfiguredForDevMode, waitForResources } from "../status/status"
 import { getTaskResult } from "../task-results"
 import { getTestResult } from "../test-results"
 import { BaseResource, KubernetesResource, KubernetesServerResource } from "../types"
@@ -91,7 +91,13 @@ export async function getKubernetesServiceStatus({
     manifests,
   })
 
-  let { state, remoteResources } = await compareDeployedResources(k8sCtx, api, namespace, prepareResult.manifests, log)
+  let { state, remoteResources, deployedWithDevMode, deployedWithHotReloading } = await compareDeployedResources(
+    k8sCtx,
+    api,
+    namespace,
+    prepareResult.manifests,
+    log
+  )
 
   const forwardablePorts = getForwardablePorts(remoteResources, service)
 
@@ -107,7 +113,7 @@ export async function getKubernetesServiceStatus({
       resourceSpec: serviceResourceSpec,
     })
 
-    if (target.metadata.annotations?.[gardenAnnotationKey("dev-mode")] === "true") {
+    if (isConfiguredForDevMode(target)) {
       await startDevModeSync({
         ctx,
         log,
@@ -128,6 +134,7 @@ export async function getKubernetesServiceStatus({
     state,
     version: state === "ready" ? service.version : undefined,
     detail: { remoteResources },
+    devMode: deployedWithDevMode || deployedWithHotReloading,
     namespaceStatuses: [namespaceStatus],
   }
 }

core/src/plugins/kubernetes/status/status.ts

@@ -31,6 +31,7 @@ import { getPods, hashManifest } from "../util"
 import { checkWorkloadStatus } from "./workload"
 import { checkWorkloadPodStatus } from "./pod"
 import { deline, gardenAnnotationKey, stableStringify } from "../../../util/string"
+import { HotReloadableResource } from "../hot-reload/hot-reload"
 
 export interface ResourceStatus<T = BaseResource> {
   state: ServiceState
@@ -266,6 +267,8 @@ export async function waitForResources({
 interface ComparisonResult {
   state: ServiceState
   remoteResources: KubernetesResource[]
+  deployedWithDevMode: boolean
+  deployedWithHotReloading: boolean
 }
 
 /**
@@ -290,6 +293,8 @@ export async function compareDeployedResources(
   const result: ComparisonResult = {
     state: "unknown",
     remoteResources: <KubernetesResource[]>deployedResources.filter((o) => o !== null),
+    deployedWithDevMode: false,
+    deployedWithHotReloading: false,
   }
 
   const logDescription = (resource: KubernetesResource) => `${resource.kind}/${resource.metadata.name}`
@@ -349,6 +354,15 @@ export async function compareDeployedResources(
       delete manifest.metadata.annotations[gardenAnnotationKey("manifest-hash")]
     }
 
+    if (manifest.kind === "DaemonSet" || manifest.kind === "Deployment" || manifest.kind === "StatefulSet") {
+      if (isConfiguredForDevMode(<HotReloadableResource>manifest)) {
+        result.deployedWithDevMode = true
+      }
+      if (isConfiguredForHotReloading(<HotReloadableResource>manifest)) {
+        result.deployedWithHotReloading = true
+      }
+    }
+
     // Start by checking for "last applied configuration" annotations and comparing against those.
     // This can be more accurate than comparing against resolved resources.
     if (deployedResource.metadata && deployedResource.metadata.annotations) {
@@ -390,6 +404,9 @@ export async function compareDeployedResources(
     // NOTE: this approach won't fly in the long run, but hopefully we can climb out of this mess when
     //       `kubectl diff` is ready, or server-side apply/diff is ready
     if (manifest.kind === "DaemonSet" || manifest.kind === "Deployment" || manifest.kind === "StatefulSet") {
+      // NOTE: this approach won't fly in the long run, but hopefully we can climb out of this mess when
+      //       `kubectl diff` is ready, or server-side apply/diff is ready
+
       // handle properties that are omitted in the response because they have the default value
       // (another design issue in the K8s API)
       if (manifest.spec.minReadySeconds === 0) {
@@ -423,6 +440,14 @@ export async function compareDeployedResources(
   return result
 }
 
+export function isConfiguredForDevMode(resource: HotReloadableResource): boolean {
+  return resource.metadata.annotations?.[gardenAnnotationKey("dev-mode")] === "true"
+}
+
+export function isConfiguredForHotReloading(resource: HotReloadableResource): boolean {
+  return resource.metadata.annotations?.[gardenAnnotationKey("hot-reload")] === "true"
+}
+
 export async function getDeployedResource(
   ctx: PluginContext,
   provider: KubernetesProvider,

core/src/tasks/deploy.ts

@@ -192,14 +192,15 @@ export class DeployTask extends BaseTask {
     const actions = await this.garden.getActionRouter()
 
     let status = serviceStatuses[this.service.name]
+    const devModeSkipRedeploy = status.devMode && (devMode || hotReload)
 
     const log = this.log.info({
       status: "active",
       section: this.service.name,
       msg: `Deploying version ${version}...`,
     })
 
-    if (!this.force && version === status.version && status.state === "ready") {
+    if (!this.force && status.state === "ready" && (version === status.version || devModeSkipRedeploy)) {
       // already deployed and ready
       log.setSuccess({
         msg: chalk.green("Already deployed"),

core/src/tasks/test.ts

@@ -39,6 +39,7 @@ export interface TestTaskParams {
   test: GardenTest
   force: boolean
   forceBuild: boolean
+  fromWatch?: boolean
   devModeServiceNames: string[]
   hotReloadServiceNames: string[]
   silent?: boolean
@@ -52,6 +53,7 @@ export class TestTask extends BaseTask {
   private test: GardenTest
   private graph: ConfigGraph
   private forceBuild: boolean
+  private fromWatch: boolean
   private devModeServiceNames: string[]
   private hotReloadServiceNames: string[]
   private silent: boolean
@@ -63,6 +65,7 @@ export class TestTask extends BaseTask {
     test,
     force,
     forceBuild,
+    fromWatch = false,
     devModeServiceNames,
     hotReloadServiceNames,
     silent = true,
@@ -73,6 +76,7 @@ export class TestTask extends BaseTask {
     this.graph = graph
     this.force = force
     this.forceBuild = forceBuild
+    this.fromWatch = fromWatch
     this.devModeServiceNames = devModeServiceNames
     this.hotReloadServiceNames = hotReloadServiceNames
     this.silent = silent
@@ -92,6 +96,7 @@ export class TestTask extends BaseTask {
       recursive: false,
       filter: (depNode) =>
         !(
+          this.fromWatch &&
           depNode.type === "deploy" &&
           includes([...this.devModeServiceNames, ...this.hotReloadServiceNames], depNode.name)
         ),
@@ -242,6 +247,7 @@ export async function getTestTasks({
   hotReloadServiceNames,
   force = false,
   forceBuild = false,
+  fromWatch = false,
 }: {
   garden: Garden
   log: LogEntry
@@ -252,6 +258,7 @@ export async function getTestTasks({
   hotReloadServiceNames: string[]
   force?: boolean
   forceBuild?: boolean
+  fromWatch?: boolean
 }) {
   // If there are no filters we return the test otherwise
   // we check if the test name matches against the filterNames array
@@ -270,6 +277,7 @@ export async function getTestTasks({
         log,
         force,
         forceBuild,
+        fromWatch,
         test: testFromConfig(module, testConfig, graph),
         devModeServiceNames,
         hotReloadServiceNames,

core/src/types/service.ts

@@ -182,6 +182,7 @@ const forwardablePortSchema = () => joi.object().keys(forwardablePortKeys())
 export interface ServiceStatus<T = any> {
   createdAt?: string
   detail: T
+  devMode?: boolean
   namespaceStatuses?: NamespaceStatus[]
   externalId?: string
   externalVersion?: string
@@ -204,6 +205,7 @@ export const serviceStatusSchema = () =>
   joi.object().keys({
     createdAt: joi.string().description("When the service was first deployed by the provider."),
     detail: joi.object().meta({ extendable: true }).description("Additional detail, specific to the provider."),
+    devMode: joi.boolean().description("Whether the service was deployed with dev mode enabled."),
     namespaceStatuses: namespaceStatusesSchema().optional(),
     externalId: joi
       .string()

core/test/integ/src/plugins/kubernetes/container/deployment.ts

@@ -21,9 +21,9 @@ import { DeployTask } from "../../../../../../src/tasks/deploy"
 import { getServiceStatuses } from "../../../../../../src/tasks/base"
 import { expectError, grouped } from "../../../../../helpers"
 import stripAnsi = require("strip-ansi")
-import { gardenAnnotationKey } from "../../../../../../src/util/string"
 import { kilobytesToString, millicpuToString } from "../../../../../../src/plugins/kubernetes/util"
 import { getResourceRequirements } from "../../../../../../src/plugins/kubernetes/container/util"
+import { isConfiguredForDevMode } from "../../../../../../src/plugins/kubernetes/status/status"
 
 describe("kubernetes container deployment handlers", () => {
   let garden: Garden
@@ -255,7 +255,7 @@ describe("kubernetes container deployment handlers", () => {
         blueGreen: false,
       })
 
-      expect(resource.metadata.annotations![gardenAnnotationKey("dev-mode")]).to.eq("true")
+      expect(isConfiguredForDevMode(resource)).to.eq(true)
 
       const initContainer = resource.spec.template?.spec?.initContainers![0]
       expect(initContainer).to.exist