From b6fffd06c01f83811251e22a33d71cc2f3c4f09e Mon Sep 17 00:00:00 2001 From: Jon Edvald Date: Tue, 17 Sep 2019 18:21:16 +0200 Subject: [PATCH] fix(kubernetes-module): handle namespace attribute correctly Previously we set the --namespace flag when applying the specified resources. We now instead ensure the namespace property is set on every relevant resource. Fixes #1191 --- .../src/plugins/kubernetes/container/logs.ts | 2 +- .../src/plugins/kubernetes/helm/logs.ts | 2 +- .../kubernetes/kubernetes-module/handlers.ts | 51 +++++++++++++++---- garden-service/src/plugins/kubernetes/logs.ts | 29 ++++++----- garden-service/src/plugins/kubernetes/util.ts | 11 +--- .../src/plugins/openfaas/openfaas.ts | 2 +- 6 files changed, 62 insertions(+), 35 deletions(-) diff --git a/garden-service/src/plugins/kubernetes/container/logs.ts b/garden-service/src/plugins/kubernetes/container/logs.ts index 81522915c0..a96300d131 100644 --- a/garden-service/src/plugins/kubernetes/container/logs.ts +++ b/garden-service/src/plugins/kubernetes/container/logs.ts @@ -30,5 +30,5 @@ export async function getServiceLogs(params: GetServiceLogsParams) { const resources = await getChartResources(k8sCtx, module, log) - return getAllLogs({ ...params, provider, namespace, resources }) + return getAllLogs({ ...params, provider, defaultNamespace: namespace, resources }) } diff --git a/garden-service/src/plugins/kubernetes/kubernetes-module/handlers.ts b/garden-service/src/plugins/kubernetes/kubernetes-module/handlers.ts index b601370c10..798b6b15ae 100644 --- a/garden-service/src/plugins/kubernetes/kubernetes-module/handlers.ts +++ b/garden-service/src/plugins/kubernetes/kubernetes-module/handlers.ts @@ -29,6 +29,7 @@ import { DeleteServiceParams } from "../../../types/plugin/service/deleteService import { GetServiceLogsParams } from "../../../types/plugin/service/getServiceLogs" import { gardenAnnotationKey } from "../../../util/string" import { getForwardablePorts, getPortForwardHandler } from "../port-forward" +import { LogEntry } from "../../../logger/log-entry" export const kubernetesHandlers: Partial> = { build, @@ -43,7 +44,7 @@ export const kubernetesHandlers: Partial): Promise { // Get the manifests here, just to validate that the files are there and are valid YAML - await getManifests(module) + await readManifests(module) return { fresh: true } } @@ -58,7 +59,7 @@ async function getServiceStatus( skipCreate: true, }) const api = await KubeApi.factory(log, k8sCtx.provider) - const manifests = await getManifests(module) + const manifests = await getManifests(api, log, module, namespace) const { state, remoteObjects } = await compareDeployedObjects(k8sCtx, api, namespace, manifests, log, false) @@ -78,16 +79,19 @@ async function deployService( const { ctx, force, module, service, log } = params const k8sCtx = ctx + const api = await KubeApi.factory(log, k8sCtx.provider) + const namespace = await getNamespace({ log, projectName: k8sCtx.projectName, provider: k8sCtx.provider, skipCreate: true, }) - const manifests = await getManifests(module) + + const manifests = await getManifests(api, log, module, namespace) const pruneSelector = getSelector(service) - await apply({ log, provider: k8sCtx.provider, manifests, force, namespace, pruneSelector }) + await apply({ log, provider: k8sCtx.provider, manifests, force, pruneSelector }) await waitForResources({ ctx: k8sCtx, @@ -105,7 +109,8 @@ async function deleteService(params: DeleteServiceParams): Promisectx const namespace = await getAppNamespace(k8sCtx, log, k8sCtx.provider) const provider = k8sCtx.provider - const manifests = await getManifests(module) + const api = await KubeApi.factory(log, provider) + const manifests = await getManifests(api, log, module, namespace) await deleteObjectsByLabel({ log, @@ -125,27 +130,51 @@ async function getServiceLogs(params: GetServiceLogsParams) { const k8sCtx = ctx const provider = k8sCtx.provider const namespace = await getAppNamespace(k8sCtx, log, provider) - const manifests = await getManifests(module) + const api = await KubeApi.factory(log, provider) + const manifests = await getManifests(api, log, module, namespace) - return getAllLogs({ ...params, provider, namespace, resources: manifests }) + return getAllLogs({ ...params, provider, defaultNamespace: namespace, resources: manifests }) } function getSelector(service: KubernetesService) { return `${gardenAnnotationKey("service")}=${service.name}` } -async function getManifests(module: KubernetesModule): Promise { +/** + * Read the manifests from the module config, as well as any referenced files in the config. + */ +async function readManifests(module: KubernetesModule) { const fileManifests = flatten(await Bluebird.map(module.spec.files, async (path) => { const absPath = resolve(module.buildPath, path) return safeLoadAll((await readFile(absPath)).toString()) })) - const manifests = [...module.spec.manifests, ...fileManifests] + return [...module.spec.manifests, ...fileManifests] +} - // Add a label, so that we can identify the manifests as part of this module, and prune if needed - return manifests.map(manifest => { +/** + * Reads the manifests and makes sure each has a namespace set (when applicable) and adds annotations. + * Use this when applying to the cluster, or comparing against deployed resources. + */ +async function getManifests( + api: KubeApi, log: LogEntry, module: KubernetesModule, defaultNamespace: string, +): Promise { + const manifests = await readManifests(module) + + return Bluebird.map(manifests, async (manifest) => { + // Ensure a namespace is set, if not already set, and if required by the resource type + if (!manifest.metadata.namespace) { + const info = await api.getApiResourceInfo(log, manifest) + + if (info.resource.namespaced) { + manifest.metadata.namespace = defaultNamespace + } + } + + // Set Garden annotations set(manifest, ["metadata", "annotations", gardenAnnotationKey("service")], module.name) set(manifest, ["metadata", "labels", gardenAnnotationKey("service")], module.name) + return manifest }) } diff --git a/garden-service/src/plugins/kubernetes/logs.ts b/garden-service/src/plugins/kubernetes/logs.ts index 15dbb4ad01..30257e9a64 100644 --- a/garden-service/src/plugins/kubernetes/logs.ts +++ b/garden-service/src/plugins/kubernetes/logs.ts @@ -13,8 +13,8 @@ import moment = require("moment") import { GetServiceLogsResult, ServiceLogEntry } from "../../types/plugin/service/getServiceLogs" import { splitFirst } from "../../util/util" import { kubectl } from "./kubectl" -import { KubernetesResource } from "./types" -import { getAllPodNames } from "./util" +import { KubernetesResource, KubernetesPod } from "./types" +import { getAllPods } from "./util" import { KubeApi } from "./api" import { Service } from "../../types/service" import Stream from "ts-stream" @@ -23,8 +23,8 @@ import Bluebird from "bluebird" import { KubernetesProvider } from "./config" interface GetLogsBaseParams { + defaultNamespace: string log: LogEntry - namespace: string provider: KubernetesProvider service: Service stream: Stream @@ -33,7 +33,7 @@ interface GetLogsBaseParams { } interface GetPodLogsParams extends GetLogsBaseParams { - podNames: string[] + pods: KubernetesPod[] } interface GetAllLogsParams extends GetLogsBaseParams { @@ -41,14 +41,14 @@ interface GetAllLogsParams extends GetLogsBaseParams { } interface GetLogsParams extends GetLogsBaseParams { - podName: string + pod: KubernetesPod } /** * Stream all logs for the given pod names and service. */ export async function getPodLogs(params: GetPodLogsParams) { - const procs = await Bluebird.map(params.podNames, podName => getLogs({ ...omit(params, "podNames"), podName })) + const procs = await Bluebird.map(params.pods, pod => getLogs({ ...omit(params, "pods"), pod })) return new Promise((resolve, reject) => { for (const proc of procs) { @@ -66,11 +66,11 @@ export async function getPodLogs(params: GetPodLogsParams) { */ export async function getAllLogs(params: GetAllLogsParams) { const api = await KubeApi.factory(params.log, params.provider) - const podNames = await getAllPodNames(api, params.namespace, params.resources) - return getPodLogs({ ...params, podNames }) + const pods = await getAllPods(api, params.defaultNamespace, params.resources) + return getPodLogs({ ...params, pods }) } -async function getLogs({ log, provider, namespace, service, stream, tail, follow, podName }: GetLogsParams) { +async function getLogs({ log, provider, service, stream, tail, follow, pod }: GetLogsParams) { // TODO: do this via API instead of kubectl const kubectlArgs = [ "logs", @@ -83,9 +83,14 @@ async function getLogs({ log, provider, namespace, service, stream, tail, follow kubectlArgs.push("--follow=true") } - kubectlArgs.push(`pod/${podName}`) + kubectlArgs.push(`pod/${pod.metadata.name}`) - const proc = await kubectl.spawn({ log, provider, namespace, args: kubectlArgs }) + const proc = await kubectl.spawn({ + args: kubectlArgs, + log, + provider, + namespace: pod.metadata.namespace, + }) let timestamp: Date proc.stdout! @@ -101,7 +106,7 @@ async function getLogs({ log, provider, namespace, service, stream, tail, follow void stream.write({ serviceName: service.name, timestamp, - msg: `${podName} ${msg}`, + msg: `${pod.metadata.name} ${msg}`, }) }) diff --git a/garden-service/src/plugins/kubernetes/util.ts b/garden-service/src/plugins/kubernetes/util.ts index 3cf85d977b..cd4b444712 100644 --- a/garden-service/src/plugins/kubernetes/util.ts +++ b/garden-service/src/plugins/kubernetes/util.ts @@ -28,7 +28,7 @@ export function getAnnotation(obj: KubernetesResource, key: string): string | nu * Given a list of resources, get all the associated pods. */ export async function getAllPods( - api: KubeApi, namespace: string, resources: KubernetesResource[], + api: KubeApi, defaultNamespace: string, resources: KubernetesResource[], ): Promise { const pods: KubernetesPod[] = flatten(await Bluebird.map(resources, async (resource) => { if (resource.apiVersion === "v1" && resource.kind === "Pod") { @@ -36,7 +36,7 @@ export async function getAllPods( } if (isWorkload(resource)) { - return getWorkloadPods(api, namespace, resource) + return getWorkloadPods(api, resource.metadata.namespace || defaultNamespace, resource) } return [] @@ -45,13 +45,6 @@ export async function getAllPods( return deduplicateResources(pods) } -/** - * Given a list of resources, get the names of all the associated pod. - */ -export async function getAllPodNames(api: KubeApi, namespace: string, resources: KubernetesResource[]) { - return (await getAllPods(api, namespace, resources)).map(p => p.metadata.name) -} - /** * Given a resources, try to retrieve a valid selector or return undefined otherwise. */ diff --git a/garden-service/src/plugins/openfaas/openfaas.ts b/garden-service/src/plugins/openfaas/openfaas.ts index 443ea721f9..768dffac38 100644 --- a/garden-service/src/plugins/openfaas/openfaas.ts +++ b/garden-service/src/plugins/openfaas/openfaas.ts @@ -192,7 +192,7 @@ async function getServiceLogs(params: GetServiceLogsParams) { const api = await KubeApi.factory(log, provider) const resources = await getResources(api, service, namespace) - return getAllLogs({ ...params, provider, namespace, resources }) + return getAllLogs({ ...params, provider, defaultNamespace: namespace, resources }) } async function deployService(params: DeployServiceParams): Promise {