Merge pull request #752 from garden-io/fix-hot-reload-param-issues

Fix hot reload param issues

garden-service/src/actions.ts

@@ -551,7 +551,7 @@ export class ActionHelper implements TypeGuard {
     const plugin = this.garden.getPlugin(pluginName)
     const schema = moduleActionDescriptions[actionType].resultSchema
 
-    const wrapped = async (...args) => {
+    const wrapped = async (...args: any[]) => {
       const result = await handler.apply(plugin, args)
       return validate(result, schema, { context: `${actionType} output from plugin ${pluginName}` })
     }

garden-service/src/commands/deploy.ts

@@ -87,7 +87,7 @@ export class DeployCommand extends Command<Args, Opts> {
   async prepare({ log, logFooter, opts }: PrepareParams<Args, Opts>) {
     logHeader({ log, emoji: "rocket", command: "Deploy" })
 
-    if (!!opts.watch) {
+    if (!!opts.watch || !!opts["hot-reload"]) {
       this.server = await startServer(logFooter)
     }
   }

garden-service/src/commands/helpers.ts

@@ -40,8 +40,6 @@ export async function validateHotReloadServiceNames(
   }
 }
 
-// TODO: Add hotReload to baseModuleSpecSchema. It's bad form to dig into module type-specific fields
-// outside the scope of its plugins.
 function supportsHotReloading(service: Service) {
-  return !!service.module.spec.hotReload
+  return service.config.hotReloadable
 }

garden-service/src/config/module.ts

@@ -9,7 +9,7 @@
 import dedent = require("dedent")
 import stableStringify = require("json-stable-stringify")
 import * as Joi from "joi"
-import { ServiceConfig, ServiceSpec } from "./service"
+import { ServiceConfig, ServiceSpec, serviceConfigSchema } from "./service"
 import {
   joiArray,
   joiIdentifier,
@@ -19,8 +19,8 @@ import {
   joiIdentifierMap,
   joiPrimitive,
 } from "./common"
-import { TestConfig, TestSpec } from "./test"
-import { TaskConfig, TaskSpec } from "./task"
+import { TestConfig, TestSpec, testConfigSchema } from "./test"
+import { TaskConfig, TaskSpec, taskConfigSchema } from "./task"
 
 export interface BuildCopySpec {
   source: string
@@ -71,7 +71,6 @@ export interface BaseModuleSpec {
   description?: string
   include?: string[]
   name: string
-  path: string
   type: string
   repositoryUrl?: string
 }
@@ -142,9 +141,9 @@ export interface ModuleConfig
   >
   extends BaseModuleSpec {
 
-  plugin?: string   // used to identify modules that are bundled as part of a plugin
-
   outputs: PrimitiveMap
+  path: string
+  plugin?: string   // used to identify modules that are bundled as part of a plugin
   serviceConfigs: ServiceConfig<S>[]
   testConfigs: TestConfig<T>[]
   taskConfigs: TaskConfig<W>[]
@@ -155,11 +154,27 @@ export interface ModuleConfig
 
 export const moduleConfigSchema = baseModuleSpecSchema
   .keys({
+    outputs: joiIdentifierMap(joiPrimitive())
+      .description("The outputs defined by the module (referenceable in other module configs)."),
+    path: Joi.string().uri({ relativeOnly: true })
+      .description("The filesystem path of the module."),
+    plugin: joiIdentifier()
+      .meta({ internal: true })
+      .description("The name of a the parent plugin of the module, if applicable."),
+    serviceConfigs: joiArray(serviceConfigSchema)
+      .description("List of services configured by this module."),
+    taskConfigs: joiArray(taskConfigSchema)
+      .description("List of tasks configured by this module."),
+    testConfigs: joiArray(testConfigSchema)
+      .description("List of tests configured by this module."),
     spec: Joi.object()
       .meta({ extendable: true })
       .description("The module spec, as defined by the provider plugin."),
+    _ConfigType: Joi.object()
+      .meta({ internal: true }),
   })
   .description("The configuration for a module.")
+  .unknown(false)
 
 export function serializeConfig(moduleConfig: ModuleConfig) {
   return stableStringify(moduleConfig)

garden-service/src/config/service.ts

@@ -8,7 +8,7 @@
 
 import deline = require("deline")
 import * as Joi from "joi"
-import { PrimitiveMap, joiIdentifier, joiIdentifierMap, joiPrimitive, joiArray, joiUserIdentifier } from "./common"
+import { joiIdentifier, joiIdentifierMap, joiPrimitive, joiArray, joiUserIdentifier } from "./common"
 
 export interface ServiceSpec { }
 
@@ -19,12 +19,11 @@ export interface ServiceSpec { }
 export interface CommonServiceSpec extends ServiceSpec {
   name: string
   dependencies: string[]
-  outputs: PrimitiveMap
 }
 
 export const serviceOutputsSchema = joiIdentifierMap(joiPrimitive())
 
-export const baseServiceSchema = Joi.object()
+export const baseServiceSpecSchema = Joi.object()
   .keys({
     name: joiUserIdentifier().required(),
     dependencies: joiArray(joiIdentifier())
@@ -38,13 +37,18 @@ export const baseServiceSchema = Joi.object()
   .description("The required attributes of a service. This is generally further defined by plugins.")
 
 export interface ServiceConfig<T extends ServiceSpec = ServiceSpec> extends CommonServiceSpec {
+  hotReloadable: boolean
   sourceModuleName?: string
+
   // Plugins can add custom fields that are kept here
   spec: T
 }
 
-export const serviceConfigSchema = baseServiceSchema
+export const serviceConfigSchema = baseServiceSpecSchema
   .keys({
+    hotReloadable: Joi.boolean()
+      .default(false)
+      .description("Set this to true if the module and service configuration supports hot reloading."),
     sourceModuleName: joiIdentifier()
       .optional()
       .description(deline`
@@ -57,3 +61,4 @@ export const serviceConfigSchema = baseServiceSchema
       .description("The service's specification, as defined by its provider plugin."),
   })
   .description("The configuration for a module's service.")
+  .unknown(false)

garden-service/src/plugins/container/config.ts

@@ -21,7 +21,7 @@ import {
 import { Service, ingressHostnameSchema } from "../../types/service"
 import { DEFAULT_PORT_PROTOCOL } from "../../constants"
 import { ModuleSpec, ModuleConfig, baseBuildSpecSchema, BaseBuildSpec } from "../../config/module"
-import { CommonServiceSpec, ServiceConfig, baseServiceSchema } from "../../config/service"
+import { CommonServiceSpec, ServiceConfig, baseServiceSpecSchema } from "../../config/service"
 import { baseTaskSpecSchema, BaseTaskSpec } from "../../config/task"
 import { baseTestSpecSchema, BaseTestSpec } from "../../config/test"
 import { joiStringMap } from "../../config/common"
@@ -194,7 +194,7 @@ const volumeSchema = Joi.object()
       .meta({ deprecated: true }),
   })
 
-const serviceSchema = baseServiceSchema
+const serviceSchema = baseServiceSpecSchema
   .keys({
     annotations: annotationsSchema
       .description("Annotations to attach to the service (Note: May not be applicable to all providers)"),

garden-service/src/plugins/container/container.ts

@@ -48,6 +48,8 @@ export async function configureContainerModule({ ctx, moduleConfig }: ConfigureM
     }
   }
 
+  const hotReloadable = !!moduleConfig.spec.hotReload
+
   // validate services
   moduleConfig.serviceConfigs = moduleConfig.spec.services.map(spec => {
     // make sure ports are correctly configured
@@ -91,7 +93,7 @@ export async function configureContainerModule({ ctx, moduleConfig }: ConfigureM
     return {
       name,
       dependencies: spec.dependencies,
-      outputs: spec.outputs,
+      hotReloadable,
       spec,
     }
   })

garden-service/src/plugins/google/google-cloud-functions.ts

@@ -31,10 +31,10 @@ import {
   GOOGLE_CLOUD_DEFAULT_REGION,
 } from "./common"
 import { GardenPlugin } from "../../types/plugin/plugin"
-import { baseServiceSchema, CommonServiceSpec } from "../../config/service"
+import { baseServiceSpecSchema, CommonServiceSpec } from "../../config/service"
 import { Provider, providerConfigBaseSchema } from "../../config/project"
 
-const gcfModuleSpecSchema = baseServiceSchema
+const gcfModuleSpecSchema = baseServiceSpecSchema
   .keys({
     entrypoint: Joi.string()
       .description("The entrypoint for the function (exported name in the function's module)"),
@@ -88,7 +88,7 @@ export async function configureGcfModule(
   moduleConfig.serviceConfigs = [{
     name,
     dependencies: spec.dependencies,
-    outputs: spec.outputs,
+    hotReloadable: false,
     spec,
   }]
 

garden-service/src/plugins/kubernetes/helm/config.ts

@@ -221,7 +221,8 @@ export async function validateHelmModule({ moduleConfig }: ConfigureModuleParams
     moduleConfig.serviceConfigs = [{
       name: moduleConfig.name,
       dependencies,
-      outputs: {},
+      // Note: We can't tell here if the source module supports hot-reloading, so we catch it in the handler if need be.
+      hotReloadable: !!sourceModuleName,
       sourceModuleName,
       spec: moduleConfig.spec,
     }]
@@ -285,7 +286,6 @@ export async function validateHelmModule({ moduleConfig }: ConfigureModuleParams
       name: spec.name,
       dependencies: spec.dependencies,
       timeout: spec.timeout,
-      env: spec.env,
       spec,
     }
   })

garden-service/src/plugins/kubernetes/kubernetes-module/config.ts

@@ -89,7 +89,7 @@ export async function configureKubernetesModule({ moduleConfig }: ConfigureModul
   moduleConfig.serviceConfigs = [{
     name: moduleConfig.name,
     dependencies: moduleConfig.spec.dependencies,
-    outputs: {},
+    hotReloadable: false,
     spec: moduleConfig.spec,
   }]
 

garden-service/src/plugins/local/local-google-cloud-functions.ts

@@ -72,6 +72,7 @@ export const gardenPlugin = (): GardenPlugin => ({
           return {
             name: spec.name,
             dependencies: spec.dependencies,
+            hotReloadable: false,
             outputs: spec.outputs,
             spec,
           }

garden-service/src/plugins/openfaas/openfaas.ts

@@ -192,12 +192,11 @@ export function gardenPlugin(): GardenPlugin {
 
           moduleConfig.serviceConfigs = [{
             dependencies: [],
+            hotReloadable: false,
             name: moduleConfig.name,
-            outputs: {},
             spec: {
               name: moduleConfig.name,
               dependencies: [],
-              outputs: {},
             },
           }]
 

garden-service/src/types/module.ts

@@ -27,6 +27,9 @@ export interface FileCopySpec {
   target: string
 }
 
+/**
+ * The Module interface adds several internally managed keys to the ModuleConfig type.
+ */
 export interface Module<
   M extends ModuleSpec = any,
   S extends ServiceSpec = any,

garden-service/src/types/plugin/params.ts

@@ -16,7 +16,7 @@ import { Module, moduleSchema } from "../module"
 import { RuntimeContext, Service, serviceSchema, runtimeContextSchema } from "../service"
 import { Task } from "../task"
 import { EnvironmentStatus, ServiceLogEntry, environmentStatusSchema } from "./outputs"
-import { moduleConfigSchema } from "../../config/module"
+import { baseModuleSpecSchema } from "../../config/module"
 import { testConfigSchema } from "../../config/test"
 import { taskSchema } from "../../config/task"
 import { ProviderConfig, projectNameSchema, providerConfigBaseSchema } from "../../config/project"
@@ -156,7 +156,7 @@ export const configureModuleParamsSchema = Joi.object()
     ctx: pluginContextSchema
       .required(),
     log: logEntrySchema,
-    moduleConfig: moduleConfigSchema
+    moduleConfig: baseModuleSpecSchema
       .required(),
   })
 

garden-service/src/types/service.ts

@@ -7,11 +7,10 @@
  */
 
 import * as Joi from "joi"
-import { getEnvVarName } from "../util/util"
+import { getEnvVarName, uniqByName } from "../util/util"
 import { PrimitiveMap, joiEnvVars, joiIdentifierMap, joiPrimitive, joiUserIdentifier } from "../config/common"
 import { Module, getModuleKey } from "./module"
-import { serviceOutputsSchema, ServiceConfig, serviceConfigSchema } from "../config/service"
-import { validate } from "../config/common"
+import { ServiceConfig, serviceConfigSchema } from "../config/service"
 import dedent = require("dedent")
 import { format } from "url"
 import { moduleVersionSchema } from "../vcs/vcs"
@@ -225,43 +224,31 @@ export async function prepareRuntimeContext(
     envVars[envVarName] = value
   }
 
-  const deps = {}
+  const output: RuntimeContext = {
+    envVars,
+    dependencies: {},
+  }
+
+  const deps = output.dependencies
+  const depModules = uniqByName([...buildDependencies, ...serviceDependencies.map(s => s.module)])
 
-  for (const m of buildDependencies) {
+  for (const m of depModules) {
     deps[m.name] = {
       version: m.version.versionString,
-      outputs: {},
+      outputs: m.outputs,
     }
   }
 
-  for (const dep of serviceDependencies) {
-    if (!deps[dep.name]) {
-      deps[dep.name] = {
-        version: dep.module.version.versionString,
-        outputs: {},
-      }
-    }
-    const depContext = deps[dep.name]
-
-    const outputs = {
-      ...dep.config.outputs,
-    }
-    const serviceEnvName = getEnvVarName(dep.name)
-
-    validate(outputs, serviceOutputsSchema, { context: `outputs for service ${dep.name}` })
-
-    for (const [key, value] of Object.entries(outputs)) {
-      const envVarName = `GARDEN_SERVICES_${serviceEnvName}_${key}`.toUpperCase()
+  for (const [name, dep] of Object.entries(deps)) {
+    const moduleEnvName = getEnvVarName(name)
 
+    for (const [key, value] of Object.entries(dep.outputs)) {
+      const envVarName = `GARDEN_MODULE_${moduleEnvName}__${key}`.toUpperCase()
       envVars[envVarName] = value
-      depContext.outputs[key] = value
     }
   }
 
-  return {
-    envVars,
-    dependencies: deps,
-  }
+  return output
 }
 
 export async function getServiceRuntimeContext(garden: Garden, graph: ConfigGraph, service: Service) {

garden-service/test/helpers.ts

@@ -107,7 +107,6 @@ export async function configureTestModule({ moduleConfig }: ConfigureModuleParam
   moduleConfig.serviceConfigs = moduleConfig.spec.services.map(spec => ({
     name: spec.name,
     dependencies: spec.dependencies,
-    outputs: spec.outputs,
     sourceModuleName: spec.sourceModuleName,
     spec,
   }))
@@ -241,7 +240,7 @@ const defaultModuleConfig: ModuleConfig = {
     {
       name: "test-service",
       dependencies: [],
-      outputs: {},
+      hotReloadable: false,
       spec: {},
     },
   ],

garden-service/test/unit/src/actions.ts

@@ -452,7 +452,7 @@ const testPlugin: PluginFactory = async () => ({
         const serviceConfigs = params.moduleConfig.spec.services.map(spec => ({
           name: spec.name,
           dependencies: spec.dependencies || [],
-          outputs: {},
+          hotReloadable: false,
           spec,
         }))