Skip to content

Commit

Permalink
chore: save
Browse files Browse the repository at this point in the history
  • Loading branch information
eysi09 committed Oct 20, 2023
1 parent acefb96 commit 5cf2ee6
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 21 deletions.
2 changes: 1 addition & 1 deletion core/src/actions/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,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
1 change: 1 addition & 0 deletions core/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ export const gardenEnv = {
GARDEN_WORKFLOW_RUN_UID: env.get("GARDEN_WORKFLOW_RUN_UID").required(false).asString(),
GARDEN_CLOUD_DOMAIN: env.get("GARDEN_CLOUD_DOMAIN").required(false).asUrlString(),
GARDEN_ENABLE_TRACING: env.get("GARDEN_ENABLE_TRACING").required(false).default("true").asBool(),
GARDEN_STORE_ACTION_TYPE: env.get("GARDEN_STORE_ACTION_TYPE").required(false).default("false").asBool(),
GARDEN_VERSION_CHECK_ENDPOINT: env
.get("GARDEN_VERSION_CHECK_ENDPOINT")
.required(false)
Expand Down
8 changes: 7 additions & 1 deletion core/src/events/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { Events, ActionStatusEventName } from "./events"
import { pick } from "lodash"
import { BuildState } from "../plugin/handlers/Build/get-status"
import { ActionStatusDetailedState, ActionCompleteState } from "./action-status-events"
import { gardenEnv } from "../constants"

type ActionKind = "build" | "deploy" | "run" | "test"

Expand Down Expand Up @@ -68,12 +69,17 @@ export function makeActionStatusPayloadBase({
startedAt: string
sessionId: string
}) {
// Due to an old bug, we were confusing type and kind in both Core and API.
// Latest versions of the API now handle this correctly, across all Core versions
// but there older versions still running that don't but are pending an update.
// Until then, this is "feature flagged".
const actionType = gardenEnv.GARDEN_STORE_ACTION_TYPE ? action.type : action.kind.toLowerCase()
return {
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,
actionUid: action.uid,
moduleName: action.moduleName(),
sessionId,
Expand Down
26 changes: 21 additions & 5 deletions core/src/graph/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -700,15 +700,17 @@ function dependenciesFromActionConfig(
const deps: ActionDependency[] = config.dependencies.map((d) => {
try {
const { kind, name } = parseActionReference(d)
return { kind, name, explicit: true, needsExecutedOutputs: false, needsStaticOutputs: false }
// FIXME @eysi: We're setting the "parent" config type as the dep type which is not correct
// and we need to the dep type.
return { kind, name, type: config.type, explicit: true, needsExecutedOutputs: false, needsStaticOutputs: false }
} catch (error) {
throw new ValidationError({
message: `Invalid dependency specified: ${error}`,
})
}
})

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

Expand All @@ -725,7 +727,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 @@ -738,7 +745,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 @@ -786,7 +797,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 @@ -36,6 +36,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 @@ -462,19 +463,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 @@ -483,7 +485,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 @@ -493,13 +495,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 @@ -524,11 +528,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 @@ -549,6 +554,7 @@ export class ConfigGraphNode {

constructor(
public kind: ActionKind,
public type: string,
public name: string,
public disabled: boolean
) {
Expand All @@ -559,6 +565,7 @@ export class ConfigGraphNode {
render(): RenderedNode {
return {
name: this.name,
type: this.type,
kind: this.kind,
key: this.name,
disabled: this.disabled,
Expand Down
10 changes: 5 additions & 5 deletions core/test/unit/src/config-graph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1567,7 +1567,7 @@ describe("ConfigGraph (module-based configs)", () => {
describe("ConfigGraphNode", () => {
describe("render", () => {
it("should render a build node", () => {
const node = new ConfigGraphNode("Build", "module-a", false)
const node = new ConfigGraphNode("Build", "container", "module-a", false)
const res = node.render()
expect(res).to.eql({
kind: "Build",
Expand All @@ -1578,7 +1578,7 @@ describe("ConfigGraphNode", () => {
})

it("should render a deploy node", () => {
const node = new ConfigGraphNode("Deploy", "service-a", false)
const node = new ConfigGraphNode("Deploy", "container", "service-a", false)
const res = node.render()
expect(res).to.eql({
kind: "Deploy",
Expand All @@ -1589,7 +1589,7 @@ describe("ConfigGraphNode", () => {
})

it("should render a run node", () => {
const node = new ConfigGraphNode("Run", "task-a", false)
const node = new ConfigGraphNode("Run", "container", "task-a", false)
const res = node.render()
expect(res).to.eql({
kind: "Run",
Expand All @@ -1600,7 +1600,7 @@ describe("ConfigGraphNode", () => {
})

it("should render a test node", () => {
const node = new ConfigGraphNode("Test", "module-a.test-a", false)
const node = new ConfigGraphNode("Test", "container", "module-a.test-a", false)
const res = node.render()
expect(res).to.eql({
kind: "Test",
Expand All @@ -1611,7 +1611,7 @@ describe("ConfigGraphNode", () => {
})

it("should indicate if the node is disabled", () => {
const node = new ConfigGraphNode("Test", "module-a.test-a", true)
const node = new ConfigGraphNode("Test", "container", "module-a.test-a", true)
const res = node.render()
expect(res).to.eql({
kind: "Test",
Expand Down
6 changes: 3 additions & 3 deletions core/test/unit/src/plugins/container/container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,15 +111,15 @@ describe("plugins.container", () => {
basePath: "module-a",
},
},
convertBuildDependency: () => ({ kind: "Build", name: "buildDep" }),
convertRuntimeDependencies: () => [{ kind: "Deploy", name: "runtimeDep" }],
convertBuildDependency: () => ({ kind: "Build", type: "container", name: "buildDep" }),
convertRuntimeDependencies: () => [{ kind: "Deploy", type: "container", name: "runtimeDep" }],
convertTestName: () => "testName",
ctx,
dummyBuild: undefined,
log,
module,
prepareRuntimeDependencies: prepareRuntimeDependencies
? () => [{ kind: "Deploy", name: "preopRuntimeDep" }]
? () => [{ kind: "Deploy", type: "container", name: "preopRuntimeDep" }]
: () => [],
services,
tasks,
Expand Down

0 comments on commit 5cf2ee6

Please sign in to comment.