From a124d314459fa04d1ed69fc44fdf15fb7efc756c Mon Sep 17 00:00:00 2001 From: Jon Edvald Date: Fri, 7 Jun 2019 21:52:43 +0200 Subject: [PATCH 1/3] chore(k8s): remove unused _system symbol in config --- garden-service/src/plugins/kubernetes/config.ts | 1 - garden-service/src/plugins/kubernetes/local/config.ts | 3 --- garden-service/src/plugins/kubernetes/system.ts | 7 +------ 3 files changed, 1 insertion(+), 10 deletions(-) diff --git a/garden-service/src/plugins/kubernetes/config.ts b/garden-service/src/plugins/kubernetes/config.ts index dd9027532f..fcfe122c51 100644 --- a/garden-service/src/plugins/kubernetes/config.ts +++ b/garden-service/src/plugins/kubernetes/config.ts @@ -278,5 +278,4 @@ export const configSchema = kubernetesConfigBase .allow("nginx", false, null) .default(false) .description("Set this to `nginx` to install/enable the NGINX ingress controller."), - _system: Joi.any().meta({ internal: true }), }) diff --git a/garden-service/src/plugins/kubernetes/local/config.ts b/garden-service/src/plugins/kubernetes/local/config.ts index d579d0645f..521929e4e2 100644 --- a/garden-service/src/plugins/kubernetes/local/config.ts +++ b/garden-service/src/plugins/kubernetes/local/config.ts @@ -23,7 +23,6 @@ import { ContainerRegistryConfig } from "../../container/config" const supportedContexts = ["docker-for-desktop", "microk8s", "minikube"] export interface LocalKubernetesConfig extends KubernetesBaseConfig { - _system?: Symbol setupIngressController: string | null } @@ -42,7 +41,6 @@ export const configSchema = kubernetesConfigBase .allow("nginx", false, null) .default("nginx") .description("Set this to null or false to skip installing/enabling the `nginx` ingress controller."), - _system: Joi.any().meta({ internal: true }), }) .description("The provider configuration for the local-kubernetes plugin.") @@ -145,7 +143,6 @@ export async function configureProvider({ config, log, projectName }: ConfigureP storage: config.storage, setupIngressController: config.setupIngressController, tlsCertificates: config.tlsCertificates, - _system: config._system, _systemServices, } diff --git a/garden-service/src/plugins/kubernetes/system.ts b/garden-service/src/plugins/kubernetes/system.ts index c5b6ed2e3a..e2b6b7ec6c 100644 --- a/garden-service/src/plugins/kubernetes/system.ts +++ b/garden-service/src/plugins/kubernetes/system.ts @@ -29,14 +29,10 @@ const GARDEN_VERSION = getPackageVersion() const SYSTEM_NAMESPACE_MIN_VERSION = "0.9.0" const systemProjectPath = join(STATIC_DIR, "kubernetes", "system") -export const systemSymbol = Symbol() + export const systemNamespace = "garden-system" export const systemMetadataNamespace = "garden-system--metadata" -export function isSystemGarden(provider: KubernetesProvider): boolean { - return provider.config._system === systemSymbol -} - export async function getSystemGarden(provider: KubernetesProvider, variables: PrimitiveMap): Promise { return Garden.factory(systemProjectPath, { environmentName: "default", @@ -54,7 +50,6 @@ export async function getSystemGarden(provider: KubernetesProvider, variables: P name: "local-kubernetes", context: provider.config.context, namespace: systemNamespace, - _system: systemSymbol, _systemServices: [], }, ], From 66aa473940cc69b629e8c44e3b279962837ae20f Mon Sep 17 00:00:00 2001 From: Jon Edvald Date: Fri, 7 Jun 2019 22:23:23 +0200 Subject: [PATCH 2/3] refactor(core): added manualInit flag to prepareEnvironment handler This signals to the handler whether it's being run implicitly, or explicitly in the context of `garden init` --- garden-service/src/actions.ts | 24 +++---- garden-service/src/commands/init.ts | 2 +- garden-service/src/plugins/kubernetes/init.ts | 65 +++++++++++-------- .../src/plugins/kubernetes/system.ts | 1 + .../plugin/provider/prepareEnvironment.ts | 5 +- 5 files changed, 52 insertions(+), 45 deletions(-) diff --git a/garden-service/src/actions.ts b/garden-service/src/actions.ts index 38d02cb0c2..1ce4c2c777 100644 --- a/garden-service/src/actions.ts +++ b/garden-service/src/actions.ts @@ -161,14 +161,9 @@ export class ActionHelper implements TypeGuard { * the user to run `garden init` */ async prepareEnvironment( - { force = false, pluginName, log, allowUserInput = false }: - { force?: boolean, pluginName?: string, log: LogEntry, allowUserInput?: boolean }, + { force = false, pluginName, log, manualInit = false }: + { force?: boolean, pluginName?: string, log: LogEntry, manualInit?: boolean }, ) { - const handlers = this.getActionHelpers("prepareEnvironment", pluginName) - // FIXME: We're calling getEnvironmentStatus before preparing the environment. - // Results in 404 errors for unprepared/missing services. - // See: https://github.com/garden-io/garden/issues/353 - const entry = log.info({ section: "providers", msg: "Getting status...", status: "active" }) const statuses = await this.getEnvironmentStatus({ pluginName, log: entry }) @@ -176,7 +171,7 @@ export class ActionHelper implements TypeGuard { .map(([name, status]) => ({ ...status, name })) .filter(status => status.ready === false && status.needManualInit === true) - if (!allowUserInput && needManualInit.length > 0) { + if (!manualInit && needManualInit.length > 0) { const names = needManualInit.map(s => s.name).join(", ") const msgPrefix = needManualInit.length === 1 ? `Provider ${names} has been updated or hasn't been configured, and requires manual initialization` @@ -190,11 +185,11 @@ export class ActionHelper implements TypeGuard { ) } - const needPrep = Object.entries(handlers).filter(([name]) => { + const prepareHandlers = this.getActionHelpers("prepareEnvironment", pluginName) + + const needPrep = Object.entries(prepareHandlers).filter(([name]) => { const status = statuses[name] || { ready: false } - const needForce = status.detail && !!status.detail.needForce - const forcePrep = force || needForce - return forcePrep || !status.ready + return (force || !status.ready) }) const output = {} @@ -206,8 +201,6 @@ export class ActionHelper implements TypeGuard { // sequentially go through the preparation steps, to allow plugins to request user input for (const [name, handler] of needPrep) { const status = statuses[name] || { ready: false } - const needForce = status.detail && !!status.detail.needForce - const forcePrep = force || needForce const envLogEntry = entry.info({ status: "active", @@ -217,7 +210,8 @@ export class ActionHelper implements TypeGuard { await handler({ ...await this.commonParams(handler, log), - force: forcePrep, + manualInit, + force, status, log: envLogEntry, }) diff --git a/garden-service/src/commands/init.ts b/garden-service/src/commands/init.ts index 1ae60ce294..a52e4cce4b 100644 --- a/garden-service/src/commands/init.ts +++ b/garden-service/src/commands/init.ts @@ -45,7 +45,7 @@ export class InitCommand extends Command { logHeader({ log, emoji: "gear", command: `Initializing ${name} environment` }) const actions = await garden.getActionHelper() - await actions.prepareEnvironment({ log, force: opts.force, allowUserInput: true }) + await actions.prepareEnvironment({ log, force: opts.force, manualInit: true }) log.info("") logFooter({ log, emoji: "heavy_check_mark", command: `Done!` }) diff --git a/garden-service/src/plugins/kubernetes/init.ts b/garden-service/src/plugins/kubernetes/init.ts index 6974fdfed1..fe01601837 100644 --- a/garden-service/src/plugins/kubernetes/init.ts +++ b/garden-service/src/plugins/kubernetes/init.ts @@ -8,7 +8,7 @@ import { KubeApi } from "./api" import { getAppNamespace, prepareNamespaces, deleteNamespaces } from "./namespace" -import { KubernetesPluginContext, KubernetesConfig } from "./config" +import { KubernetesPluginContext, KubernetesConfig, KubernetesProvider } from "./config" import { checkTillerStatus, installTiller } from "./helm/tiller" import { prepareSystemServices, @@ -56,6 +56,8 @@ export async function getEnvironmentStatus({ ctx, log }: GetEnvironmentStatusPar const systemServiceNames = k8sCtx.provider.config._systemServices let needManualInit = false + const detail = { systemReady, projectReady, serviceStatuses: {}, systemServiceState: "unknown" } + if (systemServiceNames.length > 0) { // Check Tiller status in system namespace let systemTillerReady = true @@ -69,7 +71,7 @@ export async function getEnvironmentStatus({ ctx, log }: GetEnvironmentStatusPar const sysNamespaceUpToDate = await systemNamespaceUpToDate(api, log, namespace, contextForLog) // Get system service statuses - const systemServiceStatuses = await getSystemServiceStatus({ + const systemServiceStatus = await getSystemServiceStatus({ ctx: k8sCtx, log, namespace, @@ -77,34 +79,24 @@ export async function getEnvironmentStatus({ ctx, log }: GetEnvironmentStatusPar variables: variables || {}, }) - systemReady = systemTillerReady && sysNamespaceUpToDate && ( - systemServiceStatuses.state === "ready" - || - (needManualInit && systemServiceStatuses.state === "outdated") - ) - - dashboardPages = systemServiceStatuses.dashboardPages - - if (needManualInit && systemServiceStatuses.state === "outdated") { - // If we require manual init and system services are outdated (as opposed to unhealthy, missing etc.), we warn - // instead of aborting. This avoids blocking users where there's variance in configuration between users of the - // same cluster, that most likely shouldn't affect usage. - log.warn({ - symbol: "warning", - msg: chalk.yellow( - "One or more cluster-wide services are outdated or their configuration does not match your current " + - "configuration. You may want to run \`garden init\` to update them, or contact your cluster admin.", - ), - }) + // We require manual init if we're installing any system services to remote clusters, to avoid conflicts + // between users or unnecessary work. + needManualInit = checkManualInit(k8sCtx.provider) + systemReady = systemTillerReady && sysNamespaceUpToDate && systemServiceStatus.state === "ready" + dashboardPages = systemServiceStatus.dashboardPages + + // If we require manual init and system services are outdated (as opposed to unhealthy, missing etc.), we warn + // in the prepareEnvironment handler, instead of flagging as not ready here. This avoids blocking users where + // there's variance in configuration between users of the same cluster, that most likely shouldn't affect usage. + if (needManualInit && systemServiceStatus.state === "outdated") { + needManualInit = false } - // We always require manual init if we're installing any system services to remote clusters, to avoid conflicts - // between users or unnecessary work. - needManualInit = ctx.provider.name !== "local-kubernetes" + detail.systemReady = systemReady + detail.serviceStatuses = systemServiceStatus.serviceStatuses + detail.systemServiceState = systemServiceStatus.state } - const detail = { systemReady, projectReady } - return { ready: projectReady && systemReady, detail, @@ -113,23 +105,40 @@ export async function getEnvironmentStatus({ ctx, log }: GetEnvironmentStatusPar } } +function checkManualInit(provider: KubernetesProvider) { + return provider.name !== "local-kubernetes" +} + /** * Performs the following actions to prepare the environment * 1. Installs Tiller in project namespace * 2. Installs Tiller in system namespace (if provider has system services) * 3. Deploys system services (if provider has system services) */ -export async function prepareEnvironment({ ctx, log, force, status }: PrepareEnvironmentParams) { +export async function prepareEnvironment({ ctx, log, force, manualInit, status }: PrepareEnvironmentParams) { const k8sCtx = ctx const variables = getVariables(k8sCtx.provider.config) - const systemReady = status.detail && !!status.detail.systemReady && !force // Install Tiller to project namespace await installTiller({ ctx: k8sCtx, provider: k8sCtx.provider, log, force }) + const systemReady = status.detail && !!status.detail.systemReady && !force const systemServiceNames = k8sCtx.provider.config._systemServices if (systemServiceNames.length > 0 && !systemReady) { + const needManualInit = checkManualInit(k8sCtx.provider) + + if (!manualInit && needManualInit && status.detail && status.detail.systemServiceState === "outdated") { + log.warn({ + symbol: "warning", + msg: chalk.yellow( + "One or more cluster-wide services are outdated or their configuration does not match your current " + + "configuration. You may want to run \`garden init\` to update them, or contact a cluster admin to do so.", + ), + }) + return {} + } + // Install Tiller to system namespace const sysGarden = await getSystemGarden(k8sCtx.provider, variables || {}) const sysCtx = await sysGarden.getPluginContext(k8sCtx.provider.name) diff --git a/garden-service/src/plugins/kubernetes/system.ts b/garden-service/src/plugins/kubernetes/system.ts index e2b6b7ec6c..0968fb906b 100644 --- a/garden-service/src/plugins/kubernetes/system.ts +++ b/garden-service/src/plugins/kubernetes/system.ts @@ -155,6 +155,7 @@ export async function getSystemServiceStatus( return { state, + serviceStatuses, dashboardPages, } } diff --git a/garden-service/src/types/plugin/provider/prepareEnvironment.ts b/garden-service/src/types/plugin/provider/prepareEnvironment.ts index 881251919a..7440818ab2 100644 --- a/garden-service/src/types/plugin/provider/prepareEnvironment.ts +++ b/garden-service/src/types/plugin/provider/prepareEnvironment.ts @@ -12,6 +12,7 @@ import { environmentStatusSchema, EnvironmentStatus } from "./getEnvironmentStat import { dedent } from "../../../util/string" export interface PrepareEnvironmentParams extends PluginActionParamsBase { + manualInit: boolean status: EnvironmentStatus force: boolean } @@ -34,9 +35,11 @@ export const prepareEnvironment = { `, paramsSchema: actionParamsSchema .keys({ - status: environmentStatusSchema, force: Joi.boolean() .description("Force re-configuration of the environment."), + manualInit: Joi.boolean() + .description("Set to true if the environment is being explicitly initialized via `garden init`."), + status: environmentStatusSchema, }), resultSchema: Joi.object().keys({}), } From 07ba9eed5ae1d191e052a2f1a803143bf08b99fd Mon Sep 17 00:00:00 2001 From: Jon Edvald Date: Fri, 7 Jun 2019 22:39:51 +0200 Subject: [PATCH 3/3] chore: fix renaming mistake --- garden-service/src/actions.ts | 12 ++++++------ garden-service/test/unit/src/actions.ts | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/garden-service/src/actions.ts b/garden-service/src/actions.ts index 1ce4c2c777..95cc213214 100644 --- a/garden-service/src/actions.ts +++ b/garden-service/src/actions.ts @@ -143,7 +143,7 @@ export class ActionHelper implements TypeGuard { async getEnvironmentStatus( { pluginName, log }: ActionHelperParams, ): Promise { - const handlers = this.getActionHelpers("getEnvironmentStatus", pluginName) + const handlers = this.getActionHandlers("getEnvironmentStatus", pluginName) const logEntry = log.debug({ msg: "Getting status...", status: "active", @@ -185,7 +185,7 @@ export class ActionHelper implements TypeGuard { ) } - const prepareHandlers = this.getActionHelpers("prepareEnvironment", pluginName) + const prepareHandlers = this.getActionHandlers("prepareEnvironment", pluginName) const needPrep = Object.entries(prepareHandlers).filter(([name]) => { const status = statuses[name] || { ready: false } @@ -229,7 +229,7 @@ export class ActionHelper implements TypeGuard { async cleanupEnvironment( { pluginName, log }: ActionHelperParams, ): Promise { - const handlers = this.getActionHelpers("cleanupEnvironment", pluginName) + const handlers = this.getActionHandlers("cleanupEnvironment", pluginName) await Bluebird.each(values(handlers), async (h) => h({ ...await this.commonParams(h, log) })) return this.getEnvironmentStatus({ pluginName, log }) } @@ -430,7 +430,7 @@ export class ActionHelper implements TypeGuard { } async getDebugInfo({ log }: { log: LogEntry }): Promise { - const handlers = this.getActionHelpers("getDebugInfo") + const handlers = this.getActionHandlers("getDebugInfo") return Bluebird.props(mapValues(handlers, async (h) => h({ ...await this.commonParams(h, log) }))) } @@ -598,7 +598,7 @@ export class ActionHelper implements TypeGuard { /** * Get a handler for the specified action. */ - public getActionHelpers(actionType: T, pluginName?: string): ActionHandlerMap { + public getActionHandlers(actionType: T, pluginName?: string): ActionHandlerMap { return this.filterActionHandlers(this.actionHandlers[actionType], pluginName) } @@ -633,7 +633,7 @@ export class ActionHelper implements TypeGuard { { actionType: T, pluginName?: string, defaultHandler?: PluginActions[T] }, ): PluginActions[T] { - const handlers = Object.values(this.getActionHelpers(actionType, pluginName)) + const handlers = Object.values(this.getActionHandlers(actionType, pluginName)) if (handlers.length) { return handlers[handlers.length - 1] diff --git a/garden-service/test/unit/src/actions.ts b/garden-service/test/unit/src/actions.ts index 2bb8e2dedb..577e6fa071 100644 --- a/garden-service/test/unit/src/actions.ts +++ b/garden-service/test/unit/src/actions.ts @@ -321,9 +321,9 @@ describe("ActionHelper", () => { }) }) - describe("getActionHelpers", () => { + describe("getActionHandlers", () => { it("should return all handlers for a type", async () => { - const handlers = actions.getActionHelpers("prepareEnvironment") + const handlers = actions.getActionHandlers("prepareEnvironment") expect(Object.keys(handlers)).to.eql([ "test-plugin",