From: Peter Amstutz Date: Thu, 22 Jul 2021 20:44:19 +0000 (-0400) Subject: 17813: continue refactor & fix tests X-Git-Tag: 2.3.0~131^2~3 X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/ad82cb7409dd62c3ac251e4f98d82ce774f6f528 17813: continue refactor & fix tests Arvados-DCO-1.1-Signed-off-by: Peter Amstutz --- diff --git a/lib/crunchrun/crunchrun.go b/lib/crunchrun/crunchrun.go index d4a8650726..e15303a315 100644 --- 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] + 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") - 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 } diff --git a/lib/crunchrun/crunchrun_test.go b/lib/crunchrun/crunchrun_test.go index 7519319e3c..fa34adf50e 100644 --- a/lib/crunchrun/crunchrun_test.go +++ b/lib/crunchrun/crunchrun_test.go @@ -112,8 +112,11 @@ type stubExecutor struct { 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" } @@ -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) SetArvadoClient(containerClient *arvados.Client, keepClient IKeepClient, container arvados.Container, keepMount string) { -} const fakeInputCollectionPDH = "ffffffffaaaaaaaa88888888eeeeeeee+1234" diff --git a/lib/crunchrun/docker.go b/lib/crunchrun/docker.go index 10a2dd38e8..656061b77e 100644 --- a/lib/crunchrun/docker.go +++ b/lib/crunchrun/docker.go @@ -46,17 +46,15 @@ func newDockerExecutor(containerUUID string, logf func(string, ...interface{}), }, 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 } - filename := arvMountPoint + "/by_id/" + container.ContainerImage + "/" + imageID + ".tar" - - f, err := os.Open(filename) + f, err := os.Open(imageTarballPath) if err != nil { return err } diff --git a/lib/crunchrun/executor.go b/lib/crunchrun/executor.go index 787edda010..65bf7427b9 100644 --- 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. - 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 diff --git a/lib/crunchrun/executor_test.go b/lib/crunchrun/executor_test.go index 5934c57b6c..0f9901d6a1 100644 --- a/lib/crunchrun/executor_test.go +++ b/lib/crunchrun/executor_test.go @@ -13,6 +13,7 @@ import ( "strings" "time" + "git.arvados.org/arvados.git/sdk/go/arvados" "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}, } - err := s.executor.LoadImage(busyboxDockerImage(c)) + err := s.executor.LoadImage("", busyboxDockerImage(c), arvados.Container{}, "", nil) c.Assert(err, IsNil) } diff --git a/lib/crunchrun/singularity.go b/lib/crunchrun/singularity.go index 5d930626a0..f9ae073dc1 100644 --- 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, - 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) @@ -95,7 +95,7 @@ func (e *singularityExecutor) checkImageCache(dockerImageID string, container ar }, 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 { @@ -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. -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") @@ -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)