Skip to content

Commit

Permalink
fix(build): validate helm modules at build rather than configure phase
Browse files Browse the repository at this point in the history
This addresses #1382 by moving the check for whether or not a helm module contains sources to the build phase, rather than the configure phase.

    This also moves the the logic for syncing dependency products to the build phase rather than the "stage build" phase.

 This corrects the case where a module's build depends on artifacts generated by and copied over from a dependency.
  • Loading branch information
khaled authored and edvald committed Dec 14, 2019
1 parent 4d2ccce commit 7551646
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 16 deletions.
13 changes: 11 additions & 2 deletions garden-service/src/plugins/kubernetes/helm/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@
*/

import { HelmModule } from "./config"
import { containsSource, getChartPath, getGardenValuesPath, getBaseModule } from "./common"
import { containsBuildSource, getChartPath, getGardenValuesPath, getBaseModule } from "./common"
import { helm } from "./helm-cli"
import { ConfigurationError } from "../../../exceptions"
import { deline } from "../../../util/string"
import { dumpYaml } from "../../../util/util"
import { LogEntry } from "../../../logger/log-entry"
import { getNamespace } from "../namespace"
Expand All @@ -26,7 +28,14 @@ export async function buildHelmModule({ ctx, module, log }: BuildModuleParams<He
})
const baseModule = getBaseModule(module)

if (!baseModule && !(await containsSource(module))) {
if (!baseModule && !(await containsBuildSource(module))) {
if (!module.spec.chart) {
throw new ConfigurationError(
deline`Module '${module.name}' neither specifies a chart name, base module,
nor contains chart sources at \`chartPath\`.`,
{ module }
)
}
log.debug("Fetching chart...")
try {
await fetchChart(k8sCtx, namespace, log, module)
Expand Down
18 changes: 15 additions & 3 deletions garden-service/src/plugins/kubernetes/helm/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,24 @@ import { MAX_RUN_RESULT_OUTPUT_LENGTH } from "../constants"

const gardenValuesFilename = "garden-values.yml"

async function containsChart(basePath: string, config: HelmModuleConfig) {
const yamlPath = join(basePath, config.spec.chartPath, "Chart.yaml")
return pathExists(yamlPath)
}

/**
* Returns true if the specified Helm module contains a template (as opposed to just referencing a remote template).
*/
export async function containsSource(config: HelmModuleConfig) {
const yamlPath = join(config.path, config.spec.chartPath, "Chart.yaml")
return pathExists(yamlPath)
return containsChart(config.path, config)
}

/**
* Returns true if the specified Helm module contains a template in its build path (as opposed to just referencing
* a remote template).
*/
export async function containsBuildSource(module: HelmModule) {
return containsChart(module.buildPath, module)
}

/**
Expand Down Expand Up @@ -118,7 +130,7 @@ export async function getChartPath(module: HelmModule) {

if (baseModule) {
return join(module.buildPath, baseModule.spec.chartPath)
} else if (await containsSource(module)) {
} else if (await containsBuildSource(module)) {
return join(module.buildPath, module.spec.chartPath)
} else {
// This value is validated to exist in the validate module action
Expand Down
10 changes: 1 addition & 9 deletions garden-service/src/plugins/kubernetes/helm/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ export const helmModuleSpecSchema = joi.object().keys({
export async function configureHelmModule({
moduleConfig,
}: ConfigureModuleParams<HelmModule>): Promise<ConfigureModuleResult<HelmModule>> {
const { base, chart, dependencies, serviceResource, skipDeploy, tasks, tests } = moduleConfig.spec
const { base, dependencies, serviceResource, skipDeploy, tasks, tests } = moduleConfig.spec

const sourceModuleName = serviceResource ? serviceResource.containerModule : undefined

Expand All @@ -292,14 +292,6 @@ export async function configureHelmModule({

const containsSources = await containsSource(moduleConfig)

if (!chart && !base && !containsSources) {
throw new ConfigurationError(
deline`Module '${moduleConfig.name}' neither specifies a chart name, base module,
nor contains chart sources at \`chartPath\`.`,
{ moduleConfig }
)
}

// Make sure referenced modules are included as build dependencies
// (This happens automatically for the service source module).
function addBuildDependency(name: string, copy?: FileCopySpec[]) {
Expand Down
3 changes: 3 additions & 0 deletions garden-service/src/tasks/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,9 @@ export class BuildTask extends BaseTask {
log.setState(`Building version ${module.version.versionString}...`)
}

const graph = await this.garden.getConfigGraph(log)
await this.garden.buildDir.syncDependencyProducts(this.module, graph, log)

let result: BuildResult
try {
result = await actions.build({
Expand Down
2 changes: 0 additions & 2 deletions garden-service/src/tasks/stage-build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,7 @@ export class StageBuildTask extends BaseTask {
})
}

const graph = await this.garden.getConfigGraph(log || this.log)
await this.garden.buildDir.syncFromSrc(this.module, log || this.log)
await this.garden.buildDir.syncDependencyProducts(this.module, graph, log || this.log)

if (log) {
log.setSuccess({
Expand Down

0 comments on commit 7551646

Please sign in to comment.