Codecov Report

Merging #1612 into master will increase coverage by 0.08%.
The diff coverage is 47.36%.

@@            Coverage Diff             @@
##           master    #1612      +/-   ##
==========================================
+ Coverage   56.05%   56.14%   +0.08%     
==========================================
  Files         306      306              
  Lines       20981    20980       -1     
==========================================
+ Hits        11761    11779      +18     
+ Misses       8371     8347      -24     
- Partials      849      854       +5

@thaJeztah FYI. I moved non-stack version to this PR as it is already fully working and it easier to debug stack version when there is less stuff.

Can you give first review? I assume that at least some kind of tests are wanted on but which kind of?

Logic is tested on swarmkit and API on Moby.

@thaJeztah reminder of this one. I really would like get feedback so I can finalize it.

@vdemeester @thaJeztah PTAL

Ah, whoops, thought I reviewed; yes, changes look good, but if you can add a unit test, that'd be great

@silvin-lubecki corrected that one and rebased to one commit.

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "replicas-max-per-node-cli" git@github.com:olljanat/cli.git somewhere
$ cd somewhere
$ git rebase -i HEAD~842358453696
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

@thaJeztah included changes you suggested and rebased to one commit.

Gave this a spin, and all looks good 😄

We could add some more tests based on the scenarios below, but I'm ok to keep that for a follow-up.

Create a service with max 2 replicas:

docker service create --replicas=2 --replicas-max-per-node=2 --name test nginx:alpine
docker service inspect --format '{{.Spec.TaskTemplate.Placement.MaxReplicas}}' test
2

Update the service (max replicas should keep its value) - this works ok;

docker service update --replicas=1 test
docker service inspect --format '{{.Spec.TaskTemplate.Placement.MaxReplicas}}' test
2

Update the max replicas to 1

docker service update --replicas-max-per-node=1 test
docker service inspect --format '{{.Spec.TaskTemplate.Placement.MaxReplicas}}' test
1

And reset to 0:

docker service update --replicas-max-per-node=0 test
docker service inspect --format '{{.Spec.TaskTemplate.Placement.MaxReplicas}}' test
0

- What I did
Added support for maximum replicas per node

Third step to be able solve:

• moby/moby#26259

- How I did it
Added new switch --replicas-max-per-node switch to docker service

- How to verify it
Create two services and specify --replicas-max-per-node one of them:

docker service create --detach=true --name web1 --replicas 2 nginx
docker service create --detach=true --name web2 --replicas 2 --replicas-max-per-node 1 nginx

See difference on command outputs:

$ docker service ls
ID                  NAME                MODE                REPLICAS               IMAGE               PORTS
0inbv7q148nn        web1                replicated          2/2                    nginx:latest        
9kry59rk4ecr        web2                replicated          1/2 (max 1 per node)   nginx:latest
        
$ docker service ps --no-trunc web2
ID                          NAME                IMAGE                                                                                  NODE                DESIRED STATE       CURRENT STATE            ERROR                                                     PORTS
bf90bhy72o2ry2pj50xh24cfp   web2.1              nginx:latest@sha256:b543f6d0983fbc25b9874e22f4fe257a567111da96fd1d8f1b44315f1236398c   limint              Running             Running 34 seconds ago                                                             
xedop9dwtilok0r56w4g7h5jm   web2.2              nginx:latest@sha256:b543f6d0983fbc25b9874e22f4fe257a567111da96fd1d8f1b44315f1236398c                       Running             Pending 35 seconds ago   "no suitable node (max replicas per node limit exceed)"   

- Description for the changelog

• Split from Add maximum replicas per node support to stack version 3.8 #1410 to simplify review

Hi! I would like to link

- What I did
Added support for maximum replicas per node

Third step to be able solve:

• moby/moby#26259

- How I did it
Added new switch --replicas-max-per-node switch to docker service

- How to verify it
Create two services and specify --replicas-max-per-node one of them:

docker service create --detach=true --name web1 --replicas 2 nginx
docker service create --detach=true --name web2 --replicas 2 --replicas-max-per-node 1 nginx

See difference on command outputs:

$ docker service ls
ID                  NAME                MODE                REPLICAS               IMAGE               PORTS
0inbv7q148nn        web1                replicated          2/2                    nginx:latest        
9kry59rk4ecr        web2                replicated          1/2 (max 1 per node)   nginx:latest
        
$ docker service ps --no-trunc web2
ID                          NAME                IMAGE                                                                                  NODE                DESIRED STATE       CURRENT STATE            ERROR                                                     PORTS
bf90bhy72o2ry2pj50xh24cfp   web2.1              nginx:latest@sha256:b543f6d0983fbc25b9874e22f4fe257a567111da96fd1d8f1b44315f1236398c   limint              Running             Running 34 seconds ago                                                             
xedop9dwtilok0r56w4g7h5jm   web2.2              nginx:latest@sha256:b543f6d0983fbc25b9874e22f4fe257a567111da96fd1d8f1b44315f1236398c                       Running             Pending 35 seconds ago   "no suitable node (max replicas per node limit exceed)"   

- Description for the changelog

• Split from Add maximum replicas per node support to stack version 3.8 #1410 to simplify review

Hi everyone!

I have a tricky issue here. I posted it on Docker Swarm (classicswarm repo -> docker-archive/classicswarm#2979), but I think this is the right place.

Please, read that issue because it is related with this update and it is not working very well when it is needed to update services already running because the "no suitable node" issue, so, is ok, it is necessary, but the implementation blocks updates on docker swarm. Please read the issue.

Thanks,
Sebastián.