[release/1.2 backport] Pin to libseccomp 2.3.3 by hakman · Pull Request #4015 · containerd/containerd

hakman · 2020-02-12T06:08:36Z

Besides the fact that lib seccomp 2.4 has huge performance regressions, it also breaks support for older distros like Debian 9 and RHEL/CentOS 7, as discussed in #4008.
This change pins to 2.3.3 where that is not an issue.

Fixes #4008.

(cherry picked from commits b5f03ea and 75d0c5f)
Signed-off-by: Ciprian Hacman ciprian.hacman@sematext.com

Zyqsempai

LGTM

hakman · 2020-02-12T09:47:37Z

@crosbymichael @thaJeztah seems that something is missing though:

Summarizing 1 Failure:
7110
7111[Fail] [k8s.io] Container runtime should support basic operations on container [It] runtime should support listing container stats [Conformance] 
7112/home/travis/gopath/src/github.com/kubernetes-sigs/cri-tools/pkg/validate/container.go:569
7113
7114Ran 71 of 78 Specs in 49.854 seconds
7115FAIL! -- 70 Passed | 1 Failed | 0 Pending | 7 Skipped
7116
7117
7118Ginkgo ran 1 suite in 49.906109744s
7119Test Suite Failed
7120--- FAIL: TestCRISuite (49.91s)
7121    cri_test.go:130: Failed to run tests in parallel: exit status 1
7122FAIL

estesp · 2020-02-13T05:28:46Z

I'm at a loss for why there are CRI test failures based on this PR. I don't see that the same things happened in master when libseccomp was pinned. Any ideas @Random-Liu?

hakman · 2020-02-13T10:52:35Z

Looks like it's not a flake. It consistently fails on:
[Fail] [k8s.io] Container runtime should support basic operations on container [It] runtime should support listing container stats [Conformance]

cpuguy83 · 2020-02-13T16:35:28Z

script/setup/install-seccomp

This doesn't seem like the right comment.

That's a mistake in master from the original commit. I just opened a PR to fix.

Ah on my phone didn't notice this was a backport.

Fixed the comment also. Thanks :)

hakman · 2020-02-14T04:54:49Z

@estesp I see the same test failure here: #4024.

lib seccomp 2.4 has huge performance regressions. This change pins to 2.3.3 where that is not an issue Signed-off-by: Michael Crosby <crosbymichael@gmail.com> (cherry picked from commit b5f03ea) Signed-off-by: Ciprian Hacman <ciprian.hacman@sematext.com>

Signed-off-by: Phil Estes <estesp@linux.vnet.ibm.com> (cherry picked from commit 75d0c5f) Signed-off-by: Ciprian Hacman <ciprian.hacman@sematext.com>

thaJeztah · 2020-02-14T13:47:13Z

Just to confirm; this holds back the version of libseccomp at compile time, so that the binary produced doesn't use the new seccomp_api_set feature, which is needed to allow compiling a binary to be compatible with both systems having libseccomp 2.3 and systems running libseccomp 2.4, correct?

hakman · 2020-02-14T13:52:29Z

@thaJeztah that is correct.

thaJeztah

LGTM

thanks; I know there's been some confusion about this in the past 😅

hakman · 2020-02-14T13:59:21Z

No problem. Thanks for looking into it. Any thoughts on when a new release could be expected?

thaJeztah · 2020-02-14T14:59:44Z

No idea yes, but likely soon, to address #4023

hakman · 2020-02-14T15:23:41Z

Sounds good. Looking forward to see these issues fixed.

codecov-io · 2020-02-17T05:23:05Z

Codecov Report

Merging #4015 into release/1.2 will increase coverage by 3.18%.
The diff coverage is n/a.

@@               Coverage Diff               @@
##           release/1.2    #4015      +/-   ##
===============================================
+ Coverage           41%   44.19%   +3.18%     
===============================================
  Files               70      100      +30     
  Lines             9537    10847    +1310     
===============================================
+ Hits              3911     4794     +883     
- Misses            5061     5313     +252     
- Partials           565      740     +175

Flag	Coverage Δ
#linux	`47.87% <ø> (?)`
#windows	`41% <ø> (ø)`	⬆️

Impacted Files	Coverage Δ
mount/temp_unix.go	`0% <0%> (ø)`
sys/reaper_linux.go	`0% <0%> (ø)`
services/server/server_linux.go	`0% <0%> (ø)`
sys/env.go	`100% <0%> (ø)`
sys/stat_unix.go	`0% <0%> (ø)`
runtime/v2/shim/shim_unix.go	`0% <0%> (ø)`
sys/reaper.go	`0% <0%> (ø)`
sys/fds.go	`0% <0%> (ø)`
sys/proc.go	`0% <0%> (ø)`
sys/mount_linux.go	`0% <0%> (ø)`
... and 26 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c2383a5...a7c9b76. Read the comment docs.

