fix(container): respect deployment registry in publishId if not expli…

…citly set (#6690) * fix(container): respect deployment registry in publishId if not explicitly set * chore: update ctx in test * chore: use gardenPlugin from kubernetes provider * test: make tests pass * test: do not mock outputs to improve test validity * fix: stub the `loadToLocalK8s` under mock testing --------- Co-authored-by: Steffen Neubauer <steffen@garden.io>
garden-io · Jan 22, 2025 · 0c8ec05 · 0c8ec05
1 parent 6a4fd01
commit 0c8ec05
Show file tree

Hide file tree

Showing 9 changed files with 99 additions and 41 deletions.
diff --git a/core/src/plugins/container/helpers.ts b/core/src/plugins/container/helpers.ts
@@ -107,9 +107,12 @@ const helpers = {
       parsedImage = helpers.parseImageId(explicitPublishId)
     } else {
       const explicitImage = action.getSpec("localId")
-      const localImageName = this.getLocalImageName(action.name, explicitImage)
+      // If localId is explicitly set we use that as the image name
+      // Otherwise we use the actions deploymentImageName output, which includes the registry
+      // if that is specificed in the kubernetes provider.
+      const publishImageName = this.getLocalImageName(action.getOutput("deployment-image-name"), explicitImage)
 
-      parsedImage = helpers.parseImageId(localImageName)
+      parsedImage = helpers.parseImageId(publishImageName)
     }
 
     if (!publishTag) {

diff --git a/core/src/plugins/kubernetes/container/build/local.ts b/core/src/plugins/kubernetes/container/build/local.ts
@@ -97,7 +97,7 @@ export const localBuild: BuildHandler = async (params) => {
   const buildResult = await base({ ...params, ctx: { ...ctx, provider: containerProvider } })
 
   if (!provider.config.deploymentRegistry) {
-    await loadToLocalK8s(params)
+    await kubernetesContainerHelpers.loadToLocalK8s(params)
     return buildResult
   }
 
@@ -116,20 +116,22 @@ export const localBuild: BuildHandler = async (params) => {
   return buildResult
 }
 
-/**
- * Loads a built local image to a local Kubernetes instance.
- */
-export async function loadToLocalK8s(params: BuildActionParams<"build", ContainerBuildAction>) {
-  const { ctx, log, action } = params
-  const provider = ctx.provider as KubernetesProvider
+export const kubernetesContainerHelpers = {
+  /**
+   * Loads a built local image to a local Kubernetes instance.
+   */
+  async loadToLocalK8s(params: BuildActionParams<"build", ContainerBuildAction>) {
+    const { ctx, log, action } = params
+    const provider = ctx.provider as KubernetesProvider
 
-  const { localImageId } = k8sGetContainerBuildActionOutputs({ provider, action })
+    const { localImageId } = k8sGetContainerBuildActionOutputs({ provider, action })
 
-  if (provider.config.clusterType === "kind") {
-    await loadImageToKind(localImageId, provider.config, log)
-  } else if (provider.config.clusterType === "microk8s") {
-    await loadImageToMicrok8s({ action, imageId: localImageId, log, ctx })
-  }
+    if (provider.config.clusterType === "kind") {
+      await loadImageToKind(localImageId, provider.config, log)
+    } else if (provider.config.clusterType === "microk8s") {
+      await loadImageToMicrok8s({ action, imageId: localImageId, log, ctx })
+    }
+  },
 }
 
 function containerProviderWithAdditionalDockerArgs(

diff --git a/core/src/plugins/kubernetes/jib-container.ts b/core/src/plugins/kubernetes/jib-container.ts
@@ -22,7 +22,7 @@ import {
   utilContainerName,
   utilDeploymentName,
 } from "./container/build/common.js"
-import { loadToLocalK8s } from "./container/build/local.js"
+import { kubernetesContainerHelpers } from "./container/build/local.js"
 import { containerHandlers } from "./container/handlers.js"
 import { getNamespaceStatus } from "./namespace.js"
 import { PodRunner } from "./run.js"
@@ -53,7 +53,7 @@ export const k8sJibContainerBuildExtension = (): BuildActionExtension<ContainerB
 
         if (spec.dockerBuild) {
           // We may need to explicitly load the image into the cluster if it's built in the docker daemon directly
-          await loadToLocalK8s(params)
+          await kubernetesContainerHelpers.loadToLocalK8s(params)
         }
         return result
       } else if (k8sCtx.provider.config.jib?.pushViaCluster) {

diff --git a/core/test/data/test-project-container-kubernetes/garden.yml b/core/test/data/test-project-container-kubernetes/garden.yml
@@ -0,0 +1,7 @@
+apiVersion: garden.io/v1
+kind: Project
+name: kubernetes-container-module-test-project
+environments:
+  - name: local
+providers:
+  - name: local-kubernetes
diff --git a/core/test/data/test-project-container-kubernetes/kubeconfig.yml b/core/test/data/test-project-container-kubernetes/kubeconfig.yml
@@ -0,0 +1,16 @@
+apiVersion: v1
+clusters:
+- cluster:
+    insecure-skip-tls-verify: true
+    server: https://localhost:6443
+  name: my-cluster
+contexts:
+- context:
+    cluster: my-cluster
+    user: me
+  name: my-cluster
+current-context: my-cluster
+kind: Config
+preferences: {}
+users:
+- name: me
diff --git a/core/test/data/test-project-container-kubernetes/module-a/Dockerfile b/core/test/data/test-project-container-kubernetes/module-a/Dockerfile
@@ -0,0 +1 @@
+FROM scratch
diff --git a/core/test/data/test-project-container-kubernetes/module-a/docker-dir/Dockerfile b/core/test/data/test-project-container-kubernetes/module-a/docker-dir/Dockerfile
@@ -0,0 +1 @@
+FROM scratch
diff --git a/core/test/data/test-project-container-kubernetes/module-a/garden.yml b/core/test/data/test-project-container-kubernetes/module-a/garden.yml
@@ -0,0 +1,16 @@
+kind: Module
+name: module-a
+type: container
+services:
+  - name: service-a
+    ports:
+      - name: http
+        containerPort: 8080
+    ingresses:
+      - path: /
+        port: http
+    healthCheck:
+      tcpPort: http
+tasks:
+  - name: task-a
+    args: [echo, A]
diff --git a/core/test/integ/src/plugins/container/container.ts b/core/test/integ/src/plugins/container/container.ts
@@ -11,7 +11,8 @@ import * as td from "testdouble"
 
 import type { PluginContext } from "../../../../../src/plugin-context.js"
 import type { ContainerProvider } from "../../../../../src/plugins/container/container.js"
-import { gardenPlugin } from "../../../../../src/plugins/container/container.js"
+import { gardenPlugin as gardenContainerPlugin } from "../../../../../src/plugins/container/container.js"
+import { gardenPlugin as gardenK8sPlugin } from "../../../../../src/plugins/kubernetes/kubernetes.js"
 import type { TestGarden } from "../../../../helpers.js"
 import { expectError, getDataDir, makeTestGarden } from "../../../../helpers.js"
 import type { ActionLog } from "../../../../../src/logger/log-entry.js"
@@ -29,11 +30,11 @@ import type { BuildActionConfig } from "../../../../../src/actions/build.js"
 import { containerHelpers, minDockerVersion } from "../../../../../src/plugins/container/helpers.js"
 import { getDockerBuildFlags } from "../../../../../src/plugins/container/build.js"
 import { DEFAULT_BUILD_TIMEOUT_SEC } from "../../../../../src/constants.js"
-
-const testVersionedId = "some/image:12345"
+import type { KubernetesProvider } from "../../../../../src/plugins/kubernetes/config.js"
+import { kubernetesContainerHelpers } from "../../../../../src/plugins/kubernetes/container/build/local.js"
 
 describe("plugins.container", () => {
-  const projectRoot = getDataDir("test-project-container")
+  const projectRoot = getDataDir("test-project-container-kubernetes")
 
   const baseConfig: BuildActionConfig<"container", ContainerBuildActionSpec> = {
     name: "test",
@@ -58,7 +59,7 @@ describe("plugins.container", () => {
   let dockerCli: sinon.SinonStub<any>
 
   beforeEach(async () => {
-    garden = await makeTestGarden(projectRoot, { plugins: [gardenPlugin()] })
+    garden = await makeTestGarden(projectRoot, { plugins: [gardenContainerPlugin(), gardenK8sPlugin()] })
     log = createActionLog({ log: garden.log, actionName: "", actionKind: "" })
     containerProvider = await garden.resolveProvider({ log: garden.log, name: "container" })
     ctx = await garden.getPluginContext({ provider: containerProvider, templateContext: undefined, events: undefined })
@@ -71,12 +72,13 @@ describe("plugins.container", () => {
 
   async function getTestBuild(cfg: BuildActionConfig): Promise<Executed<ContainerBuildAction>> {
     sinon.replace(containerHelpers, "actionHasDockerfile", async () => true)
+    sinon.replace(kubernetesContainerHelpers, "loadToLocalK8s", async () => undefined)
 
     dockerCli = sinon.stub(containerHelpers, "dockerCli")
     dockerCli.returns(
       Promise.resolve({
         all: "test log",
-        stdout: testVersionedId,
+        stdout: "test log",
         stderr: "",
         code: 0,
         proc: null,
@@ -87,7 +89,9 @@ describe("plugins.container", () => {
     const graph = await garden.getConfigGraph({ emit: false, log })
     const build = graph.getBuild(cfg.name)
     const resolved = await garden.resolveAction({ action: build, graph, log })
-    return garden.executeAction({ action: resolved, graph, log })
+    const executed = await garden.executeAction({ action: resolved, graph, log })
+
+    return executed
   }
 
   describe("publishContainerBuild", () => {
@@ -100,10 +104,6 @@ describe("plugins.container", () => {
 
       const action = await getTestBuild(config)
 
-      sinon.replace(action, "getOutput", (o: string) =>
-        // eslint-disable-next-line @typescript-eslint/no-explicit-any
-        o === "localImageId" ? testVersionedId : action.getOutput(<any>o)
-      )
       sinon.restore()
 
       const _dockerCli = sinon.stub(containerHelpers, "dockerCli")
@@ -113,7 +113,7 @@ describe("plugins.container", () => {
 
       sinon.assert.calledWithMatch(_dockerCli.firstCall, {
         cwd: action.getBuildPath(),
-        args: ["tag", action.getOutput("local-image-id"), publishId],
+        args: ["tag", `test:${action.versionString()}`, publishId],
       })
 
       sinon.assert.calledWithMatch(_dockerCli.secondCall, {
@@ -124,17 +124,9 @@ describe("plugins.container", () => {
 
     it("should use specified tag if provided", async () => {
       const config = cloneDeep(baseConfig)
-      const action = td.object(await getTestBuild(config))
+      const action = await getTestBuild(config)
 
       sinon.restore()
-
-      sinon.replace(action, "getOutput", (o: string) =>
-        // eslint-disable-next-line @typescript-eslint/no-explicit-any
-        o === "localImageId" ? testVersionedId : action.getOutput(<any>o)
-      )
-
-      sinon.replace(containerHelpers, "actionHasDockerfile", async () => true)
-
       const _dockerCli = sinon.stub(containerHelpers, "dockerCli")
 
       const result = await publishContainerBuild({ ctx, log, action, tagOverride: "custom-tag" })
@@ -144,7 +136,7 @@ describe("plugins.container", () => {
         _dockerCli,
         sinon.match({
           cwd: action.getBuildPath(),
-          args: ["tag", testVersionedId, "test:custom-tag"],
+          args: ["tag", `test:${action.versionString()}`, "test:custom-tag"],
         })
       )
 
@@ -174,7 +166,7 @@ describe("plugins.container", () => {
 
       it("should use spec.publishId if defined", async () => {
         const config = cloneDeep(baseConfig)
-        config.spec.publishId = testVersionedId
+        config.spec.publishId = "some/image:12345"
 
         action = await getTestBuild(config)
 
@@ -196,11 +188,31 @@ describe("plugins.container", () => {
         const config = cloneDeep(baseConfig)
 
         action = await getTestBuild(config)
-
         const result = await publishContainerBuild({ ctx, log, action })
         assertPublishId(`test:${action.versionString()}`, result.detail)
       })
+      it("should fall back to action.outputs.deploymentImageName if spec.localId and spec.publishId are not defined - with kubernetes provider with deployment registry", async () => {
+        const kubernetesProvider = (await garden.resolveProvider({
+          log,
+          name: "local-kubernetes",
+        })) as KubernetesProvider
+        kubernetesProvider.config.deploymentRegistry = {
+          hostname: "foo.io",
+          namespace: "bar",
+          insecure: false,
+        }
+        ctx = await garden.getPluginContext({
+          provider: kubernetesProvider,
+          templateContext: undefined,
+          events: undefined,
+        })
+        const config = cloneDeep(baseConfig)
 
+        action = await getTestBuild(config)
+
+        const result = await publishContainerBuild({ ctx, log, action })
+        assertPublishId(`foo.io/bar/test:${action.versionString()}`, result.detail)
+      })
       it("should respect tagOverride, which corresponds to garden publish --tag command line option", async () => {
         const config = cloneDeep(baseConfig)