feat(core): expose service and task dependency outputs at runtime #1123

edvald · 2019-08-23T15:07:58Z

What this PR does / why we need it:

This is a fairly substantial addition, which allows users to reference
runtime outputs from services and tasks in dependant services and tasks.
It involves adding a couple of new TaskGraph tasks, so that we can
resolve service statuses and task results in dependency order. We also
substantially overhaul the usage and testing of runtime contexts across
the board, since it is now a more fundamental feature.

The usage is documented in a new section in the Configuration Files
guide, please start with that when reviewing.

Which issue(s) this PR fixes:

Closes #1095

eysi09

Had a few comments and questions.

Also, would we also want to include test outputs?

Another thing I noticed is that garden init starts checking the status for kubernetes-dashboard et al but then just stops, like so:

➜  demo-project (runtime-outputs) g init
Initializing local environment ⚙️

✔ providers                 → Getting status... → Done
   ✔ garden-system             → Initializing...
      ✔ providers                 → Getting status... → Done
    kubernetes-dashboard      → Checking status...
    default-backend           → Checking status...
    ingress-controller        → Checking status...

Done! ✔️

eysi09 · 2019-08-27T11:37:38Z

docs/using-garden/configuration-files.md

@@ -457,6 +457,36 @@ services:
 For a full reference of the keys available in template strings, please look at the
 [Template Strings Reference](../reference/template-strings.md).

+#### Runtime outputs


We can do it in a separate PR but we really need to split this doc up.

True. But in a separate PR for sure. Maybe an Advanced configuration guide would be good.

eysi09 · 2019-08-27T11:52:09Z

docs/using-garden/configuration-files.md

@@ -457,6 +457,36 @@ services:
 For a full reference of the keys available in template strings, please look at the
 [Template Strings Reference](../reference/template-strings.md).

+#### Runtime outputs
+
+Template keys prefixed with `runtime.` have some special semantics. They are used to expose runtime outputs from services and tasks, and therefore are resolved later than other template strings. _This means that you cannot use them for some fields, such as most identifiers, because those need to be resolved before validating the configuration._


How does the user know what fields exactly can use runtime. template strings?

That's tricky, because it's going to be quite dependent on individual module type schemas. So we can't describe it here, nor can we easily flag this in schemas.

eysi09 · 2019-08-27T12:11:33Z

docs/reference/module-types/container.md

@@ -1132,3 +1133,18 @@ outputs:
  ...
  deployment-image-name: "my-deployment-registry.io/my-org/my-module"
 ```
+
+
+### Task outputs


Shouldn't we also add service outputs here?

We do in the generator, there just aren't any in this case.

eysi09 · 2019-08-27T12:15:24Z

docs/reference/module-types/exec.md

+The following keys are available via the `${runtime.tasks.<task-name>}` template string key for `exec` module tasks.
+Note that these are only resolved when deploying/running dependants of the task, so they are not usable for every field.
+
+### `runtime.tasks.<task-name>.outputs.log`


Are we foreseeing any other output types than log? Also log feels a little off to me, would stdout be clearer?

Feels like this could just be runtime.tasks<task-name>.stdout (or runtime.tasks<task-name>.output), but maybe I'm missing something.

Not for this module type in particular, but tasks could feasibly have any number of outputs. Say if a provider outputs file paths for artifacts, particular metadata etc.

eysi09 · 2019-08-27T12:28:07Z

garden-service/src/runtime-context.ts

+export async function prepareRuntimeContext(
+  { garden, module, dependencies, serviceStatuses, taskResults }: PrepareRuntimeContextParams,
+): Promise<RuntimeContext> {
+  // const buildDepKeys = module.build.dependencies.map(dep => getModuleKey(dep.name, dep.plugin))


Remove this?

eysi09 · 2019-08-27T12:28:43Z

garden-service/src/runtime-context.ts

+  taskResults: { [name: string]: RunTaskResult }
+}
+
+export async function prepareRuntimeContext(


It would be nice to better document the runtimeContext and how we use it.

eysi09 · 2019-08-27T12:33:39Z

garden-service/src/actions.ts

@@ -550,9 +572,28 @@ export class ActionHelper implements TypeGuard {
      defaultHandler,
    })

+    // Resolve ${runtime.*} template strings if needed.
+    if (runtimeContext && (await getRuntimeTemplateReferences(module)).length > 0) {


Feels like this and the code above could be de-duplicated.

Feels like, yes, but I felt the code is subtly different enough for an abstraction to be unnecessary.

eysi09 · 2019-08-27T12:45:25Z

garden-service/src/tasks/base.ts

+export function getRunTaskResults(dependencyResults: TaskResults): { [name: string]: RunTaskResult } {
+  const storedResults = pickBy(dependencyResults, r => r.type === "get-task-result")
+  const runResults = pickBy(dependencyResults, r => r.type === "task")
+  // TaskTask results take precedence over GetTaskResultTask results


That's a mouthful :)

Yeah, once we rename the TaskGraph to ActionGraph and *Tasks to *Actions this will be more digestable :P

edvald · 2019-08-27T15:07:10Z

I'll take a look at the Getting status... issue, hadn't noticed that when testing.

Re: Test outputs, we could add them as well, but there was no immediately obvious use case—you could only pass test outputs to other tests, and I'm not sure if/how that's useful—so I figured we could fairly easily add those later.

This is a fairly substantial addition, which allows users to reference runtime outputs from services and tasks in dependant services and tasks. It involves adding a couple of new TaskGraph tasks, so that we can resolve service statuses and task results in dependency order. We also substantially overhaul the usage and testing of runtime contexts across the board, since it is now a more fundamental feature. The usage is documented in a new section in the Configuration Files guide, please start with that when reviewing.

edvald force-pushed the runtime-outputs branch from 1282fe3 to 86e1a01 Compare August 23, 2019 15:18

thsig self-assigned this Aug 23, 2019

edvald mentioned this pull request Aug 23, 2019

Terraform provider #1096

Merged

eysi09 reviewed Aug 27, 2019

View reviewed changes

edvald force-pushed the runtime-outputs branch 2 times, most recently from 252c79b to 10e37fe Compare August 27, 2019 15:39

eysi09 approved these changes Aug 27, 2019

View reviewed changes

edvald added 3 commits August 28, 2019 14:50

fix: review fixes (TBS)

f8fe739

fix: logger issues (TBS)

7f830dd

edvald force-pushed the runtime-outputs branch from fe91d94 to 7f830dd Compare August 28, 2019 13:04

edvald merged commit fca6a72 into master Aug 28, 2019

edvald deleted the runtime-outputs branch August 28, 2019 15:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(core): expose service and task dependency outputs at runtime #1123

feat(core): expose service and task dependency outputs at runtime #1123

edvald commented Aug 23, 2019

eysi09 left a comment

eysi09 Aug 27, 2019

edvald Aug 27, 2019

eysi09 Aug 27, 2019

edvald Aug 27, 2019

eysi09 Aug 27, 2019

edvald Aug 27, 2019

eysi09 Aug 27, 2019

eysi09 Aug 27, 2019

edvald Aug 27, 2019

eysi09 Aug 27, 2019

eysi09 Aug 27, 2019

eysi09 Aug 27, 2019

edvald Aug 27, 2019

eysi09 Aug 27, 2019

edvald Aug 27, 2019

edvald commented Aug 27, 2019

feat(core): expose service and task dependency outputs at runtime #1123

feat(core): expose service and task dependency outputs at runtime #1123

Conversation

edvald commented Aug 23, 2019

eysi09 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edvald commented Aug 27, 2019