Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Features contribute lifecycle hooks #390

Merged
merged 47 commits into from
Mar 2, 2023
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
efa0f62
refactor Feature interface
joshspicer Jan 18, 2023
279da40
Feat: Add in inline command lifecycle hook contribution from Features
joshspicer Jan 19, 2023
f43350e
markerfile to exercise order of execution
joshspicer Jan 23, 2023
652c249
Merge branch 'main' of https://github.com/devcontainers/cli into josh…
joshspicer Jan 23, 2023
b3c6d87
add in two helper scripts with the same name in two different Features
joshspicer Jan 24, 2023
d0d99fc
test with ${featureRoot}
joshspicer Jan 24, 2023
9aeade3
make Feature artifacts executable by all users
joshspicer Jan 24, 2023
b88f1cd
test for advanced lifehook scenario
joshspicer Jan 24, 2023
909e0fe
add
joshspicer Feb 1, 2023
93ed16b
typecheck
joshspicer Feb 1, 2023
780c346
improve test
joshspicer Feb 1, 2023
d0323e3
refactor variable names from 'postCreate' -> 'lifecycleHook'
joshspicer Feb 1, 2023
cd591d3
refactor variable names from 'postCreate' -> 'lifecycleHook'
joshspicer Feb 1, 2023
b5ec2f0
Merge branch 'joshspicer/features-lifecycles-hooks' of https://github…
joshspicer Feb 1, 2023
1502b47
update test for running in parallel
joshspicer Feb 1, 2023
af35b5d
Merge branch 'main' of https://github.com/devcontainers/cli into josh…
joshspicer Feb 2, 2023
bbadb67
update test
joshspicer Feb 3, 2023
0d3941d
Display the 'origin' of the lifecycle command in the output log durin…
joshspicer Feb 4, 2023
f320096
Change copy path for Features on-container from /tmp-build-features t…
joshspicer Feb 4, 2023
9596961
run lifecycle commands in Feature install order
joshspicer Feb 6, 2023
311a397
stop passing config object where unused
joshspicer Feb 6, 2023
0350081
Rename LifecycleCommandOriginMap -> LifecycleHooksInstallMap
joshspicer Feb 6, 2023
26406cf
add test to assert installsAfter is respected
joshspicer Feb 6, 2023
cafa379
rm -rf test dir
joshspicer Feb 6, 2023
cf819cf
assert contents in metadata label
joshspicer Feb 6, 2023
7b32586
unused test stub
joshspicer Feb 6, 2023
0f1bfd5
exercise dev container test cmd
joshspicer Feb 6, 2023
3e5f804
Merge branch 'main' of https://github.com/devcontainers/cli into josh…
joshspicer Feb 6, 2023
5e7ebcb
increase test timeouts
joshspicer Feb 6, 2023
c000b10
'' -> ''
joshspicer Feb 7, 2023
6687ad4
revert changing dest location to /opt/build-features and remerge main
joshspicer Feb 9, 2023
8ede783
pass contentSourceRootPath as a variable instead of string subst
joshspicer Feb 9, 2023
91e160b
remove dead code referencing "hasAcquires"
joshspicer Feb 9, 2023
e8b4b6f
cannot change line 256
joshspicer Feb 10, 2023
bff51a9
persist final Features copy to /usr/share/devcontainer/features
joshspicer Feb 10, 2023
07ad411
update existing test
joshspicer Feb 10, 2023
99836c3
feature dir need not be writable by anyone but its owner
joshspicer Feb 10, 2023
2e2731f
add test to assert resume will trigger lifecycle hooks
joshspicer Feb 10, 2023
4c37690
Test: "${featureRootFolder} substituted lifecycle hooks trigger on re…
joshspicer Feb 10, 2023
2adef08
missing semicolon
joshspicer Feb 10, 2023
59c1f4a
preserve featureRootFolder in metadata and replace right before usage
joshspicer Feb 13, 2023
cd87a51
rename metadata entry to featureRootFolder for clarity and only emit …
joshspicer Feb 13, 2023
d4f974b
merged config has substituted featureRootFolder values
joshspicer Feb 14, 2023
dd7667b
Merge branch 'main' into joshspicer/features-lifecycles-hooks
joshspicer Feb 27, 2023
b4e40a5
update tests to remove ${featureRootFolder}
joshspicer Feb 28, 2023
73c3a4f
remove ${featureRootFolder} variable substiution
joshspicer Feb 28, 2023
14a1c33
move Features container build folder back to /tmp
joshspicer Mar 1, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,5 @@ build-tmp
.DS_Store
.env
output
*.testMarker
src/test/container-features/configs/temp_lifecycle-hooks-alternative-order
121 changes: 70 additions & 51 deletions src/spec-common/injectHeadless.ts

