From 70ccc4437313e8c37d73a9ddbf1579fb46d027a0 Mon Sep 17 00:00:00 2001 From: Chris Major Date: Mon, 10 Oct 2022 16:00:35 +0000 Subject: [PATCH 1/7] add GPU host requirment to `configuration.ts` --- src/spec-configuration/configuration.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/spec-configuration/configuration.ts b/src/spec-configuration/configuration.ts index 18ae74eea..6004894dc 100644 --- a/src/spec-configuration/configuration.ts +++ b/src/spec-configuration/configuration.ts @@ -21,10 +21,16 @@ export type UserEnvProbe = 'none' | 'loginInteractiveShell' | 'interactiveShell' export type DevContainerConfigCommand = 'initializeCommand' | 'onCreateCommand' | 'updateContentCommand' | 'postCreateCommand' | 'postStartCommand' | 'postAttachCommand'; +export interface HostGPURequirements { + cores?: number; + memory?: string; +} + export interface HostRequirements { cpus?: number; memory?: string; storage?: string; + gpu?: boolean | 'optional' | HostGPURequirements; } export interface DevContainerFeature { From d6fe1386c671ac08d53ecf092a7c6e7d314ef442 Mon Sep 17 00:00:00 2001 From: Chris Major Date: Mon, 10 Oct 2022 16:33:33 +0000 Subject: [PATCH 2/7] added Metadata functions to support merging GPU requirements --- src/spec-node/imageMetadata.ts | 34 ++++++++++++++++++++++++++++++++-- src/test/imageMetadata.test.ts | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/src/spec-node/imageMetadata.ts b/src/spec-node/imageMetadata.ts index ceaa12726..779393cb1 100644 --- a/src/spec-node/imageMetadata.ts +++ b/src/spec-node/imageMetadata.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { ContainerError } from '../spec-common/errors'; -import { DevContainerConfig, DevContainerConfigCommand, DevContainerFromDockerComposeConfig, DevContainerFromDockerfileConfig, DevContainerFromImageConfig, getDockerComposeFilePaths, getDockerfilePath, HostRequirements, isDockerFileConfig, PortAttributes, UserEnvProbe } from '../spec-configuration/configuration'; +import { DevContainerConfig, DevContainerConfigCommand, DevContainerFromDockerComposeConfig, DevContainerFromDockerfileConfig, DevContainerFromImageConfig, getDockerComposeFilePaths, getDockerfilePath, HostGPURequirements, HostRequirements, isDockerFileConfig, PortAttributes, UserEnvProbe } from '../spec-configuration/configuration'; import { Feature, FeaturesConfig, Mount } from '../spec-configuration/containerFeaturesConfiguration'; import { ContainerDetails, DockerCLIParameters, ImageDetails } from '../spec-shutdown/dockerUtils'; import { Log } from '../spec-utils/log'; @@ -163,13 +163,43 @@ function mergeHostRequirements(imageMetadata: ImageMetadataEntry[]) { const cpus = Math.max(...imageMetadata.map(m => m.hostRequirements?.cpus || 0)); const memory = Math.max(...imageMetadata.map(m => parseBytes(m.hostRequirements?.memory || '0'))); const storage = Math.max(...imageMetadata.map(m => parseBytes(m.hostRequirements?.storage || '0'))); - return cpus || memory || storage ? { + const gpu = imageMetadata.map(m => m.hostRequirements?.gpu).reduce(mergeGpuRequirements, undefined); + return cpus || memory || storage || gpu ? { cpus, memory: memory ? `${memory}` : undefined, storage: storage ? `${storage}` : undefined, + gpu: gpu, } : undefined; } +function mergeGpuRequirements(a: undefined | boolean | 'optional' | HostGPURequirements, b: undefined | boolean | 'optional' | HostGPURequirements): undefined | boolean | 'optional' | HostGPURequirements { + // simple cases if either are undefined/false we use the other one + if (a === undefined || a === false) { + return b; + } else if (b === undefined || b === false) { + return a; + } else if (a === 'optional' && b === 'optional ') { + return 'optional'; + } else { + const aObject = asHostGPURequirements(a); + const bObject = asHostGPURequirements(b); + const cores = Math.max(aObject.cores || 0, bObject.cores || 0); + const memory = Math.max(parseBytes(aObject.memory || '0'), parseBytes(bObject.memory || '0')); + return { + cores: cores ? cores : undefined, + memory: memory ? `${memory}` : undefined, + }; + } +} + +function asHostGPURequirements(a: undefined | boolean | 'optional' | HostGPURequirements): HostGPURequirements { + if (typeof a !== 'object') { + return {}; + } else { + return a as HostGPURequirements; + } +} + function parseBytes(str: string) { const m = /^(\d+)([tgmk]b)?$/.exec(str); if (m) { diff --git a/src/test/imageMetadata.test.ts b/src/test/imageMetadata.test.ts index 91d5c5e89..19ba869d7 100644 --- a/src/test/imageMetadata.test.ts +++ b/src/test/imageMetadata.test.ts @@ -6,6 +6,7 @@ import * as assert from 'assert'; import * as path from 'path'; import { URI } from 'vscode-uri'; +import { HostGPURequirements } from '../spec-configuration/configuration'; import { Feature, FeaturesConfig, FeatureSet } from '../spec-configuration/containerFeaturesConfiguration'; import { experimentalImageMetadataDefault } from '../spec-node/devContainers'; import { getDevcontainerMetadata, getDevcontainerMetadataLabel, getImageMetadata, getImageMetadataFromContainer, imageMetadataLabel, mergeConfiguration } from '../spec-node/imageMetadata'; @@ -245,6 +246,39 @@ describe('Image Metadata', function () { assert.strictEqual(merged.remoteEnv?.ENV4, 'feature1'); }); }); + + it('should merge gpu requirements from devcontainer.json and features', () => { + const merged = mergeConfiguration({ + configFilePath: URI.parse('file:///devcontainer.json'), + image: 'image', + hostRequirements: { + gpu: 'optional' + } + }, [ + { + hostRequirements: { + gpu: true + } + }, + { + hostRequirements: { + gpu: { + cores: 4 + } + } + }, + { + hostRequirements: { + gpu: { + memory: '8gb' + } + } + } + ]); + const gpuRequirement = merged.hostRequirements?.gpu as HostGPURequirements; + assert.strictEqual(gpuRequirement?.cores, 4); + assert.strictEqual(gpuRequirement?.memory, 8 * (2 ** 30)); + }); }); function getFeaturesConfig(features: Feature[]): FeaturesConfig { From 8c66be1dafc33f96de4b52e51a89c9bd6b76f51a Mon Sep 17 00:00:00 2001 From: Chris Major Date: Mon, 10 Oct 2022 16:37:02 +0000 Subject: [PATCH 3/7] use GPU's when requested in singleContainer flow --- src/spec-node/singleContainer.ts | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/spec-node/singleContainer.ts b/src/spec-node/singleContainer.ts index e4b97bb8d..7184ae66b 100644 --- a/src/spec-node/singleContainer.ts +++ b/src/spec-node/singleContainer.ts @@ -5,7 +5,7 @@ import { createContainerProperties, startEventSeen, ResolverResult, getTunnelInformation, getDockerfilePath, getDockerContextPath, DockerResolverParameters, isDockerFileConfig, uriToWSLFsPath, WorkspaceConfiguration, getFolderImageName, inspectDockerImage, logUMask, SubstitutedConfig } from './utils'; -import { ContainerProperties, setupInContainer, ResolverProgress } from '../spec-common/injectHeadless'; +import { ContainerProperties, setupInContainer, ResolverProgress, ResolverParameters } from '../spec-common/injectHeadless'; import { ContainerError, toErrorText } from '../spec-common/errors'; import { ContainerDetails, listContainers, DockerCLIParameters, inspectContainers, dockerCLI, dockerPtyCLI, toPtyExecParameters, ImageDetails, toExecParameters } from '../spec-shutdown/dockerUtils'; import { DevContainerConfig, DevContainerFromDockerfileConfig, DevContainerFromImageConfig } from '../spec-configuration/configuration'; @@ -305,6 +305,22 @@ export async function findDevContainer(params: DockerCLIParameters | DockerResol return details.filter(container => container.State.Status !== 'removing')[0]; } +export async function extraRunArgs(common: ResolverParameters, params: DockerCLIParameters | DockerResolverParameters, config: DevContainerFromDockerfileConfig | DevContainerFromImageConfig) { + const extraArguments: string[] = []; + if (config.hostRequirements?.gpu) { + const result = await dockerCLI(params, 'info', '-f', '{{.Runtimes.nvidia}}'); + const runtimeFound = result.stdout.includes('nvidia-container-runtime'); + if (runtimeFound) { + common.output.write(`GPU support found, add GPU flags to docker call.`); + extraArguments.push('--gpus', 'all'); + } else { + if (config.hostRequirements?.gpu !== 'optional') { + common.output.write('No GPU support found yet a GPU was required - consider marking it as "optional"', LogLevel.Warning); + } + } + } + return extraArguments; +} export async function spawnDevContainer(params: DockerResolverParameters, config: DevContainerFromDockerfileConfig | DevContainerFromImageConfig, mergedConfig: MergedDevContainerConfig, imageName: string, labels: string[], workspaceMount: string | undefined, imageDetails: (() => Promise) | undefined, containerUser: string | undefined) { const { common } = params; @@ -372,6 +388,7 @@ while sleep 1 & wait $!; do :; done`, '-']; // `wait $!` allows for the `trap` t ...containerEnv, ...containerUserArgs, ...(config.runArgs || []), + ...(await extraRunArgs(common, params, config) || []), ...featureArgs, ...entrypoint, imageName, From 215ba10d57feaea8db5af5b654f1daf66de5f660 Mon Sep 17 00:00:00 2001 From: Chris Major Date: Mon, 10 Oct 2022 16:48:19 +0000 Subject: [PATCH 4/7] add GPU functionality to dockerCompose --- src/spec-node/dockerCompose.ts | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/src/spec-node/dockerCompose.ts b/src/spec-node/dockerCompose.ts index 1fdbbcf14..5b644b76e 100644 --- a/src/spec-node/dockerCompose.ts +++ b/src/spec-node/dockerCompose.ts @@ -386,7 +386,7 @@ async function startContainer(params: DockerResolverParameters, buildParams: Doc // Save override docker-compose file to disk. // Persisted folder is a path that will be maintained between sessions // Note: As a fallback, persistedFolder is set to the build's tmpDir() directory - const overrideFilePath = await writeFeaturesComposeOverrideFile(updatedImageName, currentImageName, mergedConfig, config, versionPrefix, imageDetails, service, idLabels, params.additionalMounts, persistedFolder, featuresStartOverrideFilePrefix, buildCLIHost, output); + const overrideFilePath = await writeFeaturesComposeOverrideFile(updatedImageName, currentImageName, mergedConfig, config, versionPrefix, imageDetails, service, idLabels, params.additionalMounts, persistedFolder, featuresStartOverrideFilePrefix, buildCLIHost, params, output); if (overrideFilePath) { // Add file path to override file as parameter composeGlobalArgs.push('-f', overrideFilePath); @@ -449,9 +449,10 @@ async function writeFeaturesComposeOverrideFile( overrideFilePath: string, overrideFilePrefix: string, buildCLIHost: CLIHost, + params: DockerResolverParameters, output: Log, ) { - const composeOverrideContent = await generateFeaturesComposeOverrideContent(updatedImageName, originalImageName, mergedConfig, config, versionPrefix, imageDetails, service, additionalLabels, additionalMounts); + const composeOverrideContent = await generateFeaturesComposeOverrideContent(updatedImageName, originalImageName, mergedConfig, config, versionPrefix, imageDetails, service, additionalLabels, additionalMounts, params); const overrideFileHasContents = !!composeOverrideContent && composeOverrideContent.length > 0 && composeOverrideContent.trim() !== ''; if (overrideFileHasContents) { output.write(`Docker Compose override file for creating container:\n${composeOverrideContent}`); @@ -470,6 +471,12 @@ async function writeFeaturesComposeOverrideFile( } } +async function checkDockerSupportForGPU(params: DockerCLIParameters | DockerResolverParameters): Promise { + const result = await dockerCLI(params, 'info', '-f', '{{.Runtimes.nvidia}}'); + const runtimeFound = result.stdout.includes('nvidia-container-runtime'); + return runtimeFound; +} + async function generateFeaturesComposeOverrideContent( updatedImageName: string, originalImageName: string, @@ -480,6 +487,7 @@ async function generateFeaturesComposeOverrideContent( service: any, additionalLabels: string[], additionalMounts: Mount[], + params: DockerResolverParameters, ) { const overrideImage = updatedImageName !== originalImageName; @@ -501,6 +509,19 @@ async function generateFeaturesComposeOverrideContent( const userCommand = overrideCommand ? [] : composeCommand /* $ already escaped. */ || (composeEntrypoint ? [/* Ignore image CMD per docker-compose.yml spec. */] : ((await imageDetails()).Config.Cmd || []).map(c => c.replace(/\$/g, '$$$$'))); // $ > $$ to escape docker-compose.yml's interpolation. + const hasGpuRequirement = config.hostRequirements?.gpu; + const addGpuCapability = hasGpuRequirement && await checkDockerSupportForGPU(params); + if (hasGpuRequirement && hasGpuRequirement !== 'optional' && !addGpuCapability) { + throw Error('Unable to detect GPU yet a GPU was required - consider marking it as "optional"'); + } + const gpuResources = addGpuCapability ? '' : ` + deploy: + resources: + reservations: + devices: + - capabilities: [gpu] +`; + return `${versionPrefix}services: '${config.service}':${overrideImage ? ` image: ${updatedImageName}` : ''} @@ -524,7 +545,7 @@ while sleep 1 & wait $$!; do :; done", "-"${userEntrypoint.map(a => `, ${JSON.st volumes:${mounts.map(m => ` - ${m.source}:${m.target}`).join('')}` : ''}${volumeMounts.length ? ` volumes:${volumeMounts.map(m => ` - ${m.source}:${m.external ? '\n external: true' : ''}`).join('')}` : ''} + ${m.source}:${m.external ? '\n external: true' : ''}`).join('')}` : ''}${gpuResources} `; } From 1ba3534cf096d11f2f495cbf8d8ec002ea8f7ddd Mon Sep 17 00:00:00 2001 From: Chris Major Date: Tue, 11 Oct 2022 13:29:43 +0000 Subject: [PATCH 5/7] refactor check for GPU avalaiblity into utils.ts --- src/spec-node/dockerCompose.ts | 10 ++-------- src/spec-node/singleContainer.ts | 6 ++---- src/spec-node/utils.ts | 8 +++++++- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/spec-node/dockerCompose.ts b/src/spec-node/dockerCompose.ts index 5b644b76e..b341bdb8a 100644 --- a/src/spec-node/dockerCompose.ts +++ b/src/spec-node/dockerCompose.ts @@ -6,7 +6,7 @@ import * as yaml from 'js-yaml'; import * as shellQuote from 'shell-quote'; -import { createContainerProperties, startEventSeen, ResolverResult, getTunnelInformation, DockerResolverParameters, inspectDockerImage, getEmptyContextFolder, getFolderImageName, SubstitutedConfig } from './utils'; +import { createContainerProperties, startEventSeen, ResolverResult, getTunnelInformation, DockerResolverParameters, inspectDockerImage, getEmptyContextFolder, getFolderImageName, SubstitutedConfig, checkDockerSupportForGPU } from './utils'; import { ContainerProperties, setupInContainer, ResolverProgress } from '../spec-common/injectHeadless'; import { ContainerError } from '../spec-common/errors'; import { Workspace } from '../spec-utils/workspaces'; @@ -471,12 +471,6 @@ async function writeFeaturesComposeOverrideFile( } } -async function checkDockerSupportForGPU(params: DockerCLIParameters | DockerResolverParameters): Promise { - const result = await dockerCLI(params, 'info', '-f', '{{.Runtimes.nvidia}}'); - const runtimeFound = result.stdout.includes('nvidia-container-runtime'); - return runtimeFound; -} - async function generateFeaturesComposeOverrideContent( updatedImageName: string, originalImageName: string, @@ -512,7 +506,7 @@ async function generateFeaturesComposeOverrideContent( const hasGpuRequirement = config.hostRequirements?.gpu; const addGpuCapability = hasGpuRequirement && await checkDockerSupportForGPU(params); if (hasGpuRequirement && hasGpuRequirement !== 'optional' && !addGpuCapability) { - throw Error('Unable to detect GPU yet a GPU was required - consider marking it as "optional"'); + params.common.output.write('No GPU support found yet a GPU was required - consider marking it as "optional"', LogLevel.Warning); } const gpuResources = addGpuCapability ? '' : ` deploy: diff --git a/src/spec-node/singleContainer.ts b/src/spec-node/singleContainer.ts index 7184ae66b..d5653e7e3 100644 --- a/src/spec-node/singleContainer.ts +++ b/src/spec-node/singleContainer.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ -import { createContainerProperties, startEventSeen, ResolverResult, getTunnelInformation, getDockerfilePath, getDockerContextPath, DockerResolverParameters, isDockerFileConfig, uriToWSLFsPath, WorkspaceConfiguration, getFolderImageName, inspectDockerImage, logUMask, SubstitutedConfig } from './utils'; +import { createContainerProperties, startEventSeen, ResolverResult, getTunnelInformation, getDockerfilePath, getDockerContextPath, DockerResolverParameters, isDockerFileConfig, uriToWSLFsPath, WorkspaceConfiguration, getFolderImageName, inspectDockerImage, logUMask, SubstitutedConfig, checkDockerSupportForGPU } from './utils'; import { ContainerProperties, setupInContainer, ResolverProgress, ResolverParameters } from '../spec-common/injectHeadless'; import { ContainerError, toErrorText } from '../spec-common/errors'; import { ContainerDetails, listContainers, DockerCLIParameters, inspectContainers, dockerCLI, dockerPtyCLI, toPtyExecParameters, ImageDetails, toExecParameters } from '../spec-shutdown/dockerUtils'; @@ -308,9 +308,7 @@ export async function findDevContainer(params: DockerCLIParameters | DockerResol export async function extraRunArgs(common: ResolverParameters, params: DockerCLIParameters | DockerResolverParameters, config: DevContainerFromDockerfileConfig | DevContainerFromImageConfig) { const extraArguments: string[] = []; if (config.hostRequirements?.gpu) { - const result = await dockerCLI(params, 'info', '-f', '{{.Runtimes.nvidia}}'); - const runtimeFound = result.stdout.includes('nvidia-container-runtime'); - if (runtimeFound) { + if (await checkDockerSupportForGPU(params)) { common.output.write(`GPU support found, add GPU flags to docker call.`); extraArguments.push('--gpus', 'all'); } else { diff --git a/src/spec-node/utils.ts b/src/spec-node/utils.ts index fa0b1da62..0c1e26809 100644 --- a/src/spec-node/utils.ts +++ b/src/spec-node/utils.ts @@ -15,7 +15,7 @@ import { ContainerProperties, getContainerProperties, ResolverParameters } from import { Workspace } from '../spec-utils/workspaces'; import { URI } from 'vscode-uri'; import { ShellServer } from '../spec-common/shellServer'; -import { inspectContainer, inspectImage, getEvents, ContainerDetails, DockerCLIParameters, dockerExecFunction, dockerPtyCLI, dockerPtyExecFunction, toDockerImageName, DockerComposeCLI, ImageDetails } from '../spec-shutdown/dockerUtils'; +import { inspectContainer, inspectImage, getEvents, ContainerDetails, DockerCLIParameters, dockerExecFunction, dockerPtyCLI, dockerPtyExecFunction, toDockerImageName, DockerComposeCLI, ImageDetails, dockerCLI } from '../spec-shutdown/dockerUtils'; import { getRemoteWorkspaceFolder } from './dockerCompose'; import { findGitRootFolder } from '../spec-common/git'; import { parentURI, uriToFsPath } from '../spec-configuration/configurationCommonUtils'; @@ -169,6 +169,12 @@ async function hasLabels(params: DockerResolverParameters, info: any, expectedLa .every(name => actualLabels[name] === expectedLabels[name]); } +export async function checkDockerSupportForGPU(params: DockerCLIParameters | DockerResolverParameters): Promise { + const result = await dockerCLI(params, 'info', '-f', '{{.Runtimes.nvidia}}'); + const runtimeFound = result.stdout.includes('nvidia-container-runtime'); + return runtimeFound; +} + export async function inspectDockerImage(params: DockerResolverParameters | DockerCLIParameters, imageName: string, pullImageOnError: boolean) { try { return await inspectImage(params, imageName); From 67727417a8c7646419f90b714a2a07ec55d98789 Mon Sep 17 00:00:00 2001 From: Chris Major Date: Tue, 11 Oct 2022 13:46:37 +0000 Subject: [PATCH 6/7] Fixing test --- src/test/imageMetadata.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/imageMetadata.test.ts b/src/test/imageMetadata.test.ts index 19ba869d7..1a80d91e8 100644 --- a/src/test/imageMetadata.test.ts +++ b/src/test/imageMetadata.test.ts @@ -277,7 +277,7 @@ describe('Image Metadata', function () { ]); const gpuRequirement = merged.hostRequirements?.gpu as HostGPURequirements; assert.strictEqual(gpuRequirement?.cores, 4); - assert.strictEqual(gpuRequirement?.memory, 8 * (2 ** 30)); + assert.strictEqual(gpuRequirement?.memory, '8589934592'); }); }); From 6eb701db513f7778e32948e737cf2d3c7dc112a4 Mon Sep 17 00:00:00 2001 From: Christof Marti Date: Mon, 7 Nov 2022 15:10:50 +0100 Subject: [PATCH 7/7] Fixes --- src/spec-node/dockerCompose.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/spec-node/dockerCompose.ts b/src/spec-node/dockerCompose.ts index c6c4b7f6c..a08c1a791 100644 --- a/src/spec-node/dockerCompose.ts +++ b/src/spec-node/dockerCompose.ts @@ -508,13 +508,12 @@ async function generateFeaturesComposeOverrideContent( if (hasGpuRequirement && hasGpuRequirement !== 'optional' && !addGpuCapability) { params.common.output.write('No GPU support found yet a GPU was required - consider marking it as "optional"', LogLevel.Warning); } - const gpuResources = addGpuCapability ? '' : ` + const gpuResources = addGpuCapability ? ` deploy: resources: reservations: devices: - - capabilities: [gpu] -`; + - capabilities: [gpu]` : ''; return `${versionPrefix}services: '${config.service}':${overrideImage ? ` @@ -537,9 +536,9 @@ while sleep 1 & wait $$!; do :; done", "-"${userEntrypoint.map(a => `, ${JSON.st labels:${additionalLabels.map(label => ` - ${label.replace(/\$/g, '$$$$')}`).join('')}` : ''}${mounts.length ? ` volumes:${mounts.map(m => ` - - ${m.source}:${m.target}`).join('')}` : ''}${volumeMounts.length ? ` + - ${m.source}:${m.target}`).join('')}` : ''}${gpuResources}${volumeMounts.length ? ` volumes:${volumeMounts.map(m => ` - ${m.source}:${m.external ? '\n external: true' : ''}`).join('')}` : ''}${gpuResources} + ${m.source}:${m.external ? '\n external: true' : ''}`).join('')}` : ''} `; }