vendor: Update libcontainer vendoring

[sboeuf]

The agent vendoring needs to be updated regarding the libcontainer
dependency so that it does not run into the following issue:

failed to write to cgroups.proc

when running kata-runtime exec commands.

Fixes #476

Signed-off-by: Sebastien Boeuf sebastien.boeuf@intel.com

[sboeuf]

Supersedes #477

[sboeuf]

/test

[sboeuf]

/test

[amshinde]

lgtm @sboeuf
Nit: I prefer to keep the vendoring and local changes in separate commits.

[sboeuf]

@amshinde ok let me split that :)

[sboeuf]

/test

[sboeuf]

@amshinde done! PTAL

[amshinde]

thanks @sboeuf !

[jodh-intel]

Thanks @sboeuf. Please can you add in the git shortlog showing the changes this re-vendoring introduces by inserting the output of the scriptlet here into the commit:

• https://github.com/kata-containers/community/blob/master/CONTRIBUTING.md#re-vendor-prs

[sboeuf]

/test

[sboeuf]

@jodh-intel commit message updated. Thanks for reminding me, I had totally forgotten about that...

[sboeuf]

@devimc
Even with this patch, the CI seems to be flaky... Could you take a quick look at what's happening here and if you find some other fixes that we need to land in order to make our CI more stable?

[jodh-intel]

Thanks @sboeuf.

lgtm

[devimc]

@sboeuf did you measure the memory footprint impact of this change? see opencontainers/runc@0a8e411#commitcomment-32280015

cc @grahamwhaley

I have a question

[devimc]

changing to dnm, I have a question

[grahamwhaley]

good catch @devimc - that had escaped my brain - yes, we should check with the metrics report generator at least @sboeuf

[sboeuf]

@devimc @grahamwhaley good point, no I haven't done this check since I thought you had done it when you advise to move to a new version of libcontainer.

[chavafg]

taking a look at the CI failures here:
I see a failure in the fedora job on docker commit test, which has been fixed by kata-containers/tests#1333.

The other issue I see is a failure in the shimv2 tests, which I haven't seen before:

STEP: test stop container
STEP: Stop container for containerID: 01a6cf6987d076eddc83f191dc663d75597c60b06efd7f28c030f3c6f1cd55e6
E0320 21:42:44.157256   17975 remote_runtime.go:229] StopContainer "01a6cf6987d076eddc83f191dc663d75597c60b06efd7f28c030f3c6f1cd55e6" from runtime service failed: rpc error: code = Unknown desc = failed to stop container "01a6cf6987d076eddc83f191dc663d75597c60b06efd7f28c030f3c6f1cd55e6": container not running: unknown
Mar 20 21:42:44.157: INFO: Unexpected error occurred: rpc error: code = Unknown desc = failed to stop container "01a6cf6987d076eddc83f191dc663d75597c60b06efd7f28c030f3c6f1cd55e6": container not running: unknown
Mar 20 21:43:44.138: INFO: stop container "01a6cf6987d076eddc83f191dc663d75597c60b06efd7f28c030f3c6f1cd55e6" timeout.

restarting jobs...

[sboeuf]

Thanks @chavafg for the quick feedback.
@grahamwhaley should we retrigger the metrics several times on this PR?

[grahamwhaley]

