Skip to content

Commit

Permalink
fix(kubernetes-module): fix namespace handling
Browse files Browse the repository at this point in the history
Before this fix, we were using the module/service name as the annotation
value for all manifests/resources.

This didn't work for `Namespace` resources in particular, since
deploying a `kubernetes` module that included a `Namespace` manifest
would result in namespaces with different names (but deployed from the
same module, e.g. by another user in the same cluster) being deleted.

The same applied to the `delete service` command when used in the same
context.

We now annotate `Namespace` manifests/resources with the namespace name,
which fixes this problem.
  • Loading branch information
thsig committed Jul 29, 2020
1 parent 5ebc50a commit f4cd7e6
Show file tree
Hide file tree
Showing 4 changed files with 260 additions and 15 deletions.
25 changes: 22 additions & 3 deletions garden-service/src/plugins/kubernetes/kubernetes-module/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,20 @@ export async function getManifests({
}
}

// Set Garden annotations
set(manifest, ["metadata", "annotations", gardenAnnotationKey("service")], module.name)
set(manifest, ["metadata", "labels", gardenAnnotationKey("service")], module.name)
/**
* Set Garden annotations.
*
* For namespace resources, we use the namespace's name as the annotation value, to ensure that namespace resources
* with different names aren't considered by Garden to be the same resource.
*
* This is relevant e.g. in the context of a shared dev cluster, where several users might create their own
* copies of a namespace resource (each named e.g. "${username}-some-namespace") through deploying a `kubernetes`
* module that includes a namespace resource in its manifests.
*/
const annotationValue =
manifest.kind === "Namespace" ? gardenNamespaceAnnotationValue(manifest.metadata.name) : module.name
set(manifest, ["metadata", "annotations", gardenAnnotationKey("service")], annotationValue)
set(manifest, ["metadata", "labels", gardenAnnotationKey("service")], annotationValue)

return manifest
})
Expand Down Expand Up @@ -78,3 +89,11 @@ export async function readManifests(module: KubernetesModule, log: LogEntry, rea

return [...module.spec.manifests, ...fileManifests]
}

/**
* We use this annotation value for namespace resources to avoid potential conflicts with module names (since module
* names can't start with `garden`).
*/
export function gardenNamespaceAnnotationValue(namespaceName: string) {
return `garden-namespace--${namespaceName}`
}
73 changes: 62 additions & 11 deletions garden-service/src/plugins/kubernetes/kubernetes-module/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/

import { uniq } from "lodash"
import Bluebird from "bluebird"
import { partition, uniq } from "lodash"

