[arvados] created: 2.6.0-205-g32a45cde3
git repository hosting
git at public.arvados.org
Mon May 29 16:15:05 UTC 2023
at 32a45cde3bd2ef806032d237b38b4d022774a2c2 (commit)
commit 32a45cde3bd2ef806032d237b38b4d022774a2c2
Author: Tom Clegg <tom at curii.com>
Date: Fri May 26 15:22:01 2023 -0400
20540: Remove arvadosclient usage in copier and git_tree.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/lib/crunchrun/copier.go b/lib/crunchrun/copier.go
index 72c714dfa..3b37227fa 100644
--- a/lib/crunchrun/copier.go
+++ b/lib/crunchrun/copier.go
@@ -51,7 +51,6 @@ type filetodo struct {
// manifest, err := (&copier{...}).Copy()
type copier struct {
client *arvados.Client
- arvClient IArvadosClient
keepClient IKeepClient
hostOutputDir string
ctrOutputDir string
@@ -356,7 +355,7 @@ func (cp *copier) getManifest(pdh string) (*manifest.Manifest, error) {
return mft, nil
}
var coll arvados.Collection
- err := cp.arvClient.Get("collections", pdh, nil, &coll)
+ err := cp.client.RequestAndDecode(&coll, "GET", "arvados/v1/collections/"+pdh, nil, nil)
if err != nil {
return nil, fmt.Errorf("error retrieving collection record for %q: %s", pdh, err)
}
diff --git a/lib/crunchrun/copier_test.go b/lib/crunchrun/copier_test.go
index 5e9249016..c8936d1a9 100644
--- a/lib/crunchrun/copier_test.go
+++ b/lib/crunchrun/copier_test.go
@@ -12,7 +12,6 @@ import (
"syscall"
"git.arvados.org/arvados.git/sdk/go/arvados"
- "git.arvados.org/arvados.git/sdk/go/arvadosclient"
"git.arvados.org/arvados.git/sdk/go/arvadostest"
"github.com/sirupsen/logrus"
check "gopkg.in/check.v1"
@@ -27,12 +26,9 @@ type copierSuite struct {
func (s *copierSuite) SetUpTest(c *check.C) {
tmpdir := c.MkDir()
- api, err := arvadosclient.MakeArvadosClient()
- c.Assert(err, check.IsNil)
s.log = bytes.Buffer{}
s.cp = copier{
client: arvados.NewClientFromEnv(),
- arvClient: api,
hostOutputDir: tmpdir,
ctrOutputDir: "/ctr/outdir",
mounts: map[string]arvados.Mount{
diff --git a/lib/crunchrun/crunchrun.go b/lib/crunchrun/crunchrun.go
index 4a514f3d8..cab09d11c 100644
--- a/lib/crunchrun/crunchrun.go
+++ b/lib/crunchrun/crunchrun.go
@@ -638,7 +638,7 @@ func (runner *ContainerRunner) SetupMounts() (map[string]bindmount, error) {
if err != nil {
return nil, fmt.Errorf("creating temp dir: %v", err)
}
- err = gitMount(mnt).extractTree(runner.ContainerArvClient, tmpdir, token)
+ err = gitMount(mnt).extractTree(runner.containerClient, tmpdir, token)
if err != nil {
return nil, err
}
@@ -1345,7 +1345,6 @@ func (runner *ContainerRunner) CaptureOutput(bindmounts map[string]bindmount) er
txt, err := (&copier{
client: runner.containerClient,
- arvClient: runner.ContainerArvClient,
keepClient: runner.ContainerKeepClient,
hostOutputDir: runner.HostOutputDir,
ctrOutputDir: runner.Container.OutputPath,
diff --git a/lib/crunchrun/crunchrun_test.go b/lib/crunchrun/crunchrun_test.go
index 9c4fe20bc..aa20104f3 100644
--- a/lib/crunchrun/crunchrun_test.go
+++ b/lib/crunchrun/crunchrun_test.go
@@ -17,6 +17,8 @@ import (
"math/rand"
"net/http"
"net/http/httptest"
+ "net/http/httputil"
+ "net/url"
"os"
"os/exec"
"path"
@@ -38,6 +40,8 @@ import (
"git.arvados.org/arvados.git/sdk/go/manifest"
. "gopkg.in/check.v1"
+ git_client "gopkg.in/src-d/go-git.v4/plumbing/transport/client"
+ git_http "gopkg.in/src-d/go-git.v4/plumbing/transport/http"
)
// Gocheck boilerplate
@@ -416,6 +420,67 @@ func (client *KeepTestClient) ManifestFileReader(m manifest.Manifest, filename s
return nil, nil
}
+type apiStubServer struct {
+ server *httptest.Server
+ proxy *httputil.ReverseProxy
+ intercept func(http.ResponseWriter, *http.Request) bool
+
+ container arvados.Container
+ logs map[string]string
+}
+
+func apiStub() (*arvados.Client, *apiStubServer) {
+ client := arvados.NewClientFromEnv()
+ apistub := &apiStubServer{}
+ apistub.server = httptest.NewTLSServer(apistub)
+ apistub.proxy = httputil.NewSingleHostReverseProxy(&url.URL{Scheme: "https", Host: client.APIHost})
+ if client.Insecure {
+ apistub.proxy.Transport = arvados.InsecureHTTPClient.Transport
+ }
+ client.APIHost = apistub.server.Listener.Addr().String()
+ return client, apistub
+}
+
+func (apistub *apiStubServer) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+ if apistub.intercept != nil && apistub.intercept(w, r) {
+ return
+ }
+ if r.Method == "POST" && r.URL.Path == "/arvados/v1/logs" {
+ var body struct {
+ Log struct {
+ EventType string `json:"event_type"`
+ Properties struct {
+ Text string
+ }
+ }
+ }
+ json.NewDecoder(r.Body).Decode(&body)
+ apistub.logs[body.Log.EventType] += body.Log.Properties.Text
+ return
+ }
+ if r.Method == "GET" && r.URL.Path == "/arvados/v1/collections/"+hwPDH {
+ json.NewEncoder(w).Encode(arvados.Collection{ManifestText: hwManifest})
+ return
+ }
+ if r.Method == "GET" && r.URL.Path == "/arvados/v1/collections/"+otherPDH {
+ json.NewEncoder(w).Encode(arvados.Collection{ManifestText: otherManifest})
+ return
+ }
+ if r.Method == "GET" && r.URL.Path == "/arvados/v1/collections/"+normalizedWithSubdirsPDH {
+ json.NewEncoder(w).Encode(arvados.Collection{ManifestText: normalizedManifestWithSubdirs})
+ return
+ }
+ if r.Method == "GET" && r.URL.Path == "/arvados/v1/collections/"+denormalizedWithSubdirsPDH {
+ json.NewEncoder(w).Encode(arvados.Collection{ManifestText: denormalizedManifestWithSubdirs})
+ return
+ }
+ if r.Method == "GET" && r.URL.Path == "/arvados/v1/containers/"+apistub.container.UUID {
+ json.NewEncoder(w).Encode(apistub.container)
+ return
+ }
+ apistub.proxy.ServeHTTP(w, r)
+}
+
func (s *TestSuite) TestLoadImage(c *C) {
s.runner.Container.ContainerImage = arvadostest.DockerImage112PDH
s.runner.Container.Mounts = map[string]arvados.Mount{
@@ -687,8 +752,9 @@ func (s *TestSuite) fullRunHelper(c *C, record string, extraMounts []string, fn
}
return d, err
}
+ client, _ := apiStub()
s.runner.MkArvClient = func(token string) (IArvadosClient, IKeepClient, *arvados.Client, error) {
- return &ArvTestClient{secretMounts: secretMounts}, &s.testContainerKeepClient, nil, nil
+ return &ArvTestClient{secretMounts: secretMounts}, &s.testContainerKeepClient, client, nil
}
if extraMounts != nil && len(extraMounts) > 0 {
@@ -1352,6 +1418,7 @@ func (s *TestSuite) TestSetupMounts(c *C) {
cr := s.runner
am := &ArvMountCmdLine{}
cr.RunArvMount = am.ArvMountTest
+ cr.containerClient, _ = apiStub()
cr.ContainerArvClient = &ArvTestClient{}
cr.ContainerKeepClient = &KeepTestClient{}
cr.Container.OutputStorageClasses = []string{"default"}
@@ -1674,7 +1741,7 @@ func (s *TestSuite) TestSetupMounts(c *C) {
{
i = 0
cr.ArvMountPoint = ""
- (*GitMountSuite)(nil).useTestGitServer(c)
+ git_client.InstallProtocol("https", git_http.NewClient(arvados.InsecureHTTPClient))
cr.token = arvadostest.ActiveToken
cr.Container.Mounts = make(map[string]arvados.Mount)
cr.Container.Mounts = map[string]arvados.Mount{
diff --git a/lib/crunchrun/git_mount.go b/lib/crunchrun/git_mount.go
index 92bb6d11d..561ea18de 100644
--- a/lib/crunchrun/git_mount.go
+++ b/lib/crunchrun/git_mount.go
@@ -48,25 +48,22 @@ func (gm gitMount) validate() error {
// ExtractTree extracts the specified tree into dir, which is an
// existing empty local directory.
-func (gm gitMount) extractTree(ac IArvadosClient, dir string, token string) error {
+func (gm gitMount) extractTree(ac *arvados.Client, dir string, token string) error {
err := gm.validate()
if err != nil {
return err
}
- baseURL, err := ac.Discovery("gitUrl")
+ dd, err := ac.DiscoveryDocument()
if err != nil {
- return fmt.Errorf("discover gitUrl from API: %s", err)
- } else if _, ok := baseURL.(string); !ok {
- return fmt.Errorf("discover gitUrl from API: expected string, found %T", baseURL)
+ return fmt.Errorf("error getting discovery document: %w", err)
}
-
- u, err := url.Parse(baseURL.(string))
+ u, err := url.Parse(dd.GitURL)
if err != nil {
- return fmt.Errorf("parse gitUrl %q: %s", baseURL, err)
+ return fmt.Errorf("parse gitUrl %q: %s", dd.GitURL, err)
}
u, err = u.Parse("/" + gm.UUID + ".git")
if err != nil {
- return fmt.Errorf("build git url from %q, %q: %s", baseURL, gm.UUID, err)
+ return fmt.Errorf("build git url from %q, %q: %s", dd.GitURL, gm.UUID, err)
}
store := memory.NewStorage()
repo, err := git.Init(store, osfs.New(dir))
diff --git a/lib/crunchrun/git_mount_test.go b/lib/crunchrun/git_mount_test.go
index e39beaa94..ac98dcc48 100644
--- a/lib/crunchrun/git_mount_test.go
+++ b/lib/crunchrun/git_mount_test.go
@@ -6,14 +6,11 @@ package crunchrun
import (
"io/ioutil"
- "net/url"
"os"
"path/filepath"
- "git.arvados.org/arvados.git/lib/config"
"git.arvados.org/arvados.git/sdk/go/arvados"
"git.arvados.org/arvados.git/sdk/go/arvadostest"
- "git.arvados.org/arvados.git/sdk/go/ctxlog"
check "gopkg.in/check.v1"
git_client "gopkg.in/src-d/go-git.v4/plumbing/transport/client"
git_http "gopkg.in/src-d/go-git.v4/plumbing/transport/http"
@@ -26,11 +23,10 @@ type GitMountSuite struct {
var _ = check.Suite(&GitMountSuite{})
func (s *GitMountSuite) SetUpTest(c *check.C) {
- s.useTestGitServer(c)
-
var err error
s.tmpdir, err = ioutil.TempDir("", "")
c.Assert(err, check.IsNil)
+ git_client.InstallProtocol("https", git_http.NewClient(arvados.InsecureHTTPClient))
}
func (s *GitMountSuite) TearDownTest(c *check.C) {
@@ -39,13 +35,14 @@ func (s *GitMountSuite) TearDownTest(c *check.C) {
}
// Commit fd3531f is crunch-run-tree-test
-func (s *GitMountSuite) TestextractTree(c *check.C) {
+func (s *GitMountSuite) TestExtractTree(c *check.C) {
gm := gitMount{
Path: "/",
UUID: arvadostest.Repository2UUID,
Commit: "fd3531f42995344f36c30b79f55f27b502f3d344",
}
- err := gm.extractTree(&ArvTestClient{}, s.tmpdir, arvadostest.ActiveToken)
+ ac := arvados.NewClientFromEnv()
+ err := gm.extractTree(ac, s.tmpdir, arvadostest.ActiveToken)
c.Check(err, check.IsNil)
fnm := filepath.Join(s.tmpdir, "dir1/dir2/file with mode 0644")
@@ -85,7 +82,7 @@ func (s *GitMountSuite) TestExtractNonTipCommit(c *check.C) {
UUID: arvadostest.Repository2UUID,
Commit: "5ebfab0522851df01fec11ec55a6d0f4877b542e",
}
- err := gm.extractTree(&ArvTestClient{}, s.tmpdir, arvadostest.ActiveToken)
+ err := gm.extractTree(arvados.NewClientFromEnv(), s.tmpdir, arvadostest.ActiveToken)
c.Check(err, check.IsNil)
fnm := filepath.Join(s.tmpdir, "file only on testbranch")
@@ -100,7 +97,7 @@ func (s *GitMountSuite) TestNonexistentRepository(c *check.C) {
UUID: "zzzzz-s0uqq-nonexistentrepo",
Commit: "5ebfab0522851df01fec11ec55a6d0f4877b542e",
}
- err := gm.extractTree(&ArvTestClient{}, s.tmpdir, arvadostest.ActiveToken)
+ err := gm.extractTree(arvados.NewClientFromEnv(), s.tmpdir, arvadostest.ActiveToken)
c.Check(err, check.NotNil)
c.Check(err, check.ErrorMatches, ".*repository not found.*")
@@ -113,7 +110,7 @@ func (s *GitMountSuite) TestNonexistentCommit(c *check.C) {
UUID: arvadostest.Repository2UUID,
Commit: "bb66b6bb6b6bbb6b6b6b66b6b6b6b6b6b6b6b66b",
}
- err := gm.extractTree(&ArvTestClient{}, s.tmpdir, arvadostest.ActiveToken)
+ err := gm.extractTree(arvados.NewClientFromEnv(), s.tmpdir, arvadostest.ActiveToken)
c.Check(err, check.NotNil)
c.Check(err, check.ErrorMatches, ".*object not found.*")
@@ -127,8 +124,8 @@ func (s *GitMountSuite) TestGitUrlDiscoveryFails(c *check.C) {
UUID: arvadostest.Repository2UUID,
Commit: "5ebfab0522851df01fec11ec55a6d0f4877b542e",
}
- err := gm.extractTree(&ArvTestClient{}, s.tmpdir, arvadostest.ActiveToken)
- c.Check(err, check.ErrorMatches, ".*gitUrl.*")
+ err := gm.extractTree(&arvados.Client{}, s.tmpdir, arvadostest.ActiveToken)
+ c.Check(err, check.ErrorMatches, ".*error getting discovery doc.*")
}
func (s *GitMountSuite) TestInvalid(c *check.C) {
@@ -186,7 +183,7 @@ func (s *GitMountSuite) TestInvalid(c *check.C) {
matcher: ".*writable.*",
},
} {
- err := trial.gm.extractTree(&ArvTestClient{}, s.tmpdir, arvadostest.ActiveToken)
+ err := trial.gm.extractTree(arvados.NewClientFromEnv(), s.tmpdir, arvadostest.ActiveToken)
c.Check(err, check.NotNil)
s.checkTmpdirContents(c, []string{})
@@ -202,15 +199,3 @@ func (s *GitMountSuite) checkTmpdirContents(c *check.C, expect []string) {
c.Check(err, check.IsNil)
c.Check(names, check.DeepEquals, expect)
}
-
-func (*GitMountSuite) useTestGitServer(c *check.C) {
- git_client.InstallProtocol("https", git_http.NewClient(arvados.InsecureHTTPClient))
-
- loader := config.NewLoader(nil, ctxlog.TestLogger(c))
- cfg, err := loader.Load()
- c.Assert(err, check.IsNil)
- cluster, err := cfg.GetCluster("")
- c.Assert(err, check.IsNil)
-
- discoveryMap["gitUrl"] = (*url.URL)(&cluster.Services.GitHTTP.ExternalURL).String()
-}
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list