Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Features contribute lifecycle hooks #390

Merged
merged 47 commits into from
Mar 2, 2023

Conversation

joshspicer
Copy link
Member

@joshspicer joshspicer commented Jan 31, 2023

This change implements:
https://github.com/devcontainers/spec/blob/b1d19a5375f667a1bb91e86311bc37c36312996b/proposals/features-contribute-lifecycle-scripts.md

The new contents of the imageMetadata (generated from the lifecycle-hooks-advanced) test can be seen below:

image

…g '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.
…o /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.
@joshspicer joshspicer marked this pull request as ready for review February 4, 2023 20:21
@joshspicer joshspicer requested a review from a team as a code owner February 4, 2023 20:21
Copy link
Member

@samruddhikhandale samruddhikhandale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, left some comments!

@joshspicer joshspicer force-pushed the joshspicer/features-lifecycles-hooks branch from 233d4df to 6687ad4 Compare February 9, 2023 22:40
@joshspicer
Copy link
Member Author

joshspicer commented Feb 13, 2023

Validating that publishing Features with lifecycle hook commands function as expected:

Note that:

  • installsAfter is respected when executing lifecycle commands within a given hook.
  • The appropriate metadata is written to the image
  • the ${featureRootFolder} is cached on the metadata and observed correctly.
  • the appropriate lifecycle hooks can be refired (in the correct order) when necessary (in this example, a postStartCommand embedded by a Feature is refired when I stop and start the container.

See : https://github.com/codspace/features-with-lifecycle-hooks/tree/7f6056555bb4364cde2fef28ec7cad78fc7f058f

With the file structure:

image

image

Stopping the container and using the CLIs up command will retrigger the postStartCommand, which is this case is:

image

image

@joshspicer
Copy link
Member Author

An example of read-configuration correctly outputting the substituted mergedConfiguration

image

Copy link
Contributor

@chrmarti chrmarti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, left one comment.

src/spec-node/imageMetadata.ts Show resolved Hide resolved
Copy link
Contributor

@chrmarti chrmarti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should agree on how we want to include the origin of the lifecycle hooks as part of devcontainers/spec#206. IMO we can still merge the work here since it only affects the mergedConfiguration which does not need to adhere to a schema in the specification yet. 👍

@joshspicer joshspicer merged commit ebdb5ae into main Mar 2, 2023
@joshspicer joshspicer deleted the joshspicer/features-lifecycles-hooks branch March 2, 2023 19:16
joshspicer added a commit to devcontainers/spec that referenced this pull request Mar 3, 2023
joshspicer added a commit to devcontainers/spec that referenced this pull request Mar 10, 2023
* Add lifecycle hooks to devContainerFeature schema

With CLI v0.32.0 and devcontainers/cli#390,
devcontainer-feature.json now support lifecycle hooks.

Spec proposal:
https://github.com/devcontainers/spec/blob/b1d19a5375f667a1bb91e86311bc37c36312996b/proposals/features-contribute-lifecycle-scripts.md

* Description for providing an option to lifecycle hook
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants