From 6c18b6d3fc9b5e1066d797ef3939f0fe240f3594 Mon Sep 17 00:00:00 2001 From: Thorarinn Sigurdsson Date: Wed, 25 Oct 2023 13:29:42 +0200 Subject: [PATCH] fix(k8s): return deployed mode in container Deploy status (#5302) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this fix, we were returning the action mode (e.g. `default`, `sync` or `local`) set on the locally generated manifests under `status.detail.mode` for `container` Deploys. This meant that e.g. the `get status` command would always show the mode as `default`. The fix was to use the action mode set on the deployed resources, like we've been doing for `kubernetes` and `helm` Deploys. Also added an integ test case for this behavior. --- .../plugins/kubernetes/container/status.ts | 13 ++-- core/src/plugins/kubernetes/status/status.ts | 14 ++--- .../kubernetes/container/deployment.ts | 60 +++++++++++++++++++ .../src/plugins/kubernetes/helm/deployment.ts | 1 + .../kubernetes/kubernetes-type/handlers.ts | 1 + 5 files changed, 76 insertions(+), 13 deletions(-) diff --git a/core/src/plugins/kubernetes/container/status.ts b/core/src/plugins/kubernetes/container/status.ts index 70108c9a55..e1cbf7bd86 100644 --- a/core/src/plugins/kubernetes/container/status.ts +++ b/core/src/plugins/kubernetes/container/status.ts @@ -47,12 +47,13 @@ export const k8sGetContainerDeployStatus: DeployActionHandler<"getStatus", Conta action, imageId, }) - const { - state, - remoteResources, - mode: deployedMode, - selectorChangedResourceKeys, - } = await compareDeployedResources({ ctx: k8sCtx, api, namespace, manifests, log }) + const { state, remoteResources, deployedMode, selectorChangedResourceKeys } = await compareDeployedResources({ + ctx: k8sCtx, + api, + namespace, + manifests, + log, + }) const ingresses = await getIngresses(action, api, provider) return prepareContainerDeployStatus({ diff --git a/core/src/plugins/kubernetes/status/status.ts b/core/src/plugins/kubernetes/status/status.ts index 86b16a3797..43ba10a500 100644 --- a/core/src/plugins/kubernetes/status/status.ts +++ b/core/src/plugins/kubernetes/status/status.ts @@ -381,7 +381,7 @@ export async function waitForResources({ interface ComparisonResult { state: DeployState remoteResources: KubernetesResource[] - mode: ActionMode + deployedMode: ActionMode /** * These resources have changes in `spec.selector`, and would need to be deleted before redeploying (since Kubernetes * doesn't allow updates to immutable fields). @@ -417,7 +417,7 @@ export async function compareDeployedResources({ const result: ComparisonResult = { state: "unknown", remoteResources: deployedResources.filter((o) => o !== null), - mode: "default", + deployedMode: "default", selectorChangedResourceKeys: detectChangedSpecSelector(manifestsMap, deployedMap), } @@ -469,12 +469,12 @@ export async function compareDeployedResources({ delete manifest.spec?.template?.metadata?.annotations?.[k8sManifestHashAnnotationKey] } - if (isWorkloadResource(manifest)) { - if (isConfiguredForSyncMode(manifest)) { - result.mode = "sync" + if (deployedResource && isWorkloadResource(deployedResource)) { + if (isConfiguredForSyncMode(deployedResource)) { + result.deployedMode = "sync" } - if (isConfiguredForLocalMode(manifest)) { - result.mode = "local" + if (isConfiguredForLocalMode(deployedResource)) { + result.deployedMode = "local" } } diff --git a/core/test/integ/src/plugins/kubernetes/container/deployment.ts b/core/test/integ/src/plugins/kubernetes/container/deployment.ts index 13f6517238..7aad2c4387 100644 --- a/core/test/integ/src/plugins/kubernetes/container/deployment.ts +++ b/core/test/integ/src/plugins/kubernetes/container/deployment.ts @@ -53,6 +53,7 @@ import { ActionRouter } from "../../../../../../src/router/router" import { ActionMode } from "../../../../../../src/actions/types" import { createActionLog } from "../../../../../../src/logger/log-entry" import { K8_POD_DEFAULT_CONTAINER_ANNOTATION_KEY } from "../../../../../../src/plugins/kubernetes/run" +import { k8sGetContainerDeployStatus } from "../../../../../../src/plugins/kubernetes/container/status" describe("kubernetes container deployment handlers", () => { let garden: TestGarden @@ -890,6 +891,65 @@ describe("kubernetes container deployment handlers", () => { }) }) + describe("k8sGetContainerDeployStatus", () => { + before(async () => { + await init("local") + }) + + after(async () => { + if (cleanup) { + cleanup() + } + }) + context("sync mode", () => { + it("should return the action mode registered on the remote resource, if any", async () => { + const deployName = "sync-mode" + const log = garden.log + const deployGraph = await garden.getConfigGraph({ + log, + emit: false, + actionModes: { sync: [`deploy.${deployName}`] }, // <---- + }) + const deployAction = await garden.resolveAction({ + action: deployGraph.getDeploy(deployName), + log, + graph: deployGraph, + }) + const deployTask = new DeployTask({ + garden, + graph: deployGraph, + log, + action: deployAction, + force: true, + forceBuild: false, + }) + + await garden.processTasks({ tasks: [deployTask], log: garden.log, throwOnError: true }) + + // Important: This is a fresh config graoh with no action modes set, as would be the case e.g. when + // calling the `get status` command. This is to test that we're indeed using the action mode written in the + // deployed resources. + const statusGraph = await garden.getConfigGraph({ + log, + emit: false, + }) + + const statusAction = await garden.resolveAction({ + action: statusGraph.getDeploy(deployName), + log, + graph: statusGraph, + }) + const actionLog = createActionLog({ log, actionName: deployName, actionKind: "deploy" }) + const status = await k8sGetContainerDeployStatus({ + ctx, + log: actionLog, + action: statusAction, + }) + expect(status.detail?.mode).to.eql("sync") + }) + }) + }) + describe("handleChangedSelector", () => { before(async () => { await init("local") diff --git a/core/test/integ/src/plugins/kubernetes/helm/deployment.ts b/core/test/integ/src/plugins/kubernetes/helm/deployment.ts index 15b48170a0..1016a33afc 100644 --- a/core/test/integ/src/plugins/kubernetes/helm/deployment.ts +++ b/core/test/integ/src/plugins/kubernetes/helm/deployment.ts @@ -233,6 +233,7 @@ describe("helmDeploy", () => { }) expect(status.state).to.equal("ready") + expect(status.detail.mode).to.eql("sync") expect(status.detail["values"][".garden"]).to.eql({ moduleName: "api", projectName: garden.projectName, diff --git a/core/test/integ/src/plugins/kubernetes/kubernetes-type/handlers.ts b/core/test/integ/src/plugins/kubernetes/kubernetes-type/handlers.ts index 853c6ecea1..5b60607c80 100644 --- a/core/test/integ/src/plugins/kubernetes/kubernetes-type/handlers.ts +++ b/core/test/integ/src/plugins/kubernetes/kubernetes-type/handlers.ts @@ -317,6 +317,7 @@ describe("kubernetes-type handlers", () => { const status = await getKubernetesDeployStatus(deployParams) expect(status.state).to.equal("not-ready") + expect(status.detail?.mode).to.equal("sync") expect(status.detail?.state).to.equal("outdated") })