From: Eric Biagiotti <ebiagiotti@veritasgenetics.com>
Date: Wed, 27 Feb 2019 19:24:58 +0000 (-0500)
Subject: Merge branch 'master' into 14885-ciso-and-conda-packaging-pr
X-Git-Tag: 1.4.0~125^2~3
X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/d0584563200b0fe69e508b2fc5b0ddb223ebccc0?hp=8fd360faf91921c557f44f079b127a4fa5830486

Merge branch 'master' into 14885-ciso-and-conda-packaging-pr

refs #14885

Arvados-DCO-1.1-Signed-off-by: Eric Biagiotti <ebiagiotti@veritasgenetics.com>
---

diff --git a/apps/workbench/app/views/layouts/body.html.erb b/apps/workbench/app/views/layouts/body.html.erb
index b017b4a29a..b2cd097f31 100644
--- a/apps/workbench/app/views/layouts/body.html.erb
+++ b/apps/workbench/app/views/layouts/body.html.erb
@@ -82,6 +82,21 @@ SPDX-License-Identifier: AGPL-3.0 %>
 		      </form>
                     </li>
                   <% end %>
+                <% if Rails.configuration.workbench2_url %>
+                <li role="menuitem">
+                  <%
+                    wb2_url = Rails.configuration.workbench2_url
+                    wb2_url += '/' if wb2_url[-1] != '/'
+                    wb2_url += 'token'
+                  %>
+                  <form action="<%= wb2_url %>" method="GET">
+                    <input type="hidden" name="api_token" value="<%= Thread.current[:arvados_api_token] %>">
+                    <button role="menuitem" type="submit">
+                      <i class="fa fa-lg fa-share-square fa-fw"></i> Go to Workbench 2
+                    </button>
+                  </form>
+                </li>
+                <% end %>
                 <li role="menuitem">
                   <%= link_to virtual_machines_user_path(current_user), role: 'menu-item' do %>
                     <i class="fa fa-lg fa-terminal fa-fw"></i> Virtual machines
diff --git a/apps/workbench/config/application.default.yml b/apps/workbench/config/application.default.yml
index 4e0a35a555..ccc7e4bbdd 100644
--- a/apps/workbench/config/application.default.yml
+++ b/apps/workbench/config/application.default.yml
@@ -326,3 +326,11 @@ common:
   # the jobs api is disabled and there are no local git repositories.
   #
   repositories: true
+
+  #
+  # Add an item to the user menu pointing to workbench2_url, if not false.
+  #
+  # Example:
+  # workbench2_url: https://workbench2.qr1hi.arvadosapi.com
+  #
+  workbench2_url: false
diff --git a/apps/workbench/config/initializers/validate_wb2_url_config.rb b/apps/workbench/config/initializers/validate_wb2_url_config.rb
new file mode 100644
index 0000000000..f9096486c1
--- /dev/null
+++ b/apps/workbench/config/initializers/validate_wb2_url_config.rb
@@ -0,0 +1,7 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+include ConfigValidators
+
+ConfigValidators::validate_wb2_url_config()
\ No newline at end of file
diff --git a/apps/workbench/lib/config_validators.rb b/apps/workbench/lib/config_validators.rb
new file mode 100644
index 0000000000..ec769168c2
--- /dev/null
+++ b/apps/workbench/lib/config_validators.rb
@@ -0,0 +1,28 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+require 'uri'
+
+module ConfigValidators
+    def validate_wb2_url_config
+        if Rails.configuration.workbench2_url
+            begin
+                if !URI.parse(Rails.configuration.workbench2_url).is_a?(URI::HTTP)
+                    Rails.logger.warn("workbench2_url config is not an HTTP URL: #{Rails.configuration.workbench2_url}")
+                    Rails.configuration.workbench2_url = false
+                elsif /.*[\/]{2,}$/.match(Rails.configuration.workbench2_url)
+                    Rails.logger.warn("workbench2_url config shouldn't have multiple trailing slashes: #{Rails.configuration.workbench2_url}")
+                    Rails.configuration.workbench2_url = false
+                else
+                    return true
+                end
+            rescue URI::InvalidURIError
+                Rails.logger.warn("workbench2_url config invalid URL: #{Rails.configuration.workbench2_url}")
+                Rails.configuration.workbench2_url = false
+            end
+        end
+        return false
+    end
+end
+
diff --git a/apps/workbench/test/integration/application_layout_test.rb b/apps/workbench/test/integration/application_layout_test.rb
index 74a42877b1..b3f704cdd9 100644
--- a/apps/workbench/test/integration/application_layout_test.rb
+++ b/apps/workbench/test/integration/application_layout_test.rb
@@ -140,6 +140,30 @@ class ApplicationLayoutTest < ActionDispatch::IntegrationTest
     end
   end
 
