[ARVADOS] updated: 2.1.0-491-gfccb8a3de
Git user
git at public.arvados.org
Thu Mar 25 20:16:45 UTC 2021
Summary of changes:
lib/crunchrun/container_exec_types.go | 14 +------
lib/crunchrun/crunchrun.go | 13 +++++--
lib/crunchrun/crunchrun_test.go | 72 +++++++++++++++++++++--------------
lib/crunchrun/docker_adapter.go | 14 +++----
lib/crunchrun/singularity.go | 7 +---
5 files changed, 63 insertions(+), 57 deletions(-)
via fccb8a3decaaa33fb9b356b028dad3e257c90fe0 (commit)
from 99b434b8234386cdacefaf42460c89add6ec0f83 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit fccb8a3decaaa33fb9b356b028dad3e257c90fe0
Author: Nico Cesar <nico at nicocesar.com>
Date: Thu Mar 25 16:16:07 2021 -0400
Still failing tests but we'll be reviewing with tom
Arvados-DCO-1.1-Signed-off-by: Nico Cesar <nico at curii.com>
diff --git a/lib/crunchrun/container_exec_types.go b/lib/crunchrun/container_exec_types.go
index 5ca24e60c..8f972daab 100644
--- a/lib/crunchrun/container_exec_types.go
+++ b/lib/crunchrun/container_exec_types.go
@@ -163,18 +163,6 @@ type ContainerInspectResponse struct {
State *ContainerState
}
-// ImageInspectResponse in the case of docker is similar to
-// github.com/docker/docker/api/types/ImageInspect
-// and for Singularity TBD.
-// MAYBE call it ExecRunnerImage? since the struct is describing an image
-// from the underlying ExecRunner
-type ImageInspectResponse struct {
- // we don't use the respones but we use ImageInspectWithRaw(context.TODO(), imageID)
- // to check if we already have the docker image, maybe we can do the
- // a imagePresent(id string) (bool)
- ID string
-}
-
// ImageLoadResponse returns information to the client about a load process.
type ImageLoadResponse struct {
// Body must be closed to avoid a resource leak
@@ -314,7 +302,7 @@ type ThinContainerExecRunner interface {
ContainerWait(ctx context.Context, container string, condition WaitCondition) (<-chan ContainerWaitOKBody, <-chan error)
ContainerInspect(ctx context.Context, id string) (ContainerInspectResponse, error)
- ImageInspectWithRaw(ctx context.Context, image string) (ImageInspectResponse, []byte, error)
+ ImageLocallyCached(ctx context.Context, image string) (bool, error)
ImageLoad(ctx context.Context, input io.Reader, quiet bool) (ImageLoadResponse, error)
ImageRemove(ctx context.Context, image string, options ImageRemoveOptions) ([]ImageDeleteResponseItem, error)
diff --git a/lib/crunchrun/crunchrun.go b/lib/crunchrun/crunchrun.go
index c8ac9ec8b..cf063ead0 100644
--- a/lib/crunchrun/crunchrun.go
+++ b/lib/crunchrun/crunchrun.go
@@ -264,8 +264,15 @@ func (runner *ContainerRunner) LoadImage() (err error) {
runner.CrunchLog.Printf("Using Docker image id '%s'", imageID)
- _, _, err = runner.ContainerExecRunner.ImageInspectWithRaw(context.TODO(), imageID)
+ imageIsThere, err := runner.ContainerExecRunner.ImageLocallyCached(context.TODO(), imageID)
+
if err != nil {
+ return err
+ }
+
+ if imageIsThere {
+ runner.CrunchLog.Printf("Docker image id '%s' is locally available, no need to fetch it from keep", imageID)
+ } else {
runner.CrunchLog.Print("Loading Docker image from keep")
var readCloser io.ReadCloser
@@ -285,10 +292,9 @@ func (runner *ContainerRunner) LoadImage() (err error) {
return fmt.Errorf("Reading response to image load: %v", err)
}
runner.CrunchLog.Printf("Docker response: %s", rbody)
- } else {
- runner.CrunchLog.Print("Docker image is available")
}
+ runner.CrunchLog.Print("Docker image is now available")
runner.ContainerExecRunner.SetImage(imageID)
runner.ContainerKeepClient.ClearBlockCache()
@@ -674,6 +680,7 @@ func (runner *ContainerRunner) SetupMounts() (err error) {
return nil
}
+// TODO: review if this can be moved to ThinContainerExecRunner interface
func (runner *ContainerRunner) ProcessDockerAttach(containerReader io.Reader) {
// Handle docker log protocol
// https://docs.docker.com/engine/reference/api/docker_remote_api_v1.15/#attach-to-a-container
diff --git a/lib/crunchrun/crunchrun_test.go b/lib/crunchrun/crunchrun_test.go
index 7690728d9..891d85482 100644
--- a/lib/crunchrun/crunchrun_test.go
+++ b/lib/crunchrun/crunchrun_test.go
@@ -90,18 +90,19 @@ var fakeAuthUUID = "zzzzz-gj3su-55pqoyepgi2glem"
var fakeAuthToken = "a3ltuwzqcu2u4sc0q7yhpc2w7s00fdcqecg5d6e0u3pfohmbjt"
type TestContainerExecRunnerClient struct {
- imageLoaded string
- logReader io.ReadCloser
- logWriter io.WriteCloser
- fn func(t *TestContainerExecRunnerClient)
- exitCode int
- stop chan bool
- cwd string
- env []string
- api *ArvTestClient
- realTemp string
- calledWait bool
- ctrExited bool
+ imageLoaded string
+ removedImages map[string]bool
+ logReader io.ReadCloser
+ logWriter io.WriteCloser
+ fn func(t *TestContainerExecRunnerClient)
+ exitCode int
+ stop chan bool
+ cwd string
+ env []string
+ api *ArvTestClient
+ realTemp string
+ calledWait bool
+ ctrExited bool
}
func NewTestContainerExecRunnerClient() *TestContainerExecRunnerClient {
@@ -109,6 +110,7 @@ func NewTestContainerExecRunnerClient() *TestContainerExecRunnerClient {
t.logReader, t.logWriter = io.Pipe()
t.stop = make(chan bool, 1)
t.cwd = "/"
+ t.removedImages = make(map[string]bool)
return t
}
@@ -185,18 +187,19 @@ func (t *TestContainerExecRunnerClient) ContainerInspect(ctx context.Context, id
return
}
-func (t *TestContainerExecRunnerClient) ImageInspectWithRaw(ctx context.Context, image string) (ImageInspectResponse, []byte, error) {
- fmt.Printf("%#v", t)
+func (t *TestContainerExecRunnerClient) ImageLocallyCached(ctx context.Context, image string) (bool, error) {
if t.exitCode == 2 {
- return ImageInspectResponse{}, nil, fmt.Errorf("Cannot connect to the Docker daemon at unix:///var/run/docker.sock. Is the docker daemon running?")
+ return false, fmt.Errorf("Cannot connect to the Docker daemon at unix:///var/run/docker.sock. Is the docker daemon running?")
}
+ hasBeenRemoved, ok := t.removedImages[image]
- if t.imageLoaded == image {
- return ImageInspectResponse{}, nil, nil
+ fmt.Printf("%s hasBeenRemoved %#v, ok %#v \n", image, hasBeenRemoved, ok)
+ if t.imageLoaded == image && (!hasBeenRemoved && ok) {
+ return true, nil
}
- return ImageInspectResponse{}, nil, errors.New("")
-}
+ return false, nil
+}
func (t *TestContainerExecRunnerClient) ImageLoad(ctx context.Context, input io.Reader, quiet bool) (ImageLoadResponse, error) {
if t.exitCode == 2 {
return ImageLoadResponse{}, fmt.Errorf("Cannot connect to the Docker daemon at unix:///var/run/docker.sock. Is the docker daemon running?")
@@ -206,10 +209,15 @@ func (t *TestContainerExecRunnerClient) ImageLoad(ctx context.Context, input io.
return ImageLoadResponse{}, err
}
t.imageLoaded = hwImageID
+ t.removedImages[hwImageID] = false
return ImageLoadResponse{Body: ioutil.NopCloser(input)}, nil
}
-func (*TestContainerExecRunnerClient) ImageRemove(ctx context.Context, image string, options ImageRemoveOptions) ([]ImageDeleteResponseItem, error) {
+func (t *TestContainerExecRunnerClient) ImageRemove(ctx context.Context, image string, options ImageRemoveOptions) ([]ImageDeleteResponseItem, error) {
+ // this holds the state of the images that have been removed so
+ // ImageLocallyCached can give a good (cached) answer
+ t.removedImages[image] = true
+
return nil, nil
}
@@ -222,11 +230,11 @@ func (t *TestContainerExecRunnerClient) GetHostConfig() (HostConfig, error) {
adapterHostConfig := HostConfig{
Resources: Resources{
- CgroupParent: "",
- NanoCPUs: 1000000,
- Memory: 100000,
- MemorySwap: 10000,
- KernelMemory: 10000,
+ CgroupParent: "fakeExecRunner", // usually "docker" for docker runners
+ NanoCPUs: int64(2) * 1000000000,
+ Memory: int64(16) * 1024 * 1024,
+ MemorySwap: int64(16) * 1024 * 1024,
+ KernelMemory: int64(16) * 1024 * 1024,
},
}
return adapterHostConfig, nil
@@ -245,7 +253,7 @@ func (t *TestContainerExecRunnerClient) SetImage(imageID string) {
}
func (t *TestContainerExecRunnerClient) GetNetworkMode() (networkMode NetworkMode) {
- return NetworkMode("...")
+ return NetworkMode("default")
}
func (t *TestContainerExecRunnerClient) SetNetworkMode(networkMode NetworkMode) {
@@ -486,8 +494,9 @@ func (s *TestSuite) TestLoadImage(c *C) {
_, err = cr.ContainerExecRunner.ImageRemove(nil, hwImageID, ImageRemoveOptions{})
c.Check(err, IsNil)
- _, _, err = cr.ContainerExecRunner.ImageInspectWithRaw(nil, hwImageID)
- c.Check(err, NotNil)
+ imageIsThere, err := cr.ContainerExecRunner.ImageLocallyCached(context.Background(), hwImageID)
+ c.Check(err, IsNil)
+ c.Check(imageIsThere, Equals, false)
cr.Container.ContainerImage = hwPDH
@@ -505,13 +514,18 @@ func (s *TestSuite) TestLoadImage(c *C) {
c.Check(kc.Called, Equals, true)
c.Check(cr.ContainerExecRunner.GetImage(), Equals, hwImageID)
- _, _, err = cr.ContainerExecRunner.ImageInspectWithRaw(nil, hwImageID)
+ imageIsThere, err = cr.ContainerExecRunner.ImageLocallyCached(context.Background(), hwImageID)
c.Check(err, IsNil)
+ c.Check(imageIsThere, Equals, true)
// (2) Test using image that's already loaded
kc.Called = false
cr.ContainerExecRunner.SetImage("")
+ imageIsThere, err = cr.ContainerExecRunner.ImageLocallyCached(context.Background(), hwImageID)
+ c.Check(err, IsNil)
+ c.Check(imageIsThere, Equals, true)
+
err = cr.LoadImage()
c.Check(err, IsNil)
c.Check(kc.Called, Equals, false)
diff --git a/lib/crunchrun/docker_adapter.go b/lib/crunchrun/docker_adapter.go
index f6c1a1a61..302be0d57 100644
--- a/lib/crunchrun/docker_adapter.go
+++ b/lib/crunchrun/docker_adapter.go
@@ -165,13 +165,13 @@ func (a *DockerAdapter) ContainerWait(ctx context.Context, container string, con
return adapterContainerWaitOKBody, dockerErr
}
-func (a *DockerAdapter) ImageInspectWithRaw(ctx context.Context, image string) (ImageInspectResponse, []byte, error) {
- dockerImageInspectResponse, rawBytes, dockerErr := a.docker.ImageInspectWithRaw(ctx, image)
-
- adapterImageInspectResponse := &ImageInspectResponse{
- ID: dockerImageInspectResponse.ID,
- }
- return *adapterImageInspectResponse, rawBytes, dockerErr
+func (a *DockerAdapter) ImageLocallyCached(ctx context.Context, image string) (bool, error) {
+ // Inherit logic before Singulatity ( see #17296 )
+ // There may be there is a better way to detect if the image is present in the
+ // local docker daemon.
+ _, _, err := a.docker.ImageInspectWithRaw(context.TODO(), image)
+ imageIsThere := (err != nil)
+ return imageIsThere, err
}
func (a *DockerAdapter) ImageLoad(ctx context.Context, input io.Reader, quiet bool) (ImageLoadResponse, error) {
diff --git a/lib/crunchrun/singularity.go b/lib/crunchrun/singularity.go
index d83cf2804..5c42da476 100644
--- a/lib/crunchrun/singularity.go
+++ b/lib/crunchrun/singularity.go
@@ -82,12 +82,9 @@ func (c SingularityClient) ContainerInspect(ctx context.Context, id string) (Con
return ContainerInspectResponse{}, nil
}
-func (c SingularityClient) ImageInspectWithRaw(ctx context.Context, image string) (ImageInspectResponse, []byte, error) {
- fmt.Printf("placeholder for ImageInspectWithRaw() %s", image)
-
- return ImageInspectResponse{}, []byte(""), nil
+func (c SingularityClient) ImageLocallyCached(ctx context.Context, image string) (bool, error) {
+ return false, nil
}
-
func (c SingularityClient) ImageLoad(ctx context.Context, input io.Reader, quiet bool) (ImageLoadResponse, error) {
fmt.Printf("placeholder for ImageLoad")
return ImageLoadResponse{}, nil
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list