Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update containerd client and dependencies to v1.2.0-rc.1 #37710

Merged
merged 2 commits into from
Oct 5, 2018
Merged

Update containerd client and dependencies to v1.2.0-rc.1 #37710

merged 2 commits into from
Oct 5, 2018

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Aug 23, 2018 •
edited

Full diffs and included changes:

full diff since rc.0: containerd/containerd@v1.2.0-rc.0...v1.2.0-rc.1

@thaJeztah thaJeztah requested a review from tianon as a code owner August 23, 2018 22:12
@kolyshkin
Copy link
Contributor

The runc bump apparently breaks docker-in-lxd (proposed fix: opencontainers/runc#1862) // cc @AkihiroSuda

@thaJeztah
Copy link
Member Author

The runc bump apparently breaks docker-in-lxd (proposed fix: opencontainers/runc#1862) // cc

Ah! Yes, I was actually thinking about that one when vendoring, but didn't check if the PR was merged 😓

@kolyshkin kolyshkin mentioned this pull request Aug 24, 2018
Closed
@codecov
Copy link

codecov bot commented Aug 28, 2018 •
edited

Codecov Report

❗ No coverage uploaded for pull request base (master@4f92583). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master   #37710   +/-   ##
=========================================
  Coverage          ?    36.1%           
=========================================
  Files             ?      610           
  Lines             ?    45136           
  Branches          ?        0           
=========================================
  Hits              ?    16298           
  Misses            ?    26603           
  Partials          ?     2235

@thaJeztah
Copy link
Member Author

Looks like this failure is pretty consistent on all Linux runs;

13:40:12 --- FAIL: TestContainerStartOnDaemonRestart (3.01s)
13:40:12 	daemon.go:290: [da6bebf524c56] waiting for daemon to start
13:40:12 	daemon.go:322: [da6bebf524c56] daemon started
13:40:12 	daemon.go:290: [da6bebf524c56] waiting for daemon to start
13:40:12 	daemon.go:322: [da6bebf524c56] daemon started
13:40:12 	daemon_linux_test.go:65: assertion failed: error is not nil: Error response from daemon: monitor task: cgroup is already being collected: unknown: failed to start test container
13:40:12 	daemon.go:280: [da6bebf524c56] exiting daemon

@kolyshkin kolyshkin mentioned this pull request Sep 11, 2018
Closed
@thaJeztah thaJeztah changed the title Update containerd and dependencies to 1.2.0-beta.1 Update containerd client and dependencies to v1.2.0-rc.0 Sep 27, 2018
@thaJeztah
Copy link
Member Author

@kolyshkin @AkihiroSuda - rebased this PR, and changed it to only update the vendored (client) code. I'll open a separate PR to update the containerd runtime/daemon and runc version

@thaJeztah
Copy link
Member Author

Ah, hm... build failing (on Windows); thought I tried it, but perhaps before some of these updates;

16:19:39 ..\..\vendor\github.com\containerd\containerd\images\archive\reference.go:22:2: cannot find package "github.com/containerd/cri/pkg/util" in any of:
16:19:39 	c:\go\src\github.com\docker\docker\vendor\github.com\containerd\cri\pkg\util (vendor tree)
16:19:39 	c:\go\src\vendor\github.com\containerd\cri\pkg\util
16:19:39 	C:\go\src\github.com\docker\docker\vendor\github.com\containerd\cri\pkg\util
16:19:39 	C:\go\src\vendor\github.com\containerd\cri\pkg\util
16:19:39 	c:\go\src\github.com\containerd\cri\pkg\util (from $GOROOT)
16:19:39 	C:\go\src\github.com\containerd\cri\pkg\util (from $GOPATH)

thaJeztah
thaJeztah commented Sep 27, 2018
View reviewed changes
vendor/github.com/containerd/containerd/images/archive/reference.go

func normalizeReference(ref string) (string, error) {
// TODO: Replace this function to not depend on reference package
normalized, err := util.NormalizeImageRef(ref)
Copy link
Member Author

Choose a reason for hiding this comment

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

Due to this line, containerd/cri is needed as a dependency. Looking at that code, it's a utility that's only a thin wrapper around code from docker/distribution. Might be good to refactor that and either move it to docker/distribution or copy the utility function;

// NormalizeImageRef normalizes the image reference following the docker convention. This is added
 // mainly for backward compatibility.
 // The reference returned can only be either tagged or digested. For reference contains both tag
 // and digest, the function returns digested reference, e.g. docker.io/library/busybox:latest@
 // sha256:7cc4b5aefd1d0cadf8d97d4350462ba51c694ebca145b08d7d41b41acc8db5aa will be returned as
 // docker.io/library/busybox@sha256:7cc4b5aefd1d0cadf8d97d4350462ba51c694ebca145b08d7d41b41acc8db5aa.
 func NormalizeImageRef(ref string) (reference.Named, error) {
 	named, err := reference.ParseNormalizedNamed(ref)
 	if err != nil {
 		return nil, err
 	}
 	if _, ok := named.(reference.NamedTagged); ok {
 		if canonical, ok := named.(reference.Canonical); ok {
 			// The reference is both tagged and digested, only
 			// return digested.
 			newNamed, err := reference.WithName(canonical.Name())
 			if err != nil {
 				return nil, err
 			}
 			newCanonical, err := reference.WithDigest(newNamed, canonical.Digest())
 			if err != nil {
 				return nil, err
 			}
 			return newCanonical, nil
 		}
 	}
 	return reference.TagNameOnly(named), nil
 }

Copy link
Member Author

Choose a reason for hiding this comment

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

@dmcgowan perhaps there's a utility elsewhere that already does that? ^^

@thaJeztah thaJeztah added this to backlog in maintainers-session Sep 27, 2018
@thaJeztah thaJeztah removed this from backlog in maintainers-session Sep 27, 2018
@thaJeztah
Copy link
Member Author

Updating containerd runtime in #37932

@thaJeztah thaJeztah mentioned this pull request Oct 3, 2018
Closed
@thaJeztah
Update containerd dependencies
31a9c9e 
This updates the containerd dependencies to match
the versions used by the vendored containerd version

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Update containerd client and dependencies to v1.2.0-rc.1
dd622c8 
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah changed the title Update containerd client and dependencies to v1.2.0-rc.0 Update containerd client and dependencies to v1.2.0-rc.1 Oct 4, 2018
@thaJeztah
Copy link
Member Author

@kolyshkin @AkihiroSuda updated to 1.2.0-rc.1 PTAL

@kolyshkin
Copy link
Contributor

LGTM, thanks @thaJeztah

@cpuguy83
Copy link
Member

cpuguy83 commented Oct 4, 2018

Are we desiring to merge the RC into master?

@thaJeztah
Copy link
Member Author

@cpuguy83 the current commit that's used is a random commit from master; if you prefer, we can wait for 1.2.0 GA (but updating the dependencies with each bump is a bit cumbersome 😅 )

@AkihiroSuda
Copy link
Member

either way, runc is still RC 😎

@thaJeztah thaJeztah mentioned this pull request Oct 5, 2018
Closed
@thaJeztah
Copy link
Member Author

@cpuguy83 @AkihiroSuda @vdemeester It's green.. good to go?

vdemeester
vdemeester approved these changes Oct 5, 2018
View reviewed changes
Copy link
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

LGTM 🐸

@vdemeester vdemeester merged commit c77cfbf into moby:master Oct 5, 2018
@thaJeztah thaJeztah deleted the bump_containerd_1.2.0_beta1 branch October 5, 2018 11:55
@thaJeztah thaJeztah mentioned this pull request Oct 5, 2018
Merged
@tiborvass
Copy link
Contributor

@thaJeztah Actually, I don't think we want to vendor containerd rc2 in the code. We want master. RC2 is for the binary.

@avagin avagin mentioned this pull request Mar 27, 2019
Closed
@rst0git rst0git mentioned this pull request Nov 8, 2020
Open
@Srinidhi2301
Copy link

hello everyone,
I do checkpoint restore on docker ( Tried on 18. ,19. , 20. versions)/ containerd (tried using several versions)
criu version: 3.11
ubuntu: 18.03
bionic arm64

Tried various combinations in version for docker and containerd from the below link

https://download.docker.com/linux/ubuntu/dists/bionic/pool/stable/arm64/

But still not successful.

In some versions, I get the below error,
Error response from daemon: failed to retrieve OCI runtime container pid: open /run/containerd/io.containerd.runtime.v1.linux/moby/1a09d8379341aba1f401a0944b52722b32edfbc56ac7b68010bcd5e5e0ae38e4/init.pid: no such file or directory: unknown

In some versions, the containers which i try to start from checkpoint does not start and print no logs.

As suggested above, I tried to use containerd containerd.io_1.2.0~rc.2-1_arm64.deb
but i was getting the below error,
Error response from daemon: failed to retrieve OCI runtime container pid: open /run/containerd/io.containerd.runtime.v1.linux/moby/1a09d8379341aba1f401a0944b52722b32edfbc56ac7b68010bcd5e5e0ae38e4/init.pid: no such file or directory: unknown

could anyone please help! Thanks in advance

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

@vdemeester vdemeester vdemeester approved these changes

@tianon tianon Awaiting requested review from tianon

Assignees

@cpuguy83 cpuguy83

Labels
area/runtime status/2-code-review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@thaJeztah @kolyshkin @cpuguy83 @AkihiroSuda @tiborvass @Srinidhi2301 @vdemeester @GordonTheTurtle