Skip to content

Commit

Permalink
fix(core): fix watch task logic for sourceModules (#1756)
Browse files Browse the repository at this point in the history
Before this fix, if a service with a sourceModule was deployed with hot
reloading enabled, a build task for its source module would be added
when its sources change (in addition to the hot reload task).

This was not the desired behavior, and would (e.g. for helm modules
using a container module as a sourceModule) result in unnecessary
rebuilds of the source module.
  • Loading branch information
thsig authored Apr 1, 2020
1 parent bb923ad commit 1f189bb
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 2 deletions.
28 changes: 26 additions & 2 deletions garden-service/src/tasks/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,29 @@ export async function getModuleWatchTasks({

const dependants = graph.getDependantsForModule(module, true)

if (intersection(module.serviceNames, hotReloadServiceNames).length === 0) {
const dependantSourceModules = dependants.build.filter((depModule) =>
depModule.serviceConfigs.find((s) => s.sourceModuleName === module.name)
)

const dependantSourceModuleServiceNames = flatten(
dependantSourceModules.map((depModule) => {
return depModule.serviceConfigs.filter((s) => s.sourceModuleName === module.name).map((s) => s.name)
})
)

const serviceNamesUsingModule = [...module.serviceNames, ...dependantSourceModuleServiceNames]

/**
* If a service is deployed with hot reloading enabled, we don't rebuild its module
* (or its sourceModule, if the service instead uses a sourceModule) when its
* sources change.
*
* Therefore, we skip adding a build task for module if one of its services is in
* hotReloadServiceNames, or if one of its build dependants' services is in
* hotReloadServiceNames and has module as its sourceModule (in which case we
* also don't add a build task for the dependant's module below).
*/
if (intersection(serviceNamesUsingModule, hotReloadServiceNames).length === 0) {
buildTasks = await BuildTask.factory({
garden,
graph,
Expand All @@ -48,9 +70,11 @@ export async function getModuleWatchTasks({
})
}

const dependantSourceModuleNames = dependantSourceModules.map((m) => m.name)

const dependantBuildTasks = flatten(
await Bluebird.map(
dependants.build.filter((m) => !m.disabled),
dependants.build.filter((m) => !m.disabled && !dependantSourceModuleNames.includes(m.name)),
(m) =>
BuildTask.factory({
garden,
Expand Down
59 changes: 59 additions & 0 deletions garden-service/test/unit/src/tasks/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,65 @@ describe("TaskHelpers", () => {
expect(sortedBaseKeys(tasks)).to.eql(["build.module-a", "deploy.service-a"])
})

it("should not add a build task for a hot-reload-enabled service's sourceModule", async () => {
const garden = await makeTestGardenA()

garden.setModuleConfigs([
{
apiVersion: DEFAULT_API_VERSION,
allowPublish: false,
build: { dependencies: [] },
disabled: false,
name: "module-a",
include: [],
outputs: {},
path: garden.projectRoot,
serviceConfigs: [],
taskConfigs: [],
spec: { services: [] },
testConfigs: [],
type: "test",
},
{
apiVersion: DEFAULT_API_VERSION,
allowPublish: false,
build: { dependencies: [{ name: "module-a", copy: [] }] },
disabled: false,
name: "module-b",
include: [],
outputs: {},
path: garden.projectRoot,
serviceConfigs: [],
taskConfigs: [],
spec: {
services: [
{
name: "service-b",
dependencies: [],
disabled: false,
sourceModule: "module-a", // <---------------
},
],
},
testConfigs: [],
type: "test",
},
])

const graph = await garden.getConfigGraph(garden.log)
const module = await graph.getModule("module-b", true)

const tasks = await getModuleWatchTasks({
garden,
graph,
log,
module,
hotReloadServiceNames: ["service-b"],
})

expect(sortedBaseKeys(tasks)).to.eql(["hot-reload.service-b"])
})

context("without hot reloading enabled", () => {
const expectedBaseKeysByChangedModule = [
{
Expand Down

0 comments on commit 1f189bb

Please sign in to comment.