[Feature]: make devpod meet spec for lifecycle hooks

[stevegt]

Before submitting

• I have searched existing issues and discussions to confirm this hasn't been requested

Overview

I need true prebuild behavior, but devpod inherited some non-compliant behavior from the loft-sh implementation. I'm still analyzing devpod, devcontainer cli, and the behavior of codespaces, so some of the following will change, but here's what I have so far.

Cleanup proposal is at the bottom, and I'm mostly looking for a signal here as to whether I should proceed with a PR or not. There is an important open question for @skevetter at the end.

Problem

• The devcontainer spec implies (see below) that onCreateCommand and updateContentCommand need to run during prebuild.
• Github codespaces do in fact run onCreateCommand and updateContentCommand during prebuild, as does the public version of devcontainer cli.
• Devpod does not seem to have the ability to do this -- 'build' runs neither onCreateCommand nor updateContentCommand. The 'devpod up' subcommand runs both, but then keeps going, running postCreateCommand etc. so it can't be used for prebuilds either.

This is causing me to resort to using the devcontainer cli for prebuilds rather than devpod.

Background

I can see how this could have occurred; the spec isn't precise about exactly when each lifecycle hook should run. The best we have is this sentence in the onCreateCommand section of the devcontainers.json reference:

Cloud services can use this command when caching or prebuilding a container.

...so it's pretty clear onCreateCommand needs to run during prebuild. But then we have this sentence in the updateContentCommand:

It executes inside the container after onCreateCommand whenever new content is available in the source tree during the creation process.

...which unfortunately doesn't explicitly say that updateContentCommand needs to run during prebuild -- it only implies it by the sequencing after onCreateCommand.

Proposal

So... If I were to provide a PR that provided prebuild behavior that matched codespaces and the devcontainer cli, would you be able to accept it @skevetter?

I might add a flag to the 'up' command, so the behavior matches that of 'devcontainer up --prebuild':

devpod up --prebuild

This would essentially run the same code path as 'build' and 'up', but stop after updateContentCommand, giving us a committed image that can then be pushed to an image repo.

But this might actually be easier to implement (less invasive) given the current code structure:

devpod build --prebuild

Comparison with other devcontainer tools

Github codespaces and the devcontainer cli both interpret the spec as, yes, onCreateCommand and updateContentCommand need to run during prebuild -- they both have a prebuild feature that runs these.

Area

Dev containers

Acceptance criteria

• user can use devpod to prebuild images with onCreateCommand and updateContentCommand having been run
• onCreateCommand runs during build per spec, not during attach.
• Other lifecycle hooks run as specified in spec rather than during attach.

Open Questions

What concerns me more at the moment is that the current RunPreAttachHooks() (below) appears to be completely wrong -- it basically runs everything, starting with onCreateCommand, rather than running only the pertinent hooks at each correct point in the build/run lifecycle. I can see how this could have happened if someone just wanted to get something working, but it was never cleaned up afterward.

So @skevetter, do you think your fork of devpod is currently making any behavior guarantees that would prevent me cleaning this up so it more closely behaves like codespaces and devcontainer's cli tool?

devpod/pkg/devcontainer/setup/lifecyclehooks.go

Line 55 in f6c3ab2

func RunPreAttachHooks(ctx context.Context, setupInfo *config.Result, log log.Logger) error {

[skevetter]

Yes, you can work on a PR that makes the lifecycle hooks more compliant with the devcontainer specification. In your changes, be sure to include end-to-end tests. If the changes are extensive, I would suggest breaking up the work over more than one PRs.

Will you link the devcontainer-cli flow you are using to pre-build reference.

[stevegt]

I agree about more than one PR -- I went into this thinking that it would be a simple change, but the deeper I dug while drafting that opening comment, the more I realized we have a bigger problem, particularly that RunPreAttachHooks() behavior. I was already thinking "this will need more than one PR" minutes before I saw your answer. ;-)

Right now I'm working on scripted evaluations of the actual lifecycle hook behavior of devcontainer-cli, codespaces, and devpod -- that work is in the decomk repo. Since this devpod cleanup is probably going to take some time, I'm likely to release decomk this week using devcontainer-cli for prebuilds -- I'll have a flow I can link to for reference at that point.