import { KubernetesModule, configureKubernetesModule, KubernetesService } from "./config"
import { KubernetesPluginContext } from "../config"
Expand All @@ -24,7 +25,7 @@ import { DeleteServiceParams } from "../../../types/plugin/service/deleteService
import { GetServiceLogsParams } from "../../../types/plugin/service/getServiceLogs"
import { gardenAnnotationKey } from "../../../util/string"
import { getForwardablePorts, getPortForwardHandler, killPortForwards } from "../port-forward"
import { getManifests, readManifests } from "./common"
import { getManifests, readManifests, gardenNamespaceAnnotationValue } from "./common"
import { testKubernetesModule } from "./test"
import { runKubernetesTask } from "./run"
import { getTestResult } from "../test-results"
Expand Down Expand Up @@ -105,8 +106,35 @@ export async function deployKubernetesService(

const manifests = await getManifests({ api, log, module, defaultNamespace: namespace })

/**
* We separate out manifests for namespace resources, since we don't want to apply a prune selector
* when applying them.
*/
const [namespaceManifests, otherManifests] = partition(manifests, (m) => m.kind === "Namespace")

if (namespaceManifests.length > 0) {
// Don't prune namespaces
await apply({ log, provider: k8sCtx.provider, manifests: namespaceManifests })
await waitForResources({
namespace,
provider: k8sCtx.provider,
serviceName: service.name,
resources: namespaceManifests,
log,
})
}
const pruneSelector = getSelector(service)
await apply({ log, provider: k8sCtx.provider, manifests, pruneSelector })
if (otherManifests.length > 0) {
// Prune everything else
await apply({ log, provider: k8sCtx.provider, manifests: otherManifests, pruneSelector })
await waitForResources({
namespace,
provider: k8sCtx.provider,
serviceName: service.name,
resources: otherManifests,
log,
})
}

await waitForResources({
namespace,
Expand Down Expand Up @@ -137,14 +165,37 @@ async function deleteService(params: DeleteServiceParams): Promise<KubernetesSer
const api = await KubeApi.factory(log, provider)
const manifests = await getManifests({ api, log, module, defaultNamespace: namespace })

await deleteObjectsBySelector({
log,
provider,
namespace,
selector: `${gardenAnnotationKey("service")}=${service.name}`,
objectTypes: uniq(manifests.map((m) => m.kind)),
includeUninitialized: false,
})
/**
* We separate out manifests for namespace resources, since we need to delete each of them by name.
*
* Unlike other resources, Garden annotates namespace resources with their name - see `getManifests` for a discussion
* of this.
*/
const [namespaceManifests, otherManifests] = partition(manifests, (m) => m.kind === "Namespace")

if (namespaceManifests.length > 0) {
await Bluebird.map(namespaceManifests, (ns) => {
const selector = `${gardenAnnotationKey("service")}=${gardenNamespaceAnnotationValue(ns.metadata.name)}`
return deleteObjectsBySelector({
log,
provider,
namespace,
selector,
objectTypes: ["Namespace"],
includeUninitialized: false,
})
})
}
if (otherManifests.length > 0) {
await deleteObjectsBySelector({
log,
provider,
namespace,
selector: `${gardenAnnotationKey("service")}=${service.name}`,
objectTypes: uniq(manifests.map((m) => m.kind)),
includeUninitialized: false,
})
}

return { state: "missing", detail: { remoteResources: [] } }
}
Expand Down
2 changes: 1 addition & 1 deletion garden-service/src/plugins/kubernetes/status/status.ts
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ export async function compareDeployedResources(
return result
}

async function getDeployedResource(
export async function getDeployedResource(
ctx: PluginContext,
provider: KubernetesProvider,
resource: KubernetesResource,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
/*
* Copyright (C) 2018-2020 Garden Technologies, Inc. <[email protected]>
*
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/

import { expect } from "chai"
import execa from "execa"
import { cloneDeep } from "lodash"
import tmp from "tmp-promise"

import { TestGarden } from "../../../../../helpers"
import { getKubernetesTestGarden } from "./common"
import { DeployTask } from "../../../../../../src/tasks/deploy"
import { getManifests } from "../../../../../../src/plugins/kubernetes/kubernetes-module/common"
import { KubeApi } from "../../../../../../src/plugins/kubernetes/api"
import { LogEntry } from "../../../../../../src/logger/log-entry"
import { KubernetesPluginContext, KubernetesProvider } from "../../../../../../src/plugins/kubernetes/config"
import { getModuleNamespace } from "../../../../../../src/plugins/kubernetes/namespace"
import { getDeployedResource } from "../../../../../../src/plugins/kubernetes/status/status"
import { ModuleConfig } from "../../../../../../src/config/module"
import { KubernetesResource, BaseResource } from "../../../../../../src/plugins/kubernetes/types"
import { DeleteServiceTask } from "../../../../../../src/tasks/delete-service"

describe("kubernetes-module handlers", () => {
let tmpDir: tmp.DirectoryResult
let garden: TestGarden
let log: LogEntry
let ctx: KubernetesPluginContext
let api: KubeApi
/**
* To speed up the test suite, getKubernetesTestGarden caches a garden instance to avoid repeatedly resolving
* providers for the various integ test cases that use it.
*
* Therefore, when overriding the module configs in a test case, we restore the original module configs when we're
* done.
*/
let moduleConfigBackup: ModuleConfig[]
let nsModuleConfig: ModuleConfig
let ns1Manifest: KubernetesResource<BaseResource> | undefined
let ns1Resource: KubernetesResource<BaseResource> | null
let ns2Manifest: KubernetesResource<BaseResource> | undefined
let ns2Resource: KubernetesResource<BaseResource> | null

const withNamespace = (moduleConfig: ModuleConfig, nsName: string): ModuleConfig => {
const cloned = cloneDeep(moduleConfig)
cloned.spec.manifests[0].metadata.name = nsName
cloned.spec.manifests[0].metadata.labels.name = nsName
return cloned
}

before(async () => {
garden = await getKubernetesTestGarden()
moduleConfigBackup = await garden.getRawModuleConfigs()
log = garden.log
const provider = <KubernetesProvider>await garden.resolveProvider(log, "local-kubernetes")
ctx = <KubernetesPluginContext>garden.getPluginContext(provider)
api = await KubeApi.factory(log, ctx.provider)
tmpDir = await tmp.dir({ unsafeCleanup: true })
await execa("git", ["init"], { cwd: tmpDir.path })
nsModuleConfig = {
apiVersion: "garden.io/v0",
kind: "Module",
disabled: false,
allowPublish: false,
build: { dependencies: [] },
description: "Kubernetes module that includes a Namespace resource",
name: "namespace-resource",
outputs: {},
path: tmpDir.path,
serviceConfigs: [],
spec: {
manifests: [
{
apiVersion: "v1",
kind: "Namespace",
metadata: {
name: "kubernetes-module-ns-1",
labels: { name: "kubernetes-module-ns-1" },
},
},
],
serviceResource: {
kind: "Deployment",
name: "busybox-deployment",
},
build: { dependencies: [] },
},
testConfigs: [],
type: "kubernetes",
taskConfigs: [],
}
})

after(async () => {
garden.setModuleConfigs(moduleConfigBackup)
await tmpDir.cleanup()
})

describe("deployKubernetesService", () => {
it("should not delete previously deployed namespace resources", async () => {
garden.setModuleConfigs([withNamespace(nsModuleConfig, "kubernetes-module-ns-1")])
let graph = await garden.getConfigGraph(log)
let k8smodule = graph.getModule("namespace-resource")
const defaultNamespace = await getModuleNamespace({ ctx, log, module: k8smodule, provider: ctx.provider })
let manifests = await getManifests({ api, log, module: k8smodule, defaultNamespace })
ns1Manifest = manifests.find((resource) => resource.kind === "Namespace")

const deployTask = new DeployTask({
garden,
graph,
log,
service: graph.getService("namespace-resource"),
force: false,
forceBuild: false,
})
await garden.processTasks([deployTask], { throwOnError: true })
ns1Resource = await getDeployedResource(ctx, ctx.provider, ns1Manifest!, log)

expect(ns1Manifest, "ns1Manifest").to.exist
expect(ns1Manifest!.metadata.name).to.match(/ns-1/)
expect(ns1Resource, "ns1Resource").to.exist

// This should result in a new namespace with a new name being deployed.
garden.setModuleConfigs([withNamespace(nsModuleConfig, "kubernetes-module-ns-2")])
graph = await garden.getConfigGraph(log)
k8smodule = graph.getModule("namespace-resource")
manifests = await getManifests({ api, log, module: k8smodule, defaultNamespace })
ns2Manifest = manifests.find((resource) => resource.kind === "Namespace")
const deployTask2 = new DeployTask({
garden,
graph,
log,
service: graph.getService("namespace-resource"),
force: true,
forceBuild: true,
})
await garden.processTasks([deployTask2], { throwOnError: true })
ns2Resource = await getDeployedResource(ctx, ctx.provider, ns2Manifest!, log)

expect(ns2Manifest, "ns2Manifest").to.exist
expect(ns2Manifest!.metadata.name).to.match(/ns-2/)
expect(ns2Resource, "ns2Resource").to.exist

// Finally, we verify that the original namespace resource is still in the cluster.
const ns1ResourceRefreshed = await getDeployedResource(ctx, ctx.provider, ns1Manifest!, log)

expect(ns1ResourceRefreshed, "originalNamespaceRefreshed").to.exist
})
})

describe("deleteService", () => {
it("should only delete namespace resources having the current name in the manifests", async () => {
// First, we verify that the namespaces created in the preceding test case are still there.
expect(await getDeployedResource(ctx, ctx.provider, ns1Manifest!, log), "ns1resource").to.exist
expect(await getDeployedResource(ctx, ctx.provider, ns2Manifest!, log), "ns2resource").to.exist

const graph = await garden.getConfigGraph(log)
const deleteServiceTask = new DeleteServiceTask({
garden,
graph,
log,
service: graph.getService("namespace-resource"),
})

// This should only delete kubernetes-module-ns-2.
await garden.processTasks([deleteServiceTask], { throwOnError: true })

expect(await getDeployedResource(ctx, ctx.provider, ns1Manifest!, log), "ns1resource").to.exist
expect(await getDeployedResource(ctx, ctx.provider, ns2Manifest!, log), "ns2resource").to.not.exist
})
})
})

0 comments on commit f4cd7e6

Please sign in to comment.