+  [
+    [false, false],
+    ['http://wb2.example.org//', false],
+    ['ftp://wb2.example.org', false],
+    ['wb2.example.org', false],
+    ['http://wb2.example.org', true],
+    ['https://wb2.example.org', true],
+    ['http://wb2.example.org/', true],
+    ['https://wb2.example.org/', true],
+  ].each do |wb2_url_config, wb2_menu_appear|
+    test "workbench2_url=#{wb2_url_config} should#{wb2_menu_appear ? '' : ' not'} show WB2 menu" do
+      Rails.configuration.workbench2_url = wb2_url_config
+      assert_equal wb2_menu_appear, ConfigValidators::validate_wb2_url_config()
+
+      visit page_with_token('active')
+      within('.navbar-fixed-top') do
+        page.find("#notifications-menu").click
+        within('.dropdown-menu') do
+          assert_equal wb2_menu_appear, page.has_text?('Go to Workbench 2')
+        end
+      end
+    end
+  end
+
   [
     ['active', true],
     ['active_with_prefs_profile_no_getting_started_shown', false],
diff --git a/doc/_includes/_install_compute_docker.liquid b/doc/_includes/_install_compute_docker.liquid
index 06db793314..ea3640e52a 100644
--- a/doc/_includes/_install_compute_docker.liquid
+++ b/doc/_includes/_install_compute_docker.liquid
@@ -73,7 +73,7 @@ h2. Download and tag the latest arvados/jobs docker image
 In order to start workflows from workbench, there needs to be Docker image tagged @arvados/jobs:latest@. The following command downloads the latest arvados/jobs image from Docker Hub, loads it into Keep, and tags it as 'latest'.  In this example @$project_uuid@ should be the the UUID of the "Arvados Standard Docker Images" project.
 
 <notextile>
-<pre><code>~$ <span class="userinput">arv-keepdocker --project-uuid $project_uuid --pull arvados/jobs latest</span>
+<pre><code>~$ <span class="userinput">arv-keepdocker --pull arvados/jobs latest --project-uuid $project_uuid</span>
 </code></pre></notextile>
 
 If the image needs to be downloaded from Docker Hub, the command can take a few minutes to complete, depending on available network bandwidth.
diff --git a/doc/user/topics/arv-docker.html.textile.liquid b/doc/user/topics/arv-docker.html.textile.liquid
index c21fbd9ad2..f34c21a9d7 100644
--- a/doc/user/topics/arv-docker.html.textile.liquid
+++ b/doc/user/topics/arv-docker.html.textile.liquid
@@ -210,6 +210,6 @@ h2. Share Docker images
 Docker images are subject to normal Arvados permissions.  If wish to share your Docker image with others (or wish to share a pipeline template that uses your Docker image) you will need to use @arv-keepdocker@ with the @--project-uuid@ option to upload the image to a shared project.
 
 <notextile>
-<pre><code>$ <span class="userinput">arv-keepdocker --project-uuid qr1hi-j7d0g-xxxxxxxxxxxxxxx arvados/jobs-with-r</span>
+<pre><code>$ <span class="userinput">arv-keepdocker arvados/jobs-with-r --project-uuid qr1hi-j7d0g-xxxxxxxxxxxxxxx</span>
 </code></pre>
 </notextile>
diff --git a/sdk/java/src/main/java/org/arvados/sdk/Arvados.java b/sdk/java/src/main/java/org/arvados/sdk/Arvados.java
index 031be97514..2b8bbee672 100644
--- a/sdk/java/src/main/java/org/arvados/sdk/Arvados.java
+++ b/sdk/java/src/main/java/org/arvados/sdk/Arvados.java
@@ -91,7 +91,6 @@ public class Arvados {
       }
     }
     arvadosRootUrl = "https://" + arvadosApiHost;