hakman · 2020-02-17T05:26:06Z

All tests are passing now, after kubernetes-sigs/cri-tools#574 was merged.

estesp

LGTM

The thirteenth patch release for `containerd` 1.2 fixes a regression introduced in v1.2.12 that caused container/shim to hang on single core machines, fixes an issue with blkio, and updates the Golang runtime to 1.12.17. Notable Updates ---------------------------------- * Fix container pid race condition [containerd#4025](containerd#4025) * Update containerd/cgroups dependency to address blkio issue [containerd#4001](containerd#4001) * Set octet-stream content-type on PUT request [containerd#4028](containerd#4028) * Pin to libseccomp 2.3.3 to preserve compatibility with hosts that do not have libseccomp 2.4 or higher installed [containerd#4015](containerd#4015) * Update Golang runtime to 1.12.17, which includes a fix to the runtime [containerd#4031](containerd#4031) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

Zyqsempai approved these changes Feb 12, 2020

View reviewed changes

Zyqsempai requested review from crosbymichael and thaJeztah February 12, 2020 09:22

hakman closed this Feb 13, 2020

hakman reopened this Feb 13, 2020

hakman closed this Feb 13, 2020

hakman reopened this Feb 13, 2020

hakman closed this Feb 13, 2020

hakman reopened this Feb 13, 2020

cpuguy83 requested changes Feb 13, 2020

View reviewed changes

crosbymichael and others added 2 commits February 14, 2020 12:58

Pin to libseccomp 2.3.3

6ad2553

lib seccomp 2.4 has huge performance regressions. This change pins to 2.3.3 where that is not an issue Signed-off-by: Michael Crosby <crosbymichael@gmail.com> (cherry picked from commit b5f03ea) Signed-off-by: Ciprian Hacman <ciprian.hacman@sematext.com>

Fix incorrect comment from copy/paste of starting script

a7c9b76

Signed-off-by: Phil Estes <estesp@linux.vnet.ibm.com> (cherry picked from commit 75d0c5f) Signed-off-by: Ciprian Hacman <ciprian.hacman@sematext.com>

hakman force-pushed the fix-libseccomp-ver branch from f7c1cbc to a7c9b76 Compare February 14, 2020 10:58

hakman mentioned this pull request Feb 14, 2020

[release/1.2 backport] Fix container pid race condition #4025

Merged

thaJeztah approved these changes Feb 14, 2020

View reviewed changes

hakman closed this Feb 17, 2020

hakman reopened this Feb 17, 2020

hakman closed this Feb 17, 2020

hakman reopened this Feb 17, 2020

hakman requested a review from estesp February 17, 2020 05:27

thaJeztah mentioned this pull request Feb 17, 2020

[release/1.2] Prepare v1.2.13 release #4033

Merged

5 tasks

estesp approved these changes Feb 17, 2020

View reviewed changes

estesp merged commit 01edb7c into containerd:release/1.2 Feb 17, 2020

hakman deleted the fix-libseccomp-ver branch June 12, 2020 06:04

thepwagner mentioned this pull request Jun 29, 2020

containerd-1.3.5: undefined symbol: seccomp_api_set #4349

Closed

Conversation

hakman commented Feb 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Zyqsempai left a comment

Choose a reason for hiding this comment

Uh oh!

hakman commented Feb 12, 2020

Uh oh!

estesp commented Feb 13, 2020

Uh oh!

hakman commented Feb 13, 2020

Uh oh!

cpuguy83 Feb 13, 2020

Choose a reason for hiding this comment

Uh oh!

estesp Feb 13, 2020

Choose a reason for hiding this comment

Uh oh!

cpuguy83 Feb 13, 2020

Choose a reason for hiding this comment

Uh oh!

cpuguy83 Feb 13, 2020

Choose a reason for hiding this comment

Uh oh!

hakman Feb 14, 2020

Choose a reason for hiding this comment

Uh oh!

hakman commented Feb 14, 2020

Uh oh!

thaJeztah commented Feb 14, 2020

Uh oh!

hakman commented Feb 14, 2020

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

hakman commented Feb 14, 2020

Uh oh!

thaJeztah commented Feb 14, 2020

Uh oh!

hakman commented Feb 14, 2020

Uh oh!

codecov-io commented Feb 17, 2020

Codecov Report

Uh oh!

hakman commented Feb 17, 2020

Uh oh!

estesp left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

hakman commented Feb 12, 2020 •

edited

Loading