Swarm Mode: Swarm endpoints #686

Yogu · 2016-08-29T15:02:15Z

APIs for docker 1.12's swarm endpoint

This change is

KostyaSha · 2016-08-29T15:04:24Z

In english please. Is it swarm mode or swarm?

Yogu · 2016-08-29T15:09:41Z

Sorry for the comments, I made them for my coworker, they're already resolved. It's Swarm Mode. This is the basis for #678 and #673.

KostyaSha · 2016-08-29T15:35:20Z

How swarm mode could be tested?

Yogu · 2016-08-29T15:49:18Z

There are unit tests (though failing atm, they passed locally... I'll have to check), using dind when multiple docker engines are required at the same time, as you suggested.

Yogu · 2016-08-29T22:07:33Z

The build still fails, but only for unrelated tests. @KostyaSha, it would be cool if you could review this MR.

Btw., is it ok for this MR to have so many commits, or should I rebase?

KostyaSha · 2016-08-31T09:16:43Z

Github allows squash merge, so don't worry.

Yogu · 2016-09-01T22:25:15Z

@KostyaSha In KostyaSha/yet-another-docker-plugin#54 you wrote this PR is stuck because of integration tests, but in https://github.com/docker-java/docker-java/pull/673#issuecomment-240420512, you told me to ignore the failing tests - so I'm not quite sure how to proceed. It would be cool to get this one merged soon so that we can base the remaining three branches (Services, Nodes and Tasks) on master and use the required Swarm functionality there for the tests.

I realize you do this in your free time and I don't want to push you or anything, but in case I can do something to get this merged quicker, let me know.

KostyaSha · 2016-09-01T23:16:20Z

src/test/java/com/github/dockerjava/core/TestDockerCmdExecFactory.java


 import com.github.dockerjava.api.async.ResultCallback;
-import com.github.dockerjava.api.command.AttachContainerCmd;
+import com.github.dockerjava.api.command.*;


don't use hash imports.

KostyaSha · 2016-09-01T23:18:21Z

Could you move all SwarmMode integration tests into separate group?

KostyaSha · 2016-09-01T23:19:15Z

Branch start is not compilable.

KostyaSha · 2016-09-27T00:43:32Z

Could you rebase?

KostyaSha · 2016-09-27T00:46:08Z

@marcuslinke i will merge if didn't find any more issues.

- Swarm integration env - disable failing tests

KostyaSha · 2016-09-29T22:31:06Z

I made swarm branch for future 3.1.0 . What firstly could be merged for swarm-mode support? (i bit confused with all issues and prs already existing)

KostyaSha · 2016-09-29T22:33:11Z

src/main/java/com/github/dockerjava/api/model/ExternalCA.java

+     * @since 1.24
+     */
+    @JsonProperty("Protocol")
+    private ExternalCAProtocol protocol;


Why not string? I see alone enum.

The definition in the engine-api follows the pattern of enums.

KostyaSha · 2016-09-29T22:34:37Z

src/main/java/com/github/dockerjava/api/model/SwarmCAConfig.java

@@ -0,0 +1,63 @@
+package com.github.dockerjava.api.model;


maybe create and move to swarm package? I guess model folder will be huge soon.

even swarmmode as swarm will be for swarm?

KostyaSha · 2016-09-29T22:35:01Z

src/main/java/com/github/dockerjava/api/model/SwarmCAConfig.java

+    private Long nodeCertExpiry;
+
+    @JsonProperty("ExternalCAs")
+    private ExternalCA[] externalCA;


Please use List

KostyaSha · 2016-09-29T22:36:25Z

src/main/java/com/github/dockerjava/api/model/TaskDefaults.java

+     * @since 1.24
+     */
+    @JsonProperty("LogDriver")
+    private Driver logDriver;


Maybe name class LogDriver ?

It's called Driver in the engine-api too, and it's pretty generic, so might be the case that it is used for other drivers in the future.

KostyaSha · 2016-09-29T22:37:13Z

src/main/java/com/github/dockerjava/core/command/JoinSwarmCmdImpl.java

+    private String advertiseAddr;
+
+    @JsonProperty("RemoteAddrs")
+    private String[] remoteAddrs;


KostyaSha · 2016-09-29T22:39:37Z

src/test/java/com/github/dockerjava/core/command/AbstractSwarmDockerClientTest.java

+
+        final RemoteApiVersion apiVersion = getVersion(dockerClient);
+        if (!apiVersion.isGreaterOrEqual(RemoteApiVersion.VERSION_1_24)) {
+            throw new SkipException("API version should be >= 1.24");


For swarm branch it should also check isNotSwarm()

Where do I find this isNotSwarm?

KostyaSha · 2016-09-29T22:41:12Z

src/test/java/com/github/dockerjava/core/command/AbstractSwarmDockerClientTest.java

+    protected DockerClient secondDockerClient;
+    private int numberOfDockersInDocker = 0;
+
+    private static final int PORT_START = 2378;


If i right understand, then test should pick random port instead of hardcode. It only affects exposed port.

The port is the one that is used to communicate with the docker API. Picking it randomly increases chances of collision and makes it harder to debug. What are the advantages of randomly picking?

KostyaSha · 2016-09-29T22:43:09Z

Great work, still reviewing...

tecgie · 2016-11-01T18:28:51Z

I am waiting to use this swarm mode api as soon as it is merged!

KostyaSha · 2016-11-01T18:48:25Z

I created ITs, started merging into branch... AFAIR i asked some questions and didn't get response... Will continue on weekend.

Yogu · 2016-11-13T05:19:32Z

Sorry for the delay. We had to finish our project and due to lack of resources, we did not update these PRs but rather worked on one branch for everything swarm related.

These is the whole Swarm API code as we are using it. We'd like to get it merged, too, so that we can use the next official version.

It is a lot of work to separate it into the different APIs again as I tried to do with the PRs here. So the easiest for us would be to squash that big thing together and apply the comments of the PRs here.

We can also try to split the code again. Then, we would start with PR #673, and once this is merged, update the other PRs because they need some of the code of #673. I'll try to update that PR now.

Yogu · 2016-11-13T06:09:35Z

Sorry, I was wrong, this is the one to merge first, Service API comes after that. I rebased, applied some of your suggestions and replied to the others.

KostyaSha · 2016-11-13T16:02:22Z

Hi, i think we need merge everything into swarm branch, then play with it and merge.

KostyaSha · 2016-11-13T16:03:02Z

Oh, i can change base in GH UI now. WIll try, one sec.

Yogu · 2016-11-15T16:17:18Z

Err i'm not sure if I did this correctly, I rebased this branch on swarm, but now it's showing some commits that are not from us.

KostyaSha · 2016-11-15T16:19:36Z

Seems it pick commits that had conflicts. I usually doing rebase -i and commenting unneeded commits, then resolving conflicts on my commits.

Yogu · 2016-11-15T16:23:18Z

Ok, thanks.

Yogu · 2016-11-15T16:40:52Z

I think the build fails because of concurrent test runs. Do you think we can run the swarm tests sequentially? Otherwise, we need to assign random/sequential ports for the swarm listen address.

KostyaSha · 2016-11-15T16:47:24Z

I had no chance to look at tests, but it seems that running dind with random ports to initialize/etc will be the right solution. Also Host/port are injected in environments if you are creating docker-java client with default vars they may pick wrong values.

codecov-io · 2016-11-20T12:07:57Z

Codecov Report

❗ No coverage uploaded for pull request base (swarm@0c21e76). Click here to learn what that means.
The diff coverage is 65.66%.

@@           Coverage Diff            @@
##             swarm     #686   +/-   ##
========================================
  Coverage         ?   70.83%           
========================================
  Files            ?      334           
  Lines            ?     6864           
  Branches         ?      586           
========================================
  Hits             ?     4862           
  Misses           ?     1722           
  Partials         ?      280

Impacted Files	Coverage Δ
...n/java/com/github/dockerjava/api/model/Driver.java	`0% <0%> (ø)`
.../github/dockerjava/api/model/PullResponseItem.java	`50% <0%> (ø)`
...thub/dockerjava/netty/exec/UpdateSwarmCmdExec.java	`100% <100%> (ø)`
...ithub/dockerjava/jaxrs/InitializeSwarmCmdExec.java	`100% <100%> (ø)`
...b/dockerjava/jaxrs/JerseyDockerCmdExecFactory.java	`66.27% <100%> (ø)`
...om/github/dockerjava/api/model/LocalNodeState.java	`100% <100%> (ø)`
...hub/dockerjava/core/command/LeaveSwarmCmdImpl.java	`100% <100%> (ø)`
...ithub/dockerjava/api/model/ExternalCAProtocol.java	`100% <100%> (ø)`
.../com/github/dockerjava/jaxrs/JoinSwarmCmdExec.java	`100% <100%> (ø)`
...b/dockerjava/core/command/InspectSwarmCmdImpl.java	`100% <100%> (ø)`
... and 27 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0c21e76...b5ab795. Read the comment docs.

tecgie · 2016-11-26T21:58:23Z

When will this pull request be merged? I am looking forward to using the swarm mode.

cirocosta · 2016-12-05T12:13:38Z

Hey, so, in general it just lacks the tests passing, right? The issue is that concurrently they fail but serially they're all fine. Is this correct?

KostyaSha · 2017-05-05T02:26:12Z

picked in #717

Yogu changed the title ~~Docker swarm~~ Swarm Mode: Swarm endpoints Aug 29, 2016

avandorp mentioned this pull request Sep 1, 2016

Support for swarm mode in 1.12? KostyaSha/yet-another-docker-plugin#54

Open

KostyaSha reviewed Sep 1, 2016
View reviewed changes

KostyaSha added area/swarm-mode API/1.24 labels Sep 1, 2016

Yogu force-pushed the docker-swarm branch 2 times, most recently from f51447f to 8ebb085 Compare September 6, 2016 22:07

add swarm integration tests (docker-java#711)

84447bf

- Swarm integration env - disable failing tests

KostyaSha reviewed Sep 29, 2016

View reviewed changes

Yogu force-pushed the docker-swarm branch from 8ebb085 to d2943dc Compare November 13, 2016 05:56

Yogu mentioned this pull request Nov 13, 2016

[WIP] Service APIs for Docker 1.12 #673

Closed

KostyaSha changed the base branch from master to swarm November 13, 2016 16:03

Yogu force-pushed the docker-swarm branch from 050786c to 30ddec1 Compare November 15, 2016 16:14

martin894 and others added 4 commits November 16, 2016 00:20

Add endpoints for Swarm in swarm mode

3b9eb7c

Apply changes from review

bd308df

Remove unused import

c3fe1a7

Fix compile errors

b5ab795

Yogu force-pushed the docker-swarm branch from 30ddec1 to b5ab795 Compare November 15, 2016 16:21

KostyaSha force-pushed the swarm branch from 84447bf to 586f212 Compare February 13, 2017 22:06

KostyaSha mentioned this pull request Feb 13, 2017

Swarm branch, picking other prs #796

Merged

3 tasks

KostyaSha added this to the 3.1.0 milestone Feb 13, 2017

KostyaSha force-pushed the swarm branch 2 times, most recently from 3052ba0 to 0c21e76 Compare May 5, 2017 02:07

KostyaSha mentioned this pull request May 5, 2017

[WIP] swarm branch to future 3.1.0 master #717

Merged

8 tasks

KostyaSha closed this May 5, 2017

Swarm Mode: Swarm endpoints #686

Swarm Mode: Swarm endpoints #686

Uh oh!

Conversation

Yogu commented Aug 29, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

KostyaSha commented Aug 29, 2016

Uh oh!

Yogu commented Aug 29, 2016

Uh oh!

KostyaSha commented Aug 29, 2016

Uh oh!

Yogu commented Aug 29, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Yogu commented Aug 29, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

KostyaSha commented Aug 31, 2016

Uh oh!

Yogu commented Sep 1, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KostyaSha commented Sep 1, 2016

Uh oh!

KostyaSha commented Sep 1, 2016

Uh oh!

KostyaSha commented Sep 27, 2016

Uh oh!

KostyaSha commented Sep 27, 2016

Uh oh!

KostyaSha commented Sep 29, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KostyaSha commented Sep 29, 2016

Uh oh!

tecgie commented Nov 1, 2016

Uh oh!

KostyaSha commented Nov 1, 2016

Uh oh!

Yogu commented Nov 13, 2016

Uh oh!

Yogu commented Nov 13, 2016

Uh oh!

KostyaSha commented Nov 13, 2016

Uh oh!

KostyaSha commented Nov 13, 2016

Uh oh!

Yogu commented Nov 15, 2016

Uh oh!

KostyaSha commented Nov 15, 2016

Uh oh!

Yogu commented Nov 15, 2016

Uh oh!

Yogu commented Nov 15, 2016

Yogu commented Aug 29, 2016 •

edited

Loading

Yogu commented Aug 29, 2016 •

edited

Loading

Yogu commented Aug 29, 2016 •

edited

Loading

codecov-io commented Nov 20, 2016 •

edited

Loading

cirocosta commented Dec 5, 2016 •

edited

Loading