Large diffs are not rendered by default.

18 changes: 18 additions & 0 deletions src/spec-common/variableSubstitution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,16 @@ function replaceContainerEnv(isWindows: boolean, configFile: URI | undefined, co
}
}

function replaceFeatureRoot(featureRoot: string | undefined, match: string, variable: string) {
switch (variable) {
case 'featureRoot':
return featureRoot || match;

default:
return match;
}
}

function replaceDevContainerId(getDevContainerId: () => string | undefined, match: string, variable: string) {
switch (variable) {
case 'devcontainerId':
Expand Down Expand Up @@ -169,3 +179,11 @@ function devcontainerIdForLabels(idLabels: Record<string, string>): string {
.padStart(52, '0');
return uniqueId;
}

export function substituteFeatureRoot(lifecycleHook: string | string[] | undefined, cachePath: string | undefined): string | string[] | undefined {
if (!lifecycleHook || !cachePath) {
return lifecycleHook;
}

return substitute0(replaceFeatureRoot.bind(undefined, cachePath), lifecycleHook);
}
55 changes: 38 additions & 17 deletions src/spec-configuration/containerFeaturesConfiguration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,34 +23,55 @@ export const V1_DEVCONTAINER_FEATURES_FILE_NAME = 'devcontainer-features.json';
// v2
export const DEVCONTAINER_FEATURE_FILE_NAME = 'devcontainer-feature.json';

export interface Feature {
export type Feature = SchemaFeatureBaseProperties & SchemaFeatureLifecycleHooks & DeprecatedSchemaFeatureProperties & InternalFeatureProperties;

export const CONTAINER_BUILD_FEATURES_DIR = '/opt/build-features';
joshspicer marked this conversation as resolved.
Show resolved Hide resolved

export interface SchemaFeatureLifecycleHooks {
onCreateCommand?: string | string[];
updateContentCommand?: string | string[];
postCreateCommand?: string | string[];
postStartCommand?: string | string[];
postAttachCommand?: string | string[];
}

// Properties who are members of the schema
export interface SchemaFeatureBaseProperties {
id: string;
version?: string;
name?: string;
description?: string;
cachePath?: string;
internalVersion?: string; // set programmatically
consecutiveId?: string;
documentationURL?: string;
licenseURL?: string;
options?: Record<string, FeatureOption>;
buildArg?: string; // old properties for temporary compatibility
containerEnv?: Record<string, string>;
mounts?: Mount[];
init?: boolean;
privileged?: boolean;
capAdd?: string[];
securityOpt?: string[];
entrypoint?: string;
include?: string[];
exclude?: string[];
value: boolean | string | Record<string, boolean | string | undefined>; // set programmatically
included: boolean; // set programmatically
customizations?: VSCodeCustomizations;
installsAfter?: string[];
deprecated?: boolean;
legacyIds?: string[];
currentId?: string; // set programmatically
}

// Properties that are set programmatically for book-keeping purposes
export interface InternalFeatureProperties {
cachePath?: string;
internalVersion?: string;
consecutiveId?: string;
value: boolean | string | Record<string, boolean | string | undefined>;
currentId?: string;
included: boolean;
}

// Old or deprecated properties maintained for backwards compatibility
export interface DeprecatedSchemaFeatureProperties {
buildArg?: string;
include?: string[];
exclude?: string[];
}

export type FeatureOption = {
Expand Down Expand Up @@ -226,14 +247,14 @@ export function getContainerFeaturesBaseDockerFile() {

FROM $_DEV_CONTAINERS_BASE_IMAGE AS dev_containers_feature_content_normalize
USER root
COPY --from=dev_containers_feature_content_source {contentSourceRootPath} /tmp/build-features/
RUN chmod -R 0700 /tmp/build-features
COPY --from=dev_containers_feature_content_source {contentSourceRootPath} /opt/build-features/
RUN chmod -R 0777 /opt/build-features
joshspicer marked this conversation as resolved.
Show resolved Hide resolved

FROM $_DEV_CONTAINERS_BASE_IMAGE AS dev_containers_target_stage

USER root

COPY --from=dev_containers_feature_content_normalize /tmp/build-features /tmp/build-features
COPY --from=dev_containers_feature_content_normalize /opt/build-features /opt/build-features

#{featureLayer}

Expand Down Expand Up @@ -312,15 +333,15 @@ function escapeQuotesForShell(input: string) {

export function getFeatureLayers(featuresConfig: FeaturesConfig, containerUser: string, remoteUser: string) {
let result = `RUN \\
echo "_CONTAINER_USER_HOME=$(getent passwd ${containerUser} | cut -d: -f6)" >> /tmp/build-features/devcontainer-features.builtin.env && \\
echo "_REMOTE_USER_HOME=$(getent passwd ${remoteUser} | cut -d: -f6)" >> /tmp/build-features/devcontainer-features.builtin.env
echo "_CONTAINER_USER_HOME=$(getent passwd ${containerUser} | cut -d: -f6)" >> /opt/build-features/devcontainer-features.builtin.env && \\
echo "_REMOTE_USER_HOME=$(getent passwd ${remoteUser} | cut -d: -f6)" >> /opt/build-features/devcontainer-features.builtin.env

`;

// Features version 1
const folders = (featuresConfig.featureSets || []).filter(y => y.internalVersion !== '2').map(x => x.features[0].consecutiveId);
folders.forEach(folder => {
result += `RUN cd /tmp/build-features/${folder} \\
result += `RUN cd /opt/build-features/${folder} \\
&& chmod +x ./install.sh \\
&& ./install.sh

Expand All @@ -331,7 +352,7 @@ echo "_REMOTE_USER_HOME=$(getent passwd ${remoteUser} | cut -d: -f6)" >> /tmp/bu
featureSet.features.forEach(feature => {
result += generateContainerEnvs(feature);
result += `
RUN cd /tmp/build-features/${feature.consecutiveId} \\
RUN cd /opt/build-features/${feature.consecutiveId} \\
&& chmod +x ./devcontainer-features-install.sh \\
&& ./devcontainer-features-install.sh

Expand Down
2 changes: 1 addition & 1 deletion src/spec-node/configContainer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ async function resolveWithLocalFolder(params: DockerResolverParameters, parsedAu
const configWithRaw = addSubstitution(configs.config, config => beforeContainerSubstitute(envListToObj(idLabels), config));
const { config } = configWithRaw;

await runUserCommand({ ...params, common: { ...common, output: common.postCreate.output } }, config.initializeCommand, common.postCreate.onDidInput);
await runUserCommand({ ...params, common: { ...common, output: common.lifecycleHook.output } }, config.initializeCommand, common.lifecycleHook.onDidInput);

let result: ResolverResult;
if (isDockerFileConfig(config) || 'image' in config) {
Expand Down
14 changes: 7 additions & 7 deletions src/spec-node/containerFeatures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -283,8 +283,8 @@ async function getFeaturesBuildOptions(params: DockerResolverParameters, devCont
await cliHost.writeFile(envPath, Buffer.from(builtinVariables.join('\n') + '\n'));

// When copying via buildkit, the content is accessed via '.' (i.e. in the context root)
// When copying via temp image, the content is in '/tmp/build-features'
const contentSourceRootPath = useBuildKitBuildContexts ? '.' : '/tmp/build-features/';
// When copying via temp image, the content is in '/opt/build-features'
const contentSourceRootPath = useBuildKitBuildContexts ? '.' : '/opt/build-features/';
const dockerfile = getContainerFeaturesBaseDockerFile()
.replace('#{nonBuildKitFeatureContentFallback}', useBuildKitBuildContexts ? '' : `FROM ${buildContentImageName} as dev_containers_feature_content_source`)
.replace('{contentSourceRootPath}', contentSourceRootPath)
Expand Down Expand Up @@ -348,7 +348,7 @@ ARG _DEV_CONTAINERS_BASE_IMAGE=placeholder
if (!useBuildKitBuildContexts) {
const buildContentDockerfile = `
FROM scratch
COPY . /tmp/build-features/
COPY . /opt/build-features/
`;
const buildContentDockerfilePath = cliHost.path.join(dstFolder, 'Dockerfile.buildContent');
await cliHost.writeFile(buildContentDockerfilePath, Buffer.from(buildContentDockerfile));
Expand Down Expand Up @@ -393,9 +393,9 @@ function getFeatureBuildStages(featuresConfig: FeaturesConfig, buildStageScripts
.map((featureSet, i) => featureSet.features
.filter(f => (includeAllConfiguredFeatures || f.included) && f.value && buildStageScripts[i][f.id]?.hasAcquire)
.map(f => `FROM mcr.microsoft.com/vscode/devcontainers/base:0-focal as ${getSourceInfoString(featureSet.sourceInformation)}_${f.id}
COPY --from=dev_containers_feature_content_normalize ${path.posix.join(contentSourceRootPath, getSourceInfoString(featureSet.sourceInformation), 'features', f.id)} ${path.posix.join('/tmp/build-features', getSourceInfoString(featureSet.sourceInformation), 'features', f.id)}
COPY --from=dev_containers_feature_content_normalize ${path.posix.join(contentSourceRootPath, getSourceInfoString(featureSet.sourceInformation), 'common')} ${path.posix.join('/tmp/build-features', getSourceInfoString(featureSet.sourceInformation), 'common')}
RUN cd ${path.posix.join('/tmp/build-features', getSourceInfoString(featureSet.sourceInformation), 'features', f.id)} && set -a && . ./devcontainer-features.env && set +a && ./bin/acquire`
COPY --from=dev_containers_feature_content_normalize ${path.posix.join(contentSourceRootPath, getSourceInfoString(featureSet.sourceInformation), 'features', f.id)} ${path.posix.join('/opt/build-features', getSourceInfoString(featureSet.sourceInformation), 'features', f.id)}
COPY --from=dev_containers_feature_content_normalize ${path.posix.join(contentSourceRootPath, getSourceInfoString(featureSet.sourceInformation), 'common')} ${path.posix.join('/opt/build-features', getSourceInfoString(featureSet.sourceInformation), 'common')}
RUN cd ${path.posix.join('/opt/build-features', getSourceInfoString(featureSet.sourceInformation), 'features', f.id)} && set -a && . ./devcontainer-features.env && set +a && ./bin/acquire`
)
)
).join('\n\n');
Expand All @@ -408,7 +408,7 @@ function getCopyFeatureBuildStages(featuresConfig: FeaturesConfig, buildStageScr
.map(f => {
const featurePath = path.posix.join('/usr/local/devcontainer-features', getSourceInfoString(featureSet.sourceInformation), f.id);
return `COPY --from=${getSourceInfoString(featureSet.sourceInformation)}_${f.id} ${featurePath} ${featurePath}${buildStageScripts[i][f.id]?.hasConfigure ? `
RUN cd ${path.posix.join('/tmp/build-features', getSourceInfoString(featureSet.sourceInformation), 'features', f.id)} && set -a && . ./devcontainer-features.env && set +a && ./bin/configure` : ''}`;
RUN cd ${path.posix.join('/opt/build-features', getSourceInfoString(featureSet.sourceInformation), 'features', f.id)} && set -a && . ./devcontainer-features.env && set +a && ./bin/configure` : ''}`;
})
)
).join('\n\n');
Expand Down
4 changes: 2 additions & 2 deletions src/spec-node/devContainers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import * as crypto from 'crypto';
import * as os from 'os';

import { DockerResolverParameters, DevContainerAuthority, UpdateRemoteUserUIDDefault, BindMountConsistency, getCacheFolder } from './utils';
import { createNullPostCreate, finishBackgroundTasks, ResolverParameters, UserEnvProbe } from '../spec-common/injectHeadless';
import { createNullLifecycleHook, finishBackgroundTasks, ResolverParameters, UserEnvProbe } from '../spec-common/injectHeadless';
import { getCLIHost, loadNativeModule } from '../spec-common/commonUtils';
import { resolve } from './configContainer';
import { URI } from 'vscode-uri';
Expand Down Expand Up @@ -123,7 +123,7 @@ export async function createDockerParams(options: ProvisionOptions, disposables:
output,
allowSystemConfigChange: true,
defaultUserEnvProbe: options.defaultUserEnvProbe,
postCreate: createNullPostCreate(options.postCreateEnabled, options.skipNonBlocking, output),
lifecycleHook: createNullLifecycleHook(options.postCreateEnabled, options.skipNonBlocking, output),
getLogLevel: () => options.logLevel,
onDidChangeLogLevel: () => ({ dispose() { } }),
loadNativeModule,
Expand Down
8 changes: 4 additions & 4 deletions src/spec-node/devContainersSpecCLI.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { SubstitutedConfig, createContainerProperties, createFeaturesTempFolder,
import { URI } from 'vscode-uri';
import { ContainerError } from '../spec-common/errors';
import { Log, LogLevel, makeLog, mapLogLevel } from '../spec-utils/log';
import { probeRemoteEnv, runPostCreateCommands, runRemoteCommand, UserEnvProbe, setupInContainer } from '../spec-common/injectHeadless';
import { probeRemoteEnv, runLifecycleHooks, runRemoteCommand, UserEnvProbe, setupInContainer } from '../spec-common/injectHeadless';
import { bailOut, buildNamedImageAndExtend, findDevContainer, hostFolderLabel } from './singleContainer';
import { extendImage } from './containerFeatures';
import { DockerCLIParameters, dockerPtyCLI, inspectContainer } from '../spec-shutdown/dockerUtils';
Expand All @@ -31,7 +31,7 @@ import { featuresPublishHandler, featuresPublishOptions } from './featuresCLI/pu
import { featureInfoTagsHandler, featuresInfoTagsOptions } from './featuresCLI/infoTags';
import { beforeContainerSubstitute, containerSubstitute, substitute } from '../spec-common/variableSubstitution';
import { getPackageConfig, PackageConfiguration } from '../spec-utils/product';
import { getDevcontainerMetadata, getImageBuildInfo, getImageMetadataFromContainer, ImageMetadataEntry, mergeConfiguration, MergedDevContainerConfig } from './imageMetadata';
import { getDevcontainerMetadata, getImageBuildInfo, getImageMetadataFromContainer, ImageMetadataEntry, lifecycleCommandOriginMapFromMetadata, mergeConfiguration, MergedDevContainerConfig } from './imageMetadata';
import { templatesPublishHandler, templatesPublishOptions } from './templatesCLI/publish';
import { templateApplyHandler, templateApplyOptions } from './templatesCLI/apply';
import { featuresInfoManifestHandler, featuresInfoManifestOptions } from './featuresCLI/infoManifest';
Expand Down Expand Up @@ -423,7 +423,7 @@ async function doSetUp({
const imageMetadata = getImageMetadataFromContainer(container, config, undefined, undefined, true, output).config;
const mergedConfig = mergeConfiguration(config.config, imageMetadata);
const containerProperties = await createContainerProperties(params, container.Id, configs?.workspaceConfig.workspaceFolder, mergedConfig.remoteUser);
await setupInContainer(common, containerProperties, mergedConfig);
await setupInContainer(common, containerProperties, mergedConfig, lifecycleCommandOriginMapFromMetadata(imageMetadata));
return {
outcome: 'success' as 'success',
dispose,
Expand Down Expand Up @@ -835,7 +835,7 @@ async function doRunUserCommands({
const containerProperties = await createContainerProperties(params, container.Id, configs?.workspaceConfig.workspaceFolder, mergedConfig.remoteUser);
const updatedConfig = containerSubstitute(cliHost.platform, config.config.configFilePath, containerProperties.env, mergedConfig);
const remoteEnv = probeRemoteEnv(common, containerProperties, updatedConfig);
const result = await runPostCreateCommands(common, containerProperties, updatedConfig, remoteEnv, stopForPersonalization);
const result = await runLifecycleHooks(common, lifecycleCommandOriginMapFromMetadata(imageMetadata), containerProperties, updatedConfig, remoteEnv, stopForPersonalization);
return {
outcome: 'success' as 'success',
result,
Expand Down
8 changes: 4 additions & 4 deletions src/spec-node/dockerCompose.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { Log, LogLevel, makeLog, terminalEscapeSequences } from '../spec-utils/l
import { getExtendImageBuildInfo, updateRemoteUserUID } from './containerFeatures';
import { Mount, parseMount } from '../spec-configuration/containerFeaturesConfiguration';
import path from 'path';
import { getDevcontainerMetadata, getImageBuildInfoFromDockerfile, getImageBuildInfoFromImage, getImageMetadataFromContainer, ImageBuildInfo, mergeConfiguration, MergedDevContainerConfig } from './imageMetadata';
import { getDevcontainerMetadata, getImageBuildInfoFromDockerfile, getImageBuildInfoFromImage, getImageMetadataFromContainer, ImageBuildInfo, lifecycleCommandOriginMapFromMetadata, mergeConfiguration, MergedDevContainerConfig } from './imageMetadata';
import { ensureDockerfileHasFinalStageName } from './dockerfileUtils';

const projectLabel = 'com.docker.compose.project';
Expand Down Expand Up @@ -68,13 +68,13 @@ async function _openDockerComposeDevContainer(params: DockerResolverParameters,
// collapsedFeaturesConfig = collapseFeaturesConfig(featuresConfig);
}

const configs = getImageMetadataFromContainer(container, configWithRaw, undefined, idLabels, common.experimentalImageMetadata, common.output).config;
const mergedConfig = mergeConfiguration(configWithRaw.config, configs);
const imageMetadata = getImageMetadataFromContainer(container, configWithRaw, undefined, idLabels, common.experimentalImageMetadata, common.output).config;
const mergedConfig = mergeConfiguration(configWithRaw.config, imageMetadata);
containerProperties = await createContainerProperties(params, container.Id, remoteWorkspaceFolder, mergedConfig.remoteUser);

const {
remoteEnv: extensionHostEnv,
} = await setupInContainer(common, containerProperties, mergedConfig);
} = await setupInContainer(common, containerProperties, mergedConfig, lifecycleCommandOriginMapFromMetadata(imageMetadata));

return {
params: common,
Expand Down
Loading