Topology-aware scheduling by aaronlehmann · Pull Request #30725 · moby/moby

aaronlehmann · 2017-02-03T20:17:32Z

This adds support for placement preferences in Swarm services (design doc: moby/swarmkit#1473, swarmkit PR: moby/swarmkit#1512). This feature allows services to be balanced over nodes based on a particular user-defined property, such as which datacenter or rack they are located in.

Convert PlacementPreferences between GRPC API and HTTP API
Add --placement-pref-add and --placement-pref-rm to CLI
Add support for placement preferences in service inspect --pretty
Add integration test

cc @aluzzardi @dongluochen @dnephin

aaronlehmann · 2017-02-03T20:18:54Z

cli/command/formatter/service.go

Note: Constraints was misspelled here, and the extra - in the template on the line above caused the lines of text to get merged together. These problems are fixed here. Unfortunately it's not the first time I've noticed problems like this in the template. We should review it carefully.

dnephin

Design LGTM

aaronlehmann · 2017-02-07T22:43:16Z

swarmkit PR was merged. No longer WIP.

Changed the command line syntax from spread,... to spread=... as suggested in moby/swarmkit#1512 (comment)

vdemeester · 2017-02-08T08:21:46Z

@aaronlehmann some test failures

22:47:48 # github.com/docker/docker/cli/command/service
22:47:48 cli/command/service/update_test.go:63: undefined: updatePlacement

vdemeester

Design LGTM

aaronlehmann · 2017-02-08T17:01:37Z

Fixed, sorry about that.

This fix updates SwarmKit to ed384f3. Notable changes since last update (3ca4775): 1. Fix duplicated ports allocation with restarted swarm. (Docker issue moby#29247) 2. Topology-aware scheduling (Docker PR moby#30725) Docker issue moby#29247 was labeled 1.13.1, though it is advised that related SwarmKit changes only to be merged to master (based on the feedback moby/swarmkit#1802 (comment)) This fix fixes moby#29247 (master only). This fix is related to moby#30725. Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

dnephin

LGTM

aaronlehmann · 2017-02-11T01:54:27Z

Rebased.

vdemeester

LGTM 🐸
/cc @thaJeztah @vieux

vdemeester · 2017-02-13T10:32:21Z

cli/command/service/opts.go

missing a " 👼

Thanks, fixed

aaronlehmann · 2017-02-16T21:55:30Z

Thanks for the thoughtful response. This was my initial instinct, but using a comma-separated list has some serious disadvantages as well. See the discussion thread starting at moby/swarmkit#1512 (comment) where this was discussed, and the alternative of using a repeated flag was encouraged.

dnephin · 2017-02-21T16:28:36Z

Looks like this needs a rebase.

Do we have other options that depend on ordering?

Yes we do, but Aaron noted that a change in order for most of those options is less significant than in this case.

aaronlehmann · 2017-02-21T17:49:39Z

Rebased

aluzzardi · 2017-02-25T01:42:44Z

I do agree with UX consistency as well...

--placement-pref would have the same behavior as --placement-pref-add during service create, it's just called differently...

aaronlehmann · 2017-02-27T21:02:36Z

Then is there a consensus around calling it --placement-pref in service create and --placement-pref-add in service update?

It's not my favorite outcome, but I don't see another way to move forward.

dnephin · 2017-02-27T21:12:49Z

Yes, I think that is the most consistent naming

This adds support for placement preferences in Swarm services. - Convert PlacementPreferences between GRPC API and HTTP API - Add --placement-pref, --placement-pref-add and --placement-pref-rm to CLI - Add support for placement preferences in service inspect --pretty - Add integration test Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>

aaronlehmann · 2017-02-27T21:30:36Z

Okay, I've updated this to use --placement-pref for service create.

vieux · 2017-02-27T23:46:06Z

that's very cool, LGTM!

aaronlehmann · 2017-03-01T23:28:48Z

@thaJeztah: Are you happy with the latest change to the CLI flags?

thaJeztah

LGTM, new names for the flags work 👍

I see there's no man page yet for these commands, so I won't hold up this PR because of that (it's something to be addressed in general)

davidhiendl · 2017-04-12T22:37:40Z

Is this supported when deploying services via docker-compose file yet? I can't seem to find any reference to it.

aaronlehmann · 2017-04-12T23:50:31Z

I don't believe it is supported in compose yet. I just filed #32584 to track this.

Topology-aware scheduling

aaronlehmann commented Feb 3, 2017

View reviewed changes

GordonTheTurtle added the status/0-triage label Feb 3, 2017

aaronlehmann mentioned this pull request Feb 3, 2017

Topology-aware scheduling moby/swarmkit#1512

Merged

aaronlehmann added status/1-design-review and removed status/0-triage labels Feb 3, 2017

vdemeester added the status/needs-vendoring label Feb 6, 2017

chanwit added area/cli Client area/swarm labels Feb 6, 2017

chanwit added this to the 1.14.0 milestone Feb 6, 2017

dnephin reviewed Feb 6, 2017

View reviewed changes

aaronlehmann force-pushed the topology branch from 7883d87 to 19c3a42 Compare February 7, 2017 22:39

aaronlehmann changed the title ~~[WIP] Topology-aware scheduling~~ Topology-aware scheduling Feb 7, 2017

aaronlehmann force-pushed the topology branch from 19c3a42 to d0e79de Compare February 7, 2017 22:42

vdemeester added the status/failing-ci Indicates that the PR in its current state fails the test suite label Feb 8, 2017

vdemeester reviewed Feb 8, 2017

View reviewed changes

aaronlehmann added status/2-code-review and removed status/1-design-review labels Feb 8, 2017

yongtang mentioned this pull request Feb 8, 2017

Update SwarmKit to ed384f3b3957f65e3111bd020f9815f3d4296fa2 #30839

Merged

dnephin approved these changes Feb 8, 2017

View reviewed changes

thaJeztah mentioned this pull request Feb 9, 2017

topology-aware networking in Swarm Mode #30850

Open

aaronlehmann removed status/failing-ci Indicates that the PR in its current state fails the test suite status/needs-vendoring labels Feb 11, 2017

aaronlehmann force-pushed the topology branch from 51e819e to 5fb5563 Compare February 11, 2017 01:54

vdemeester approved these changes Feb 13, 2017

View reviewed changes

aaronlehmann force-pushed the topology branch from 5fb5563 to 53cb846 Compare February 13, 2017 17:43

GordonTheTurtle assigned dnephin Feb 18, 2017

aaronlehmann force-pushed the topology branch from 4b6c948 to 9e21a26 Compare February 21, 2017 17:49

aaronlehmann force-pushed the topology branch from 9e21a26 to b262eee Compare February 27, 2017 21:28

aaronlehmann force-pushed the topology branch from b262eee to 17288c6 Compare February 27, 2017 21:30

vieux added the rebuild/janky label Feb 27, 2017

GordonTheTurtle removed the rebuild/janky label Feb 27, 2017

thaJeztah approved these changes Mar 3, 2017

View reviewed changes

thaJeztah added impact/api impact/changelog impact/cli impact/documentation labels Mar 3, 2017

thaJeztah merged commit 3a5a1c3 into moby:master Mar 3, 2017

albers mentioned this pull request Mar 13, 2017

Add bash completion for topology-aware scheduling #31786

Merged

sdurrheimer mentioned this pull request Mar 22, 2017

Add zsh completion for 'docker service {create,update} --placement-pref' #32024

Merged

aaronlehmann deleted the topology branch April 5, 2017 21:32

aaronlehmann mentioned this pull request Apr 12, 2017

Support placement preferences in stack deploy #32584

Closed

dnephin pushed a commit to dnephin/docker that referenced this pull request Apr 17, 2017

Merge pull request moby#30725 from aaronlehmann/topology

ff11b05

Topology-aware scheduling

thaJeztah mentioned this pull request Nov 8, 2017

Placement Preferences not satisfying spread #35433

Closed

Conversation

aaronlehmann commented Feb 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aaronlehmann Feb 3, 2017

Choose a reason for hiding this comment

Uh oh!

dnephin left a comment

Choose a reason for hiding this comment

Uh oh!

aaronlehmann commented Feb 7, 2017

Uh oh!

vdemeester commented Feb 8, 2017

Uh oh!

vdemeester left a comment

Choose a reason for hiding this comment

Uh oh!

aaronlehmann commented Feb 8, 2017

Uh oh!

dnephin left a comment

Choose a reason for hiding this comment

Uh oh!

aaronlehmann commented Feb 11, 2017

Uh oh!

vdemeester left a comment

Choose a reason for hiding this comment

Uh oh!

vdemeester Feb 13, 2017

Choose a reason for hiding this comment

Uh oh!

aaronlehmann Feb 13, 2017

Choose a reason for hiding this comment

Uh oh!

aaronlehmann commented Feb 16, 2017

Uh oh!

dnephin commented Feb 21, 2017

Uh oh!

aaronlehmann commented Feb 21, 2017

Uh oh!

aluzzardi commented Feb 25, 2017

Uh oh!

aaronlehmann commented Feb 27, 2017

Uh oh!

dnephin commented Feb 27, 2017

Uh oh!

aaronlehmann commented Feb 27, 2017

Uh oh!

vieux commented Feb 27, 2017

Uh oh!

aaronlehmann commented Mar 1, 2017

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

davidhiendl commented Apr 12, 2017

Uh oh!

aaronlehmann commented Apr 12, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

aaronlehmann commented Feb 3, 2017 •

edited

Loading