Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(kubernetes-module): handle namespace attribute correctly #1197

Merged
merged 2 commits into from
Sep 18, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a comment above this function and the getManifests function below so the difference is clear.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

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
12 changes: 10 additions & 2 deletions garden-service/src/util/ext-tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -283,8 +283,16 @@ export class BinaryCmd extends Library {
}

async stdout(params: ExecParams) {
const res = await this.exec(params)
return res.stdout
try {
const res = await this.exec(params)
return res.stdout
} catch (err) {
// Add log output to error
if (err.all) {
err.message += "\n\n" + err.all
}
throw err
}
}

async json(params: ExecParams) {
Expand Down