Skip to content

make engine support cluster config event#34032

Merged
thaJeztah merged 2 commits into
moby:masterfrom
allencloud:support-config-event-in-cluster
Jul 12, 2017
Merged

make engine support cluster config event#34032
thaJeztah merged 2 commits into
moby:masterfrom
allencloud:support-config-event-in-cluster

Conversation

@allencloud

@allencloud allencloud commented Jul 10, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

Signed-off-by: allencloud [email protected]

I found that docker-ce 17.06.0-ce has already supported cluster events, such as node, service, secret and so on. While I think there is still config events missing.

ping @dongluochen @aaronlehmann

- What I did

  1. make engine support cluster config event
  2. add a test case for config event
  3. update swagger.yml to add configs event

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@allencloud
allencloud force-pushed the support-config-event-in-cluster branch from 23fd254 to 64d1318 Compare July 10, 2017 05:20
@allencloud

allencloud commented Jul 10, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

I think both the CI failure is unrelated, and could anyone help to re-trigger them? @cpuguy83 @vdemeester @thaJeztah

@allencloud allencloud mentioned this pull request Jul 10, 2017
Closed
@vieux

vieux commented Jul 10, 2017

Copy link
Copy Markdown
Contributor

LGTM ping @aaronlehmann @aluzzardi

@aaronlehmann

aaronlehmann commented Jul 10, 2017

Copy link
Copy Markdown

LGTM

Does this need any doc updates?

@allencloud

allencloud commented Jul 10, 2017

Copy link
Copy Markdown
Contributor Author

Since this change will affect the event, I think swagger.yml needs updated. And I hope to add this in PR #34035. Because there are more things missing related to cluster events.
Please remind me if I miss something. @thaJeztah

@allencloud allencloud mentioned this pull request Jul 11, 2017
Merged
@allencloud
allencloud force-pushed the support-config-event-in-cluster branch from 64d1318 to 7d2764e Compare July 11, 2017 09:23
@allencloud

allencloud commented Jul 11, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Since #34035 has been merged, I started to add doc update for config events in swagger.yml. PTAL @aaronlehmann @thaJeztah

@aaronlehmann

aaronlehmann commented Jul 11, 2017

Copy link
Copy Markdown

LGTM

thaJeztah
thaJeztah approved these changes Jul 11, 2017
View reviewed changes

@thaJeztah thaJeztah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@thaJeztah

thaJeztah commented Jul 11, 2017

Copy link
Copy Markdown
Member

oh, actually; can you add a line to https://github.com/moby/moby/blob/master/docs/api/version-history.md ?

@thaJeztah

thaJeztah commented Jul 11, 2017

Copy link
Copy Markdown
Member

hmm, flakiness? https://jenkins.dockerproject.org/job/Docker-PRs-experimental/35595/console

10:27:50 ----------------------------------------------------------------------
10:27:50 FAIL: docker_api_swarm_test.go:281: DockerSwarmSuite.TestAPISwarmLeaderElection
10:27:50 
10:27:50 [d44619f297605] waiting for daemon to start
10:27:50 [d44619f297605] daemon started
10:27:50 
10:27:50 [d2a7f9c3887d5] waiting for daemon to start
10:27:50 [d2a7f9c3887d5] daemon started
10:27:50 
10:27:50 [d7c4c91a9d93d] waiting for daemon to start
10:27:50 [d7c4c91a9d93d] daemon started
10:27:50 
10:27:50 [d44619f297605] exiting daemon
10:27:50 [d44619f297605] waiting for daemon to start
10:27:50 [d44619f297605] daemon started
10:27:50 
10:27:50 docker_api_swarm_test.go:341:
10:27:50     // and that after we added d1 back, the leader hasn't changed
10:27:50     c.Assert(leader.NodeID, checker.Equals, stableleader.NodeID)
10:27:50 ... obtained string = "40kbahq4jhfxfsevo4kfzavjl"
10:27:50 ... expected string = "acz541uxpwsb9820dwu6b8qt5"
10:27:50 
10:27:50 [d44619f297605] exiting daemon
10:27:50 [d2a7f9c3887d5] exiting daemon
10:27:50 [d7c4c91a9d93d] exiting daemon
10:28:25

@allencloud
allencloud force-pushed the support-config-event-in-cluster branch from 7d2764e to c8d6477 Compare July 12, 2017 00:48
@allencloud

allencloud commented Jul 12, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Oh, I missed version_history.md. @thaJeztah
In this PR, I will add version change of config events in api version 1.31.
However, I also find that service, node and secret events are also missing in api version 1.30.
And I will do a follow-up #34065 to add these.

thaJeztah
thaJeztah approved these changes Jul 12, 2017
View reviewed changes

@thaJeztah thaJeztah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks!

@allencloud

allencloud commented Jul 12, 2017

Copy link
Copy Markdown
Contributor Author

Oh, it is great to see all green, since recently we have experienced lot of test failure. 😄

@thaJeztah

thaJeztah commented Jul 12, 2017

Copy link
Copy Markdown
Member

Oh, it is great to see all green, since recently we have experienced lot of test failure

This PR should resolve some of the test failures / flakyness we saw; #34061

@thaJeztah

thaJeztah commented Jul 12, 2017

Copy link
Copy Markdown
Member

And, yes, all green 💚 Thanks!

@thaJeztah
thaJeztah merged commit 334702a into moby:master Jul 12, 2017
@allencloud
allencloud deleted the support-config-event-in-cluster branch July 12, 2017 03:26
@andrewhsu andrewhsu mentioned this pull request Aug 15, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thaJeztah thaJeztah thaJeztah approved these changes

Assignees

No one assigned

Labels

impact/api impact/changelog status/2-code-review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@allencloud @vieux @aaronlehmann @thaJeztah @GordonTheTurtle