Skip to content
This repository was archived by the owner on Oct 13, 2023. It is now read-only.

[17.11] update vndr of swarmkit to bbcec9b9#295

Merged
andrewhsu merged 2 commits into
docker-archive:17.11from
andrewhsu:sk
Nov 7, 2017
Merged

[17.11] update vndr of swarmkit to bbcec9b9#295
andrewhsu merged 2 commits into
docker-archive:17.11from
andrewhsu:sk

Conversation

@andrewhsu
Copy link
Copy Markdown
Contributor

@andrewhsu andrewhsu commented Nov 6, 2017

to fast-forward swarmkit code to commit moby/swarmkit@312be59 and bring in backport:

comparison of changes pulled in from upstream: moby/swarmkit@872861d...bbcec9b

$ cd components/engine
$ vi vendor.conf
$ make BIND_DIR=. shell
$ vndr github.com/docker/swarmkit

@andrewhsu
update vndr of swarmkit to bbcec9b9
f41a4e7 
Signed-off-by: Andrew Hsu <andrewhsu@docker.com>
@andrewhsu andrewhsu added this to the 17.11.0 milestone Nov 6, 2017
@andrewhsu andrewhsu changed the title update vndr of swarmkit to bbcec9b9 [17.11] update vndr of swarmkit to bbcec9b9 Nov 6, 2017
@andrewhsu
Copy link
Copy Markdown
Contributor Author

andrewhsu commented Nov 6, 2017 •
edited
Loading

Hmm...seeing failure in x86_64 job and s390x job:

FAIL: docker_cli_swarm_unix_test.go:15: DockerSwarmSuite.TestSwarmVolumePlugin

[d6b94cb304a98] waiting for daemon to start
[d6b94cb304a98] daemon started

docker_cli_swarm_unix_test.go:22:
    // Make sure task stays pending before plugin is available
    waitAndAssert(c, defaultReconciliationTimeout, d.CheckServiceTasksInState("top", swarm.TaskStatePending, "missing plugin on 1 node"), checker.Equals, 1)
docker_utils_test.go:465:
    c.Assert(v, checker, args...)
... obtained int = 0
... expected int = 1

@nishanttotla
Copy link
Copy Markdown
Contributor

nishanttotla commented Nov 6, 2017 •
edited
Loading

I wonder if this failure is related to moby/swarmkit#2392.

@nishanttotla
Copy link
Copy Markdown
Contributor

nishanttotla commented Nov 6, 2017 •
edited
Loading

I believe the test failure is actually related to: moby/swarmkit#2287

We'll need to fix the test itself to account for the change in error/message. Note that this commit is actually being vendored into 17.11 for the first time.

cc @dperny @andrewhsu

@nishanttotla
Fix error message for TestSwarmVolumePlugin
75d1e69 
Signed-off-by: Nishant Totla <nishanttotla@gmail.com>
@nishanttotla
Copy link
Copy Markdown
Contributor

nishanttotla commented Nov 7, 2017

Updated PR based on https://github.com/moby/moby/pull/34424/files#diff-d3de445335175bbc4849280e680d4534R22 (thanks to @thaJeztah)

@marcusmartins
Copy link
Copy Markdown
Contributor

marcusmartins commented Nov 7, 2017 •
edited
Loading

@anshulpundir @dperny can you review the latest commit 75d1e69?

@marcusmartins marcusmartins requested a review from dperny November 7, 2017 17:12
marcusmartins
marcusmartins reviewed Nov 7, 2017
View reviewed changes
Comment thread components/engine/integration-cli/daemon/daemon_swarm.go
var count int
for _, task := range tasks {
if task.Status.State == state {
if errorMessage == "" || strings.Contains(task.Status.Err, errorMessage) {
Copy link
Copy Markdown
Contributor

@marcusmartins marcusmartins Nov 7, 2017

Choose a reason for hiding this comment

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

@nishanttotla is this a API change that we have to worry about?

Copy link
Copy Markdown
Contributor

@nishanttotla nishanttotla Nov 7, 2017

Choose a reason for hiding this comment

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

This is an API change for sure, but I don't think we have to worry too much about it. In case someone outside is consuming this field, their workflow will break but it's not a huge change and not hard to fix.

@andrewhsu let me know if we haven't updated this in the release notes yet, I can help do that.

Copy link
Copy Markdown
Member

@thaJeztah thaJeztah Nov 7, 2017

Choose a reason for hiding this comment

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

Information is now output in another field, so yes, slight API change in that respect (also why I originally left it out in my summary of changes to cherry-pick), but probably ok if we mention it

@marcusmartins
Copy link
Copy Markdown
Contributor

marcusmartins commented Nov 7, 2017 •
edited
Loading

Left a question #295 (review) but otherwise LGTM

@vieux
Copy link
Copy Markdown
Contributor

vieux commented Nov 7, 2017

LGTM

@andrewhsu andrewhsu merged commit d0eae44 into docker-archive:17.11 Nov 7, 2017
@andrewhsu andrewhsu deleted the sk branch November 7, 2017 20:03
docker-jenkins pushed a commit that referenced this pull request Feb 13, 2019
@docker-unir
Merge pull request #295 from tonistiigi/rootless-extras
d5d7c28 
Merged with https://github.com/seemethere/unir
Upstream-commit: b41cb66
Component: packaging
docker-jenkins pushed a commit that referenced this pull request Aug 8, 2019
@andrewhsu
Merge pull request #295 from thaJeztah/18.09_backport_chroot_unsupported
e884c72 
[18.09 backport] Add realChroot for non linux/windows
Upstream-commit: 09431f90c89781b1a8051b1cbad563a821f470cc
Component: engine
silvin-lubecki pushed a commit to silvin-lubecki/docker-ce that referenced this pull request Feb 3, 2020
@andrewhsu
Merge pull request docker-archive#295 from andrewhsu/sk
6c74f7e 
[17.11] update vndr of swarmkit to bbcec9b9
akrasnov-drv pushed a commit to drivenets/docker-ce that referenced this pull request Apr 23, 2023
@docker-unir
Merge pull request docker-archive#295 from tonistiigi/rootless-extras
b41cb66 
Merged with https://github.com/seemethere/unir
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@dperny dperny Awaiting requested review from dperny

4 more reviewers

@marcusmartins marcusmartins marcusmartins left review comments

@thaJeztah thaJeztah thaJeztah left review comments

@nishanttotla nishanttotla nishanttotla approved these changes

@anshulpundir anshulpundir anshulpundir approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

17.11.0

Development

Successfully merging this pull request may close these issues.

6 participants

@andrewhsu @nishanttotla @marcusmartins @vieux @thaJeztah @anshulpundir