17813: continue refactor & fix tests

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz@curii.com>

diff --git a/lib/crunchrun/crunchrun.go b/lib/crunchrun/crunchrun.go
index d4a8650726d2aae9d6b222cf0a6baa89a2091dad..e15303a3155afe81d72e8ce61e881ce76d5282d7 100644 (file)
--- a/lib/crunchrun/crunchrun.go
+++ b/lib/crunchrun/crunchrun.go
@@ -260,11 +260,12 @@ func (runner *ContainerRunner) LoadImage() (string, error) {
                return "", fmt.Errorf("cannot choose from multiple tar files in image collection: %v", tarfiles)
        }
        imageID := tarfiles[0][:len(tarfiles[0])-4]
                return "", fmt.Errorf("cannot choose from multiple tar files in image collection: %v", tarfiles)
        }
        imageID := tarfiles[0][:len(tarfiles[0])-4]

+       imageTarballPath := runner.ArvMountPoint + "/by_id/" + runner.Container.ContainerImage + "/" + imageID + ".tar"

        runner.CrunchLog.Printf("Using Docker image id %q", imageID)
 
        runner.CrunchLog.Print("Loading Docker image from keep")
        runner.CrunchLog.Printf("Using Docker image id %q", imageID)
 
        runner.CrunchLog.Print("Loading Docker image from keep")

-       err = runner.executor.LoadImage(imageID, runner.Container, runner.ArvMountPoint,
-               runner.containerClient, runner.ContainerKeepClient)
+       err = runner.executor.LoadImage(imageID, imageTarballPath, runner.Container, runner.ArvMountPoint,
+               runner.containerClient)

        if err != nil {
                return "", err
        }
        if err != nil {
                return "", err
        }

diff --git a/lib/crunchrun/crunchrun_test.go b/lib/crunchrun/crunchrun_test.go
index 7519319e3c6632f42580dda54e4c3154483c55a5..fa34adf50e38c664fbecf87c9d60893a7f060fa4 100644 (file)
--- a/lib/crunchrun/crunchrun_test.go
+++ b/lib/crunchrun/crunchrun_test.go
@@ -112,8 +112,11 @@ type stubExecutor struct {
        exit        chan int
 }
 
        exit        chan int
 }
 

-func (e *stubExecutor) ImageLoaded(imageID string) bool { return e.imageLoaded }
-func (e *stubExecutor) LoadImage(filename string) error { e.loaded = filename; return e.loadErr }
+func (e *stubExecutor) LoadImage(imageId string, tarball string, container arvados.Container, keepMount string,
+       containerClient *arvados.Client) error {
+       e.loaded = tarball
+       return e.loadErr
+}

 func (e *stubExecutor) Create(spec containerSpec) error { e.created = spec; return e.createErr }
 func (e *stubExecutor) Start() error                    { e.exit = make(chan int, 1); go e.runFunc(); return e.startErr }
 func (e *stubExecutor) CgroupID() string                { return "cgroupid" }
 func (e *stubExecutor) Create(spec containerSpec) error { e.created = spec; return e.createErr }
 func (e *stubExecutor) Start() error                    { e.exit = make(chan int, 1); go e.runFunc(); return e.startErr }
 func (e *stubExecutor) CgroupID() string                { return "cgroupid" }


@@ -122,8 +125,6 @@ func (e *stubExecutor) Close()                          { e.closed = true }
 func (e *stubExecutor) Wait(context.Context) (int, error) {
        return <-e.exit, e.waitErr
 }
 func (e *stubExecutor) Wait(context.Context) (int, error) {
        return <-e.exit, e.waitErr
 }

-func (e *stubExecutor) SetArvadoClient(containerClient *arvados.Client, keepClient IKeepClient, container arvados.Container, keepMount string) {
-}

 
 const fakeInputCollectionPDH = "ffffffffaaaaaaaa88888888eeeeeeee+1234"
 
 
 const fakeInputCollectionPDH = "ffffffffaaaaaaaa88888888eeeeeeee+1234"
 

