Skip to content

Commit b57d1eb

Browse files
kindredjcharterskindredjcharters
committed
feat: [KSBP-101776] - Enhance parallel deployment logic and testing
1 parent de78384 commit b57d1eb

2 files changed

Lines changed: 246 additions & 44 deletions

File tree

‎k8s-deployer/src/test-suite-handler.ts‎

Lines changed: 35 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ export const generatePrefixByDate = (date: Date, env: string): Prefix => {
4040
* 1. all components in the graph in topological order, deploying parallel-flagged components concurrently within each level
4141
* 2. test app for the graph.
4242
*/
43-
const deployGraph = async (config: Config, workspace: string, testSuiteId: string, graph: Schema.Graph, namespace: Namespace, testAppDirForRemoteTestSuite?: string): Promise<GraphDeploymentResult> => {
43+
export const deployGraph = async (config: Config, workspace: string, testSuiteId: string, graph: Schema.Graph, namespace: Namespace, testAppDirForRemoteTestSuite?: string): Promise<GraphDeploymentResult> => {
4444
// Dependencies are already validated in main(), so it's safe to directly sort here.
4545
const { sortedComponents, levels } = topologicalSort(graph.components)
4646

@@ -59,39 +59,47 @@ const deployGraph = async (config: Config, workspace: string, testSuiteId: strin
5959
graph.testApp.location.path = testAppDirForRemoteTestSuite
6060
}
6161

62-
// Deploy components level by level. Within each level, components with parallel:true are deployed concurrently.
62+
// Deploy components level by level.
63+
// Within each level, parallel-flagged components run concurrently with each other AND with the
64+
// sequential chain — both groups start at the same time and the level only advances once both
65+
// are done.
6366
const deployments: Array<DeployedComponent> = []
6467
let componentIndex = 0
6568
const deployComponentsPromise = (async () => {
6669
for (const level of levels) {
6770
const parallelGroup = level.filter(c => c.parallel === true)
6871
const sequentialGroup = level.filter(c => c.parallel !== true)
6972

70-
// Deploy all parallel-flagged components in this level concurrently
71-
if (parallelGroup.length > 0) {
72-
logger.info("")
73-
logger.info("Deploying %d component(s) in parallel for suite \"%s\": %s", parallelGroup.length, testSuiteId, parallelGroup.map(c => c.id).join(", "))
74-
const parallelResults = await Promise.all(
75-
parallelGroup.map(async componentSpec => {
76-
const idx = ++componentIndex
77-
logger.info("Deploying graph component (%s of %s) \"%s\" for suite \"%s\"...", idx, sortedComponents.length, componentSpec.name, testSuiteId)
78-
const commitSha = await Deployer.deployComponent(config, workspace, componentSpec, namespace)
79-
logger.info("Graph component \"%s\" for suite \"%s\" deployed.", componentSpec.name, testSuiteId)
80-
return new DeployedComponent(commitSha, componentSpec)
81-
})
82-
)
83-
deployments.push(...parallelResults)
84-
}
85-
86-
// Deploy sequential components one by one
87-
for (const componentSpec of sequentialGroup) {
88-
const idx = ++componentIndex
89-
logger.info("")
90-
logger.info("Deploying graph component (%s of %s) \"%s\" for suite \"%s\"...", idx, sortedComponents.length, componentSpec.name, testSuiteId)
91-
logger.info("")
92-
const commitSha = await Deployer.deployComponent(config, workspace, componentSpec, namespace)
93-
deployments.push(new DeployedComponent(commitSha, componentSpec))
94-
}
73+
// Fire both groups concurrently; await both before moving to the next level.
74+
const parallelChain = parallelGroup.length > 0
75+
? (async () => {
76+
logger.info("")
77+
logger.info("Deploying %d component(s) in parallel for suite \"%s\": %s", parallelGroup.length, testSuiteId, parallelGroup.map(c => c.id).join(", "))
78+
const results = await Promise.all(
79+
parallelGroup.map(async componentSpec => {
80+
const idx = ++componentIndex
81+
logger.info("Deploying graph component (%s of %s) \"%s\" for suite \"%s\"...", idx, sortedComponents.length, componentSpec.name, testSuiteId)
82+
const commitSha = await Deployer.deployComponent(config, workspace, componentSpec, namespace)
83+
logger.info("Graph component \"%s\" for suite \"%s\" deployed.", componentSpec.name, testSuiteId)
84+
return new DeployedComponent(commitSha, componentSpec)
85+
})
86+
)
87+
deployments.push(...results)
88+
})()
89+
: Promise.resolve()
90+
91+
const sequentialChain = (async () => {
92+
for (const componentSpec of sequentialGroup) {
93+
const idx = ++componentIndex
94+
logger.info("")
95+
logger.info("Deploying graph component (%s of %s) \"%s\" for suite \"%s\"...", idx, sortedComponents.length, componentSpec.name, testSuiteId)
96+
logger.info("")
97+
const commitSha = await Deployer.deployComponent(config, workspace, componentSpec, namespace)
98+
deployments.push(new DeployedComponent(commitSha, componentSpec))
99+
}
100+
})()
101+
102+
await Promise.all([parallelChain, sequentialChain])
95103
}
96104
})()
97105

‎k8s-deployer/test/test-suite-handler.spec.ts‎

Lines changed: 211 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -220,10 +220,11 @@ describe("Deployment happy path", async () => {
220220
chai.expect(fsAccessStubs.getCall(2).calledWith("lock-manager/deployment/pit/is-deployment-ready.sh")).be.true
221221
chai.expect(fsAccessStubs.getCall(3).calledWith("23456_t1/comp-1")).be.false
222222
chai.expect(fsAccessStubs.getCall(3).calledWith("./comp-1")).be.true
223-
chai.expect(fsAccessStubs.getCall(4).calledWith("comp-1/deployment/pit/deploy.sh")).be.true
224-
chai.expect(fsAccessStubs.getCall(5).calledWith("comp-1/deployment/pit/is-deployment-ready.sh")).be.true
225-
chai.expect(fsAccessStubs.getCall(6).calledWith("./comp-1-test-app")).be.true
226-
chai.expect(fsAccessStubs.getCall(7).calledWith("comp-1-test-app/deployment/pit/deploy.sh")).be.true
223+
// testApp deploys concurrently with comp-1, so its directory check interleaves
224+
chai.expect(fsAccessStubs.getCall(4).calledWith("./comp-1-test-app")).be.true
225+
chai.expect(fsAccessStubs.getCall(5).calledWith("comp-1/deployment/pit/deploy.sh")).be.true
226+
chai.expect(fsAccessStubs.getCall(6).calledWith("comp-1-test-app/deployment/pit/deploy.sh")).be.true
227+
chai.expect(fsAccessStubs.getCall(7).calledWith("comp-1/deployment/pit/is-deployment-ready.sh")).be.true
227228
chai.expect(fsAccessStubs.getCall(8).calledWith("comp-1-test-app/deployment/pit/is-deployment-ready.sh")).be.true
228229

229230
// check shell calls
@@ -258,25 +259,26 @@ describe("Deployment happy path", async () => {
258259
{ homeDir: "lock-manager" })
259260
).be.true
260261

262+
// testApp deploys concurrently with comp-1, so git log calls interleave
261263
chai.expect(execStub.getCall(5).calledWith(`cd comp-1 && git log --pretty=format:"%h" -1`)).be.true
262264

263-
chai.expect(execStub.getCall(6).calledWith(
265+
chai.expect(execStub.getCall(6).calledWith(`cd comp-1-test-app && git log --pretty=format:"%h" -1`)).be.true
266+
267+
chai.expect(execStub.getCall(7).calledWith(
264268
"deployment/pit/deploy.sh nsChild",
265269
{ homeDir: "comp-1", logFileName: "12345_t1/logs/deploy-nsChild-comp-1.log", tailTarget: sinon.match.any })
266270
).be.true
267271

268-
chai.expect(execStub.getCall(7).calledWith(
269-
"deployment/pit/is-deployment-ready.sh nsChild",
270-
{ homeDir: "comp-1" }
271-
)).be.true
272-
273-
chai.expect(execStub.getCall(8).calledWith(`cd comp-1-test-app && git log --pretty=format:"%h" -1`)).be.true
274-
275-
chai.expect(execStub.getCall(9).calledWith(
272+
chai.expect(execStub.getCall(8).calledWith(
276273
"deployment/pit/deploy.sh nsChild t1",
277274
{ homeDir: "comp-1-test-app", logFileName: `12345_t1/logs/deploy-nsChild-comp-1-test-app.log`, tailTarget: sinon.match.any })
278275
).be.true
279276

277+
chai.expect(execStub.getCall(9).calledWith(
278+
"deployment/pit/is-deployment-ready.sh nsChild",
279+
{ homeDir: "comp-1" })
280+
).be.true
281+
280282
chai.expect(execStub.getCall(10).calledWith(
281283
"deployment/pit/is-deployment-ready.sh nsChild",
282284
{ homeDir: "comp-1-test-app" })
@@ -435,10 +437,202 @@ describe("Deployment happy path", async () => {
435437
chai.expect(fsAccessStubs.getCall(2).calledWith("lock-manager/deployment/pit/is-deployment-ready.sh")).be.true
436438
chai.expect(fsAccessStubs.getCall(3).calledWith("23456_t2/comp-1")).be.false
437439
chai.expect(fsAccessStubs.getCall(3).calledWith("./comp-1")).be.true
438-
chai.expect(fsAccessStubs.getCall(4).calledWith("./deployment/pit/deploy.sh")).be.true
439-
chai.expect(fsAccessStubs.getCall(5).calledWith("./deployment/pit/is-deployment-ready.sh")).be.true
440-
chai.expect(fsAccessStubs.getCall(6).calledWith("./comp-1-test-app")).be.true
441-
chai.expect(fsAccessStubs.getCall(7).calledWith("comp-1-test-app/deployment/pit/deploy.sh")).be.true
440+
// testApp deploys concurrently with comp-1, so its directory check interleaves
441+
chai.expect(fsAccessStubs.getCall(4).calledWith("./comp-1-test-app")).be.true
442+
chai.expect(fsAccessStubs.getCall(5).calledWith("./deployment/pit/deploy.sh")).be.true
443+
chai.expect(fsAccessStubs.getCall(6).calledWith("comp-1-test-app/deployment/pit/deploy.sh")).be.true
444+
chai.expect(fsAccessStubs.getCall(7).calledWith("./deployment/pit/is-deployment-ready.sh")).be.true
442445
chai.expect(fsAccessStubs.getCall(8).calledWith("comp-1-test-app/deployment/pit/is-deployment-ready.sh")).be.true
443446
})
444447
})
448+
449+
describe("deployGraph - deployment ordering and concurrency", async () => {
450+
const config: Config = {
451+
commitSha: "test-sha",
452+
workspace: "/tmp",
453+
clusterUrl: "http://localhost",
454+
parentNamespace: "ns",
455+
subNamespacePrefix: DEFAULT_SUB_NAMESPACE_PREFIX,
456+
subNamespaceGeneratorType: SUB_NAMESPACE_GENERATOR_TYPE_DATE,
457+
pitfile: "pitfile.yml",
458+
namespaceTimeoutSeconds: 2,
459+
report: {},
460+
targetEnv: "test",
461+
testStatusPollFrequencyMs: 100,
462+
testTimeoutMs: 1000,
463+
deployCheckFrequencyMs: 100,
464+
params: new Map(),
465+
useMockLockManager: false,
466+
servicesAreExposedViaProxy: false,
467+
lockManagerApiRetries: 1,
468+
enableCleanups: false,
469+
testRunnerAppPort: 80
470+
}
471+
const namespace = "test-ns"
472+
const testSuiteId = "suite-1"
473+
const workspace = "test-workspace"
474+
475+
type Gate = { resolve: () => void; promise: Promise<void> }
476+
const makeGate = (): Gate => {
477+
let resolve!: () => void
478+
const promise = new Promise<void>(r => { resolve = r })
479+
return { resolve, promise }
480+
}
481+
482+
const makeSpec = (id: string, opts: { parallel?: boolean; dependsOn?: string[] } = {}) => ({
483+
name: `${id}-name`, id,
484+
location: { type: LocationType.Local },
485+
deploy: { command: `${id}/deploy.sh`, statusCheck: { command: `${id}/ready.sh` } },
486+
undeploy: { command: `${id}/undeploy.sh` },
487+
...opts
488+
})
489+
490+
const loadWithStub = async (deployStub: sinon.SinonStub) =>
491+
esmock(
492+
"../src/test-suite-handler.js",
493+
{ "../src/deployer.js": { deployComponent: deployStub } },
494+
{ "../src/logger.js": { logger: { debug: () => {}, info: () => {}, warn: () => {}, error: () => {} } } }
495+
)
496+
497+
it("returns GraphDeploymentResult with all deployed components", async () => {
498+
const deployStub = sinon.stub().callsFake(async (_cfg, _ws, spec) => `sha-${spec.id}`)
499+
const SuiteHandler = await loadWithStub(deployStub)
500+
const graph = {
501+
testApp: makeSpec("test-app"),
502+
components: [makeSpec("comp-a"), makeSpec("comp-b")]
503+
}
504+
const result = await SuiteHandler.deployGraph(config, workspace, testSuiteId, graph, namespace)
505+
chai.expect(result.components).to.have.length(2)
506+
chai.expect(result.components.map((d: any) => d.component.id)).to.include.members(["comp-a", "comp-b"])
507+
chai.expect(result.testApp.component.id).to.equal("test-app")
508+
chai.expect(result.testApp.commitSha).to.equal("sha-test-app")
509+
})
510+
511+
it("deploys parallel-flagged components concurrently", async () => {
512+
const gates: Record<string, Gate> = { B: makeGate(), C: makeGate(), testApp: makeGate() }
513+
const started: string[] = []
514+
const completed: string[] = []
515+
const deployStub = sinon.stub().callsFake(async (_cfg, _ws, spec) => {
516+
started.push(spec.id)
517+
await gates[spec.id].promise
518+
completed.push(spec.id)
519+
return `sha-${spec.id}`
520+
})
521+
const SuiteHandler = await loadWithStub(deployStub)
522+
const graph = {
523+
testApp: makeSpec("testApp"),
524+
components: [makeSpec("B", { parallel: true }), makeSpec("C", { parallel: true })]
525+
}
526+
const deployPromise = SuiteHandler.deployGraph(config, workspace, testSuiteId, graph, namespace)
527+
// Both B and C should have started before either completes
528+
chai.expect(started).to.include("B")
529+
chai.expect(started).to.include("C")
530+
chai.expect(completed).to.not.include("B")
531+
chai.expect(completed).to.not.include("C")
532+
gates["B"].resolve()
533+
gates["C"].resolve()
534+
gates["testApp"].resolve()
535+
await deployPromise
536+
chai.expect(completed).to.include.members(["B", "C"])
537+
})
538+
539+
it("deploys mixed parallel/sequential at same level concurrently", async () => {
540+
const gates: Record<string, Gate> = { B: makeGate(), C: makeGate(), testApp: makeGate() }
541+
const started: string[] = []
542+
const completed: string[] = []
543+
const deployStub = sinon.stub().callsFake(async (_cfg, _ws, spec) => {
544+
started.push(spec.id)
545+
await gates[spec.id].promise
546+
completed.push(spec.id)
547+
return `sha-${spec.id}`
548+
})
549+
const SuiteHandler = await loadWithStub(deployStub)
550+
// B is parallel:true, C has no parallel flag — same dependency level, so neither waits on the other
551+
const graph = {
552+
testApp: makeSpec("testApp"),
553+
components: [makeSpec("B", { parallel: true }), makeSpec("C")]
554+
}
555+
const deployPromise = SuiteHandler.deployGraph(config, workspace, testSuiteId, graph, namespace)
556+
// B (parallelGroup) and C (sequentialGroup) fire their respective chains simultaneously —
557+
// both should be in-flight before either completes
558+
chai.expect(started).to.include("B")
559+
chai.expect(started).to.include("C")
560+
chai.expect(completed).to.not.include("B")
561+
chai.expect(completed).to.not.include("C")
562+
gates["B"].resolve()
563+
gates["C"].resolve()
564+
gates["testApp"].resolve()
565+
await deployPromise
566+
chai.expect(completed).to.include.members(["B", "C"])
567+
})
568+
569+
it("second dependency level does not start until first level completes", async () => {
570+
// Topology: A → B(parallel), A → C, B → D, C → D
571+
const gates: Record<string, Gate> = {
572+
A: makeGate(), B: makeGate(), C: makeGate(), D: makeGate(), testApp: makeGate()
573+
}
574+
const started: string[] = []
575+
const deployStub = sinon.stub().callsFake(async (_cfg, _ws, spec) => {
576+
started.push(spec.id)
577+
await gates[spec.id].promise
578+
return `sha-${spec.id}`
579+
})
580+
const SuiteHandler = await loadWithStub(deployStub)
581+
const graph = {
582+
testApp: makeSpec("testApp"),
583+
components: [
584+
makeSpec("A"),
585+
makeSpec("B", { parallel: true, dependsOn: ["A"] }),
586+
makeSpec("C", { dependsOn: ["A"] }),
587+
makeSpec("D", { dependsOn: ["B", "C"] })
588+
]
589+
}
590+
const deployPromise = SuiteHandler.deployGraph(config, workspace, testSuiteId, graph, namespace)
591+
592+
// Level 1: only A started; testApp is already in-flight concurrently
593+
chai.expect(started).to.include("A")
594+
chai.expect(started).to.include("testApp")
595+
chai.expect(started).to.not.include("B")
596+
chai.expect(started).to.not.include("C")
597+
chai.expect(started).to.not.include("D")
598+
599+
// Resolve A — level 2 (B and C) should start
600+
gates["A"].resolve()
601+
await new Promise(r => setTimeout(r, 0))
602+
chai.expect(started).to.include("B")
603+
chai.expect(started).to.include("C")
604+
chai.expect(started).to.not.include("D")
605+
606+
// Resolve B and C — level 3 (D) should start
607+
gates["B"].resolve()
608+
gates["C"].resolve()
609+
await new Promise(r => setTimeout(r, 0))
610+
chai.expect(started).to.include("D")
611+
612+
gates["D"].resolve()
613+
gates["testApp"].resolve()
614+
await deployPromise
615+
})
616+
617+
it("deploys test app concurrently with the component chain", async () => {
618+
const gates: Record<string, Gate> = { A: makeGate(), testApp: makeGate() }
619+
const started: string[] = []
620+
const deployStub = sinon.stub().callsFake(async (_cfg, _ws, spec) => {
621+
started.push(spec.id)
622+
await gates[spec.id].promise
623+
return `sha-${spec.id}`
624+
})
625+
const SuiteHandler = await loadWithStub(deployStub)
626+
const graph = {
627+
testApp: makeSpec("testApp"),
628+
components: [makeSpec("A")]
629+
}
630+
const deployPromise = SuiteHandler.deployGraph(config, workspace, testSuiteId, graph, namespace)
631+
// A and testApp should both be in-flight before either completes
632+
chai.expect(started).to.include("A")
633+
chai.expect(started).to.include("testApp")
634+
gates["A"].resolve()
635+
gates["testApp"].resolve()
636+
await deployPromise
637+
})
638+
})

0 commit comments

Comments
 (0)