fix: avoid action execution for the static outputs references of impl…

…icit dependencies. (#4975) * chore: remove unused code * fix: avoid action execution for the static outputs references * chore: remove unused duplicate helper * refactor: use helper for constructing action ref string * chore: fix linter issues * refactor: renaming for clarity and remove unnecessary local var * refactor: renaming vars and remove unnecessary ?
garden-io · Aug 24, 2023 · da589eb · da589eb
1 parent eaa85c6
commit da589eb
Show file tree

Hide file tree

Showing 4 changed files with 75 additions and 18 deletions.
diff --git a/core/src/actions/types.ts b/core/src/actions/types.ts
@@ -195,10 +195,6 @@ export type ExecutedActionWrapperParams<
 
 export type GetActionOutputType<T> = T extends BaseAction<any, infer O> ? O : any
 
-export function actionReferenceToString(ref: ActionReference) {
-  return `${ref.kind.toLowerCase()}.${ref.name}`
-}
-
 export type ActionConfig = ValuesType<ActionConfigTypes>
 export type Action = BuildAction | DeployAction | RunAction | TestAction
 export type ResolvedAction = ResolvedBuildAction | ResolvedDeployAction | ResolvedRunAction | ResolvedTestAction

diff --git a/core/src/graph/actions.ts b/core/src/graph/actions.ts
@@ -42,7 +42,7 @@ import { ConfigurationError, InternalError, PluginError, ValidationError } from
 import { overrideVariables, type Garden } from "../garden"
 import type { Log } from "../logger/log-entry"
 import type { ActionTypeDefinition } from "../plugin/action-types"
-import { getActionTypeBases } from "../plugins"
+import { ActionDefinitionMap, getActionTypeBases } from "../plugins"
 import type { ActionRouter } from "../router/router"
 import { getExecuteTaskForAction } from "../tasks/helpers"
 import { ResolveActionTask } from "../tasks/resolve-action"
@@ -297,7 +297,7 @@ export const actionFromConfig = profileAsync(async function actionFromConfig({
     })
   }
 
-  const dependencies = dependenciesFromActionConfig(log, config, configsByKey, definition, templateContext)
+  const dependencies = dependenciesFromActionConfig(log, config, configsByKey, definition, templateContext, actionTypes)
 
   if (config.exclude?.includes("**/*")) {
     if (config.include && config.include.length !== 0) {
@@ -664,7 +664,8 @@ function dependenciesFromActionConfig(
   config: ActionConfig,
   configsByKey: ActionConfigsByKey,
   definition: MaybeUndefined<ActionTypeDefinition<any>>,
-  templateContext: ConfigContext
+  templateContext: ConfigContext,
+  actionTypes: ActionDefinitionMap
 ) {
   const description = describeActionConfig(config)
 
@@ -721,12 +722,12 @@ function dependenciesFromActionConfig(
 
   // Action template references in spec/variables
   // -> We avoid depending on action execution when referencing static output keys
-  const staticKeys = definition?.staticOutputsSchema ? describeSchema(definition.staticOutputsSchema).keys : []
+  const staticOutputKeys = definition?.staticOutputsSchema ? describeSchema(definition.staticOutputsSchema).keys : []
 
   for (const ref of getActionTemplateReferences(config)) {
     let needsExecuted = false
 
-    const outputKey = ref.fullRef[4]
+    const outputKey = ref.fullRef[4] as string
 
     if (maybeTemplateString(ref.name)) {
       try {
@@ -738,8 +739,25 @@ function dependenciesFromActionConfig(
         continue
       }
     }
+    // also avoid execution when referencing the static output keys of the ref action type.
+    // e.g. a helm deploy referencing container build static output deploymentImageName
+    // ${actions.build.my-container.outputs.deploymentImageName}
+    const refActionKey = actionReferenceToString(ref)
+    const refActionType = configsByKey[refActionKey]?.type
+    let refStaticOutputKeys: string[] = []
+    if (refActionType) {
+      const refActionSpec = actionTypes[ref.kind][refActionType]?.spec
+      refStaticOutputKeys = refActionSpec?.staticOutputsSchema
+        ? describeSchema(refActionSpec.staticOutputsSchema).keys
+        : []
+    }
 
-    if (ref.fullRef[3] === "outputs" && outputKey && !staticKeys?.includes(<string>outputKey)) {
+    if (
+      ref.fullRef[3] === "outputs" &&
+      outputKey &&
+      !staticOutputKeys.includes(outputKey) &&
+      !refStaticOutputKeys.includes(outputKey)
+    ) {
       needsExecuted = true
     }
 

diff --git a/core/src/plugins/kubernetes/helm/status.ts b/core/src/plugins/kubernetes/helm/status.ts
@@ -13,7 +13,7 @@ import { getReleaseName, loadTemplate } from "./common"
 import { KubernetesPluginContext } from "../config"
 import { getForwardablePorts } from "../port-forward"
 import { KubernetesResource, KubernetesServerResource } from "../types"
-import { getActionNamespace, getActionNamespaceStatus } from "../namespace"
+import { getActionNamespace } from "../namespace"
 import { getTargetResource, isWorkload } from "../util"
 import { getDeployedResource, isConfiguredForLocalMode } from "../status/status"
 import { KubeApi } from "../api"
@@ -52,13 +52,6 @@ export const getHelmDeployStatus: DeployActionHandler<"getStatus", HelmDeployAct
   let state: DeployState
   let helmStatus: ServiceStatus
 
-  const namespaceStatus = await getActionNamespaceStatus({
-    ctx: k8sCtx,
-    log,
-    action,
-    provider,
-  })
-
   const mode = action.mode()
   let deployedMode: ActionMode = "default"
 

diff --git a/core/test/unit/src/actions/action-configs-to-graph.ts b/core/test/unit/src/actions/action-configs-to-graph.ts
@@ -395,6 +395,56 @@ describe("actionConfigsToGraph", () => {
     ])
   })
 
+  it("does not mark an implicit dependency needing execution if a static output of dependency is referenced", async () => {
+    const graph = await actionConfigsToGraph({
+      garden,
+      log,
+      groupConfigs: [],
+      configs: [
+        {
+          kind: "Build",
+          type: "container",
+          name: "foo",
+          timeout: DEFAULT_BUILD_TIMEOUT_SEC,
+          internal: {
+            basePath: tmpDir.path,
+          },
+          spec: {},
+        },
+        {
+          kind: "Deploy",
+          type: "test",
+          name: "bar",
+          timeout: DEFAULT_BUILD_TIMEOUT_SEC,
+          internal: {
+            basePath: tmpDir.path,
+          },
+          spec: {
+            command: ["echo", "${actions.build.foo.outputs.deploymentImageName}"],
+          },
+        },
+      ],
+      moduleGraph: new ModuleGraph([], {}),
+      actionModes: {},
+      linkedSources: {},
+      environmentName: garden.environmentName,
+    })
+
+    const action = graph.getDeploy("bar")
+    const deps = action.getDependencyReferences()
+
+    expect(deps).to.eql([
+      {
+        explicit: false,
+        kind: "Build",
+        name: "foo",
+        fullRef: ["actions", "build", "foo", "outputs", "deploymentImageName"],
+        needsExecutedOutputs: false,
+        needsStaticOutputs: true,
+      },
+    ])
+  })
+
   it("correctly sets compatibleTypes for an action type with no base", async () => {
     const graph = await actionConfigsToGraph({
       garden,