From 3fb518d50c1062945ea75eb7c45f0ceb920f68e6 Mon Sep 17 00:00:00 2001 From: Jon Edvald Date: Thu, 19 Sep 2019 11:13:57 +0200 Subject: [PATCH 1/4] improvement(k8s): update kubectl to v1.16.0 --- garden-service/src/plugins/kubernetes/kubectl.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/garden-service/src/plugins/kubernetes/kubectl.ts b/garden-service/src/plugins/kubernetes/kubectl.ts index 5856acaa83..abba255c3d 100644 --- a/garden-service/src/plugins/kubernetes/kubectl.ts +++ b/garden-service/src/plugins/kubernetes/kubectl.ts @@ -155,16 +155,16 @@ export const kubectl = new Kubectl({ defaultTimeout: KUBECTL_DEFAULT_TIMEOUT, specs: { darwin: { - url: "https://storage.googleapis.com/kubernetes-release/release/v1.14.0/bin/darwin/amd64/kubectl", - sha256: "26bb69f6ac819700d12be3339c19887a2e496ef3e487e896af2375bf1455cb9f", + url: "https://storage.googleapis.com/kubernetes-release/release/v1.16.0/bin/darwin/amd64/kubectl", + sha256: "a81b23abe67e70f8395ff7a3659bea6610fba98cda1126ef19e0a995f0075d54", }, linux: { - url: "https://storage.googleapis.com/kubernetes-release/release/v1.14.0/bin/linux/amd64/kubectl", - sha256: "99ade995156c1f2fcb01c587fd91be7aae9009c4a986f43438e007265ca112e8", + url: "https://storage.googleapis.com/kubernetes-release/release/v1.16.0/bin/linux/amd64/kubectl", + sha256: "4fc8a7024ef17b907820890f11ba7e59a6a578fa91ea593ce8e58b3260f7fb88", }, win32: { - url: "https://storage.googleapis.com/kubernetes-release/release/v1.14.0/bin/windows/amd64/kubectl.exe", - sha256: "427fd942e356ce44d6c396674bba486ace99f99e45f9121c513c7dd98ff999f0", + url: "https://storage.googleapis.com/kubernetes-release/release/v1.16.0/bin/windows/amd64/kubectl.exe", + sha256: "a7e4e527735f5bc49ad80b92f4a9d3bb6aebd129f9a708baac80465ebc33a9bc", }, }, }) From 5a5d53935329dcfeb99e8733b9bf68ce637cb375 Mon Sep 17 00:00:00 2001 From: Jon Edvald Date: Thu, 19 Sep 2019 11:45:14 +0200 Subject: [PATCH 2/4] improvement(k8s): better error logging for kubectl port forwards --- .../src/plugins/kubernetes/port-forward.ts | 28 +++++++++++++++++-- garden-service/src/util/ext-tools.ts | 18 ++++++++++-- 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/garden-service/src/plugins/kubernetes/port-forward.ts b/garden-service/src/plugins/kubernetes/port-forward.ts index 6271d5dc13..0ad0d14eab 100644 --- a/garden-service/src/plugins/kubernetes/port-forward.ts +++ b/garden-service/src/plugins/kubernetes/port-forward.ts @@ -22,6 +22,7 @@ import { KubernetesResource } from "./types" import { ForwardablePort } from "../../types/service" import { isBuiltIn } from "./util" import { LogEntry } from "../../logger/log-entry" +import { RuntimeError } from "../../exceptions" // TODO: implement stopPortForward handler @@ -90,8 +91,25 @@ export async function getPortForward( log.silly(`Running 'kubectl ${portForwardArgs.join(" ")}'`) const proc = await kubectl.spawn({ log, provider: k8sCtx.provider, namespace, args: portForwardArgs }) + let output = "" + + return new Promise((resolve, reject) => { + let resolved = false + + const portForward = { targetResource, port, proc, localPort } + + proc.on("close", (code) => { + if (registeredPortForwards[key]) { + delete registeredPortForwards[key] + } + if (!resolved) { + reject(new RuntimeError( + `Port forward exited with code ${code} before establishing connection:\n\n${output}`, + { code, portForward }, + )) + } + }) - return new Promise((resolve) => { proc.on("error", (error) => { !proc.killed && proc.kill() throw error @@ -100,13 +118,19 @@ export async function getPortForward( proc.stdout!.on("data", (line) => { // This is unfortunately the best indication that we have that the connection is up... log.silly(`[${targetResource} port forwarder] ${line}`) + output += line if (line.toString().includes("Forwarding from ")) { - const portForward = { targetResource, port, proc, localPort } registeredPortForwards[key] = portForward + resolved = true resolve(portForward) } }) + + proc.stderr!.on("data", (line) => { + log.silly(`[${targetResource} port forwarder] ${line}`) + output += line + }) }) })) } diff --git a/garden-service/src/util/ext-tools.ts b/garden-service/src/util/ext-tools.ts index d679233d09..91c2b955cb 100644 --- a/garden-service/src/util/ext-tools.ts +++ b/garden-service/src/util/ext-tools.ts @@ -263,11 +263,14 @@ export class BinaryCmd extends Library { if (!args) { args = [] } + if (!cwd) { + cwd = dirname(path) + } - log.debug(`Execing ${path} ${args.join(" ")}`) + log.debug(`Execing '${path} ${args.join(" ")}' in ${cwd}`) const proc = execa(path, args, { - cwd: cwd || dirname(path), + cwd, timeout: this.getTimeout(timeout) * 1000, env, input, @@ -302,7 +305,16 @@ export class BinaryCmd extends Library { async spawn({ args, cwd, env, log }: ExecParams) { const path = await this.getPath(log) - return crossSpawn(path, args || [], { cwd: cwd || dirname(path), env }) + + if (!args) { + args = [] + } + if (!cwd) { + cwd = dirname(path) + } + + log.debug(`Spawning '${path} ${args.join(" ")}' in ${cwd}`) + return crossSpawn(path, args, { cwd, env }) } async spawnAndWait({ args, cwd, env, log, ignoreError, outputStream, timeout, tty }: SpawnParams) { From 37ecd0a639981c5d799c4a3eb8345b8061eecd13 Mon Sep 17 00:00:00 2001 From: Jon Edvald Date: Thu, 19 Sep 2019 13:50:45 +0200 Subject: [PATCH 3/4] refactor(plugin): make ServiceStatus detail type-safe --- garden-service/src/actions.ts | 4 +-- .../src/plugins/google/google-app-engine.ts | 4 +-- .../plugins/google/google-cloud-functions.ts | 2 +- .../kubernetes/container/deployment.ts | 33 +++++++++++-------- .../plugins/kubernetes/container/status.ts | 20 +++++++---- .../src/plugins/kubernetes/helm/deployment.ts | 18 ++++++---- .../src/plugins/kubernetes/helm/status.ts | 17 +++++++--- .../kubernetes/kubernetes-module/handlers.ts | 29 +++++++++------- .../src/plugins/kubernetes/status/status.ts | 9 +++-- .../src/plugins/kubernetes/system.ts | 5 +-- .../src/plugins/local/local-docker-swarm.ts | 7 ++-- .../src/plugins/openfaas/openfaas.ts | 7 ++-- .../src/plugins/terraform/module.ts | 2 ++ garden-service/src/types/service.ts | 6 ++-- garden-service/test/helpers.ts | 4 +-- garden-service/test/unit/src/actions.ts | 20 ++++++----- garden-service/test/unit/src/commands/call.ts | 3 ++ .../test/unit/src/commands/delete.ts | 20 ++++++----- .../test/unit/src/commands/deploy.ts | 23 +++++++++---- .../test/unit/src/config/config-context.ts | 2 ++ .../test/unit/src/runtime-context.ts | 2 ++ 21 files changed, 150 insertions(+), 87 deletions(-) diff --git a/garden-service/src/actions.ts b/garden-service/src/actions.ts index 6071a440f5..49062292d1 100644 --- a/garden-service/src/actions.ts +++ b/garden-service/src/actions.ts @@ -71,7 +71,7 @@ import { HotReloadServiceParams, HotReloadServiceResult } from "./types/plugin/s import { RunServiceParams } from "./types/plugin/service/runService" import { GetTaskResultParams } from "./types/plugin/task/getTaskResult" import { RunTaskParams, RunTaskResult } from "./types/plugin/task/runTask" -import { ServiceStatus, ServiceStatusMap } from "./types/service" +import { ServiceStatus, ServiceStatusMap, ServiceState } from "./types/service" import { Omit } from "./util/util" import { DebugInfoMap } from "./types/plugin/provider/getDebugInfo" import { PrepareEnvironmentParams, PrepareEnvironmentResult } from "./types/plugin/provider/prepareEnvironment" @@ -784,5 +784,5 @@ const dummyPublishHandler = async ({ module }) => { const dummyDeleteServiceHandler = async ({ module, log }: DeleteServiceParams) => { const msg = `No delete service handler available for module type ${module.type}` log.setError(msg) - return {} + return { state: "missing" as ServiceState, detail: {} } } diff --git a/garden-service/src/plugins/google/google-app-engine.ts b/garden-service/src/plugins/google/google-app-engine.ts index 96a4070a18..9c3dab9041 100644 --- a/garden-service/src/plugins/google/google-app-engine.ts +++ b/garden-service/src/plugins/google/google-app-engine.ts @@ -60,7 +60,7 @@ export const gardenPlugin = (): GardenPlugin => ({ // const services = await this.gcloud(project).json(["app", "services", "list"]) // const instances: any[] = await this.gcloud(project).json(["app", "instances", "list"]) - return {} + return { state: "unknown", detail: {} } }, async deployService({ ctx, service, runtimeContext, log }: DeployServiceParams) { @@ -104,7 +104,7 @@ export const gardenPlugin = (): GardenPlugin => ({ log.info({ section: service.name, msg: `App deployed` }) - return {} + return { state: "ready", detail: {} } }, }, }, diff --git a/garden-service/src/plugins/google/google-cloud-functions.ts b/garden-service/src/plugins/google/google-cloud-functions.ts index 492930b00d..43b82768ea 100644 --- a/garden-service/src/plugins/google/google-cloud-functions.ts +++ b/garden-service/src/plugins/google/google-cloud-functions.ts @@ -144,7 +144,7 @@ export async function getServiceStatus( if (!status) { // not deployed yet - return {} + return { state: "missing", detail: {} } } // TODO: map states properly diff --git a/garden-service/src/plugins/kubernetes/container/deployment.ts b/garden-service/src/plugins/kubernetes/container/deployment.ts index 96cf0a09d8..da39e6cbe8 100644 --- a/garden-service/src/plugins/kubernetes/container/deployment.ts +++ b/garden-service/src/plugins/kubernetes/container/deployment.ts @@ -23,7 +23,7 @@ import { KubernetesProvider, KubernetesPluginContext } from "../config" import { configureHotReload } from "../hot-reload" import { KubernetesResource, KubernetesServerResource } from "../types" import { ConfigurationError } from "../../../exceptions" -import { getContainerServiceStatus } from "./status" +import { getContainerServiceStatus, ContainerServiceStatus } from "./status" import { containerHelpers } from "../../container/helpers" import { LogEntry } from "../../../logger/log-entry" import { DeployServiceParams } from "../../../types/plugin/service/deployService" @@ -35,7 +35,9 @@ import { RuntimeContext } from "../../../runtime-context" export const DEFAULT_CPU_REQUEST = "10m" export const DEFAULT_MEMORY_REQUEST = "64Mi" -export async function deployContainerService(params: DeployServiceParams): Promise { +export async function deployContainerService( + params: DeployServiceParams, +): Promise { const { deploymentStrategy } = params.ctx.provider.config if (deploymentStrategy === "blue-green") { @@ -46,7 +48,7 @@ export async function deployContainerService(params: DeployServiceParams): Promise { + params: DeployServiceParams): Promise { const { ctx, service, runtimeContext, force, log, hotReload } = params const k8sCtx = ctx @@ -200,7 +202,7 @@ export async function deployContainerServiceBlueGreen( return getContainerServiceStatus(params) } -export async function createContainerObjects( +export async function createContainerManifests( ctx: PluginContext, log: LogEntry, service: ContainerService, @@ -213,18 +215,21 @@ export async function createContainerObjects( const namespace = await getAppNamespace(k8sCtx, log, provider) const api = await KubeApi.factory(log, provider) const ingresses = await createIngressResources(api, provider, namespace, service) - const deployment = await createDeployment({ provider, service, runtimeContext, namespace, enableHotReload, log }) + const workload = await createWorkloadResource({ provider, service, runtimeContext, namespace, enableHotReload, log }) const kubeservices = await createServiceResources(service, namespace) - const objects = [deployment, ...kubeservices, ...ingresses] + const manifests = [workload, ...kubeservices, ...ingresses] - return objects.map(obj => { + for (const obj of manifests) { + set(obj, ["metadata", "labels", gardenAnnotationKey("module")], service.module.name) + set(obj, ["metadata", "labels", gardenAnnotationKey("service")], service.name) + set(obj, ["metadata", "labels", gardenAnnotationKey("generated")], "true") + set(obj, ["metadata", "labels", gardenAnnotationKey("version")], version.versionString) set(obj, ["metadata", "annotations", gardenAnnotationKey("generated")], "true") set(obj, ["metadata", "annotations", gardenAnnotationKey("version")], version.versionString) - set(obj, ["metadata", "labels", "module"], service.module.name) - set(obj, ["metadata", "labels", "service"], service.name) - return obj - }) + } + + return { workload, manifests } } interface CreateDeploymentParams { @@ -236,9 +241,9 @@ interface CreateDeploymentParams { log: LogEntry, } -export async function createDeployment( +export async function createWorkloadResource( { provider, service, runtimeContext, namespace, enableHotReload, log }: CreateDeploymentParams, -): Promise { +): Promise { const spec = service.spec let configuredReplicas = service.spec.replicas @@ -569,7 +574,6 @@ export async function deleteService(params: DeleteServiceParams): Promise export async function getContainerServiceStatus( { ctx, module, service, runtimeContext, log, hotReload }: GetServiceStatusParams, -): Promise { +): Promise { const k8sCtx = ctx // TODO: hash and compare all the configuration files (otherwise internal changes don't get deployed) @@ -34,8 +42,8 @@ export async function getContainerServiceStatus( const namespace = await getAppNamespace(k8sCtx, log, provider) // FIXME: [objects, matched] and ingresses can be run in parallel - const objects = await createContainerObjects(k8sCtx, log, service, runtimeContext, hotReload) - const { state, remoteObjects } = await compareDeployedObjects(k8sCtx, api, namespace, objects, log, true) + const { workload, manifests } = await createContainerManifests(k8sCtx, log, service, runtimeContext, hotReload) + const { state, remoteResources } = await compareDeployedResources(k8sCtx, api, namespace, manifests, log, true) const ingresses = await getIngresses(service, api, provider) const forwardablePorts: ForwardablePort[] = service.spec.ports @@ -55,7 +63,7 @@ export async function getContainerServiceStatus( ingresses, state, version: state === "ready" ? version.versionString : undefined, - detail: { remoteObjects }, + detail: { remoteResources, workload }, } } diff --git a/garden-service/src/plugins/kubernetes/helm/deployment.ts b/garden-service/src/plugins/kubernetes/helm/deployment.ts index b2550fa713..ab1951b9bd 100644 --- a/garden-service/src/plugins/kubernetes/helm/deployment.ts +++ b/garden-service/src/plugins/kubernetes/helm/deployment.ts @@ -6,7 +6,6 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -import { ServiceStatus } from "../../../types/service" import { getAppNamespace } from "../namespace" import { waitForResources } from "../status/status" import { helm } from "./helm-cli" @@ -19,7 +18,7 @@ import { getServiceResourceSpec, getValueFileArgs, } from "./common" -import { getReleaseStatus } from "./status" +import { getReleaseStatus, HelmServiceStatus } from "./status" import { configureHotReload, HotReloadableResource } from "../hot-reload" import { apply } from "../kubectl" import { KubernetesPluginContext } from "../config" @@ -31,7 +30,7 @@ import { getForwardablePorts } from "../port-forward" export async function deployService( { ctx, module, service, log, force, hotReload }: DeployServiceParams, -): Promise { +): Promise { let hotReloadSpec: ContainerHotReloadSpec | null = null let hotReloadTarget: HotReloadableResource | null = null @@ -97,7 +96,13 @@ export async function deployService( // FIXME: we should get these objects from the cluster, and not from the local `helm template` command, because // they may be legitimately inconsistent. - await waitForResources({ ctx, provider, serviceName: service.name, resources: chartResources, log }) + const remoteResources = await waitForResources({ + ctx, + provider, + serviceName: service.name, + resources: chartResources, + log, + }) const forwardablePorts = getForwardablePorts(chartResources) @@ -105,10 +110,11 @@ export async function deployService( forwardablePorts, state: "ready", version: module.version.versionString, + detail: { remoteResources }, } } -export async function deleteService(params: DeleteServiceParams): Promise { +export async function deleteService(params: DeleteServiceParams): Promise { const { ctx, log, module } = params const k8sCtx = ctx @@ -117,5 +123,5 @@ export async function deleteService(params: DeleteServiceParams): Promise + export async function getServiceStatus( { ctx, module, service, log, hotReload }: GetServiceStatusParams, -): Promise { +): Promise { const k8sCtx = ctx // need to build to be able to check the status const buildStatus = await getExecModuleBuildStatus({ ctx: k8sCtx, module, log }) @@ -65,11 +72,11 @@ export async function getServiceStatus( const api = await KubeApi.factory(log, provider) const namespace = await getAppNamespace(k8sCtx, log, provider) - let { state, remoteObjects } = await compareDeployedObjects(k8sCtx, api, namespace, chartResources, log, false) + let { state, remoteResources } = await compareDeployedObjects(k8sCtx, api, namespace, chartResources, log, false) - const forwardablePorts = getForwardablePorts(remoteObjects) + const forwardablePorts = getForwardablePorts(remoteResources) - const detail = { remoteObjects } + const detail = { remoteResources } return { forwardablePorts, @@ -92,6 +99,6 @@ export async function getReleaseStatus( } } catch (_) { // release doesn't exist - return { state: "missing" } + return { state: "missing", detail: {} } } } diff --git a/garden-service/src/plugins/kubernetes/kubernetes-module/handlers.ts b/garden-service/src/plugins/kubernetes/kubernetes-module/handlers.ts index 798b6b15ae..373627657e 100644 --- a/garden-service/src/plugins/kubernetes/kubernetes-module/handlers.ts +++ b/garden-service/src/plugins/kubernetes/kubernetes-module/handlers.ts @@ -15,13 +15,13 @@ import { safeLoadAll } from "js-yaml" import { KubernetesModule, configureKubernetesModule, KubernetesService, describeType } from "./config" import { getNamespace, getAppNamespace } from "../namespace" import { KubernetesPluginContext } from "../config" -import { KubernetesResource } from "../types" +import { KubernetesResource, KubernetesServerResource } from "../types" import { ServiceStatus } from "../../../types/service" import { compareDeployedObjects, waitForResources } from "../status/status" import { KubeApi } from "../api" import { ModuleAndRuntimeActions } from "../../../types/plugin/plugin" import { getAllLogs } from "../logs" -import { deleteObjectsByLabel, apply } from "../kubectl" +import { deleteObjectsBySelector, apply } from "../kubectl" import { BuildModuleParams, BuildResult } from "../../../types/plugin/module/build" import { GetServiceStatusParams } from "../../../types/plugin/service/getServiceStatus" import { DeployServiceParams } from "../../../types/plugin/service/deployService" @@ -42,6 +42,12 @@ export const kubernetesHandlers: Partial + async function build({ module }: BuildModuleParams): Promise { // Get the manifests here, just to validate that the files are there and are valid YAML await readManifests(module) @@ -50,7 +56,7 @@ async function build({ module }: BuildModuleParams): Promise, -): Promise { +): Promise { const k8sCtx = ctx const namespace = await getNamespace({ log, @@ -61,21 +67,21 @@ async function getServiceStatus( const api = await KubeApi.factory(log, k8sCtx.provider) const manifests = await getManifests(api, log, module, namespace) - const { state, remoteObjects } = await compareDeployedObjects(k8sCtx, api, namespace, manifests, log, false) + const { state, remoteResources } = await compareDeployedObjects(k8sCtx, api, namespace, manifests, log, false) - const forwardablePorts = getForwardablePorts(remoteObjects) + const forwardablePorts = getForwardablePorts(remoteResources) return { forwardablePorts, state, version: state === "ready" ? module.version.versionString : undefined, - detail: { remoteObjects }, + detail: { remoteResources }, } } async function deployService( params: DeployServiceParams, -): Promise { +): Promise { const { ctx, force, module, service, log } = params const k8sCtx = ctx @@ -104,7 +110,7 @@ async function deployService( return getServiceStatus(params) } -async function deleteService(params: DeleteServiceParams): Promise { +async function deleteService(params: DeleteServiceParams): Promise { const { ctx, log, service, module } = params const k8sCtx = ctx const namespace = await getAppNamespace(k8sCtx, log, k8sCtx.provider) @@ -112,17 +118,16 @@ async function deleteService(params: DeleteServiceParams): Promise m.kind)), includeUninitialized: false, }) - return { state: "missing" } + return { state: "missing", detail: { remoteResources: [] } } } async function getServiceLogs(params: GetServiceLogsParams) { diff --git a/garden-service/src/plugins/kubernetes/status/status.ts b/garden-service/src/plugins/kubernetes/status/status.ts index e2cecc42c5..39c731a184 100644 --- a/garden-service/src/plugins/kubernetes/status/status.ts +++ b/garden-service/src/plugins/kubernetes/status/status.ts @@ -170,12 +170,13 @@ export async function waitForResources({ ctx, provider, serviceName, resources, const api = await KubeApi.factory(log, provider) const namespace = await getAppNamespace(ctx, log, provider) + let statuses: ResourceStatus[] while (true) { await sleep(2000 + 500 * loops) loops += 1 - const statuses = await checkResourceStatuses(api, namespace, resources, log) + statuses = await checkResourceStatuses(api, namespace, resources, log) for (const status of statuses) { if (status.state === "unhealthy") { @@ -219,11 +220,13 @@ export async function waitForResources({ ctx, provider, serviceName, resources, } statusLine.setState({ symbol: "info", section: serviceName, msg: `Resources ready` }) + + return statuses.map(s => s.resource) } interface ComparisonResult { state: ServiceState - remoteObjects: KubernetesResource[] + remoteResources: KubernetesResource[] } /** @@ -244,7 +247,7 @@ export async function compareDeployedObjects( const result: ComparisonResult = { state: "unknown", - remoteObjects: deployedObjects.filter(o => o !== null), + remoteResources: deployedObjects.filter(o => o !== null), } const logDescription = (resource: KubernetesResource) => `${resource.kind}/${resource.metadata.name}` diff --git a/garden-service/src/plugins/kubernetes/system.ts b/garden-service/src/plugins/kubernetes/system.ts index 28ff5bb647..cd298836ac 100644 --- a/garden-service/src/plugins/kubernetes/system.ts +++ b/garden-service/src/plugins/kubernetes/system.ts @@ -26,6 +26,7 @@ import { PrimitiveMap } from "../../config/common" import { combineStates } from "../../types/service" import { KubernetesResource } from "./types" import { defaultDotIgnoreFiles } from "../../util/fs" +import { HelmServiceStatus } from "./helm/status" const GARDEN_VERSION = getPackageVersion() const SYSTEM_NAMESPACE_MIN_VERSION = "0.9.0" @@ -155,9 +156,9 @@ export async function getSystemServiceStatus( if (serviceNames.includes("kubernetes-dashboard")) { const defaultHostname = ctx.provider.config.defaultHostname - const dashboardStatus = serviceStatuses["kubernetes-dashboard"] + const dashboardStatus = serviceStatuses["kubernetes-dashboard"] as HelmServiceStatus const dashboardServiceResource = find( - (dashboardStatus.detail || {}).remoteObjects || [], + dashboardStatus.detail.remoteResources, o => o.kind === "Service", ) diff --git a/garden-service/src/plugins/local/local-docker-swarm.ts b/garden-service/src/plugins/local/local-docker-swarm.ts index 8c95850925..f0ef3c1603 100644 --- a/garden-service/src/plugins/local/local-docker-swarm.ts +++ b/garden-service/src/plugins/local/local-docker-swarm.ts @@ -257,7 +257,7 @@ async function getServiceStatus({ ctx, service }: GetServiceStatusParams) { +async function getServiceStatus( + { ctx, module, service, log }: GetServiceStatusParams, +): Promise { const openFaasCtx = ctx const k8sProvider = getK8sProvider(ctx.provider.dependencies) @@ -289,7 +291,7 @@ async function getServiceStatus({ ctx, module, service, log }: GetServiceStatusP deployment = await api.apps.readNamespacedDeployment(service.name, namespace) } catch (err) { if (err.code === 404) { - return {} + return { state: "missing", detail: {} } } else { throw err } @@ -305,5 +307,6 @@ async function getServiceStatus({ ctx, module, service, log }: GetServiceStatusP state: status.state, version, ingresses, + detail: {}, } } diff --git a/garden-service/src/plugins/terraform/module.ts b/garden-service/src/plugins/terraform/module.ts index 9ed220b708..36feb30936 100644 --- a/garden-service/src/plugins/terraform/module.ts +++ b/garden-service/src/plugins/terraform/module.ts @@ -136,6 +136,7 @@ export async function getTerraformStatus( state: status.ready ? "ready" : "outdated", version: module.version.versionString, outputs: await getTfOutputs(log, provider.config.version, root), + detail: {}, } } @@ -152,6 +153,7 @@ export async function deployTerraform( state: "ready", version: module.version.versionString, outputs: await getTfOutputs(log, provider.config.version, root), + detail: {}, } } else { // This clause is here as a fail-safe, but shouldn't come up in normal usage because the status handler won't diff --git a/garden-service/src/types/service.ts b/garden-service/src/types/service.ts index d40530f6bc..996b671207 100644 --- a/garden-service/src/types/service.ts +++ b/garden-service/src/types/service.ts @@ -157,9 +157,9 @@ export const forwardablePortKeys = { const forwardablePortSchema = joi.object() .keys(forwardablePortKeys) -export interface ServiceStatus { +export interface ServiceStatus { createdAt?: string - detail?: any + detail: T externalId?: string externalVersion?: string forwardablePorts?: ForwardablePort[], @@ -168,7 +168,7 @@ export interface ServiceStatus { lastError?: string outputs?: PrimitiveMap runningReplicas?: number - state?: ServiceState + state: ServiceState updatedAt?: string version?: string } diff --git a/garden-service/test/helpers.ts b/garden-service/test/helpers.ts index 606d880fd8..2492aa9bfe 100644 --- a/garden-service/test/helpers.ts +++ b/garden-service/test/helpers.ts @@ -176,8 +176,8 @@ export const testPlugin: PluginFactory = (): GardenPlugin => { build: buildExecModule, runModule, - async getServiceStatus() { return {} }, - async deployService() { return {} }, + async getServiceStatus() { return { state: "ready", detail: {} } }, + async deployService() { return { state: "ready", detail: {} } }, async runService( { ctx, service, interactive, runtimeContext, timeout, log }: RunServiceParams, diff --git a/garden-service/test/unit/src/actions.ts b/garden-service/test/unit/src/actions.ts index 575b6bb281..f88d9100ab 100644 --- a/garden-service/test/unit/src/actions.ts +++ b/garden-service/test/unit/src/actions.ts @@ -235,26 +235,26 @@ describe("ActionHelper", () => { describe("getServiceStatus", () => { it("should correctly call the corresponding plugin handler", async () => { const result = await actions.getServiceStatus({ log, service, runtimeContext, hotReload: false }) - expect(result).to.eql({ forwardablePorts: [], state: "ready" }) + expect(result).to.eql({ forwardablePorts: [], state: "ready", detail: {} }) }) it("should resolve runtime template strings", async () => { const result = await actions.getServiceStatus({ log, service, runtimeContext, hotReload: false }) - expect(result).to.eql({ forwardablePorts: [], state: "ready" }) + expect(result).to.eql({ forwardablePorts: [], state: "ready", detail: {} }) }) }) describe("deployService", () => { it("should correctly call the corresponding plugin handler", async () => { const result = await actions.deployService({ log, service, runtimeContext, force: true, hotReload: false }) - expect(result).to.eql({ forwardablePorts: [], state: "ready" }) + expect(result).to.eql({ forwardablePorts: [], state: "ready", detail: {} }) }) }) describe("deleteService", () => { it("should correctly call the corresponding plugin handler", async () => { const result = await actions.deleteService({ log, service, runtimeContext }) - expect(result).to.eql({ forwardablePorts: [], state: "ready" }) + expect(result).to.eql({ forwardablePorts: [], state: "ready", detail: {} }) }) }) @@ -438,7 +438,9 @@ describe("ActionHelper", () => { module: serviceA.module, serviceStatuses: { "service-b": { + state: "ready", outputs: { foo: "bar" }, + detail: {}, }, }, taskResults: {}, @@ -456,7 +458,7 @@ describe("ActionHelper", () => { defaultHandler: async (params) => { expect(params.module.spec.foo).to.equal("bar") - return { forwardablePorts: [], state: "ready" } + return { forwardablePorts: [], state: "ready", detail: {} } }, }) }) @@ -527,7 +529,9 @@ describe("ActionHelper", () => { module: taskA.module, serviceStatuses: { "service-b": { + state: "ready", outputs: { foo: "bar" }, + detail: {}, }, }, taskResults: {}, @@ -747,17 +751,17 @@ const testPlugin: PluginFactory = async () => ({ getServiceStatus: async (params) => { validate(params, moduleActionDescriptions.getServiceStatus.paramsSchema) - return { state: "ready" } + return { state: "ready", detail: {} } }, deployService: async (params) => { validate(params, moduleActionDescriptions.deployService.paramsSchema) - return { state: "ready" } + return { state: "ready", detail: {} } }, deleteService: async (params) => { validate(params, moduleActionDescriptions.deleteService.paramsSchema) - return { state: "ready" } + return { state: "ready", detail: {} } }, execInService: async (params) => { diff --git a/garden-service/test/unit/src/commands/call.ts b/garden-service/test/unit/src/commands/call.ts index dec3268df6..66a027d028 100644 --- a/garden-service/test/unit/src/commands/call.ts +++ b/garden-service/test/unit/src/commands/call.ts @@ -18,6 +18,7 @@ const testProvider: PluginFactory = () => { protocol: "http", port: 32000, }], + detail: {}, }, "service-b": { state: "ready", @@ -27,9 +28,11 @@ const testProvider: PluginFactory = () => { port: 32000, protocol: "http", }], + detail: {}, }, "service-c": { state: "ready", + detail: {}, }, } diff --git a/garden-service/test/unit/src/commands/delete.ts b/garden-service/test/unit/src/commands/delete.ts index 5f8f7a9734..e53a5147a7 100644 --- a/garden-service/test/unit/src/commands/delete.ts +++ b/garden-service/test/unit/src/commands/delete.ts @@ -58,7 +58,7 @@ describe("DeleteSecretCommand", () => { }) const getServiceStatus = async (): Promise => { - return { state: "ready" } + return { state: "ready", detail: {} } } describe("DeleteEnvironmentCommand", () => { @@ -80,7 +80,7 @@ describe("DeleteEnvironmentCommand", () => { const deleteService = async ({ service }): Promise => { deletedServices.push(service.name) - return { state: "missing" } + return { state: "missing", detail: {} } } return { @@ -121,10 +121,10 @@ describe("DeleteEnvironmentCommand", () => { expect(result!.environmentStatuses["test-plugin"]["ready"]).to.be.false expect(result!.serviceStatuses).to.eql({ - "service-a": { forwardablePorts: [], state: "missing" }, - "service-b": { forwardablePorts: [], state: "missing" }, - "service-c": { forwardablePorts: [], state: "missing" }, - "service-d": { forwardablePorts: [], state: "missing" }, + "service-a": { forwardablePorts: [], state: "missing", detail: {} }, + "service-b": { forwardablePorts: [], state: "missing", detail: {} }, + "service-c": { forwardablePorts: [], state: "missing", detail: {} }, + "service-d": { forwardablePorts: [], state: "missing", detail: {} }, }) expect(deletedServices.sort()).to.eql(["service-a", "service-b", "service-c", "service-d"]) }) @@ -136,10 +136,12 @@ describe("DeleteServiceCommand", () => { "service-a": { state: "unknown", ingresses: [], + detail: {}, }, "service-b": { state: "unknown", ingresses: [], + detail: {}, }, } @@ -176,7 +178,7 @@ describe("DeleteServiceCommand", () => { opts: withDefaultGlobalOpts({}), }) expect(result).to.eql({ - "service-a": { forwardablePorts: [], state: "unknown", ingresses: [] }, + "service-a": { forwardablePorts: [], state: "unknown", ingresses: [], detail: {} }, }) }) @@ -193,8 +195,8 @@ describe("DeleteServiceCommand", () => { opts: withDefaultGlobalOpts({}), }) expect(result).to.eql({ - "service-a": { forwardablePorts: [], state: "unknown", ingresses: [] }, - "service-b": { forwardablePorts: [], state: "unknown", ingresses: [] }, + "service-a": { forwardablePorts: [], state: "unknown", ingresses: [], detail: {} }, + "service-b": { forwardablePorts: [], state: "unknown", ingresses: [], detail: {} }, }) }) }) diff --git a/garden-service/test/unit/src/commands/deploy.ts b/garden-service/test/unit/src/commands/deploy.ts index 64a19e8108..fd8ce012c2 100644 --- a/garden-service/test/unit/src/commands/deploy.ts +++ b/garden-service/test/unit/src/commands/deploy.ts @@ -39,20 +39,23 @@ const testProvider: PluginFactory = () => { port: 80, protocol: "http", }], + detail: {}, }, "service-c": { state: "ready", + detail: {}, }, } const getServiceStatus = async ({ service }: GetServiceStatusParams): Promise => { - return testStatuses[service.name] || {} + return testStatuses[service.name] || { state: "unknown", detail: {} } } const deployService = async ({ service }: DeployServiceParams) => { const newStatus = { version: "1", state: "ready", + detail: {}, } testStatuses[service.name] = newStatus @@ -128,23 +131,27 @@ describe("DeployCommand", () => { }, ], state: "ready", + detail: {}, }, "get-service-status.service-b": { forwardablePorts: [], state: "unknown", + detail: {}, }, "get-service-status.service-c": { forwardablePorts: [], state: "ready", + detail: {}, }, "get-service-status.service-d": { forwardablePorts: [], state: "unknown", + detail: {}, }, - "deploy.service-a": { forwardablePorts: [], version: "1", state: "ready" }, - "deploy.service-b": { forwardablePorts: [], version: "1", state: "ready" }, - "deploy.service-c": { forwardablePorts: [], version: "1", state: "ready" }, - "deploy.service-d": { forwardablePorts: [], version: "1", state: "ready" }, + "deploy.service-a": { forwardablePorts: [], version: "1", state: "ready", detail: {} }, + "deploy.service-b": { forwardablePorts: [], version: "1", state: "ready", detail: {} }, + "deploy.service-c": { forwardablePorts: [], version: "1", state: "ready", detail: {} }, + "deploy.service-d": { forwardablePorts: [], version: "1", state: "ready", detail: {} }, }) }) @@ -192,13 +199,15 @@ describe("DeployCommand", () => { }, ], state: "ready", + detail: {}, }, "get-service-status.service-b": { forwardablePorts: [], state: "unknown", + detail: {}, }, - "deploy.service-a": { forwardablePorts: [], version: "1", state: "ready" }, - "deploy.service-b": { forwardablePorts: [], version: "1", state: "ready" }, + "deploy.service-a": { forwardablePorts: [], version: "1", state: "ready", detail: {} }, + "deploy.service-b": { forwardablePorts: [], version: "1", state: "ready", detail: {} }, }) }) }) diff --git a/garden-service/test/unit/src/config/config-context.ts b/garden-service/test/unit/src/config/config-context.ts index c0600ed2f1..035d479821 100644 --- a/garden-service/test/unit/src/config/config-context.ts +++ b/garden-service/test/unit/src/config/config-context.ts @@ -329,7 +329,9 @@ describe("ModuleConfigContext", () => { module: serviceA.module, serviceStatuses: { "service-b": { + state: "ready", outputs: { foo: "bar" }, + detail: {}, }, }, taskResults: { diff --git a/garden-service/test/unit/src/runtime-context.ts b/garden-service/test/unit/src/runtime-context.ts index 3b2cc620e6..c041982842 100644 --- a/garden-service/test/unit/src/runtime-context.ts +++ b/garden-service/test/unit/src/runtime-context.ts @@ -108,7 +108,9 @@ describe("prepareRuntimeContext", () => { }, serviceStatuses: { "service-b": { + state: "ready", outputs, + detail: {}, }, }, taskResults: {}, From 6d00df44dc6eb75c175e5222a46ac287ecda089d Mon Sep 17 00:00:00 2001 From: Jon Edvald Date: Thu, 19 Sep 2019 14:45:38 +0200 Subject: [PATCH 4/4] fix(k8s): exec-ing and hot-reloading only worked for Deployments Meaning, DaemonSet and StatefulSet workloads weren't handled right. Recent changes for blue-green deployments also caused issues, which are solved by these same changes. --- .../commands/cleanup-cluster-registry.ts | 6 +- .../kubernetes/container/deployment.ts | 136 +++++------------- .../src/plugins/kubernetes/container/logs.ts | 4 +- .../src/plugins/kubernetes/container/run.ts | 18 +-- .../plugins/kubernetes/container/service.ts | 2 +- .../src/plugins/kubernetes/helm/hot-reload.ts | 18 ++- .../src/plugins/kubernetes/hot-reload.ts | 78 ++++++++-- .../src/plugins/kubernetes/kubectl.ts | 18 +-- garden-service/src/util/string.ts | 8 +- .../plugins/kubernetes/container/service.ts | 8 +- 10 files changed, 141 insertions(+), 155 deletions(-) diff --git a/garden-service/src/plugins/kubernetes/commands/cleanup-cluster-registry.ts b/garden-service/src/plugins/kubernetes/commands/cleanup-cluster-registry.ts index c41353f696..1997f4f921 100644 --- a/garden-service/src/plugins/kubernetes/commands/cleanup-cluster-registry.ts +++ b/garden-service/src/plugins/kubernetes/commands/cleanup-cluster-registry.ts @@ -24,7 +24,7 @@ import { systemNamespace } from "../system" import { PluginError } from "../../../exceptions" import { apply, kubectl } from "../kubectl" import { waitForResources } from "../status/status" -import { execInDeployment } from "../container/run" +import { execInWorkload } from "../container/run" import { dedent, deline } from "../../../util/string" import { execInBuilder, getBuilderPodName, BuilderExecParams, buildSyncDeploymentName } from "../container/build" import { getPods } from "../util" @@ -226,11 +226,11 @@ async function runRegistryGarbageCollection(ctx: KubernetesPluginContext, api: K // Run garbage collection log.info("Running garbage collection...") - await execInDeployment({ + await execInWorkload({ provider, log, namespace: systemNamespace, - deploymentName: CLUSTER_REGISTRY_DEPLOYMENT_NAME, + workload: modifiedDeployment, command: ["/bin/registry", "garbage-collect", "/etc/docker/registry/config.yml"], interactive: false, }) diff --git a/garden-service/src/plugins/kubernetes/container/deployment.ts b/garden-service/src/plugins/kubernetes/container/deployment.ts index da39e6cbe8..8874882fc1 100644 --- a/garden-service/src/plugins/kubernetes/container/deployment.ts +++ b/garden-service/src/plugins/kubernetes/container/deployment.ts @@ -7,28 +7,27 @@ */ import chalk from "chalk" -import Bluebird from "bluebird" import { V1Container } from "@kubernetes/client-node" -import { Service, ServiceStatus } from "../../../types/service" +import { Service } from "../../../types/service" import { extend, find, keyBy, merge, set } from "lodash" import { ContainerModule, ContainerService } from "../../container/config" import { createIngressResources } from "./ingress" import { createServiceResources } from "./service" import { waitForResources, compareDeployedObjects } from "../status/status" -import { apply, deleteObjectsByLabel } from "../kubectl" +import { apply, deleteObjectsBySelector } from "../kubectl" import { getAppNamespace } from "../namespace" import { PluginContext } from "../../../plugin-context" import { KubeApi } from "../api" import { KubernetesProvider, KubernetesPluginContext } from "../config" import { configureHotReload } from "../hot-reload" -import { KubernetesResource, KubernetesServerResource } from "../types" +import { KubernetesWorkload } from "../types" import { ConfigurationError } from "../../../exceptions" import { getContainerServiceStatus, ContainerServiceStatus } from "./status" import { containerHelpers } from "../../container/helpers" import { LogEntry } from "../../../logger/log-entry" import { DeployServiceParams } from "../../../types/plugin/service/deployService" import { DeleteServiceParams } from "../../../types/plugin/service/deleteService" -import { millicpuToString, kilobytesToString, prepareEnvVars } from "../util" +import { millicpuToString, kilobytesToString, prepareEnvVars, workloadTypes } from "../util" import { gardenAnnotationKey } from "../../../util/string" import { RuntimeContext } from "../../../runtime-context" @@ -54,11 +53,10 @@ export async function deployContainerServiceRolling( const namespace = await getAppNamespace(k8sCtx, log, k8sCtx.provider) - const manifests = await createContainerObjects(k8sCtx, log, service, runtimeContext, hotReload) + const { manifests } = await createContainerManifests(k8sCtx, log, service, runtimeContext, hotReload) - // TODO: use Helm instead of kubectl apply const provider = k8sCtx.provider - const pruneSelector = "service=" + service.name + const pruneSelector = gardenAnnotationKey("service") + "=" + service.name await apply({ log, provider, manifests, force, namespace, pruneSelector }) @@ -73,25 +71,15 @@ export async function deployContainerServiceRolling( return getContainerServiceStatus(params) } -// Given an array of k8s resources and a Garden service returns matching k8s resource -function getResourcesForService(items: KubernetesServerResource[], service): KubernetesServerResource[] { - return items.filter((resource) => { - return resource.metadata - && resource.metadata.labels - && resource.metadata.labels["module"] === service.module.name - && resource.metadata.labels["service"] === service.name - }) -} - export async function deployContainerServiceBlueGreen( - params: DeployServiceParams): Promise { + params: DeployServiceParams): Promise { const { ctx, service, runtimeContext, force, log, hotReload } = params const k8sCtx = ctx const namespace = await getAppNamespace(k8sCtx, log, k8sCtx.provider) // Create all the resource manifests for the Garden service which will be deployed - const manifests = await createContainerObjects(k8sCtx, log, service, runtimeContext, hotReload) + const { manifests } = await createContainerManifests(k8sCtx, log, service, runtimeContext, hotReload) const provider = k8sCtx.provider const api = await KubeApi.factory(log, provider) @@ -118,14 +106,11 @@ export async function deployContainerServiceBlueGreen( } else { // A k8s service matching the current Garden service exist in the cluster. // Proceeding with blue-green deployment + const newVersion = service.module.version.versionString + const versionKey = gardenAnnotationKey("version") // Remove Service manifest from generated resources const filteredManifests = manifests.filter(manifest => manifest.kind !== "Service") - // Retrieve new (yet-to-be-deployed) Deployment manifest - const deploymentManifest = find(manifests, (manifest) => { - return manifest.kind === "Deployment" - && manifest.metadata.labels[gardenAnnotationKey("version")] === service.module.version.versionString - }) // Apply new Deployment manifest (deploy the Green version) await apply({ log, provider, manifests: filteredManifests, force, namespace }) @@ -141,12 +126,12 @@ export async function deployContainerServiceBlueGreen( const servicePatchBody = { metadata: { annotations: { - [gardenAnnotationKey("version")]: deploymentManifest.metadata.labels.version, + [versionKey]: newVersion, }, }, spec: { selector: { - [gardenAnnotationKey("version")]: deploymentManifest.metadata.labels.version, + [versionKey]: newVersion, }, }, } @@ -177,27 +162,17 @@ export async function deployContainerServiceBlueGreen( // Clenup unused deployments: // as a feature we delete all the deployments which don't match any deployed Service. - - const deployments = await api.apps.listNamespacedDeployment(namespace) - // Retrieve all unused deployments for current service - const unusedDeployments = getResourcesForService(deployments.items, service) - .filter(deployment => deployment.metadata.labels - && deployment.metadata.labels[gardenAnnotationKey("version")] - !== deploymentManifest.metadata.labels[gardenAnnotationKey("version")]) - - if (unusedDeployments) { - // Delete old Deployments (Blue) - await Bluebird.map( - unusedDeployments, oldDeployment => api.apps.deleteNamespacedDeployment(oldDeployment.metadata.name, namespace), - ) - await waitForResources({ - ctx: k8sCtx, - provider: k8sCtx.provider, - serviceName: `Cleanup deployments`, - resources: manifests, - log, - }) - } + log.verbose(`Cleaning up old workloads`) + await deleteObjectsBySelector({ + log, + provider, + namespace, + objectTypes: workloadTypes, + // Find workloads that match this service, but have a different version + selector: + `${gardenAnnotationKey("service")}=${service.name},` + + `${versionKey}!=${newVersion}`, + }) } return getContainerServiceStatus(params) } @@ -223,8 +198,6 @@ export async function createContainerManifests( for (const obj of manifests) { set(obj, ["metadata", "labels", gardenAnnotationKey("module")], service.module.name) set(obj, ["metadata", "labels", gardenAnnotationKey("service")], service.name) - set(obj, ["metadata", "labels", gardenAnnotationKey("generated")], "true") - set(obj, ["metadata", "labels", gardenAnnotationKey("version")], version.versionString) set(obj, ["metadata", "annotations", gardenAnnotationKey("generated")], "true") set(obj, ["metadata", "annotations", gardenAnnotationKey("version")], version.versionString) } @@ -310,20 +283,10 @@ export async function createWorkloadResource( container.args = service.spec.args } - // if (config.entrypoint) { - // container.command = [config.entrypoint] - // } - if (spec.healthCheck) { configureHealthCheck(container, spec) } - // if (service.privileged) { - // container.securityContext = { - // privileged: true, - // } - // } - if (spec.volumes && spec.volumes.length) { configureVolumes(deployment, container, spec) } @@ -405,28 +368,30 @@ export async function createWorkloadResource( return deployment } -function deploymentConfig(service: Service, configuredReplicas: number, namespace: string): object { +function getDeploymentName(service: Service) { + return `${service.name}-${service.module.version.versionString}` +} +function deploymentConfig(service: Service, configuredReplicas: number, namespace: string): object { const labels = { - module: service.module.name, - service: service.name, + [gardenAnnotationKey("module")]: service.module.name, + [gardenAnnotationKey("service")]: service.name, [gardenAnnotationKey("version")]: service.module.version.versionString, } - let selector: any = { + let selector = { matchLabels: { - service: service.name, + [gardenAnnotationKey("service")]: service.name, + [gardenAnnotationKey("version")]: service.module.version.versionString, }, } - selector.matchLabels[gardenAnnotationKey("version")] = service.module.version.versionString - // TODO: moar type-safety return { kind: "Deployment", apiVersion: "apps/v1", metadata: { - name: `${service.name}-${service.module.version.versionString}`, + name: getDeploymentName(service), annotations: { // we can use this to avoid overriding the replica count if it has been manually scaled "garden.io/configured.replicas": configuredReplicas.toString(), @@ -448,10 +413,6 @@ function deploymentConfig(service: Service, configuredReplicas: number, namespac restartPolicy: "Always", terminationGracePeriodSeconds: 5, dnsPolicy: "ClusterFirst", - // TODO: support private registries - // imagePullSecrets: [ - // { name: DOCKER_AUTH_SECRET_NAME }, - // ], volumes: [], }, }, @@ -557,45 +518,20 @@ export function rsyncTargetPath(path: string) { .replace(/\/*$/, "/") } -export async function deleteService(params: DeleteServiceParams): Promise { +export async function deleteService(params: DeleteServiceParams): Promise { const { ctx, log, service } = params const k8sCtx = ctx const namespace = await getAppNamespace(k8sCtx, log, k8sCtx.provider) const provider = k8sCtx.provider - await deleteContainerDeployment({ namespace, provider, serviceName: service.name, log }) - await deleteObjectsByLabel({ + await deleteObjectsBySelector({ log, provider, namespace, - labelKey: "service", - labelValue: service.name, + selector: `${gardenAnnotationKey("service")}=${service.name}`, objectTypes: ["deployment", "replicaset", "pod", "service", "ingress", "daemonset"], includeUninitialized: false, }) -} - -export async function deleteContainerDeployment( - { namespace, provider, serviceName, log }: - { namespace: string, provider: KubernetesProvider, serviceName: string, log: LogEntry }, -) { - - let found = true - const api = await KubeApi.factory(log, provider) - - try { - await api.extensions.deleteNamespacedDeployment(serviceName, namespace, {}) - } catch (err) { - if (err.code === 404) { - found = false - } else { - throw err - } - } - - if (log) { - found ? log.setSuccess("Service deleted") : log.setWarn("Service not deployed") - } return { state: "missing", detail: { remoteResources: [], workload: null } } } diff --git a/garden-service/src/plugins/kubernetes/container/logs.ts b/garden-service/src/plugins/kubernetes/container/logs.ts index a96300d131..158a338a82 100644 --- a/garden-service/src/plugins/kubernetes/container/logs.ts +++ b/garden-service/src/plugins/kubernetes/container/logs.ts @@ -11,7 +11,7 @@ import { ContainerModule } from "../../container/config" import { getAppNamespace } from "../namespace" import { getAllLogs } from "../logs" import { KubernetesPluginContext } from "../config" -import { createDeployment } from "./deployment" +import { createWorkloadResource } from "./deployment" import { emptyRuntimeContext } from "../../../runtime-context" export async function getServiceLogs(params: GetServiceLogsParams) { @@ -20,7 +20,7 @@ export async function getServiceLogs(params: GetServiceLogsParams) { const { ctx, log, service, command, interactive } = params @@ -43,37 +44,36 @@ export async function execInService(params: ExecInServiceParams const namespace = await getAppNamespace(k8sCtx, log, k8sCtx.provider) // TODO: this check should probably live outside of the plugin - if (!includes(["ready", "outdated"], status.state)) { + if (!status.detail.workload || !includes(["ready", "outdated"], status.state)) { throw new DeploymentError(`Service ${service.name} is not running`, { name: service.name, state: status.state, }) } - return execInDeployment({ provider, log, namespace, deploymentName: service.name, command, interactive }) + return execInWorkload({ provider, log, namespace, workload: status.detail.workload, command, interactive }) } -export async function execInDeployment( - { provider, log, namespace, deploymentName, command, interactive }: +export async function execInWorkload( + { provider, log, namespace, workload, command, interactive }: { provider: KubernetesProvider, log: LogEntry, namespace: string, - deploymentName: string, + workload: KubernetesWorkload, command: string[], interactive: boolean, }, ) { const api = await KubeApi.factory(log, provider) - const deployment = await api.apps.readNamespacedDeployment(deploymentName, namespace) - const pods = await getWorkloadPods(api, namespace, deployment) + const pods = await getWorkloadPods(api, namespace, workload) const pod = pods[0] if (!pod) { // This should not happen because of the prior status check, but checking to be sure - throw new DeploymentError(`Could not find running pod for ${deploymentName}`, { - deploymentName, + throw new DeploymentError(`Could not find running pod for ${workload.kind}/${workload.metadata.name}`, { + workload, }) } diff --git a/garden-service/src/plugins/kubernetes/container/service.ts b/garden-service/src/plugins/kubernetes/container/service.ts index a4c16ea0fa..e41fdbcc53 100644 --- a/garden-service/src/plugins/kubernetes/container/service.ts +++ b/garden-service/src/plugins/kubernetes/container/service.ts @@ -25,7 +25,7 @@ export async function createServiceResources(service: ContainerService, namespac spec: { ports: servicePorts, selector: { - service: service.name, + [gardenAnnotationKey("service")]: service.name, [gardenAnnotationKey("version")]: service.module.version.versionString, }, type, diff --git a/garden-service/src/plugins/kubernetes/helm/hot-reload.ts b/garden-service/src/plugins/kubernetes/helm/hot-reload.ts index 2d3c895a1f..e8831b13f7 100644 --- a/garden-service/src/plugins/kubernetes/helm/hot-reload.ts +++ b/garden-service/src/plugins/kubernetes/helm/hot-reload.ts @@ -11,9 +11,10 @@ import { ConfigurationError } from "../../../exceptions" import { deline } from "../../../util/string" import { ContainerModule } from "../../container/config" import { getChartResources, findServiceResource, getServiceResourceSpec } from "./common" -import { syncToService, HotReloadableKind } from "../hot-reload" +import { syncToService } from "../hot-reload" import { KubernetesPluginContext } from "../config" import { HotReloadServiceParams, HotReloadServiceResult } from "../../../types/plugin/service/hotReloadService" +import { getAppNamespace } from "../namespace" /** * The hot reload action handler for Helm charts. @@ -26,7 +27,7 @@ export async function hotReloadHelmChart( const chartResources = await getChartResources(ctx, service.module, log) const resourceSpec = service.spec.serviceResource - const target = await findServiceResource({ + const workload = await findServiceResource({ ctx, log, module, @@ -34,14 +35,17 @@ export async function hotReloadHelmChart( resourceSpec, }) - await syncToService( - ctx, + const k8sCtx = ctx as KubernetesPluginContext + const namespace = await getAppNamespace(k8sCtx, log, k8sCtx.provider) + + await syncToService({ + ctx: k8sCtx, service, hotReloadSpec, - target.kind, - target.metadata.name!, + workload, log, - ) + namespace, + }) return {} } diff --git a/garden-service/src/plugins/kubernetes/hot-reload.ts b/garden-service/src/plugins/kubernetes/hot-reload.ts index a28b3f3c71..efa62c7c34 100644 --- a/garden-service/src/plugins/kubernetes/hot-reload.ts +++ b/garden-service/src/plugins/kubernetes/hot-reload.ts @@ -13,8 +13,8 @@ import { V1Deployment, V1DaemonSet, V1StatefulSet } from "@kubernetes/client-nod import { ContainerModule, ContainerHotReloadSpec } from "../container/config" import { RuntimeError, ConfigurationError } from "../../exceptions" import { resolve as resolvePath, dirname } from "path" -import { deline } from "../../util/string" -import { set } from "lodash" +import { deline, gardenAnnotationKey } from "../../util/string" +import { set, sortBy } from "lodash" import { Service } from "../../types/service" import { LogEntry } from "../../logger/log-entry" import { getResourceContainer } from "./helm/common" @@ -23,8 +23,11 @@ import { RSYNC_PORT } from "./constants" import { getAppNamespace } from "./namespace" import { KubernetesPluginContext } from "./config" import { HotReloadServiceParams, HotReloadServiceResult } from "../../types/plugin/service/hotReloadService" -import { KubernetesResource } from "./types" +import { KubernetesResource, KubernetesWorkload, KubernetesList } from "./types" import { normalizeLocalRsyncPath } from "../../util/fs" +import { createWorkloadResource } from "./container/deployment" +import { kubectl } from "./kubectl" +import { labelSelectorToString } from "./util" export const RSYNC_PORT_NAME = "garden-rsync" @@ -53,7 +56,7 @@ export function configureHotReload({ }: ConfigureHotReloadParams) { const kind = target.kind - set(target, ["metadata", "annotations", "garden.io/hot-reload"], "true") + set(target, ["metadata", "annotations", gardenAnnotationKey("hot-reload")], "true") const containers = target.spec.template.spec.containers || [] const mainContainer = getResourceContainer(target, containerName) @@ -160,16 +163,57 @@ export function configureHotReload({ export async function hotReloadContainer( { ctx, log, service, module }: HotReloadServiceParams, ): Promise { - const hotReloadConfig = module.spec.hotReload + const hotReloadSpec = module.spec.hotReload - if (!hotReloadConfig) { + if (!hotReloadSpec) { throw new ConfigurationError( `Module ${module.name} must specify the \`hotReload\` key for service ${service.name} to be hot-reloadable.`, { moduleName: module.name, serviceName: service.name }, ) } - await syncToService(ctx, service, hotReloadConfig, "Deployment", service.name, log) + const k8sCtx = ctx as KubernetesPluginContext + const provider = k8sCtx.provider + const namespace = await getAppNamespace(k8sCtx, log, provider) + + // Find the currently deployed workload by labels + const manifest = await createWorkloadResource({ + provider, + service, + runtimeContext: { envVars: {}, dependencies: [] }, + namespace, + enableHotReload: true, + log, + }) + const selector = labelSelectorToString({ + [gardenAnnotationKey("service")]: service.name, + }) + // TODO: make and use a KubeApi method for this + const res: KubernetesList = await kubectl.json({ + args: ["get", manifest.kind, "-l", selector], + log, + namespace, + provider, + }) + const list = res.items.filter(r => r.metadata.annotations![gardenAnnotationKey("hot-reload")] === "true") + + if (list.length === 0) { + throw new RuntimeError(`Unable to find deployed instance of service ${service.name} with hot-reloading enabled`, { + service, + listResult: res, + }) + } + + const workload = sortBy(list, r => r.metadata.creationTimestamp)[list.length - 1] + + await syncToService({ + log, + ctx: k8sCtx, + service, + workload, + hotReloadSpec, + namespace, + }) return {} } @@ -220,19 +264,22 @@ function rsyncTargetPath(path: string) { .replace(/\/*$/, "/") } -/** - * Ensure a tunnel is set up for connecting to the target service's sync container, and perform a sync. - */ -export async function syncToService( +interface SyncToServiceParams { ctx: KubernetesPluginContext, service: Service, hotReloadSpec: ContainerHotReloadSpec, - targetKind: HotReloadableKind, - targetName: string, + namespace: string, + workload: KubernetesWorkload, log: LogEntry, +} + +/** + * Ensure a tunnel is set up for connecting to the target service's sync container, and perform a sync. + */ +export async function syncToService( + { ctx, service, hotReloadSpec, namespace, workload, log }: SyncToServiceParams, ) { - const targetResource = `${targetKind.toLowerCase()}/${targetName}` - const namespace = await getAppNamespace(ctx, log, ctx.provider) + const targetResource = `${workload.kind.toLowerCase()}/${workload.metadata.name}` const doSync = async () => { const portForward = await getPortForward({ ctx, log, namespace, targetResource, port: RSYNC_PORT }) @@ -263,6 +310,7 @@ export async function syncToService( throw new RuntimeError(`Unexpected error while synchronising to service ${service.name}: ${error.message}`, { error, serviceName: service.name, + targetResource, }) } } diff --git a/garden-service/src/plugins/kubernetes/kubectl.ts b/garden-service/src/plugins/kubernetes/kubectl.ts index abba255c3d..2f20103f02 100644 --- a/garden-service/src/plugins/kubernetes/kubectl.ts +++ b/garden-service/src/plugins/kubernetes/kubectl.ts @@ -48,19 +48,17 @@ export interface DeleteObjectsParams { log: LogEntry, provider: KubernetesProvider namespace: string, - labelKey: string, - labelValue: string, + selector: string, objectTypes: string[], includeUninitialized?: boolean, } -export async function deleteObjectsByLabel( +export async function deleteObjectsBySelector( { log, provider, namespace, - labelKey, - labelValue, + selector, objectTypes, includeUninitialized = false, }: DeleteObjectsParams) { @@ -69,18 +67,12 @@ export async function deleteObjectsByLabel( "delete", objectTypes.join(","), "-l", - `${labelKey}=${labelValue}`, + selector, ] includeUninitialized && args.push("--include-uninitialized") - const result = await kubectl.stdout({ provider, namespace, args, log }) - - try { - return JSON.parse(result) - } catch (_) { - return result - } + return kubectl.stdout({ provider, namespace, args, log }) } interface KubectlParams extends ExecParams { diff --git a/garden-service/src/util/string.ts b/garden-service/src/util/string.ts index 408a706148..bb522a70cb 100644 --- a/garden-service/src/util/string.ts +++ b/garden-service/src/util/string.ts @@ -18,9 +18,15 @@ export const urlJoin = _urlJoin as (...args: string[]) => string const gardenAnnotationPrefix = "garden.io/" -export type GardenAnnotationKey = "generated" | "module" | "moduleVersion" | "service" | "task" | "test" | "version" +export type GardenAnnotationKey = + "generated" | "hot-reload" | "module" | "moduleVersion" | "service" | "task" | "test" | "version" export function gardenAnnotationKey(key: GardenAnnotationKey) { + // FIXME: We need to work out a transition for existing deployments, but we had previously set these two keys + // without the prefix and K8s doesn't allow modifying label selectors on existing workloads. (yay.) + if (key === "module" || key === "service") { + return key + } return gardenAnnotationPrefix + key } diff --git a/garden-service/test/unit/src/plugins/kubernetes/container/service.ts b/garden-service/test/unit/src/plugins/kubernetes/container/service.ts index 34b00088ca..49071c7fb1 100644 --- a/garden-service/test/unit/src/plugins/kubernetes/container/service.ts +++ b/garden-service/test/unit/src/plugins/kubernetes/container/service.ts @@ -40,7 +40,7 @@ describe("createServiceResources", () => { }, ], selector: { - service: "service-a", + [gardenAnnotationKey("service")]: "service-a", [gardenAnnotationKey("version")]: service.module.version.versionString, }, type: "ClusterIP", @@ -78,7 +78,7 @@ describe("createServiceResources", () => { }, ], selector: { - service: "service-a", + [gardenAnnotationKey("service")]: "service-a", [gardenAnnotationKey("version")]: service.module.version.versionString, }, type: "ClusterIP", @@ -115,7 +115,7 @@ describe("createServiceResources", () => { }, ], selector: { - service: "service-a", + [gardenAnnotationKey("service")]: "service-a", [gardenAnnotationKey("version")]: service.module.version.versionString, }, type: "NodePort", @@ -151,7 +151,7 @@ describe("createServiceResources", () => { }, ], selector: { - service: "service-a", + [gardenAnnotationKey("service")]: "service-a", [gardenAnnotationKey("version")]: service.module.version.versionString, }, type: "NodePort",