diff --git a/lib/crunchrun/docker.go b/lib/crunchrun/docker.go
index 10a2dd38e8109182d1f41e9f1364a54e8b11d67a..656061b77ec552a811c26dfe18be870b154c1b1e 100644 (file)
--- a/lib/crunchrun/docker.go
+++ b/lib/crunchrun/docker.go
@@ -46,17 +46,15 @@ func newDockerExecutor(containerUUID string, logf func(string, ...interface{}),
        }, err
 }
 
        }, err
 }
 

-func (e *dockerExecutor) LoadImage(imageID string, container arvados.Container, arvMountPoint string,
-       containerClient *arvados.Client, keepClient IKeepClient) error {
+func (e *dockerExecutor) LoadImage(imageID string, imageTarballPath string, container arvados.Container, arvMountPoint string,
+       containerClient *arvados.Client) error {

        _, _, err := e.dockerclient.ImageInspectWithRaw(context.TODO(), imageID)
        if err == nil {
                // already loaded
                return nil
        }
 
        _, _, err := e.dockerclient.ImageInspectWithRaw(context.TODO(), imageID)
        if err == nil {
                // already loaded
                return nil
        }
 

-       filename := arvMountPoint + "/by_id/" + container.ContainerImage + "/" + imageID + ".tar"
-
-       f, err := os.Open(filename)
+       f, err := os.Open(imageTarballPath)

        if err != nil {
                return err
        }
        if err != nil {
                return err
        }

diff --git a/lib/crunchrun/executor.go b/lib/crunchrun/executor.go
index 787edda010a139954fcb2731da7e807de9933c7f..65bf7427b9601c465fb21d811c5cb79d2d41a0f8 100644 (file)
--- a/lib/crunchrun/executor.go
+++ b/lib/crunchrun/executor.go
@@ -36,8 +36,8 @@ type containerSpec struct {
 type containerExecutor interface {
        // ImageLoad loads the image from the given tarball such that
        // it can be used to create/start a container.
 type containerExecutor interface {
        // ImageLoad loads the image from the given tarball such that
        // it can be used to create/start a container.

-       LoadImage(imageID string, container arvados.Container, keepMount string,
-               containerClient *arvados.Client, keepClient IKeepClient) error
+       LoadImage(imageID string, imageTarballPath string, container arvados.Container, keepMount string,
+               containerClient *arvados.Client) error

 
        // Wait for the container process to finish, and return its
        // exit code. If applicable, also remove the stopped container
 
        // Wait for the container process to finish, and return its
        // exit code. If applicable, also remove the stopped container

diff --git a/lib/crunchrun/executor_test.go b/lib/crunchrun/executor_test.go
index 5934c57b6c5f90bf971664c614a8348fb18b9e50..0f9901d6a1ff0d6ebb268c23b107f5ff5514244b 100644 (file)
--- a/lib/crunchrun/executor_test.go
+++ b/lib/crunchrun/executor_test.go
@@ -13,6 +13,7 @@ import (
        "strings"
        "time"
 
        "strings"
        "time"
 

+       "git.arvados.org/arvados.git/sdk/go/arvados"

        "golang.org/x/net/context"
        . "gopkg.in/check.v1"
 )
        "golang.org/x/net/context"
        . "gopkg.in/check.v1"
 )


@@ -70,7 +71,7 @@ func (s *executorSuite) SetUpTest(c *C) {
                Stdout:      nopWriteCloser{&s.stdout},
                Stderr:      nopWriteCloser{&s.stderr},
        }
                Stdout:      nopWriteCloser{&s.stdout},
                Stderr:      nopWriteCloser{&s.stderr},
        }

-       err := s.executor.LoadImage(busyboxDockerImage(c))
+       err := s.executor.LoadImage("", busyboxDockerImage(c), arvados.Container{}, "", nil)

        c.Assert(err, IsNil)
 }
 
        c.Assert(err, IsNil)
 }
 

diff --git a/lib/crunchrun/singularity.go b/lib/crunchrun/singularity.go
index 5d930626a00b1a1281a6815e94e796d66a0b9946..f9ae073dc12c82b7b848020ce8e270ed4480cdef 100644 (file)
--- a/lib/crunchrun/singularity.go
+++ b/lib/crunchrun/singularity.go
@@ -72,7 +72,7 @@ func (e *singularityExecutor) getOrCreateProject(ownerUuid string, name string,
 }
 
 func (e *singularityExecutor) checkImageCache(dockerImageID string, container arvados.Container, arvMountPoint string,
 }
 
 func (e *singularityExecutor) checkImageCache(dockerImageID string, container arvados.Container, arvMountPoint string,

-       containerClient *arvados.Client, keepClient IKeepClient) (collectionUuid string, err error) {
+       containerClient *arvados.Client) (collectionUuid string, err error) {

 
        // Cache the image to keep
        cacheGroup, err := e.getOrCreateProject(container.RuntimeUserUUID, ".cache", containerClient)
 
        // Cache the image to keep
        cacheGroup, err := e.getOrCreateProject(container.RuntimeUserUUID, ".cache", containerClient)


@@ -95,7 +95,7 @@ func (e *singularityExecutor) checkImageCache(dockerImageID string, container ar
                },
                        Limit: 1})
        if err != nil {
                },
                        Limit: 1})
        if err != nil {

-               return "", fmt.Errorf("error querying for collection '%v': %v", err)
+               return "", fmt.Errorf("error querying for collection '%v': %v", collectionName, err)

        }
        var imageCollection arvados.Collection
        if len(cl.Items) == 1 {
        }
        var imageCollection arvados.Collection
        if len(cl.Items) == 1 {


@@ -123,19 +123,22 @@ func (e *singularityExecutor) checkImageCache(dockerImageID string, container ar
 
 // LoadImage will satisfy ContainerExecuter interface transforming
 // containerImage into a sif file for later use.
 
 // LoadImage will satisfy ContainerExecuter interface transforming
 // containerImage into a sif file for later use.

-func (e *singularityExecutor) LoadImage(dockerImageID string, container arvados.Container, arvMountPoint string,
-       containerClient *arvados.Client, keepClient IKeepClient) error {
+func (e *singularityExecutor) LoadImage(dockerImageID string, imageTarballPath string, container arvados.Container, arvMountPoint string,
+       containerClient *arvados.Client) error {

 
 

-       sifCollectionUUID, err := e.checkImageCache(dockerImageID, container, arvMountPoint, containerClient, keepClient)
-       if err != nil {
-               return err
+       var sifCollectionUUID string
+       var imageFilename string
+       if containerClient != nil {
+               sifCollectionUUID, err := e.checkImageCache(dockerImageID, container, arvMountPoint, containerClient)
+               if err != nil {
+                       return err
+               }
+               imageFilename = fmt.Sprintf("%s/by_uuid/%s/image.sif", arvMountPoint, sifCollectionUUID)
+       } else {
+               imageFilename = e.tmpdir + "/image.sif"

        }
 
        }
 

-       imageTarballPath := arvMountPoint + "/by_id/" + container.ContainerImage + "/" + dockerImageID + ".tar"
-
-       imageFilename := fmt.Sprintf("%s/by_uuid/%s/image.sif", arvMountPoint, sifCollectionUUID)
-
-       if _, err = os.Stat(imageFilename); os.IsNotExist(err) {
+       if _, err := os.Stat(imageFilename); os.IsNotExist(err) {

                exec.Command("find", arvMountPoint+"/by_id/").Run()
 
                e.logf("building singularity image")
                exec.Command("find", arvMountPoint+"/by_id/").Run()
 
                e.logf("building singularity image")


@@ -166,6 +169,11 @@ func (e *singularityExecutor) LoadImage(dockerImageID string, container arvados.
                }
        }
 
                }
        }
 

+       if containerClient == nil {
+               e.imageFilename = imageFilename
+               return nil
+       }
+

        // update TTL to now + two weeks
        exp := time.Now().Add(24 * 7 * 2 * time.Hour)
 
        // update TTL to now + two weeks
        exp := time.Now().Add(24 * 7 * 2 * time.Hour)