Skip to content

Commit

Permalink
improvement(api): send action type to cloud (#5447)
Browse files Browse the repository at this point in the history
Before this improvement we weren't sending the action type to Cloud due
to some legacy issues which have since been resolved.

The action type is used in Cloud in e.g. the graph UI and on command
results.

Note that Cloud has already been updated to handle the action type and
those changes rolled out to all users, so this is only required Core
side work.
  • Loading branch information
eysi09 authored Oct 10, 2024
1 parent dcfef40 commit 15fe974
Show file tree
Hide file tree
Showing 14 changed files with 152 additions and 73 deletions.
2 changes: 1 addition & 1 deletion core/src/actions/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ export interface ActionDependencyAttributes {
needsExecutedOutputs: boolean // Set to true if action cannot be resolved without the dependency executed
}

export type ActionDependency = ActionReference & ActionDependencyAttributes
export type ActionDependency = ActionReference & ActionDependencyAttributes & { type: string }

export interface ActionModes {
sync?: boolean
Expand Down
2 changes: 1 addition & 1 deletion core/src/events/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ export function makeActionStatusPayloadBase({
actionName: action.name,
actionVersion: action.versionString(),
// NOTE: The type/kind needs to be lower case in the event payload
actionType: action.kind.toLowerCase(),
actionKind: action.kind.toLowerCase(),
actionType: action.type,
actionUid: action.uid,
moduleName: action.moduleName(),
sessionId,
Expand Down
46 changes: 35 additions & 11 deletions core/src/graph/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ import { describeSchema, parseActionReference } from "../config/common.js"
import type { GroupConfig } from "../config/group.js"
import { ActionConfigContext } from "../config/template-contexts/actions.js"
import { validateWithPath } from "../config/validation.js"
import { ConfigurationError, PluginError, InternalError, ValidationError, GardenError } from "../exceptions.js"
import { ConfigurationError, PluginError, InternalError, GardenError } from "../exceptions.js"
import { overrideVariables, type Garden } from "../garden.js"
import type { Log } from "../logger/log-entry.js"
import type { ActionTypeDefinition } from "../plugin/action-types.js"
Expand Down Expand Up @@ -916,17 +916,27 @@ function dependenciesFromActionConfig({
}

const deps: ActionDependency[] = config.dependencies.map((d) => {
try {
const { kind, name } = parseActionReference(d)
return { kind, name, explicit: true, needsExecutedOutputs: false, needsStaticOutputs: false }
} catch (error) {
throw new ValidationError({
message: `Invalid dependency specified: ${error}`,
const { kind, name } = parseActionReference(d)
const depKey = actionReferenceToString(d)
const depConfig = configsByKey[depKey]

if (!depConfig) {
throw new ConfigurationError({
message: `${description} references depdendency ${depKey}, but no such action could be found`,
})
}

return {
kind,
name,
type: depConfig.type,
explicit: true,
needsExecutedOutputs: false,
needsStaticOutputs: false,
}
})

function addDep(ref: ActionReference, attributes: ActionDependencyAttributes) {
function addDep(ref: ActionReference & { type: string }, attributes: ActionDependencyAttributes) {
addActionDependency({ ...ref, ...attributes }, deps)
}

Expand All @@ -943,7 +953,12 @@ function dependenciesFromActionConfig({
})
}

addDep(ref, { explicit: true, needsExecutedOutputs: false, needsStaticOutputs: false })
const refWithType = {
...ref,
type: config.type,
}

addDep(refWithType, { explicit: true, needsExecutedOutputs: false, needsStaticOutputs: false })
}
} else if (config.build) {
// -> build field on runtime actions
Expand All @@ -956,7 +971,11 @@ function dependenciesFromActionConfig({
})
}

addDep(ref, { explicit: true, needsExecutedOutputs: false, needsStaticOutputs: false })
const refWithType = {
...ref,
type: config.type,
}
addDep(refWithType, { explicit: true, needsExecutedOutputs: false, needsStaticOutputs: false })
}

// Action template references in spec/variables
Expand Down Expand Up @@ -1003,7 +1022,12 @@ function dependenciesFromActionConfig({
needsExecuted = true
}

addDep(ref, { explicit: false, needsExecutedOutputs: needsExecuted, needsStaticOutputs: !needsExecuted })
const refWithType = {
...ref,
type: refActionType,
}

addDep(refWithType, { explicit: false, needsExecutedOutputs: needsExecuted, needsStaticOutputs: !needsExecuted })
}

return deps
Expand Down
19 changes: 13 additions & 6 deletions core/src/graph/config-graph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export type RenderedEdge = { dependant: RenderedNode; dependency: RenderedNode }
export interface RenderedNode {
kind: ActionKind
name: string
type: string
moduleName?: string
key: string
disabled: boolean
Expand Down Expand Up @@ -481,19 +482,20 @@ export abstract class BaseConfigGraph<

protected addActionInternal<K extends ActionKind>(action: Action) {
this.actions[action.kind][action.name] = <PickTypeByKind<K, B, D, R, T>>action
const node = this.getNode(action.kind, action.name, action.isDisabled())
const node = this.getNode(action.kind, action.type, action.name, action.isDisabled())

for (const dep of action.getDependencyReferences()) {
this.addRelation({
dependant: node,
dependencyKind: dep.kind,
dependencyType: dep.type,
dependencyName: dep.name,
})
}
}

// Idempotent.
protected getNode(kind: ActionKind, name: string, disabled: boolean) {
protected getNode(kind: ActionKind, type: string, name: string, disabled: boolean) {
const key = nodeKey(kind, name)
const existingNode = this.dependencyGraph[key]
if (existingNode) {
Expand All @@ -502,7 +504,7 @@ export abstract class BaseConfigGraph<
}
return existingNode
} else {
const newNode = new ConfigGraphNode(kind, name, disabled)
const newNode = new ConfigGraphNode(kind, type, name, disabled)
this.dependencyGraph[key] = newNode
return newNode
}
Expand All @@ -512,13 +514,15 @@ export abstract class BaseConfigGraph<
protected addRelation({
dependant,
dependencyKind,
dependencyType,
dependencyName,
}: {
dependant: ConfigGraphNode
dependencyKind: ActionKind
dependencyType: string
dependencyName: string
}) {
const dependency = this.getNode(dependencyKind, dependencyName, false)
const dependency = this.getNode(dependencyKind, dependencyType, dependencyName, false)
dependant.addDependency(dependency)
dependency.addDependant(dependant)
}
Expand All @@ -543,11 +547,12 @@ export class MutableConfigGraph extends ConfigGraph {
const dependant = this.getActionByRef(by)
const dependency = this.getActionByRef(on)

dependant.addDependency({ kind: dependency.kind, name: dependency.name, ...attributes })
dependant.addDependency({ kind: dependency.kind, type: dependency.type, name: dependency.name, ...attributes })

this.addRelation({
dependant: this.getNode(dependant.kind, dependant.name, dependant.isDisabled()),
dependant: this.getNode(dependant.kind, dependant.type, dependant.name, dependant.isDisabled()),
dependencyKind: dependency.kind,
dependencyType: dependency.type,
dependencyName: dependency.name,
})
}
Expand All @@ -568,6 +573,7 @@ export class ConfigGraphNode {

constructor(
public kind: ActionKind,
public type: string,
public name: string,
public disabled: boolean
) {
Expand All @@ -581,6 +587,7 @@ export class ConfigGraphNode {
kind: this.kind,
key: this.name,
disabled: this.disabled,
type: this.type,
}
}

Expand Down
14 changes: 14 additions & 0 deletions core/src/util/testing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -300,11 +300,25 @@ export class TestGarden extends Garden {
this.addActionConfig(config)
}

/**
* Replace all module configs with the one provided.
*/
setModuleConfigs(moduleConfigs: PartialModuleConfig[]) {
this.state.configsScanned = true
this.moduleConfigs = keyBy(moduleConfigs.map(moduleConfigWithDefaults), "name")
}

/**
* Merge the provided module configs with the existing ones.
*/
mergeModuleConfigs(moduleConfigs: PartialModuleConfig[]) {
this.state.configsScanned = true
this.moduleConfigs = {
...this.moduleConfigs,
...keyBy(moduleConfigs.map(moduleConfigWithDefaults), "name"),
}
}

setActionConfigs(actionConfigs: PartialActionConfig[]) {
this.actionConfigs = {
Build: {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ tasks:
command: [sh, -c, "echo ok"]

---

kind: Run
name: echo-run-exec
type: kubernetes-exec
Expand All @@ -56,7 +55,6 @@ spec:
command: [echo, ok]

---

kind: Test
name: echo-test-exec
type: kubernetes-exec
Expand All @@ -66,4 +64,4 @@ spec:
resource:
kind: Deployment
name: busybox-deployment
command: [echo, ok]
command: [echo, ok]
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ describe("kubernetes-type handlers", () => {
}

async function deployInNamespace({ nsName, deployName }: { nsName: string; deployName: string }) {
garden.setModuleConfigs([withNamespace(nsModuleConfig, nsName)])
garden.mergeModuleConfigs([withNamespace(nsModuleConfig, nsName)])
const graph = await garden.getConfigGraph({ log, emit: false })
const action = graph.getDeploy(deployName)
const resolvedAction = await garden.resolveAction<KubernetesDeployAction>({ action, log: garden.log, graph })
Expand Down
5 changes: 5 additions & 0 deletions core/test/unit/src/actions/action-configs-to-graph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ describe("actionConfigsToGraph", () => {
explicit: true,
kind: "Build",
name: "foo",
type: "test",
needsExecutedOutputs: false,
needsStaticOutputs: false,
},
Expand Down Expand Up @@ -283,6 +284,7 @@ describe("actionConfigsToGraph", () => {
{
explicit: true,
kind: "Build",
type: "test",
name: "foo",
needsExecutedOutputs: false,
needsStaticOutputs: false,
Expand Down Expand Up @@ -331,6 +333,7 @@ describe("actionConfigsToGraph", () => {
{
explicit: false,
kind: "Build",
type: "test",
name: "foo",
fullRef: ["actions", "build", "foo", "version"],
needsExecutedOutputs: false,
Expand Down Expand Up @@ -380,6 +383,7 @@ describe("actionConfigsToGraph", () => {
{
explicit: false,
kind: "Build",
type: "test",
name: "foo",
fullRef: ["actions", "build", "foo", "outputs", "bar"],
needsExecutedOutputs: true,
Expand Down Expand Up @@ -429,6 +433,7 @@ describe("actionConfigsToGraph", () => {
{
explicit: false,
kind: "Build",
type: "container",
name: "foo",
fullRef: ["actions", "build", "foo", "outputs", "deploymentImageName"],
needsExecutedOutputs: false,
Expand Down
Loading

0 comments on commit 15fe974

Please sign in to comment.