refactor to 3 exported API calls

sylabs · Apr 11, 2019 · 56555ec · 56555ec
1 parent 8be119f
commit 56555ec
Show file tree

Hide file tree

Showing 3 changed files with 89 additions and 143 deletions.
diff --git a/client/build.go b/client/build.go
@@ -11,79 +11,55 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
-	"strings"
 
 	"github.com/globalsign/mgo/bson"
 	"github.com/golang/glog"
 	"github.com/gorilla/websocket"
-	"github.com/pkg/errors"
 	jsonresp "github.com/sylabs/json-resp"
-	"github.com/sylabs/scs-library-client/client"
 )
 
-// CloudURI holds the URI of the Library web front-end.
-const CloudURI = "https://cloud.sylabs.io"
+// Build submits a build job to the Build Service. The context controls the
+// lifetime of the request.
+func (c *Client) SubmitBuild(ctx context.Context, d Definition, libraryRef string, libraryURL string) (rd ResponseData, err error) {
 
-func (c *Client) Build(ctx context.Context, imagePath string, definition Definition, isDetached bool) (string, error) {
-	var libraryRef string
-
-	if strings.HasPrefix(imagePath, "library://") {
-		// Image destination is Library.
-		libraryRef = imagePath
+	b, err := json.Marshal(RequestData{
+		Definition: d,
+		LibraryRef: libraryRef,
+		LibraryURL: libraryURL,
+	})
+	if err != nil {
+		return
 	}
 
-	// Send build request to Remote Build Service
-	rd, err := c.doBuildRequest(ctx, definition, libraryRef)
+	req, err := c.newRequest(http.MethodPost, "/v1/build", "", bytes.NewReader(b))
 	if err != nil {
-		err = errors.Wrap(err, "failed to post request to remote build service")
-		glog.Warningf("%v", err)
-		return "", err
+		return
 	}
+	req = req.WithContext(ctx)
+	req.Header.Set("Content-Type", "application/json")
+	glog.V(2).Infof("Sending build request to %s", req.URL.String())
 
-	// If we're doing an detached build, print help on how to download the image
-	libraryRefRaw := strings.TrimPrefix(rd.LibraryRef, "library://")
-	if isDetached {
-		// TODO - move this code outside this client, should be in singularity
-		fmt.Printf("Build submitted! Once it is complete, the image can be retrieved by running:\n")
-		fmt.Printf("\tsingularity pull --library %v library://%v\n\n", rd.LibraryURL, libraryRefRaw)
-		fmt.Printf("Alternatively, you can access it from a browser at:\n\t%v/library/%v\n", CloudURI, libraryRefRaw)
+	res, err := c.HTTPClient.Do(req)
+	if err != nil {
+		return
 	}
+	defer res.Body.Close()
 
-	// If we're doing an attached build, stream output and then download the resulting file
-	if !isDetached {
-		err = c.streamOutput(ctx, rd.WSURL)
-		if err != nil {
-			err = errors.Wrap(err, "failed to stream output from remote build service")
-			glog.Warningf("%v", err)
-			return "", err
-		}
-
-		// Get build status
-		rd, err = c.doStatusRequest(ctx, rd.ID)
-		if err != nil {
-			err = errors.Wrap(err, "failed to get status from remote build service")
-			glog.Warningf("%v", err)
-			return "", err
-		}
-
-		// Do not try to download image if not complete or image size is 0
-		if !rd.IsComplete {
-			return "", errors.New("build has not completed")
-		}
-		if rd.ImageSize <= 0 {
-			return "", errors.New("build image size <= 0")
-		}
+	err = jsonresp.ReadResponse(res.Body, &rd)
+	if err == nil {
+		glog.V(2).Infof("Build response - id: %s, wsurl: %s, libref: %s",
+			rd.ID.Hex(), rd.WSURL, rd.LibraryRef)
 	}
-
-	return rd.LibraryRef, nil
+	return
 }
 
-// streamOutput attaches via websocket and streams output to the console
-func (c *Client) streamOutput(ctx context.Context, url string) (err error) {
+// StreamOutput reads log output from the websocket URL. The context controls
+// the lifetime of the request.
+func (c *Client) StreamOutput(ctx context.Context, wsURL string) error {
 	h := http.Header{}
 	c.setRequestHeaders(h)
 
-	ws, resp, err := websocket.DefaultDialer.Dial(url, h)
+	ws, resp, err := websocket.DefaultDialer.Dial(wsURL, h)
 	if err != nil {
 		glog.V(2).Infof("websocket dial err - %s, partial response: %+v", err, resp)
 		return err
@@ -118,48 +94,9 @@ func (c *Client) streamOutput(ctx context.Context, url string) (err error) {
 	}
 }
 
-// doBuildRequest creates a new build on a Remote Build Service
-func (c *Client) doBuildRequest(ctx context.Context, d Definition, libraryRef string) (rd ResponseData, err error) {
-	if libraryRef != "" && !client.IsLibraryPushRef(libraryRef) {
-		err = fmt.Errorf("invalid library reference: %v", libraryRef)
-		glog.Warningf("%v", err)
-		return ResponseData{}, err
-	}
-
-	b, err := json.Marshal(RequestData{
-		Definition: d,
-		LibraryRef: libraryRef,
-		LibraryURL: c.LibraryURL.String(),
-	})
-	if err != nil {
-		return
-	}
-
-	req, err := c.newRequest(http.MethodPost, "/v1/build", "", bytes.NewReader(b))
-	if err != nil {
-		return
-	}
-	req = req.WithContext(ctx)
-	req.Header.Set("Content-Type", "application/json")
-	glog.V(2).Infof("Sending build request to %s", req.URL.String())
-
-	res, err := c.HTTPClient.Do(req)
-	if err != nil {
-		return
-	}
-	defer res.Body.Close()
-
-	err = jsonresp.ReadResponse(res.Body, &rd)
-	if err == nil {
-		glog.V(2).Infof("Build response - id: %s, wsurl: %s, libref: %s",
-			rd.ID.Hex(), rd.WSURL, rd.LibraryRef)
-	}
-	return
-}
-
-// doStatusRequest gets the status of a build from the Remote Build Service
-func (c *Client) doStatusRequest(ctx context.Context, id bson.ObjectId) (rd ResponseData, err error) {
-	req, err := c.newRequest(http.MethodGet, "/v1/build/"+id.Hex(), "", nil)
+// GetBuildStatus gets the status of a build from the Remote Build Service
+func (c *Client) GetBuildStatus(ctx context.Context, buildID bson.ObjectId) (rd ResponseData, err error) {
+	req, err := c.newRequest(http.MethodGet, "/v1/build/"+buildID.Hex(), "", nil)
 	if err != nil {
 		return
 	}

diff --git a/client/build_test.go b/client/build_test.go
@@ -175,37 +175,37 @@ func TestBuild(t *testing.T) {
 	// Table of tests to run
 	// nolint:maligned
 	tests := []struct {
-		description        string
-		expectSuccess      bool
-		imagePath          string
-		libraryURL         string
-		buildResponseCode  int
-		wsResponseCode     int
-		wsCloseCode        int
-		statusResponseCode int
-		imageResponseCode  int
-		ctx                context.Context
-		isDetached         bool
+		description         string
+		expectSubmitSuccess bool
+		expectStreamSuccess bool
+		expectStatusSuccess bool
+		imagePath           string
+		libraryURL          string
+		buildResponseCode   int
+		wsResponseCode      int
+		wsCloseCode         int
+		statusResponseCode  int
+		imageResponseCode   int
+		ctx                 context.Context
+		isDetached          bool
 	}{
-		{"SuccessAttached", true, f.Name(), "", http.StatusCreated, http.StatusOK, websocket.CloseNormalClosure, http.StatusOK, http.StatusOK, context.Background(), false},
-		{"SuccessDetached", true, f.Name(), "", http.StatusCreated, http.StatusOK, websocket.CloseNormalClosure, http.StatusOK, http.StatusOK, context.Background(), true},
-		{"SuccessLibraryRef", true, "library://user/collection/image", "", http.StatusCreated, http.StatusOK, websocket.CloseNormalClosure, http.StatusOK, http.StatusOK, context.Background(), false},
-		{"SuccessLibraryRefURL", true, "library://user/collection/image", m.httpAddr, http.StatusCreated, http.StatusOK, websocket.CloseNormalClosure, http.StatusOK, http.StatusOK, context.Background(), false},
-		{"BadLibraryRef", false, "library://bad", "", http.StatusCreated, http.StatusOK, websocket.CloseNormalClosure, http.StatusOK, http.StatusOK, context.Background(), false},
-		{"AddBuildFailure", false, f.Name(), "", http.StatusUnauthorized, http.StatusOK, websocket.CloseNormalClosure, http.StatusOK, http.StatusOK, context.Background(), false},
-		{"WebsocketFailure", false, f.Name(), "", http.StatusCreated, http.StatusUnauthorized, websocket.CloseNormalClosure, http.StatusOK, http.StatusOK, context.Background(), false},
-		{"WebsocketAbnormalClosure", false, f.Name(), "", http.StatusCreated, http.StatusOK, websocket.CloseAbnormalClosure, http.StatusOK, http.StatusOK, context.Background(), false},
-		{"GetStatusFailure", false, f.Name(), "", http.StatusCreated, http.StatusOK, websocket.CloseNormalClosure, http.StatusUnauthorized, http.StatusOK, context.Background(), false},
-		{"ContextExpired", false, f.Name(), "", http.StatusCreated, http.StatusOK, websocket.CloseNormalClosure, http.StatusOK, http.StatusOK, ctx, false},
+		{"SuccessAttached", true, true, true, f.Name(), "", http.StatusCreated, http.StatusOK, websocket.CloseNormalClosure, http.StatusOK, http.StatusOK, context.Background(), false},
+		{"SuccessDetached", true, true, true, f.Name(), "", http.StatusCreated, http.StatusOK, websocket.CloseNormalClosure, http.StatusOK, http.StatusOK, context.Background(), true},
+		{"SuccessLibraryRef", true, true, true, "library://user/collection/image", "", http.StatusCreated, http.StatusOK, websocket.CloseNormalClosure, http.StatusOK, http.StatusOK, context.Background(), false},
+		{"SuccessLibraryRefURL", true, true, true, "library://user/collection/image", m.httpAddr, http.StatusCreated, http.StatusOK, websocket.CloseNormalClosure, http.StatusOK, http.StatusOK, context.Background(), false},
+		{"AddBuildFailure", false, false, false, f.Name(), "", http.StatusUnauthorized, http.StatusOK, websocket.CloseNormalClosure, http.StatusOK, http.StatusOK, context.Background(), false},
+		{"WebsocketFailure", true, false, true, f.Name(), "", http.StatusCreated, http.StatusUnauthorized, websocket.CloseNormalClosure, http.StatusOK, http.StatusOK, context.Background(), false},
+		{"WebsocketAbnormalClosure", true, false, true, f.Name(), "", http.StatusCreated, http.StatusOK, websocket.CloseAbnormalClosure, http.StatusOK, http.StatusOK, context.Background(), false},
+		{"GetStatusFailure", true, true, false, f.Name(), "", http.StatusCreated, http.StatusOK, websocket.CloseNormalClosure, http.StatusUnauthorized, http.StatusOK, context.Background(), false},
+		{"ContextExpired", false, false, false, f.Name(), "", http.StatusCreated, http.StatusOK, websocket.CloseNormalClosure, http.StatusOK, http.StatusOK, ctx, false},
 	}
 
 	// Loop over test cases
 	for _, tt := range tests {
 		t.Run(tt.description, func(t *testing.T) {
 			rb, err := NewClient(&Config{
-				BaseURL:    url.String(),
-				AuthToken:  authToken,
-				LibraryURL: url.String(),
+				BaseURL:   url.String(),
+				AuthToken: authToken,
 			})
 			if err != nil {
 				t.Fatalf("failed to get new remote builder: %v", err)
@@ -219,17 +219,42 @@ func TestBuild(t *testing.T) {
 			m.imageResponseCode = tt.imageResponseCode
 
 			// Do it!
-			_, err = rb.Build(tt.ctx, tt.imagePath, Definition{}, tt.isDetached)
+			rd, err := rb.SubmitBuild(tt.ctx, Definition{}, tt.imagePath, url.String())
+			if !tt.expectSubmitSuccess {
+				// Ensure the handler returned an error
+				if err == nil {
+					t.Fatalf("unexpected submit success")
+				}
+				return
+			}
+			// Ensure the handler returned no error, and the response is as expected
+			if err != nil {
+				t.Fatalf("unexpected submit failure: %v", err)
+			}
 
-			if tt.expectSuccess {
+			err = rb.StreamOutput(tt.ctx, rd.WSURL)
+			if tt.expectStreamSuccess {
 				// Ensure the handler returned no error, and the response is as expected
 				if err != nil {
-					t.Fatalf("unexpected failure: %v", err)
+					t.Fatalf("unexpected stream failure: %v", err)
 				}
 			} else {
 				// Ensure the handler returned an error
 				if err == nil {
-					t.Fatalf("unexpected success")
+					t.Fatalf("unexpected stream success")
+				}
+			}
+
+			rd, err = rb.GetBuildStatus(tt.ctx, rd.ID)
+			if tt.expectStatusSuccess {
+				// Ensure the handler returned no error, and the response is as expected
+				if err != nil {
+					t.Fatalf("unexpected status failure: %v", err)
+				}
+			} else {
+				// Ensure the handler returned an error
+				if err == nil {
+					t.Fatalf("unexpected status success")
 				}
 			}
 		})
@@ -251,7 +276,6 @@ func TestDoBuildRequest(t *testing.T) {
 	}{
 		{"SuccessAttached", true, "", http.StatusCreated, context.Background()},
 		{"SuccessLibraryRef", true, "library://user/collection/image", http.StatusCreated, context.Background()},
-		{"BadLibraryRef", false, "library://bad", http.StatusCreated, context.Background()},
 		{"NotFoundAttached", false, "", http.StatusNotFound, context.Background()},
 		{"ContextExpiredAttached", false, "", http.StatusCreated, ctx},
 	}
@@ -279,7 +303,7 @@ func TestDoBuildRequest(t *testing.T) {
 			m.buildResponseCode = tt.responseCode
 
 			// Call the handler
-			rd, err := rb.doBuildRequest(tt.ctx, Definition{}, tt.libraryRef)
+			rd, err := rb.SubmitBuild(tt.ctx, Definition{}, tt.libraryRef, "")
 
 			if tt.expectSuccess {
 				// Ensure the handler returned no error, and the response is as expected
@@ -351,7 +375,7 @@ func TestDoStatusRequest(t *testing.T) {
 			m.statusResponseCode = tt.responseCode
 
 			// Call the handler
-			rd, err := rb.doStatusRequest(tt.ctx, id)
+			rd, err := rb.GetBuildStatus(tt.ctx, id)
 
 			if tt.expectSuccess {
 				// Ensure the handler returned no error, and the response is as expected

diff --git a/client/client.go b/client/client.go
@@ -18,8 +18,6 @@ type Config struct {
 	BaseURL string
 	// Auth token to include in the Authorization header of each request (if supplied).
 	AuthToken string
-	// Library URL of the service (https://library.sylabs.io is used if not supplied).
-	LibraryURL string
 	// User agent to include in each request (if supplied).
 	UserAgent string
 	// HTTPClient to use to make HTTP requests (if supplied).
@@ -35,8 +33,6 @@ type Client struct {
 	BaseURL *url.URL
 	// Auth token to include in the Authorization header of each request (if supplied).
 	AuthToken string
-	// Library URL of the service
-	LibraryURL *url.URL
 	// User agent to include in each request (if supplied).
 	UserAgent string
 	// HTTPClient to use to make HTTP requests.
@@ -59,21 +55,10 @@ func NewClient(cfg *Config) (c *Client, err error) {
 		return nil, err
 	}
 
-	// Determine library URL
-	lu := "https://library.sylabs.io"
-	if cfg.LibraryURL != "" {
-		lu = cfg.LibraryURL
-	}
-	libraryURL, err := url.Parse(lu)
-	if err != nil {
-		return nil, err
-	}
-
 	c = &Client{
-		BaseURL:    baseURL,
-		AuthToken:  cfg.AuthToken,
-		LibraryURL: libraryURL,
-		UserAgent:  cfg.UserAgent,
+		BaseURL:   baseURL,
+		AuthToken: cfg.AuthToken,
+		UserAgent: cfg.UserAgent,
 	}
 
 	// Set HTTP client