[ARVADOS] created: 2.1.0-2456-g503860c34

Git user git at public.arvados.org
Fri May 13 19:35:06 UTC 2022


        at  503860c347e620432ee501c1edc245fca94bf729 (commit)


commit 503860c347e620432ee501c1edc245fca94bf729
Author: Tom Clegg <tom at curii.com>
Date:   Fri May 13 15:34:26 2022 -0400

    19099: Fix singularity config script.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/install/deps.go b/lib/install/deps.go
index 2d9da72b9..0d4fe7e9d 100644
--- a/lib/install/deps.go
+++ b/lib/install/deps.go
@@ -338,11 +338,14 @@ make -C ./builddir install
 			}
 		}
 
+		// Allow users in the "sudo" group to use
+		// --network=bridge without --fakeroot. (Currently
+		// tests use --fakeroot anyway.)
 		err = inst.runBash(`
 install /usr/bin/nsenter /var/lib/arvados/bin/nsenter
 setcap "cap_sys_admin+pei cap_sys_chroot+pei" /var/lib/arvados/bin/nsenter
-singularity config global --set 'allow net networks' bridge
-singularity config global --set 'allow net groups' sudo
+/var/lib/arvados/bin/singularity config global --set 'allow net networks' bridge
+/var/lib/arvados/bin/singularity config global --set 'allow net groups' sudo
 `, stdout, stderr)
 		if err != nil {
 			return 1

commit 8c52491b9feedc523f5aa30721a5a496ebfe7ea6
Author: Tom Clegg <tom at curii.com>
Date:   Fri May 13 13:39:44 2022 -0400

    19099: docker is optional for run-tests.sh.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/build/run-tests.sh b/build/run-tests.sh
index ae368585e..4fbb4e6f0 100755
--- a/build/run-tests.sh
+++ b/build/run-tests.sh
@@ -269,13 +269,13 @@ sanity_checks() {
     echo -n 'graphviz: '
     dot -V || fatal "No graphviz. Try: apt-get install graphviz"
     echo -n 'geckodriver: '
-    geckodriver --version | grep ^geckodriver || echo "No geckodriver. Try: wget -O- https://github.com/mozilla/geckodriver/releases/download/v0.23.0/geckodriver-v0.23.0-linux64.tar.gz | sudo tar -C /usr/local/bin -xzf - geckodriver"
+    geckodriver --version | grep ^geckodriver || echo "No geckodriver. Try: arvados-server install"
     echo -n 'singularity: '
     singularity --version || fatal "No singularity. Try: arvados-server install"
     echo -n 'docker client: '
-    docker --version || fatal "No docker client. Try: arvados-server install"
+    docker --version || echo "No docker client. Try: arvados-server install"
     echo -n 'docker server: '
-    docker info --format='{{.ServerVersion}}' || fatal "No docker server. Try: arvados-server install"
+    docker info --format='{{.ServerVersion}}' || echo "No docker server. Try: arvados-server install"
 
     if [[ "$NEED_SDK_R" = true ]]; then
       # R SDK stuff

commit 5331fde2afc6224b10e828ad09f3ffe05f7f4e5e
Author: Tom Clegg <tom at curii.com>
Date:   Fri May 13 13:22:36 2022 -0400

    19099: Show lsns debug info if test fails.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/crunchrun/executor_test.go b/lib/crunchrun/executor_test.go
index c516a8b98..dcb4265c6 100644
--- a/lib/crunchrun/executor_test.go
+++ b/lib/crunchrun/executor_test.go
@@ -12,6 +12,7 @@ import (
 	"net"
 	"net/http"
 	"os"
+	"os/exec"
 	"strings"
 	"time"
 
@@ -200,6 +201,9 @@ func (s *executorSuite) TestIPAddress(c *C) {
 		resp, err := http.DefaultClient.Do(req)
 		c.Assert(err, IsNil)
 		c.Check(resp.StatusCode, Equals, http.StatusTeapot)
+	} else {
+		lsns, err := exec.Command("lsns").CombinedOutput()
+		c.Logf("lsns (err == %v):\n%s", err, lsns)
 	}
 
 	s.executor.Stop()

commit 3ad49eb34f3c7c30588af3cab6a3c9a593dcd5ad
Author: Tom Clegg <tom at curii.com>
Date:   Fri May 13 13:08:59 2022 -0400

    19099: Log singularity and docker versions in test runs.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/build/run-tests.sh b/build/run-tests.sh
index 0f996f77e..ae368585e 100755
--- a/build/run-tests.sh
+++ b/build/run-tests.sh
@@ -270,6 +270,12 @@ sanity_checks() {
     dot -V || fatal "No graphviz. Try: apt-get install graphviz"
     echo -n 'geckodriver: '
     geckodriver --version | grep ^geckodriver || echo "No geckodriver. Try: wget -O- https://github.com/mozilla/geckodriver/releases/download/v0.23.0/geckodriver-v0.23.0-linux64.tar.gz | sudo tar -C /usr/local/bin -xzf - geckodriver"
+    echo -n 'singularity: '
+    singularity --version || fatal "No singularity. Try: arvados-server install"
+    echo -n 'docker client: '
+    docker --version || fatal "No docker client. Try: arvados-server install"
+    echo -n 'docker server: '
+    docker info --format='{{.ServerVersion}}' || fatal "No docker server. Try: arvados-server install"
 
     if [[ "$NEED_SDK_R" = true ]]; then
       # R SDK stuff

commit e30b7ec3040cac89a2e134fddf8cb47c1905ea82
Author: Tom Clegg <tom at curii.com>
Date:   Fri May 13 11:08:20 2022 -0400

    19099: Update tests to new crunchrun.Gateway fields.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/cmd/arvados-client/container_gateway_test.go b/cmd/arvados-client/container_gateway_test.go
index 89e926f59..f4a140c40 100644
--- a/cmd/arvados-client/container_gateway_test.go
+++ b/cmd/arvados-client/container_gateway_test.go
@@ -49,16 +49,14 @@ func (s *ClientSuite) TestShellGateway(c *check.C) {
 	h := hmac.New(sha256.New, []byte(arvadostest.SystemRootToken))
 	fmt.Fprint(h, uuid)
 	authSecret := fmt.Sprintf("%x", h.Sum(nil))
-	dcid := "theperthcountyconspiracy"
 	gw := crunchrun.Gateway{
-		DockerContainerID: &dcid,
-		ContainerUUID:     uuid,
-		Address:           "0.0.0.0:0",
-		AuthSecret:        authSecret,
+		ContainerUUID: uuid,
+		Address:       "0.0.0.0:0",
+		AuthSecret:    authSecret,
 		// Just forward connections to localhost instead of a
 		// container, so we can test without running a
 		// container.
-		ContainerIPAddress: func() (string, error) { return "0.0.0.0", nil },
+		Target: crunchrun.GatewayTargetStub{},
 	}
 	err := gw.Start()
 	c.Assert(err, check.IsNil)
@@ -88,9 +86,8 @@ func (s *ClientSuite) TestShellGateway(c *check.C) {
 	cmd.Env = append(cmd.Env, "ARVADOS_API_TOKEN="+arvadostest.ActiveTokenV2)
 	cmd.Stdout = &stdout
 	cmd.Stderr = &stderr
-	c.Check(cmd.Run(), check.NotNil)
-	c.Log(stderr.String())
-	c.Check(stderr.String(), check.Matches, `(?ms).*(No such container: theperthcountyconspiracy|exec: \"docker\": executable file not found in \$PATH).*`)
+	c.Check(cmd.Run(), check.IsNil)
+	c.Check(stdout.String(), check.Equals, "ok\n")
 
 	// Set up an http server, and try using "arvados-client shell"
 	// to forward traffic to it.
diff --git a/lib/controller/localdb/container_gateway_test.go b/lib/controller/localdb/container_gateway_test.go
index 70037cc50..271760420 100644
--- a/lib/controller/localdb/container_gateway_test.go
+++ b/lib/controller/localdb/container_gateway_test.go
@@ -56,12 +56,11 @@ func (s *ContainerGatewaySuite) SetUpSuite(c *check.C) {
 	authKey := fmt.Sprintf("%x", h.Sum(nil))
 
 	s.gw = &crunchrun.Gateway{
-		DockerContainerID:  new(string),
-		ContainerUUID:      s.ctrUUID,
-		AuthSecret:         authKey,
-		Address:            "localhost:0",
-		Log:                ctxlog.TestLogger(c),
-		ContainerIPAddress: func() (string, error) { return "localhost", nil },
+		ContainerUUID: s.ctrUUID,
+		AuthSecret:    authKey,
+		Address:       "localhost:0",
+		Log:           ctxlog.TestLogger(c),
+		Target:        crunchrun.GatewayTargetStub{},
 	}
 	c.Assert(s.gw.Start(), check.IsNil)
 	rootctx := auth.NewContext(context.Background(), &auth.Credentials{Tokens: []string{s.cluster.SystemRootToken}})
diff --git a/lib/crunchrun/container_gateway.go b/lib/crunchrun/container_gateway.go
index 62979da21..01457015e 100644
--- a/lib/crunchrun/container_gateway.go
+++ b/lib/crunchrun/container_gateway.go
@@ -36,6 +36,13 @@ type GatewayTarget interface {
 	IPAddress() (string, error)
 }
 
+type GatewayTargetStub struct{}
+
+func (GatewayTargetStub) IPAddress() (string, error) { return "127.0.0.1", nil }
+func (GatewayTargetStub) InjectCommand(ctx context.Context, detachKeys, username string, usingTTY bool, cmd []string) (*exec.Cmd, error) {
+	return exec.CommandContext(ctx, cmd[0], cmd[1:]...), nil
+}
+
 type Gateway struct {
 	ContainerUUID string
 	Address       string // listen host:port; if port=0, Start() will change it to the selected port
diff --git a/lib/crunchrun/executor_test.go b/lib/crunchrun/executor_test.go
index ea8eedaa1..c516a8b98 100644
--- a/lib/crunchrun/executor_test.go
+++ b/lib/crunchrun/executor_test.go
@@ -183,7 +183,7 @@ func (s *executorSuite) TestIPAddress(c *C) {
 	c.Assert(s.executor.Start(), IsNil)
 	starttime := time.Now()
 
-	ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(2*time.Second))
+	ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(10*time.Second))
 	defer cancel()
 
 	for ctx.Err() == nil {

commit 2a21ea7ddc0739f9ce54589600be7f136ddd83fa
Author: Tom Clegg <tom at curii.com>
Date:   Fri May 13 10:23:21 2022 -0400

    19099: Use --fakeroot to test network isolation without being root.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/crunchrun/singularity.go b/lib/crunchrun/singularity.go
index 921f58ff0..f154728c7 100644
--- a/lib/crunchrun/singularity.go
+++ b/lib/crunchrun/singularity.go
@@ -12,6 +12,7 @@ import (
 	"net"
 	"os"
 	"os/exec"
+	"os/user"
 	"regexp"
 	"sort"
 	"strconv"
@@ -24,6 +25,7 @@ import (
 
 type singularityExecutor struct {
 	logf          func(string, ...interface{})
+	fakeroot      bool // use --fakeroot flag, allow --network=bridge when non-root (currently only used by tests)
 	spec          containerSpec
 	tmpdir        string
 	child         *exec.Cmd
@@ -247,9 +249,21 @@ func (e *singularityExecutor) Create(spec containerSpec) error {
 }
 
 func (e *singularityExecutor) execCmd(path string) *exec.Cmd {
-	args := []string{path, "exec", "--containall", "--cleanenv", "--pwd", e.spec.WorkingDir, "--net"}
+	args := []string{path, "exec", "--containall", "--cleanenv", "--pwd=" + e.spec.WorkingDir}
+	if e.fakeroot {
+		args = append(args, "--fakeroot")
+	}
 	if !e.spec.EnableNetwork {
-		args = append(args, "--network=none")
+		args = append(args, "--net", "--network=none")
+	} else if u, err := user.Current(); err == nil && u.Uid == "0" || e.fakeroot {
+		// Specifying --network=bridge fails unless (a) we are
+		// root, (b) we are using --fakeroot, or (c)
+		// singularity has been configured to allow our
+		// uid/gid to use it like so:
+		//
+		// singularity config global --set 'allow net networks' bridge
+		// singularity config global --set 'allow net groups' mygroup
+		args = append(args, "--net", "--network=bridge")
 	}
 	if e.spec.CUDADeviceCount != 0 {
 		args = append(args, "--nv")
diff --git a/lib/crunchrun/singularity_test.go b/lib/crunchrun/singularity_test.go
index 8a2e62d7e..2bad082ba 100644
--- a/lib/crunchrun/singularity_test.go
+++ b/lib/crunchrun/singularity_test.go
@@ -28,6 +28,21 @@ func (s *singularitySuite) SetUpSuite(c *C) {
 	}
 }
 
+func (s *singularitySuite) TearDownSuite(c *C) {
+	if s.executor != nil {
+		s.executor.Close()
+	}
+}
+
+func (s *singularitySuite) TestIPAddress(c *C) {
+	// In production, executor will choose --network=bridge
+	// because uid=0 under arvados-dispatch-cloud. But in test
+	// cases, uid!=0, which means --network=bridge is conditional
+	// on --fakeroot.
+	s.executor.(*singularityExecutor).fakeroot = true
+	s.executorSuite.TestIPAddress(c)
+}
+
 func (s *singularitySuite) TestInject(c *C) {
 	path, err := exec.LookPath("nsenter")
 	if err != nil || path != "/var/lib/arvados/bin/nsenter" {
@@ -55,6 +70,6 @@ func (s *singularityStubSuite) TestSingularityExecArgs(c *C) {
 	c.Check(err, IsNil)
 	e.imageFilename = "/fake/image.sif"
 	cmd := e.execCmd("./singularity")
-	c.Check(cmd.Args, DeepEquals, []string{"./singularity", "exec", "--containall", "--cleanenv", "--pwd", "/WorkingDir", "--net", "--network=none", "--nv", "--bind", "/hostpath:/mnt:ro", "/fake/image.sif"})
+	c.Check(cmd.Args, DeepEquals, []string{"./singularity", "exec", "--containall", "--cleanenv", "--pwd=/WorkingDir", "--net", "--network=none", "--nv", "--bind", "/hostpath:/mnt:ro", "/fake/image.sif"})
 	c.Check(cmd.Env, DeepEquals, []string{"SINGULARITYENV_FOO=bar"})
 }

commit 43fdb06a1620d926bdaec00582c82a4190805d86
Author: Tom Clegg <tom at curii.com>
Date:   Fri May 13 02:43:14 2022 -0400

    19099: Enable container shell when using singularity runtime.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/crunchrun/executor_test.go b/lib/crunchrun/executor_test.go
index 1c963f921..ea8eedaa1 100644
--- a/lib/crunchrun/executor_test.go
+++ b/lib/crunchrun/executor_test.go
@@ -6,6 +6,7 @@ package crunchrun
 
 import (
 	"bytes"
+	"fmt"
 	"io"
 	"io/ioutil"
 	"net"
@@ -208,7 +209,11 @@ func (s *executorSuite) TestIPAddress(c *C) {
 }
 
 func (s *executorSuite) TestInject(c *C) {
+	hostdir := c.MkDir()
+	c.Assert(os.WriteFile(hostdir+"/testfile", []byte("first tube"), 0777), IsNil)
+	mountdir := fmt.Sprintf("/injecttest-%d", os.Getpid())
 	s.spec.Command = []string{"sleep", "10"}
+	s.spec.BindMounts = map[string]bindmount{mountdir: {HostPath: hostdir, ReadOnly: true}}
 	c.Assert(s.executor.Create(s.spec), IsNil)
 	c.Assert(s.executor.Start(), IsNil)
 	starttime := time.Now()
@@ -216,13 +221,23 @@ func (s *executorSuite) TestInject(c *C) {
 	ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(2*time.Second))
 	defer cancel()
 
-	injectcmd := []string{"cat", "/proc/1/cmdline"}
+	// Allow InjectCommand to fail a few times while the container
+	// is starting
+	for ctx.Err() == nil {
+		_, err := s.executor.InjectCommand(ctx, "", "root", false, []string{"true"})
+		if err == nil {
+			break
+		}
+		time.Sleep(time.Second / 10)
+	}
+
+	injectcmd := []string{"cat", mountdir + "/testfile"}
 	cmd, err := s.executor.InjectCommand(ctx, "", "root", false, injectcmd)
 	c.Assert(err, IsNil)
 	out, err := cmd.CombinedOutput()
 	c.Logf("inject %s => %q", injectcmd, out)
 	c.Check(err, IsNil)
-	c.Check(string(out), Equals, "sleep\00010\000")
+	c.Check(string(out), Equals, "first tube")
 
 	s.executor.Stop()
 	code, _ := s.executor.Wait(ctx)
diff --git a/lib/crunchrun/singularity.go b/lib/crunchrun/singularity.go
index 6ba65200d..921f58ff0 100644
--- a/lib/crunchrun/singularity.go
+++ b/lib/crunchrun/singularity.go
@@ -5,12 +5,16 @@
 package crunchrun
 
 import (
+	"bytes"
 	"errors"
 	"fmt"
 	"io/ioutil"
+	"net"
 	"os"
 	"os/exec"
+	"regexp"
 	"sort"
+	"strconv"
 	"syscall"
 	"time"
 
@@ -243,11 +247,10 @@ func (e *singularityExecutor) Create(spec containerSpec) error {
 }
 
 func (e *singularityExecutor) execCmd(path string) *exec.Cmd {
-	args := []string{path, "exec", "--containall", "--cleanenv", "--pwd", e.spec.WorkingDir}
+	args := []string{path, "exec", "--containall", "--cleanenv", "--pwd", e.spec.WorkingDir, "--net"}
 	if !e.spec.EnableNetwork {
-		args = append(args, "--net", "--network=none")
+		args = append(args, "--network=none")
 	}
-
 	if e.spec.CUDADeviceCount != 0 {
 		args = append(args, "--nv")
 	}
@@ -352,9 +355,116 @@ func (e *singularityExecutor) Close() {
 }
 
 func (e *singularityExecutor) InjectCommand(ctx context.Context, detachKeys, username string, usingTTY bool, injectcmd []string) (*exec.Cmd, error) {
-	return nil, errors.New("unimplemented")
+	target, err := e.containedProcess()
+	if err != nil {
+		return nil, err
+	}
+	return exec.CommandContext(ctx, "nsenter", append([]string{fmt.Sprintf("--target=%d", target), "--all"}, injectcmd...)...), nil
 }
 
+var (
+	errContainerHasNoIPAddress = errors.New("container has no IP address distinct from host")
+)
+
 func (e *singularityExecutor) IPAddress() (string, error) {
-	return "", errors.New("unimplemented")
+	target, err := e.containedProcess()
+	if err != nil {
+		return "", err
+	}
+	targetIPs, err := processIPs(target)
+	if err != nil {
+		return "", err
+	}
+	selfIPs, err := processIPs(os.Getpid())
+	if err != nil {
+		return "", err
+	}
+	for ip := range targetIPs {
+		if !selfIPs[ip] {
+			return ip, nil
+		}
+	}
+	return "", errContainerHasNoIPAddress
+}
+
+func processIPs(pid int) (map[string]bool, error) {
+	fibtrie, err := os.ReadFile(fmt.Sprintf("/proc/%d/net/fib_trie", pid))
+	if err != nil {
+		return nil, err
+	}
+
+	addrs := map[string]bool{}
+	// When we see a pair of lines like this:
+	//
+	//              |-- 10.1.2.3
+	//                 /32 host LOCAL
+	//
+	// ...we set addrs["10.1.2.3"] = true
+	lines := bytes.Split(fibtrie, []byte{'\n'})
+	for linenumber, line := range lines {
+		if !bytes.HasSuffix(line, []byte("/32 host LOCAL")) {
+			continue
+		}
+		if linenumber < 1 {
+			continue
+		}
+		i := bytes.LastIndexByte(lines[linenumber-1], ' ')
+		if i < 0 || i >= len(line)-7 {
+			continue
+		}
+		addr := string(lines[linenumber-1][i+1:])
+		if net.ParseIP(addr).To4() != nil {
+			addrs[addr] = true
+		}
+	}
+	return addrs, nil
+}
+
+var (
+	errContainerNotStarted = errors.New("container has not started yet")
+	errCannotFindChild     = errors.New("failed to find any process inside the container")
+	reProcStatusPPid       = regexp.MustCompile(`\nPPid:\t(\d+)\n`)
+)
+
+// Return the PID of a process that is inside the container (not
+// necessarily the topmost/pid=1 process in the container).
+func (e *singularityExecutor) containedProcess() (int, error) {
+	if e.child == nil || e.child.Process == nil {
+		return 0, errContainerNotStarted
+	}
+	lsns, err := exec.Command("lsns").CombinedOutput()
+	if err != nil {
+		return 0, fmt.Errorf("lsns: %w", err)
+	}
+	for _, line := range bytes.Split(lsns, []byte{'\n'}) {
+		fields := bytes.Fields(line)
+		if len(fields) < 4 {
+			continue
+		}
+		if !bytes.Equal(fields[1], []byte("pid")) {
+			continue
+		}
+		pid, err := strconv.ParseInt(string(fields[3]), 10, 64)
+		if err != nil {
+			return 0, fmt.Errorf("error parsing PID field in lsns output: %q", fields[3])
+		}
+		for parent := pid; ; {
+			procstatus, err := os.ReadFile(fmt.Sprintf("/proc/%d/status", parent))
+			if err != nil {
+				break
+			}
+			m := reProcStatusPPid.FindSubmatch(procstatus)
+			if m == nil {
+				break
+			}
+			parent, err = strconv.ParseInt(string(m[1]), 10, 64)
+			if err != nil {
+				break
+			}
+			if int(parent) == e.child.Process.Pid {
+				return int(pid), nil
+			}
+		}
+	}
+	return 0, errCannotFindChild
 }
diff --git a/lib/crunchrun/singularity_test.go b/lib/crunchrun/singularity_test.go
index cdeafee88..8a2e62d7e 100644
--- a/lib/crunchrun/singularity_test.go
+++ b/lib/crunchrun/singularity_test.go
@@ -28,6 +28,14 @@ func (s *singularitySuite) SetUpSuite(c *C) {
 	}
 }
 
+func (s *singularitySuite) TestInject(c *C) {
+	path, err := exec.LookPath("nsenter")
+	if err != nil || path != "/var/lib/arvados/bin/nsenter" {
+		c.Skip("looks like /var/lib/arvados/bin/nsenter is not installed -- re-run `arvados-server install`?")
+	}
+	s.executorSuite.TestInject(c)
+}
+
 var _ = Suite(&singularityStubSuite{})
 
 // singularityStubSuite tests don't really invoke singularity, so we
diff --git a/lib/install/deps.go b/lib/install/deps.go
index cdf28e09c..2d9da72b9 100644
--- a/lib/install/deps.go
+++ b/lib/install/deps.go
@@ -338,6 +338,16 @@ make -C ./builddir install
 			}
 		}
 
+		err = inst.runBash(`
+install /usr/bin/nsenter /var/lib/arvados/bin/nsenter
+setcap "cap_sys_admin+pei cap_sys_chroot+pei" /var/lib/arvados/bin/nsenter
+singularity config global --set 'allow net networks' bridge
+singularity config global --set 'allow net groups' sudo
+`, stdout, stderr)
+		if err != nil {
+			return 1
+		}
+
 		// The entry in /etc/locale.gen is "en_US.UTF-8"; once
 		// it's installed, locale -a reports it as
 		// "en_US.utf8".

commit 7ebe828a435dcaa1b5668b72adbaad495059f211
Author: Tom Clegg <tom at curii.com>
Date:   Thu May 12 10:12:29 2022 -0400

    19099: Refactor container shell backend so it's not docker-specific.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/crunchrun/container_gateway.go b/lib/crunchrun/container_gateway.go
index 2ec24bac7..62979da21 100644
--- a/lib/crunchrun/container_gateway.go
+++ b/lib/crunchrun/container_gateway.go
@@ -17,30 +17,33 @@ import (
 	"os"
 	"os/exec"
 	"sync"
-	"sync/atomic"
 	"syscall"
-	"time"
 
 	"git.arvados.org/arvados.git/lib/selfsigned"
 	"git.arvados.org/arvados.git/sdk/go/ctxlog"
 	"git.arvados.org/arvados.git/sdk/go/httpserver"
 	"github.com/creack/pty"
-	dockerclient "github.com/docker/docker/client"
 	"github.com/google/shlex"
 	"golang.org/x/crypto/ssh"
 	"golang.org/x/net/context"
 )
 
+type GatewayTarget interface {
+	// Command that will execute cmd inside the container
+	InjectCommand(ctx context.Context, detachKeys, username string, usingTTY bool, cmd []string) (*exec.Cmd, error)
+
+	// IP address inside container
+	IPAddress() (string, error)
+}
+
 type Gateway struct {
-	DockerContainerID *string
-	ContainerUUID     string
-	Address           string // listen host:port; if port=0, Start() will change it to the selected port
-	AuthSecret        string
-	Log               interface {
+	ContainerUUID string
+	Address       string // listen host:port; if port=0, Start() will change it to the selected port
+	AuthSecret    string
+	Target        GatewayTarget
+	Log           interface {
 		Printf(fmt string, args ...interface{})
 	}
-	// return local ip address of running container, or "" if not available
-	ContainerIPAddress func() (string, error)
 
 	sshConfig   ssh.ServerConfig
 	requestAuth string
@@ -241,15 +244,11 @@ func (gw *Gateway) handleDirectTCPIP(ctx context.Context, newch ssh.NewChannel)
 		return
 	}
 
-	var dstaddr string
-	if gw.ContainerIPAddress != nil {
-		dstaddr, err = gw.ContainerIPAddress()
-		if err != nil {
-			fmt.Fprintf(ch.Stderr(), "container has no IP address: %s\n", err)
-			return
-		}
-	}
-	if dstaddr == "" {
+	dstaddr, err := gw.Target.IPAddress()
+	if err != nil {
+		fmt.Fprintf(ch.Stderr(), "container has no IP address: %s\n", err)
+		return
+	} else if dstaddr == "" {
 		fmt.Fprintf(ch.Stderr(), "container has no IP address\n")
 		return
 	}
@@ -301,12 +300,25 @@ func (gw *Gateway) handleSession(ctx context.Context, newch ssh.NewChannel, deta
 				execargs = []string{"/bin/bash", "-login"}
 			}
 			go func() {
-				cmd := exec.CommandContext(ctx, "docker", "exec", "-i", "--detach-keys="+detachKeys, "--user="+username)
+				var resp struct {
+					Status uint32
+				}
+				defer func() {
+					ch.SendRequest("exit-status", false, ssh.Marshal(&resp))
+					ch.Close()
+				}()
+
+				cmd, err := gw.Target.InjectCommand(ctx, detachKeys, username, tty0 != nil, execargs)
+				if err != nil {
+					fmt.Fprintln(ch.Stderr(), err)
+					ch.CloseWrite()
+					resp.Status = 1
+					return
+				}
 				cmd.Stdin = ch
 				cmd.Stdout = ch
 				cmd.Stderr = ch.Stderr()
 				if tty0 != nil {
-					cmd.Args = append(cmd.Args, "-t")
 					cmd.Stdin = tty0
 					cmd.Stdout = tty0
 					cmd.Stderr = tty0
@@ -318,17 +330,12 @@ func (gw *Gateway) handleSession(ctx context.Context, newch ssh.NewChannel, deta
 					// Send our own debug messages to tty as well.
 					logw = tty0
 				}
-				cmd.Args = append(cmd.Args, *gw.DockerContainerID)
-				cmd.Args = append(cmd.Args, execargs...)
 				cmd.SysProcAttr = &syscall.SysProcAttr{
 					Setctty: tty0 != nil,
 					Setsid:  true,
 				}
 				cmd.Env = append(os.Environ(), termEnv...)
-				err := cmd.Run()
-				var resp struct {
-					Status uint32
-				}
+				err = cmd.Run()
 				if exiterr, ok := err.(*exec.ExitError); ok {
 					if status, ok := exiterr.Sys().(syscall.WaitStatus); ok {
 						resp.Status = uint32(status.ExitStatus())
@@ -341,8 +348,6 @@ func (gw *Gateway) handleSession(ctx context.Context, newch ssh.NewChannel, deta
 				if resp.Status == 0 && (err != nil || errClose != nil) {
 					resp.Status = 1
 				}
-				ch.SendRequest("exit-status", false, ssh.Marshal(&resp))
-				ch.Close()
 			}()
 		case "pty-req":
 			eol = "\r\n"
@@ -398,31 +403,3 @@ func (gw *Gateway) handleSession(ctx context.Context, newch ssh.NewChannel, deta
 		}
 	}
 }
-
-func dockerContainerIPAddress(containerID *string) func() (string, error) {
-	var saved atomic.Value
-	return func() (string, error) {
-		if ip, ok := saved.Load().(*string); ok {
-			return *ip, nil
-		}
-		docker, err := dockerclient.NewClient(dockerclient.DefaultDockerHost, "1.21", nil, nil)
-		if err != nil {
-			return "", fmt.Errorf("cannot create docker client: %s", err)
-		}
-		ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(time.Minute))
-		defer cancel()
-		ctr, err := docker.ContainerInspect(ctx, *containerID)
-		if err != nil {
-			return "", fmt.Errorf("cannot get docker container info: %s", err)
-		}
-		ip := ctr.NetworkSettings.IPAddress
-		if ip == "" {
-			// TODO: try to enable networking if it wasn't
-			// already enabled when the container was
-			// created.
-			return "", fmt.Errorf("container has no IP address")
-		}
-		saved.Store(&ip)
-		return ip, nil
-	}
-}
diff --git a/lib/crunchrun/crunchrun.go b/lib/crunchrun/crunchrun.go
index 474fbf4ad..0364db78e 100644
--- a/lib/crunchrun/crunchrun.go
+++ b/lib/crunchrun/crunchrun.go
@@ -1901,14 +1901,13 @@ func (command) RunCommand(prog string, args []string, stdin io.Reader, stdout, s
 		// dispatcher did not tell us which external IP
 		// address to advertise --> no gateway service
 		cr.CrunchLog.Printf("Not starting a gateway server (GatewayAddress was not provided by dispatcher)")
-	} else if de, ok := cr.executor.(*dockerExecutor); ok {
+	} else {
 		cr.gateway = Gateway{
-			Address:            gwListen,
-			AuthSecret:         gwAuthSecret,
-			ContainerUUID:      containerUUID,
-			DockerContainerID:  &de.containerID,
-			Log:                cr.CrunchLog,
-			ContainerIPAddress: dockerContainerIPAddress(&de.containerID),
+			Address:       gwListen,
+			AuthSecret:    gwAuthSecret,
+			ContainerUUID: containerUUID,
+			Target:        cr.executor,
+			Log:           cr.CrunchLog,
 		}
 		err = cr.gateway.Start()
 		if err != nil {
diff --git a/lib/crunchrun/crunchrun_test.go b/lib/crunchrun/crunchrun_test.go
index 1d2c7b09f..9e2286d68 100644
--- a/lib/crunchrun/crunchrun_test.go
+++ b/lib/crunchrun/crunchrun_test.go
@@ -136,6 +136,10 @@ func (e *stubExecutor) Close()                          { e.closed = true }
 func (e *stubExecutor) Wait(context.Context) (int, error) {
 	return <-e.exit, e.waitErr
 }
+func (e *stubExecutor) InjectCommand(ctx context.Context, _, _ string, _ bool, _ []string) (*exec.Cmd, error) {
+	return nil, errors.New("unimplemented")
+}
+func (e *stubExecutor) IPAddress() (string, error) { return "", errors.New("unimplemented") }
 
 const fakeInputCollectionPDH = "ffffffffaaaaaaaa88888888eeeeeeee+1234"
 
diff --git a/lib/crunchrun/docker.go b/lib/crunchrun/docker.go
index e62f2a39b..dde96b08e 100644
--- a/lib/crunchrun/docker.go
+++ b/lib/crunchrun/docker.go
@@ -8,7 +8,9 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
+	"os/exec"
 	"strings"
+	"sync/atomic"
 	"time"
 
 	"git.arvados.org/arvados.git/sdk/go/arvados"
@@ -27,6 +29,7 @@ type dockerExecutor struct {
 	watchdogInterval time.Duration
 	dockerclient     *dockerclient.Client
 	containerID      string
+	savedIPAddress   atomic.Value
 	doneIO           chan struct{}
 	errIO            error
 }
@@ -297,3 +300,34 @@ func (e *dockerExecutor) handleStdoutStderr(stdout, stderr io.Writer, reader io.
 func (e *dockerExecutor) Close() {
 	e.dockerclient.ContainerRemove(context.TODO(), e.containerID, dockertypes.ContainerRemoveOptions{Force: true})
 }
+
+func (e *dockerExecutor) InjectCommand(ctx context.Context, detachKeys, username string, usingTTY bool, injectcmd []string) (*exec.Cmd, error) {
+	cmd := exec.CommandContext(ctx, "docker", "exec", "-i", "--detach-keys="+detachKeys, "--user="+username)
+	if usingTTY {
+		cmd.Args = append(cmd.Args, "-t")
+	}
+	cmd.Args = append(cmd.Args, e.containerID)
+	cmd.Args = append(cmd.Args, injectcmd...)
+	return cmd, nil
+}
+
+func (e *dockerExecutor) IPAddress() (string, error) {
+	if ip, ok := e.savedIPAddress.Load().(*string); ok {
+		return *ip, nil
+	}
+	ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(time.Minute))
+	defer cancel()
+	ctr, err := e.dockerclient.ContainerInspect(ctx, e.containerID)
+	if err != nil {
+		return "", fmt.Errorf("cannot get docker container info: %s", err)
+	}
+	ip := ctr.NetworkSettings.IPAddress
+	if ip == "" {
+		// TODO: try to enable networking if it wasn't
+		// already enabled when the container was
+		// created.
+		return "", fmt.Errorf("container has no IP address")
+	}
+	e.savedIPAddress.Store(&ip)
+	return ip, nil
+}
diff --git a/lib/crunchrun/executor.go b/lib/crunchrun/executor.go
index dc1bc20b7..b5b788433 100644
--- a/lib/crunchrun/executor.go
+++ b/lib/crunchrun/executor.go
@@ -62,4 +62,6 @@ type containerExecutor interface {
 
 	// Name of runtime engine ("docker", "singularity")
 	Runtime() string
+
+	GatewayTarget
 }
diff --git a/lib/crunchrun/executor_test.go b/lib/crunchrun/executor_test.go
index 99af0530f..1c963f921 100644
--- a/lib/crunchrun/executor_test.go
+++ b/lib/crunchrun/executor_test.go
@@ -8,6 +8,7 @@ import (
 	"bytes"
 	"io"
 	"io/ioutil"
+	"net"
 	"net/http"
 	"os"
 	"strings"
@@ -174,6 +175,61 @@ func (s *executorSuite) TestExecStdoutStderr(c *C) {
 	c.Check(s.stderr.String(), Equals, "barwaz\n")
 }
 
+func (s *executorSuite) TestIPAddress(c *C) {
+	s.spec.Command = []string{"nc", "-l", "-p", "1951", "-e", "printf", `HTTP/1.1 418 I'm a teapot\r\n\r\n`}
+	s.spec.EnableNetwork = true
+	c.Assert(s.executor.Create(s.spec), IsNil)
+	c.Assert(s.executor.Start(), IsNil)
+	starttime := time.Now()
+
+	ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(2*time.Second))
+	defer cancel()
+
+	for ctx.Err() == nil {
+		time.Sleep(time.Second / 10)
+		_, err := s.executor.IPAddress()
+		if err == nil {
+			break
+		}
+	}
+	ip, err := s.executor.IPAddress()
+	if c.Check(err, IsNil) && c.Check(ip, Not(Equals), "") {
+		req, err := http.NewRequest("BREW", "http://"+net.JoinHostPort(ip, "1951"), nil)
+		c.Assert(err, IsNil)
+		resp, err := http.DefaultClient.Do(req)
+		c.Assert(err, IsNil)
+		c.Check(resp.StatusCode, Equals, http.StatusTeapot)
+	}
+
+	s.executor.Stop()
+	code, _ := s.executor.Wait(ctx)
+	c.Logf("container ran for %v", time.Now().Sub(starttime))
+	c.Check(code, Equals, -1)
+}
+
+func (s *executorSuite) TestInject(c *C) {
+	s.spec.Command = []string{"sleep", "10"}
+	c.Assert(s.executor.Create(s.spec), IsNil)
+	c.Assert(s.executor.Start(), IsNil)
+	starttime := time.Now()
+
+	ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(2*time.Second))
+	defer cancel()
+
+	injectcmd := []string{"cat", "/proc/1/cmdline"}
+	cmd, err := s.executor.InjectCommand(ctx, "", "root", false, injectcmd)
+	c.Assert(err, IsNil)
+	out, err := cmd.CombinedOutput()
+	c.Logf("inject %s => %q", injectcmd, out)
+	c.Check(err, IsNil)
+	c.Check(string(out), Equals, "sleep\00010\000")
+
+	s.executor.Stop()
+	code, _ := s.executor.Wait(ctx)
+	c.Logf("container ran for %v", time.Now().Sub(starttime))
+	c.Check(code, Equals, -1)
+}
+
 func (s *executorSuite) checkRun(c *C, expectCode int) {
 	c.Assert(s.executor.Create(s.spec), IsNil)
 	c.Assert(s.executor.Start(), IsNil)
diff --git a/lib/crunchrun/singularity.go b/lib/crunchrun/singularity.go
index 64a377325..6ba65200d 100644
--- a/lib/crunchrun/singularity.go
+++ b/lib/crunchrun/singularity.go
@@ -5,6 +5,7 @@
 package crunchrun
 
 import (
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -349,3 +350,11 @@ func (e *singularityExecutor) Close() {
 		e.logf("error removing temp dir: %s", err)
 	}
 }
+
+func (e *singularityExecutor) InjectCommand(ctx context.Context, detachKeys, username string, usingTTY bool, injectcmd []string) (*exec.Cmd, error) {
+	return nil, errors.New("unimplemented")
+}
+
+func (e *singularityExecutor) IPAddress() (string, error) {
+	return "", errors.New("unimplemented")
+}

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list