Skip to content

Commit

Permalink
fix(kubernetes-module): handle namespace attribute correctly
Browse files Browse the repository at this point in the history
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
  • Loading branch information
edvald committed Sep 18, 2019
1 parent 4322057 commit b6fffd0
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 35 deletions.
2 changes: 1 addition & 1 deletion garden-service/src/plugins/kubernetes/container/logs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,5 @@ export async function getServiceLogs(params: GetServiceLogsParams<ContainerModul
log,
})]

return getAllLogs({ ...params, provider, namespace, resources })
return getAllLogs({ ...params, provider, defaultNamespace: namespace, resources })
}
2 changes: 1 addition & 1 deletion garden-service/src/plugins/kubernetes/helm/logs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,5 @@ export async function getServiceLogs(params: GetServiceLogsParams<HelmModule>) {

const resources = await getChartResources(k8sCtx, module, log)

return getAllLogs({ ...params, provider, namespace, resources })
return getAllLogs({ ...params, provider, defaultNamespace: namespace, resources })
}
51 changes: 40 additions & 11 deletions garden-service/src/plugins/kubernetes/kubernetes-module/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<ModuleAndRuntimeActions<KubernetesModule>> = {
build,
Expand All @@ -43,7 +44,7 @@ export const kubernetesHandlers: Partial<ModuleAndRuntimeActions<KubernetesModul

async function build({ module }: BuildModuleParams<KubernetesModule>): Promise<BuildResult> {
// 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 }
}

Expand All @@ -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)

Expand All @@ -78,16 +79,19 @@ async function deployService(
const { ctx, force, module, service, log } = params

const k8sCtx = <KubernetesPluginContext>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,
Expand All @@ -105,7 +109,8 @@ async function deleteService(params: DeleteServiceParams): Promise<ServiceStatus
const k8sCtx = <KubernetesPluginContext>ctx
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,
Expand All @@ -125,27 +130,51 @@ async function getServiceLogs(params: GetServiceLogsParams<KubernetesModule>) {
const k8sCtx = <KubernetesPluginContext>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<KubernetesResource[]> {
/**
* 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<KubernetesResource[]> {
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
})
}
29 changes: 17 additions & 12 deletions garden-service/src/plugins/kubernetes/logs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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<ServiceLogEntry>
Expand All @@ -33,22 +33,22 @@ interface GetLogsBaseParams {
}

interface GetPodLogsParams extends GetLogsBaseParams {
podNames: string[]
pods: KubernetesPod[]
}

interface GetAllLogsParams extends GetLogsBaseParams {
resources: KubernetesResource[]
}

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<GetServiceLogsResult>((resolve, reject) => {
for (const proc of procs) {
Expand All @@ -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",
Expand All @@ -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!
Expand All @@ -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}`,
})
})

Expand Down
11 changes: 2 additions & 9 deletions garden-service/src/plugins/kubernetes/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,15 @@ 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<KubernetesPod[]> {
const pods: KubernetesPod[] = flatten(await Bluebird.map(resources, async (resource) => {
if (resource.apiVersion === "v1" && resource.kind === "Pod") {
return [<KubernetesPod>resource]
}

if (isWorkload(resource)) {
return getWorkloadPods(api, namespace, <KubernetesWorkload>resource)
return getWorkloadPods(api, resource.metadata.namespace || defaultNamespace, <KubernetesWorkload>resource)
}

return []
Expand All @@ -45,13 +45,6 @@ export async function getAllPods(
return <KubernetesPod[]>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.
*/
Expand Down
2 changes: 1 addition & 1 deletion garden-service/src/plugins/openfaas/openfaas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ async function getServiceLogs(params: GetServiceLogsParams<OpenFaasModule>) {
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<OpenFaasModule>): Promise<ServiceStatus> {
Expand Down

0 comments on commit b6fffd0

Please sign in to comment.