-    arvadosRootUrl += (arvadosApiHost.endsWith("/")) ? "" : "/";
 
     if (hostInsecure != null) {
       arvadosApiHostInsecure = Boolean.valueOf(hostInsecure);
diff --git a/services/keepstore/azure_blob_volume.go b/services/keepstore/azure_blob_volume.go
index 5da2055b77..4f7339facf 100644
--- a/services/keepstore/azure_blob_volume.go
+++ b/services/keepstore/azure_blob_volume.go
@@ -603,6 +603,9 @@ func (v *AzureBlobVolume) translateError(err error) error {
 	switch {
 	case err == nil:
 		return err
+	case strings.Contains(err.Error(), "StatusCode=503"):
+		// "storage: service returned error: StatusCode=503, ErrorCode=ServerBusy, ErrorMessage=The server is busy" (See #14804)
+		return VolumeBusyError
 	case strings.Contains(err.Error(), "Not Found"):
 		// "storage: service returned without a response body (404 Not Found)"
 		return os.ErrNotExist
diff --git a/services/keepstore/handler_test.go b/services/keepstore/handler_test.go
index c37a4d112f..32b360b127 100644
--- a/services/keepstore/handler_test.go
+++ b/services/keepstore/handler_test.go
@@ -49,6 +49,7 @@ type RequestTester struct {
 //   - permissions on, authenticated request, unsigned locator
 //   - permissions on, unauthenticated request, signed locator
 //   - permissions on, authenticated request, expired locator
+//   - permissions on, authenticated request, signed locator, transient error from backend
 //
 func TestGetHandler(t *testing.T) {
 	defer teardown()
@@ -151,6 +152,23 @@ func TestGetHandler(t *testing.T) {
 	ExpectStatusCode(t,
 		"Authenticated request, expired locator",
 		ExpiredError.HTTPCode, response)
+
+	// Authenticated request, signed locator
+	// => 503 Server busy (transient error)
+
+	// Set up the block owning volume to respond with errors
+	vols[0].(*MockVolume).Bad = true
+	vols[0].(*MockVolume).BadVolumeError = VolumeBusyError
+	response = IssueRequest(&RequestTester{
+		method:   "GET",
+		uri:      signedLocator,
+		apiToken: knownToken,
+	})
+	// A transient error from one volume while the other doesn't find the block
+	// should make the service return a 503 so that clients can retry.
+	ExpectStatusCode(t,
+		"Volume backend busy",
+		503, response)
 }
 
 // Test PutBlockHandler on the following situations:
diff --git a/services/keepstore/handlers.go b/services/keepstore/handlers.go
index e079b96784..2a1bbc972f 100644
--- a/services/keepstore/handlers.go
+++ b/services/keepstore/handlers.go
@@ -20,11 +20,10 @@ import (
 	"sync"
 	"time"
 
-	"github.com/gorilla/mux"
-
 	"git.curoverse.com/arvados.git/sdk/go/arvados"
 	"git.curoverse.com/arvados.git/sdk/go/health"
 	"git.curoverse.com/arvados.git/sdk/go/httpserver"
+	"github.com/gorilla/mux"
 )
 
 type router struct {
@@ -669,6 +668,11 @@ func GetBlock(ctx context.Context, hash string, buf []byte, resp http.ResponseWr
 			if !os.IsNotExist(err) {
 				log.Printf("%s: Get(%s): %s", vol, hash, err)
 			}
+			// If some volume returns a transient error, return it to the caller
+			// instead of "Not found" so it can retry.
+			if err == VolumeBusyError {
+				errorToCaller = err.(*KeepError)
+			}
 			continue
 		}
 		// Check the file checksum.
diff --git a/services/keepstore/keepstore.go b/services/keepstore/keepstore.go
index 6ae414bf93..a6c8cd9954 100644
--- a/services/keepstore/keepstore.go
+++ b/services/keepstore/keepstore.go
@@ -50,6 +50,7 @@ var (
 	DiskHashError       = &KeepError{500, "Hash mismatch in stored data"}
 	ExpiredError        = &KeepError{401, "Expired permission signature"}
 	NotFoundError       = &KeepError{404, "Not Found"}
+	VolumeBusyError     = &KeepError{503, "Volume backend busy"}
 	GenericError        = &KeepError{500, "Fail"}
 	FullError           = &KeepError{503, "Full"}
 	SizeRequiredError   = &KeepError{411, "Missing Content-Length"}
diff --git a/services/keepstore/keepstore_test.go b/services/keepstore/keepstore_test.go
index 26d49946a4..d1d380466b 100644
--- a/services/keepstore/keepstore_test.go
+++ b/services/keepstore/keepstore_test.go
@@ -7,6 +7,7 @@ package main
 import (
 	"bytes"
 	"context"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"os"
@@ -165,6 +166,7 @@ func TestPutBlockOneVol(t *testing.T) {
 
 	vols := KeepVM.AllWritable()
 	vols[0].(*MockVolume).Bad = true
+	vols[0].(*MockVolume).BadVolumeError = errors.New("Bad volume")
 
 	// Check that PutBlock stores the data as expected.
 	if n, err := PutBlock(context.Background(), TestBlock, TestHash); err != nil || n < 1 {
diff --git a/services/keepstore/volume_test.go b/services/keepstore/volume_test.go
index 43ddd090cc..046f3fac2e 100644
--- a/services/keepstore/volume_test.go
+++ b/services/keepstore/volume_test.go
@@ -40,7 +40,8 @@ type MockVolume struct {
 	Timestamps map[string]time.Time
 
 	// Bad volumes return an error for every operation.
-	Bad bool
+	Bad            bool
+	BadVolumeError error
 
 	// Touchable volumes' Touch() method succeeds for a locator
 	// that has been Put().
@@ -104,7 +105,7 @@ func (v *MockVolume) Compare(ctx context.Context, loc string, buf []byte) error
 	v.gotCall("Compare")
 	<-v.Gate
 	if v.Bad {
-		return errors.New("Bad volume")
+		return v.BadVolumeError
 	} else if block, ok := v.Store[loc]; ok {
 		if fmt.Sprintf("%x", md5.Sum(block)) != loc {
 			return DiskHashError
@@ -122,7 +123,7 @@ func (v *MockVolume) Get(ctx context.Context, loc string, buf []byte) (int, erro
 	v.gotCall("Get")
 	<-v.Gate
 	if v.Bad {
-		return 0, errors.New("Bad volume")
+		return 0, v.BadVolumeError
 	} else if block, ok := v.Store[loc]; ok {
 		copy(buf[:len(block)], block)
 		return len(block), nil
@@ -134,7 +135,7 @@ func (v *MockVolume) Put(ctx context.Context, loc string, block []byte) error {
 	v.gotCall("Put")
 	<-v.Gate
 	if v.Bad {
-		return errors.New("Bad volume")
+		return v.BadVolumeError
 	}
 	if v.Readonly {
 		return MethodDisabledError
@@ -162,7 +163,7 @@ func (v *MockVolume) Mtime(loc string) (time.Time, error) {
 	var mtime time.Time
 	var err error
 	if v.Bad {
-		err = errors.New("Bad volume")
+		err = v.BadVolumeError
 	} else if t, ok := v.Timestamps[loc]; ok {
 		mtime = t
 	} else {
diff --git a/tools/arvbox/lib/arvbox/docker/service/workbench/run-service b/tools/arvbox/lib/arvbox/docker/service/workbench/run-service
index 68c87233f0..6f13ee0278 100755
--- a/tools/arvbox/lib/arvbox/docker/service/workbench/run-service
+++ b/tools/arvbox/lib/arvbox/docker/service/workbench/run-service
@@ -44,6 +44,7 @@ $RAILS_ENV:
   arvados_docsite: http://$localip:${services[doc]}/
   force_ssl: false
   composer_url: http://$localip:${services[composer]}
+  workbench2_url: https://$localip:${services[workbench2-ssl]}
 EOF
 
 bundle exec rake assets:precompile