Fix concurrency waiting when running containers

[quantumsheep]

Summary

During container creation, pack was using the Docker <1.30 container wait way of doing. This made building waits undefinitely in some cases.

Before

ContainerWait is blocking.

After

ContainerWait is not blocking and we use the channels to wait for the states we wants.

Related

Resolves #1732

[quantumsheep]

Issue fixed, it was an error on my side, not on buildpack's side

[natalieparellano]

@quantumsheep thanks for the update!

[quantumsheep]

@natalieparellano Ok so... it's effectively an issue from buildpacks, if the request takes some time to be performed, the state is already created so it blocks infinitely.

I had the impression it was fixed on my side but my code just was faster than the Docker container, coming back to a slower iteration, it blocks.

[quantumsheep]

After investigating a little bit more, it seems like buildpacks was conceived to work with Docker < 1.30, at that time they used to not block on ContainerWait but instead returning two channels which can then be used to wait for the container to be ready. That's what was being used by buildpacks.

[joe-kimmel-vmw]

Hi @quantumsheep - thanks for iterating on this and explaining what's going on!

I looked into this together with @natalieparellano today. What you did makes sense, but it's basically shimming us backwards into an older api style. It looks like in the newer api style, we could do without your Wrapper function if we called start, attach, and then wait (instead of calling wait first)? I'm basing this guess on the sdk examples provided by docker.

Would you check whether we can also succeed in your use-case without the wrapper you wrote, by re-ordering our docker API calls? If it works, this would be preferable from our perspective since it would update our usage of docker to the more modern paradigm, rather than shimming for backwards compatibility.

Thank you!

[quantumsheep]

@joe-kimmel-vmw thanks for you answer!

The only way I can do this while keeping the current workflow is by editing the Handler signature, which will be breaking change. If you're fine with that then I'm ready to make the change but the current solution might be better if breaking changes are not wanted.

Also it might not work because a container can exit before we use ContainerWait if we're not fast enough.

[natalieparellano]

I'll defer to the pack maintainers, but IMO editing the Handler signature should be acceptable - it's already in an internal package so the surface area should be small.

[quantumsheep]

editing the Handler signature should be acceptable - it's already in an internal package so the surface area should be small.

I really insist on the fact that it might not work because a container can exit before we use ContainerWait if we're not fast enough. (I tested it 👀)

[joe-kimmel-vmw]

Thanks @quantumsheep - it sounds like the solution in this PR is the one to go with! Really appreciate you doing the extra legwork to verify.