Skip to content

Commit

Permalink
fix(k8s): ensure patchResources can patch namespace
Browse files Browse the repository at this point in the history
When applying K8s resources, we create a ConfigMap with metadata about
the resources for faster lookup.

This metadata includes the resource namespace among other things.

Before this fix, we'd create the ConfigMap before applying the resource
patch which meant that if you patched the actual resource namespace,
Garden wouldn't store that information.

Later when looking up the status of the resource, Garden would use the
data from the ConfigMap and basically look for the resource in the
wrong namespace and return status "not-ready" (when in fact the resource
could be ready, just in a different namespace).
  • Loading branch information
eysi09 committed Nov 2, 2023
1 parent d9b0169 commit 12664d7
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 12 deletions.
22 changes: 10 additions & 12 deletions core/src/plugins/kubernetes/kubernetes-type/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,19 +150,7 @@ export async function getManifests({
}

const declaredManifests = await readManifests(ctx, action, log)

if (action.kind === "Deploy") {
// Add metadata ConfigMap to aid quick status check
const metadataManifest = getMetadataManifest(action, defaultNamespace, declaredManifests)
const declaredMetadataManifest: DeclaredManifest = {
declaration: { type: "inline", index: declaredManifests.length },
manifest: metadataManifest,
}
declaredManifests.push(declaredMetadataManifest)
}

const patchedManifests = await Promise.all(declaredManifests.map(patchManifest))

const unmatchedPatches = (actionSpec.patchResources || []).filter((p) => {
const manifest = declaredManifests.find((m) => m.manifest.kind === p.kind && m.manifest.metadata.name === p.name)
if (manifest) {
Expand All @@ -177,6 +165,16 @@ export async function getManifests({
)
}

if (action.kind === "Deploy") {
// Add metadata ConfigMap to aid quick status check
const metadataManifest = getMetadataManifest(action, defaultNamespace, patchedManifests)
const declaredMetadataManifest: DeclaredManifest = {
declaration: { type: "inline", index: patchedManifests.length },
manifest: metadataManifest,
}
patchedManifests.push(declaredMetadataManifest)
}

const postProcessedManifests = await Promise.all(patchedManifests.map(postProcessManifest))

validateDeclaredManifests(postProcessedManifests)
Expand Down
48 changes: 48 additions & 0 deletions core/test/integ/src/plugins/kubernetes/kubernetes-type/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,54 @@ describe("getManifests", () => {
expect(manifests[0].spec.replicas).to.eql(3)
expect(manifests[1].data.hello).to.eql("patched-world")
})
it("should store patched version in metadata ConfigMap", async () => {
const action = cloneDeep(graph.getDeploy("deploy-action"))
action["_config"]["spec"]["patchResources"] = [
{
name: "busybox-deployment",
kind: "Deployment",
patch: {
metadata: {
namespace: "patched-namespace-deployment",
},
},
},
{
name: "test-configmap",
kind: "ConfigMap",
patch: {
metadata: {
namespace: "patched-namespace-configmap",
},
},
},
]
const resolvedAction = await garden.resolveAction<KubernetesDeployAction>({
action,
log: garden.log,
graph,
})

const manifests = await getManifests({ ctx, api, action: resolvedAction, log: garden.log, defaultNamespace })

const metadataConfigMap = manifests.filter((m) => m.metadata.name === "garden-meta-deploy-deploy-action")
expect(JSON.parse(metadataConfigMap[0].data.manifestMetadata)).to.eql({
"Deployment/busybox-deployment": {
apiVersion: "apps/v1",
key: "Deployment/busybox-deployment",
kind: "Deployment",
name: "busybox-deployment",
namespace: "patched-namespace-deployment", // <--- The patched namespace should be used here
},
"ConfigMap/test-configmap": {
apiVersion: "v1",
key: "ConfigMap/test-configmap",
kind: "ConfigMap",
name: "test-configmap",
namespace: "patched-namespace-configmap", // <--- The patched namespace should be used here
},
})
})
it("should apply patches to file and inline manifests", async () => {
const action = cloneDeep(graph.getDeploy("deploy-action"))
action["_config"]["spec"]["manifests"] = [
Expand Down

0 comments on commit 12664d7

Please sign in to comment.