Skip to content

Commit

Permalink
fix(k8s): return deployed mode in container Deploy status (#5302)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
thsig committed Oct 25, 2023
1 parent 34bb880 commit 6c18b6d
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 13 deletions.
13 changes: 7 additions & 6 deletions core/src/plugins/kubernetes/container/status.ts
Expand Up @@ -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({
Expand Down
14 changes: 7 additions & 7 deletions core/src/plugins/kubernetes/status/status.ts
Expand Up @@ -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).
Expand Down Expand Up @@ -417,7 +417,7 @@ export async function compareDeployedResources({
const result: ComparisonResult = {
state: "unknown",
remoteResources: <KubernetesResource[]>deployedResources.filter((o) => o !== null),
mode: "default",
deployedMode: "default",
selectorChangedResourceKeys: detectChangedSpecSelector(manifestsMap, deployedMap),
}

Expand Down Expand Up @@ -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"
}
}

Expand Down
60 changes: 60 additions & 0 deletions core/test/integ/src/plugins/kubernetes/container/deployment.ts
Expand Up @@ -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
Expand Down Expand Up @@ -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<ContainerDeployAction>({
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<ContainerDeployAction>({
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")
Expand Down
1 change: 1 addition & 0 deletions core/test/integ/src/plugins/kubernetes/helm/deployment.ts
Expand Up @@ -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,
Expand Down
Expand Up @@ -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")
})

Expand Down

0 comments on commit 6c18b6d

Please sign in to comment.