diff --git a/src/server/services/__tests__/agentSession.test.ts b/src/server/services/__tests__/agentSession.test.ts index 2f20ca05..b74e64ee 100644 --- a/src/server/services/__tests__/agentSession.test.ts +++ b/src/server/services/__tests__/agentSession.test.ts @@ -225,6 +225,39 @@ const mockRedis = { del: jest.fn().mockResolvedValue(1), }; +function createDeferred() { + let resolve!: (value: T) => void; + let reject!: (error?: unknown) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + + return { + promise, + resolve, + reject, + }; +} + +function buildDevModeSnapshot(deploymentName = 'service') { + return { + deployment: { + deploymentName, + containerName: deploymentName, + replicas: null, + image: 'node:20', + command: null, + workingDir: null, + env: null, + volumeMounts: null, + volumes: null, + nodeSelector: null, + }, + service: null, + }; +} + const mockedBuildServiceModule = jest.requireMock('server/services/build').__mocked as { deleteQueueAdd: jest.Mock; deleteBuild: jest.Mock; @@ -237,21 +270,7 @@ jest.spyOn(RedisClient, 'getInstance').mockReturnValue({ close: jest.fn(), } as any); -const mockEnableDevMode = jest.fn().mockResolvedValue({ - deployment: { - deploymentName: 'service', - containerName: 'service', - replicas: null, - image: 'node:20', - command: null, - workingDir: null, - env: null, - volumeMounts: null, - volumes: null, - nodeSelector: null, - }, - service: null, -}); +const mockEnableDevMode = jest.fn().mockResolvedValue(buildDevModeSnapshot()); const mockDisableDevMode = jest.fn().mockResolvedValue(undefined); (DevModeManager as jest.Mock).mockImplementation(() => ({ enableDevMode: mockEnableDevMode, @@ -369,21 +388,7 @@ describe('AgentSessionService', () => { mockRedis.setex.mockResolvedValue('OK'); mockRedis.get.mockResolvedValue(null); mockRedis.del.mockResolvedValue(1); - mockEnableDevMode.mockResolvedValue({ - deployment: { - deploymentName: 'service', - containerName: 'service', - replicas: null, - image: 'node:20', - command: null, - workingDir: null, - env: null, - volumeMounts: null, - volumes: null, - nodeSelector: null, - }, - service: null, - }); + mockEnableDevMode.mockResolvedValue(buildDevModeSnapshot()); mockDisableDevMode.mockResolvedValue(undefined); (isGvisorAvailable as jest.Mock).mockResolvedValue(false); (createAgentPvc as jest.Mock).mockResolvedValue({}); @@ -821,6 +826,41 @@ describe('AgentSessionService', () => { ); }); + it('starts dev mode for multiple services in parallel during session creation', async () => { + const webEnable = createDeferred>(); + const apiEnable = createDeferred>(); + mockEnableDevMode.mockImplementationOnce(() => webEnable.promise).mockImplementationOnce(() => apiEnable.promise); + + const optsWithServices: CreateSessionOptions = { + ...baseOpts, + services: [ + { + name: 'web', + deployId: 1, + resourceName: 'web-build-uuid', + devConfig: { image: 'node:20', command: 'pnpm dev' }, + }, + { + name: 'api', + deployId: 2, + resourceName: 'api-build-uuid', + devConfig: { image: 'node:20', command: 'pnpm start' }, + }, + ], + }; + + const createPromise = AgentSessionService.createSession(optsWithServices); + await new Promise((resolve) => setImmediate(resolve)); + + expect(mockEnableDevMode).toHaveBeenCalledTimes(2); + expect(mockSessionQuery.patch).not.toHaveBeenCalled(); + + webEnable.resolve(buildDevModeSnapshot('web-build-uuid')); + apiEnable.resolve(buildDevModeSnapshot('api-build-uuid')); + + await expect(createPromise).resolves.toEqual(expect.objectContaining({ status: 'active' })); + }); + it('does not pin services to the session node when same-node placement is disabled', async () => { const optsWithServices: CreateSessionOptions = { ...baseOpts, @@ -850,6 +890,65 @@ describe('AgentSessionService', () => { ); }); + it('restores successful sibling services when one parallel dev-mode enable fails', async () => { + const optsWithServices: CreateSessionOptions = { + ...baseOpts, + services: [ + { + name: 'web', + deployId: 1, + resourceName: 'web-build-uuid', + devConfig: { image: 'node:20', command: 'pnpm dev' }, + }, + { + name: 'api', + deployId: 2, + resourceName: 'api-build-uuid', + devConfig: { image: 'node:20', command: 'pnpm start' }, + }, + ], + }; + + mockEnableDevMode + .mockResolvedValueOnce(buildDevModeSnapshot('web-build-uuid')) + .mockRejectedValueOnce(new Error('api dev mode failed')); + + const deployManagerDeploy = jest.fn().mockResolvedValue(undefined); + (DeploymentManager as jest.Mock).mockImplementation(() => ({ + deploy: deployManagerDeploy, + })); + + const revertDeploys = [ + { + id: 1, + uuid: 'deploy-1', + build: { namespace: 'test-ns' }, + deployable: { name: 'web', type: 'github', deploymentDependsOn: [] }, + }, + ]; + mockDeployQuery.withGraphFetched.mockResolvedValue(revertDeploys); + + await expect(AgentSessionService.createSession(optsWithServices)).rejects.toThrow('api dev mode failed'); + + expect(DeploymentManager).toHaveBeenCalledWith(revertDeploys); + expect(deployManagerDeploy).toHaveBeenCalled(); + expect(mockDisableDevMode).toHaveBeenCalledTimes(2); + expect(mockDisableDevMode).toHaveBeenNthCalledWith( + 1, + 'test-ns', + 'deploy-1', + 'deploy-1', + buildDevModeSnapshot('web-build-uuid') + ); + expect(mockDisableDevMode).toHaveBeenNthCalledWith( + 2, + 'test-ns', + 'deploy-1', + 'deploy-1', + buildDevModeSnapshot('web-build-uuid') + ); + }); + it('renders dev env templates with the shared build env renderer before enabling dev mode', async () => { const buildContext = { uuid: 'sample-build-123', @@ -1274,6 +1373,188 @@ describe('AgentSessionService', () => { ); }); + it('starts dev mode for multiple attached services in parallel', async () => { + const webEnable = createDeferred>(); + const apiEnable = createDeferred>(); + mockEnableDevMode.mockImplementationOnce(() => webEnable.promise).mockImplementationOnce(() => apiEnable.promise); + + mockSessionQuery.findOne.mockResolvedValue({ + id: 321, + uuid: 'sess-1', + status: 'active', + buildUuid: 'build-123', + buildKind: 'environment', + namespace: 'test-ns', + podName: 'agent-aaaaaaaa', + pvcName: 'agent-pvc-aaaaaaaa', + workspaceRepos: [ + { + repo: 'example-org/example-repo', + repoUrl: 'https://github.com/example-org/example-repo.git', + branch: 'feature/current', + mountPath: '/workspace', + primary: true, + }, + ], + selectedServices: [], + devModeSnapshots: {}, + }); + (loadAgentSessionServiceCandidates as jest.Mock).mockResolvedValue([ + { + name: 'web', + type: 'github', + deployId: 11, + devConfig: { + image: 'node:20', + command: 'pnpm dev', + installCommand: 'cd /workspace/apps/web && pnpm install', + workDir: '/workspace/apps/web', + }, + repo: 'example-org/example-repo', + branch: 'feature/current', + revision: '0123456789abcdef0123456789abcdef01234567', + baseDeploy: { + id: 11, + uuid: 'web-build-uuid', + }, + }, + { + name: 'api', + type: 'github', + deployId: 22, + devConfig: { + image: 'node:20', + command: 'pnpm start', + installCommand: 'cd /workspace/apps/api && pnpm install', + workDir: '/workspace/apps/api', + }, + repo: 'example-org/example-repo', + branch: 'feature/current', + revision: 'fedcba98765432100123456789abcdef01234567', + baseDeploy: { + id: 22, + uuid: 'api-build-uuid', + }, + }, + ]); + + const attachPromise = AgentSessionService.attachServices('sess-1', ['web', 'api']); + await new Promise((resolve) => setImmediate(resolve)); + + expect(mockEnableDevMode).toHaveBeenCalledTimes(2); + expect(mockSessionQuery.patch).not.toHaveBeenCalled(); + + webEnable.resolve(buildDevModeSnapshot('web-build-uuid')); + apiEnable.resolve(buildDevModeSnapshot('api-build-uuid')); + + await expect(attachPromise).resolves.toBeUndefined(); + }); + + it('restores successful attached services when one parallel dev-mode enable fails', async () => { + mockSessionQuery.findOne.mockResolvedValue({ + id: 321, + uuid: 'sess-1', + status: 'active', + buildUuid: 'build-123', + buildKind: 'environment', + namespace: 'test-ns', + podName: 'agent-aaaaaaaa', + pvcName: 'agent-pvc-aaaaaaaa', + workspaceRepos: [ + { + repo: 'example-org/example-repo', + repoUrl: 'https://github.com/example-org/example-repo.git', + branch: 'feature/current', + mountPath: '/workspace', + primary: true, + }, + ], + selectedServices: [], + devModeSnapshots: {}, + }); + (loadAgentSessionServiceCandidates as jest.Mock).mockResolvedValue([ + { + name: 'web', + type: 'github', + deployId: 11, + devConfig: { + image: 'node:20', + command: 'pnpm dev', + installCommand: 'cd /workspace/apps/web && pnpm install', + workDir: '/workspace/apps/web', + }, + repo: 'example-org/example-repo', + branch: 'feature/current', + revision: '0123456789abcdef0123456789abcdef01234567', + baseDeploy: { + id: 11, + uuid: 'web-build-uuid', + }, + }, + { + name: 'api', + type: 'github', + deployId: 22, + devConfig: { + image: 'node:20', + command: 'pnpm start', + installCommand: 'cd /workspace/apps/api && pnpm install', + workDir: '/workspace/apps/api', + }, + repo: 'example-org/example-repo', + branch: 'feature/current', + revision: 'fedcba98765432100123456789abcdef01234567', + baseDeploy: { + id: 22, + uuid: 'api-build-uuid', + }, + }, + ]); + mockEnableDevMode + .mockResolvedValueOnce(buildDevModeSnapshot('web-build-uuid')) + .mockRejectedValueOnce(new Error('api dev mode failed')); + + const deployManagerDeploy = jest.fn().mockResolvedValue(undefined); + (DeploymentManager as jest.Mock).mockImplementation(() => ({ + deploy: deployManagerDeploy, + })); + + const revertDeploys = [ + { + id: 11, + uuid: 'deploy-11', + build: { namespace: 'test-ns' }, + deployable: { name: 'web', type: 'github', deploymentDependsOn: [] }, + }, + ]; + mockDeployQuery.withGraphFetched.mockResolvedValue(revertDeploys); + + await expect(AgentSessionService.attachServices('sess-1', ['web', 'api'])).rejects.toThrow('api dev mode failed'); + + expect(mockSessionQuery.patch).not.toHaveBeenCalled(); + expect(mockDeployQuery.patch).not.toHaveBeenCalled(); + expect(DeploymentManager).toHaveBeenCalledWith(revertDeploys); + expect(deployManagerDeploy).toHaveBeenCalled(); + expect(mockDisableDevMode).toHaveBeenCalledTimes(2); + expect(mockDisableDevMode).toHaveBeenNthCalledWith( + 1, + 'test-ns', + 'deploy-11', + 'deploy-11', + buildDevModeSnapshot('web-build-uuid') + ); + expect(mockDisableDevMode).toHaveBeenNthCalledWith( + 2, + 'test-ns', + 'deploy-11', + 'deploy-11', + buildDevModeSnapshot('web-build-uuid') + ); + expect(mockDisableDevMode.mock.invocationCallOrder[0]).toBeLessThan( + deployManagerDeploy.mock.invocationCallOrder[0] + ); + }); + it('honors the session stored same-node policy when attaching services', async () => { const globalConfigService = jest.requireMock('server/services/globalConfig').default; globalConfigService.getInstance.mockReturnValueOnce({ diff --git a/src/server/services/agentSession.ts b/src/server/services/agentSession.ts index d979b22a..725fdee6 100644 --- a/src/server/services/agentSession.ts +++ b/src/server/services/agentSession.ts @@ -187,6 +187,12 @@ async function attachStartupFailures; type SessionService = NonNullable[number]; type RequestedSessionService = string | RequestedAgentSessionServiceRef; +type DevModeEnabledService = { + deployId: number; + deploymentName: string; + serviceName: string; + snapshot: DevModeResourceSnapshot; +}; function getSessionSnapshot( snapshots: SessionSnapshotMap | null | undefined, @@ -215,6 +221,77 @@ function mergeSelectedServices( return mergedServices; } +class DevModeBatchEnableError extends Error { + successfulServices: DevModeEnabledService[]; + failures: Array<{ deployId: number; error: unknown }>; + + constructor(successfulServices: DevModeEnabledService[], failures: Array<{ deployId: number; error: unknown }>) { + const primaryError = failures[0]?.error; + super(primaryError instanceof Error ? primaryError.message : String(primaryError ?? 'Failed to enable dev mode')); + this.name = 'DevModeBatchEnableError'; + this.successfulServices = successfulServices; + this.failures = failures; + } +} + +function buildSnapshotMapFromEnabledServices(enabledServices: DevModeEnabledService[]): SessionSnapshotMap { + return Object.fromEntries(enabledServices.map((service) => [String(service.deployId), service.snapshot])); +} + +async function enableServicesInDevModeParallel(opts: { + namespace: string; + pvcName: string; + services: Array>; + requiredNodeName?: string; +}): Promise { + if (opts.services.length === 0) { + return []; + } + + const results = await Promise.allSettled( + opts.services.map(async (service): Promise => { + const deploymentName = service.resourceName || service.name; + const snapshot = await new DevModeManager().enableDevMode({ + namespace: opts.namespace, + deploymentName, + serviceName: deploymentName, + pvcName: opts.pvcName, + devConfig: service.devConfig, + requiredNodeName: opts.requiredNodeName, + }); + + return { + deployId: service.deployId, + deploymentName, + serviceName: deploymentName, + snapshot, + }; + }) + ); + + const successfulServices: DevModeEnabledService[] = []; + const failures: Array<{ deployId: number; error: unknown }> = []; + + results.forEach((result, index) => { + if (result.status === 'fulfilled') { + successfulServices.push(result.value); + return; + } + + const deployId = opts.services[index]?.deployId; + failures.push({ + deployId: typeof deployId === 'number' ? deployId : -1, + error: result.reason, + }); + }); + + if (failures.length > 0) { + throw new DevModeBatchEnableError(successfulServices, failures); + } + + return successfulServices; +} + async function resolveAgentPodNodeName(namespace: string, podName: string): Promise { const kc = new k8s.KubeConfig(); kc.loadFromDefault(); @@ -719,8 +796,9 @@ export default class AgentSessionService { const podName = buildAgentSessionPodName(sessionUuid, opts.buildUuid); const apiKeySecretName = `agent-secret-${sessionUuid.slice(0, 8)}`; const requestedModelId = opts.model?.trim() || undefined; - const mutatedDeploys: number[] = []; const devModeSnapshots: SessionSnapshotMap = {}; + const enabledDevModeDeployIds: number[] = []; + const persistedDevModeDeployIds: number[] = []; let failureStage: AgentSessionStartupFailureStage = 'create_session'; let sessionPersisted = false; let session: AgentSession | null = null; @@ -882,28 +960,37 @@ export default class AgentSessionService { throw new Error(`Session workspace pod ${podName} did not report a scheduled node`); } - const devModeManager = new DevModeManager(); - for (const svc of resolvedServices || []) { - const resourceName = svc.resourceName || svc.name; - const snapshot = await devModeManager.enableDevMode({ - namespace: opts.namespace, - deploymentName: resourceName, - serviceName: resourceName, - pvcName, - devConfig: svc.devConfig, - requiredNodeName: keepAttachedServicesOnSessionNode ? agentNodeName || undefined : undefined, - }); - mutatedDeploys.push(svc.deployId); - devModeSnapshots[String(svc.deployId)] = snapshot; + const enabledServices = await enableServicesInDevModeParallel({ + namespace: opts.namespace, + pvcName, + services: resolvedServices || [], + requiredNodeName: keepAttachedServicesOnSessionNode ? agentNodeName || undefined : undefined, + }).catch((error) => { + if (error instanceof DevModeBatchEnableError) { + enabledDevModeDeployIds.push(...error.successfulServices.map((service) => service.deployId)); + Object.assign(devModeSnapshots, buildSnapshotMapFromEnabledServices(error.successfulServices)); + } + + throw error; + }); + + enabledDevModeDeployIds.push(...enabledServices.map((service) => service.deployId)); + Object.assign(devModeSnapshots, buildSnapshotMapFromEnabledServices(enabledServices)); + + if (enabledServices.length > 0) { await AgentSession.query() .findById(session.id) .patch({ devModeSnapshots, } as unknown as Partial); - await Deploy.query().findById(svc.deployId).patch({ + } + + for (const service of enabledServices) { + await Deploy.query().findById(service.deployId).patch({ devMode: true, devModeSessionId: session.id, }); + persistedDevModeDeployIds.push(service.deployId); } await createSessionWorkspaceService(opts.namespace, podName, opts.buildUuid); @@ -1007,13 +1094,13 @@ export default class AgentSessionService { } const revertPromise = - mutatedDeploys.length > 0 + enabledDevModeDeployIds.length > 0 ? (async () => { const deploysToRevert = await Deploy.query() - .whereIn('id', mutatedDeploys) + .whereIn('id', enabledDevModeDeployIds) .withGraphFetched(DEV_MODE_REDEPLOY_GRAPH) .catch(() => [] as Deploy[]); - for (const deployId of mutatedDeploys) { + for (const deployId of persistedDevModeDeployIds) { await Deploy.query() .findById(deployId) .patch({ devMode: false, devModeSessionId: null }) @@ -1254,29 +1341,34 @@ export default class AgentSessionService { throw new Error(`Session workspace pod ${session.podName} did not report a scheduled node`); } - const devModeManager = new DevModeManager(); - const mutatedDeploys: number[] = []; + const enabledDevModeDeployIds: number[] = []; + const persistedDevModeDeployIds: number[] = []; const addedSnapshots: SessionSnapshotMap = {}; try { - for (const service of resolvedServices || []) { - const resourceName = service.resourceName || service.name; - const snapshot = await devModeManager.enableDevMode({ - namespace: session.namespace, - deploymentName: resourceName, - serviceName: resourceName, - pvcName: session.pvcName, - devConfig: service.devConfig, - requiredNodeName: keepAttachedServicesOnSessionNode ? agentNodeName : undefined, - }); + const enabledServices = await enableServicesInDevModeParallel({ + namespace: session.namespace, + pvcName: session.pvcName, + services: resolvedServices || [], + requiredNodeName: keepAttachedServicesOnSessionNode ? agentNodeName || undefined : undefined, + }).catch((error) => { + if (error instanceof DevModeBatchEnableError) { + enabledDevModeDeployIds.push(...error.successfulServices.map((service) => service.deployId)); + Object.assign(addedSnapshots, buildSnapshotMapFromEnabledServices(error.successfulServices)); + } + + throw error; + }); - mutatedDeploys.push(service.deployId); - addedSnapshots[String(service.deployId)] = snapshot; + enabledDevModeDeployIds.push(...enabledServices.map((service) => service.deployId)); + Object.assign(addedSnapshots, buildSnapshotMapFromEnabledServices(enabledServices)); + for (const service of enabledServices) { await Deploy.query().findById(service.deployId).patch({ devMode: true, devModeSessionId: session.id, }); + persistedDevModeDeployIds.push(service.deployId); } await AgentSession.query() @@ -1303,13 +1395,13 @@ export default class AgentSessionService { }` ); } catch (error) { - if (mutatedDeploys.length > 0) { + if (enabledDevModeDeployIds.length > 0) { const deploysToRevert = await Deploy.query() - .whereIn('id', mutatedDeploys) + .whereIn('id', enabledDevModeDeployIds) .withGraphFetched(DEV_MODE_REDEPLOY_GRAPH) .catch(() => [] as Deploy[]); - for (const deployId of mutatedDeploys) { + for (const deployId of persistedDevModeDeployIds) { await Deploy.query() .findById(deployId) .patch({ devMode: false, devModeSessionId: null })