@sboeuf - I think the best way to look at impact is to use the report tool, as that way you get to do a side-by-side comparison of before/after on your machine, and the report shows a lot more useful detail than the metrics CI will.
If you only want to look at boot and memory impact, then use -t -d to the grabdata.sh in the tool, and that will save you a lot of time (won't run the fio tests for instance, that take ages).

[sboeuf]

Thanks @grahamwhaley
Any chance you can run the report tool @chavafg?

[chavafg]

I still see failures on shimv2 on fedora and initrd jobs.
From http://jenkins.katacontainers.io/job/kata-containers-agent-fedora-PR/428/console:

container_stats_test.go:318: Verify container stats for sandbox "bb773f8ec63ebb273a1d2a67957f1dfe8b3d84dc33fdcf150cb3fc5de62738c4" and container "139dd4b689a504a1859418084ec7fe40eb0ec64801428e38269ae1a3ed6a1886" filter
    assertions.go:237: 
                        
	Error Trace:	container_stats_test.go:274
        
			container_stats_test.go:323
        
	Error:		Received unexpected error rpc error: code = Unknown desc = failed to stop container "139dd4b689a504a1859418084ec7fe40eb0ec64801428e38269ae1a3ed6a1886": container not running: unknown
        
    assertions.go:237: 
                        
	Error Trace:	container_stats_test.go:270
        
			container_stats_test.go:323
        
	Error:		Received unexpected error rpc error: code = Unknown desc = failed to set removing state for container "139dd4b689a504a1859418084ec7fe40eb0ec64801428e38269ae1a3ed6a1886": container is still running, to stop first
        
FAIL
./hack/test-utils.sh: line 53: 105027 Terminated              keepalive "sudo PATH=${PATH} ${ROOT}/_output/containerd ${CONTAINERD_FLAGS}" 

from http://jenkins.katacontainers.io/job/kata-containers-agent-ubuntu-18-04-PR-initrd/362/console

    container_stats_test.go:318: Verify container stats for sandbox "db9e5827c204ca1310261749c40d1b2afcafb5653f4d8a2e7fe01e634c477a40" and container "39afc5e17c7bcc39cc638e71c66b8c2d2ea0ec19a683f420b4d18ff734fba67b" filter
    assertions.go:237: 
                        
	Error Trace:	container_stats_test.go:274
        
			container_stats_test.go:323
        
	Error:		Received unexpected error rpc error: code = Unknown desc = failed to stop container "39afc5e17c7bcc39cc638e71c66b8c2d2ea0ec19a683f420b4d18ff734fba67b": container not running: unknown
        
    assertions.go:237: 
                        
	Error Trace:	container_stats_test.go:270
        
			container_stats_test.go:323
        
	Error:		Received unexpected error rpc error: code = Unknown desc = failed to set removing state for container "39afc5e17c7bcc39cc638e71c66b8c2d2ea0ec19a683f420b4d18ff734fba67b": container is still running, to stop first

@sboeuf I'll run the tool and get back with results when got them.

[chavafg]

@sboeuf @grahamwhaley @devimc
I don't see much difference in footprint and boot time. But here is the report, can you ptal?
metrics_report.pdf

[grahamwhaley]

thx @chavafg - if anything, agent_pr490 looks a touch better - 5% drop in KSM footprint and 1.86% drop in time to workload. so, lgtm!

[sboeuf]

Thanks @chavafg for running this, very useful input!
So now it's just a matter of identifying what is going wrong with initrd and fedora jobs.

@chavafg is the shimv2 being tested in other jobs? Or are those jobs the only ones testing it?

[chavafg]

@sboeuf shimv2 is also being tested on the other jobs, but on these 2 jobs seem to fail constantly. Not sure if we are introducing some instabilities here... Will try to reproduce locally.

[sboeuf]

Removing do-not-merge label as the metrics results look good!

[sboeuf]

@lifupan could you please take a look at the issue related to the shim-v2 and reported by the CI as highlighted by @chavafg here

[bergwolf]

Removing do-not-merge label as the metrics results look good!

Hi @sboeuf, I don't think it can be measured with a simple boot metrics as we did before. With the cve-2019-5736 fix, each exec will copy one agent binary into memory. So if we launch a container and do 100 exec there, it can consume 100*(binary size of agent) MB of memory space (and it's not even counted as container memory, cc @egernst as it can also affect sandbox overhead even for runc). I don't think we are that kind of luxury in terms of extra memory in the guest. Runc guys also shared the concern of memory consumption but host memory is more available than the guest so it affects kata more than runc.

Adding back dnm until the concern of memory footprint is addressed.

[gnawux]

@bergwolf we need an in-depth discussion on this topic and have a decision on it.

[bergwolf]

@gnawux @sboeuf As I was trying to summarize the current situation, I found that the runc(libcontainer) has modified its fix to cve-2019-5736 from opencontainers/runc@0a8e411 to opencontainers/runc#1984.

The current fix of protecting the host runc binary works as follows:

1. Create a readonly bindmount (on container rootfs) to /proc/self/exe, open it readonly and umount it
2. If above doesn't work, fallback to copying to a memfd
3. If above doesn't work, fallback to copying to a O_TMPFILE file
4. If above doesn't work, fallback to copying to a local file with open-unlinking

Since the very first method does not involve any data copy, it consumes almost no additional memory overhead. Unless it falls for some reason, we won't see any visible memory impact for kata containers. For that reason, I'm giving the PR LGTM.

OTOH, if some day we implement readonly container rootfs support, the fix can still have memory footprint impact. A simple workaround is to create the bindmount to a file under /tmp directory if failing to create it under _LIBCONTAINER_STATEDIR. We might want to push such a change to libcontainer before adding readonly container rootfs support in kata containers.