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 scale command #10979

Merged
merged 2 commits into from
Sep 13, 2023
Merged

add scale command #10979

merged 2 commits into from
Sep 13, 2023

Conversation

glours
Copy link
Contributor

@glours glours commented Sep 6, 2023

What I did
Add a scale command

Related issue
https://docker.atlassian.net/browse/ENV-310
#10965

(not mandatory) A picture of a cute animal, if possible in relation to what you did
image

@glours glours self-assigned this Sep 6, 2023
@glours glours force-pushed the alpha-scale-cmd branch 3 times, most recently from f14aa70 to 352d7b7 Compare September 6, 2023 21:18
@codecov
Copy link

codecov bot commented Sep 6, 2023

Codecov Report

Patch coverage: 68.57% and project coverage change: +0.17% 🎉

Comparison is base (1311546) 57.72% compared to head (05a6da0) 57.90%.
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10979      +/-   ##
==========================================
+ Coverage   57.72%   57.90%   +0.17%     
==========================================
  Files         127      129       +2     
  Lines       11092    11160      +68     
==========================================
+ Hits         6403     6462      +59     
- Misses       4058     4060       +2     
- Partials      631      638       +7     
Files Changed Coverage Δ
pkg/api/api.go 28.84% <ø> (ø)
pkg/api/proxy.go 39.51% <50.00%> (+0.31%) ⬆️
pkg/compose/compose.go 61.32% <50.00%> (ø)
pkg/compose/scale.go 57.14% <57.14%> (ø)
cmd/compose/scale.go 72.22% <72.22%> (ø)
cmd/compose/compose.go 67.92% <100.00%> (+0.08%) ⬆️

... and 5 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@glours glours force-pushed the alpha-scale-cmd branch 2 times, most recently from f8a96fe to bbb93a7 Compare September 7, 2023 07:46
@ndeloof
Copy link
Contributor

ndeloof commented Sep 7, 2023

scale used to be a docker-compose v1 command, so I don't think we need this to be introduced as an alpha command, just resurrect the legacy one

$ docker-compose-v1 scale --help
Set number of containers to run for a service.

Numbers are specified in the form `service=num` as arguments.
For example:

    $ docker-compose scale web=2 worker=3

@glours glours force-pushed the alpha-scale-cmd branch 3 times, most recently from 53dda54 to 9054dab Compare September 7, 2023 19:56
@glours glours changed the title add scale command as alpha add scale command Sep 7, 2023
@glours glours marked this pull request as ready for review September 7, 2023 20:06
@glours glours requested review from ndeloof and milas September 7, 2023 20:07
@milas
Copy link
Contributor

milas commented Sep 7, 2023

Noting that we should update docs after merging this: https://docs.docker.com/compose/migrate/#unsupported-in-v2

The following were deprecated in Compose V1 and aren't supported in Compose V2:

  • docker-compose scale. Use docker compose up --scale instead.

Copy link
Contributor

@milas milas left a comment

Choose a reason for hiding this comment

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

LGTM

cmd/compose/scale.go Outdated Show resolved Hide resolved
cmd/compose/scale.go Outdated Show resolved Hide resolved
@glours
Copy link
Contributor Author

glours commented Sep 7, 2023

Ok my test assertions are fragiles as we can't bet which container will be removed 😬

scale_test.go:54: Running command: docker compose --project-directory fixtures/scale scale dbadmin=1
    scale_test.go:55: assertion failed: expression is false: strings.Contains(res.Combined(), "Container scale-basic-tests-dbadmin-1  Running"):  Container scale-basic-tests-dbadmin-2  Running

I'll improve this tomorrow

@glours glours force-pushed the alpha-scale-cmd branch 3 times, most recently from e1cc36a to b69cea4 Compare September 8, 2023 08:53
@milas
Copy link
Contributor

milas commented Sep 8, 2023

I'm surprised your assertion failed...I thought we'd made scale deterministic at some point, so that it would always leave the "oldest" (lowest-numbered) replicas

@glours
Copy link
Contributor Author

glours commented Sep 8, 2023

@milas I think the 2 replicas were created at the same time 🤷‍♂️
Anyway it's fix now and assertions are way more robust!

@glours glours force-pushed the alpha-scale-cmd branch 2 times, most recently from 17056d0 to 2c51c69 Compare September 13, 2023 08:35
Signed-off-by: Guillaume Lours <705411+glours@users.noreply.github.com>
Co-authored-by: Milas Bowman <devnull@milas.dev>
Signed-off-by: Guillaume Lours <705411+glours@users.noreply.github.com>
@ndeloof ndeloof merged commit 9d7e0ad into docker:main Sep 13, 2023
25 checks passed
matt9ucci added a commit to matt9ucci/DockerCompletion that referenced this pull request Apr 8, 2024
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.

3 participants