Merge pull request #825 from garden-io/init-improvement

Init flow improvement

garden-service/src/actions.ts

@@ -143,7 +143,7 @@ export class ActionHelper implements TypeGuard {
   async getEnvironmentStatus(
     { pluginName, log }: ActionHelperParams<GetEnvironmentStatusParams>,
   ): Promise<EnvironmentStatusMap> {
-    const handlers = this.getActionHelpers("getEnvironmentStatus", pluginName)
+    const handlers = this.getActionHandlers("getEnvironmentStatus", pluginName)
     const logEntry = log.debug({
       msg: "Getting status...",
       status: "active",
@@ -161,22 +161,17 @@ 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 })
 
     const needManualInit = Object.entries(statuses)
       .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.getActionHandlers("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,
       })
@@ -235,7 +229,7 @@ export class ActionHelper implements TypeGuard {
   async cleanupEnvironment(
     { pluginName, log }: ActionHelperParams<CleanupEnvironmentParams>,
   ): Promise<EnvironmentStatusMap> {
-    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 })
   }
@@ -436,7 +430,7 @@ export class ActionHelper implements TypeGuard {
   }
 
   async getDebugInfo({ log }: { log: LogEntry }): Promise<DebugInfoMap> {
-    const handlers = this.getActionHelpers("getDebugInfo")
+    const handlers = this.getActionHandlers("getDebugInfo")
     return Bluebird.props(mapValues(handlers, async (h) => h({ ...await this.commonParams(h, log) })))
   }
 
@@ -604,7 +598,7 @@ export class ActionHelper implements TypeGuard {
   /**
    * Get a handler for the specified action.
    */
-  public getActionHelpers<T extends keyof PluginActions>(actionType: T, pluginName?: string): ActionHandlerMap<T> {
+  public getActionHandlers<T extends keyof PluginActions>(actionType: T, pluginName?: string): ActionHandlerMap<T> {
     return this.filterActionHandlers(this.actionHandlers[actionType], pluginName)
   }
 
@@ -639,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]

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!` })

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 }),
   })

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,42 +71,32 @@ 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,
       serviceNames: systemServiceNames,
       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 = <KubernetesPluginContext>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 = <KubernetesPluginContext>await sysGarden.getPluginContext(k8sCtx.provider.name)

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,
   }
 

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<Garden> {
   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: [],
         },
       ],
@@ -160,6 +155,7 @@ export async function getSystemServiceStatus(
 
   return {
     state,
+    serviceStatuses,
     dashboardPages,
   }
 }

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({}),
 }

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",