From efa0f62bcdb19b62701a2770fa98984e0422b689 Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Wed, 18 Jan 2023 23:11:58 +0000 Subject: [PATCH 01/41] refactor Feature interface --- .../containerFeaturesConfiguration.ts | 31 +++++++++++++------ 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/src/spec-configuration/containerFeaturesConfiguration.ts b/src/spec-configuration/containerFeaturesConfiguration.ts index 2b3355740..f687b2183 100644 --- a/src/spec-configuration/containerFeaturesConfiguration.ts +++ b/src/spec-configuration/containerFeaturesConfiguration.ts @@ -23,18 +23,17 @@ 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 = SchemaFeatureProperties & DeprecatedSchemaFeatureProperties & InternalFeatureProperties; + +// Properties who are members of the schema +export interface SchemaFeatureProperties { id: string; version?: string; name?: string; description?: string; - cachePath?: string; - internalVersion?: string; // set programmatically - consecutiveId?: string; documentationURL?: string; licenseURL?: string; options?: Record; - buildArg?: string; // old properties for temporary compatibility containerEnv?: Record; mounts?: Mount[]; init?: boolean; @@ -42,15 +41,27 @@ export interface Feature { capAdd?: string[]; securityOpt?: string[]; entrypoint?: string; - include?: string[]; - exclude?: string[]; - value: boolean | string | Record; // 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; + currentId?: string; + included: boolean; +} + +// Old or deprecated properties maintained for backwards compatibility +export interface DeprecatedSchemaFeatureProperties { + buildArg?: string; + include?: string[]; + exclude?: string[]; } export type FeatureOption = { From 279da408d92eea185164a073724cb062be74e3f1 Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Thu, 19 Jan 2023 22:51:47 +0000 Subject: [PATCH 02/41] Feat: Add in inline command lifecycle hook contribution from Features --- .gitignore | 1 + .../containerFeaturesConfiguration.ts | 5 ++ src/spec-node/imageMetadata.ts | 5 ++ .../.devcontainer/Dockerfile | 1 + .../.devcontainer/devcontainer.json | 14 ++++++ .../panda/devcontainer-feature.json | 11 ++++ .../.devcontainer/panda/install.sh | 12 +++++ .../tiger/devcontainer-feature.json | 11 ++++ .../.devcontainer/tiger/install.sh | 12 +++++ .../container-features/lifecycleHooks.test.ts | 50 +++++++++++++++++++ 10 files changed, 122 insertions(+) create mode 100644 src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/Dockerfile create mode 100644 src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/devcontainer.json create mode 100644 src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/panda/devcontainer-feature.json create mode 100644 src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/panda/install.sh create mode 100644 src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/tiger/devcontainer-feature.json create mode 100644 src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/tiger/install.sh create mode 100644 src/test/container-features/lifecycleHooks.test.ts diff --git a/.gitignore b/.gitignore index b954d43f1..57205e24f 100644 --- a/.gitignore +++ b/.gitignore @@ -11,3 +11,4 @@ build-tmp .DS_Store .env output +*.testMarker \ No newline at end of file diff --git a/src/spec-configuration/containerFeaturesConfiguration.ts b/src/spec-configuration/containerFeaturesConfiguration.ts index f687b2183..154021a55 100644 --- a/src/spec-configuration/containerFeaturesConfiguration.ts +++ b/src/spec-configuration/containerFeaturesConfiguration.ts @@ -45,6 +45,11 @@ export interface SchemaFeatureProperties { installsAfter?: string[]; deprecated?: boolean; legacyIds?: string[]; + onCreateCommand?: string | string[]; + updateContentCommand?: string | string[]; + postCreateCommand?: string | string[]; + postStartCommand?: string | string[]; + postAttachCommand?: string | string[]; } // Properties that are set programmatically for book-keeping purposes diff --git a/src/spec-node/imageMetadata.ts b/src/spec-node/imageMetadata.ts index 69aba3fd5..cf615e7b3 100644 --- a/src/spec-node/imageMetadata.ts +++ b/src/spec-node/imageMetadata.ts @@ -46,6 +46,11 @@ const pickUpdateableConfigProperties: (keyof DevContainerConfig & keyof ImageMet ]; const pickFeatureProperties: Exclude[] = [ + 'onCreateCommand', + 'updateContentCommand', + 'postCreateCommand', + 'postStartCommand', + 'postAttachCommand', 'init', 'privileged', 'capAdd', diff --git a/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/Dockerfile b/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/Dockerfile new file mode 100644 index 000000000..c286a4279 --- /dev/null +++ b/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/Dockerfile @@ -0,0 +1 @@ +FROM mcr.microsoft.com/devcontainers/base:ubuntu \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/devcontainer.json b/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/devcontainer.json new file mode 100644 index 000000000..bc518ef83 --- /dev/null +++ b/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/devcontainer.json @@ -0,0 +1,14 @@ +{ + "build": { + "dockerfile": "Dockerfile" + }, + "features": { + "./tiger": {}, + "./panda": {} + }, + "updateContentCommand": "touch devContainer.updateContentCommand.testMarker", + "onCreateCommand": "touch devContainer.onCreateCommand.testMarker", + "postCreateCommand": "touch devContainer.postCreateCommand.testMarker", + "postStartCommand": "touch devContainer.postStartCommand.testMarker", + "postAttachCommand": "touch devContainer.postAttachCommand.testMarker" +} \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/panda/devcontainer-feature.json b/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/panda/devcontainer-feature.json new file mode 100644 index 000000000..468b2a56b --- /dev/null +++ b/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/panda/devcontainer-feature.json @@ -0,0 +1,11 @@ +{ + "id": "panda", + "version": "4.5.6", + "name": "Echos the panda emoji. That is it!", + "options": {}, + "updateContentCommand": "touch panda.updateContentCommand.testMarker", + "onCreateCommand": "touch panda.onCreateCommand.testMarker", + "postCreateCommand": "touch panda.postCreateCommand.testMarker", + "postStartCommand": "touch panda.postStartCommand.testMarker", + "postAttachCommand": "touch panda.postAttachCommand.testMarker" +} \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/panda/install.sh b/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/panda/install.sh new file mode 100644 index 000000000..be7e04401 --- /dev/null +++ b/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/panda/install.sh @@ -0,0 +1,12 @@ +#!/bin/sh +set -e + +echo "Activating feature 'panda'" + +cat > /usr/local/bin/panda \ +<< EOF +#!/bin/sh +echo 🐼 +EOF + +chmod +x /usr/local/bin/panda \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/tiger/devcontainer-feature.json b/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/tiger/devcontainer-feature.json new file mode 100644 index 000000000..b7d0bb134 --- /dev/null +++ b/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/tiger/devcontainer-feature.json @@ -0,0 +1,11 @@ +{ + "id": "tiger", + "version": "8.9.10", + "name": "Echos the tiger emoji. That is it!", + "options": {}, + "updateContentCommand": "touch tiger.updateContentCommand.testMarker", + "onCreateCommand": "touch tiger.onCreateCommand.testMarker", + "postCreateCommand": "touch tiger.postCreateCommand.testMarker", + "postStartCommand": "touch tiger.postStartCommand.testMarker", + "postAttachCommand": "touch tiger.postAttachCommand.testMarker" +} \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/tiger/install.sh b/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/tiger/install.sh new file mode 100644 index 000000000..bf6d6dfd9 --- /dev/null +++ b/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/tiger/install.sh @@ -0,0 +1,12 @@ +#!/bin/sh +set -e + +echo "Activating feature 'tiger'" + +cat > /usr/local/bin/tiger \ +<< EOF +#!/bin/sh +echo 🐯 +EOF + +chmod +x /usr/local/bin/tiger \ No newline at end of file diff --git a/src/test/container-features/lifecycleHooks.test.ts b/src/test/container-features/lifecycleHooks.test.ts new file mode 100644 index 000000000..fd30e2a84 --- /dev/null +++ b/src/test/container-features/lifecycleHooks.test.ts @@ -0,0 +1,50 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { assert } from 'chai'; +import * as path from 'path'; +import { devContainerDown, devContainerUp, shellExec } from '../testUtils'; + +const pkg = require('../../../package.json'); + +describe('Feature lifecycle hooks', function () { + this.timeout('120s'); + + const tmp = path.relative(process.cwd(), path.join(__dirname, 'tmp5')); + const cli = `npx --prefix ${tmp} devcontainer`; + + before('Install', async () => { + await shellExec(`rm -rf ${tmp}/node_modules`); + await shellExec(`mkdir -p ${tmp}`); + await shellExec(`npm --prefix ${tmp} install devcontainers-cli-${pkg.version}.tgz`); + }); + + describe('lifecycle-hooks-inline-commands', () => { + + describe(`devcontainer up`, () => { + let containerId: string | null = null; + const testFolder = `${__dirname}/configs/lifecycle-hooks-inline-commands`; + + before(async () => { + await shellExec(`rm -f ${testFolder}/*.testMarker`, undefined, undefined, true); + containerId = (await devContainerUp(cli, testFolder, { 'logLevel': 'trace' })).containerId; + }); + + after(async () => { + await devContainerDown({ containerId }); + await shellExec(`rm -f ${testFolder}/*.testMarker`, undefined, undefined, true); + }); + + it('marker files should exist', async () => { + const res = await shellExec(`${cli} exec --workspace-folder ${testFolder} ls -altr`); + const response = JSON.parse(res.stdout); + console.log(res.stderr); + assert.equal(response.outcome, 'success'); + assert.match(res.stderr, /tiger.postCreateCommand.testMarker/); + }); + }); + }); + +}); \ No newline at end of file From f43350eafdbaa5fc76dca6fe802c060a16329cce Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Mon, 23 Jan 2023 23:48:22 +0000 Subject: [PATCH 03/41] markerfile to exercise order of execution --- .../.devcontainer/Dockerfile | 1 + .../.devcontainer/devcontainer.json | 17 +++++++ .../otter/devcontainer-feature.json | 13 +++++ .../.devcontainer/otter/install.sh | 12 +++++ .../rabbit/devcontainer-feature.json | 13 +++++ .../.devcontainer/rabbit/install.sh | 12 +++++ .../.devcontainer/createMarker.sh | 14 +++++ .../.devcontainer/devcontainer.json | 13 +++-- .../panda/devcontainer-feature.json | 13 +++-- .../tiger/devcontainer-feature.json | 13 +++-- .../container-features/lifecycleHooks.test.ts | 51 ++++++++++++++++++- 11 files changed, 156 insertions(+), 16 deletions(-) create mode 100644 src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/Dockerfile create mode 100644 src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/devcontainer.json create mode 100644 src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json create mode 100644 src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/install.sh create mode 100644 src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/devcontainer-feature.json create mode 100644 src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/install.sh create mode 100755 src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/createMarker.sh diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/Dockerfile b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/Dockerfile new file mode 100644 index 000000000..c286a4279 --- /dev/null +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/Dockerfile @@ -0,0 +1 @@ +FROM mcr.microsoft.com/devcontainers/base:ubuntu \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/devcontainer.json b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/devcontainer.json new file mode 100644 index 000000000..27e449316 --- /dev/null +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/devcontainer.json @@ -0,0 +1,17 @@ +{ + "build": { + "dockerfile": "Dockerfile" + }, + "features": { + "./otter": {}, + "./rabbit": {} + }, + // "updateContentCommand": "touch devContainer.updateContentCommand.testMarker", + // "onCreateCommand": "touch devContainer.onCreateCommand.testMarker", + // "postCreateCommand": [ + // "touch", + // "devContainer.postCreateCommand.testMarker" + // ], + "postStartCommand": "touch `rabbit`.postStartCommand.testMarker", // The 'rabbit' command is installed and added to the path by .devcontainer/rabbit/install.sh + "postAttachCommand": "touch `otter`.postAttachCommand.testMarker" // The 'otter' command is installed and added to the path by .devcontainer/otter/install.sh +} \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json new file mode 100644 index 000000000..9bbca4780 --- /dev/null +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json @@ -0,0 +1,13 @@ +{ + "id": "otter", + "version": "1.2.3", + "options": {} + // "onCreateCommand": "touch tiger.onCreateCommand.testMarker", + // "updateContentCommand": "touch tiger.updateContentCommand.testMarker", + // "postCreateCommand": "touch tiger.postCreateCommand.testMarker", + // "postStartCommand": "touch tiger.postStartCommand.testMarker", + // "postAttachCommand": [ + // "touch", + // "tiger.postAttachCommand.testMarker" + // ] +} \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/install.sh b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/install.sh new file mode 100644 index 000000000..e937ebb91 --- /dev/null +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/install.sh @@ -0,0 +1,12 @@ +#!/bin/sh +set -e + +echo "Activating feature 'otter'" + +cat > /usr/local/bin/otter \ +<< EOF +#!/bin/sh +echo "i-am-an-otter" +EOF + +chmod +x /usr/local/bin/otter \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/devcontainer-feature.json b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/devcontainer-feature.json new file mode 100644 index 000000000..06da7b088 --- /dev/null +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/devcontainer-feature.json @@ -0,0 +1,13 @@ +{ + "id": "rabbit", + "version": "100.200.300", + "options": {} + // "onCreateCommand": "touch rabbit.onCreateCommand.testMarker", + // "updateContentCommand": [ + // "touch", + // "panda.updateContentCommand.testMarker" + // ], + // "postCreateCommand": "touch panda.postCreateCommand.testMarker", + // "postStartCommand": "touch panda.postStartCommand.testMarker", + // "postAttachCommand": "touch panda.postAttachCommand.testMarker" +} \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/install.sh b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/install.sh new file mode 100644 index 000000000..49ea5a294 --- /dev/null +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/install.sh @@ -0,0 +1,12 @@ +#!/bin/sh +set -e + +echo "Activating feature 'rabbit'" + +cat > /usr/local/bin/rabbit \ +<< EOF +#!/bin/sh +echo "i-am-a-rabbit" +EOF + +chmod +x /usr/local/bin/rabbit \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/createMarker.sh b/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/createMarker.sh new file mode 100755 index 000000000..de9ef399a --- /dev/null +++ b/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/createMarker.sh @@ -0,0 +1,14 @@ +#!/bin/bash + +MARKER_FILE_NAME="$1" + +echo "Starting '${MARKER_FILE_NAME}'...." +sleep 2s + +[[ -f saved_value.testMarker ]] || echo 0 > saved_value.testMarker +n=$(< saved_value.testMarker) +cat saved_value.testMarker > "${n}.${MARKER_FILE_NAME}" +echo $(( n + 1 )) > saved_value.testMarker + +echo "Ending '${MARKER_FILE_NAME}'...." +sleep 2s \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/devcontainer.json b/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/devcontainer.json index bc518ef83..f44b23fd1 100644 --- a/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/devcontainer.json +++ b/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/devcontainer.json @@ -6,9 +6,12 @@ "./tiger": {}, "./panda": {} }, - "updateContentCommand": "touch devContainer.updateContentCommand.testMarker", - "onCreateCommand": "touch devContainer.onCreateCommand.testMarker", - "postCreateCommand": "touch devContainer.postCreateCommand.testMarker", - "postStartCommand": "touch devContainer.postStartCommand.testMarker", - "postAttachCommand": "touch devContainer.postAttachCommand.testMarker" + "onCreateCommand": "./.devcontainer/createMarker.sh devContainer.onCreateCommand.testMarker", + "updateContentCommand": "./.devcontainer/createMarker.sh devContainer.updateContentCommand.testMarker", + "postCreateCommand": [ + ".devcontainer/createMarker.sh", + "devContainer.postCreateCommand.testMarker" + ], + "postStartCommand": "./.devcontainer/createMarker.sh devContainer.postStartCommand.testMarker", + "postAttachCommand": "./.devcontainer/createMarker.sh devContainer.postAttachCommand.testMarker" } \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/panda/devcontainer-feature.json b/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/panda/devcontainer-feature.json index 468b2a56b..4a7c12c20 100644 --- a/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/panda/devcontainer-feature.json +++ b/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/panda/devcontainer-feature.json @@ -3,9 +3,12 @@ "version": "4.5.6", "name": "Echos the panda emoji. That is it!", "options": {}, - "updateContentCommand": "touch panda.updateContentCommand.testMarker", - "onCreateCommand": "touch panda.onCreateCommand.testMarker", - "postCreateCommand": "touch panda.postCreateCommand.testMarker", - "postStartCommand": "touch panda.postStartCommand.testMarker", - "postAttachCommand": "touch panda.postAttachCommand.testMarker" + "updateContentCommand": [ + ".devcontainer/createMarker.sh", + "panda.updateContentCommand.testMarker" + ], + "onCreateCommand": "./.devcontainer/createMarker.sh panda.onCreateCommand.testMarker", + "postCreateCommand": "./.devcontainer/createMarker.sh panda.postCreateCommand.testMarker", + "postStartCommand": "./.devcontainer/createMarker.sh panda.postStartCommand.testMarker", + "postAttachCommand": "./.devcontainer/createMarker.sh panda.postAttachCommand.testMarker" } \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/tiger/devcontainer-feature.json b/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/tiger/devcontainer-feature.json index b7d0bb134..6b1f6919b 100644 --- a/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/tiger/devcontainer-feature.json +++ b/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/tiger/devcontainer-feature.json @@ -3,9 +3,12 @@ "version": "8.9.10", "name": "Echos the tiger emoji. That is it!", "options": {}, - "updateContentCommand": "touch tiger.updateContentCommand.testMarker", - "onCreateCommand": "touch tiger.onCreateCommand.testMarker", - "postCreateCommand": "touch tiger.postCreateCommand.testMarker", - "postStartCommand": "touch tiger.postStartCommand.testMarker", - "postAttachCommand": "touch tiger.postAttachCommand.testMarker" + "updateContentCommand": "./.devcontainer/createMarker.sh tiger.updateContentCommand.testMarker", + "onCreateCommand": "./.devcontainer/createMarker.sh tiger.onCreateCommand.testMarker", + "postCreateCommand": "./.devcontainer/createMarker.sh tiger.postCreateCommand.testMarker", + "postStartCommand": "./.devcontainer/createMarker.sh tiger.postStartCommand.testMarker", + "postAttachCommand": [ + ".devcontainer/createMarker.sh", + "tiger.postAttachCommand.testMarker" + ] } \ No newline at end of file diff --git a/src/test/container-features/lifecycleHooks.test.ts b/src/test/container-features/lifecycleHooks.test.ts index fd30e2a84..a9457302f 100644 --- a/src/test/container-features/lifecycleHooks.test.ts +++ b/src/test/container-features/lifecycleHooks.test.ts @@ -42,9 +42,58 @@ describe('Feature lifecycle hooks', function () { const response = JSON.parse(res.stdout); console.log(res.stderr); assert.equal(response.outcome, 'success'); - assert.match(res.stderr, /tiger.postCreateCommand.testMarker/); + + assert.match(res.stderr, /0.panda.onCreateCommand.testMarker/); + assert.match(res.stderr, /3.panda.updateContentCommand.testMarker/); + assert.match(res.stderr, /6.panda.postCreateCommand.testMarker/); + assert.match(res.stderr, /9.panda.postStartCommand.testMarker/); + assert.match(res.stderr, /12.panda.postAttachCommand.testMarker/); + + assert.match(res.stderr, /1.tiger.onCreateCommand.testMarker/); + assert.match(res.stderr, /4.tiger.updateContentCommand.testMarker/); + assert.match(res.stderr, /7.tiger.postCreateCommand.testMarker/); + assert.match(res.stderr, /10.tiger.postStartCommand.testMarker/); + assert.match(res.stderr, /13.tiger.postAttachCommand.testMarker/); + + assert.match(res.stderr, /2.devContainer.onCreateCommand.testMarker/); + assert.match(res.stderr, /5.devContainer.updateContentCommand.testMarker/); + assert.match(res.stderr, /8.devContainer.postCreateCommand.testMarker/); + assert.match(res.stderr, /11.devContainer.postStartCommand.testMarker/); + assert.match(res.stderr, /14.devContainer.postAttachCommand.testMarker/); + }); + }); + }); + + describe('lifecycle-hooks-advanced', () => { + + describe(`devcontainer up`, () => { + let containerId: string | null = null; + const testFolder = `${__dirname}/configs/lifecycle-hooks-advanced`; + + before(async () => { + await shellExec(`rm -f ${testFolder}/*.testMarker`, undefined, undefined, true); + containerId = (await devContainerUp(cli, testFolder, { 'logLevel': 'trace' })).containerId; + }); + + after(async () => { + await devContainerDown({ containerId }); + await shellExec(`rm -f ${testFolder}/*.testMarker`, undefined, undefined, true); + }); + + it('Feature command added to path can be executed in a lifecycle scripts', async () => { + const res = await shellExec(`${cli} exec --workspace-folder ${testFolder} ls -altr`); + const response = JSON.parse(res.stdout); + console.log(res.stderr); + assert.equal(response.outcome, 'success'); + + assert.match(res.stderr, /i-am-a-rabbit.postStartCommand.testMarker/); + assert.match(res.stderr, /i-am-an-otter.postAttachCommand.testMarker/); }); }); }); + describe('lifecycle-hooks-parallel-execution', () => { + assert.fail('TODO'); + }); + }); \ No newline at end of file From b3c6d87e1022ebb95a805985ad2f2b7aab941f2b Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Tue, 24 Jan 2023 00:19:38 +0000 Subject: [PATCH 04/41] add in two helper scripts with the same name in two different Features --- .../.devcontainer/otter/helper_script.sh | 3 + .../.devcontainer/rabbit/helper_script.sh | 3 + .../container-features/lifecycleHooks.test.ts | 62 +++++++++++-------- 3 files changed, 42 insertions(+), 26 deletions(-) create mode 100644 src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/helper_script.sh create mode 100644 src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/helper_script.sh diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/helper_script.sh b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/helper_script.sh new file mode 100644 index 000000000..070386929 --- /dev/null +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/helper_script.sh @@ -0,0 +1,3 @@ +#!/bin/bash + +echo "Hello from otter helper_script.sh" \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/helper_script.sh b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/helper_script.sh new file mode 100644 index 000000000..149dbcad2 --- /dev/null +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/helper_script.sh @@ -0,0 +1,3 @@ +#!/bin/bash + +echo "Hello from rabbit helper_script.sh" \ No newline at end of file diff --git a/src/test/container-features/lifecycleHooks.test.ts b/src/test/container-features/lifecycleHooks.test.ts index a9457302f..c07addadf 100644 --- a/src/test/container-features/lifecycleHooks.test.ts +++ b/src/test/container-features/lifecycleHooks.test.ts @@ -26,7 +26,7 @@ describe('Feature lifecycle hooks', function () { describe(`devcontainer up`, () => { let containerId: string | null = null; const testFolder = `${__dirname}/configs/lifecycle-hooks-inline-commands`; - + before(async () => { await shellExec(`rm -f ${testFolder}/*.testMarker`, undefined, undefined, true); containerId = (await devContainerUp(cli, testFolder, { 'logLevel': 'trace' })).containerId; @@ -35,31 +35,33 @@ describe('Feature lifecycle hooks', function () { after(async () => { await devContainerDown({ containerId }); await shellExec(`rm -f ${testFolder}/*.testMarker`, undefined, undefined, true); - }); + }); it('marker files should exist', async () => { const res = await shellExec(`${cli} exec --workspace-folder ${testFolder} ls -altr`); const response = JSON.parse(res.stdout); - console.log(res.stderr); assert.equal(response.outcome, 'success'); - assert.match(res.stderr, /0.panda.onCreateCommand.testMarker/); - assert.match(res.stderr, /3.panda.updateContentCommand.testMarker/); - assert.match(res.stderr, /6.panda.postCreateCommand.testMarker/); - assert.match(res.stderr, /9.panda.postStartCommand.testMarker/); - assert.match(res.stderr, /12.panda.postAttachCommand.testMarker/); - - assert.match(res.stderr, /1.tiger.onCreateCommand.testMarker/); - assert.match(res.stderr, /4.tiger.updateContentCommand.testMarker/); - assert.match(res.stderr, /7.tiger.postCreateCommand.testMarker/); - assert.match(res.stderr, /10.tiger.postStartCommand.testMarker/); - assert.match(res.stderr, /13.tiger.postAttachCommand.testMarker/); - - assert.match(res.stderr, /2.devContainer.onCreateCommand.testMarker/); - assert.match(res.stderr, /5.devContainer.updateContentCommand.testMarker/); - assert.match(res.stderr, /8.devContainer.postCreateCommand.testMarker/); - assert.match(res.stderr, /11.devContainer.postStartCommand.testMarker/); - assert.match(res.stderr, /14.devContainer.postAttachCommand.testMarker/); + const outputOfExecCommand = res.stderr; + console.log(outputOfExecCommand); + + assert.match(outputOfExecCommand, /0.panda.onCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /3.panda.updateContentCommand.testMarker/); + assert.match(outputOfExecCommand, /6.panda.postCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /9.panda.postStartCommand.testMarker/); + assert.match(outputOfExecCommand, /12.panda.postAttachCommand.testMarker/); + + assert.match(outputOfExecCommand, /1.tiger.onCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /4.tiger.updateContentCommand.testMarker/); + assert.match(outputOfExecCommand, /7.tiger.postCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /10.tiger.postStartCommand.testMarker/); + assert.match(outputOfExecCommand, /13.tiger.postAttachCommand.testMarker/); + + assert.match(outputOfExecCommand, /2.devContainer.onCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /5.devContainer.updateContentCommand.testMarker/); + assert.match(outputOfExecCommand, /8.devContainer.postCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /11.devContainer.postStartCommand.testMarker/); + assert.match(outputOfExecCommand, /14.devContainer.postAttachCommand.testMarker/); }); }); }); @@ -83,17 +85,25 @@ describe('Feature lifecycle hooks', function () { it('Feature command added to path can be executed in a lifecycle scripts', async () => { const res = await shellExec(`${cli} exec --workspace-folder ${testFolder} ls -altr`); const response = JSON.parse(res.stdout); - console.log(res.stderr); assert.equal(response.outcome, 'success'); - assert.match(res.stderr, /i-am-a-rabbit.postStartCommand.testMarker/); - assert.match(res.stderr, /i-am-an-otter.postAttachCommand.testMarker/); + const outputOfExecCommand = res.stderr; + console.log(outputOfExecCommand); + + // Executes the command that was installed by each Feature's 'install.sh'. + // The command is installed to a directory on the $PATH so it can be executed from the lifecycle script. + assert.match(outputOfExecCommand, /i-am-a-rabbit.postStartCommand.testMarker/); + assert.match(outputOfExecCommand, /i-am-an-otter.postAttachCommand.testMarker/); + + // Since lifecycle scripts are executed relative to the workspace folder, + // to run a script bundled with the Feature, the user needs to use the '${featureRoot}' variable. + }); }); }); - describe('lifecycle-hooks-parallel-execution', () => { - assert.fail('TODO'); - }); + // describe('lifecycle-hooks-parallel-execution', () => { + // assert.fail('TODO'); + // }); }); \ No newline at end of file From d0d99fc83cbe7fa52afe074b667b14cb45f99f52 Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Tue, 24 Jan 2023 00:35:38 +0000 Subject: [PATCH 05/41] test with ${featureRoot} --- .../otter/devcontainer-feature.json | 21 +++++++++++-------- .../.devcontainer/otter/helper_script.sh | 4 +++- .../rabbit/devcontainer-feature.json | 21 +++++++++++-------- .../.devcontainer/rabbit/helper_script.sh | 4 +++- .../container-features/lifecycleHooks.test.ts | 14 ++++++++++++- 5 files changed, 43 insertions(+), 21 deletions(-) mode change 100644 => 100755 src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/helper_script.sh mode change 100644 => 100755 src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/helper_script.sh diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json index 9bbca4780..bd4e74252 100644 --- a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json @@ -1,13 +1,16 @@ { "id": "otter", "version": "1.2.3", - "options": {} - // "onCreateCommand": "touch tiger.onCreateCommand.testMarker", - // "updateContentCommand": "touch tiger.updateContentCommand.testMarker", - // "postCreateCommand": "touch tiger.postCreateCommand.testMarker", - // "postStartCommand": "touch tiger.postStartCommand.testMarker", - // "postAttachCommand": [ - // "touch", - // "tiger.postAttachCommand.testMarker" - // ] + "options": {}, + "updateContentCommand": "${featureRoot}/helper_script.sh updateContentCommand", + "onCreateCommand": "${featureRoot}/helper_script.sh onCreateCommand", + "postCreateCommand": "${featureRoot}/helper_script.sh postCreateCommand", + "postStartCommand": [ + "${featureRoot}/helper_script.sh", + "postStartCommand" + ], + "onAttachCommand": [ + "${featureRoot}/helper_script.sh", + "onAttachCommand" + ] } \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/helper_script.sh b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/helper_script.sh old mode 100644 new mode 100755 index 070386929..caafa1e18 --- a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/helper_script.sh +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/helper_script.sh @@ -1,3 +1,5 @@ #!/bin/bash -echo "Hello from otter helper_script.sh" \ No newline at end of file +MARKER_FILE_NAME="$1" +echo "Hello from otter helper_script.sh involved by ${MARKER_FILE_NAME}" +touch "helperScript.otter.${MARKER_FILE_NAME}.markerFile" \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/devcontainer-feature.json b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/devcontainer-feature.json index 06da7b088..00b802c78 100644 --- a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/devcontainer-feature.json +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/devcontainer-feature.json @@ -1,13 +1,16 @@ { "id": "rabbit", "version": "100.200.300", - "options": {} - // "onCreateCommand": "touch rabbit.onCreateCommand.testMarker", - // "updateContentCommand": [ - // "touch", - // "panda.updateContentCommand.testMarker" - // ], - // "postCreateCommand": "touch panda.postCreateCommand.testMarker", - // "postStartCommand": "touch panda.postStartCommand.testMarker", - // "postAttachCommand": "touch panda.postAttachCommand.testMarker" + "options": {}, + "updateContentCommand": "${featureRoot}/helper_script.sh updateContentCommand", + "onCreateCommand": "${featureRoot}/helper_script.sh onCreateCommand", + "postCreateCommand": "${featureRoot}/helper_script.sh postCreateCommand", + "postStartCommand": [ + "${featureRoot}/helper_script.sh", + "postStartCommand" + ], + "onAttachCommand": [ + "${featureRoot}/helper_script.sh", + "onAttachCommand" + ] } \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/helper_script.sh b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/helper_script.sh old mode 100644 new mode 100755 index 149dbcad2..c68c92032 --- a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/helper_script.sh +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/helper_script.sh @@ -1,3 +1,5 @@ #!/bin/bash -echo "Hello from rabbit helper_script.sh" \ No newline at end of file +MARKER_FILE_NAME="$1" +echo "Hello from rabbit helper_script.sh involved by ${MARKER_FILE_NAME}" +touch "helperScript.rabbit.${MARKER_FILE_NAME}.markerFile" \ No newline at end of file diff --git a/src/test/container-features/lifecycleHooks.test.ts b/src/test/container-features/lifecycleHooks.test.ts index c07addadf..fcb698b17 100644 --- a/src/test/container-features/lifecycleHooks.test.ts +++ b/src/test/container-features/lifecycleHooks.test.ts @@ -97,7 +97,19 @@ describe('Feature lifecycle hooks', function () { // Since lifecycle scripts are executed relative to the workspace folder, // to run a script bundled with the Feature, the user needs to use the '${featureRoot}' variable. - + // This variable can only be used in a devcontainer-feature.json's lifecycle scripts. + // And will return the temporary directory where the Feature's files are copied to. + assert.match(outputOfExecCommand, /helperScript.rabbit.onCreateCommand.markerFile/); + assert.match(outputOfExecCommand, /helperScript.rabbit.updateContent.markerFile/); + assert.match(outputOfExecCommand, /helperScript.rabbit.postCreate.markerFile/); + assert.match(outputOfExecCommand, /helperScript.rabbit.postStart.markerFile/); + assert.match(outputOfExecCommand, /helperScript.rabbit.postAttach.markerFile/); + + assert.match(outputOfExecCommand, /helperScript.otter.onCreateCommand.markerFile/); + assert.match(outputOfExecCommand, /helperScript.otter.updateContent.markerFile/); + assert.match(outputOfExecCommand, /helperScript.otter.postCreate.markerFile/); + assert.match(outputOfExecCommand, /helperScript.otter.postStart.markerFile/); + assert.match(outputOfExecCommand, /helperScript.otter.postAttach.markerFile/); }); }); }); From 9aeade3522cc56899b77807d5bf2c7e0dcfc719b Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Tue, 24 Jan 2023 00:51:18 +0000 Subject: [PATCH 06/41] make Feature artifacts executable by all users --- src/spec-configuration/containerFeaturesConfiguration.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/spec-configuration/containerFeaturesConfiguration.ts b/src/spec-configuration/containerFeaturesConfiguration.ts index 1a639cde5..1660d48ff 100644 --- a/src/spec-configuration/containerFeaturesConfiguration.ts +++ b/src/spec-configuration/containerFeaturesConfiguration.ts @@ -243,7 +243,7 @@ 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 +RUN chmod -R 0777 /tmp/build-features FROM $_DEV_CONTAINERS_BASE_IMAGE AS dev_containers_target_stage From b88f1cd607e9ed1a0bf01230fe11d372f41ee461 Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Tue, 24 Jan 2023 21:02:32 +0000 Subject: [PATCH 07/41] test for advanced lifehook scenario --- .../otter/devcontainer-feature.json | 19 ++++++++------- .../.devcontainer/otter/helper_script.sh | 2 +- .../rabbit/devcontainer-feature.json | 24 +++++++++---------- .../.devcontainer/rabbit/helper_script.sh | 2 +- .../container-features/lifecycleHooks.test.ts | 22 ++++++++--------- 5 files changed, 35 insertions(+), 34 deletions(-) diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json index bd4e74252..42a4f234a 100644 --- a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json @@ -2,15 +2,16 @@ "id": "otter", "version": "1.2.3", "options": {}, - "updateContentCommand": "${featureRoot}/helper_script.sh updateContentCommand", - "onCreateCommand": "${featureRoot}/helper_script.sh onCreateCommand", - "postCreateCommand": "${featureRoot}/helper_script.sh postCreateCommand", + // "updateContentCommand": "${featureRoot}/helper_script.sh updateContentCommand", + // "onCreateCommand": "${featureRoot}/helper_script.sh onCreateCommand", + // "postCreateCommand": "${featureRoot}/helper_script.sh postCreateCommand", "postStartCommand": [ - "${featureRoot}/helper_script.sh", - "postStartCommand" - ], - "onAttachCommand": [ - "${featureRoot}/helper_script.sh", - "onAttachCommand" + "/tmp/build-features/otter_1/helper_script.sh", + "manually-adding-path" ] + //, + // "onAttachCommand": [ + // "${featureRoot}/helper_script.sh", + // "onAttachCommand" + // ] } \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/helper_script.sh b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/helper_script.sh index caafa1e18..f84e79eed 100755 --- a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/helper_script.sh +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/helper_script.sh @@ -2,4 +2,4 @@ MARKER_FILE_NAME="$1" echo "Hello from otter helper_script.sh involved by ${MARKER_FILE_NAME}" -touch "helperScript.otter.${MARKER_FILE_NAME}.markerFile" \ No newline at end of file +touch "helperScript.otter.${MARKER_FILE_NAME}.testMarker" \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/devcontainer-feature.json b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/devcontainer-feature.json index 00b802c78..db8df3872 100644 --- a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/devcontainer-feature.json +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/devcontainer-feature.json @@ -1,16 +1,16 @@ { "id": "rabbit", "version": "100.200.300", - "options": {}, - "updateContentCommand": "${featureRoot}/helper_script.sh updateContentCommand", - "onCreateCommand": "${featureRoot}/helper_script.sh onCreateCommand", - "postCreateCommand": "${featureRoot}/helper_script.sh postCreateCommand", - "postStartCommand": [ - "${featureRoot}/helper_script.sh", - "postStartCommand" - ], - "onAttachCommand": [ - "${featureRoot}/helper_script.sh", - "onAttachCommand" - ] + "options": {} + // "updateContentCommand": "${featureRoot}/helper_script.sh updateContentCommand", + // "onCreateCommand": "${featureRoot}/helper_script.sh onCreateCommand", + // "postCreateCommand": "${featureRoot}/helper_script.sh postCreateCommand", + // "postStartCommand": [ + // "${featureRoot}/helper_script.sh", + // "postStartCommand" + // ], + // "onAttachCommand": [ + // "${featureRoot}/helper_script.sh", + // "onAttachCommand" + // ] } \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/helper_script.sh b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/helper_script.sh index c68c92032..9b2772322 100755 --- a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/helper_script.sh +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/helper_script.sh @@ -2,4 +2,4 @@ MARKER_FILE_NAME="$1" echo "Hello from rabbit helper_script.sh involved by ${MARKER_FILE_NAME}" -touch "helperScript.rabbit.${MARKER_FILE_NAME}.markerFile" \ No newline at end of file +touch "helperScript.rabbit.${MARKER_FILE_NAME}.testMarker" \ No newline at end of file diff --git a/src/test/container-features/lifecycleHooks.test.ts b/src/test/container-features/lifecycleHooks.test.ts index fcb698b17..3aebfbfa2 100644 --- a/src/test/container-features/lifecycleHooks.test.ts +++ b/src/test/container-features/lifecycleHooks.test.ts @@ -99,17 +99,17 @@ describe('Feature lifecycle hooks', function () { // to run a script bundled with the Feature, the user needs to use the '${featureRoot}' variable. // This variable can only be used in a devcontainer-feature.json's lifecycle scripts. // And will return the temporary directory where the Feature's files are copied to. - assert.match(outputOfExecCommand, /helperScript.rabbit.onCreateCommand.markerFile/); - assert.match(outputOfExecCommand, /helperScript.rabbit.updateContent.markerFile/); - assert.match(outputOfExecCommand, /helperScript.rabbit.postCreate.markerFile/); - assert.match(outputOfExecCommand, /helperScript.rabbit.postStart.markerFile/); - assert.match(outputOfExecCommand, /helperScript.rabbit.postAttach.markerFile/); - - assert.match(outputOfExecCommand, /helperScript.otter.onCreateCommand.markerFile/); - assert.match(outputOfExecCommand, /helperScript.otter.updateContent.markerFile/); - assert.match(outputOfExecCommand, /helperScript.otter.postCreate.markerFile/); - assert.match(outputOfExecCommand, /helperScript.otter.postStart.markerFile/); - assert.match(outputOfExecCommand, /helperScript.otter.postAttach.markerFile/); + assert.match(outputOfExecCommand, /helperScript.rabbit.onCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /helperScript.rabbit.updateContent.testMarker/); + assert.match(outputOfExecCommand, /helperScript.rabbit.postCreate.testMarker/); + assert.match(outputOfExecCommand, /helperScript.rabbit.postStart.testMarker/); + assert.match(outputOfExecCommand, /helperScript.rabbit.postAttach.testMarker/); + + assert.match(outputOfExecCommand, /helperScript.otter.onCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /helperScript.otter.updateContent.testMarker/); + assert.match(outputOfExecCommand, /helperScript.otter.postCreate.testMarker/); + assert.match(outputOfExecCommand, /helperScript.otter.postStart.testMarker/); + assert.match(outputOfExecCommand, /helperScript.otter.postAttach.testMarker/); }); }); }); From 909e0feeb8e6ef068ad6c77449e5ff262490bfcb Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Wed, 1 Feb 2023 00:25:55 +0000 Subject: [PATCH 08/41] add --- src/spec-common/variableSubstitution.ts | 18 ++++++++++++++ .../containerFeaturesConfiguration.ts | 17 +++++++------ src/spec-node/imageMetadata.ts | 24 ++++++++++++++++--- .../.devcontainer/devcontainer.json | 6 ----- .../otter/devcontainer-feature.json | 20 ++++++++-------- .../.devcontainer/otter/helper_script.sh | 2 +- .../rabbit/devcontainer-feature.json | 24 +++++++++---------- .../.devcontainer/rabbit/helper_script.sh | 2 +- .../container-features/lifecycleHooks.test.ts | 16 ++++++------- 9 files changed, 81 insertions(+), 48 deletions(-) diff --git a/src/spec-common/variableSubstitution.ts b/src/spec-common/variableSubstitution.ts index 531f47546..0bc296388 100644 --- a/src/spec-common/variableSubstitution.ts +++ b/src/spec-common/variableSubstitution.ts @@ -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': @@ -169,3 +179,11 @@ function devcontainerIdForLabels(idLabels: Record): 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); +} \ No newline at end of file diff --git a/src/spec-configuration/containerFeaturesConfiguration.ts b/src/spec-configuration/containerFeaturesConfiguration.ts index 1660d48ff..e6684aff8 100644 --- a/src/spec-configuration/containerFeaturesConfiguration.ts +++ b/src/spec-configuration/containerFeaturesConfiguration.ts @@ -23,10 +23,18 @@ export const V1_DEVCONTAINER_FEATURES_FILE_NAME = 'devcontainer-features.json'; // v2 export const DEVCONTAINER_FEATURE_FILE_NAME = 'devcontainer-feature.json'; -export type Feature = SchemaFeatureProperties & DeprecatedSchemaFeatureProperties & InternalFeatureProperties; +export type Feature = SchemaFeatureBaseProperties & SchemaFeatureLifecycleHooks & DeprecatedSchemaFeatureProperties & InternalFeatureProperties; + +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 SchemaFeatureProperties { +export interface SchemaFeatureBaseProperties { id: string; version?: string; name?: string; @@ -45,11 +53,6 @@ export interface SchemaFeatureProperties { installsAfter?: string[]; deprecated?: boolean; legacyIds?: string[]; - onCreateCommand?: string | string[]; - updateContentCommand?: string | string[]; - postCreateCommand?: string | string[]; - postStartCommand?: string | string[]; - postAttachCommand?: string | string[]; } // Properties that are set programmatically for book-keeping purposes diff --git a/src/spec-node/imageMetadata.ts b/src/spec-node/imageMetadata.ts index 295d16983..62a35fe18 100644 --- a/src/spec-node/imageMetadata.ts +++ b/src/spec-node/imageMetadata.ts @@ -3,9 +3,11 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import path from 'path'; import { ContainerError } from '../spec-common/errors'; +import { substituteFeatureRoot } from '../spec-common/variableSubstitution'; import { DevContainerConfig, DevContainerConfigCommand, DevContainerFromDockerComposeConfig, DevContainerFromDockerfileConfig, DevContainerFromImageConfig, getDockerComposeFilePaths, getDockerfilePath, HostGPURequirements, HostRequirements, isDockerFileConfig, PortAttributes, UserEnvProbe } from '../spec-configuration/configuration'; -import { Feature, FeaturesConfig, Mount, parseMount } from '../spec-configuration/containerFeaturesConfiguration'; +import { Feature, FeaturesConfig, Mount, parseMount, SchemaFeatureLifecycleHooks } from '../spec-configuration/containerFeaturesConfiguration'; import { ContainerDetails, DockerCLIParameters, ImageDetails } from '../spec-shutdown/dockerUtils'; import { Log } from '../spec-utils/log'; import { getBuildInfoForService, readDockerComposeConfig } from './dockerCompose'; @@ -45,12 +47,16 @@ const pickUpdateableConfigProperties: (keyof DevContainerConfig & keyof ImageMet 'remoteEnv', ]; -const pickFeatureProperties: Exclude[] = [ +const pickFeatureLifecycleHookProperties: Exclude[] = [ 'onCreateCommand', 'updateContentCommand', 'postCreateCommand', 'postStartCommand', 'postAttachCommand', +]; + +const pickFeatureProperties: Exclude[] = [ + ...pickFeatureLifecycleHookProperties, 'init', 'privileged', 'capAdd', @@ -252,7 +258,19 @@ function collectOrUndefined(entries: T[], property: K): No export function getDevcontainerMetadata(baseImageMetadata: SubstitutedConfig, devContainerConfig: SubstitutedConfig, featuresConfig: FeaturesConfig | undefined, omitPropertyOverride: string[] = []): SubstitutedConfig { const effectivePickFeatureProperties = pickFeatureProperties.filter(property => !omitPropertyOverride.includes(property)); - const raw = featuresConfig?.featureSets.map(featureSet => featureSet.features.map(feature => ({ + // Variable substitution + const _ = featuresConfig?.featureSets.forEach(featureSet => + featureSet.features.forEach(f => { + pickFeatureLifecycleHookProperties.forEach(hook => { + const buildPath = `/tmp/build-features/${f.consecutiveId}`; + if (f[hook]) { + f[hook] = substituteFeatureRoot(f[hook], buildPath); + } + }); + })); + + const raw = featuresConfig?.featureSets.map(featureSet => + featureSet.features.map(feature => ({ id: featureSet.sourceInformation.userFeatureId, ...pick(feature, effectivePickFeatureProperties), }))).flat() || []; diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/devcontainer.json b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/devcontainer.json index 27e449316..b40fb9abd 100644 --- a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/devcontainer.json +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/devcontainer.json @@ -6,12 +6,6 @@ "./otter": {}, "./rabbit": {} }, - // "updateContentCommand": "touch devContainer.updateContentCommand.testMarker", - // "onCreateCommand": "touch devContainer.onCreateCommand.testMarker", - // "postCreateCommand": [ - // "touch", - // "devContainer.postCreateCommand.testMarker" - // ], "postStartCommand": "touch `rabbit`.postStartCommand.testMarker", // The 'rabbit' command is installed and added to the path by .devcontainer/rabbit/install.sh "postAttachCommand": "touch `otter`.postAttachCommand.testMarker" // The 'otter' command is installed and added to the path by .devcontainer/otter/install.sh } \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json index 42a4f234a..bed186c0b 100644 --- a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json @@ -2,16 +2,16 @@ "id": "otter", "version": "1.2.3", "options": {}, - // "updateContentCommand": "${featureRoot}/helper_script.sh updateContentCommand", - // "onCreateCommand": "${featureRoot}/helper_script.sh onCreateCommand", - // "postCreateCommand": "${featureRoot}/helper_script.sh postCreateCommand", + "updateContentCommand": "${featureRoot}/helper_script.sh updateContentCommand", + "onCreateCommand": "${featureRoot}/helper_script.sh onCreateCommand", + "postCreateCommand": "${featureRoot}/helper_script.sh postCreateCommand", "postStartCommand": [ - "/tmp/build-features/otter_1/helper_script.sh", - "manually-adding-path" + "${featureRoot}/helper_script.sh", + "postStartCommand" + ] + , + "postAttachCommand": [ + "${featureRoot}/helper_script.sh", + "postAttachCommand" ] - //, - // "onAttachCommand": [ - // "${featureRoot}/helper_script.sh", - // "onAttachCommand" - // ] } \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/helper_script.sh b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/helper_script.sh index f84e79eed..f52d7dcd8 100755 --- a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/helper_script.sh +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/helper_script.sh @@ -1,5 +1,5 @@ #!/bin/bash MARKER_FILE_NAME="$1" -echo "Hello from otter helper_script.sh involved by ${MARKER_FILE_NAME}" +echo "Hello from otter helper_script.sh invoked by ${MARKER_FILE_NAME}" touch "helperScript.otter.${MARKER_FILE_NAME}.testMarker" \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/devcontainer-feature.json b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/devcontainer-feature.json index db8df3872..21651d22f 100644 --- a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/devcontainer-feature.json +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/devcontainer-feature.json @@ -1,16 +1,16 @@ { "id": "rabbit", "version": "100.200.300", - "options": {} - // "updateContentCommand": "${featureRoot}/helper_script.sh updateContentCommand", - // "onCreateCommand": "${featureRoot}/helper_script.sh onCreateCommand", - // "postCreateCommand": "${featureRoot}/helper_script.sh postCreateCommand", - // "postStartCommand": [ - // "${featureRoot}/helper_script.sh", - // "postStartCommand" - // ], - // "onAttachCommand": [ - // "${featureRoot}/helper_script.sh", - // "onAttachCommand" - // ] + "options": {}, + "updateContentCommand": "${featureRoot}/helper_script.sh updateContentCommand", + "onCreateCommand": "${featureRoot}/helper_script.sh onCreateCommand", + "postCreateCommand": "${featureRoot}/helper_script.sh postCreateCommand", + "postStartCommand": [ + "${featureRoot}/helper_script.sh", + "postStartCommand" + ], + "postAttachCommand": [ + "${featureRoot}/helper_script.sh", + "postAttachCommand" + ] } \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/helper_script.sh b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/helper_script.sh index 9b2772322..133963fc3 100755 --- a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/helper_script.sh +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/helper_script.sh @@ -1,5 +1,5 @@ #!/bin/bash MARKER_FILE_NAME="$1" -echo "Hello from rabbit helper_script.sh involved by ${MARKER_FILE_NAME}" +echo "Hello from rabbit helper_script.sh invoked by ${MARKER_FILE_NAME}" touch "helperScript.rabbit.${MARKER_FILE_NAME}.testMarker" \ No newline at end of file diff --git a/src/test/container-features/lifecycleHooks.test.ts b/src/test/container-features/lifecycleHooks.test.ts index 3aebfbfa2..b7443bdff 100644 --- a/src/test/container-features/lifecycleHooks.test.ts +++ b/src/test/container-features/lifecycleHooks.test.ts @@ -100,16 +100,16 @@ describe('Feature lifecycle hooks', function () { // This variable can only be used in a devcontainer-feature.json's lifecycle scripts. // And will return the temporary directory where the Feature's files are copied to. assert.match(outputOfExecCommand, /helperScript.rabbit.onCreateCommand.testMarker/); - assert.match(outputOfExecCommand, /helperScript.rabbit.updateContent.testMarker/); - assert.match(outputOfExecCommand, /helperScript.rabbit.postCreate.testMarker/); - assert.match(outputOfExecCommand, /helperScript.rabbit.postStart.testMarker/); - assert.match(outputOfExecCommand, /helperScript.rabbit.postAttach.testMarker/); + assert.match(outputOfExecCommand, /helperScript.rabbit.updateContentCommand.testMarker/); + assert.match(outputOfExecCommand, /helperScript.rabbit.postCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /helperScript.rabbit.postStartCommand.testMarker/); + assert.match(outputOfExecCommand, /helperScript.rabbit.postAttachCommand.testMarker/); assert.match(outputOfExecCommand, /helperScript.otter.onCreateCommand.testMarker/); - assert.match(outputOfExecCommand, /helperScript.otter.updateContent.testMarker/); - assert.match(outputOfExecCommand, /helperScript.otter.postCreate.testMarker/); - assert.match(outputOfExecCommand, /helperScript.otter.postStart.testMarker/); - assert.match(outputOfExecCommand, /helperScript.otter.postAttach.testMarker/); + assert.match(outputOfExecCommand, /helperScript.otter.updateContentCommand.testMarker/); + assert.match(outputOfExecCommand, /helperScript.otter.postCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /helperScript.otter.postStartCommand.testMarker/); + assert.match(outputOfExecCommand, /helperScript.otter.postAttachCommand.testMarker/); }); }); }); From 93ed16b2dbf8ce0b51a04e472015e5b15a503159 Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Wed, 1 Feb 2023 01:59:32 +0000 Subject: [PATCH 09/41] typecheck --- src/spec-node/imageMetadata.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/spec-node/imageMetadata.ts b/src/spec-node/imageMetadata.ts index 62a35fe18..bb7c00eef 100644 --- a/src/spec-node/imageMetadata.ts +++ b/src/spec-node/imageMetadata.ts @@ -3,7 +3,6 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import path from 'path'; import { ContainerError } from '../spec-common/errors'; import { substituteFeatureRoot } from '../spec-common/variableSubstitution'; import { DevContainerConfig, DevContainerConfigCommand, DevContainerFromDockerComposeConfig, DevContainerFromDockerfileConfig, DevContainerFromImageConfig, getDockerComposeFilePaths, getDockerfilePath, HostGPURequirements, HostRequirements, isDockerFileConfig, PortAttributes, UserEnvProbe } from '../spec-configuration/configuration'; @@ -259,7 +258,8 @@ export function getDevcontainerMetadata(baseImageMetadata: SubstitutedConfig !omitPropertyOverride.includes(property)); // Variable substitution - const _ = featuresConfig?.featureSets.forEach(featureSet => + // eslint-disable-next-line code-no-unused-expressions + featuresConfig?.featureSets.forEach(featureSet => featureSet.features.forEach(f => { pickFeatureLifecycleHookProperties.forEach(hook => { const buildPath = `/tmp/build-features/${f.consecutiveId}`; From 780c3469116ffb34a8e0403b232e2d6a8f3322a2 Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Wed, 1 Feb 2023 02:27:08 +0000 Subject: [PATCH 10/41] improve test --- .../.devcontainer/devcontainer.json | 4 ++++ .../.devcontainer/helper_script.sh | 5 +++++ .../.devcontainer/otter/devcontainer-feature.json | 5 ++++- .../.devcontainer/rabbit/devcontainer-feature.json | 5 ++++- .../.devcontainer/createMarker.sh | 4 ++-- src/test/container-features/lifecycleHooks.test.ts | 14 ++++++++++++-- 6 files changed, 31 insertions(+), 6 deletions(-) create mode 100755 src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/helper_script.sh diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/devcontainer.json b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/devcontainer.json index b40fb9abd..2eb96b21a 100644 --- a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/devcontainer.json +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/devcontainer.json @@ -6,6 +6,10 @@ "./otter": {}, "./rabbit": {} }, + "postCreateCommand": { + "parallel1": ".devcontainer/helper_script.sh parallel_postCreateCommand_1", + "parallel2": ".devcontainer/helper_script.sh parallel_postCreateCommand_2" + }, "postStartCommand": "touch `rabbit`.postStartCommand.testMarker", // The 'rabbit' command is installed and added to the path by .devcontainer/rabbit/install.sh "postAttachCommand": "touch `otter`.postAttachCommand.testMarker" // The 'otter' command is installed and added to the path by .devcontainer/otter/install.sh } \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/helper_script.sh b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/helper_script.sh new file mode 100755 index 000000000..1c8c5c658 --- /dev/null +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/helper_script.sh @@ -0,0 +1,5 @@ +#!/bin/bash + +MARKER_FILE_NAME="$1" +echo "Hello from the .devcontainer helper_script.sh invoked by ${MARKER_FILE_NAME}" +touch "helperScript.devContainer.${MARKER_FILE_NAME}.testMarker" \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json index bed186c0b..cff7eaa2f 100644 --- a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json @@ -4,7 +4,10 @@ "options": {}, "updateContentCommand": "${featureRoot}/helper_script.sh updateContentCommand", "onCreateCommand": "${featureRoot}/helper_script.sh onCreateCommand", - "postCreateCommand": "${featureRoot}/helper_script.sh postCreateCommand", + "postCreateCommand": { + "parallel1": "${featureRoot}/helper_script.sh parallel_postCreateCommand_1", + "parallel2": "${featureRoot}/helper_script.sh parallel_postCreateCommand_2" + }, "postStartCommand": [ "${featureRoot}/helper_script.sh", "postStartCommand" diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/devcontainer-feature.json b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/devcontainer-feature.json index 21651d22f..795d29fd8 100644 --- a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/devcontainer-feature.json +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/devcontainer-feature.json @@ -4,7 +4,10 @@ "options": {}, "updateContentCommand": "${featureRoot}/helper_script.sh updateContentCommand", "onCreateCommand": "${featureRoot}/helper_script.sh onCreateCommand", - "postCreateCommand": "${featureRoot}/helper_script.sh postCreateCommand", + "postCreateCommand": { + "parallel1": "${featureRoot}/helper_script.sh parallel_postCreateCommand_1", + "parallel2": "${featureRoot}/helper_script.sh parallel_postCreateCommand_2" + }, "postStartCommand": [ "${featureRoot}/helper_script.sh", "postStartCommand" diff --git a/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/createMarker.sh b/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/createMarker.sh index de9ef399a..d9b11d784 100755 --- a/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/createMarker.sh +++ b/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/createMarker.sh @@ -3,7 +3,7 @@ MARKER_FILE_NAME="$1" echo "Starting '${MARKER_FILE_NAME}'...." -sleep 2s +sleep 1s [[ -f saved_value.testMarker ]] || echo 0 > saved_value.testMarker n=$(< saved_value.testMarker) @@ -11,4 +11,4 @@ cat saved_value.testMarker > "${n}.${MARKER_FILE_NAME}" echo $(( n + 1 )) > saved_value.testMarker echo "Ending '${MARKER_FILE_NAME}'...." -sleep 2s \ No newline at end of file +sleep 1s \ No newline at end of file diff --git a/src/test/container-features/lifecycleHooks.test.ts b/src/test/container-features/lifecycleHooks.test.ts index b7443bdff..60b76c0db 100644 --- a/src/test/container-features/lifecycleHooks.test.ts +++ b/src/test/container-features/lifecycleHooks.test.ts @@ -95,21 +95,31 @@ describe('Feature lifecycle hooks', function () { assert.match(outputOfExecCommand, /i-am-a-rabbit.postStartCommand.testMarker/); assert.match(outputOfExecCommand, /i-am-an-otter.postAttachCommand.testMarker/); + assert.match(outputOfExecCommand, /helperScript.devContainer.parallel_postCreateCommand_1.testMarker/); + assert.match(outputOfExecCommand, /helperScript.devContainer.parallel_postCreateCommand_2.testMarker/); + // Since lifecycle scripts are executed relative to the workspace folder, // to run a script bundled with the Feature, the user needs to use the '${featureRoot}' variable. // This variable can only be used in a devcontainer-feature.json's lifecycle scripts. // And will return the temporary directory where the Feature's files are copied to. + + // -- 'Rabbit' Feature assert.match(outputOfExecCommand, /helperScript.rabbit.onCreateCommand.testMarker/); assert.match(outputOfExecCommand, /helperScript.rabbit.updateContentCommand.testMarker/); - assert.match(outputOfExecCommand, /helperScript.rabbit.postCreateCommand.testMarker/); assert.match(outputOfExecCommand, /helperScript.rabbit.postStartCommand.testMarker/); assert.match(outputOfExecCommand, /helperScript.rabbit.postAttachCommand.testMarker/); + assert.match(outputOfExecCommand, /helperScript.rabbit.parallel_postCreateCommand_1.testMarker/); + assert.match(outputOfExecCommand, /helperScript.rabbit.parallel_postCreateCommand_2.testMarker/); + + // -- 'Otter' Feature assert.match(outputOfExecCommand, /helperScript.otter.onCreateCommand.testMarker/); assert.match(outputOfExecCommand, /helperScript.otter.updateContentCommand.testMarker/); - assert.match(outputOfExecCommand, /helperScript.otter.postCreateCommand.testMarker/); assert.match(outputOfExecCommand, /helperScript.otter.postStartCommand.testMarker/); assert.match(outputOfExecCommand, /helperScript.otter.postAttachCommand.testMarker/); + + assert.match(outputOfExecCommand, /helperScript.otter.parallel_postCreateCommand_1.testMarker/); + assert.match(outputOfExecCommand, /helperScript.otter.parallel_postCreateCommand_2.testMarker/); }); }); }); From d0323e39add65d76da0c34f42c1ab262c5e91494 Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Wed, 1 Feb 2023 21:38:08 +0000 Subject: [PATCH 11/41] refactor variable names from 'postCreate' -> 'lifecycleHook' --- src/spec-common/injectHeadless.ts | 62 +++++++++++++-------------- src/spec-node/configContainer.ts | 2 +- src/spec-node/devContainers.ts | 4 +- src/spec-node/devContainersSpecCLI.ts | 4 +- 4 files changed, 36 insertions(+), 36 deletions(-) diff --git a/src/spec-common/injectHeadless.ts b/src/spec-common/injectHeadless.ts index 3924a3067..15608da01 100644 --- a/src/spec-common/injectHeadless.ts +++ b/src/spec-common/injectHeadless.ts @@ -50,7 +50,7 @@ export interface ResolverParameters { output: Log; allowSystemConfigChange: boolean; defaultUserEnvProbe: UserEnvProbe; - postCreate: PostCreate; + lifecycleHook: LifecycleHook; getLogLevel: () => LogLevel; onDidChangeLogLevel: Event; loadNativeModule: (moduleName: string) => Promise; @@ -68,7 +68,7 @@ export interface ResolverParameters { skipPersistingCustomizationsFromFeatures: boolean; } -export interface PostCreate { +export interface LifecycleHook { enabled: boolean; skipNonBlocking: boolean; output: Log; @@ -76,7 +76,7 @@ export interface PostCreate { done: () => void; } -export function createNullPostCreate(enabled: boolean, skipNonBlocking: boolean, output: Log): PostCreate { +export function createNullLifecycleHook(enabled: boolean, skipNonBlocking: boolean, output: Log): LifecycleHook { function listener(data: Buffer) { emitter.fire(data.toString()); } @@ -310,11 +310,11 @@ export function getSystemVarFolder(params: ResolverParameters): string { export async function setupInContainer(params: ResolverParameters, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig) { await patchEtcEnvironment(params, containerProperties); await patchEtcProfile(params, containerProperties); - const computeRemoteEnv = params.computeExtensionHostEnv || params.postCreate.enabled; + const computeRemoteEnv = params.computeExtensionHostEnv || params.lifecycleHook.enabled; const updatedConfig = containerSubstitute(params.cliHost.platform, config.configFilePath, containerProperties.env, config); const remoteEnv = computeRemoteEnv ? probeRemoteEnv(params, containerProperties, updatedConfig) : Promise.resolve({}); - if (params.postCreate.enabled) { - await runPostCreateCommands(params, containerProperties, updatedConfig, remoteEnv, false); + if (params.lifecycleHook.enabled) { + await runLifecycleHooks(params, containerProperties, updatedConfig, remoteEnv, false); } return { remoteEnv: params.computeExtensionHostEnv ? await remoteEnv : {}, @@ -330,8 +330,8 @@ export function probeRemoteEnv(params: ResolverParameters, containerProperties: } as Record)); } -export async function runPostCreateCommands(params: ResolverParameters, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, remoteEnv: Promise>, stopForPersonalization: boolean): Promise<'skipNonBlocking' | 'prebuild' | 'stopForPersonalization' | 'done'> { - const skipNonBlocking = params.postCreate.skipNonBlocking; +export async function runLifecycleHooks(params: ResolverParameters, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, remoteEnv: Promise>, stopForPersonalization: boolean): Promise<'skipNonBlocking' | 'prebuild' | 'stopForPersonalization' | 'done'> { + const skipNonBlocking = params.lifecycleHook.skipNonBlocking; const waitFor = config.waitFor || defaultWaitFor; if (skipNonBlocking && waitFor === 'initializeCommand') { return 'skipNonBlocking'; @@ -394,13 +394,13 @@ export async function getOSRelease(shellServer: ShellServer) { async function runPostCreateCommand(params: ResolverParameters, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, postCommandName: 'onCreateCommand' | 'updateContentCommand' | 'postCreateCommand', remoteEnv: Promise>, rerun: boolean) { const markerFile = path.posix.join(containerProperties.userDataFolder, `.${postCommandName}Marker`); const doRun = !!containerProperties.createdAt && await updateMarkerFile(containerProperties.shellServer, markerFile, containerProperties.createdAt) || rerun; - await runPostCommands(params, containerProperties, config, postCommandName, remoteEnv, doRun); + await runLifecycleCommands(params, containerProperties, config, postCommandName, remoteEnv, doRun); } async function runPostStartCommand(params: ResolverParameters, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, remoteEnv: Promise>) { const markerFile = path.posix.join(containerProperties.userDataFolder, '.postStartCommandMarker'); const doRun = !!containerProperties.startedAt && await updateMarkerFile(containerProperties.shellServer, markerFile, containerProperties.startedAt); - await runPostCommands(params, containerProperties, config, 'postStartCommand', remoteEnv, doRun); + await runLifecycleCommands(params, containerProperties, config, 'postStartCommand', remoteEnv, doRun); } async function updateMarkerFile(shellServer: ShellServer, location: string, content: string) { @@ -413,24 +413,24 @@ async function updateMarkerFile(shellServer: ShellServer, location: string, cont } async function runPostAttachCommand(params: ResolverParameters, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, remoteEnv: Promise>) { - await runPostCommands(params, containerProperties, config, 'postAttachCommand', remoteEnv, true); + await runLifecycleCommands(params, containerProperties, config, 'postAttachCommand', remoteEnv, true); } -async function runPostCommands(params: ResolverParameters, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, postCommandName: 'onCreateCommand' | 'updateContentCommand' | 'postCreateCommand' | 'postStartCommand' | 'postAttachCommand', remoteEnv: Promise>, doRun: boolean) { - return Promise.all((config[`${postCommandName}s`] || []).map(config => runPostCommand(params, containerProperties, config, postCommandName, remoteEnv, doRun))); +async function runLifecycleCommands(params: ResolverParameters, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, lifecycleHookName: 'onCreateCommand' | 'updateContentCommand' | 'postCreateCommand' | 'postStartCommand' | 'postAttachCommand', remoteEnv: Promise>, doRun: boolean) { + return Promise.all((config[`${lifecycleHookName}s`] || []).map(config => runLifecycleCommand(params, containerProperties, config, lifecycleHookName, remoteEnv, doRun))); } -async function runPostCommand({ postCreate }: ResolverParameters, containerProperties: ContainerProperties, postCommand: string | string[], postCommandName: 'onCreateCommand' | 'updateContentCommand' | 'postCreateCommand' | 'postStartCommand' | 'postAttachCommand', remoteEnv: Promise>, doRun: boolean) { +async function runLifecycleCommand({ lifecycleHook: postCreate }: ResolverParameters, containerProperties: ContainerProperties, userCommand: string | string[], lifecycleHookName: 'onCreateCommand' | 'updateContentCommand' | 'postCreateCommand' | 'postStartCommand' | 'postAttachCommand', remoteEnv: Promise>, doRun: boolean) { let hasCommand = false; - if (typeof postCommand === 'string') { - hasCommand = postCommand.trim().length > 0; - } else if (Array.isArray(postCommand)) { - hasCommand = postCommand.length > 0; - } else if (typeof postCommand === 'object') { - hasCommand = Object.keys(postCommand).length > 0; - } - if (doRun && postCommand && hasCommand) { - const progressName = `Running ${postCommandName}...`; + if (typeof userCommand === 'string') { + hasCommand = userCommand.trim().length > 0; + } else if (Array.isArray(userCommand)) { + hasCommand = userCommand.length > 0; + } else if (typeof userCommand === 'object') { + hasCommand = Object.keys(userCommand).length > 0; + } + if (doRun && userCommand && hasCommand) { + const progressName = `Running ${lifecycleHookName}...`; const infoOutput = makeLog({ event(e: LogEvent) { postCreate.output.event(e); @@ -484,14 +484,14 @@ async function runPostCommand({ postCreate }: ResolverParameters, containerPrope }); } - infoOutput.raw(`\x1b[1mRunning the ${postCommandName} from devcontainer.json...\x1b[0m\r\n\r\n`); + infoOutput.raw(`\x1b[1mRunning the ${lifecycleHookName} from devcontainer.json...\x1b[0m\r\n\r\n`); let commands; - if (typeof postCommand === 'string' || Array.isArray(postCommand)) { - commands = [runSingleCommand(postCommand)]; + if (typeof userCommand === 'string' || Array.isArray(userCommand)) { + commands = [runSingleCommand(userCommand)]; } else { - commands = Object.keys(postCommand).map(name => { - const command = postCommand[name]; + commands = Object.keys(userCommand).map(name => { + const command = userCommand[name]; return runSingleCommand(command, name); }); } @@ -503,13 +503,13 @@ async function runPostCommand({ postCreate }: ResolverParameters, containerPrope status: 'failed', }); if (err && (err.code === 130 || err.signal === 2)) { // SIGINT seen on darwin as code === 130, would also make sense as signal === 2. - infoOutput.raw(`\r\n\x1b[1m${postCommandName} interrupted.\x1b[0m\r\n\r\n`); + infoOutput.raw(`\r\n\x1b[1m${lifecycleHookName} interrupted.\x1b[0m\r\n\r\n`); } else { if (err?.code) { - infoOutput.write(toErrorText(`${postCommandName} failed with exit code ${err.code}. Skipping any further user-provided commands.`)); + infoOutput.write(toErrorText(`${lifecycleHookName} failed with exit code ${err.code}. Skipping any further user-provided commands.`)); } throw new ContainerError({ - description: `The ${postCommandName} in the devcontainer.json failed.`, + description: `The ${lifecycleHookName} in the devcontainer.json failed.`, originalError: err, }); } diff --git a/src/spec-node/configContainer.ts b/src/spec-node/configContainer.ts index facdafbb6..b721ec161 100644 --- a/src/spec-node/configContainer.ts +++ b/src/spec-node/configContainer.ts @@ -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) { diff --git a/src/spec-node/devContainers.ts b/src/spec-node/devContainers.ts index 1891f16db..40c425efc 100644 --- a/src/spec-node/devContainers.ts +++ b/src/spec-node/devContainers.ts @@ -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'; @@ -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, diff --git a/src/spec-node/devContainersSpecCLI.ts b/src/spec-node/devContainersSpecCLI.ts index c16caba5a..8f9b42cf2 100644 --- a/src/spec-node/devContainersSpecCLI.ts +++ b/src/spec-node/devContainersSpecCLI.ts @@ -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'; @@ -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, containerProperties, updatedConfig, remoteEnv, stopForPersonalization); return { outcome: 'success' as 'success', result, From cd591d33d357aced658c26b3718c63de1b386b0a Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Wed, 1 Feb 2023 21:38:08 +0000 Subject: [PATCH 12/41] refactor variable names from 'postCreate' -> 'lifecycleHook' --- src/spec-common/injectHeadless.ts | 70 +++++++++++++-------------- src/spec-node/configContainer.ts | 2 +- src/spec-node/devContainers.ts | 4 +- src/spec-node/devContainersSpecCLI.ts | 4 +- 4 files changed, 40 insertions(+), 40 deletions(-) diff --git a/src/spec-common/injectHeadless.ts b/src/spec-common/injectHeadless.ts index 3924a3067..74498c30e 100644 --- a/src/spec-common/injectHeadless.ts +++ b/src/spec-common/injectHeadless.ts @@ -50,7 +50,7 @@ export interface ResolverParameters { output: Log; allowSystemConfigChange: boolean; defaultUserEnvProbe: UserEnvProbe; - postCreate: PostCreate; + lifecycleHook: LifecycleHook; getLogLevel: () => LogLevel; onDidChangeLogLevel: Event; loadNativeModule: (moduleName: string) => Promise; @@ -68,7 +68,7 @@ export interface ResolverParameters { skipPersistingCustomizationsFromFeatures: boolean; } -export interface PostCreate { +export interface LifecycleHook { enabled: boolean; skipNonBlocking: boolean; output: Log; @@ -76,7 +76,7 @@ export interface PostCreate { done: () => void; } -export function createNullPostCreate(enabled: boolean, skipNonBlocking: boolean, output: Log): PostCreate { +export function createNullLifecycleHook(enabled: boolean, skipNonBlocking: boolean, output: Log): LifecycleHook { function listener(data: Buffer) { emitter.fire(data.toString()); } @@ -111,9 +111,9 @@ export interface PortAttributes { export type UserEnvProbe = 'none' | 'loginInteractiveShell' | 'interactiveShell' | 'loginShell'; -export type DevContainerConfigCommand = 'initializeCommand' | 'onCreateCommand' | 'updateContentCommand' | 'postCreateCommand' | 'postStartCommand' | 'postAttachCommand'; +export type DevContainerLifecycleHook = 'initializeCommand' | 'onCreateCommand' | 'updateContentCommand' | 'postCreateCommand' | 'postStartCommand' | 'postAttachCommand'; -const defaultWaitFor: DevContainerConfigCommand = 'updateContentCommand'; +const defaultWaitFor: DevContainerLifecycleHook = 'updateContentCommand'; type LifecycleCommand = string | string[]; @@ -129,7 +129,7 @@ export interface CommonDevContainerConfig { postCreateCommand?: LifecycleCommand | Record; postStartCommand?: LifecycleCommand | Record; postAttachCommand?: LifecycleCommand | Record; - waitFor?: DevContainerConfigCommand; + waitFor?: DevContainerLifecycleHook; userEnvProbe?: UserEnvProbe; } @@ -139,7 +139,7 @@ export interface CommonContainerMetadata { postCreateCommand?: string | string[]; postStartCommand?: string | string[]; postAttachCommand?: string | string[]; - waitFor?: DevContainerConfigCommand; + waitFor?: DevContainerLifecycleHook; remoteEnv?: Record; userEnvProbe?: UserEnvProbe; } @@ -310,11 +310,11 @@ export function getSystemVarFolder(params: ResolverParameters): string { export async function setupInContainer(params: ResolverParameters, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig) { await patchEtcEnvironment(params, containerProperties); await patchEtcProfile(params, containerProperties); - const computeRemoteEnv = params.computeExtensionHostEnv || params.postCreate.enabled; + const computeRemoteEnv = params.computeExtensionHostEnv || params.lifecycleHook.enabled; const updatedConfig = containerSubstitute(params.cliHost.platform, config.configFilePath, containerProperties.env, config); const remoteEnv = computeRemoteEnv ? probeRemoteEnv(params, containerProperties, updatedConfig) : Promise.resolve({}); - if (params.postCreate.enabled) { - await runPostCreateCommands(params, containerProperties, updatedConfig, remoteEnv, false); + if (params.lifecycleHook.enabled) { + await runLifecycleHooks(params, containerProperties, updatedConfig, remoteEnv, false); } return { remoteEnv: params.computeExtensionHostEnv ? await remoteEnv : {}, @@ -330,8 +330,8 @@ export function probeRemoteEnv(params: ResolverParameters, containerProperties: } as Record)); } -export async function runPostCreateCommands(params: ResolverParameters, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, remoteEnv: Promise>, stopForPersonalization: boolean): Promise<'skipNonBlocking' | 'prebuild' | 'stopForPersonalization' | 'done'> { - const skipNonBlocking = params.postCreate.skipNonBlocking; +export async function runLifecycleHooks(params: ResolverParameters, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, remoteEnv: Promise>, stopForPersonalization: boolean): Promise<'skipNonBlocking' | 'prebuild' | 'stopForPersonalization' | 'done'> { + const skipNonBlocking = params.lifecycleHook.skipNonBlocking; const waitFor = config.waitFor || defaultWaitFor; if (skipNonBlocking && waitFor === 'initializeCommand') { return 'skipNonBlocking'; @@ -394,13 +394,13 @@ export async function getOSRelease(shellServer: ShellServer) { async function runPostCreateCommand(params: ResolverParameters, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, postCommandName: 'onCreateCommand' | 'updateContentCommand' | 'postCreateCommand', remoteEnv: Promise>, rerun: boolean) { const markerFile = path.posix.join(containerProperties.userDataFolder, `.${postCommandName}Marker`); const doRun = !!containerProperties.createdAt && await updateMarkerFile(containerProperties.shellServer, markerFile, containerProperties.createdAt) || rerun; - await runPostCommands(params, containerProperties, config, postCommandName, remoteEnv, doRun); + await runLifecycleCommands(params, containerProperties, config, postCommandName, remoteEnv, doRun); } async function runPostStartCommand(params: ResolverParameters, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, remoteEnv: Promise>) { const markerFile = path.posix.join(containerProperties.userDataFolder, '.postStartCommandMarker'); const doRun = !!containerProperties.startedAt && await updateMarkerFile(containerProperties.shellServer, markerFile, containerProperties.startedAt); - await runPostCommands(params, containerProperties, config, 'postStartCommand', remoteEnv, doRun); + await runLifecycleCommands(params, containerProperties, config, 'postStartCommand', remoteEnv, doRun); } async function updateMarkerFile(shellServer: ShellServer, location: string, content: string) { @@ -413,24 +413,24 @@ async function updateMarkerFile(shellServer: ShellServer, location: string, cont } async function runPostAttachCommand(params: ResolverParameters, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, remoteEnv: Promise>) { - await runPostCommands(params, containerProperties, config, 'postAttachCommand', remoteEnv, true); + await runLifecycleCommands(params, containerProperties, config, 'postAttachCommand', remoteEnv, true); } -async function runPostCommands(params: ResolverParameters, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, postCommandName: 'onCreateCommand' | 'updateContentCommand' | 'postCreateCommand' | 'postStartCommand' | 'postAttachCommand', remoteEnv: Promise>, doRun: boolean) { - return Promise.all((config[`${postCommandName}s`] || []).map(config => runPostCommand(params, containerProperties, config, postCommandName, remoteEnv, doRun))); +async function runLifecycleCommands(params: ResolverParameters, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, lifecycleHookName: 'onCreateCommand' | 'updateContentCommand' | 'postCreateCommand' | 'postStartCommand' | 'postAttachCommand', remoteEnv: Promise>, doRun: boolean) { + return Promise.all((config[`${lifecycleHookName}s`] || []).map(config => runLifecycleCommand(params, containerProperties, config, lifecycleHookName, remoteEnv, doRun))); } -async function runPostCommand({ postCreate }: ResolverParameters, containerProperties: ContainerProperties, postCommand: string | string[], postCommandName: 'onCreateCommand' | 'updateContentCommand' | 'postCreateCommand' | 'postStartCommand' | 'postAttachCommand', remoteEnv: Promise>, doRun: boolean) { +async function runLifecycleCommand({ lifecycleHook: postCreate }: ResolverParameters, containerProperties: ContainerProperties, userCommand: string | string[], lifecycleHookName: 'onCreateCommand' | 'updateContentCommand' | 'postCreateCommand' | 'postStartCommand' | 'postAttachCommand', remoteEnv: Promise>, doRun: boolean) { let hasCommand = false; - if (typeof postCommand === 'string') { - hasCommand = postCommand.trim().length > 0; - } else if (Array.isArray(postCommand)) { - hasCommand = postCommand.length > 0; - } else if (typeof postCommand === 'object') { - hasCommand = Object.keys(postCommand).length > 0; - } - if (doRun && postCommand && hasCommand) { - const progressName = `Running ${postCommandName}...`; + if (typeof userCommand === 'string') { + hasCommand = userCommand.trim().length > 0; + } else if (Array.isArray(userCommand)) { + hasCommand = userCommand.length > 0; + } else if (typeof userCommand === 'object') { + hasCommand = Object.keys(userCommand).length > 0; + } + if (doRun && userCommand && hasCommand) { + const progressName = `Running ${lifecycleHookName}...`; const infoOutput = makeLog({ event(e: LogEvent) { postCreate.output.event(e); @@ -484,14 +484,14 @@ async function runPostCommand({ postCreate }: ResolverParameters, containerPrope }); } - infoOutput.raw(`\x1b[1mRunning the ${postCommandName} from devcontainer.json...\x1b[0m\r\n\r\n`); + infoOutput.raw(`\x1b[1mRunning the ${lifecycleHookName} from devcontainer.json...\x1b[0m\r\n\r\n`); let commands; - if (typeof postCommand === 'string' || Array.isArray(postCommand)) { - commands = [runSingleCommand(postCommand)]; + if (typeof userCommand === 'string' || Array.isArray(userCommand)) { + commands = [runSingleCommand(userCommand)]; } else { - commands = Object.keys(postCommand).map(name => { - const command = postCommand[name]; + commands = Object.keys(userCommand).map(name => { + const command = userCommand[name]; return runSingleCommand(command, name); }); } @@ -503,13 +503,13 @@ async function runPostCommand({ postCreate }: ResolverParameters, containerPrope status: 'failed', }); if (err && (err.code === 130 || err.signal === 2)) { // SIGINT seen on darwin as code === 130, would also make sense as signal === 2. - infoOutput.raw(`\r\n\x1b[1m${postCommandName} interrupted.\x1b[0m\r\n\r\n`); + infoOutput.raw(`\r\n\x1b[1m${lifecycleHookName} interrupted.\x1b[0m\r\n\r\n`); } else { if (err?.code) { - infoOutput.write(toErrorText(`${postCommandName} failed with exit code ${err.code}. Skipping any further user-provided commands.`)); + infoOutput.write(toErrorText(`${lifecycleHookName} failed with exit code ${err.code}. Skipping any further user-provided commands.`)); } throw new ContainerError({ - description: `The ${postCommandName} in the devcontainer.json failed.`, + description: `The ${lifecycleHookName} in the devcontainer.json failed.`, originalError: err, }); } diff --git a/src/spec-node/configContainer.ts b/src/spec-node/configContainer.ts index facdafbb6..b721ec161 100644 --- a/src/spec-node/configContainer.ts +++ b/src/spec-node/configContainer.ts @@ -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) { diff --git a/src/spec-node/devContainers.ts b/src/spec-node/devContainers.ts index 1891f16db..40c425efc 100644 --- a/src/spec-node/devContainers.ts +++ b/src/spec-node/devContainers.ts @@ -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'; @@ -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, diff --git a/src/spec-node/devContainersSpecCLI.ts b/src/spec-node/devContainersSpecCLI.ts index c16caba5a..8f9b42cf2 100644 --- a/src/spec-node/devContainersSpecCLI.ts +++ b/src/spec-node/devContainersSpecCLI.ts @@ -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'; @@ -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, containerProperties, updatedConfig, remoteEnv, stopForPersonalization); return { outcome: 'success' as 'success', result, From 1502b4736bb313e406c2a17de77a2d81beb35002 Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Wed, 1 Feb 2023 22:42:21 +0000 Subject: [PATCH 13/41] update test for running in parallel --- .../.devcontainer/createMarker.sh | 2 +- .../.devcontainer/devcontainer.json | 8 ++-- .../container-features/lifecycleHooks.test.ts | 39 ++++++++----------- 3 files changed, 22 insertions(+), 27 deletions(-) diff --git a/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/createMarker.sh b/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/createMarker.sh index d9b11d784..af33cd222 100755 --- a/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/createMarker.sh +++ b/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/createMarker.sh @@ -7,7 +7,7 @@ sleep 1s [[ -f saved_value.testMarker ]] || echo 0 > saved_value.testMarker n=$(< saved_value.testMarker) -cat saved_value.testMarker > "${n}.${MARKER_FILE_NAME}" +echo "${n}.`date +%s%3N`" > "${MARKER_FILE_NAME}" echo $(( n + 1 )) > saved_value.testMarker echo "Ending '${MARKER_FILE_NAME}'...." diff --git a/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/devcontainer.json b/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/devcontainer.json index f44b23fd1..e946a2563 100644 --- a/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/devcontainer.json +++ b/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/devcontainer.json @@ -6,12 +6,12 @@ "./tiger": {}, "./panda": {} }, - "onCreateCommand": "./.devcontainer/createMarker.sh devContainer.onCreateCommand.testMarker", - "updateContentCommand": "./.devcontainer/createMarker.sh devContainer.updateContentCommand.testMarker", + "onCreateCommand": ".devcontainer/createMarker.sh devContainer.onCreateCommand.testMarker", + "updateContentCommand": ".devcontainer/createMarker.sh devContainer.updateContentCommand.testMarker", "postCreateCommand": [ ".devcontainer/createMarker.sh", "devContainer.postCreateCommand.testMarker" ], - "postStartCommand": "./.devcontainer/createMarker.sh devContainer.postStartCommand.testMarker", - "postAttachCommand": "./.devcontainer/createMarker.sh devContainer.postAttachCommand.testMarker" + "postStartCommand": ".devcontainer/createMarker.sh devContainer.postStartCommand.testMarker", + "postAttachCommand": ".devcontainer/createMarker.sh devContainer.postAttachCommand.testMarker" } \ No newline at end of file diff --git a/src/test/container-features/lifecycleHooks.test.ts b/src/test/container-features/lifecycleHooks.test.ts index 60b76c0db..4f9abafcd 100644 --- a/src/test/container-features/lifecycleHooks.test.ts +++ b/src/test/container-features/lifecycleHooks.test.ts @@ -45,23 +45,23 @@ describe('Feature lifecycle hooks', function () { const outputOfExecCommand = res.stderr; console.log(outputOfExecCommand); - assert.match(outputOfExecCommand, /0.panda.onCreateCommand.testMarker/); - assert.match(outputOfExecCommand, /3.panda.updateContentCommand.testMarker/); - assert.match(outputOfExecCommand, /6.panda.postCreateCommand.testMarker/); - assert.match(outputOfExecCommand, /9.panda.postStartCommand.testMarker/); - assert.match(outputOfExecCommand, /12.panda.postAttachCommand.testMarker/); - - assert.match(outputOfExecCommand, /1.tiger.onCreateCommand.testMarker/); - assert.match(outputOfExecCommand, /4.tiger.updateContentCommand.testMarker/); - assert.match(outputOfExecCommand, /7.tiger.postCreateCommand.testMarker/); - assert.match(outputOfExecCommand, /10.tiger.postStartCommand.testMarker/); - assert.match(outputOfExecCommand, /13.tiger.postAttachCommand.testMarker/); - - assert.match(outputOfExecCommand, /2.devContainer.onCreateCommand.testMarker/); - assert.match(outputOfExecCommand, /5.devContainer.updateContentCommand.testMarker/); - assert.match(outputOfExecCommand, /8.devContainer.postCreateCommand.testMarker/); - assert.match(outputOfExecCommand, /11.devContainer.postStartCommand.testMarker/); - assert.match(outputOfExecCommand, /14.devContainer.postAttachCommand.testMarker/); + assert.match(outputOfExecCommand, /panda.onCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /panda.updateContentCommand.testMarker/); + assert.match(outputOfExecCommand, /panda.postCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /panda.postStartCommand.testMarker/); + assert.match(outputOfExecCommand, /panda.postAttachCommand.testMarker/); + + assert.match(outputOfExecCommand, /tiger.onCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /tiger.updateContentCommand.testMarker/); + assert.match(outputOfExecCommand, /tiger.postCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /tiger.postStartCommand.testMarker/); + assert.match(outputOfExecCommand, /tiger.postAttachCommand.testMarker/); + + assert.match(outputOfExecCommand, /devContainer.onCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /devContainer.updateContentCommand.testMarker/); + assert.match(outputOfExecCommand, /devContainer.postCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /devContainer.postStartCommand.testMarker/); + assert.match(outputOfExecCommand, /devContainer.postAttachCommand.testMarker/); }); }); }); @@ -123,9 +123,4 @@ describe('Feature lifecycle hooks', function () { }); }); }); - - // describe('lifecycle-hooks-parallel-execution', () => { - // assert.fail('TODO'); - // }); - }); \ No newline at end of file From bbadb677af1fbbf6b7f909338446f4b88d133643 Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Fri, 3 Feb 2023 20:01:54 +0000 Subject: [PATCH 14/41] update test --- .../lifecycle-hooks-advanced/.devcontainer/devcontainer.json | 5 ++++- .../.devcontainer/otter/devcontainer-feature.json | 5 ++++- .../.devcontainer/rabbit/devcontainer-feature.json | 5 ++++- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/devcontainer.json b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/devcontainer.json index 2eb96b21a..c2d027c14 100644 --- a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/devcontainer.json +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/devcontainer.json @@ -8,7 +8,10 @@ }, "postCreateCommand": { "parallel1": ".devcontainer/helper_script.sh parallel_postCreateCommand_1", - "parallel2": ".devcontainer/helper_script.sh parallel_postCreateCommand_2" + "parallel2": [ + ".devcontainer/helper_script.sh", + "parallel_postCreateCommand_2" + ] }, "postStartCommand": "touch `rabbit`.postStartCommand.testMarker", // The 'rabbit' command is installed and added to the path by .devcontainer/rabbit/install.sh "postAttachCommand": "touch `otter`.postAttachCommand.testMarker" // The 'otter' command is installed and added to the path by .devcontainer/otter/install.sh diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json index cff7eaa2f..77c5c9c65 100644 --- a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json @@ -6,7 +6,10 @@ "onCreateCommand": "${featureRoot}/helper_script.sh onCreateCommand", "postCreateCommand": { "parallel1": "${featureRoot}/helper_script.sh parallel_postCreateCommand_1", - "parallel2": "${featureRoot}/helper_script.sh parallel_postCreateCommand_2" + "parallel2": [ + "${featureRoot}/helper_script.sh", + "parallel_postCreateCommand_2" + ] }, "postStartCommand": [ "${featureRoot}/helper_script.sh", diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/devcontainer-feature.json b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/devcontainer-feature.json index 795d29fd8..e689500cb 100644 --- a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/devcontainer-feature.json +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/devcontainer-feature.json @@ -6,7 +6,10 @@ "onCreateCommand": "${featureRoot}/helper_script.sh onCreateCommand", "postCreateCommand": { "parallel1": "${featureRoot}/helper_script.sh parallel_postCreateCommand_1", - "parallel2": "${featureRoot}/helper_script.sh parallel_postCreateCommand_2" + "parallel2": [ + "${featureRoot}/helper_script.sh", + "parallel_postCreateCommand_2" + ] }, "postStartCommand": [ "${featureRoot}/helper_script.sh", From 0d3941d5b8e987810690014ac857253df38e6d80 Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Sat, 4 Feb 2023 19:56:54 +0000 Subject: [PATCH 15/41] Display the 'origin' of the lifecycle command in the output log during 'up' Logs like 'Running the postCreateCommand from devcontainer.json' are updated to include the origin of the command e.g: 'Running the postCreateCommand from Feature 'common-utils'. This patch adds in a mapping object that is passed along and used to map a command to an origin. This change is a bit less efficient, with the added benefit of not needing to modify the existing merging logic. --- src/spec-common/injectHeadless.ts | 79 +++++++++++++------ src/spec-node/devContainersSpecCLI.ts | 6 +- src/spec-node/dockerCompose.ts | 8 +- src/spec-node/imageMetadata.ts | 44 ++++++++--- src/spec-node/singleContainer.ts | 13 +-- .../container-features/lifecycleHooks.test.ts | 35 +++++++- src/test/testUtils.ts | 3 +- 7 files changed, 137 insertions(+), 51 deletions(-) diff --git a/src/spec-common/injectHeadless.ts b/src/spec-common/injectHeadless.ts index 74498c30e..b805fbab1 100644 --- a/src/spec-common/injectHeadless.ts +++ b/src/spec-common/injectHeadless.ts @@ -76,6 +76,13 @@ export interface LifecycleHook { done: () => void; } +export type LifecycleCommandOriginMap = { + [lifecycleHook in DevContainerLifecycleHook]: { + command: LifecycleCommand; + origin: string; + }[]; +}; + export function createNullLifecycleHook(enabled: boolean, skipNonBlocking: boolean, output: Log): LifecycleHook { function listener(data: Buffer) { emitter.fire(data.toString()); @@ -115,7 +122,7 @@ export type DevContainerLifecycleHook = 'initializeCommand' | 'onCreateCommand' const defaultWaitFor: DevContainerLifecycleHook = 'updateContentCommand'; -type LifecycleCommand = string | string[]; +export type LifecycleCommand = string | string[] | { [key: string]: string | string[] }; export interface CommonDevContainerConfig { configFilePath?: URI; @@ -157,11 +164,11 @@ const replaceProperties = [ ] as const; interface UpdatedConfigProperties { - onCreateCommands?: (string | string[])[]; - updateContentCommands?: (string | string[])[]; - postCreateCommands?: (string | string[])[]; - postStartCommands?: (string | string[])[]; - postAttachCommands?: (string | string[])[]; + onCreateCommands?: LifecycleCommand[]; + updateContentCommands?: LifecycleCommand[]; + postCreateCommands?: LifecycleCommand[]; + postStartCommands?: LifecycleCommand[]; + postAttachCommands?: LifecycleCommand[]; } export interface OSRelease { @@ -307,14 +314,14 @@ export function getSystemVarFolder(params: ResolverParameters): string { return params.containerSystemDataFolder || '/var/devcontainer'; } -export async function setupInContainer(params: ResolverParameters, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig) { +export async function setupInContainer(params: ResolverParameters, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, lifecycleCommandOriginMap: LifecycleCommandOriginMap) { await patchEtcEnvironment(params, containerProperties); await patchEtcProfile(params, containerProperties); const computeRemoteEnv = params.computeExtensionHostEnv || params.lifecycleHook.enabled; const updatedConfig = containerSubstitute(params.cliHost.platform, config.configFilePath, containerProperties.env, config); const remoteEnv = computeRemoteEnv ? probeRemoteEnv(params, containerProperties, updatedConfig) : Promise.resolve({}); if (params.lifecycleHook.enabled) { - await runLifecycleHooks(params, containerProperties, updatedConfig, remoteEnv, false); + await runLifecycleHooks(params, lifecycleCommandOriginMap, containerProperties, updatedConfig, remoteEnv, false); } return { remoteEnv: params.computeExtensionHostEnv ? await remoteEnv : {}, @@ -330,19 +337,19 @@ export function probeRemoteEnv(params: ResolverParameters, containerProperties: } as Record)); } -export async function runLifecycleHooks(params: ResolverParameters, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, remoteEnv: Promise>, stopForPersonalization: boolean): Promise<'skipNonBlocking' | 'prebuild' | 'stopForPersonalization' | 'done'> { +export async function runLifecycleHooks(params: ResolverParameters, lifecycleCommandOriginMap: LifecycleCommandOriginMap, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, remoteEnv: Promise>, stopForPersonalization: boolean): Promise<'skipNonBlocking' | 'prebuild' | 'stopForPersonalization' | 'done'> { const skipNonBlocking = params.lifecycleHook.skipNonBlocking; const waitFor = config.waitFor || defaultWaitFor; if (skipNonBlocking && waitFor === 'initializeCommand') { return 'skipNonBlocking'; } - await runPostCreateCommand(params, containerProperties, config, 'onCreateCommand', remoteEnv, false); + await runPostCreateCommand(params, lifecycleCommandOriginMap, containerProperties, config, 'onCreateCommand', remoteEnv, false); if (skipNonBlocking && waitFor === 'onCreateCommand') { return 'skipNonBlocking'; } - await runPostCreateCommand(params, containerProperties, config, 'updateContentCommand', remoteEnv, !!params.prebuild); + await runPostCreateCommand(params, lifecycleCommandOriginMap, containerProperties, config, 'updateContentCommand', remoteEnv, !!params.prebuild); if (skipNonBlocking && waitFor === 'updateContentCommand') { return 'skipNonBlocking'; } @@ -351,7 +358,7 @@ export async function runLifecycleHooks(params: ResolverParameters, containerPro return 'prebuild'; } - await runPostCreateCommand(params, containerProperties, config, 'postCreateCommand', remoteEnv, false); + await runPostCreateCommand(params, lifecycleCommandOriginMap, containerProperties, config, 'postCreateCommand', remoteEnv, false); if (skipNonBlocking && waitFor === 'postCreateCommand') { return 'skipNonBlocking'; } @@ -364,13 +371,13 @@ export async function runLifecycleHooks(params: ResolverParameters, containerPro return 'stopForPersonalization'; } - await runPostStartCommand(params, containerProperties, config, remoteEnv); + await runPostStartCommand(params, lifecycleCommandOriginMap, containerProperties, config, remoteEnv); if (skipNonBlocking && waitFor === 'postStartCommand') { return 'skipNonBlocking'; } if (!params.skipPostAttach) { - await runPostAttachCommand(params, containerProperties, config, remoteEnv); + await runPostAttachCommand(params, lifecycleCommandOriginMap, containerProperties, config, remoteEnv); } return 'done'; } @@ -391,16 +398,16 @@ export async function getOSRelease(shellServer: ShellServer) { return { hardware, id, version }; } -async function runPostCreateCommand(params: ResolverParameters, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, postCommandName: 'onCreateCommand' | 'updateContentCommand' | 'postCreateCommand', remoteEnv: Promise>, rerun: boolean) { +async function runPostCreateCommand(params: ResolverParameters, lifecycleCommandOriginMap: LifecycleCommandOriginMap, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, postCommandName: 'onCreateCommand' | 'updateContentCommand' | 'postCreateCommand', remoteEnv: Promise>, rerun: boolean) { const markerFile = path.posix.join(containerProperties.userDataFolder, `.${postCommandName}Marker`); const doRun = !!containerProperties.createdAt && await updateMarkerFile(containerProperties.shellServer, markerFile, containerProperties.createdAt) || rerun; - await runLifecycleCommands(params, containerProperties, config, postCommandName, remoteEnv, doRun); + await runLifecycleCommands(params, lifecycleCommandOriginMap, containerProperties, config, postCommandName, remoteEnv, doRun); } -async function runPostStartCommand(params: ResolverParameters, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, remoteEnv: Promise>) { +async function runPostStartCommand(params: ResolverParameters, lifecycleCommandOriginMap: LifecycleCommandOriginMap, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, remoteEnv: Promise>) { const markerFile = path.posix.join(containerProperties.userDataFolder, '.postStartCommandMarker'); const doRun = !!containerProperties.startedAt && await updateMarkerFile(containerProperties.shellServer, markerFile, containerProperties.startedAt); - await runLifecycleCommands(params, containerProperties, config, 'postStartCommand', remoteEnv, doRun); + await runLifecycleCommands(params, lifecycleCommandOriginMap, containerProperties, config, 'postStartCommand', remoteEnv, doRun); } async function updateMarkerFile(shellServer: ShellServer, location: string, content: string) { @@ -412,15 +419,34 @@ async function updateMarkerFile(shellServer: ShellServer, location: string, cont } } -async function runPostAttachCommand(params: ResolverParameters, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, remoteEnv: Promise>) { - await runLifecycleCommands(params, containerProperties, config, 'postAttachCommand', remoteEnv, true); +async function runPostAttachCommand(params: ResolverParameters, lifecycleCommandOriginMap: LifecycleCommandOriginMap, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, remoteEnv: Promise>) { + await runLifecycleCommands(params, lifecycleCommandOriginMap, containerProperties, config, 'postAttachCommand', remoteEnv, true); +} + +// Determines the origin (devcontainer.json, or a Feature) from a map for a given lifecycle hook + command +// This is never expected to be undefined. +function getLifecycleCommandOriginFromMap(map: LifecycleCommandOriginMap, cmd: LifecycleCommand, hook: DevContainerLifecycleHook): string | undefined { + return map[hook].find(target => { + const targetCmd = target.command; + if (typeof cmd === 'string' && typeof targetCmd === 'string') { + return targetCmd === cmd; + } else { + // Either 'array' or 'object' case. + // This compare should be ok since the order will be preserved in the metadata. + return JSON.stringify(cmd) === JSON.stringify(targetCmd); + } + })?.origin; } -async function runLifecycleCommands(params: ResolverParameters, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, lifecycleHookName: 'onCreateCommand' | 'updateContentCommand' | 'postCreateCommand' | 'postStartCommand' | 'postAttachCommand', remoteEnv: Promise>, doRun: boolean) { - return Promise.all((config[`${lifecycleHookName}s`] || []).map(config => runLifecycleCommand(params, containerProperties, config, lifecycleHookName, remoteEnv, doRun))); +async function runLifecycleCommands(params: ResolverParameters, lifecycleCommandOriginMap: LifecycleCommandOriginMap, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, lifecycleHookName: 'onCreateCommand' | 'updateContentCommand' | 'postCreateCommand' | 'postStartCommand' | 'postAttachCommand', remoteEnv: Promise>, doRun: boolean) { + return Promise.all((config[`${lifecycleHookName}s`] || []).map(command => { + const origin = getLifecycleCommandOriginFromMap(lifecycleCommandOriginMap, command, lifecycleHookName); + const displayOrigin = origin ? (origin === 'devcontainer.json' ? origin : `Feature '${origin}'`) : '???'; /// '???' should never happen. + return runLifecycleCommand(params, containerProperties, command, displayOrigin, lifecycleHookName, remoteEnv, doRun); + })); } -async function runLifecycleCommand({ lifecycleHook: postCreate }: ResolverParameters, containerProperties: ContainerProperties, userCommand: string | string[], lifecycleHookName: 'onCreateCommand' | 'updateContentCommand' | 'postCreateCommand' | 'postStartCommand' | 'postAttachCommand', remoteEnv: Promise>, doRun: boolean) { +async function runLifecycleCommand({ lifecycleHook: postCreate }: ResolverParameters, containerProperties: ContainerProperties, userCommand: LifecycleCommand, userCommandOrigin: string, lifecycleHookName: 'onCreateCommand' | 'updateContentCommand' | 'postCreateCommand' | 'postStartCommand' | 'postAttachCommand', remoteEnv: Promise>, doRun: boolean) { let hasCommand = false; if (typeof userCommand === 'string') { hasCommand = userCommand.trim().length > 0; @@ -473,8 +499,9 @@ async function runLifecycleCommand({ lifecycleHook: postCreate }: ResolverParame const printMode = name ? 'off' : 'continuous'; const { cmdOutput } = await runRemoteCommand({ ...postCreate, output: infoOutput }, containerProperties, typeof postCommand === 'string' ? ['/bin/sh', '-c', postCommand] : postCommand, remoteCwd, { remoteEnv: await remoteEnv, print: printMode }); + // 'name' is set when parallel execution syntax is used. if (name) { - infoOutput.raw(`\x1b[1mRunning ${name} from devcontainer.json...\x1b[0m\r\n${cmdOutput}\r\n`); + infoOutput.raw(`\x1b[1mRunning ${name} from ${userCommandOrigin}...\x1b[0m\r\n${cmdOutput}\r\n`); } infoOutput.event({ @@ -484,7 +511,7 @@ async function runLifecycleCommand({ lifecycleHook: postCreate }: ResolverParame }); } - infoOutput.raw(`\x1b[1mRunning the ${lifecycleHookName} from devcontainer.json...\x1b[0m\r\n\r\n`); + infoOutput.raw(`\x1b[1mRunning the ${lifecycleHookName} from ${userCommandOrigin}...\x1b[0m\r\n\r\n`); let commands; if (typeof userCommand === 'string' || Array.isArray(userCommand)) { @@ -509,7 +536,7 @@ async function runLifecycleCommand({ lifecycleHook: postCreate }: ResolverParame infoOutput.write(toErrorText(`${lifecycleHookName} failed with exit code ${err.code}. Skipping any further user-provided commands.`)); } throw new ContainerError({ - description: `The ${lifecycleHookName} in the devcontainer.json failed.`, + description: `The ${lifecycleHookName} in the ${userCommandOrigin} failed.`, originalError: err, }); } diff --git a/src/spec-node/devContainersSpecCLI.ts b/src/spec-node/devContainersSpecCLI.ts index 8f9b42cf2..99ff043db 100644 --- a/src/spec-node/devContainersSpecCLI.ts +++ b/src/spec-node/devContainersSpecCLI.ts @@ -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'; @@ -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, @@ -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 runLifecycleHooks(common, containerProperties, updatedConfig, remoteEnv, stopForPersonalization); + const result = await runLifecycleHooks(common, lifecycleCommandOriginMapFromMetadata(imageMetadata), containerProperties, updatedConfig, remoteEnv, stopForPersonalization); return { outcome: 'success' as 'success', result, diff --git a/src/spec-node/dockerCompose.ts b/src/spec-node/dockerCompose.ts index da24c1981..48c796d91 100644 --- a/src/spec-node/dockerCompose.ts +++ b/src/spec-node/dockerCompose.ts @@ -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'; @@ -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, diff --git a/src/spec-node/imageMetadata.ts b/src/spec-node/imageMetadata.ts index bb7c00eef..7e741fb90 100644 --- a/src/spec-node/imageMetadata.ts +++ b/src/spec-node/imageMetadata.ts @@ -4,6 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { ContainerError } from '../spec-common/errors'; +import { LifecycleCommand, LifecycleCommandOriginMap } from '../spec-common/injectHeadless'; import { substituteFeatureRoot } from '../spec-common/variableSubstitution'; import { DevContainerConfig, DevContainerConfigCommand, DevContainerFromDockerComposeConfig, DevContainerFromDockerfileConfig, DevContainerFromImageConfig, getDockerComposeFilePaths, getDockerfilePath, HostGPURequirements, HostRequirements, isDockerFileConfig, PortAttributes, UserEnvProbe } from '../spec-configuration/configuration'; import { Feature, FeaturesConfig, Mount, parseMount, SchemaFeatureLifecycleHooks } from '../spec-configuration/containerFeaturesConfiguration'; @@ -74,11 +75,11 @@ export interface ImageMetadataEntry { entrypoint?: string; mounts?: (Mount | string)[]; customizations?: Record; - onCreateCommand?: string | string[]; - updateContentCommand?: string | string[]; - postCreateCommand?: string | string[]; - postStartCommand?: string | string[]; - postAttachCommand?: string | string[]; + onCreateCommand?: LifecycleCommand; + updateContentCommand?: LifecycleCommand; + postCreateCommand?: LifecycleCommand; + postStartCommand?: LifecycleCommand; + postAttachCommand?: LifecycleCommand; waitFor?: DevContainerConfigCommand; remoteUser?: string; containerUser?: string; @@ -112,14 +113,37 @@ const replaceProperties = [ interface UpdatedConfigProperties { customizations?: Record; entrypoints?: string[]; - onCreateCommands?: (string | string[])[]; - updateContentCommands?: (string | string[])[]; - postCreateCommands?: (string | string[])[]; - postStartCommands?: (string | string[])[]; - postAttachCommands?: (string | string[])[]; + onCreateCommands?: LifecycleCommand[]; + updateContentCommands?: LifecycleCommand[]; + postCreateCommands?: LifecycleCommand[]; + postStartCommands?: LifecycleCommand[]; + postAttachCommands?: LifecycleCommand[]; shutdownAction?: 'none' | 'stopContainer' | 'stopCompose'; } +export function lifecycleCommandOriginMapFromMetadata(metadata: ImageMetadataEntry[]): LifecycleCommandOriginMap { + const map: LifecycleCommandOriginMap = { + onCreateCommand: [], + updateContentCommand: [], + postCreateCommand: [], + postStartCommand: [], + postAttachCommand: [], + initializeCommand: [] + }; + for (const entry of metadata) { + const id = entry.id; // Only Features have IDs encoded in the metadata. + const origin = id ?? 'devcontainer.json'; + for (const hook of pickFeatureLifecycleHookProperties) { + const command = entry[hook]; + if (command) { + map[hook].push({ origin, command }); + } + } + } + return map; +} + + export function mergeConfiguration(config: DevContainerConfig, imageMetadata: ImageMetadataEntry[]): MergedDevContainerConfig { const customizations = imageMetadata.reduce((obj, entry) => { for (const key in entry.customizations) { diff --git a/src/spec-node/singleContainer.ts b/src/spec-node/singleContainer.ts index d10453d2e..83e55fd50 100644 --- a/src/spec-node/singleContainer.ts +++ b/src/spec-node/singleContainer.ts @@ -11,7 +11,7 @@ import { ContainerDetails, listContainers, DockerCLIParameters, inspectContainer import { DevContainerConfig, DevContainerFromDockerfileConfig, DevContainerFromImageConfig } from '../spec-configuration/configuration'; import { LogLevel, Log, makeLog } from '../spec-utils/log'; import { extendImage, getExtendImageBuildInfo, updateRemoteUserUID } from './containerFeatures'; -import { getDevcontainerMetadata, getImageBuildInfoFromDockerfile, getImageMetadataFromContainer, ImageMetadataEntry, mergeConfiguration, MergedDevContainerConfig } from './imageMetadata'; +import { getDevcontainerMetadata, getImageBuildInfoFromDockerfile, getImageMetadataFromContainer, ImageMetadataEntry, lifecycleCommandOriginMapFromMetadata, mergeConfiguration, MergedDevContainerConfig } from './imageMetadata'; import { ensureDockerfileHasFinalStageName } from './dockerfileUtils'; export const hostFolderLabel = 'devcontainer.local_folder'; // used to label containers created from a workspace/folder @@ -26,6 +26,7 @@ export async function openDockerfileDevContainer(params: DockerResolverParameter try { container = await findExistingContainer(params, idLabels); + let imageMetadata: ImageMetadataEntry[]; let mergedConfig: MergedDevContainerConfig; if (container) { // let _collapsedFeatureConfig: Promise; @@ -37,11 +38,11 @@ export async function openDockerfileDevContainer(params: DockerResolverParameter // })()); // }; await startExistingContainer(params, idLabels, container); - const imageMetadata = getImageMetadataFromContainer(container, configWithRaw, undefined, idLabels, common.experimentalImageMetadata, common.output).config; + imageMetadata = getImageMetadataFromContainer(container, configWithRaw, undefined, idLabels, common.experimentalImageMetadata, common.output).config; mergedConfig = mergeConfiguration(config, imageMetadata); } else { const res = await buildNamedImageAndExtend(params, configWithRaw, additionalFeatures, true); - const imageMetadata = res.imageMetadata.config; + imageMetadata = res.imageMetadata.config; mergedConfig = mergeConfiguration(config, imageMetadata); const { containerUser } = mergedConfig; const updatedImageName = await updateRemoteUserUID(params, mergedConfig, res.updatedImageName[0], res.imageDetails, findUserArg(config.runArgs) || containerUser); @@ -61,7 +62,7 @@ export async function openDockerfileDevContainer(params: DockerResolverParameter } containerProperties = await createContainerProperties(params, container.Id, workspaceConfig.workspaceFolder, mergedConfig.remoteUser); - return await setupContainer(container, params, containerProperties, config, mergedConfig); + return await setupContainer(container, params, containerProperties, config, mergedConfig, imageMetadata); } catch (e) { throw createSetupError(e, container, params, containerProperties, config); @@ -88,11 +89,11 @@ function createSetupError(originalError: any, container: ContainerDetails | unde return err; } -async function setupContainer(container: ContainerDetails, params: DockerResolverParameters, containerProperties: ContainerProperties, config: DevContainerFromDockerfileConfig | DevContainerFromImageConfig, mergedConfig: MergedDevContainerConfig): Promise { +async function setupContainer(container: ContainerDetails, params: DockerResolverParameters, containerProperties: ContainerProperties, config: DevContainerFromDockerfileConfig | DevContainerFromImageConfig, mergedConfig: MergedDevContainerConfig, imageMetadata: ImageMetadataEntry[]): Promise { const { common } = params; const { remoteEnv: extensionHostEnv, - } = await setupInContainer(common, containerProperties, mergedConfig); + } = await setupInContainer(common, containerProperties, mergedConfig, lifecycleCommandOriginMapFromMetadata(imageMetadata)); return { params: common, diff --git a/src/test/container-features/lifecycleHooks.test.ts b/src/test/container-features/lifecycleHooks.test.ts index 4f9abafcd..516c63061 100644 --- a/src/test/container-features/lifecycleHooks.test.ts +++ b/src/test/container-features/lifecycleHooks.test.ts @@ -70,11 +70,14 @@ describe('Feature lifecycle hooks', function () { describe(`devcontainer up`, () => { let containerId: string | null = null; + let containerUpStandardError: string; const testFolder = `${__dirname}/configs/lifecycle-hooks-advanced`; before(async () => { await shellExec(`rm -f ${testFolder}/*.testMarker`, undefined, undefined, true); - containerId = (await devContainerUp(cli, testFolder, { 'logLevel': 'trace' })).containerId; + const res = await devContainerUp(cli, testFolder, { 'logLevel': 'trace' }); + containerId = res.containerId; + containerUpStandardError = res.stderr; }); after(async () => { @@ -93,10 +96,16 @@ describe('Feature lifecycle hooks', function () { // Executes the command that was installed by each Feature's 'install.sh'. // The command is installed to a directory on the $PATH so it can be executed from the lifecycle script. assert.match(outputOfExecCommand, /i-am-a-rabbit.postStartCommand.testMarker/); + assert.match(containerUpStandardError, /Running the postCreateCommand from devcontainer.json/); + assert.match(outputOfExecCommand, /i-am-an-otter.postAttachCommand.testMarker/); + assert.match(containerUpStandardError, /Running the postAttachCommand from devcontainer.json/); assert.match(outputOfExecCommand, /helperScript.devContainer.parallel_postCreateCommand_1.testMarker/); + assert.match(containerUpStandardError, /Running parallel1 from devcontainer.json.../); + assert.match(outputOfExecCommand, /helperScript.devContainer.parallel_postCreateCommand_2.testMarker/); + assert.match(containerUpStandardError, /Running parallel2 from devcontainer.json.../); // Since lifecycle scripts are executed relative to the workspace folder, // to run a script bundled with the Feature, the user needs to use the '${featureRoot}' variable. @@ -105,21 +114,45 @@ describe('Feature lifecycle hooks', function () { // -- 'Rabbit' Feature assert.match(outputOfExecCommand, /helperScript.rabbit.onCreateCommand.testMarker/); + assert.match(containerUpStandardError, /Running the onCreateCommand from Feature '\.\/rabbit'/); + assert.match(outputOfExecCommand, /helperScript.rabbit.updateContentCommand.testMarker/); + assert.match(containerUpStandardError, /Running the updateContentCommand from Feature '\.\/rabbit'/); + assert.match(outputOfExecCommand, /helperScript.rabbit.postStartCommand.testMarker/); + assert.match(containerUpStandardError, /Running the postStartCommand from Feature '\.\/rabbit'/); + assert.match(outputOfExecCommand, /helperScript.rabbit.postAttachCommand.testMarker/); + assert.match(containerUpStandardError, /Running the postAttachCommand from Feature '\.\/rabbit'/); assert.match(outputOfExecCommand, /helperScript.rabbit.parallel_postCreateCommand_1.testMarker/); + assert.match(containerUpStandardError, /Running parallel1 from Feature '\.\/rabbit'/); + assert.match(outputOfExecCommand, /helperScript.rabbit.parallel_postCreateCommand_2.testMarker/); + assert.match(containerUpStandardError, /Running parallel2 from Feature '\.\/rabbit'/); + // -- 'Otter' Feature assert.match(outputOfExecCommand, /helperScript.otter.onCreateCommand.testMarker/); + assert.match(containerUpStandardError, /Running the onCreateCommand from Feature '\.\/otter'/); + assert.match(outputOfExecCommand, /helperScript.otter.updateContentCommand.testMarker/); + assert.match(containerUpStandardError, /Running the updateContentCommand from Feature '\.\/otter'/); + assert.match(outputOfExecCommand, /helperScript.otter.postStartCommand.testMarker/); + assert.match(containerUpStandardError, /Running the postStartCommand from Feature '\.\/otter'/); + assert.match(outputOfExecCommand, /helperScript.otter.postAttachCommand.testMarker/); + assert.match(containerUpStandardError, /Running the postAttachCommand from Feature '\.\/otter'/); assert.match(outputOfExecCommand, /helperScript.otter.parallel_postCreateCommand_1.testMarker/); + assert.match(containerUpStandardError, /Running parallel1 from Feature '\.\/otter'/); + assert.match(outputOfExecCommand, /helperScript.otter.parallel_postCreateCommand_2.testMarker/); + assert.match(containerUpStandardError, /Running parallel2 from Feature '\.\/otter'/); + + // -- Assert that at no point did logging the lifecycle hook fail. + assert.notMatch(containerUpStandardError, /Running the (.*) from \?\?\?/); }); }); }); diff --git a/src/test/testUtils.ts b/src/test/testUtils.ts index de7288a20..7d638a918 100644 --- a/src/test/testUtils.ts +++ b/src/test/testUtils.ts @@ -21,6 +21,7 @@ export interface UpResult { outcome: string; containerId: string; composeProjectName: string | undefined; + stderr: string; } export interface ExecResult { @@ -52,7 +53,7 @@ export async function devContainerUp(cli: string, workspaceFolder: string, optio assert.equal(response.outcome, 'success'); const { outcome, containerId, composeProjectName } = response as UpResult; assert.ok(containerId, 'Container id not found.'); - return { outcome, containerId, composeProjectName }; + return { outcome, containerId, composeProjectName, stderr: res.stderr }; } export async function devContainerDown(options: { containerId?: string | null; composeProjectName?: string | null }) { if (options.containerId) { From f320096e8c1558476802c0103c168e61efb43e93 Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Sat, 4 Feb 2023 20:09:26 +0000 Subject: [PATCH 16/41] Change copy path for Features on-container from /tmp-build-features to /opt/build-features This is to support resuming a container in Codespaces with Feature-contributed lifecycle hooks. Codespaces bind-mounts a different volume on top of /tmp, so the container's /tmp directory is not persisted. --- .../containerFeaturesConfiguration.ts | 16 +++++++++------- src/spec-node/containerFeatures.ts | 14 +++++++------- src/spec-node/imageMetadata.ts | 2 +- .../generateFeaturesConfig.test.ts | 16 ++++++++-------- 4 files changed, 25 insertions(+), 23 deletions(-) diff --git a/src/spec-configuration/containerFeaturesConfiguration.ts b/src/spec-configuration/containerFeaturesConfiguration.ts index e6684aff8..89ec97d7e 100644 --- a/src/spec-configuration/containerFeaturesConfiguration.ts +++ b/src/spec-configuration/containerFeaturesConfiguration.ts @@ -25,6 +25,8 @@ export const DEVCONTAINER_FEATURE_FILE_NAME = 'devcontainer-feature.json'; export type Feature = SchemaFeatureBaseProperties & SchemaFeatureLifecycleHooks & DeprecatedSchemaFeatureProperties & InternalFeatureProperties; +export const CONTAINER_BUILD_FEATURES_DIR = '/opt/build-features'; + export interface SchemaFeatureLifecycleHooks { onCreateCommand?: string | string[]; updateContentCommand?: string | string[]; @@ -245,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 0777 /tmp/build-features +COPY --from=dev_containers_feature_content_source {contentSourceRootPath} /opt/build-features/ +RUN chmod -R 0777 /opt/build-features 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} @@ -331,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 @@ -350,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 diff --git a/src/spec-node/containerFeatures.ts b/src/spec-node/containerFeatures.ts index 2a7056030..6e3844c94 100644 --- a/src/spec-node/containerFeatures.ts +++ b/src/spec-node/containerFeatures.ts @@ -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) @@ -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)); @@ -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'); @@ -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'); diff --git a/src/spec-node/imageMetadata.ts b/src/spec-node/imageMetadata.ts index 7e741fb90..5fe2dbf6a 100644 --- a/src/spec-node/imageMetadata.ts +++ b/src/spec-node/imageMetadata.ts @@ -286,7 +286,7 @@ export function getDevcontainerMetadata(baseImageMetadata: SubstitutedConfig featureSet.features.forEach(f => { pickFeatureLifecycleHookProperties.forEach(hook => { - const buildPath = `/tmp/build-features/${f.consecutiveId}`; + const buildPath = `/opt/build-features/${f.consecutiveId}`; if (f[hook]) { f[hook] = substituteFeatureRoot(f[hook], buildPath); } diff --git a/src/test/container-features/generateFeaturesConfig.test.ts b/src/test/container-features/generateFeaturesConfig.test.ts index ddd7c687f..314825605 100644 --- a/src/test/container-features/generateFeaturesConfig.test.ts +++ b/src/test/container-features/generateFeaturesConfig.test.ts @@ -72,14 +72,14 @@ describe('validate generateFeaturesConfig()', function () { // getFeatureLayers const actualLayers = getFeatureLayers(featuresConfig, 'testContainerUser', 'testRemoteUser'); const expectedLayers = `RUN \\ -echo "_CONTAINER_USER_HOME=$(getent passwd testContainerUser | cut -d: -f6)" >> /tmp/build-features/devcontainer-features.builtin.env && \\ -echo "_REMOTE_USER_HOME=$(getent passwd testRemoteUser | cut -d: -f6)" >> /tmp/build-features/devcontainer-features.builtin.env +echo "_CONTAINER_USER_HOME=$(getent passwd testContainerUser | cut -d: -f6)" >> /opt/build-features/devcontainer-features.builtin.env && \\ +echo "_REMOTE_USER_HOME=$(getent passwd testRemoteUser | cut -d: -f6)" >> /opt/build-features/devcontainer-features.builtin.env -RUN cd /tmp/build-features/first_1 \\ +RUN cd /opt/build-features/first_1 \\ && chmod +x ./install.sh \\ && ./install.sh -RUN cd /tmp/build-features/second_2 \\ +RUN cd /opt/build-features/second_2 \\ && chmod +x ./install.sh \\ && ./install.sh @@ -130,16 +130,16 @@ RUN cd /tmp/build-features/second_2 \\ // getFeatureLayers const actualLayers = getFeatureLayers(featuresConfig, 'testContainerUser', 'testRemoteUser'); const expectedLayers = `RUN \\ -echo "_CONTAINER_USER_HOME=$(getent passwd testContainerUser | cut -d: -f6)" >> /tmp/build-features/devcontainer-features.builtin.env && \\ -echo "_REMOTE_USER_HOME=$(getent passwd testRemoteUser | cut -d: -f6)" >> /tmp/build-features/devcontainer-features.builtin.env +echo "_CONTAINER_USER_HOME=$(getent passwd testContainerUser | cut -d: -f6)" >> /opt/build-features/devcontainer-features.builtin.env && \\ +echo "_REMOTE_USER_HOME=$(getent passwd testRemoteUser | cut -d: -f6)" >> /opt/build-features/devcontainer-features.builtin.env -RUN cd /tmp/build-features/color_3 \\ +RUN cd /opt/build-features/color_3 \\ && chmod +x ./devcontainer-features-install.sh \\ && ./devcontainer-features-install.sh -RUN cd /tmp/build-features/hello_4 \\ +RUN cd /opt/build-features/hello_4 \\ && chmod +x ./devcontainer-features-install.sh \\ && ./devcontainer-features-install.sh From 95969610d9f2207c989b21849e83c886e0097dba Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Mon, 6 Feb 2023 19:56:48 +0000 Subject: [PATCH 17/41] run lifecycle commands in Feature install order --- src/spec-common/injectHeadless.ts | 28 ++++++--------- .../.devcontainer/createMarker.sh | 2 +- .../tiger/devcontainer-feature.json | 3 ++ .../container-features/lifecycleHooks.test.ts | 36 +++++++++---------- 4 files changed, 32 insertions(+), 37 deletions(-) diff --git a/src/spec-common/injectHeadless.ts b/src/spec-common/injectHeadless.ts index b805fbab1..7bb14d5b6 100644 --- a/src/spec-common/injectHeadless.ts +++ b/src/spec-common/injectHeadless.ts @@ -344,6 +344,8 @@ export async function runLifecycleHooks(params: ResolverParameters, lifecycleCom return 'skipNonBlocking'; } + params.output.write('LifecycleCommandExecutionMap: ' + JSON.stringify(lifecycleCommandOriginMap, undefined, 4), LogLevel.Trace); + await runPostCreateCommand(params, lifecycleCommandOriginMap, containerProperties, config, 'onCreateCommand', remoteEnv, false); if (skipNonBlocking && waitFor === 'onCreateCommand') { return 'skipNonBlocking'; @@ -423,27 +425,17 @@ async function runPostAttachCommand(params: ResolverParameters, lifecycleCommand await runLifecycleCommands(params, lifecycleCommandOriginMap, containerProperties, config, 'postAttachCommand', remoteEnv, true); } -// Determines the origin (devcontainer.json, or a Feature) from a map for a given lifecycle hook + command -// This is never expected to be undefined. -function getLifecycleCommandOriginFromMap(map: LifecycleCommandOriginMap, cmd: LifecycleCommand, hook: DevContainerLifecycleHook): string | undefined { - return map[hook].find(target => { - const targetCmd = target.command; - if (typeof cmd === 'string' && typeof targetCmd === 'string') { - return targetCmd === cmd; - } else { - // Either 'array' or 'object' case. - // This compare should be ok since the order will be preserved in the metadata. - return JSON.stringify(cmd) === JSON.stringify(targetCmd); - } - })?.origin; -} async function runLifecycleCommands(params: ResolverParameters, lifecycleCommandOriginMap: LifecycleCommandOriginMap, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, lifecycleHookName: 'onCreateCommand' | 'updateContentCommand' | 'postCreateCommand' | 'postStartCommand' | 'postAttachCommand', remoteEnv: Promise>, doRun: boolean) { - return Promise.all((config[`${lifecycleHookName}s`] || []).map(command => { - const origin = getLifecycleCommandOriginFromMap(lifecycleCommandOriginMap, command, lifecycleHookName); + const commandsForHook = lifecycleCommandOriginMap[lifecycleHookName]; + if (commandsForHook.length === 0) { + return; + } + + for (const { command, origin } of commandsForHook) { const displayOrigin = origin ? (origin === 'devcontainer.json' ? origin : `Feature '${origin}'`) : '???'; /// '???' should never happen. - return runLifecycleCommand(params, containerProperties, command, displayOrigin, lifecycleHookName, remoteEnv, doRun); - })); + await runLifecycleCommand(params, containerProperties, command, displayOrigin, lifecycleHookName, remoteEnv, doRun); + } } async function runLifecycleCommand({ lifecycleHook: postCreate }: ResolverParameters, containerProperties: ContainerProperties, userCommand: LifecycleCommand, userCommandOrigin: string, lifecycleHookName: 'onCreateCommand' | 'updateContentCommand' | 'postCreateCommand' | 'postStartCommand' | 'postAttachCommand', remoteEnv: Promise>, doRun: boolean) { diff --git a/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/createMarker.sh b/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/createMarker.sh index af33cd222..c8146b8d8 100755 --- a/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/createMarker.sh +++ b/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/createMarker.sh @@ -7,7 +7,7 @@ sleep 1s [[ -f saved_value.testMarker ]] || echo 0 > saved_value.testMarker n=$(< saved_value.testMarker) -echo "${n}.`date +%s%3N`" > "${MARKER_FILE_NAME}" +echo "${n}.`date +%s%3N`" > "${n}.${MARKER_FILE_NAME}" echo $(( n + 1 )) > saved_value.testMarker echo "Ending '${MARKER_FILE_NAME}'...." diff --git a/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/tiger/devcontainer-feature.json b/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/tiger/devcontainer-feature.json index 6b1f6919b..29ef857c3 100644 --- a/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/tiger/devcontainer-feature.json +++ b/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/tiger/devcontainer-feature.json @@ -10,5 +10,8 @@ "postAttachCommand": [ ".devcontainer/createMarker.sh", "tiger.postAttachCommand.testMarker" + ], + "installsAfter": [ + "./panda" ] } \ No newline at end of file diff --git a/src/test/container-features/lifecycleHooks.test.ts b/src/test/container-features/lifecycleHooks.test.ts index 516c63061..8920a278e 100644 --- a/src/test/container-features/lifecycleHooks.test.ts +++ b/src/test/container-features/lifecycleHooks.test.ts @@ -37,7 +37,7 @@ describe('Feature lifecycle hooks', function () { await shellExec(`rm -f ${testFolder}/*.testMarker`, undefined, undefined, true); }); - it('marker files should exist', async () => { + it('marker files should exist and executed in stable order', async () => { const res = await shellExec(`${cli} exec --workspace-folder ${testFolder} ls -altr`); const response = JSON.parse(res.stdout); assert.equal(response.outcome, 'success'); @@ -45,23 +45,23 @@ describe('Feature lifecycle hooks', function () { const outputOfExecCommand = res.stderr; console.log(outputOfExecCommand); - assert.match(outputOfExecCommand, /panda.onCreateCommand.testMarker/); - assert.match(outputOfExecCommand, /panda.updateContentCommand.testMarker/); - assert.match(outputOfExecCommand, /panda.postCreateCommand.testMarker/); - assert.match(outputOfExecCommand, /panda.postStartCommand.testMarker/); - assert.match(outputOfExecCommand, /panda.postAttachCommand.testMarker/); - - assert.match(outputOfExecCommand, /tiger.onCreateCommand.testMarker/); - assert.match(outputOfExecCommand, /tiger.updateContentCommand.testMarker/); - assert.match(outputOfExecCommand, /tiger.postCreateCommand.testMarker/); - assert.match(outputOfExecCommand, /tiger.postStartCommand.testMarker/); - assert.match(outputOfExecCommand, /tiger.postAttachCommand.testMarker/); - - assert.match(outputOfExecCommand, /devContainer.onCreateCommand.testMarker/); - assert.match(outputOfExecCommand, /devContainer.updateContentCommand.testMarker/); - assert.match(outputOfExecCommand, /devContainer.postCreateCommand.testMarker/); - assert.match(outputOfExecCommand, /devContainer.postStartCommand.testMarker/); - assert.match(outputOfExecCommand, /devContainer.postAttachCommand.testMarker/); + assert.match(outputOfExecCommand, /0.panda.onCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /3.panda.updateContentCommand.testMarker/); + assert.match(outputOfExecCommand, /6.panda.postCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /9.panda.postStartCommand.testMarker/); + assert.match(outputOfExecCommand, /12.panda.postAttachCommand.testMarker/); + + assert.match(outputOfExecCommand, /1.tiger.onCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /4.tiger.updateContentCommand.testMarker/); + assert.match(outputOfExecCommand, /7.tiger.postCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /10.tiger.postStartCommand.testMarker/); + assert.match(outputOfExecCommand, /13.tiger.postAttachCommand.testMarker/); + + assert.match(outputOfExecCommand, /2.devContainer.onCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /5.devContainer.updateContentCommand.testMarker/); + assert.match(outputOfExecCommand, /8.devContainer.postCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /11.devContainer.postStartCommand.testMarker/); + assert.match(outputOfExecCommand, /14.devContainer.postAttachCommand.testMarker/); }); }); }); From 311a39797f7ff9e7dcd8e0c7429a807004428a49 Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Mon, 6 Feb 2023 21:16:58 +0000 Subject: [PATCH 18/41] stop passing config object where unused --- src/spec-common/injectHeadless.ts | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/spec-common/injectHeadless.ts b/src/spec-common/injectHeadless.ts index 7bb14d5b6..ef4650c86 100644 --- a/src/spec-common/injectHeadless.ts +++ b/src/spec-common/injectHeadless.ts @@ -337,21 +337,21 @@ export function probeRemoteEnv(params: ResolverParameters, containerProperties: } as Record)); } -export async function runLifecycleHooks(params: ResolverParameters, lifecycleCommandOriginMap: LifecycleCommandOriginMap, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, remoteEnv: Promise>, stopForPersonalization: boolean): Promise<'skipNonBlocking' | 'prebuild' | 'stopForPersonalization' | 'done'> { +export async function runLifecycleHooks(params: ResolverParameters, lifecycleHooksInstallMap: LifecycleCommandOriginMap, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, remoteEnv: Promise>, stopForPersonalization: boolean): Promise<'skipNonBlocking' | 'prebuild' | 'stopForPersonalization' | 'done'> { const skipNonBlocking = params.lifecycleHook.skipNonBlocking; const waitFor = config.waitFor || defaultWaitFor; if (skipNonBlocking && waitFor === 'initializeCommand') { return 'skipNonBlocking'; } - params.output.write('LifecycleCommandExecutionMap: ' + JSON.stringify(lifecycleCommandOriginMap, undefined, 4), LogLevel.Trace); + params.output.write('LifecycleCommandExecutionMap: ' + JSON.stringify(lifecycleHooksInstallMap, undefined, 4), LogLevel.Trace); - await runPostCreateCommand(params, lifecycleCommandOriginMap, containerProperties, config, 'onCreateCommand', remoteEnv, false); + await runPostCreateCommand(params, lifecycleHooksInstallMap, containerProperties, 'onCreateCommand', remoteEnv, false); if (skipNonBlocking && waitFor === 'onCreateCommand') { return 'skipNonBlocking'; } - await runPostCreateCommand(params, lifecycleCommandOriginMap, containerProperties, config, 'updateContentCommand', remoteEnv, !!params.prebuild); + await runPostCreateCommand(params, lifecycleHooksInstallMap, containerProperties, 'updateContentCommand', remoteEnv, !!params.prebuild); if (skipNonBlocking && waitFor === 'updateContentCommand') { return 'skipNonBlocking'; } @@ -360,7 +360,7 @@ export async function runLifecycleHooks(params: ResolverParameters, lifecycleCom return 'prebuild'; } - await runPostCreateCommand(params, lifecycleCommandOriginMap, containerProperties, config, 'postCreateCommand', remoteEnv, false); + await runPostCreateCommand(params, lifecycleHooksInstallMap, containerProperties, 'postCreateCommand', remoteEnv, false); if (skipNonBlocking && waitFor === 'postCreateCommand') { return 'skipNonBlocking'; } @@ -373,13 +373,13 @@ export async function runLifecycleHooks(params: ResolverParameters, lifecycleCom return 'stopForPersonalization'; } - await runPostStartCommand(params, lifecycleCommandOriginMap, containerProperties, config, remoteEnv); + await runPostStartCommand(params, lifecycleHooksInstallMap, containerProperties, remoteEnv); if (skipNonBlocking && waitFor === 'postStartCommand') { return 'skipNonBlocking'; } if (!params.skipPostAttach) { - await runPostAttachCommand(params, lifecycleCommandOriginMap, containerProperties, config, remoteEnv); + await runPostAttachCommand(params, lifecycleHooksInstallMap, containerProperties, remoteEnv); } return 'done'; } @@ -400,16 +400,16 @@ export async function getOSRelease(shellServer: ShellServer) { return { hardware, id, version }; } -async function runPostCreateCommand(params: ResolverParameters, lifecycleCommandOriginMap: LifecycleCommandOriginMap, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, postCommandName: 'onCreateCommand' | 'updateContentCommand' | 'postCreateCommand', remoteEnv: Promise>, rerun: boolean) { +async function runPostCreateCommand(params: ResolverParameters, lifecycleCommandOriginMap: LifecycleCommandOriginMap, containerProperties: ContainerProperties, postCommandName: 'onCreateCommand' | 'updateContentCommand' | 'postCreateCommand', remoteEnv: Promise>, rerun: boolean) { const markerFile = path.posix.join(containerProperties.userDataFolder, `.${postCommandName}Marker`); const doRun = !!containerProperties.createdAt && await updateMarkerFile(containerProperties.shellServer, markerFile, containerProperties.createdAt) || rerun; - await runLifecycleCommands(params, lifecycleCommandOriginMap, containerProperties, config, postCommandName, remoteEnv, doRun); + await runLifecycleCommands(params, lifecycleCommandOriginMap, containerProperties, postCommandName, remoteEnv, doRun); } -async function runPostStartCommand(params: ResolverParameters, lifecycleCommandOriginMap: LifecycleCommandOriginMap, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, remoteEnv: Promise>) { +async function runPostStartCommand(params: ResolverParameters, lifecycleCommandOriginMap: LifecycleCommandOriginMap, containerProperties: ContainerProperties, remoteEnv: Promise>) { const markerFile = path.posix.join(containerProperties.userDataFolder, '.postStartCommandMarker'); const doRun = !!containerProperties.startedAt && await updateMarkerFile(containerProperties.shellServer, markerFile, containerProperties.startedAt); - await runLifecycleCommands(params, lifecycleCommandOriginMap, containerProperties, config, 'postStartCommand', remoteEnv, doRun); + await runLifecycleCommands(params, lifecycleCommandOriginMap, containerProperties, 'postStartCommand', remoteEnv, doRun); } async function updateMarkerFile(shellServer: ShellServer, location: string, content: string) { @@ -421,12 +421,12 @@ async function updateMarkerFile(shellServer: ShellServer, location: string, cont } } -async function runPostAttachCommand(params: ResolverParameters, lifecycleCommandOriginMap: LifecycleCommandOriginMap, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, remoteEnv: Promise>) { - await runLifecycleCommands(params, lifecycleCommandOriginMap, containerProperties, config, 'postAttachCommand', remoteEnv, true); +async function runPostAttachCommand(params: ResolverParameters, lifecycleCommandOriginMap: LifecycleCommandOriginMap, containerProperties: ContainerProperties, remoteEnv: Promise>) { + await runLifecycleCommands(params, lifecycleCommandOriginMap, containerProperties, 'postAttachCommand', remoteEnv, true); } -async function runLifecycleCommands(params: ResolverParameters, lifecycleCommandOriginMap: LifecycleCommandOriginMap, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, lifecycleHookName: 'onCreateCommand' | 'updateContentCommand' | 'postCreateCommand' | 'postStartCommand' | 'postAttachCommand', remoteEnv: Promise>, doRun: boolean) { +async function runLifecycleCommands(params: ResolverParameters, lifecycleCommandOriginMap: LifecycleCommandOriginMap, containerProperties: ContainerProperties, lifecycleHookName: 'onCreateCommand' | 'updateContentCommand' | 'postCreateCommand' | 'postStartCommand' | 'postAttachCommand', remoteEnv: Promise>, doRun: boolean) { const commandsForHook = lifecycleCommandOriginMap[lifecycleHookName]; if (commandsForHook.length === 0) { return; From 0350081ed987fc26b1783d91411f9427a32b3e56 Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Mon, 6 Feb 2023 21:18:19 +0000 Subject: [PATCH 19/41] Rename LifecycleCommandOriginMap -> LifecycleHooksInstallMap --- src/spec-common/injectHeadless.ts | 16 ++++++++-------- src/spec-node/imageMetadata.ts | 6 +++--- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/spec-common/injectHeadless.ts b/src/spec-common/injectHeadless.ts index ef4650c86..4986f2828 100644 --- a/src/spec-common/injectHeadless.ts +++ b/src/spec-common/injectHeadless.ts @@ -76,11 +76,11 @@ export interface LifecycleHook { done: () => void; } -export type LifecycleCommandOriginMap = { +export type LifecycleHooksInstallMap = { [lifecycleHook in DevContainerLifecycleHook]: { command: LifecycleCommand; origin: string; - }[]; + }[]; // In installation order. }; export function createNullLifecycleHook(enabled: boolean, skipNonBlocking: boolean, output: Log): LifecycleHook { @@ -314,7 +314,7 @@ export function getSystemVarFolder(params: ResolverParameters): string { return params.containerSystemDataFolder || '/var/devcontainer'; } -export async function setupInContainer(params: ResolverParameters, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, lifecycleCommandOriginMap: LifecycleCommandOriginMap) { +export async function setupInContainer(params: ResolverParameters, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, lifecycleCommandOriginMap: LifecycleHooksInstallMap) { await patchEtcEnvironment(params, containerProperties); await patchEtcProfile(params, containerProperties); const computeRemoteEnv = params.computeExtensionHostEnv || params.lifecycleHook.enabled; @@ -337,7 +337,7 @@ export function probeRemoteEnv(params: ResolverParameters, containerProperties: } as Record)); } -export async function runLifecycleHooks(params: ResolverParameters, lifecycleHooksInstallMap: LifecycleCommandOriginMap, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, remoteEnv: Promise>, stopForPersonalization: boolean): Promise<'skipNonBlocking' | 'prebuild' | 'stopForPersonalization' | 'done'> { +export async function runLifecycleHooks(params: ResolverParameters, lifecycleHooksInstallMap: LifecycleHooksInstallMap, containerProperties: ContainerProperties, config: CommonMergedDevContainerConfig, remoteEnv: Promise>, stopForPersonalization: boolean): Promise<'skipNonBlocking' | 'prebuild' | 'stopForPersonalization' | 'done'> { const skipNonBlocking = params.lifecycleHook.skipNonBlocking; const waitFor = config.waitFor || defaultWaitFor; if (skipNonBlocking && waitFor === 'initializeCommand') { @@ -400,13 +400,13 @@ export async function getOSRelease(shellServer: ShellServer) { return { hardware, id, version }; } -async function runPostCreateCommand(params: ResolverParameters, lifecycleCommandOriginMap: LifecycleCommandOriginMap, containerProperties: ContainerProperties, postCommandName: 'onCreateCommand' | 'updateContentCommand' | 'postCreateCommand', remoteEnv: Promise>, rerun: boolean) { +async function runPostCreateCommand(params: ResolverParameters, lifecycleCommandOriginMap: LifecycleHooksInstallMap, containerProperties: ContainerProperties, postCommandName: 'onCreateCommand' | 'updateContentCommand' | 'postCreateCommand', remoteEnv: Promise>, rerun: boolean) { const markerFile = path.posix.join(containerProperties.userDataFolder, `.${postCommandName}Marker`); const doRun = !!containerProperties.createdAt && await updateMarkerFile(containerProperties.shellServer, markerFile, containerProperties.createdAt) || rerun; await runLifecycleCommands(params, lifecycleCommandOriginMap, containerProperties, postCommandName, remoteEnv, doRun); } -async function runPostStartCommand(params: ResolverParameters, lifecycleCommandOriginMap: LifecycleCommandOriginMap, containerProperties: ContainerProperties, remoteEnv: Promise>) { +async function runPostStartCommand(params: ResolverParameters, lifecycleCommandOriginMap: LifecycleHooksInstallMap, containerProperties: ContainerProperties, remoteEnv: Promise>) { const markerFile = path.posix.join(containerProperties.userDataFolder, '.postStartCommandMarker'); const doRun = !!containerProperties.startedAt && await updateMarkerFile(containerProperties.shellServer, markerFile, containerProperties.startedAt); await runLifecycleCommands(params, lifecycleCommandOriginMap, containerProperties, 'postStartCommand', remoteEnv, doRun); @@ -421,12 +421,12 @@ async function updateMarkerFile(shellServer: ShellServer, location: string, cont } } -async function runPostAttachCommand(params: ResolverParameters, lifecycleCommandOriginMap: LifecycleCommandOriginMap, containerProperties: ContainerProperties, remoteEnv: Promise>) { +async function runPostAttachCommand(params: ResolverParameters, lifecycleCommandOriginMap: LifecycleHooksInstallMap, containerProperties: ContainerProperties, remoteEnv: Promise>) { await runLifecycleCommands(params, lifecycleCommandOriginMap, containerProperties, 'postAttachCommand', remoteEnv, true); } -async function runLifecycleCommands(params: ResolverParameters, lifecycleCommandOriginMap: LifecycleCommandOriginMap, containerProperties: ContainerProperties, lifecycleHookName: 'onCreateCommand' | 'updateContentCommand' | 'postCreateCommand' | 'postStartCommand' | 'postAttachCommand', remoteEnv: Promise>, doRun: boolean) { +async function runLifecycleCommands(params: ResolverParameters, lifecycleCommandOriginMap: LifecycleHooksInstallMap, containerProperties: ContainerProperties, lifecycleHookName: 'onCreateCommand' | 'updateContentCommand' | 'postCreateCommand' | 'postStartCommand' | 'postAttachCommand', remoteEnv: Promise>, doRun: boolean) { const commandsForHook = lifecycleCommandOriginMap[lifecycleHookName]; if (commandsForHook.length === 0) { return; diff --git a/src/spec-node/imageMetadata.ts b/src/spec-node/imageMetadata.ts index 5fe2dbf6a..6bafa95da 100644 --- a/src/spec-node/imageMetadata.ts +++ b/src/spec-node/imageMetadata.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { ContainerError } from '../spec-common/errors'; -import { LifecycleCommand, LifecycleCommandOriginMap } from '../spec-common/injectHeadless'; +import { LifecycleCommand, LifecycleHooksInstallMap } from '../spec-common/injectHeadless'; import { substituteFeatureRoot } from '../spec-common/variableSubstitution'; import { DevContainerConfig, DevContainerConfigCommand, DevContainerFromDockerComposeConfig, DevContainerFromDockerfileConfig, DevContainerFromImageConfig, getDockerComposeFilePaths, getDockerfilePath, HostGPURequirements, HostRequirements, isDockerFileConfig, PortAttributes, UserEnvProbe } from '../spec-configuration/configuration'; import { Feature, FeaturesConfig, Mount, parseMount, SchemaFeatureLifecycleHooks } from '../spec-configuration/containerFeaturesConfiguration'; @@ -121,8 +121,8 @@ interface UpdatedConfigProperties { shutdownAction?: 'none' | 'stopContainer' | 'stopCompose'; } -export function lifecycleCommandOriginMapFromMetadata(metadata: ImageMetadataEntry[]): LifecycleCommandOriginMap { - const map: LifecycleCommandOriginMap = { +export function lifecycleCommandOriginMapFromMetadata(metadata: ImageMetadataEntry[]): LifecycleHooksInstallMap { + const map: LifecycleHooksInstallMap = { onCreateCommand: [], updateContentCommand: [], postCreateCommand: [], From 26406cfa4d7e746d4c3d29e27808e6151456aaff Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Mon, 6 Feb 2023 21:37:01 +0000 Subject: [PATCH 20/41] add test to assert installsAfter is respected --- .gitignore | 3 +- .../otter/devcontainer-feature.json | 3 +- .../container-features/lifecycleHooks.test.ts | 129 +++++++++++++----- 3 files changed, 97 insertions(+), 38 deletions(-) diff --git a/.gitignore b/.gitignore index 57205e24f..2c69dd1c6 100644 --- a/.gitignore +++ b/.gitignore @@ -11,4 +11,5 @@ build-tmp .DS_Store .env output -*.testMarker \ No newline at end of file +*.testMarker +src/test/container-features/configs/temp_lifecycle-hooks-alternative-order diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json index 77c5c9c65..0ce830a8e 100644 --- a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json @@ -14,8 +14,7 @@ "postStartCommand": [ "${featureRoot}/helper_script.sh", "postStartCommand" - ] - , + ], "postAttachCommand": [ "${featureRoot}/helper_script.sh", "postAttachCommand" diff --git a/src/test/container-features/lifecycleHooks.test.ts b/src/test/container-features/lifecycleHooks.test.ts index 8920a278e..508bdf6c0 100644 --- a/src/test/container-features/lifecycleHooks.test.ts +++ b/src/test/container-features/lifecycleHooks.test.ts @@ -5,6 +5,7 @@ import { assert } from 'chai'; import * as path from 'path'; +import { Feature } from '../../spec-configuration/containerFeaturesConfiguration'; import { devContainerDown, devContainerUp, shellExec } from '../testUtils'; const pkg = require('../../../package.json'); @@ -23,47 +24,105 @@ describe('Feature lifecycle hooks', function () { describe('lifecycle-hooks-inline-commands', () => { - describe(`devcontainer up`, () => { - let containerId: string | null = null; - const testFolder = `${__dirname}/configs/lifecycle-hooks-inline-commands`; + let containerId: string | null = null; + const testFolder = `${__dirname}/configs/lifecycle-hooks-inline-commands`; - before(async () => { - await shellExec(`rm -f ${testFolder}/*.testMarker`, undefined, undefined, true); - containerId = (await devContainerUp(cli, testFolder, { 'logLevel': 'trace' })).containerId; - }); + before(async () => { + await shellExec(`rm -f ${testFolder}/*.testMarker`, undefined, undefined, true); + containerId = (await devContainerUp(cli, testFolder, { 'logLevel': 'trace' })).containerId; + }); - after(async () => { - await devContainerDown({ containerId }); - await shellExec(`rm -f ${testFolder}/*.testMarker`, undefined, undefined, true); - }); + after(async () => { + await devContainerDown({ containerId }); + await shellExec(`rm -f ${testFolder}/*.testMarker`, undefined, undefined, true); + }); - it('marker files should exist and executed in stable order', async () => { - const res = await shellExec(`${cli} exec --workspace-folder ${testFolder} ls -altr`); - const response = JSON.parse(res.stdout); - assert.equal(response.outcome, 'success'); + it('marker files should exist and executed in stable order', async () => { + const res = await shellExec(`${cli} exec --workspace-folder ${testFolder} ls -altr`); + const response = JSON.parse(res.stdout); + assert.equal(response.outcome, 'success'); + + const outputOfExecCommand = res.stderr; + console.log(outputOfExecCommand); + + assert.match(outputOfExecCommand, /0.panda.onCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /3.panda.updateContentCommand.testMarker/); + assert.match(outputOfExecCommand, /6.panda.postCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /9.panda.postStartCommand.testMarker/); + assert.match(outputOfExecCommand, /12.panda.postAttachCommand.testMarker/); + + assert.match(outputOfExecCommand, /1.tiger.onCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /4.tiger.updateContentCommand.testMarker/); + assert.match(outputOfExecCommand, /7.tiger.postCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /10.tiger.postStartCommand.testMarker/); + assert.match(outputOfExecCommand, /13.tiger.postAttachCommand.testMarker/); + + assert.match(outputOfExecCommand, /2.devContainer.onCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /5.devContainer.updateContentCommand.testMarker/); + assert.match(outputOfExecCommand, /8.devContainer.postCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /11.devContainer.postStartCommand.testMarker/); + assert.match(outputOfExecCommand, /14.devContainer.postAttachCommand.testMarker/); + }); + }); - const outputOfExecCommand = res.stderr; - console.log(outputOfExecCommand); + describe('lifecycle-hooks-alternative-order', () => { + // This is the same test as 'lifecycle-hooks-inline-commands' but with the the 'installsAfter' order changed (tiger -> panda -> devContainer). - assert.match(outputOfExecCommand, /0.panda.onCreateCommand.testMarker/); - assert.match(outputOfExecCommand, /3.panda.updateContentCommand.testMarker/); - assert.match(outputOfExecCommand, /6.panda.postCreateCommand.testMarker/); - assert.match(outputOfExecCommand, /9.panda.postStartCommand.testMarker/); - assert.match(outputOfExecCommand, /12.panda.postAttachCommand.testMarker/); - - assert.match(outputOfExecCommand, /1.tiger.onCreateCommand.testMarker/); - assert.match(outputOfExecCommand, /4.tiger.updateContentCommand.testMarker/); - assert.match(outputOfExecCommand, /7.tiger.postCreateCommand.testMarker/); - assert.match(outputOfExecCommand, /10.tiger.postStartCommand.testMarker/); - assert.match(outputOfExecCommand, /13.tiger.postAttachCommand.testMarker/); - - assert.match(outputOfExecCommand, /2.devContainer.onCreateCommand.testMarker/); - assert.match(outputOfExecCommand, /5.devContainer.updateContentCommand.testMarker/); - assert.match(outputOfExecCommand, /8.devContainer.postCreateCommand.testMarker/); - assert.match(outputOfExecCommand, /11.devContainer.postStartCommand.testMarker/); - assert.match(outputOfExecCommand, /14.devContainer.postAttachCommand.testMarker/); - }); + let containerId: string | null = null; + const testFolder = `${__dirname}/configs/temp_lifecycle-hooks-alternative-order`; + + before(async () => { + await shellExec(`rm -rf ${testFolder}`, undefined, undefined, true); + await shellExec(`mkdir -p ${testFolder}`); + await shellExec(`bash -c 'cp -r . ${testFolder}'`, { cwd: `${__dirname}/configs/lifecycle-hooks-inline-commands` }); + + // Read in the JSON from the two Feature's devcontainer-feature.json + const pandaFeatureJson: Feature = JSON.parse((await shellExec(`cat ${testFolder}/.devcontainer/panda/devcontainer-feature.json`)).stdout); + const tigerFeatureJson: Feature = JSON.parse((await shellExec(`cat ${testFolder}/.devcontainer/tiger/devcontainer-feature.json`)).stdout); + + // Remove the installsAfter from the tiger's devcontainer-feature.json and add it to the panda's devcontainer-feature.json + delete tigerFeatureJson.installsAfter; + pandaFeatureJson.installsAfter = ['./tiger']; + + // Write the JSON back to the two Feature's devcontainer-feature.json + await shellExec(`echo '${JSON.stringify(pandaFeatureJson)}' > ${testFolder}/.devcontainer/panda/devcontainer-feature.json`); + await shellExec(`echo '${JSON.stringify(tigerFeatureJson)}' > ${testFolder}/.devcontainer/tiger/devcontainer-feature.json`); + + containerId = (await devContainerUp(cli, testFolder, { 'logLevel': 'trace' })).containerId; + }); + + after(async () => { + await devContainerDown({ containerId }); + await shellExec(`rm -f ${testFolder}/*.testMarker`, undefined, undefined, true); }); + + it('marker files should exist and executed in stable order', async () => { + const res = await shellExec(`${cli} exec --workspace-folder ${testFolder} ls -altr`); + const response = JSON.parse(res.stdout); + assert.equal(response.outcome, 'success'); + + const outputOfExecCommand = res.stderr; + console.log(outputOfExecCommand); + + assert.match(outputOfExecCommand, /0.tiger.onCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /3.tiger.updateContentCommand.testMarker/); + assert.match(outputOfExecCommand, /6.tiger.postCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /9.tiger.postStartCommand.testMarker/); + assert.match(outputOfExecCommand, /12.tiger.postAttachCommand.testMarker/); + + assert.match(outputOfExecCommand, /1.panda.onCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /4.panda.updateContentCommand.testMarker/); + assert.match(outputOfExecCommand, /7.panda.postCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /10.panda.postStartCommand.testMarker/); + assert.match(outputOfExecCommand, /13.panda.postAttachCommand.testMarker/); + + assert.match(outputOfExecCommand, /2.devContainer.onCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /5.devContainer.updateContentCommand.testMarker/); + assert.match(outputOfExecCommand, /8.devContainer.postCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /11.devContainer.postStartCommand.testMarker/); + assert.match(outputOfExecCommand, /14.devContainer.postAttachCommand.testMarker/); + }); + }); describe('lifecycle-hooks-advanced', () => { From cafa37940747e5a4385f55ce409affbb70ba5af7 Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Mon, 6 Feb 2023 21:37:42 +0000 Subject: [PATCH 21/41] rm -rf test dir --- src/test/container-features/lifecycleHooks.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/container-features/lifecycleHooks.test.ts b/src/test/container-features/lifecycleHooks.test.ts index 508bdf6c0..5b3dce600 100644 --- a/src/test/container-features/lifecycleHooks.test.ts +++ b/src/test/container-features/lifecycleHooks.test.ts @@ -93,7 +93,7 @@ describe('Feature lifecycle hooks', function () { after(async () => { await devContainerDown({ containerId }); - await shellExec(`rm -f ${testFolder}/*.testMarker`, undefined, undefined, true); + await shellExec(`rm -rf ${testFolder}`, undefined, undefined, true); }); it('marker files should exist and executed in stable order', async () => { From cf819cff61fb8782cd771221fb0b91ef0ecd3468 Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Mon, 6 Feb 2023 21:57:48 +0000 Subject: [PATCH 22/41] assert contents in metadata label --- .../localFeatureA/devcontainer-feature.json | 17 +++- .../container-features/lifecycleHooks.test.ts | 77 +++++++++++-------- src/test/imageMetadata.test.ts | 14 ++++ 3 files changed, 75 insertions(+), 33 deletions(-) diff --git a/src/test/configs/image-metadata/.devcontainer/localFeatureA/devcontainer-feature.json b/src/test/configs/image-metadata/.devcontainer/localFeatureA/devcontainer-feature.json index 3b8be3b51..77f064bdf 100644 --- a/src/test/configs/image-metadata/.devcontainer/localFeatureA/devcontainer-feature.json +++ b/src/test/configs/image-metadata/.devcontainer/localFeatureA/devcontainer-feature.json @@ -9,5 +9,20 @@ "extensionB" ] } - } + }, + "updateContentCommand": [ + "one", + "two" + ], + "onCreateCommand": { + "command": "three", + "commandWithArgs": [ + "four", + "arg1", + "arg2" + ] + }, + "postCreateCommand": "five", + "postStartCommand": "six", + "postAttachCommand": "seven" } diff --git a/src/test/container-features/lifecycleHooks.test.ts b/src/test/container-features/lifecycleHooks.test.ts index 5b3dce600..16c25cf42 100644 --- a/src/test/container-features/lifecycleHooks.test.ts +++ b/src/test/container-features/lifecycleHooks.test.ts @@ -23,50 +23,63 @@ describe('Feature lifecycle hooks', function () { }); describe('lifecycle-hooks-inline-commands', () => { - - let containerId: string | null = null; const testFolder = `${__dirname}/configs/lifecycle-hooks-inline-commands`; - before(async () => { - await shellExec(`rm -f ${testFolder}/*.testMarker`, undefined, undefined, true); - containerId = (await devContainerUp(cli, testFolder, { 'logLevel': 'trace' })).containerId; - }); + describe('devcontainer up', () => { + let containerId: string | null = null; - after(async () => { - await devContainerDown({ containerId }); - await shellExec(`rm -f ${testFolder}/*.testMarker`, undefined, undefined, true); - }); + before(async () => { + await shellExec(`rm -f ${testFolder}/*.testMarker`, undefined, undefined, true); + containerId = (await devContainerUp(cli, testFolder, { 'logLevel': 'trace' })).containerId; + }); - it('marker files should exist and executed in stable order', async () => { - const res = await shellExec(`${cli} exec --workspace-folder ${testFolder} ls -altr`); - const response = JSON.parse(res.stdout); - assert.equal(response.outcome, 'success'); + after(async () => { + await devContainerDown({ containerId }); + await shellExec(`rm -f ${testFolder}/*.testMarker`, undefined, undefined, true); + }); - const outputOfExecCommand = res.stderr; - console.log(outputOfExecCommand); + it('marker files should exist and executed in stable order', async () => { + const res = await shellExec(`${cli} exec --workspace-folder ${testFolder} ls -altr`); + const response = JSON.parse(res.stdout); + assert.equal(response.outcome, 'success'); - assert.match(outputOfExecCommand, /0.panda.onCreateCommand.testMarker/); - assert.match(outputOfExecCommand, /3.panda.updateContentCommand.testMarker/); - assert.match(outputOfExecCommand, /6.panda.postCreateCommand.testMarker/); - assert.match(outputOfExecCommand, /9.panda.postStartCommand.testMarker/); - assert.match(outputOfExecCommand, /12.panda.postAttachCommand.testMarker/); + const outputOfExecCommand = res.stderr; + console.log(outputOfExecCommand); - assert.match(outputOfExecCommand, /1.tiger.onCreateCommand.testMarker/); - assert.match(outputOfExecCommand, /4.tiger.updateContentCommand.testMarker/); - assert.match(outputOfExecCommand, /7.tiger.postCreateCommand.testMarker/); - assert.match(outputOfExecCommand, /10.tiger.postStartCommand.testMarker/); - assert.match(outputOfExecCommand, /13.tiger.postAttachCommand.testMarker/); + assert.match(outputOfExecCommand, /0.panda.onCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /3.panda.updateContentCommand.testMarker/); + assert.match(outputOfExecCommand, /6.panda.postCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /9.panda.postStartCommand.testMarker/); + assert.match(outputOfExecCommand, /12.panda.postAttachCommand.testMarker/); + + assert.match(outputOfExecCommand, /1.tiger.onCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /4.tiger.updateContentCommand.testMarker/); + assert.match(outputOfExecCommand, /7.tiger.postCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /10.tiger.postStartCommand.testMarker/); + assert.match(outputOfExecCommand, /13.tiger.postAttachCommand.testMarker/); + + assert.match(outputOfExecCommand, /2.devContainer.onCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /5.devContainer.updateContentCommand.testMarker/); + assert.match(outputOfExecCommand, /8.devContainer.postCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /11.devContainer.postStartCommand.testMarker/); + assert.match(outputOfExecCommand, /14.devContainer.postAttachCommand.testMarker/); + }); + }); - assert.match(outputOfExecCommand, /2.devContainer.onCreateCommand.testMarker/); - assert.match(outputOfExecCommand, /5.devContainer.updateContentCommand.testMarker/); - assert.match(outputOfExecCommand, /8.devContainer.postCreateCommand.testMarker/); - assert.match(outputOfExecCommand, /11.devContainer.postStartCommand.testMarker/); - assert.match(outputOfExecCommand, /14.devContainer.postAttachCommand.testMarker/); + describe('devcontainer build', () => { + before(async () => { + const res = await shellExec(`${cli} build --workspace-folder ${testFolder} color`); + }); + + it('metadata label exists and in expeced order', async () => { + + }); }); }); describe('lifecycle-hooks-alternative-order', () => { - // This is the same test as 'lifecycle-hooks-inline-commands' but with the the 'installsAfter' order changed (tiger -> panda -> devContainer). + // This is the same test as 'lifecycle-hooks-inline-commands' + // but with the the 'installsAfter' order changed (tiger -> panda -> devContainer). let containerId: string | null = null; const testFolder = `${__dirname}/configs/temp_lifecycle-hooks-alternative-order`; diff --git a/src/test/imageMetadata.test.ts b/src/test/imageMetadata.test.ts index c94ee3eb2..07891452e 100644 --- a/src/test/imageMetadata.test.ts +++ b/src/test/imageMetadata.test.ts @@ -56,6 +56,20 @@ describe('Image Metadata', function () { assert.strictEqual(metadata[0].id, 'baseFeature-substituted'); assert.strictEqual(metadata[1].id, './localFeatureA-substituted'); assert.strictEqual(metadata[1].init, true); + + assert.deepStrictEqual(metadata[1].updateContentCommand, ['one', 'two']); + assert.deepStrictEqual(metadata[1].onCreateCommand, { + 'command': 'three', + 'commandWithArgs': [ + 'four', + 'arg1', + 'arg2' + ] + }); + assert.deepStrictEqual(metadata[1].postCreateCommand, 'five'); + assert.deepStrictEqual(metadata[1].postStartCommand, 'six'); + assert.deepStrictEqual(metadata[1].postAttachCommand, 'seven'); + assert.strictEqual(metadata[2].id, './localFeatureB-substituted'); assert.strictEqual(metadata[2].privileged, true); assert.strictEqual(raw.length, 3); From 7b3258683ffd045e0efbe8f497679bbf627f2305 Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Mon, 6 Feb 2023 21:59:22 +0000 Subject: [PATCH 23/41] unused test stub --- src/test/container-features/lifecycleHooks.test.ts | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/test/container-features/lifecycleHooks.test.ts b/src/test/container-features/lifecycleHooks.test.ts index 16c25cf42..207af1569 100644 --- a/src/test/container-features/lifecycleHooks.test.ts +++ b/src/test/container-features/lifecycleHooks.test.ts @@ -65,16 +65,6 @@ describe('Feature lifecycle hooks', function () { assert.match(outputOfExecCommand, /14.devContainer.postAttachCommand.testMarker/); }); }); - - describe('devcontainer build', () => { - before(async () => { - const res = await shellExec(`${cli} build --workspace-folder ${testFolder} color`); - }); - - it('metadata label exists and in expeced order', async () => { - - }); - }); }); describe('lifecycle-hooks-alternative-order', () => { From 0f1bfd5da6aac4a0f39fb93774c9975a44aff8a5 Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Mon, 6 Feb 2023 22:10:56 +0000 Subject: [PATCH 24/41] exercise dev container test cmd --- .../src/a/devcontainer-feature.json | 8 ++++++ .../lifecycle-hooks/src/a/install.sh | 13 +++++++++ .../src/b/devcontainer-feature.json | 5 ++++ .../lifecycle-hooks/src/b/install.sh | 7 +++++ .../lifecycle-hooks/test/a/test.sh | 8 ++++++ .../lifecycle-hooks/test/b/test.sh | 8 ++++++ .../featuresCLICommands.test.ts | 27 +++++++++++++++++++ 7 files changed, 76 insertions(+) create mode 100644 src/test/container-features/example-v2-features-sets/lifecycle-hooks/src/a/devcontainer-feature.json create mode 100644 src/test/container-features/example-v2-features-sets/lifecycle-hooks/src/a/install.sh create mode 100644 src/test/container-features/example-v2-features-sets/lifecycle-hooks/src/b/devcontainer-feature.json create mode 100644 src/test/container-features/example-v2-features-sets/lifecycle-hooks/src/b/install.sh create mode 100644 src/test/container-features/example-v2-features-sets/lifecycle-hooks/test/a/test.sh create mode 100644 src/test/container-features/example-v2-features-sets/lifecycle-hooks/test/b/test.sh diff --git a/src/test/container-features/example-v2-features-sets/lifecycle-hooks/src/a/devcontainer-feature.json b/src/test/container-features/example-v2-features-sets/lifecycle-hooks/src/a/devcontainer-feature.json new file mode 100644 index 000000000..c23c4cff2 --- /dev/null +++ b/src/test/container-features/example-v2-features-sets/lifecycle-hooks/src/a/devcontainer-feature.json @@ -0,0 +1,8 @@ +{ + "id": "a", + "version": "1.0.0", + "installsAfter": [ + "./b" + ], + "onCreateCommand": "touch a.onCreateCommand.testMarker && echo 'A-ON-CREATE-COMMAND'" +} \ No newline at end of file diff --git a/src/test/container-features/example-v2-features-sets/lifecycle-hooks/src/a/install.sh b/src/test/container-features/example-v2-features-sets/lifecycle-hooks/src/a/install.sh new file mode 100644 index 000000000..21e05d112 --- /dev/null +++ b/src/test/container-features/example-v2-features-sets/lifecycle-hooks/src/a/install.sh @@ -0,0 +1,13 @@ +#!/bin/sh +set -e + +echo "Activating feature 'a'" + +touch /usr/local/bin/a + +# InstallsAfter Feature B + +if [ ! -f /usr/local/bin/b ]; then + echo "Feature B not available!" + exit 1 +fi \ No newline at end of file diff --git a/src/test/container-features/example-v2-features-sets/lifecycle-hooks/src/b/devcontainer-feature.json b/src/test/container-features/example-v2-features-sets/lifecycle-hooks/src/b/devcontainer-feature.json new file mode 100644 index 000000000..c44240502 --- /dev/null +++ b/src/test/container-features/example-v2-features-sets/lifecycle-hooks/src/b/devcontainer-feature.json @@ -0,0 +1,5 @@ +{ + "id": "b", + "version": "1.0.0", + "onCreateCommand": "touch b.onCreateCommand.testMarker && echo 'B-ON-CREATE-COMMAND'" +} \ No newline at end of file diff --git a/src/test/container-features/example-v2-features-sets/lifecycle-hooks/src/b/install.sh b/src/test/container-features/example-v2-features-sets/lifecycle-hooks/src/b/install.sh new file mode 100644 index 000000000..224934dda --- /dev/null +++ b/src/test/container-features/example-v2-features-sets/lifecycle-hooks/src/b/install.sh @@ -0,0 +1,7 @@ +#!/bin/sh +set -e + +echo "Activating feature 'b'" + +touch /usr/local/bin/b + diff --git a/src/test/container-features/example-v2-features-sets/lifecycle-hooks/test/a/test.sh b/src/test/container-features/example-v2-features-sets/lifecycle-hooks/test/a/test.sh new file mode 100644 index 000000000..036849417 --- /dev/null +++ b/src/test/container-features/example-v2-features-sets/lifecycle-hooks/test/a/test.sh @@ -0,0 +1,8 @@ +#!/bin/bash + +set -e + +# Optional: Import test library +source dev-container-features-test-lib + +test -f /usr/local/bin/a \ No newline at end of file diff --git a/src/test/container-features/example-v2-features-sets/lifecycle-hooks/test/b/test.sh b/src/test/container-features/example-v2-features-sets/lifecycle-hooks/test/b/test.sh new file mode 100644 index 000000000..fc39d54b3 --- /dev/null +++ b/src/test/container-features/example-v2-features-sets/lifecycle-hooks/test/b/test.sh @@ -0,0 +1,8 @@ +#!/bin/bash + +set -e + +# Optional: Import test library +source dev-container-features-test-lib + +test -f /usr/local/bin/b \ No newline at end of file diff --git a/src/test/container-features/featuresCLICommands.test.ts b/src/test/container-features/featuresCLICommands.test.ts index 419b0aaf1..3829b85c8 100644 --- a/src/test/container-features/featuresCLICommands.test.ts +++ b/src/test/container-features/featuresCLICommands.test.ts @@ -329,6 +329,33 @@ describe('CLI features subcommands', async function () { assert.isTrue(hasExpectedTestReport); }); + it('lifecycle-hooks', async function () { + const collectionFolder = `${__dirname}/example-v2-features-sets/lifecycle-hooks`; + let success = false; + let result: ExecResult | undefined = undefined; + try { + result = await shellExec(`${cli} features test --log-level trace ${collectionFolder}`); + success = true; + + } catch (error) { + assert.fail('features test sub-command should not throw'); + } + + assert.isTrue(success); + assert.isDefined(result); + + const onCreateFiredFeatureA = result.stderr.includes('A-ON-CREATE-COMMAND'); + assert.isTrue(onCreateFiredFeatureA); + const onCreateFiredFeatureB = result.stderr.includes('B-ON-CREATE-COMMAND'); + assert.isTrue(onCreateFiredFeatureB); + + const expectedTestReport = ` ================== TEST REPORT ================== +✅ Passed: 'a' +✅ Passed: 'b'`; + const hasExpectedTestReport = result.stdout.includes(expectedTestReport); + assert.isTrue(hasExpectedTestReport); + }); + it('installsAfter fruit -> hello', async function () { const collectionFolder = `${__dirname}/configs/example-installsAfter`; let result: ExecResult | undefined = undefined; From 5e7ebcbeec3fc3d8e2f655a1e627029802e4f075 Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Mon, 6 Feb 2023 23:39:51 +0000 Subject: [PATCH 25/41] increase test timeouts --- src/test/cli.exec.base.ts | 2 +- src/test/cli.up.test.ts | 2 +- src/test/container-features/e2e.test.ts | 2 +- .../b-installs-after-a/src/a/devcontainer-feature.json | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/cli.exec.base.ts b/src/test/cli.exec.base.ts index 76419e68c..142d3b437 100644 --- a/src/test/cli.exec.base.ts +++ b/src/test/cli.exec.base.ts @@ -12,7 +12,7 @@ const pkg = require('../../package.json'); export function describeTests1({ text, options }: BuildKitOption) { describe('Dev Containers CLI', function () { - this.timeout('240s'); + this.timeout('360s'); const tmp = path.relative(process.cwd(), path.join(__dirname, 'tmp')); const cli = `npx --prefix ${tmp} devcontainer`; diff --git a/src/test/cli.up.test.ts b/src/test/cli.up.test.ts index b55f38ea9..b2cca1ca7 100644 --- a/src/test/cli.up.test.ts +++ b/src/test/cli.up.test.ts @@ -12,7 +12,7 @@ import { devContainerDown, devContainerUp, shellExec, UpResult, pathExists } fro const pkg = require('../../package.json'); describe('Dev Containers CLI', function () { - this.timeout('120s'); + this.timeout('240s'); const tmp = path.relative(process.cwd(), path.join(__dirname, 'tmp')); const cli = `npx --prefix ${tmp} devcontainer`; diff --git a/src/test/container-features/e2e.test.ts b/src/test/container-features/e2e.test.ts index 8ff2aef78..e896e82f7 100644 --- a/src/test/container-features/e2e.test.ts +++ b/src/test/container-features/e2e.test.ts @@ -11,7 +11,7 @@ import { devContainerDown, devContainerUp, shellExec } from '../testUtils'; const pkg = require('../../../package.json'); describe('Dev Container Features E2E (remote)', function () { - this.timeout('120s'); + this.timeout('240s'); const tmp = path.relative(process.cwd(), path.join(__dirname, 'tmp')); const cli = `npx --prefix ${tmp} devcontainer`; diff --git a/src/test/container-features/example-v2-features-sets/b-installs-after-a/src/a/devcontainer-feature.json b/src/test/container-features/example-v2-features-sets/b-installs-after-a/src/a/devcontainer-feature.json index e6aee827f..d39911d5b 100644 --- a/src/test/container-features/example-v2-features-sets/b-installs-after-a/src/a/devcontainer-feature.json +++ b/src/test/container-features/example-v2-features-sets/b-installs-after-a/src/a/devcontainer-feature.json @@ -1,4 +1,4 @@ { "id": "a", - "version": "1.0.0", + "version": "1.0.0" } \ No newline at end of file From c000b10f7dbeecb6abf42a791a0a5097a0bb3a6e Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Tue, 7 Feb 2023 18:23:05 +0000 Subject: [PATCH 26/41] '' -> '' --- src/spec-common/variableSubstitution.ts | 6 +++--- .../.devcontainer/otter/devcontainer-feature.json | 12 ++++++------ .../.devcontainer/rabbit/devcontainer-feature.json | 12 ++++++------ src/test/container-features/lifecycleHooks.test.ts | 2 +- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/spec-common/variableSubstitution.ts b/src/spec-common/variableSubstitution.ts index 0bc296388..86581a22f 100644 --- a/src/spec-common/variableSubstitution.ts +++ b/src/spec-common/variableSubstitution.ts @@ -124,10 +124,10 @@ function replaceContainerEnv(isWindows: boolean, configFile: URI | undefined, co } } -function replaceFeatureRoot(featureRoot: string | undefined, match: string, variable: string) { +function replaceFeatureRoot(featureRootFolder: string | undefined, match: string, variable: string) { switch (variable) { - case 'featureRoot': - return featureRoot || match; + case 'featureRootFolder': + return featureRootFolder || match; default: return match; diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json index 0ce830a8e..88e62fc45 100644 --- a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json @@ -2,21 +2,21 @@ "id": "otter", "version": "1.2.3", "options": {}, - "updateContentCommand": "${featureRoot}/helper_script.sh updateContentCommand", - "onCreateCommand": "${featureRoot}/helper_script.sh onCreateCommand", + "updateContentCommand": "${featureRootFolder}/helper_script.sh updateContentCommand", + "onCreateCommand": "${featureRootFolder}/helper_script.sh onCreateCommand", "postCreateCommand": { - "parallel1": "${featureRoot}/helper_script.sh parallel_postCreateCommand_1", + "parallel1": "${featureRootFolder}/helper_script.sh parallel_postCreateCommand_1", "parallel2": [ - "${featureRoot}/helper_script.sh", + "${featureRootFolder}/helper_script.sh", "parallel_postCreateCommand_2" ] }, "postStartCommand": [ - "${featureRoot}/helper_script.sh", + "${featureRootFolder}/helper_script.sh", "postStartCommand" ], "postAttachCommand": [ - "${featureRoot}/helper_script.sh", + "${featureRootFolder}/helper_script.sh", "postAttachCommand" ] } \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/devcontainer-feature.json b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/devcontainer-feature.json index e689500cb..fb798522f 100644 --- a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/devcontainer-feature.json +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/devcontainer-feature.json @@ -2,21 +2,21 @@ "id": "rabbit", "version": "100.200.300", "options": {}, - "updateContentCommand": "${featureRoot}/helper_script.sh updateContentCommand", - "onCreateCommand": "${featureRoot}/helper_script.sh onCreateCommand", + "updateContentCommand": "${featureRootFolder}/helper_script.sh updateContentCommand", + "onCreateCommand": "${featureRootFolder}/helper_script.sh onCreateCommand", "postCreateCommand": { - "parallel1": "${featureRoot}/helper_script.sh parallel_postCreateCommand_1", + "parallel1": "${featureRootFolder}/helper_script.sh parallel_postCreateCommand_1", "parallel2": [ - "${featureRoot}/helper_script.sh", + "${featureRootFolder}/helper_script.sh", "parallel_postCreateCommand_2" ] }, "postStartCommand": [ - "${featureRoot}/helper_script.sh", + "${featureRootFolder}/helper_script.sh", "postStartCommand" ], "postAttachCommand": [ - "${featureRoot}/helper_script.sh", + "${featureRootFolder}/helper_script.sh", "postAttachCommand" ] } \ No newline at end of file diff --git a/src/test/container-features/lifecycleHooks.test.ts b/src/test/container-features/lifecycleHooks.test.ts index 207af1569..e6eb706bc 100644 --- a/src/test/container-features/lifecycleHooks.test.ts +++ b/src/test/container-features/lifecycleHooks.test.ts @@ -170,7 +170,7 @@ describe('Feature lifecycle hooks', function () { assert.match(containerUpStandardError, /Running parallel2 from devcontainer.json.../); // Since lifecycle scripts are executed relative to the workspace folder, - // to run a script bundled with the Feature, the user needs to use the '${featureRoot}' variable. + // to run a script bundled with the Feature, the user needs to use the '${featureRootFolder}' variable. // This variable can only be used in a devcontainer-feature.json's lifecycle scripts. // And will return the temporary directory where the Feature's files are copied to. From 8ede78348238da4227a99d5485d060c2f6782ecb Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Thu, 9 Feb 2023 23:29:03 +0000 Subject: [PATCH 27/41] pass contentSourceRootPath as a variable instead of string subst --- src/spec-configuration/containerFeaturesConfiguration.ts | 9 +++------ src/spec-node/containerFeatures.ts | 3 +-- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/spec-configuration/containerFeaturesConfiguration.ts b/src/spec-configuration/containerFeaturesConfiguration.ts index 967a90e79..e0cb7c5b8 100644 --- a/src/spec-configuration/containerFeaturesConfiguration.ts +++ b/src/spec-configuration/containerFeaturesConfiguration.ts @@ -239,27 +239,24 @@ export function getSourceInfoString(srcInfo: SourceInformation): string { } // TODO: Move to node layer. -export function getContainerFeaturesBaseDockerFile() { +export function getContainerFeaturesBaseDockerFile(contentSourceRootPath: string) { return ` -#{featureBuildStages} #{nonBuildKitFeatureContentFallback} FROM $_DEV_CONTAINERS_BASE_IMAGE AS dev_containers_feature_content_normalize USER root -COPY --from=dev_containers_feature_content_source {contentSourceRootPath}/devcontainer-features.builtin.env /tmp/build-features/ +COPY --from=dev_containers_feature_content_source ${contentSourceRootPath}/devcontainer-features.builtin.env /tmp/build-features/ RUN chmod -R 0777 /tmp/build-features 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 ${contentSourceRootPath} /tmp/build-features #{featureLayer} -#{copyFeatureBuildStages} - #{containerEnv} ARG _DEV_CONTAINERS_IMAGE_USER=root diff --git a/src/spec-node/containerFeatures.ts b/src/spec-node/containerFeatures.ts index 6640284d5..9f8cadc06 100644 --- a/src/spec-node/containerFeatures.ts +++ b/src/spec-node/containerFeatures.ts @@ -285,9 +285,8 @@ async function getFeaturesBuildOptions(params: DockerResolverParameters, devCont // 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/'; - const dockerfile = getContainerFeaturesBaseDockerFile() + const dockerfile = getContainerFeaturesBaseDockerFile(contentSourceRootPath) .replace('#{nonBuildKitFeatureContentFallback}', useBuildKitBuildContexts ? '' : `FROM ${buildContentImageName} as dev_containers_feature_content_source`) - .replace('{contentSourceRootPath}', contentSourceRootPath) .replace('#{featureBuildStages}', getFeatureBuildStages(featuresConfig, buildStageScripts, contentSourceRootPath)) .replace('#{featureLayer}', getFeatureLayers(featuresConfig, containerUser, remoteUser, useBuildKitBuildContexts, contentSourceRootPath)) .replace('#{containerEnv}', generateContainerEnvs(featuresConfig)) From 91e160bb0a2b9bcb1a25562cf56c9a593f2146bc Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Thu, 9 Feb 2023 23:29:29 +0000 Subject: [PATCH 28/41] remove dead code referencing "hasAcquires" --- src/spec-node/containerFeatures.ts | 27 --------------------------- 1 file changed, 27 deletions(-) diff --git a/src/spec-node/containerFeatures.ts b/src/spec-node/containerFeatures.ts index 9f8cadc06..0d352751b 100644 --- a/src/spec-node/containerFeatures.ts +++ b/src/spec-node/containerFeatures.ts @@ -287,10 +287,8 @@ async function getFeaturesBuildOptions(params: DockerResolverParameters, devCont const contentSourceRootPath = useBuildKitBuildContexts ? '.' : '/tmp/build-features/'; const dockerfile = getContainerFeaturesBaseDockerFile(contentSourceRootPath) .replace('#{nonBuildKitFeatureContentFallback}', useBuildKitBuildContexts ? '' : `FROM ${buildContentImageName} as dev_containers_feature_content_source`) - .replace('#{featureBuildStages}', getFeatureBuildStages(featuresConfig, buildStageScripts, contentSourceRootPath)) .replace('#{featureLayer}', getFeatureLayers(featuresConfig, containerUser, remoteUser, useBuildKitBuildContexts, contentSourceRootPath)) .replace('#{containerEnv}', generateContainerEnvs(featuresConfig)) - .replace('#{copyFeatureBuildStages}', getCopyFeatureBuildStages(featuresConfig, buildStageScripts)) .replace('#{devcontainerMetadata}', getDevcontainerMetadataLabel(imageMetadata, common.experimentalImageMetadata)) .replace('#{containerEnvMetadata}', getContainerEnvMetadata(devContainerConfig.config.containerEnv)) ; @@ -388,31 +386,6 @@ export function findContainerUsers(imageMetadata: SubstitutedConfig[], contentSourceRootPath: string) { - return ([] as string[]).concat(...featuresConfig.featureSets - .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` - ) - ) - ).join('\n\n'); -} - -function getCopyFeatureBuildStages(featuresConfig: FeaturesConfig, buildStageScripts: Record[]) { - return ([] as string[]).concat(...featuresConfig.featureSets - .map((featureSet, i) => featureSet.features - .filter(f => (includeAllConfiguredFeatures || f.included) && f.value && buildStageScripts[i][f.id]?.hasAcquire) - .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` : ''}`; - }) - ) - ).join('\n\n'); -} function getFeatureEnvVariables(f: Feature) { const values = getFeatureValueObject(f); From e8b4b6fe90c74976b0a79caf2aedb9dca4caa2a7 Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Fri, 10 Feb 2023 00:09:37 +0000 Subject: [PATCH 29/41] cannot change line 256 --- src/spec-configuration/containerFeaturesConfiguration.ts | 6 +++--- src/spec-node/containerFeatures.ts | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/spec-configuration/containerFeaturesConfiguration.ts b/src/spec-configuration/containerFeaturesConfiguration.ts index e0cb7c5b8..f657b12aa 100644 --- a/src/spec-configuration/containerFeaturesConfiguration.ts +++ b/src/spec-configuration/containerFeaturesConfiguration.ts @@ -246,14 +246,14 @@ export function getContainerFeaturesBaseDockerFile(contentSourceRootPath: string FROM $_DEV_CONTAINERS_BASE_IMAGE AS dev_containers_feature_content_normalize USER root -COPY --from=dev_containers_feature_content_source ${contentSourceRootPath}/devcontainer-features.builtin.env /tmp/build-features/ +COPY --from=dev_containers_feature_content_source ${path.posix.join(contentSourceRootPath, 'devcontainer-features.builtin.env')} /tmp/build-features/ RUN chmod -R 0777 /tmp/build-features FROM $_DEV_CONTAINERS_BASE_IMAGE AS dev_containers_target_stage USER root -COPY --from=dev_containers_feature_content_normalize ${contentSourceRootPath} /tmp/build-features +COPY --from=dev_containers_feature_content_normalize /tmp/build-features /tmp/build-features #{featureLayer} @@ -330,7 +330,7 @@ function escapeQuotesForShell(input: string) { return input.replace(new RegExp(`'`, 'g'), `'\\''`); } -export function getFeatureLayers(featuresConfig: FeaturesConfig, containerUser: string, remoteUser: string, useBuildKitBuildContexts = false, contentSourceRootPath = '/tmp/build-features/') { +export function getFeatureLayers(featuresConfig: FeaturesConfig, containerUser: string, remoteUser: string, useBuildKitBuildContexts = false, contentSourceRootPath = '/tmp/build-features') { 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 diff --git a/src/spec-node/containerFeatures.ts b/src/spec-node/containerFeatures.ts index 0d352751b..63719f33c 100644 --- a/src/spec-node/containerFeatures.ts +++ b/src/spec-node/containerFeatures.ts @@ -284,7 +284,7 @@ async function getFeaturesBuildOptions(params: DockerResolverParameters, devCont // 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/'; + const contentSourceRootPath = useBuildKitBuildContexts ? '.' : '/tmp/build-features'; const dockerfile = getContainerFeaturesBaseDockerFile(contentSourceRootPath) .replace('#{nonBuildKitFeatureContentFallback}', useBuildKitBuildContexts ? '' : `FROM ${buildContentImageName} as dev_containers_feature_content_source`) .replace('#{featureLayer}', getFeatureLayers(featuresConfig, containerUser, remoteUser, useBuildKitBuildContexts, contentSourceRootPath)) From bff51a9792e72a59c8028ae9dcba8811bb6b73f9 Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Fri, 10 Feb 2023 16:05:44 +0000 Subject: [PATCH 30/41] persist final Features copy to /usr/share/devcontainer/features --- .../containerFeaturesConfiguration.ts | 45 ++++++++++--------- src/spec-node/containerFeatures.ts | 2 +- src/spec-node/imageMetadata.ts | 5 ++- .../generateFeaturesConfig.test.ts | 32 ++++++------- 4 files changed, 44 insertions(+), 40 deletions(-) diff --git a/src/spec-configuration/containerFeaturesConfiguration.ts b/src/spec-configuration/containerFeaturesConfiguration.ts index f657b12aa..89264e780 100644 --- a/src/spec-configuration/containerFeaturesConfiguration.ts +++ b/src/spec-configuration/containerFeaturesConfiguration.ts @@ -25,7 +25,7 @@ export const DEVCONTAINER_FEATURE_FILE_NAME = 'devcontainer-feature.json'; export type Feature = SchemaFeatureBaseProperties & SchemaFeatureLifecycleHooks & DeprecatedSchemaFeatureProperties & InternalFeatureProperties; -export const CONTAINER_BUILD_FEATURES_DIR = '/tmp/build-features'; +export const FEATURES_CONTAINER_DEST_FOLDER = '/usr/share/devcontainer/features'; export interface SchemaFeatureLifecycleHooks { onCreateCommand?: string | string[]; @@ -247,13 +247,14 @@ export function getContainerFeaturesBaseDockerFile(contentSourceRootPath: string FROM $_DEV_CONTAINERS_BASE_IMAGE AS dev_containers_feature_content_normalize USER root COPY --from=dev_containers_feature_content_source ${path.posix.join(contentSourceRootPath, 'devcontainer-features.builtin.env')} /tmp/build-features/ -RUN chmod -R 0777 /tmp/build-features +RUN chmod -R 0777 /tmp/build-features/ 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 +RUN mkdir -p ${FEATURES_CONTAINER_DEST_FOLDER} +COPY --from=dev_containers_feature_content_normalize /tmp/build-features/ ${FEATURES_CONTAINER_DEST_FOLDER} #{featureLayer} @@ -331,9 +332,11 @@ function escapeQuotesForShell(input: string) { } export function getFeatureLayers(featuresConfig: FeaturesConfig, containerUser: string, remoteUser: string, useBuildKitBuildContexts = false, contentSourceRootPath = '/tmp/build-features') { + + const builtinsEnvFile = `${path.posix.join(FEATURES_CONTAINER_DEST_FOLDER, 'devcontainer-features.builtin.env')}`; 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)" >> ${builtinsEnvFile} && \\ +echo "_REMOTE_USER_HOME=$(getent passwd ${remoteUser} | cut -d: -f6)" >> ${builtinsEnvFile} `; @@ -341,22 +344,22 @@ echo "_REMOTE_USER_HOME=$(getent passwd ${remoteUser} | cut -d: -f6)" >> /tmp/bu const folders = (featuresConfig.featureSets || []).filter(y => y.internalVersion !== '2').map(x => x.features[0].consecutiveId); folders.forEach(folder => { const source = path.posix.join(contentSourceRootPath, folder!); + const dest = path.posix.join(FEATURES_CONTAINER_DEST_FOLDER, folder!); if (!useBuildKitBuildContexts) { - result += `COPY --chown=root:root --from=dev_containers_feature_content_source ${source} /tmp/build-features/${folder} -RUN chmod -R 0777 /tmp/build-features/${folder} \\ -&& cd /tmp/build-features/${folder} \\ + result += `COPY --chown=root:root --from=dev_containers_feature_content_source ${source} ${dest} +RUN chmod -R 0777 ${dest} \\ +&& cd ${dest} \\ && chmod +x ./install.sh \\ && ./install.sh `; } else { result += `RUN --mount=type=bind,from=dev_containers_feature_content_source,source=${source},target=/tmp/build-features-src/${folder} \\ - cp -ar /tmp/build-features-src/${folder} /tmp/build-features/ \\ - && chmod -R 0777 /tmp/build-features/${folder} \\ - && cd /tmp/build-features/${folder} \\ + cp -ar /tmp/build-features-src/${folder} ${FEATURES_CONTAINER_DEST_FOLDER} \\ + && chmod -R 0777 ${dest} \\ + && cd ${dest} \\ && chmod +x ./install.sh \\ - && ./install.sh \\ - && rm -rf /tmp/build-features/${folder} + && ./install.sh `; } @@ -366,11 +369,12 @@ RUN chmod -R 0777 /tmp/build-features/${folder} \\ featureSet.features.forEach(feature => { result += generateContainerEnvs(feature); const source = path.posix.join(contentSourceRootPath, feature.consecutiveId!); + const dest = path.posix.join(FEATURES_CONTAINER_DEST_FOLDER, feature.consecutiveId!); if (!useBuildKitBuildContexts) { result += ` -COPY --chown=root:root --from=dev_containers_feature_content_source ${source} /tmp/build-features/${feature.consecutiveId} -RUN chmod -R 0777 /tmp/build-features/${feature.consecutiveId} \\ -&& cd /tmp/build-features/${feature.consecutiveId} \\ +COPY --chown=root:root --from=dev_containers_feature_content_source ${source} ${dest} +RUN chmod -R 0777 ${dest} \\ +&& cd ${dest} \\ && chmod +x ./devcontainer-features-install.sh \\ && ./devcontainer-features-install.sh @@ -378,12 +382,11 @@ RUN chmod -R 0777 /tmp/build-features/${feature.consecutiveId} \\ } else { result += ` RUN --mount=type=bind,from=dev_containers_feature_content_source,source=${source},target=/tmp/build-features-src/${feature.consecutiveId} \\ - cp -ar /tmp/build-features-src/${feature.consecutiveId} /tmp/build-features/ \\ - && chmod -R 0777 /tmp/build-features/${feature.consecutiveId} \\ - && cd /tmp/build-features/${feature.consecutiveId} \\ + cp -ar /tmp/build-features-src/${feature.consecutiveId} ${FEATURES_CONTAINER_DEST_FOLDER} \\ + && chmod -R 0777 ${dest} \\ + && cd ${dest} \\ && chmod +x ./devcontainer-features-install.sh \\ - && ./devcontainer-features-install.sh \\ - && rm -rf /tmp/build-features/${feature.consecutiveId} + && ./devcontainer-features-install.sh `; } diff --git a/src/spec-node/containerFeatures.ts b/src/spec-node/containerFeatures.ts index 63719f33c..0d352751b 100644 --- a/src/spec-node/containerFeatures.ts +++ b/src/spec-node/containerFeatures.ts @@ -284,7 +284,7 @@ async function getFeaturesBuildOptions(params: DockerResolverParameters, devCont // 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'; + const contentSourceRootPath = useBuildKitBuildContexts ? '.' : '/tmp/build-features/'; const dockerfile = getContainerFeaturesBaseDockerFile(contentSourceRootPath) .replace('#{nonBuildKitFeatureContentFallback}', useBuildKitBuildContexts ? '' : `FROM ${buildContentImageName} as dev_containers_feature_content_source`) .replace('#{featureLayer}', getFeatureLayers(featuresConfig, containerUser, remoteUser, useBuildKitBuildContexts, contentSourceRootPath)) diff --git a/src/spec-node/imageMetadata.ts b/src/spec-node/imageMetadata.ts index 8d1833305..5872f30b8 100644 --- a/src/spec-node/imageMetadata.ts +++ b/src/spec-node/imageMetadata.ts @@ -3,11 +3,12 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import path from 'path'; import { ContainerError } from '../spec-common/errors'; import { LifecycleCommand, LifecycleHooksInstallMap } from '../spec-common/injectHeadless'; import { substituteFeatureRoot } from '../spec-common/variableSubstitution'; import { DevContainerConfig, DevContainerConfigCommand, DevContainerFromDockerComposeConfig, DevContainerFromDockerfileConfig, DevContainerFromImageConfig, getDockerComposeFilePaths, getDockerfilePath, HostGPURequirements, HostRequirements, isDockerFileConfig, PortAttributes, UserEnvProbe } from '../spec-configuration/configuration'; -import { Feature, FeaturesConfig, Mount, parseMount, SchemaFeatureLifecycleHooks } from '../spec-configuration/containerFeaturesConfiguration'; +import { Feature, FeaturesConfig, FEATURES_CONTAINER_DEST_FOLDER, Mount, parseMount, SchemaFeatureLifecycleHooks } from '../spec-configuration/containerFeaturesConfiguration'; import { ContainerDetails, DockerCLIParameters, ImageDetails } from '../spec-shutdown/dockerUtils'; import { Log } from '../spec-utils/log'; import { getBuildInfoForService, readDockerComposeConfig } from './dockerCompose'; @@ -286,7 +287,7 @@ export function getDevcontainerMetadata(baseImageMetadata: SubstitutedConfig featureSet.features.forEach(f => { pickFeatureLifecycleHookProperties.forEach(hook => { - const buildPath = `/tmp/build-features/${f.consecutiveId}`; + const buildPath = path.posix.join(FEATURES_CONTAINER_DEST_FOLDER, f.consecutiveId!); if (f[hook]) { f[hook] = substituteFeatureRoot(f[hook], buildPath); } diff --git a/src/test/container-features/generateFeaturesConfig.test.ts b/src/test/container-features/generateFeaturesConfig.test.ts index ce9916145..939994a2d 100644 --- a/src/test/container-features/generateFeaturesConfig.test.ts +++ b/src/test/container-features/generateFeaturesConfig.test.ts @@ -72,18 +72,18 @@ describe('validate generateFeaturesConfig()', function () { // getFeatureLayers const actualLayers = getFeatureLayers(featuresConfig, 'testContainerUser', 'testRemoteUser'); const expectedLayers = `RUN \\ -echo "_CONTAINER_USER_HOME=$(getent passwd testContainerUser | cut -d: -f6)" >> /tmp/build-features/devcontainer-features.builtin.env && \\ -echo "_REMOTE_USER_HOME=$(getent passwd testRemoteUser | cut -d: -f6)" >> /tmp/build-features/devcontainer-features.builtin.env +echo "_CONTAINER_USER_HOME=$(getent passwd testContainerUser | cut -d: -f6)" >> /usr/share/devcontainer/features/devcontainer-features.builtin.env && \\ +echo "_REMOTE_USER_HOME=$(getent passwd testRemoteUser | cut -d: -f6)" >> /usr/share/devcontainer/features/devcontainer-features.builtin.env -COPY --chown=root:root --from=dev_containers_feature_content_source /tmp/build-features/first_1 /tmp/build-features/first_1 -RUN chmod -R 0777 /tmp/build-features/first_1 \\ -&& cd /tmp/build-features/first_1 \\ +COPY --chown=root:root --from=dev_containers_feature_content_source /tmp/build-features/first_1 /usr/share/devcontainer/features/first_1 +RUN chmod -R 0777 /usr/share/devcontainer/features/first_1 \\ +&& cd /usr/share/devcontainer/features/first_1 \\ && chmod +x ./install.sh \\ && ./install.sh -COPY --chown=root:root --from=dev_containers_feature_content_source /tmp/build-features/second_2 /tmp/build-features/second_2 -RUN chmod -R 0777 /tmp/build-features/second_2 \\ -&& cd /tmp/build-features/second_2 \\ +COPY --chown=root:root --from=dev_containers_feature_content_source /tmp/build-features/second_2 /usr/share/devcontainer/features/second_2 +RUN chmod -R 0777 /usr/share/devcontainer/features/second_2 \\ +&& cd /usr/share/devcontainer/features/second_2 \\ && chmod +x ./install.sh \\ && ./install.sh @@ -134,20 +134,20 @@ RUN chmod -R 0777 /tmp/build-features/second_2 \\ // getFeatureLayers const actualLayers = getFeatureLayers(featuresConfig, 'testContainerUser', 'testRemoteUser'); const expectedLayers = `RUN \\ -echo "_CONTAINER_USER_HOME=$(getent passwd testContainerUser | cut -d: -f6)" >> /tmp/build-features/devcontainer-features.builtin.env && \\ -echo "_REMOTE_USER_HOME=$(getent passwd testRemoteUser | cut -d: -f6)" >> /tmp/build-features/devcontainer-features.builtin.env +echo "_CONTAINER_USER_HOME=$(getent passwd testContainerUser | cut -d: -f6)" >> /usr/share/devcontainer/features/devcontainer-features.builtin.env && \\ +echo "_REMOTE_USER_HOME=$(getent passwd testRemoteUser | cut -d: -f6)" >> /usr/share/devcontainer/features/devcontainer-features.builtin.env -COPY --chown=root:root --from=dev_containers_feature_content_source /tmp/build-features/color_3 /tmp/build-features/color_3 -RUN chmod -R 0777 /tmp/build-features/color_3 \\ -&& cd /tmp/build-features/color_3 \\ +COPY --chown=root:root --from=dev_containers_feature_content_source /tmp/build-features/color_3 /usr/share/devcontainer/features/color_3 +RUN chmod -R 0777 /usr/share/devcontainer/features/color_3 \\ +&& cd /usr/share/devcontainer/features/color_3 \\ && chmod +x ./devcontainer-features-install.sh \\ && ./devcontainer-features-install.sh -COPY --chown=root:root --from=dev_containers_feature_content_source /tmp/build-features/hello_4 /tmp/build-features/hello_4 -RUN chmod -R 0777 /tmp/build-features/hello_4 \\ -&& cd /tmp/build-features/hello_4 \\ +COPY --chown=root:root --from=dev_containers_feature_content_source /tmp/build-features/hello_4 /usr/share/devcontainer/features/hello_4 +RUN chmod -R 0777 /usr/share/devcontainer/features/hello_4 \\ +&& cd /usr/share/devcontainer/features/hello_4 \\ && chmod +x ./devcontainer-features-install.sh \\ && ./devcontainer-features-install.sh From 07ad411bdd1dbf3f1fbfbfacb6663b9ef1b91745 Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Fri, 10 Feb 2023 18:58:05 +0000 Subject: [PATCH 31/41] update existing test --- src/spec-configuration/containerFeaturesConfiguration.ts | 5 +++++ src/test/imageMetadata.test.ts | 2 ++ 2 files changed, 7 insertions(+) diff --git a/src/spec-configuration/containerFeaturesConfiguration.ts b/src/spec-configuration/containerFeaturesConfiguration.ts index 89264e780..92d41bad3 100644 --- a/src/spec-configuration/containerFeaturesConfiguration.ts +++ b/src/spec-configuration/containerFeaturesConfiguration.ts @@ -959,6 +959,11 @@ async function fetchFeatures(params: { extensionPath: string; cwd: string; outpu feature.cachePath = featCachePath; feature.consecutiveId = consecutiveId; + if (!feature.consecutiveId || !feature.id || !featureSet?.sourceInformation || !featureSet.sourceInformation.userFeatureId) { + const err = "Internal Features error. Missing required attribute(s)." + throw new Error(err); + } + const featureDebugId = `${feature.consecutiveId}_${sourceInfoType}`; output.write(`* Fetching feature: ${featureDebugId}`); diff --git a/src/test/imageMetadata.test.ts b/src/test/imageMetadata.test.ts index 07891452e..666b02d6c 100644 --- a/src/test/imageMetadata.test.ts +++ b/src/test/imageMetadata.test.ts @@ -190,6 +190,7 @@ describe('Image Metadata', function () { id: 'someFeature', value: 'someValue', included: true, + consecutiveId: 'someFeature_1', } ])); assert.strictEqual(metadata.length, 2); @@ -348,6 +349,7 @@ describe('Image Metadata', function () { id: 'someFeature', value: 'someValue', included: true, + consecutiveId: 'someFeature_1', } ])), true); const expected = [ From 99836c3c6ed786b42ba67b466cb107f1a7ea99e1 Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Fri, 10 Feb 2023 19:08:34 +0000 Subject: [PATCH 32/41] feature dir need not be writable by anyone but its owner --- .../containerFeaturesConfiguration.ts | 12 ++++++------ .../generateFeaturesConfig.test.ts | 8 ++++---- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/spec-configuration/containerFeaturesConfiguration.ts b/src/spec-configuration/containerFeaturesConfiguration.ts index 92d41bad3..bab63a17e 100644 --- a/src/spec-configuration/containerFeaturesConfiguration.ts +++ b/src/spec-configuration/containerFeaturesConfiguration.ts @@ -247,7 +247,7 @@ export function getContainerFeaturesBaseDockerFile(contentSourceRootPath: string FROM $_DEV_CONTAINERS_BASE_IMAGE AS dev_containers_feature_content_normalize USER root COPY --from=dev_containers_feature_content_source ${path.posix.join(contentSourceRootPath, 'devcontainer-features.builtin.env')} /tmp/build-features/ -RUN chmod -R 0777 /tmp/build-features/ +RUN chmod -R 0755 /tmp/build-features/ FROM $_DEV_CONTAINERS_BASE_IMAGE AS dev_containers_target_stage @@ -347,7 +347,7 @@ echo "_REMOTE_USER_HOME=$(getent passwd ${remoteUser} | cut -d: -f6)" >> ${built const dest = path.posix.join(FEATURES_CONTAINER_DEST_FOLDER, folder!); if (!useBuildKitBuildContexts) { result += `COPY --chown=root:root --from=dev_containers_feature_content_source ${source} ${dest} -RUN chmod -R 0777 ${dest} \\ +RUN chmod -R 0755 ${dest} \\ && cd ${dest} \\ && chmod +x ./install.sh \\ && ./install.sh @@ -356,7 +356,7 @@ RUN chmod -R 0777 ${dest} \\ } else { result += `RUN --mount=type=bind,from=dev_containers_feature_content_source,source=${source},target=/tmp/build-features-src/${folder} \\ cp -ar /tmp/build-features-src/${folder} ${FEATURES_CONTAINER_DEST_FOLDER} \\ - && chmod -R 0777 ${dest} \\ + && chmod -R 0755 ${dest} \\ && cd ${dest} \\ && chmod +x ./install.sh \\ && ./install.sh @@ -373,7 +373,7 @@ RUN chmod -R 0777 ${dest} \\ if (!useBuildKitBuildContexts) { result += ` COPY --chown=root:root --from=dev_containers_feature_content_source ${source} ${dest} -RUN chmod -R 0777 ${dest} \\ +RUN chmod -R 0755 ${dest} \\ && cd ${dest} \\ && chmod +x ./devcontainer-features-install.sh \\ && ./devcontainer-features-install.sh @@ -383,7 +383,7 @@ RUN chmod -R 0777 ${dest} \\ result += ` RUN --mount=type=bind,from=dev_containers_feature_content_source,source=${source},target=/tmp/build-features-src/${feature.consecutiveId} \\ cp -ar /tmp/build-features-src/${feature.consecutiveId} ${FEATURES_CONTAINER_DEST_FOLDER} \\ - && chmod -R 0777 ${dest} \\ + && chmod -R 0755 ${dest} \\ && cd ${dest} \\ && chmod +x ./devcontainer-features-install.sh \\ && ./devcontainer-features-install.sh @@ -960,7 +960,7 @@ async function fetchFeatures(params: { extensionPath: string; cwd: string; outpu feature.consecutiveId = consecutiveId; if (!feature.consecutiveId || !feature.id || !featureSet?.sourceInformation || !featureSet.sourceInformation.userFeatureId) { - const err = "Internal Features error. Missing required attribute(s)." + const err = 'Internal Features error. Missing required attribute(s).'; throw new Error(err); } diff --git a/src/test/container-features/generateFeaturesConfig.test.ts b/src/test/container-features/generateFeaturesConfig.test.ts index 939994a2d..18fdefa05 100644 --- a/src/test/container-features/generateFeaturesConfig.test.ts +++ b/src/test/container-features/generateFeaturesConfig.test.ts @@ -76,13 +76,13 @@ echo "_CONTAINER_USER_HOME=$(getent passwd testContainerUser | cut -d: -f6)" >> echo "_REMOTE_USER_HOME=$(getent passwd testRemoteUser | cut -d: -f6)" >> /usr/share/devcontainer/features/devcontainer-features.builtin.env COPY --chown=root:root --from=dev_containers_feature_content_source /tmp/build-features/first_1 /usr/share/devcontainer/features/first_1 -RUN chmod -R 0777 /usr/share/devcontainer/features/first_1 \\ +RUN chmod -R 0755 /usr/share/devcontainer/features/first_1 \\ && cd /usr/share/devcontainer/features/first_1 \\ && chmod +x ./install.sh \\ && ./install.sh COPY --chown=root:root --from=dev_containers_feature_content_source /tmp/build-features/second_2 /usr/share/devcontainer/features/second_2 -RUN chmod -R 0777 /usr/share/devcontainer/features/second_2 \\ +RUN chmod -R 0755 /usr/share/devcontainer/features/second_2 \\ && cd /usr/share/devcontainer/features/second_2 \\ && chmod +x ./install.sh \\ && ./install.sh @@ -139,14 +139,14 @@ echo "_REMOTE_USER_HOME=$(getent passwd testRemoteUser | cut -d: -f6)" >> /usr/s COPY --chown=root:root --from=dev_containers_feature_content_source /tmp/build-features/color_3 /usr/share/devcontainer/features/color_3 -RUN chmod -R 0777 /usr/share/devcontainer/features/color_3 \\ +RUN chmod -R 0755 /usr/share/devcontainer/features/color_3 \\ && cd /usr/share/devcontainer/features/color_3 \\ && chmod +x ./devcontainer-features-install.sh \\ && ./devcontainer-features-install.sh COPY --chown=root:root --from=dev_containers_feature_content_source /tmp/build-features/hello_4 /usr/share/devcontainer/features/hello_4 -RUN chmod -R 0777 /usr/share/devcontainer/features/hello_4 \\ +RUN chmod -R 0755 /usr/share/devcontainer/features/hello_4 \\ && cd /usr/share/devcontainer/features/hello_4 \\ && chmod +x ./devcontainer-features-install.sh \\ && ./devcontainer-features-install.sh From 2e2731f430149e1995c12b0a9c3ce2b3d1744ae6 Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Fri, 10 Feb 2023 21:36:56 +0000 Subject: [PATCH 33/41] add test to assert resume will trigger lifecycle hooks --- .../.devcontainer/createMarker.sh | 3 +- .../container-features/lifecycleHooks.test.ts | 80 +++++++++++++------ src/test/testUtils.ts | 6 +- 3 files changed, 60 insertions(+), 29 deletions(-) diff --git a/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/createMarker.sh b/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/createMarker.sh index c8146b8d8..d6533e732 100755 --- a/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/createMarker.sh +++ b/src/test/container-features/configs/lifecycle-hooks-inline-commands/.devcontainer/createMarker.sh @@ -10,5 +10,4 @@ n=$(< saved_value.testMarker) echo "${n}.`date +%s%3N`" > "${n}.${MARKER_FILE_NAME}" echo $(( n + 1 )) > saved_value.testMarker -echo "Ending '${MARKER_FILE_NAME}'...." -sleep 1s \ No newline at end of file +echo "Ending '${MARKER_FILE_NAME}'...." \ No newline at end of file diff --git a/src/test/container-features/lifecycleHooks.test.ts b/src/test/container-features/lifecycleHooks.test.ts index e6eb706bc..1fc66995b 100644 --- a/src/test/container-features/lifecycleHooks.test.ts +++ b/src/test/container-features/lifecycleHooks.test.ts @@ -6,7 +6,7 @@ import { assert } from 'chai'; import * as path from 'path'; import { Feature } from '../../spec-configuration/containerFeaturesConfiguration'; -import { devContainerDown, devContainerUp, shellExec } from '../testUtils'; +import { devContainerDown, devContainerStop, devContainerUp, shellExec } from '../testUtils'; const pkg = require('../../../package.json'); @@ -38,31 +38,63 @@ describe('Feature lifecycle hooks', function () { await shellExec(`rm -f ${testFolder}/*.testMarker`, undefined, undefined, true); }); - it('marker files should exist and executed in stable order', async () => { - const res = await shellExec(`${cli} exec --workspace-folder ${testFolder} ls -altr`); - const response = JSON.parse(res.stdout); - assert.equal(response.outcome, 'success'); + it('marker files should exist, be executed in stable order, and hooks should postStart/attach should trigger on a resume', async () => { + + { + const res = await shellExec(`${cli} exec --workspace-folder ${testFolder} ls -altr`); + const response = JSON.parse(res.stdout); + assert.equal(response.outcome, 'success'); + + const outputOfExecCommand = res.stderr; + console.log(outputOfExecCommand); + + assert.match(outputOfExecCommand, /0.panda.onCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /3.panda.updateContentCommand.testMarker/); + assert.match(outputOfExecCommand, /6.panda.postCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /9.panda.postStartCommand.testMarker/); + assert.match(outputOfExecCommand, /12.panda.postAttachCommand.testMarker/); + + assert.match(outputOfExecCommand, /1.tiger.onCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /4.tiger.updateContentCommand.testMarker/); + assert.match(outputOfExecCommand, /7.tiger.postCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /10.tiger.postStartCommand.testMarker/); + assert.match(outputOfExecCommand, /13.tiger.postAttachCommand.testMarker/); + + assert.match(outputOfExecCommand, /2.devContainer.onCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /5.devContainer.updateContentCommand.testMarker/); + assert.match(outputOfExecCommand, /8.devContainer.postCreateCommand.testMarker/); + assert.match(outputOfExecCommand, /11.devContainer.postStartCommand.testMarker/); + assert.match(outputOfExecCommand, /14.devContainer.postAttachCommand.testMarker/); + + // This shouldn't have happened _yet_. + assert.notMatch(outputOfExecCommand, /15.panda.postStartCommand.testMarker/); + } + + // Stop the container. + await devContainerStop({ containerId }); + + { + // Attempt to bring the same container up, which should just re-run the postStart and postAttach hooks + const resume = await devContainerUp(cli, testFolder, { logLevel: 'trace' }); + assert.equal(resume.containerId, containerId); // Restarting the same container. + assert.equal(resume.outcome, 'success'); + + const res = await shellExec(`${cli} exec --workspace-folder ${testFolder} ls -altr`); + const response = JSON.parse(res.stdout); + assert.equal(response.outcome, 'success'); + + const outputOfExecCommand = res.stderr; + console.log(outputOfExecCommand); + + assert.match(outputOfExecCommand, /15.panda.postStartCommand.testMarker/); + assert.match(outputOfExecCommand, /16.tiger.postStartCommand.testMarker/); + assert.match(outputOfExecCommand, /17.devContainer.postStartCommand.testMarker/); + assert.match(outputOfExecCommand, /18.panda.postAttachCommand.testMarker/); + assert.match(outputOfExecCommand, /19.tiger.postAttachCommand.testMarker/); + assert.match(outputOfExecCommand, /20.devContainer.postAttachCommand.testMarker/); + } - const outputOfExecCommand = res.stderr; - console.log(outputOfExecCommand); - assert.match(outputOfExecCommand, /0.panda.onCreateCommand.testMarker/); - assert.match(outputOfExecCommand, /3.panda.updateContentCommand.testMarker/); - assert.match(outputOfExecCommand, /6.panda.postCreateCommand.testMarker/); - assert.match(outputOfExecCommand, /9.panda.postStartCommand.testMarker/); - assert.match(outputOfExecCommand, /12.panda.postAttachCommand.testMarker/); - - assert.match(outputOfExecCommand, /1.tiger.onCreateCommand.testMarker/); - assert.match(outputOfExecCommand, /4.tiger.updateContentCommand.testMarker/); - assert.match(outputOfExecCommand, /7.tiger.postCreateCommand.testMarker/); - assert.match(outputOfExecCommand, /10.tiger.postStartCommand.testMarker/); - assert.match(outputOfExecCommand, /13.tiger.postAttachCommand.testMarker/); - - assert.match(outputOfExecCommand, /2.devContainer.onCreateCommand.testMarker/); - assert.match(outputOfExecCommand, /5.devContainer.updateContentCommand.testMarker/); - assert.match(outputOfExecCommand, /8.devContainer.postCreateCommand.testMarker/); - assert.match(outputOfExecCommand, /11.devContainer.postStartCommand.testMarker/); - assert.match(outputOfExecCommand, /14.devContainer.postAttachCommand.testMarker/); }); }); }); diff --git a/src/test/testUtils.ts b/src/test/testUtils.ts index 7d638a918..88eeebc85 100644 --- a/src/test/testUtils.ts +++ b/src/test/testUtils.ts @@ -55,12 +55,12 @@ export async function devContainerUp(cli: string, workspaceFolder: string, optio assert.ok(containerId, 'Container id not found.'); return { outcome, containerId, composeProjectName, stderr: res.stderr }; } -export async function devContainerDown(options: { containerId?: string | null; composeProjectName?: string | null }) { +export async function devContainerDown(options: { containerId?: string | null; composeProjectName?: string | null, doNotThrow?: boolean }) { if (options.containerId) { - await shellExec(`docker rm -f ${options.containerId}`); + await shellExec(`docker rm -f ${options.containerId}`, undefined, undefined, options.doNotThrow); } if (options.composeProjectName) { - await shellExec(`docker compose --project-name ${options.composeProjectName} down`); + await shellExec(`docker compose --project-name ${options.composeProjectName} down`, undefined, undefined, options.doNotThrow); } } export async function devContainerStop(options: { containerId?: string | null; composeProjectName?: string | null }) { From 4c37690634723cecf1a812d3754abd992935bd85 Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Fri, 10 Feb 2023 21:52:22 +0000 Subject: [PATCH 34/41] Test: "${featureRootFolder} substituted lifecycle hooks trigger on resume --- .../.devcontainer/Dockerfile | 1 + .../.devcontainer/devcontainer.json | 8 ++++ .../.devcontainer/hippo/createMarker.sh | 13 +++++++ .../hippo/devcontainer-feature.json | 8 ++++ .../.devcontainer/hippo/install.sh | 12 ++++++ .../container-features/lifecycleHooks.test.ts | 38 +++++++++++++++++++ 6 files changed, 80 insertions(+) create mode 100644 src/test/container-features/configs/lifecycle-hooks-resume-existing-container/.devcontainer/Dockerfile create mode 100644 src/test/container-features/configs/lifecycle-hooks-resume-existing-container/.devcontainer/devcontainer.json create mode 100755 src/test/container-features/configs/lifecycle-hooks-resume-existing-container/.devcontainer/hippo/createMarker.sh create mode 100644 src/test/container-features/configs/lifecycle-hooks-resume-existing-container/.devcontainer/hippo/devcontainer-feature.json create mode 100644 src/test/container-features/configs/lifecycle-hooks-resume-existing-container/.devcontainer/hippo/install.sh diff --git a/src/test/container-features/configs/lifecycle-hooks-resume-existing-container/.devcontainer/Dockerfile b/src/test/container-features/configs/lifecycle-hooks-resume-existing-container/.devcontainer/Dockerfile new file mode 100644 index 000000000..c286a4279 --- /dev/null +++ b/src/test/container-features/configs/lifecycle-hooks-resume-existing-container/.devcontainer/Dockerfile @@ -0,0 +1 @@ +FROM mcr.microsoft.com/devcontainers/base:ubuntu \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-resume-existing-container/.devcontainer/devcontainer.json b/src/test/container-features/configs/lifecycle-hooks-resume-existing-container/.devcontainer/devcontainer.json new file mode 100644 index 000000000..ab00a985a --- /dev/null +++ b/src/test/container-features/configs/lifecycle-hooks-resume-existing-container/.devcontainer/devcontainer.json @@ -0,0 +1,8 @@ +{ + "build": { + "dockerfile": "Dockerfile" + }, + "features": { + "./hippo": {} + } +} \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-resume-existing-container/.devcontainer/hippo/createMarker.sh b/src/test/container-features/configs/lifecycle-hooks-resume-existing-container/.devcontainer/hippo/createMarker.sh new file mode 100755 index 000000000..d6533e732 --- /dev/null +++ b/src/test/container-features/configs/lifecycle-hooks-resume-existing-container/.devcontainer/hippo/createMarker.sh @@ -0,0 +1,13 @@ +#!/bin/bash + +MARKER_FILE_NAME="$1" + +echo "Starting '${MARKER_FILE_NAME}'...." +sleep 1s + +[[ -f saved_value.testMarker ]] || echo 0 > saved_value.testMarker +n=$(< saved_value.testMarker) +echo "${n}.`date +%s%3N`" > "${n}.${MARKER_FILE_NAME}" +echo $(( n + 1 )) > saved_value.testMarker + +echo "Ending '${MARKER_FILE_NAME}'...." \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-resume-existing-container/.devcontainer/hippo/devcontainer-feature.json b/src/test/container-features/configs/lifecycle-hooks-resume-existing-container/.devcontainer/hippo/devcontainer-feature.json new file mode 100644 index 000000000..2340db9e9 --- /dev/null +++ b/src/test/container-features/configs/lifecycle-hooks-resume-existing-container/.devcontainer/hippo/devcontainer-feature.json @@ -0,0 +1,8 @@ +{ + "id": "hippo", + "version": "1.0.1", + "name": "Hippo", + "options": {}, + "postStartCommand": "${featureRootFolder}/createMarker.sh hippo.postStartCommand.testMarker", + "postAttachCommand": "${featureRootFolder}/createMarker.sh hippo.postAttachCommand.testMarker" +} \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-resume-existing-container/.devcontainer/hippo/install.sh b/src/test/container-features/configs/lifecycle-hooks-resume-existing-container/.devcontainer/hippo/install.sh new file mode 100644 index 000000000..7a3c83ced --- /dev/null +++ b/src/test/container-features/configs/lifecycle-hooks-resume-existing-container/.devcontainer/hippo/install.sh @@ -0,0 +1,12 @@ +#!/bin/sh +set -e + +echo "Activating feature 'hippo'" + +cat > /usr/local/bin/hippo \ +<< EOF +#!/bin/sh +echo 🦛 +EOF + +chmod +x /usr/local/bin/hippo \ No newline at end of file diff --git a/src/test/container-features/lifecycleHooks.test.ts b/src/test/container-features/lifecycleHooks.test.ts index 1fc66995b..0738749ec 100644 --- a/src/test/container-features/lifecycleHooks.test.ts +++ b/src/test/container-features/lifecycleHooks.test.ts @@ -160,6 +160,44 @@ describe('Feature lifecycle hooks', function () { }); + describe('lifecycle-hooks-resume-existing-container', () => { + let containerId: string | null = null; + const testFolder = `${__dirname}/configs/lifecycle-hooks-resume-existing-container`; + + // Clean up + before(async () => { + await shellExec(`rm -f ${testFolder}/*.testMarker`, undefined, undefined, true); + containerId = (await devContainerUp(cli, testFolder, { 'logLevel': 'trace' })).containerId; + }); + + // Ensure clean after running. + after(async () => { + await devContainerDown({ containerId, doNotThrow: true }); + await shellExec(`rm -f ${testFolder}/*.testMarker`, undefined, undefined, true); + }); + + it('the appropriate lifecycle hooks are executed when resuming an existing container', async () => { + + await devContainerStop({ containerId }); + // Attempt to bring the same container up, which should just re-run the postStart and postAttach hooks + const resume = await devContainerUp(cli, testFolder, { logLevel: 'trace' }); + assert.equal(resume.containerId, containerId); // Restarting the same container. + assert.equal(resume.outcome, 'success'); + + const res = await shellExec(`${cli} exec --workspace-folder ${testFolder} ls -altr`); + const response = JSON.parse(res.stdout); + assert.equal(response.outcome, 'success'); + + const outputOfExecCommand = res.stderr; + console.log(outputOfExecCommand); + + assert.match(outputOfExecCommand, /0.hippo.postStartCommand.testMarker/); + assert.match(outputOfExecCommand, /1.hippo.postAttachCommand.testMarker/); + assert.match(outputOfExecCommand, /2.hippo.postStartCommand.testMarker/); + assert.match(outputOfExecCommand, /3.hippo.postAttachCommand.testMarker/); + }); + }); + describe('lifecycle-hooks-advanced', () => { describe(`devcontainer up`, () => { From 2adef08006b92c94586e1d07cd6421fbe4a1063f Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Fri, 10 Feb 2023 22:00:40 +0000 Subject: [PATCH 35/41] missing semicolon --- src/test/testUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/testUtils.ts b/src/test/testUtils.ts index 88eeebc85..5c116822e 100644 --- a/src/test/testUtils.ts +++ b/src/test/testUtils.ts @@ -55,7 +55,7 @@ export async function devContainerUp(cli: string, workspaceFolder: string, optio assert.ok(containerId, 'Container id not found.'); return { outcome, containerId, composeProjectName, stderr: res.stderr }; } -export async function devContainerDown(options: { containerId?: string | null; composeProjectName?: string | null, doNotThrow?: boolean }) { +export async function devContainerDown(options: { containerId?: string | null; composeProjectName?: string | null; doNotThrow?: boolean }) { if (options.containerId) { await shellExec(`docker rm -f ${options.containerId}`, undefined, undefined, options.doNotThrow); } From 59c1f4adda39f0ea374cbb998d99ec50eb33920e Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Mon, 13 Feb 2023 18:37:51 +0000 Subject: [PATCH 36/41] preserve featureRootFolder in metadata and replace right before usage --- src/spec-common/variableSubstitution.ts | 4 +- src/spec-node/imageMetadata.ts | 54 ++++++++++++++++--------- 2 files changed, 36 insertions(+), 22 deletions(-) diff --git a/src/spec-common/variableSubstitution.ts b/src/spec-common/variableSubstitution.ts index 86581a22f..48b86d085 100644 --- a/src/spec-common/variableSubstitution.ts +++ b/src/spec-common/variableSubstitution.ts @@ -8,6 +8,7 @@ import * as crypto from 'crypto'; import { ContainerError } from './errors'; import { URI } from 'vscode-uri'; +import { LifecycleCommand } from './injectHeadless'; export interface SubstitutionContext { platform: NodeJS.Platform; @@ -180,10 +181,9 @@ function devcontainerIdForLabels(idLabels: Record): string { return uniqueId; } -export function substituteFeatureRoot(lifecycleHook: string | string[] | undefined, cachePath: string | undefined): string | string[] | undefined { +export function substituteFeatureRoot(lifecycleHook: LifecycleCommand | undefined, cachePath: string | undefined): LifecycleCommand | undefined { if (!lifecycleHook || !cachePath) { return lifecycleHook; } - return substitute0(replaceFeatureRoot.bind(undefined, cachePath), lifecycleHook); } \ No newline at end of file diff --git a/src/spec-node/imageMetadata.ts b/src/spec-node/imageMetadata.ts index 5872f30b8..403623cd4 100644 --- a/src/spec-node/imageMetadata.ts +++ b/src/spec-node/imageMetadata.ts @@ -69,6 +69,7 @@ const pickFeatureProperties: Exclude(entries: T[], property: K): No return values.length ? values : undefined; } +function cacheFeatureContainerDestFolder(imageMetadata: ImageMetadataEntry[], featuresConfig?: FeaturesConfig): ImageMetadataEntry[] { + if (!featuresConfig) { + return imageMetadata; + } + + imageMetadata.map(entry => { + const id = entry.id; + if (!id) { + return; + } + + const targetFeatureSet = featuresConfig?.featureSets.find(fs => fs.sourceInformation.userFeatureId === id); + if (!targetFeatureSet) { + return; + } + + const buildPath = path.posix.join(FEATURES_CONTAINER_DEST_FOLDER, targetFeatureSet.features[0].consecutiveId!); + entry.featureContainerDestFolder = buildPath; + }); + return imageMetadata; +} + export function getDevcontainerMetadata(baseImageMetadata: SubstitutedConfig, devContainerConfig: SubstitutedConfig, featuresConfig: FeaturesConfig | undefined, omitPropertyOverride: string[] = []): SubstitutedConfig { const effectivePickFeatureProperties = pickFeatureProperties.filter(property => !omitPropertyOverride.includes(property)); - // Variable substitution - // eslint-disable-next-line code-no-unused-expressions - featuresConfig?.featureSets.forEach(featureSet => - featureSet.features.forEach(f => { - pickFeatureLifecycleHookProperties.forEach(hook => { - const buildPath = path.posix.join(FEATURES_CONTAINER_DEST_FOLDER, f.consecutiveId!); - if (f[hook]) { - f[hook] = substituteFeatureRoot(f[hook], buildPath); - } - }); - })); - - const raw = featuresConfig?.featureSets.map(featureSet => + const pickedRaw = featuresConfig?.featureSets.map(featureSet => featureSet.features.map(feature => ({ id: featureSet.sourceInformation.userFeatureId, ...pick(feature, effectivePickFeatureProperties), }))).flat() || []; + const raw = cacheFeatureContainerDestFolder([ + ...baseImageMetadata.raw, + ...pickedRaw, + pick(devContainerConfig.raw, pickConfigProperties), + ].filter(config => Object.keys(config).length), featuresConfig); + return { config: [ ...baseImageMetadata.config, ...raw.map(devContainerConfig.substitute), pick(devContainerConfig.config, pickConfigProperties), ].filter(config => Object.keys(config).length), - raw: [ - ...baseImageMetadata.raw, - ...raw, - pick(devContainerConfig.raw, pickConfigProperties), - ].filter(config => Object.keys(config).length), + raw, substitute: devContainerConfig.substitute, }; } From cd87a51a668cf12111ca5a62c5b6d538cf91747e Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Mon, 13 Feb 2023 19:36:23 +0000 Subject: [PATCH 37/41] rename metadata entry to featureRootFolder for clarity and only emit in featureRaw --- src/spec-node/imageMetadata.ts | 43 +++++++++------------------------- src/test/imageMetadata.test.ts | 1 + 2 files changed, 12 insertions(+), 32 deletions(-) diff --git a/src/spec-node/imageMetadata.ts b/src/spec-node/imageMetadata.ts index 403623cd4..0dbae04df 100644 --- a/src/spec-node/imageMetadata.ts +++ b/src/spec-node/imageMetadata.ts @@ -69,7 +69,7 @@ const pickFeatureProperties: Exclude(entries: T[], property: K): No return values.length ? values : undefined; } -function cacheFeatureContainerDestFolder(imageMetadata: ImageMetadataEntry[], featuresConfig?: FeaturesConfig): ImageMetadataEntry[] { - if (!featuresConfig) { - return imageMetadata; - } - - imageMetadata.map(entry => { - const id = entry.id; - if (!id) { - return; - } - - const targetFeatureSet = featuresConfig?.featureSets.find(fs => fs.sourceInformation.userFeatureId === id); - if (!targetFeatureSet) { - return; - } - - const buildPath = path.posix.join(FEATURES_CONTAINER_DEST_FOLDER, targetFeatureSet.features[0].consecutiveId!); - entry.featureContainerDestFolder = buildPath; - }); - return imageMetadata; -} - export function getDevcontainerMetadata(baseImageMetadata: SubstitutedConfig, devContainerConfig: SubstitutedConfig, featuresConfig: FeaturesConfig | undefined, omitPropertyOverride: string[] = []): SubstitutedConfig { const effectivePickFeatureProperties = pickFeatureProperties.filter(property => !omitPropertyOverride.includes(property)); - const pickedRaw = featuresConfig?.featureSets.map(featureSet => + const featureRaw = featuresConfig?.featureSets.map(featureSet => featureSet.features.map(feature => ({ - id: featureSet.sourceInformation.userFeatureId, - ...pick(feature, effectivePickFeatureProperties), - }))).flat() || []; + id: featureSet.sourceInformation.userFeatureId, + featureRootFolder: featureSet ? path.posix.join(FEATURES_CONTAINER_DEST_FOLDER, featureSet.features[0].consecutiveId!) : undefined, + ...pick(feature, effectivePickFeatureProperties), + }))).flat() || []; - const raw = cacheFeatureContainerDestFolder([ + const raw = [ ...baseImageMetadata.raw, - ...pickedRaw, + ...featureRaw, pick(devContainerConfig.raw, pickConfigProperties), - ].filter(config => Object.keys(config).length), featuresConfig); + ].filter(config => Object.keys(config).length); return { config: [ ...baseImageMetadata.config, - ...raw.map(devContainerConfig.substitute), + ...featureRaw.map(devContainerConfig.substitute), pick(devContainerConfig.config, pickConfigProperties), ].filter(config => Object.keys(config).length), raw, diff --git a/src/test/imageMetadata.test.ts b/src/test/imageMetadata.test.ts index 666b02d6c..972343a12 100644 --- a/src/test/imageMetadata.test.ts +++ b/src/test/imageMetadata.test.ts @@ -358,6 +358,7 @@ describe('Image Metadata', function () { }, { id: 'ghcr.io/my-org/my-repo/someFeature:1', + featureRootFolder: '/usr/share/devcontainer/features/someFeature_1' }, { remoteUser: 'testUser', From d4f974b498141bbd425c4785fb56e049da5cddc5 Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Tue, 14 Feb 2023 01:04:57 +0000 Subject: [PATCH 38/41] merged config has substituted featureRootFolder values --- src/spec-node/imageMetadata.ts | 21 ++++++++++++++----- .../container-features/lifecycleHooks.test.ts | 2 +- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/spec-node/imageMetadata.ts b/src/spec-node/imageMetadata.ts index 0dbae04df..cd4d7c12b 100644 --- a/src/spec-node/imageMetadata.ts +++ b/src/spec-node/imageMetadata.ts @@ -146,6 +146,17 @@ export function lifecycleCommandOriginMapFromMetadata(metadata: ImageMetadataEnt return map; } +function mergeLifecycleHooks(metadata: ImageMetadataEntry[], hook: (keyof SchemaFeatureLifecycleHooks)): LifecycleCommand[] | undefined { + const collected: LifecycleCommand[] = []; + for (const entry of metadata) { + const command = entry[hook]; + const substitutedCommand = substituteFeatureRoot(command, entry.featureRootFolder); + if (substitutedCommand) { + collected.push(substitutedCommand); + } + } + return collected; +} export function mergeConfiguration(config: DevContainerConfig, imageMetadata: ImageMetadataEntry[]): MergedDevContainerConfig { const customizations = imageMetadata.reduce((obj, entry) => { @@ -170,11 +181,11 @@ export function mergeConfiguration(config: DevContainerConfig, imageMetadata: Im entrypoints: collectOrUndefined(imageMetadata, 'entrypoint'), mounts: mergeMounts(imageMetadata), customizations: Object.keys(customizations).length ? customizations : undefined, - onCreateCommands: collectOrUndefined(imageMetadata, 'onCreateCommand'), - updateContentCommands: collectOrUndefined(imageMetadata, 'updateContentCommand'), - postCreateCommands: collectOrUndefined(imageMetadata, 'postCreateCommand'), - postStartCommands: collectOrUndefined(imageMetadata, 'postStartCommand'), - postAttachCommands: collectOrUndefined(imageMetadata, 'postAttachCommand'), + onCreateCommands: mergeLifecycleHooks(imageMetadata, 'onCreateCommand'), + updateContentCommands: mergeLifecycleHooks(imageMetadata, 'updateContentCommand'), + postCreateCommands: mergeLifecycleHooks(imageMetadata, 'postCreateCommand'), + postStartCommands: mergeLifecycleHooks(imageMetadata, 'postStartCommand'), + postAttachCommands: mergeLifecycleHooks(imageMetadata, 'postAttachCommand'), waitFor: reversed.find(entry => entry.waitFor)?.waitFor, remoteUser: reversed.find(entry => entry.remoteUser)?.remoteUser, containerUser: reversed.find(entry => entry.containerUser)?.containerUser, diff --git a/src/test/container-features/lifecycleHooks.test.ts b/src/test/container-features/lifecycleHooks.test.ts index 0738749ec..97bd2995f 100644 --- a/src/test/container-features/lifecycleHooks.test.ts +++ b/src/test/container-features/lifecycleHooks.test.ts @@ -217,7 +217,7 @@ describe('Feature lifecycle hooks', function () { await shellExec(`rm -f ${testFolder}/*.testMarker`, undefined, undefined, true); }); - it('Feature command added to path can be executed in a lifecycle scripts', async () => { + it('executes lifecycle hooks in advanced cases', async () => { const res = await shellExec(`${cli} exec --workspace-folder ${testFolder} ls -altr`); const response = JSON.parse(res.stdout); assert.equal(response.outcome, 'success'); From b4e40a5da65ed1ee7faefe51faf5f59e347a6ffb Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Tue, 28 Feb 2023 23:48:54 +0000 Subject: [PATCH 39/41] update tests to remove ${featureRootFolder} --- .../.devcontainer/otter/devcontainer-feature.json | 12 ++++++------ .../.devcontainer/otter/install.sh | 5 +++++ .../.devcontainer/rabbit/devcontainer-feature.json | 12 ++++++------ .../.devcontainer/rabbit/install.sh | 6 ++++++ .../.devcontainer/hippo/devcontainer-feature.json | 4 ++-- .../.devcontainer/hippo/install.sh | 5 +++++ src/test/container-features/lifecycleHooks.test.ts | 5 ++--- 7 files changed, 32 insertions(+), 17 deletions(-) diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json index 88e62fc45..a53c1fd76 100644 --- a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/devcontainer-feature.json @@ -2,21 +2,21 @@ "id": "otter", "version": "1.2.3", "options": {}, - "updateContentCommand": "${featureRootFolder}/helper_script.sh updateContentCommand", - "onCreateCommand": "${featureRootFolder}/helper_script.sh onCreateCommand", + "updateContentCommand": "/usr/features/otter/helper_script.sh updateContentCommand", + "onCreateCommand": "/usr/features/otter/helper_script.sh onCreateCommand", "postCreateCommand": { - "parallel1": "${featureRootFolder}/helper_script.sh parallel_postCreateCommand_1", + "parallel1": "/usr/features/otter/helper_script.sh parallel_postCreateCommand_1", "parallel2": [ - "${featureRootFolder}/helper_script.sh", + "/usr/features/otter/helper_script.sh", "parallel_postCreateCommand_2" ] }, "postStartCommand": [ - "${featureRootFolder}/helper_script.sh", + "/usr/features/otter/helper_script.sh", "postStartCommand" ], "postAttachCommand": [ - "${featureRootFolder}/helper_script.sh", + "/usr/features/otter/helper_script.sh", "postAttachCommand" ] } \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/install.sh b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/install.sh index e937ebb91..e1799fdae 100644 --- a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/install.sh +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/otter/install.sh @@ -9,4 +9,9 @@ cat > /usr/local/bin/otter \ echo "i-am-an-otter" EOF +# Copy helper script into somewhere that will persist +mkdir -p /usr/features/otter +chmod -R 0755 /usr/features/otter +cp ./helper_script.sh /usr/features/otter/helper_script.sh + chmod +x /usr/local/bin/otter \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/devcontainer-feature.json b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/devcontainer-feature.json index fb798522f..3dbad71bd 100644 --- a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/devcontainer-feature.json +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/devcontainer-feature.json @@ -2,21 +2,21 @@ "id": "rabbit", "version": "100.200.300", "options": {}, - "updateContentCommand": "${featureRootFolder}/helper_script.sh updateContentCommand", - "onCreateCommand": "${featureRootFolder}/helper_script.sh onCreateCommand", + "updateContentCommand": "/usr/features/rabbit/helper_script.sh updateContentCommand", + "onCreateCommand": "/usr/features/rabbit/helper_script.sh onCreateCommand", "postCreateCommand": { - "parallel1": "${featureRootFolder}/helper_script.sh parallel_postCreateCommand_1", + "parallel1": "/usr/features/rabbit/helper_script.sh parallel_postCreateCommand_1", "parallel2": [ - "${featureRootFolder}/helper_script.sh", + "/usr/features/rabbit/helper_script.sh", "parallel_postCreateCommand_2" ] }, "postStartCommand": [ - "${featureRootFolder}/helper_script.sh", + "/usr/features/rabbit/helper_script.sh", "postStartCommand" ], "postAttachCommand": [ - "${featureRootFolder}/helper_script.sh", + "/usr/features/rabbit/helper_script.sh", "postAttachCommand" ] } \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/install.sh b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/install.sh index 49ea5a294..7fe0dd5a2 100644 --- a/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/install.sh +++ b/src/test/container-features/configs/lifecycle-hooks-advanced/.devcontainer/rabbit/install.sh @@ -9,4 +9,10 @@ cat > /usr/local/bin/rabbit \ echo "i-am-a-rabbit" EOF +# Copy helper script into somewhere that will persist +mkdir -p /usr/features/rabbit +chmod -R 0755 /usr/features/rabbit +cp ./helper_script.sh /usr/features/rabbit/helper_script.sh + + chmod +x /usr/local/bin/rabbit \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-resume-existing-container/.devcontainer/hippo/devcontainer-feature.json b/src/test/container-features/configs/lifecycle-hooks-resume-existing-container/.devcontainer/hippo/devcontainer-feature.json index 2340db9e9..aaa08461d 100644 --- a/src/test/container-features/configs/lifecycle-hooks-resume-existing-container/.devcontainer/hippo/devcontainer-feature.json +++ b/src/test/container-features/configs/lifecycle-hooks-resume-existing-container/.devcontainer/hippo/devcontainer-feature.json @@ -3,6 +3,6 @@ "version": "1.0.1", "name": "Hippo", "options": {}, - "postStartCommand": "${featureRootFolder}/createMarker.sh hippo.postStartCommand.testMarker", - "postAttachCommand": "${featureRootFolder}/createMarker.sh hippo.postAttachCommand.testMarker" + "postStartCommand": "/usr/features/hippo/createMarker.sh hippo.postStartCommand.testMarker", + "postAttachCommand": "/usr/features/hippo/createMarker.sh hippo.postAttachCommand.testMarker" } \ No newline at end of file diff --git a/src/test/container-features/configs/lifecycle-hooks-resume-existing-container/.devcontainer/hippo/install.sh b/src/test/container-features/configs/lifecycle-hooks-resume-existing-container/.devcontainer/hippo/install.sh index 7a3c83ced..291325142 100644 --- a/src/test/container-features/configs/lifecycle-hooks-resume-existing-container/.devcontainer/hippo/install.sh +++ b/src/test/container-features/configs/lifecycle-hooks-resume-existing-container/.devcontainer/hippo/install.sh @@ -9,4 +9,9 @@ cat > /usr/local/bin/hippo \ echo 🦛 EOF +# Copy helper script into somewhere that will persist +mkdir -p /usr/features/hippo +cp ./createMarker.sh /usr/features/hippo +chmod -R 0755 /usr/features/hippo + chmod +x /usr/local/bin/hippo \ No newline at end of file diff --git a/src/test/container-features/lifecycleHooks.test.ts b/src/test/container-features/lifecycleHooks.test.ts index 97bd2995f..14b6d3470 100644 --- a/src/test/container-features/lifecycleHooks.test.ts +++ b/src/test/container-features/lifecycleHooks.test.ts @@ -240,9 +240,8 @@ describe('Feature lifecycle hooks', function () { assert.match(containerUpStandardError, /Running parallel2 from devcontainer.json.../); // Since lifecycle scripts are executed relative to the workspace folder, - // to run a script bundled with the Feature, the user needs to use the '${featureRootFolder}' variable. - // This variable can only be used in a devcontainer-feature.json's lifecycle scripts. - // And will return the temporary directory where the Feature's files are copied to. + // to run a script bundled with the Feature, the Feature author needs to copy that script to a persistent directory. + // These Features' install scripts do that. // -- 'Rabbit' Feature assert.match(outputOfExecCommand, /helperScript.rabbit.onCreateCommand.testMarker/); From 73c3a4f6f6cc88ae6b2a4e3e7cf16d6ed2af5394 Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Tue, 28 Feb 2023 23:56:02 +0000 Subject: [PATCH 40/41] remove ${featureRootFolder} variable substiution --- src/spec-common/variableSubstitution.ts | 18 ------------------ src/spec-node/imageMetadata.ts | 16 +++++----------- src/test/imageMetadata.test.ts | 1 - 3 files changed, 5 insertions(+), 30 deletions(-) diff --git a/src/spec-common/variableSubstitution.ts b/src/spec-common/variableSubstitution.ts index 48b86d085..d973f0cd6 100644 --- a/src/spec-common/variableSubstitution.ts +++ b/src/spec-common/variableSubstitution.ts @@ -8,7 +8,6 @@ import * as crypto from 'crypto'; import { ContainerError } from './errors'; import { URI } from 'vscode-uri'; -import { LifecycleCommand } from './injectHeadless'; export interface SubstitutionContext { platform: NodeJS.Platform; @@ -125,16 +124,6 @@ function replaceContainerEnv(isWindows: boolean, configFile: URI | undefined, co } } -function replaceFeatureRoot(featureRootFolder: string | undefined, match: string, variable: string) { - switch (variable) { - case 'featureRootFolder': - return featureRootFolder || match; - - default: - return match; - } -} - function replaceDevContainerId(getDevContainerId: () => string | undefined, match: string, variable: string) { switch (variable) { case 'devcontainerId': @@ -179,11 +168,4 @@ function devcontainerIdForLabels(idLabels: Record): string { .toString(32) .padStart(52, '0'); return uniqueId; -} - -export function substituteFeatureRoot(lifecycleHook: LifecycleCommand | undefined, cachePath: string | undefined): LifecycleCommand | undefined { - if (!lifecycleHook || !cachePath) { - return lifecycleHook; - } - return substitute0(replaceFeatureRoot.bind(undefined, cachePath), lifecycleHook); } \ No newline at end of file diff --git a/src/spec-node/imageMetadata.ts b/src/spec-node/imageMetadata.ts index cd4d7c12b..9dadeee4e 100644 --- a/src/spec-node/imageMetadata.ts +++ b/src/spec-node/imageMetadata.ts @@ -3,12 +3,10 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import path from 'path'; import { ContainerError } from '../spec-common/errors'; import { LifecycleCommand, LifecycleHooksInstallMap } from '../spec-common/injectHeadless'; -import { substituteFeatureRoot } from '../spec-common/variableSubstitution'; import { DevContainerConfig, DevContainerConfigCommand, DevContainerFromDockerComposeConfig, DevContainerFromDockerfileConfig, DevContainerFromImageConfig, getDockerComposeFilePaths, getDockerfilePath, HostGPURequirements, HostRequirements, isDockerFileConfig, PortAttributes, UserEnvProbe } from '../spec-configuration/configuration'; -import { Feature, FeaturesConfig, FEATURES_CONTAINER_DEST_FOLDER, Mount, parseMount, SchemaFeatureLifecycleHooks } from '../spec-configuration/containerFeaturesConfiguration'; +import { Feature, FeaturesConfig, Mount, parseMount, SchemaFeatureLifecycleHooks } from '../spec-configuration/containerFeaturesConfiguration'; import { ContainerDetails, DockerCLIParameters, ImageDetails } from '../spec-shutdown/dockerUtils'; import { Log } from '../spec-utils/log'; import { getBuildInfoForService, readDockerComposeConfig } from './dockerCompose'; @@ -69,7 +67,6 @@ const pickFeatureProperties: Exclude featureSet.features.map(feature => ({ id: featureSet.sourceInformation.userFeatureId, - featureRootFolder: featureSet ? path.posix.join(FEATURES_CONTAINER_DEST_FOLDER, featureSet.features[0].consecutiveId!) : undefined, ...pick(feature, effectivePickFeatureProperties), }))).flat() || []; diff --git a/src/test/imageMetadata.test.ts b/src/test/imageMetadata.test.ts index 972343a12..666b02d6c 100644 --- a/src/test/imageMetadata.test.ts +++ b/src/test/imageMetadata.test.ts @@ -358,7 +358,6 @@ describe('Image Metadata', function () { }, { id: 'ghcr.io/my-org/my-repo/someFeature:1', - featureRootFolder: '/usr/share/devcontainer/features/someFeature_1' }, { remoteUser: 'testUser', From 14a1c33b585c31f556b84b768e528a268a674113 Mon Sep 17 00:00:00 2001 From: Josh Spicer Date: Wed, 1 Mar 2023 00:36:12 +0000 Subject: [PATCH 41/41] move Features container build folder back to /tmp --- .../containerFeaturesConfiguration.ts | 22 +++++++------ .../generateFeaturesConfig.test.ts | 32 +++++++++---------- 2 files changed, 28 insertions(+), 26 deletions(-) diff --git a/src/spec-configuration/containerFeaturesConfiguration.ts b/src/spec-configuration/containerFeaturesConfiguration.ts index bab63a17e..162495462 100644 --- a/src/spec-configuration/containerFeaturesConfiguration.ts +++ b/src/spec-configuration/containerFeaturesConfiguration.ts @@ -25,7 +25,7 @@ export const DEVCONTAINER_FEATURE_FILE_NAME = 'devcontainer-feature.json'; export type Feature = SchemaFeatureBaseProperties & SchemaFeatureLifecycleHooks & DeprecatedSchemaFeatureProperties & InternalFeatureProperties; -export const FEATURES_CONTAINER_DEST_FOLDER = '/usr/share/devcontainer/features'; +export const FEATURES_CONTAINER_TEMP_DEST_FOLDER = '/tmp/dev-container-features'; export interface SchemaFeatureLifecycleHooks { onCreateCommand?: string | string[]; @@ -253,8 +253,8 @@ FROM $_DEV_CONTAINERS_BASE_IMAGE AS dev_containers_target_stage USER root -RUN mkdir -p ${FEATURES_CONTAINER_DEST_FOLDER} -COPY --from=dev_containers_feature_content_normalize /tmp/build-features/ ${FEATURES_CONTAINER_DEST_FOLDER} +RUN mkdir -p ${FEATURES_CONTAINER_TEMP_DEST_FOLDER} +COPY --from=dev_containers_feature_content_normalize /tmp/build-features/ ${FEATURES_CONTAINER_TEMP_DEST_FOLDER} #{featureLayer} @@ -333,7 +333,7 @@ function escapeQuotesForShell(input: string) { export function getFeatureLayers(featuresConfig: FeaturesConfig, containerUser: string, remoteUser: string, useBuildKitBuildContexts = false, contentSourceRootPath = '/tmp/build-features') { - const builtinsEnvFile = `${path.posix.join(FEATURES_CONTAINER_DEST_FOLDER, 'devcontainer-features.builtin.env')}`; + const builtinsEnvFile = `${path.posix.join(FEATURES_CONTAINER_TEMP_DEST_FOLDER, 'devcontainer-features.builtin.env')}`; let result = `RUN \\ echo "_CONTAINER_USER_HOME=$(getent passwd ${containerUser} | cut -d: -f6)" >> ${builtinsEnvFile} && \\ echo "_REMOTE_USER_HOME=$(getent passwd ${remoteUser} | cut -d: -f6)" >> ${builtinsEnvFile} @@ -344,7 +344,7 @@ echo "_REMOTE_USER_HOME=$(getent passwd ${remoteUser} | cut -d: -f6)" >> ${built const folders = (featuresConfig.featureSets || []).filter(y => y.internalVersion !== '2').map(x => x.features[0].consecutiveId); folders.forEach(folder => { const source = path.posix.join(contentSourceRootPath, folder!); - const dest = path.posix.join(FEATURES_CONTAINER_DEST_FOLDER, folder!); + const dest = path.posix.join(FEATURES_CONTAINER_TEMP_DEST_FOLDER, folder!); if (!useBuildKitBuildContexts) { result += `COPY --chown=root:root --from=dev_containers_feature_content_source ${source} ${dest} RUN chmod -R 0755 ${dest} \\ @@ -355,11 +355,12 @@ RUN chmod -R 0755 ${dest} \\ `; } else { result += `RUN --mount=type=bind,from=dev_containers_feature_content_source,source=${source},target=/tmp/build-features-src/${folder} \\ - cp -ar /tmp/build-features-src/${folder} ${FEATURES_CONTAINER_DEST_FOLDER} \\ + cp -ar /tmp/build-features-src/${folder} ${FEATURES_CONTAINER_TEMP_DEST_FOLDER} \\ && chmod -R 0755 ${dest} \\ && cd ${dest} \\ && chmod +x ./install.sh \\ - && ./install.sh + && ./install.sh \\ + && rm -rf ${dest} `; } @@ -369,7 +370,7 @@ RUN chmod -R 0755 ${dest} \\ featureSet.features.forEach(feature => { result += generateContainerEnvs(feature); const source = path.posix.join(contentSourceRootPath, feature.consecutiveId!); - const dest = path.posix.join(FEATURES_CONTAINER_DEST_FOLDER, feature.consecutiveId!); + const dest = path.posix.join(FEATURES_CONTAINER_TEMP_DEST_FOLDER, feature.consecutiveId!); if (!useBuildKitBuildContexts) { result += ` COPY --chown=root:root --from=dev_containers_feature_content_source ${source} ${dest} @@ -382,11 +383,12 @@ RUN chmod -R 0755 ${dest} \\ } else { result += ` RUN --mount=type=bind,from=dev_containers_feature_content_source,source=${source},target=/tmp/build-features-src/${feature.consecutiveId} \\ - cp -ar /tmp/build-features-src/${feature.consecutiveId} ${FEATURES_CONTAINER_DEST_FOLDER} \\ + cp -ar /tmp/build-features-src/${feature.consecutiveId} ${FEATURES_CONTAINER_TEMP_DEST_FOLDER} \\ && chmod -R 0755 ${dest} \\ && cd ${dest} \\ && chmod +x ./devcontainer-features-install.sh \\ - && ./devcontainer-features-install.sh + && ./devcontainer-features-install.sh \\ + && rm -rf ${dest} `; } diff --git a/src/test/container-features/generateFeaturesConfig.test.ts b/src/test/container-features/generateFeaturesConfig.test.ts index 18fdefa05..24fdcd875 100644 --- a/src/test/container-features/generateFeaturesConfig.test.ts +++ b/src/test/container-features/generateFeaturesConfig.test.ts @@ -72,18 +72,18 @@ describe('validate generateFeaturesConfig()', function () { // getFeatureLayers const actualLayers = getFeatureLayers(featuresConfig, 'testContainerUser', 'testRemoteUser'); const expectedLayers = `RUN \\ -echo "_CONTAINER_USER_HOME=$(getent passwd testContainerUser | cut -d: -f6)" >> /usr/share/devcontainer/features/devcontainer-features.builtin.env && \\ -echo "_REMOTE_USER_HOME=$(getent passwd testRemoteUser | cut -d: -f6)" >> /usr/share/devcontainer/features/devcontainer-features.builtin.env +echo "_CONTAINER_USER_HOME=$(getent passwd testContainerUser | cut -d: -f6)" >> /tmp/dev-container-features/devcontainer-features.builtin.env && \\ +echo "_REMOTE_USER_HOME=$(getent passwd testRemoteUser | cut -d: -f6)" >> /tmp/dev-container-features/devcontainer-features.builtin.env -COPY --chown=root:root --from=dev_containers_feature_content_source /tmp/build-features/first_1 /usr/share/devcontainer/features/first_1 -RUN chmod -R 0755 /usr/share/devcontainer/features/first_1 \\ -&& cd /usr/share/devcontainer/features/first_1 \\ +COPY --chown=root:root --from=dev_containers_feature_content_source /tmp/build-features/first_1 /tmp/dev-container-features/first_1 +RUN chmod -R 0755 /tmp/dev-container-features/first_1 \\ +&& cd /tmp/dev-container-features/first_1 \\ && chmod +x ./install.sh \\ && ./install.sh -COPY --chown=root:root --from=dev_containers_feature_content_source /tmp/build-features/second_2 /usr/share/devcontainer/features/second_2 -RUN chmod -R 0755 /usr/share/devcontainer/features/second_2 \\ -&& cd /usr/share/devcontainer/features/second_2 \\ +COPY --chown=root:root --from=dev_containers_feature_content_source /tmp/build-features/second_2 /tmp/dev-container-features/second_2 +RUN chmod -R 0755 /tmp/dev-container-features/second_2 \\ +&& cd /tmp/dev-container-features/second_2 \\ && chmod +x ./install.sh \\ && ./install.sh @@ -134,20 +134,20 @@ RUN chmod -R 0755 /usr/share/devcontainer/features/second_2 \\ // getFeatureLayers const actualLayers = getFeatureLayers(featuresConfig, 'testContainerUser', 'testRemoteUser'); const expectedLayers = `RUN \\ -echo "_CONTAINER_USER_HOME=$(getent passwd testContainerUser | cut -d: -f6)" >> /usr/share/devcontainer/features/devcontainer-features.builtin.env && \\ -echo "_REMOTE_USER_HOME=$(getent passwd testRemoteUser | cut -d: -f6)" >> /usr/share/devcontainer/features/devcontainer-features.builtin.env +echo "_CONTAINER_USER_HOME=$(getent passwd testContainerUser | cut -d: -f6)" >> /tmp/dev-container-features/devcontainer-features.builtin.env && \\ +echo "_REMOTE_USER_HOME=$(getent passwd testRemoteUser | cut -d: -f6)" >> /tmp/dev-container-features/devcontainer-features.builtin.env -COPY --chown=root:root --from=dev_containers_feature_content_source /tmp/build-features/color_3 /usr/share/devcontainer/features/color_3 -RUN chmod -R 0755 /usr/share/devcontainer/features/color_3 \\ -&& cd /usr/share/devcontainer/features/color_3 \\ +COPY --chown=root:root --from=dev_containers_feature_content_source /tmp/build-features/color_3 /tmp/dev-container-features/color_3 +RUN chmod -R 0755 /tmp/dev-container-features/color_3 \\ +&& cd /tmp/dev-container-features/color_3 \\ && chmod +x ./devcontainer-features-install.sh \\ && ./devcontainer-features-install.sh -COPY --chown=root:root --from=dev_containers_feature_content_source /tmp/build-features/hello_4 /usr/share/devcontainer/features/hello_4 -RUN chmod -R 0755 /usr/share/devcontainer/features/hello_4 \\ -&& cd /usr/share/devcontainer/features/hello_4 \\ +COPY --chown=root:root --from=dev_containers_feature_content_source /tmp/build-features/hello_4 /tmp/dev-container-features/hello_4 +RUN chmod -R 0755 /tmp/dev-container-features/hello_4 \\ +&& cd /tmp/dev-container-features/hello_4 \\ && chmod +x ./devcontainer-features-install.sh \\ && ./devcontainer-features-install.sh