Skip to content

Commit

Permalink
fix: pvc modules/actions
Browse files Browse the repository at this point in the history 
* fix container module conversion
  * add volumes to runs and tests
  * add dependencies for pvc actions referenced
  * do not add volumes as dependencies for builds
* fix volume validation for container runtime actions
  • Loading branch information
@Orzelius
Orzelius committed Jun 6, 2023
1 parent 55f1ea9 commit c1b1531
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 31 deletions.
6 changes: 5 additions & 1 deletion core/src/plugins/container/config.ts
View file
Expand Up @@ -118,7 +118,7 @@ interface Annotations {
}

const deploymentStrategies = ["RollingUpdate", "Recreate"] as const
export type DeploymentStrategy = typeof deploymentStrategies[number]
export type DeploymentStrategy = (typeof deploymentStrategies)[number]
export const defaultDeploymentStrategy: DeploymentStrategy = "RollingUpdate"

export const commandExample = ["/bin/sh", "-c"]
Expand Down Expand Up @@ -1059,4 +1059,8 @@ export type ContainerActionConfig =
| ContainerBuildActionConfig

export type ContainerRuntimeAction = ContainerDeployAction | ContainerRunAction | ContainerTestAction
export type ContainerRuntimeActionConfig =
| ContainerDeployActionConfig
| ContainerRunActionConfig
| ContainerTestActionConfig
export type ContainerAction = ContainerRuntimeAction | ContainerBuildAction
82 changes: 54 additions & 28 deletions core/src/plugins/container/container.ts
View file
Expand Up @@ -16,6 +16,8 @@ import {
ContainerActionConfig,
ContainerBuildActionConfig,
ContainerModule,
ContainerModuleVolumeSpec,
ContainerRuntimeActionConfig,
containerModuleOutputsSchema,
containerModuleSpecSchema,
defaultDockerfileName,
Expand Down Expand Up @@ -209,7 +211,7 @@ function convertContainerModuleRuntimeActions(
convertParams: ConvertModuleParams<ContainerModule>,
buildAction: ContainerBuildActionConfig | ExecBuildConfig | undefined,
needsContainerBuild: boolean
): ContainerActionConfig[] {
) {
const { module, services, tasks, tests, prepareRuntimeDependencies } = convertParams
const actions: ContainerActionConfig[] = []

Expand All @@ -219,6 +221,24 @@ function convertContainerModuleRuntimeActions(
deploymentImageId = containerHelpers.getModuleDeploymentImageId(module, module.version, undefined)
}

const volumeModulesReferenced: string[] = []
function configureActionVolumes(action: ContainerRuntimeActionConfig, volumeSpec: ContainerModuleVolumeSpec[]) {
volumeSpec.forEach((v) => {
const referencedPvcAction = v.module ? { kind: <"Deploy">"Deploy", name: v.module } : undefined
action.spec.volumes.push({
...omit(v, "module"),
action: referencedPvcAction,
})
if (referencedPvcAction) {
action.dependencies?.push(referencedPvcAction)
}
if (v.module) {
volumeModulesReferenced.push(v.module)
}
})
return action
}

for (const service of services) {
const action: ContainerActionConfig = {
kind: "Deploy",
Expand All @@ -234,18 +254,14 @@ function convertContainerModuleRuntimeActions(
spec: {
...omit(service.spec, ["name", "dependencies", "disabled"]),
image: deploymentImageId,
volumes: [], // added later
volumes: [],
},
}
action.spec.volumes = service.config.spec.volumes.map((v) => ({
...omit(v, "module"),
action: v.module ? { kind: "Deploy", name: v.module } : undefined,
}))
actions.push(action)
actions.push(configureActionVolumes(action, service.config.spec.volumes))
}

for (const task of tasks) {
actions.push({
const action: ContainerActionConfig = {
kind: "Run",
type: "container",
name: task.name,
Expand All @@ -259,12 +275,14 @@ function convertContainerModuleRuntimeActions(
spec: {
...omit(task.spec, ["name", "dependencies", "disabled", "timeout"]),
image: needsContainerBuild ? undefined : module.spec.image,
volumes: [],
},
})
}
actions.push(configureActionVolumes(action, task.config.spec.volumes))
}

for (const test of tests) {
actions.push({
const action: ContainerActionConfig = {
kind: "Test",
type: "container",
name: module.name + "-" + test.name,
Expand All @@ -278,11 +296,13 @@ function convertContainerModuleRuntimeActions(
spec: {
...omit(test.spec, ["name", "dependencies", "disabled", "timeout"]),
image: needsContainerBuild ? undefined : module.spec.image,
volumes: [],
},
})
}
actions.push(configureActionVolumes(action, test.config.spec.volumes))
}

return actions
return { actions, volumeModulesReferenced }
}

export async function convertContainerModule(params: ConvertModuleParams<ContainerModule>) {
Expand Down Expand Up @@ -324,8 +344,15 @@ export async function convertContainerModule(params: ConvertModuleParams<Contain
actions.push(buildAction)
}

const runtimeActions = convertContainerModuleRuntimeActions(params, buildAction, needsContainerBuild)
const { actions: runtimeActions, volumeModulesReferenced } = convertContainerModuleRuntimeActions(
params,
buildAction,
needsContainerBuild
)
actions.push(...runtimeActions)
if (buildAction) {
buildAction.dependencies = buildAction?.dependencies?.filter((d) => !volumeModulesReferenced.includes(d.name))
}

return {
group: {
Expand Down Expand Up @@ -360,7 +387,7 @@ export const gardenPlugin = () =>
async getOutputs({ action }) {
// TODO: figure out why this cast is needed here
return {
outputs: (getContainerBuildActionOutputs(action) as unknown) as DeepPrimitiveMap,
outputs: getContainerBuildActionOutputs(action) as unknown as DeepPrimitiveMap,
}
},

Expand Down Expand Up @@ -429,17 +456,6 @@ export const gardenPlugin = () =>
}
}

for (const volume of spec.volumes) {
if (volume.action && !action.hasDependency(volume.action)) {
throw new ConfigurationError(
`${action.longDescription()} references action ${
volume.action
} under \`spec.volumes\` but does not declare a dependency on it. Please add an explicit dependency on the volume action.`,
{ spec }
)
}
}

return {}
},

Expand Down Expand Up @@ -559,8 +575,7 @@ export const gardenPlugin = () =>
{
platform: "windows",
architecture: "amd64",
url:
"https://github.com/rgl/docker-ce-windows-binaries-vagrant/releases/download/v20.10.9/docker-20.10.9.zip",
url: "https://github.com/rgl/docker-ce-windows-binaries-vagrant/releases/download/v20.10.9/docker-20.10.9.zip",
sha256: "360ca42101d453022eea17747ae0328709c7512e71553b497b88b7242b9b0ee4",
extract: {
format: "zip",
Expand All @@ -574,7 +589,7 @@ export const gardenPlugin = () =>

function validateRuntimeCommon(action: Resolved<ContainerRuntimeAction>) {
const { build } = action.getConfig()
const { image } = action.getSpec()
const { image, volumes } = action.getSpec()

if (!build && !image) {
throw new ConfigurationError(`${action.longDescription()} must specify one of \`build\` or \`spec.image\``, {
Expand All @@ -599,4 +614,15 @@ function validateRuntimeCommon(action: Resolved<ContainerRuntimeAction>) {
)
}
}

for (const volume of volumes) {
if (volume.action && !action.hasDependency(volume.action)) {
throw new ConfigurationError(
`${action.longDescription()} references action ${
volume.action
} under \`spec.volumes\` but does not declare a dependency on it. Please add an explicit dependency on the volume action.`,
{ volume }
)
}
}
}
4 changes: 2 additions & 2 deletions core/src/plugins/container/moduleConfig.ts
View file
Expand Up @@ -38,7 +38,7 @@ import { kebabCase, mapKeys } from "lodash"
// To reduce the amount of edits to make before removing module configs
export * from "./config"

interface ContainerModuleVolumeSpec extends ContainerVolumeSpecBase {
export interface ContainerModuleVolumeSpec extends ContainerVolumeSpecBase {
module?: string
}

Expand All @@ -55,7 +55,7 @@ export type ContainerTestSpec = BaseTestSpec &
volumes: ContainerModuleVolumeSpec[]
}
export const containerModuleTestSchema = () =>
baseTestSpecSchema().keys({ ...containerTestSpecKeys(), volumes: moduleVolumesSchema() })
baseTestSpecSchema().keys({ ...containerTestSpecKeys(), volumes: moduleVolumesSchema().default([]) })

export type ContainerTaskSpec = BaseTaskSpec &
ContainerRunActionSpec & {
Expand Down

0 comments on commit c1b1531

Please sign in to comment.