Arvados

very first take trying to create an abstraction that matches 1 to 1 with docker for now,

c1d1f0502b8a0f049dba41da2f6b19a0d4b03d77

developer-run-tests: #2380

TODO:

• [DONE] 0a27815bd review ContainerConfig, HostConfig settings and add them to the ThinContainerExecRunner interface as Get/Set methods to abstract from the internal representation
• review all the networking related options and see if can be simplified
• make a run with crunch-run --container-runner singularity to see how it behaves
• add tests related to singularity

Commit:0a27815bd 17241-singularity-take1

As I'm reading all the documentation available about singularity I want to write down some notes:

It is also important to note that the philosophy of Singularity is Integration over Isolation. Most container run times strive to isolate your container from the host system and other containers as much as possible. Singularity, on the other hand, assumes that the user’s primary goals are portability, reproducibility, and ease of use and that isolation is often a tertiary concern.
Therefore, Singularity only isolates the mount namespace by default, and will bind mount several host directories such as $HOME and /tmp into the container at runtime. If needed, additional levels of isolation can be achieved by passing options causing Singularity to enter any or all of the other kernel namespaces and to prevent automatic bind mounting. These measures allow users to interact with the host system from within the container in sensible ways.

(taken from https://sylabs.io/guides/3.7/user-guide/security.html )

I see a potential problem here, since singularity tries to incorporate the HOST files as part of the container in a transparent way, this could cause problems if crunch-run is running everything with the same user and maybe in a shared environment

fccb8a3de 17241-singularity-take1

Tom let's review this branch before I continue down the wrong path

43b39915bae3f3c24ab31cfbc7aefdce88f84dcb branch 17296-singularity-take2

we had a conversation with tom and we agree that lib/crunchrun/container_exec.go looks good to start making lib/crunchrun/container_exec_test.go to add all the tests for a contarner exec.

Added singularity v3 to compute{0,1}.9tee4, removed it from api.9tee4 (commit ecfae2b..4a694a1@saltstack)

e587495bf 17296-singularity-take2

17296-singularity @ 8fd331405028ebdbb97de58560057564aa530105 -- developer-run-tests: #2477

I think this is approximately functional enough to merge as alpha/experimental. More work is definitely needed to make it a fully supported feature.

• Container gateway won't work with singularity. (Todo: make sure it at least fails gracefully, and mention in docs.)
• Singularity doesn't even attempt to pass through VCPUs/memory. (I tried passing VCPUs but it just made the container fail to start.)
• Singularity "build" step (convert docker image) fails if squashfs-tools is not installed. (Should we try to report this more gracefully?)
• No cache for the docker-to-singularity image conversion.
• crunchstat.txt tracks host stats instead of container stats. (Haven't looked into how to do cgroups with singularity.)

• We now read the docker image from arv-mount instead of a ManifestFileReader. Might give us slightly better startup latency due to readahead?
• Rewrote a lot of the testing code. Tests are still rather copy-pastey and could use more cleanup but I felt like I needed to stop short of rewriting Everything.

• Config key "Containers.RuntimeEngine" make sense? Should it be Containers.CloudVMs.RuntimeEngine instead?
• I noticed our "load docker image" code assumes the sha256 in the tarball filename matches the ID of the image and probably behaves strangely if it doesn't. (I didn't try to fix this.)
• Very exciting, we finally have an integration test for crunch-run that uses the real apiserver, arv-mount, and docker/singularity.

17296-singularity @ 5a902f04bc1efdd30398195cb50df85ebab75d84 -- developer-run-tests: #2478

addresses a test failure (/var/lib/arvados/tmp isn't necessarily writable by the non-root user that runs tests).

Tom Clegg wrote:

17296-singularity @ 5a902f04bc1efdd30398195cb50df85ebab75d84 -- developer-run-tests: #2478

addresses a test failure (/var/lib/arvados/tmp isn't necessarily writable by the non-root user that runs tests).

• slight behavior change in loading docker images: before, the .tar file of the image had to be the first file in the collection, and that was loaded. Any subsequent .tar files were ignored. Now, it is an error if there is more than one .tar file. This change should probably be mentioned in the upgrade docs (though it's unlikely someone would run into it).

• since this branch does not add any documentation yet, I've created a story for Singularity documentation (#17726) to the epic and added a note there about the previous bullet.

• I think the config key "Containers.RuntimeEngine" makes sense; the plan is for singularity support to be available in non-cloud dispatchers as well, right?

• In RunCommand() (lib/crunchrun/crunchrun.go), line 1695 and later, would it make sense to print a warning for the first two conditions that cause the gateway service not to be started?

• Looks like the new integration test is failing because it can't find the docker binary. That is then followed by a panic in TearDownSuite (eek). I see you test for the presence of Docker in crunchrun/docker_test.go, but not yet in crunchrun/integration_test.go (developer-run-tests-remainder: #2592 /console):

14:24:59 ----------------------------------------------------------------------
14:24:59 FAIL: integration_test.go:38: integrationSuite.SetUpSuite
14:24:59 
14:24:59 
14:24:59 integration_test.go:43:
14:24:59     c.Assert(err, IsNil)
14:24:59 ... value *exec.Error = &exec.Error{Name:"docker", Err:(*errors.errorString)(0xc0000a83c0)} ("exec: \"docker\": executable file not found in $PATH")
14:24:59 
14:24:59 
14:24:59 ----------------------------------------------------------------------
14:24:59 PANIC: integration_test.go:76: integrationSuite.TearDownSuite
14:24:59 
14:24:59 ... Panic: runtime error: invalid memory address or nil pointer dereference (PC=0x43A7D8)
14:24:59 
14:24:59 /usr/src/go/src/runtime/panic.go:965
14:24:59   in gopanic
14:24:59 /usr/src/go/src/runtime/panic.go:212
14:24:59   in panicmem
14:24:59 /usr/src/go/src/runtime/signal_unix.go:734
14:24:59   in sigpanic
14:24:59 /tmp/workspace/developer-run-tests-remainder/sdk/go/arvados/client.go:321
14:24:59   in Client.RequestAndDecodeContext
14:24:59 /tmp/workspace/developer-run-tests-remainder/sdk/go/arvados/client.go:312
14:24:59   in Client.RequestAndDecode
14:24:59 integration_test.go:77
14:24:59   in integrationSuite.TearDownSuite
14:24:59 /usr/src/go/src/reflect/value.go:337
14:24:59   in Value.Call
14:24:59 /usr/src/go/src/runtime/asm_amd64.s:1371
14:24:59   in goexit

Other than that, LGTM.

developer-run-tests: #2495

• added note about docker image collections (will need to adjust before merging so it goes in the forthcoming "upgrading from 2.2" section instead of 2.1)
• added logs when not starting gateway service
• added docker check so tests should pass on jenkins

Tom Clegg wrote:

17296-singularity @ fb5cab780281c6cfb834cbfae64f3fcac7918d19 -- developer-run-tests: #2495

• added note about docker image collections (will need to adjust before merging so it goes in the forthcoming "upgrading from 2.2" section instead of 2.1)
• added logs when not starting gateway service
• added docker check so tests should pass on jenkins

LGTM, thanks!