Skip to content

Upgrade containerd version and update containerd executor#2595

Open
svenpopping wants to merge 9 commits into
moby:masterfrom
delftswa2018:upgrade-containerd-version
Open

Upgrade containerd version and update containerd executor#2595
svenpopping wants to merge 9 commits into
moby:masterfrom
delftswa2018:upgrade-containerd-version

Conversation

@svenpopping
Copy link
Copy Markdown
Contributor

In PR #2568 the "containerd" executor was removed, because it targeted an older version of containerd which was not being maintained anymore.

This PR aims to reintroduce the containerd executor (v1.0.2). This version is being actively maintained.

In the case this PR does not get merged, maybe the following lines, which seem to be related to the previous containerd implementation, should be removed:

[swarmd/main.go:288] mainCmd.Flags().String("containerd-addr", "", "Address of containerd instance of agent.")
[swarmd/main.go:289] mainCmd.Flags().String("containerd-namespace", "swarmd", "Namespace to use when using containerd agent.")

Signed-off-by: Sven Popping <sven@popping.co>
Signed-off-by: Sven Popping <sven@popping.co>
Signed-off-by: Sven Popping <sven@popping.co>
Signed-off-by: Sven Popping <sven@popping.co>
Signed-off-by: Sven Popping <sven@popping.co>
Signed-off-by: Sven Popping <sven@popping.co>
Signed-off-by: Sven Popping <sven@popping.co>
Signed-off-by: Sven Popping <sven@popping.co>
Signed-off-by: Sven Popping <sven@popping.co>
@svenpopping
Copy link
Copy Markdown
Contributor Author

The failing test is the same as in issue #2559

@stevvooe
Copy link
Copy Markdown
Contributor

stevvooe commented Apr 5, 2018

This is awesome!

My recommendation would be to update to the master branch of containerd. That client will keep us up to date but will work with 1.0+ daemons. There shouldn't too many changes.

@thaJeztah
Copy link
Copy Markdown
Member

ping @ijc @cyli @dperny PTAL

@svenpopping I opened a PR to (temporarily, if this PR gets accepted) remove the unused flags; #2704

If that one gets merged; you can revert the change as part of this PR 👍

@ijc
Copy link
Copy Markdown
Contributor

ijc commented Jul 11, 2018

I don't have bandwidth for a full review, but FWIW I'm happy to see the code resurrected and maintained. @stevvooe's recommendation to use the containerd master client is a good one, although it might be wise to arrange for CI testing of the different minor versions since you do need to take care which interfaces you use if you want to support older ones in practice.

Somewhat unrelated but IMHO the IDE related .gitignore changes belong locally, either in .git/info/exclude or $HOME/.gitignore not committed to every project.

@thaJeztah
Copy link
Copy Markdown
Member

Somewhat unrelated but IMHO the IDE related .gitignore changes belong locally, either in .git/info/exclude or $HOME/.gitignore not committed to every project.

Ah, yes wanted to point to https://github.com/moby/moby/blob/e5cce50c7ea727c1c901b6e1443bd4291d62fce3/.gitignore#L2-L3 - perhaps we should add a similar line in this repository

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants