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 runc to v1.0.0-rc91 and pickup latest containerd/cri #4357

Merged
merged 1 commit into from Jul 5, 2020
Merged

Update runc to v1.0.0-rc91 and pickup latest containerd/cri #4357

merged 1 commit into from Jul 5, 2020

Conversation

dims
Copy link
Member

@dims dims commented Jul 2, 2020 •
edited

https://github.com/opencontainers/runc/releases/tag/v1.0.0-rc91

There were changes in containerd/cri that needed to be picked up as well for v1.0.0-rc91 runc to work.

Signed-off-by: Davanum Srinivas davanum@gmail.com

@theopenlab-ci
Copy link

theopenlab-ci bot commented Jul 2, 2020

Build succeeded.

@theopenlab-ci
Copy link

theopenlab-ci bot commented Jul 2, 2020

Build succeeded.

@dims dims force-pushed the update-runc-to-v1.0.0-rc91 branch from 54f7a79 to 67e2a43 Compare July 2, 2020 02:53
@theopenlab-ci
Copy link

theopenlab-ci bot commented Jul 2, 2020

Build succeeded.

@dims dims changed the title Update runc to v1.0.0-rc91 [WIP] Update runc to v1.0.0-rc91 Jul 2, 2020
@AkihiroSuda AkihiroSuda mentioned this pull request Jul 2, 2020
Closed
@dims dims force-pushed the update-runc-to-v1.0.0-rc91 branch from 67e2a43 to e47e6be Compare July 2, 2020 03:36
@theopenlab-ci
Copy link

theopenlab-ci bot commented Jul 2, 2020

Build succeeded.

@AkihiroSuda
Copy link
Member

Needs containerd/cri#1521

@fuweid
Copy link
Member

fuweid commented Jul 2, 2020

Need to rebase because of #4356

@AkihiroSuda AkihiroSuda added this to the 1.4 milestone Jul 2, 2020
@AkihiroSuda AkihiroSuda mentioned this pull request Jul 2, 2020
Merged
thaJeztah
thaJeztah reviewed Jul 2, 2020
View reviewed changes
vendor.conf Outdated Show resolved Hide resolved
thaJeztah
thaJeztah reviewed Jul 2, 2020
View reviewed changes
vendor.conf Outdated
github.com/opencontainers/runc v1.0.0-rc10
github.com/opencontainers/runtime-spec v1.0.2
github.com/opencontainers/runc v1.0.0-rc91
github.com/opencontainers/runtime-spec 237cc4f519e2e8f9b235bacccfa8ef5a84df2875
Copy link
Member

Choose a reason for hiding this comment

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

😞 looks like this is needed for opencontainers/runc#2424. Let me open a request to have a new tag

Meanwhile, could you perhaps add a comment to describe which version this is?

Suggested change
github.com/opencontainers/runtime-spec 237cc4f519e2e8f9b235bacccfa8ef5a84df2875
github.com/opencontainers/runtime-spec 237cc4f519e2e8f9b235bacccfa8ef5a84df2875 # v1.0.2-8-g237cc4f

@thaJeztah
Copy link
Member

#4239 was merged, so you can drop the urfave, and gomd2man bumps from this PR

I also opened opencontainers/runtime-spec#1052 to ask the runtime-spec maintainers to consider a new release (not a blocker for this PR)

thaJeztah
thaJeztah reviewed Jul 2, 2020
View reviewed changes
vendor.conf Outdated
go.etcd.io/bbolt v1.3.5
go.opencensus.io v0.22.0
golang.org/x/xerrors 9bdfabe68543c54f90421aeb9a60ef8061b5b544
Copy link
Member

Choose a reason for hiding this comment

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

I'm new to this package. From the description:

This repository holds the transition packages for the new Go 1.13 error values.
See golang.org/design/29934-error-values.

Looks like this is for the new errors.Is(), errors.Unwrap() functionality in Go 1.13. We should look where it's used and if it's actually needed, now that all current go versions support that

Looks like it's used by https://github.com/cilium/ebpf; cilium/ebpf@5d50e74 (proposed in cilium/ebpf#38).

Perhaps we should propose them to drop Go 1.12 (as it's EOL), and use the standard errors package.

Copy link
Member

Choose a reason for hiding this comment

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

Opened cilium/ebpf#116 to remove this dependency

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks for filing cilium/ebpf#116 @thaJeztah !

Copy link
Member

Choose a reason for hiding this comment

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

cilium/ebpf#116 was accepted and merged, so we could potentially update that ebpf and not need the new dependency. @dims you may know if it's used in other places where it should be updated first?

hmm.. I see it's marked as a dependency of cgroups, which may be on an older version that doesn't have the x/xerrors dependency;

containerd/vendor.conf

Lines 56 to 57 in a6dd1f2

# cgroups dependencies
github.com/cilium/ebpf 4032b1d8aae306b7bb94a2a11002932caf88c644

Copy link
Member Author

Choose a reason for hiding this comment

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

yea, it's cgroups and runc. should be ok to just update both when we get a chance and be ahead of them for now i think. I'll update to newer ebpf in this PR shortly

Copy link
Member

Choose a reason for hiding this comment

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

Opened opencontainers/runc#2493 and containerd/cgroups#169

@dims dims force-pushed the update-runc-to-v1.0.0-rc91 branch from e47e6be to fa68b37 Compare July 2, 2020 10:13
@theopenlab-ci
Copy link

theopenlab-ci bot commented Jul 2, 2020

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.

@dims dims force-pushed the update-runc-to-v1.0.0-rc91 branch from fa68b37 to 9212101 Compare July 2, 2020 12:23
@theopenlab-ci
Copy link

theopenlab-ci bot commented Jul 2, 2020

Build succeeded.

@dims dims force-pushed the update-runc-to-v1.0.0-rc91 branch from 9212101 to 2dd0b74 Compare July 2, 2020 12:37
@theopenlab-ci
Copy link

theopenlab-ci bot commented Jul 2, 2020

Build succeeded.

@dims
Copy link
Member Author

dims commented Jul 2, 2020

linter fails with the following: We need @AkihiroSuda 's PR to land first in containerd/cri#1521

level=warning msg="[runner] Can't run linter unused: buildssa: analysis skipped: errors in package: [/home/runner/work/containerd/containerd/src/github.com/containerd/containerd/cmd/containerd/builtins_cri.go:21:10: could not import github.com/containerd/cri (/home/runner/work/containerd/containerd/src/github.com/containerd/containerd/vendor/github.com/containerd/cri/cri.go:45:2: could not import github.com/containerd/cri/pkg/server (/home/runner/work/containerd/containerd/src/github.com/containerd/containerd/vendor/github.com/containerd/cri/pkg/server/container_create.go:36:13: could not import github.com/containerd/cri/pkg/containerd/opts (/home/runner/work/containerd/containerd/src/github.com/containerd/containerd/vendor/github.com/containerd/cri/pkg/containerd/opts/spec_unix.go:327:13: cannot use dev.Permissions (variable of type configs.DevicePermissions) as string value in struct literal)))]"
level=warning msg="[runner] Can't run linter goanalysis_metalinter: S1005: failed prerequisites: inspect@github.com/containerd/containerd/cmd/containerd, isgenerated@github.com/containerd/containerd/cmd/containerd"

@dims dims force-pushed the update-runc-to-v1.0.0-rc91 branch from 2dd0b74 to 6b8f22a Compare July 3, 2020 22:28
@theopenlab-ci
Copy link

theopenlab-ci bot commented Jul 3, 2020

Build succeeded.

@dims dims force-pushed the update-runc-to-v1.0.0-rc91 branch from 6b8f22a to f186be3 Compare July 4, 2020 02:00
@theopenlab-ci
Copy link

theopenlab-ci bot commented Jul 4, 2020

Build succeeded.

@dims dims changed the title [WIP] Update runc to v1.0.0-rc91 Update runc to v1.0.0-rc91 and pickup latest containerd/cri Jul 4, 2020
@dims dims requested a review from dmcgowan July 4, 2020 11:56
@mikebrow
Copy link
Member

mikebrow commented Jul 4, 2020

need to look into that spurious critest pipe failure.. anyhow restarted tests and it's fine

mikebrow
mikebrow reviewed Jul 4, 2020
View reviewed changes
Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

See comment

vendor.conf Outdated Show resolved Hide resolved
@dims dims force-pushed the update-runc-to-v1.0.0-rc91 branch from f186be3 to a1eafdd Compare July 4, 2020 18:39
@theopenlab-ci
Copy link

theopenlab-ci bot commented Jul 4, 2020

Build succeeded.

@dims dims force-pushed the update-runc-to-v1.0.0-rc91 branch from a1eafdd to 963625d Compare July 4, 2020 18:46
@theopenlab-ci
Copy link

theopenlab-ci bot commented Jul 4, 2020

Build succeeded.

mikebrow
mikebrow approved these changes Jul 4, 2020
View reviewed changes
Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

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

LGTM

@AkihiroSuda AkihiroSuda merged commit 3919dbc into containerd:master Jul 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thaJeztah thaJeztah thaJeztah left review comments

@AkihiroSuda AkihiroSuda AkihiroSuda approved these changes

@mikebrow mikebrow mikebrow approved these changes

@dmcgowan dmcgowan Awaiting requested review from dmcgowan

Assignees

@AkihiroSuda AkihiroSuda

@mikebrow mikebrow

Labels
None yet
Projects
None yet
Milestone
1.4
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@dims @AkihiroSuda @fuweid @thaJeztah @mikebrow