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

Add compose create #1668

Merged
merged 2 commits into from
Dec 20, 2022
Merged

Add compose create #1668

merged 2 commits into from
Dec 20, 2022

Conversation

djdongjin
Copy link
Member

@djdongjin djdongjin commented Dec 19, 2022

This PR adds compose create command and its flags: https://docs.docker.com/engine/reference/commandline/compose_create/

There are some follow-up TODOs/refactors which I think we can track in issues/follow-up PRs because they're not trivial and involves several dependent commands and pkgs. Including them in the same PR might make it too large/confusing.

  • some compose commands have logic in cmd instead of pkg (also due to other dependency being in cmd), making it hard to reuse.
  • current compose up implementation should be visited/decoupled:
    • ideally, up should equal to create + start (logic are resuable, up can utilize created containers)
    • at least we should decouple and reuse some code from up in create (run as well)
  • (related) refactor compose up|create|start (possibly compose run as well)

This should be the last compose command 😄 (before some refactor happens). After this, there are two remaining commands:

  • compose scale: depcrated in v2, I assume we no longer need this?
  • compose events: to make it work needs other compose commands releasing relevant events (e.g., service container create/stop/start/etc). it might be easier to implement after we refactor the code base and simplifies some of logic.

@djdongjin djdongjin force-pushed the compose-create branch 2 times, most recently from fe5c91d to 326a9e0 Compare December 19, 2022 04:25
@djdongjin djdongjin changed the title [WIP] Add compose create Add compose create Dec 19, 2022
@djdongjin djdongjin marked this pull request as ready for review December 19, 2022 05:01
@AkihiroSuda AkihiroSuda added this to the v1.2.0 (tentative) milestone Dec 20, 2022
"golang.org/x/sync/errgroup"
)

// FYI: https://github.com/docker/compose/blob/v2/pkg/api/api.go#L423
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// FYI: https://github.com/docker/compose/blob/v2/pkg/api/api.go#L423
// FYI: https://github.com/docker/compose/blob/v2.14.1/pkg/api/api.go#L423

for permanence.

Same applies to other comment lines too

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

Signed-off-by: Jin Dong <jindon@amazon.com>
Signed-off-by: Jin Dong <jindon@amazon.com>
Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks

@AkihiroSuda AkihiroSuda merged commit 25e8298 into containerd:main Dec 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants