[ARVADOS] updated: 2.2.1-34-g65ca88516

Git user git at public.arvados.org
Wed Aug 25 21:15:53 UTC 2021


Summary of changes:
 apps/workbench/Gemfile.lock                        |  2 +-
 apps/workbench/app/views/layouts/body.html.erb     |  6 --
 .../test/integration/application_layout_test.rb    |  2 -
 build/run-library.sh                               |  4 +-
 lib/config/config.default.yml                      |  1 +
 lib/config/export.go                               |  1 +
 lib/config/generated_config.go                     |  1 +
 lib/controller/integration_test.go                 | 46 +++++++++++-
 sdk/cwl/arvados_cwl/__init__.py                    |  4 ++
 sdk/cwl/arvados_cwl/arvtool.py                     | 10 ++-
 sdk/cwl/tests/test_submit.py                       | 16 +++++
 sdk/go/arvados/config.go                           |  1 +
 sdk/python/arvados/commands/keepdocker.py          |  3 +
 sdk/python/arvados/commands/put.py                 | 20 +++++-
 sdk/python/tests/test_arv_put.py                   | 84 ++++++++++++----------
 services/api/Gemfile.lock                          |  2 +-
 services/api/app/mailers/user_notifier.rb          |  7 +-
 .../api/app/models/api_client_authorization.rb     | 36 ++++++++--
 services/api/app/models/collection.rb              |  4 +-
 services/api/config/arvados_config.rb              |  1 +
 services/api/test/unit/collection_test.rb          | 17 +++++
 services/api/test/unit/user_notifier_test.rb       |  2 +
 services/fuse/arvados_fuse/fusedir.py              | 84 +++++++++++++++++-----
 23 files changed, 272 insertions(+), 82 deletions(-)

       via  65ca88516ae94716031f3f8fbd092ca9603d0572 (commit)
       via  07b08ba44b2a7b6325c4c3d1c51843ad8937faa5 (commit)
       via  77e5b6f64f54fb725bf462a39010ef2e21280577 (commit)
       via  4b08f199f83f09c5d58ba611d350d1885d359a9d (commit)
       via  6e28149f29d489eb799aa86dc01ff41835c1d1f7 (commit)
       via  eb99b5411c2512b4cfbc94433f3bc1e5d5cb773d (commit)
       via  c9a70cb670c9fdd73e3cef845968c89ea4dd8051 (commit)
       via  8b23523b8894106d7ac287fc222bf52900f39a79 (commit)
       via  f5792f8ccacee1bff26b521488745c1c2a74d6b2 (commit)
       via  9e373b68991fd4213aa3d44dbc684761e483b41f (commit)
       via  e390a79e2408a8e5df65f0b33cad84a3fca244af (commit)
       via  6ad2354ec1cdfa3636c85a63b3c909d62bab22a4 (commit)
       via  d1e1624bd32e96badb70b4793d3e2372c447acb0 (commit)
       via  7acab3f1a4eeeb7dc2e965b44f0d3b46572f3a8b (commit)
       via  b6b79755119983a5ca000205207a8fe24aaed550 (commit)
       via  20f8d664211ed80fb64289ec8a866cf7d741e6f3 (commit)
       via  6139d2a76538a9872399cf3329e4b001f9b7d8a9 (commit)
      from  1275d744db9177f16ffd71c76c5c567f6dfb9a9a (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 65ca88516ae94716031f3f8fbd092ca9603d0572
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date:   Tue Aug 17 15:20:59 2021 -0300

    18004: Fixes a couple of race condition bugs related to caching remote users.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>

diff --git a/lib/controller/integration_test.go b/lib/controller/integration_test.go
index d86766241..d1e98804d 100644
--- a/lib/controller/integration_test.go
+++ b/lib/controller/integration_test.go
@@ -188,7 +188,7 @@ func (s *IntegrationSuite) TestGetCollectionByPDH(c *check.C) {
 }
 
 // Tests bug #18004
-func (s *IntegrationSuite) TestRemoteTokenCacheRace(c *check.C) {
+func (s *IntegrationSuite) TestRemoteUserAndTokenCacheRace(c *check.C) {
 	conn1 := s.testClusters["z1111"].Conn()
 	rootctx1, _, _ := s.testClusters["z1111"].RootClients()
 	rootctx2, _, _ := s.testClusters["z2222"].RootClients()
@@ -207,7 +207,7 @@ func (s *IntegrationSuite) TestRemoteTokenCacheRace(c *check.C) {
 			defer wg2.Done()
 			wg1.Wait()
 			_, err := conn2.UserGetCurrent(rootctx2, arvados.GetOptions{})
-			c.Check(err, check.IsNil)
+			c.Check(err, check.IsNil, check.Commentf("warm up phase failed"))
 		}()
 	}
 	wg1.Done()
@@ -223,7 +223,7 @@ func (s *IntegrationSuite) TestRemoteTokenCacheRace(c *check.C) {
 			wg1.Wait()
 			// Retrieve the remote collection from cluster z2222.
 			_, err := conn2.UserGetCurrent(userctx1, arvados.GetOptions{})
-			c.Check(err, check.IsNil)
+			c.Check(err, check.IsNil, check.Commentf("testing phase failed"))
 		}()
 	}
 	wg1.Done()
diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb
index 52f2cee06..7c7ed759c 100644
--- a/services/api/app/models/api_client_authorization.rb
+++ b/services/api/app/models/api_client_authorization.rb
@@ -319,7 +319,17 @@ class ApiClientAuthorization < ArvadosModel
         user.last_name = "from cluster #{remote_user_prefix}"
       end
 
-      user.save!
+      begin
+        user.save!
+      rescue ActiveRecord::RecordInvalid, ActiveRecord::RecordNotUnique
+        Rails.logger.debug("remote user #{remote_user['uuid']} already exists, retrying...")
+        # Some other request won the race: retry fetching the user record.
+        user = User.find_by_uuid(remote_user['uuid'])
+        if !user
+          Rails.logger.warn("cannot find or create remote user #{remote_user['uuid']}")
+          return nil
+        end
+      end
 
       if user.is_invited && !remote_user['is_invited']
         # Remote user is not "invited" state, they should be unsetup, which
@@ -364,12 +374,24 @@ class ApiClientAuthorization < ArvadosModel
       exp = [db_current_time + Rails.configuration.Login.RemoteTokenRefresh,
              remote_token.andand['expires_at']].compact.min
       scopes = remote_token.andand['scopes'] || ['all']
-      auth = ApiClientAuthorization.find_or_create_by(uuid: token_uuid) do |auth|
-        auth.user = user
-        auth.api_token = stored_secret
-        auth.api_client_id = 0
-        auth.scopes = scopes
-        auth.expires_at = exp
+      begin
+        retries ||= 0
+        auth = ApiClientAuthorization.find_or_create_by(uuid: token_uuid) do |auth|
+          auth.user = user
+          auth.api_token = stored_secret
+          auth.api_client_id = 0
+          auth.scopes = scopes
+          auth.expires_at = exp
+        end
+      rescue ActiveRecord::RecordNotUnique
+        Rails.logger.debug("cached remote token #{token_uuid} already exists, retrying...")
+        # Some other request won the race: retry just once before erroring out
+        if (retries += 1) <= 1
+          retry
+        else
+          Rails.logger.warn("cannot find or create cached remote token #{token_uuid}")
+          return nil
+        end
       end
       auth.update_attributes!(user: user,
                               api_token: stored_secret,

commit 07b08ba44b2a7b6325c4c3d1c51843ad8937faa5
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date:   Tue Aug 10 12:15:04 2021 -0300

    18004: Adds test exposing the race condition.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>

diff --git a/lib/controller/integration_test.go b/lib/controller/integration_test.go
index 44c99bf30..d86766241 100644
--- a/lib/controller/integration_test.go
+++ b/lib/controller/integration_test.go
@@ -20,6 +20,7 @@ import (
 	"path/filepath"
 	"strconv"
 	"strings"
+	"sync"
 
 	"git.arvados.org/arvados.git/lib/boot"
 	"git.arvados.org/arvados.git/lib/config"
@@ -186,6 +187,49 @@ func (s *IntegrationSuite) TestGetCollectionByPDH(c *check.C) {
 	c.Check(coll.PortableDataHash, check.Equals, pdh)
 }
 
+// Tests bug #18004
+func (s *IntegrationSuite) TestRemoteTokenCacheRace(c *check.C) {
+	conn1 := s.testClusters["z1111"].Conn()
+	rootctx1, _, _ := s.testClusters["z1111"].RootClients()
+	rootctx2, _, _ := s.testClusters["z2222"].RootClients()
+	conn2 := s.testClusters["z2222"].Conn()
+	userctx1, _, _, _ := s.testClusters["z1111"].UserClients(rootctx1, c, conn1, "user2 at example.com", true)
+
+	var wg1, wg2 sync.WaitGroup
+	creqs := 100
+
+	// Make concurrent requests to z2222 with a local token to make sure more
+	// than one worker is listening.
+	wg1.Add(1)
+	for i := 0; i < creqs; i++ {
+		wg2.Add(1)
+		go func() {
+			defer wg2.Done()
+			wg1.Wait()
+			_, err := conn2.UserGetCurrent(rootctx2, arvados.GetOptions{})
+			c.Check(err, check.IsNil)
+		}()
+	}
+	wg1.Done()
+	wg2.Wait()
+
+	// Real test pass -- use a new remote token than the one used in the warm-up
+	// phase.
+	wg1.Add(1)
+	for i := 0; i < creqs; i++ {
+		wg2.Add(1)
+		go func() {
+			defer wg2.Done()
+			wg1.Wait()
+			// Retrieve the remote collection from cluster z2222.
+			_, err := conn2.UserGetCurrent(userctx1, arvados.GetOptions{})
+			c.Check(err, check.IsNil)
+		}()
+	}
+	wg1.Done()
+	wg2.Wait()
+}
+
 func (s *IntegrationSuite) TestS3WithFederatedToken(c *check.C) {
 	if _, err := exec.LookPath("s3cmd"); err != nil {
 		c.Skip("s3cmd not in PATH")
@@ -433,7 +477,7 @@ func (s *IntegrationSuite) TestCreateContainerRequestWithBadToken(c *check.C) {
 }
 
 // We test the direct access to the database
-// normally an integration test would not have a database access, but  in this case we need
+// normally an integration test would not have a database access, but in this case we need
 // to test tokens that are secret, so there is no API response that will give them back
 func (s *IntegrationSuite) dbConn(c *check.C, clusterID string) (*sql.DB, *sql.Conn) {
 	ctx := context.Background()

commit 77e5b6f64f54fb725bf462a39010ef2e21280577
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date:   Tue Jul 13 16:14:49 2021 -0300

    17913: Upgrades 'addressable' gem to 2.8.0
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>

diff --git a/apps/workbench/Gemfile.lock b/apps/workbench/Gemfile.lock
index c177b14de..dc62a9c64 100644
--- a/apps/workbench/Gemfile.lock
+++ b/apps/workbench/Gemfile.lock
@@ -57,7 +57,7 @@ GEM
       i18n (>= 0.7, < 2)
       minitest (~> 5.1)
       tzinfo (~> 1.1)
-    addressable (2.7.0)
+    addressable (2.8.0)
       public_suffix (>= 2.0.2, < 5.0)
     andand (1.3.3)
     angularjs-rails (1.3.15)
diff --git a/services/api/Gemfile.lock b/services/api/Gemfile.lock
index 3781a5bab..8dcc3b6e6 100644
--- a/services/api/Gemfile.lock
+++ b/services/api/Gemfile.lock
@@ -53,7 +53,7 @@ GEM
       activemodel (>= 3.0.0)
       activesupport (>= 3.0.0)
       rack (>= 1.1.0)
-    addressable (2.7.0)
+    addressable (2.8.0)
       public_suffix (>= 2.0.2, < 5.0)
     andand (1.3.3)
     arel (9.0.0)

commit 4b08f199f83f09c5d58ba611d350d1885d359a9d
Author: Peter Amstutz <peter.amstutz at curii.com>
Date:   Mon Aug 16 15:33:10 2021 -0400

    17983: Fix writable fuse deadlock
    
    Resulting from threads competing for llfuse.lock and collection lock
    and one code path locking in the wrong order.
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>

diff --git a/services/fuse/arvados_fuse/fusedir.py b/services/fuse/arvados_fuse/fusedir.py
index a2e3ac139..69977bda9 100644
--- a/services/fuse/arvados_fuse/fusedir.py
+++ b/services/fuse/arvados_fuse/fusedir.py
@@ -298,20 +298,52 @@ class CollectionDirectoryBase(Directory):
     def on_event(self, event, collection, name, item):
         if collection == self.collection:
             name = self.sanitize_filename(name)
-            _logger.debug("collection notify %s %s %s %s", event, collection, name, item)
-            with llfuse.lock:
-                if event == arvados.collection.ADD:
-                    self.new_entry(name, item, self.mtime())
-                elif event == arvados.collection.DEL:
-                    ent = self._entries[name]
-                    del self._entries[name]
-                    self.inodes.invalidate_entry(self, name)
-                    self.inodes.del_entry(ent)
-                elif event == arvados.collection.MOD:
-                    if hasattr(item, "fuse_entry") and item.fuse_entry is not None:
-                        self.inodes.invalidate_inode(item.fuse_entry)
-                    elif name in self._entries:
-                        self.inodes.invalidate_inode(self._entries[name])
+
+            #
+            # It's possible for another thread to have llfuse.lock and
+            # be waiting on collection.lock.  Meanwhile, we released
+            # llfuse.lock earlier in the stack, but are still holding
+            # on to the collection lock, and now we need to re-acquire
+            # llfuse.lock.  If we don't release the collection lock,
+            # we'll deadlock where we're holding the collection lock
+            # waiting for llfuse.lock and the other thread is holding
+            # llfuse.lock and waiting for the collection lock.
+            #
+            # The correct locking order here is to take llfuse.lock
+            # first, then the collection lock.
+            #
+            # Since collection.lock is an RLock, it might be locked
+            # multiple times, so we need to release it multiple times,
+            # keep a count, then re-lock it the correct number of
+            # times.
+            #
+            lockcount = 0
+            try:
+                while True:
+                    self.collection.lock.release()
+                    lockcount += 1
+            except RuntimeError:
+                pass
+
+            try:
+                with llfuse.lock:
+                    with self.collection.lock:
+                        if event == arvados.collection.ADD:
+                            self.new_entry(name, item, self.mtime())
+                        elif event == arvados.collection.DEL:
+                            ent = self._entries[name]
+                            del self._entries[name]
+                            self.inodes.invalidate_entry(self, name)
+                            self.inodes.del_entry(ent)
+                        elif event == arvados.collection.MOD:
+                            if hasattr(item, "fuse_entry") and item.fuse_entry is not None:
+                                self.inodes.invalidate_inode(item.fuse_entry)
+                            elif name in self._entries:
+                                self.inodes.invalidate_inode(self._entries[name])
+            finally:
+                while lockcount > 0:
+                    self.collection.lock.acquire()
+                    lockcount -= 1
 
     def populate(self, mtime):
         self._mtime = mtime
@@ -584,10 +616,26 @@ class TmpCollectionDirectory(CollectionDirectoryBase):
     def on_event(self, *args, **kwargs):
         super(TmpCollectionDirectory, self).on_event(*args, **kwargs)
         if self.collection_record_file:
-            with llfuse.lock:
-                self.collection_record_file.invalidate()
-            self.inodes.invalidate_inode(self.collection_record_file)
-            _logger.debug("%s invalidated collection record", self)
+
+            # See discussion in CollectionDirectoryBase.on_event
+            lockcount = 0
+            try:
+                while True:
+                    self.collection.lock.release()
+                    lockcount += 1
+            except RuntimeError:
+                pass
+
+            try:
+                with llfuse.lock:
+                    with self.collection.lock:
+                        self.collection_record_file.invalidate()
+                        self.inodes.invalidate_inode(self.collection_record_file)
+                        _logger.debug("%s invalidated collection record", self)
+            finally:
+                while lockcount > 0:
+                    self.collection.lock.acquire()
+                    lockcount -= 1
 
     def collection_record(self):
         with llfuse.lock_released:

commit 6e28149f29d489eb799aa86dc01ff41835c1d1f7
Author: Ward Vandewege <ward at curii.com>
Date:   Fri Aug 13 15:33:17 2021 -0400

    18001: address review comments.
    
    No issue #
    
    Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward at curii.com>

diff --git a/services/api/config/arvados_config.rb b/services/api/config/arvados_config.rb
index a571f041e..fa4fd1b24 100644
--- a/services/api/config/arvados_config.rb
+++ b/services/api/config/arvados_config.rb
@@ -112,7 +112,7 @@ arvcfg.declare_config "Users.UserProfileNotificationAddress", String, :user_prof
 arvcfg.declare_config "Users.AdminNotifierEmailFrom", String, :admin_notifier_email_from
 arvcfg.declare_config "Users.EmailSubjectPrefix", String, :email_subject_prefix
 arvcfg.declare_config "Users.UserNotifierEmailFrom", String, :user_notifier_email_from
-arvcfg.declare_config "Users.UserNotifierEmailBcc", Hash, ->(cfg, k, v) { arrayToHash cfg, "Users.UserNotifierEmailBcc", v }
+arvcfg.declare_config "Users.UserNotifierEmailBcc", Hash
 arvcfg.declare_config "Users.NewUserNotificationRecipients", Hash, :new_user_notification_recipients, ->(cfg, k, v) { arrayToHash cfg, "Users.NewUserNotificationRecipients", v }
 arvcfg.declare_config "Users.NewInactiveUserNotificationRecipients", Hash, :new_inactive_user_notification_recipients, method(:arrayToHash)
 arvcfg.declare_config "Login.SSO.ProviderAppSecret", String, :sso_app_secret

commit eb99b5411c2512b4cfbc94433f3bc1e5d5cb773d
Author: Ward Vandewege <ward at curii.com>
Date:   Tue Aug 10 11:00:02 2021 -0400

    18001: add Users/UserNotifierEmailBcc configuration option, which is the
           e-mail address that will be bcc'd on the new user welcome e-mail.
    
    Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward at curii.com>

diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index bfd41f7d7..d0e770f92 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -273,6 +273,7 @@ Clusters:
       AdminNotifierEmailFrom: arvados at example.com
       EmailSubjectPrefix: "[ARVADOS] "
       UserNotifierEmailFrom: arvados at example.com
+      UserNotifierEmailBcc: {}
       NewUserNotificationRecipients: {}
       NewInactiveUserNotificationRecipients: {}
 
diff --git a/lib/config/export.go b/lib/config/export.go
index 6fe19cd2d..29bbd8536 100644
--- a/lib/config/export.go
+++ b/lib/config/export.go
@@ -224,6 +224,7 @@ var whitelist = map[string]bool{
 	"Users.NewUsersAreActive":                             false,
 	"Users.PreferDomainForUsername":                       false,
 	"Users.UserNotifierEmailFrom":                         false,
+	"Users.UserNotifierEmailBcc":                          false,
 	"Users.UserProfileNotificationAddress":                false,
 	"Users.UserSetupMailText":                             false,
 	"Volumes":                                             true,
diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index 07fba5d14..21ad02293 100644
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -279,6 +279,7 @@ Clusters:
       AdminNotifierEmailFrom: arvados at example.com
       EmailSubjectPrefix: "[ARVADOS] "
       UserNotifierEmailFrom: arvados at example.com
+      UserNotifierEmailBcc: {}
       NewUserNotificationRecipients: {}
       NewInactiveUserNotificationRecipients: {}
 
diff --git a/sdk/go/arvados/config.go b/sdk/go/arvados/config.go
index cdc25a548..288f1951e 100644
--- a/sdk/go/arvados/config.go
+++ b/sdk/go/arvados/config.go
@@ -238,6 +238,7 @@ type Cluster struct {
 		NewUserNotificationRecipients         StringSet
 		NewUsersAreActive                     bool
 		UserNotifierEmailFrom                 string
+		UserNotifierEmailBcc                  StringSet
 		UserProfileNotificationAddress        string
 		PreferDomainForUsername               string
 		UserSetupMailText                     string
diff --git a/services/api/app/mailers/user_notifier.rb b/services/api/app/mailers/user_notifier.rb
index ad887d035..03000a373 100644
--- a/services/api/app/mailers/user_notifier.rb
+++ b/services/api/app/mailers/user_notifier.rb
@@ -9,7 +9,12 @@ class UserNotifier < ActionMailer::Base
 
   def account_is_setup(user)
     @user = user
-    mail(to: user.email, subject: 'Welcome to Arvados - account enabled')
+    if not Rails.configuration.Users.UserNotifierEmailBcc.empty? then
+      @bcc = Rails.configuration.Users.UserNotifierEmailBcc.keys
+      mail(to: user.email, subject: 'Welcome to Arvados - account enabled', bcc: @bcc)
+    else
+      mail(to: user.email, subject: 'Welcome to Arvados - account enabled')
+    end
   end
 
 end
diff --git a/services/api/config/arvados_config.rb b/services/api/config/arvados_config.rb
index 2c259919a..a571f041e 100644
--- a/services/api/config/arvados_config.rb
+++ b/services/api/config/arvados_config.rb
@@ -112,6 +112,7 @@ arvcfg.declare_config "Users.UserProfileNotificationAddress", String, :user_prof
 arvcfg.declare_config "Users.AdminNotifierEmailFrom", String, :admin_notifier_email_from
 arvcfg.declare_config "Users.EmailSubjectPrefix", String, :email_subject_prefix
 arvcfg.declare_config "Users.UserNotifierEmailFrom", String, :user_notifier_email_from
+arvcfg.declare_config "Users.UserNotifierEmailBcc", Hash, ->(cfg, k, v) { arrayToHash cfg, "Users.UserNotifierEmailBcc", v }
 arvcfg.declare_config "Users.NewUserNotificationRecipients", Hash, :new_user_notification_recipients, ->(cfg, k, v) { arrayToHash cfg, "Users.NewUserNotificationRecipients", v }
 arvcfg.declare_config "Users.NewInactiveUserNotificationRecipients", Hash, :new_inactive_user_notification_recipients, method(:arrayToHash)
 arvcfg.declare_config "Login.SSO.ProviderAppSecret", String, :sso_app_secret
diff --git a/services/api/test/unit/user_notifier_test.rb b/services/api/test/unit/user_notifier_test.rb
index c288786c1..e58c273a6 100644
--- a/services/api/test/unit/user_notifier_test.rb
+++ b/services/api/test/unit/user_notifier_test.rb
@@ -10,6 +10,7 @@ class UserNotifierTest < ActionMailer::TestCase
   test "account is setup" do
     user = users :active
 
+    Rails.configuration.Users.UserNotifierEmailBcc = ConfigLoader.to_OrderedOptions({"bcc-notify at example.com"=>{},"bcc-notify2 at example.com"=>{}})
     Rails.configuration.Users.UserSetupMailText = %{
 <% if not @user.full_name.empty? -%>
 <%= @user.full_name %>,
@@ -33,6 +34,7 @@ The Arvados team.
 
     # Test the body of the sent email contains what we expect it to
     assert_equal Rails.configuration.Users.UserNotifierEmailFrom, email.from.first
+    assert_equal Rails.configuration.Users.UserNotifierEmailBcc.stringify_keys.keys, email.bcc
     assert_equal user.email, email.to.first
     assert_equal 'Welcome to Arvados - account enabled', email.subject
     assert (email.body.to_s.include? 'Your Arvados shell account has been set up'),

commit c9a70cb670c9fdd73e3cef845968c89ea4dd8051
Author: Peter Amstutz <peter.amstutz at curii.com>
Date:   Thu Aug 12 15:43:18 2021 -0400

    17984: Fix tests
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>

diff --git a/sdk/cwl/tests/test_submit.py b/sdk/cwl/tests/test_submit.py
index 29d27544e..6cf59f25e 100644
--- a/sdk/cwl/tests/test_submit.py
+++ b/sdk/cwl/tests/test_submit.py
@@ -853,6 +853,7 @@ class TestSubmit(unittest.TestCase):
     @stubs
     def test_submit_container_project(self, stubs):
         project_uuid = 'zzzzz-j7d0g-zzzzzzzzzzzzzzz'
+        stubs.api.groups().get().execute.return_value = {"group_class": "project"}
         exited = arvados_cwl.main(
             ["--submit", "--no-wait", "--api=containers", "--debug", "--project-uuid="+project_uuid,
                 "tests/wf/submit_wf.cwl", "tests/submit_test_job.json"],
@@ -1392,6 +1393,7 @@ class TestCreateWorkflow(unittest.TestCase):
     @stubs
     def test_create(self, stubs):
         project_uuid = 'zzzzz-j7d0g-zzzzzzzzzzzzzzz'
+        stubs.api.groups().get().execute.return_value = {"group_class": "project"}
 
         exited = arvados_cwl.main(
             ["--create-workflow", "--debug",
@@ -1421,6 +1423,7 @@ class TestCreateWorkflow(unittest.TestCase):
     @stubs
     def test_create_name(self, stubs):
         project_uuid = 'zzzzz-j7d0g-zzzzzzzzzzzzzzz'
+        stubs.api.groups().get().execute.return_value = {"group_class": "project"}
 
         exited = arvados_cwl.main(
             ["--create-workflow", "--debug",
@@ -1496,6 +1499,7 @@ class TestCreateWorkflow(unittest.TestCase):
     @stubs
     def test_create_collection_per_tool(self, stubs):
         project_uuid = 'zzzzz-j7d0g-zzzzzzzzzzzzzzz'
+        stubs.api.groups().get().execute.return_value = {"group_class": "project"}
 
         exited = arvados_cwl.main(
             ["--create-workflow", "--debug",
@@ -1525,6 +1529,7 @@ class TestCreateWorkflow(unittest.TestCase):
     @stubs
     def test_create_with_imports(self, stubs):
         project_uuid = 'zzzzz-j7d0g-zzzzzzzzzzzzzzz'
+        stubs.api.groups().get().execute.return_value = {"group_class": "project"}
 
         exited = arvados_cwl.main(
             ["--create-workflow", "--debug",
@@ -1543,6 +1548,7 @@ class TestCreateWorkflow(unittest.TestCase):
     @stubs
     def test_create_with_no_input(self, stubs):
         project_uuid = 'zzzzz-j7d0g-zzzzzzzzzzzzzzz'
+        stubs.api.groups().get().execute.return_value = {"group_class": "project"}
 
         exited = arvados_cwl.main(
             ["--create-workflow", "--debug",

commit 8b23523b8894106d7ac287fc222bf52900f39a79
Author: Peter Amstutz <peter.amstutz at curii.com>
Date:   Tue Aug 10 15:44:27 2021 -0400

    17984: Improve error reporting a bit
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>

diff --git a/sdk/cwl/arvados_cwl/__init__.py b/sdk/cwl/arvados_cwl/__init__.py
index 04db611fb..2e491fe5b 100644
--- a/sdk/cwl/arvados_cwl/__init__.py
+++ b/sdk/cwl/arvados_cwl/__init__.py
@@ -22,6 +22,7 @@ import cwltool.main
 import cwltool.workflow
 import cwltool.process
 import cwltool.argparser
+from cwltool.errors import WorkflowException
 from cwltool.process import shortname, UnsupportedRequirement, use_custom_schema
 from cwltool.utils import adjustFileObjs, adjustDirObjs, get_listing
 
@@ -301,6 +302,9 @@ def main(args, stdout, stderr, api_client=None, keep_client=None,
         if keep_client is None:
             keep_client = arvados.keep.KeepClient(api_client=api_client, num_retries=4)
         executor = ArvCwlExecutor(api_client, arvargs, keep_client=keep_client, num_retries=4)
+    except WorkflowException as e:
+        logger.error(e, exc_info=(sys.exc_info()[1] if arvargs.debug else False))
+        return 1
     except Exception:
         logger.exception("Error creating the Arvados CWL Executor")
         return 1
diff --git a/sdk/cwl/arvados_cwl/arvtool.py b/sdk/cwl/arvados_cwl/arvtool.py
index c79b1e67b..13664a8df 100644
--- a/sdk/cwl/arvados_cwl/arvtool.py
+++ b/sdk/cwl/arvados_cwl/arvtool.py
@@ -19,7 +19,7 @@ def validate_cluster_target(arvrunner, runtimeContext):
     if runtimeContext.project_uuid:
         cluster_target = runtimeContext.submit_runner_cluster or arvrunner.api._rootDesc["uuidPrefix"]
         if not runtimeContext.project_uuid.startswith(cluster_target):
-            raise WorkflowException("Project uuid '%s' must be for target cluster '%s'" % (runtimeContext.project_uuid, cluster_target))
+            raise WorkflowException("Project uuid '%s' should start with id of target cluster '%s'" % (runtimeContext.project_uuid, cluster_target))
 
         try:
             if runtimeContext.project_uuid[5:12] == '-tpzed-':
@@ -29,7 +29,7 @@ def validate_cluster_target(arvrunner, runtimeContext):
                 if proj["group_class"] != "project":
                     raise Exception("not a project, group_class is '%s'" % (proj["group_class"]))
         except Exception as e:
-            raise WorkflowException("Invalid value for project uuid '%s': %s" % (runtimeContext.project_uuid, e))
+            raise WorkflowException("Invalid project uuid '%s': %s" % (runtimeContext.project_uuid, e))
 
 def set_cluster_target(tool, arvrunner, builder, runtimeContext):
     cluster_target_req = None

commit f5792f8ccacee1bff26b521488745c1c2a74d6b2
Author: Peter Amstutz <peter.amstutz at curii.com>
Date:   Tue Aug 10 15:29:45 2021 -0400

    17984: validation accepts user uuid in --project-uuid
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>

diff --git a/sdk/cwl/arvados_cwl/arvtool.py b/sdk/cwl/arvados_cwl/arvtool.py
index 891769235..c79b1e67b 100644
--- a/sdk/cwl/arvados_cwl/arvtool.py
+++ b/sdk/cwl/arvados_cwl/arvtool.py
@@ -20,10 +20,16 @@ def validate_cluster_target(arvrunner, runtimeContext):
         cluster_target = runtimeContext.submit_runner_cluster or arvrunner.api._rootDesc["uuidPrefix"]
         if not runtimeContext.project_uuid.startswith(cluster_target):
             raise WorkflowException("Project uuid '%s' must be for target cluster '%s'" % (runtimeContext.project_uuid, cluster_target))
+
         try:
-            arvrunner.api.groups().get(uuid=runtimeContext.project_uuid).execute()
+            if runtimeContext.project_uuid[5:12] == '-tpzed-':
+                arvrunner.api.users().get(uuid=runtimeContext.project_uuid).execute()
+            else:
+                proj = arvrunner.api.groups().get(uuid=runtimeContext.project_uuid).execute()
+                if proj["group_class"] != "project":
+                    raise Exception("not a project, group_class is '%s'" % (proj["group_class"]))
         except Exception as e:
-            raise WorkflowException("Invalid project uuid '%s': %s" % (runtimeContext.project_uuid, e))
+            raise WorkflowException("Invalid value for project uuid '%s': %s" % (runtimeContext.project_uuid, e))
 
 def set_cluster_target(tool, arvrunner, builder, runtimeContext):
     cluster_target_req = None
diff --git a/sdk/cwl/tests/test_submit.py b/sdk/cwl/tests/test_submit.py
index 12daf6b67..29d27544e 100644
--- a/sdk/cwl/tests/test_submit.py
+++ b/sdk/cwl/tests/test_submit.py
@@ -1265,12 +1265,14 @@ class TestSubmit(unittest.TestCase):
 
     @stubs
     def test_submit_validate_project_uuid(self, stubs):
+        # Fails with bad cluster prefix
         exited = arvados_cwl.main(
             ["--submit", "--no-wait", "--api=containers", "--debug", "--project-uuid=zzzzb-j7d0g-zzzzzzzzzzzzzzz",
              "tests/wf/submit_wf.cwl", "tests/submit_test_job.json"],
             stubs.capture_stdout, sys.stderr, api_client=stubs.api, keep_client=stubs.keep_client)
         self.assertEqual(exited, 1)
 
+        # Project lookup fails
         stubs.api.groups().get().execute.side_effect = Exception("Bad project")
         exited = arvados_cwl.main(
             ["--submit", "--no-wait", "--api=containers", "--debug", "--project-uuid=zzzzz-j7d0g-zzzzzzzzzzzzzzx",
@@ -1278,6 +1280,14 @@ class TestSubmit(unittest.TestCase):
             stubs.capture_stdout, sys.stderr, api_client=stubs.api, keep_client=stubs.keep_client)
         self.assertEqual(exited, 1)
 
+        # It should work this time because it is looking up a user (and only group is stubbed out to fail)
+        exited = arvados_cwl.main(
+            ["--submit", "--no-wait", "--api=containers", "--debug", "--project-uuid=zzzzz-tpzed-zzzzzzzzzzzzzzx",
+             "tests/wf/submit_wf.cwl", "tests/submit_test_job.json"],
+            stubs.capture_stdout, sys.stderr, api_client=stubs.api, keep_client=stubs.keep_client)
+        self.assertEqual(exited, 0)
+
+
     @mock.patch("arvados.collection.CollectionReader")
     @stubs
     def test_submit_uuid_inputs(self, stubs, collectionReader):

commit 9e373b68991fd4213aa3d44dbc684761e483b41f
Author: Ward Vandewege <ward at curii.com>
Date:   Wed Aug 11 11:19:35 2021 -0400

    17985: fix binstub for cwltool in our arvados-cwl-runner package.
    
    Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward at curii.com>

diff --git a/build/run-library.sh b/build/run-library.sh
index 51d2f35de..97fb81fef 100755
--- a/build/run-library.sh
+++ b/build/run-library.sh
@@ -690,8 +690,8 @@ fpm_build_virtualenv () {
   fi
 
   # the python3-arvados-cwl-runner package comes with cwltool, expose that version
-  if [[ -e "$WORKSPACE/$PKG_DIR/dist/build/usr/share/$python/dist/python-arvados-cwl-runner/bin/cwltool" ]]; then
-    COMMAND_ARR+=("usr/share/$python/dist/python-arvados-cwl-runner/bin/cwltool=/usr/bin/")
+  if [[ -e "$WORKSPACE/$PKG_DIR/dist/build/usr/share/$python/dist/$PYTHON_PKG/bin/cwltool" ]]; then
+    COMMAND_ARR+=("usr/share/$python/dist/$PYTHON_PKG/bin/cwltool=/usr/bin/")
   fi
 
   COMMAND_ARR+=(".")

commit e390a79e2408a8e5df65f0b33cad84a3fca244af
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date:   Wed Aug 11 15:37:46 2021 -0300

    18005: Fixes the bug.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>

diff --git a/services/api/app/models/collection.rb b/services/api/app/models/collection.rb
index 4e7b64cf5..c522088ff 100644
--- a/services/api/app/models/collection.rb
+++ b/services/api/app/models/collection.rb
@@ -150,7 +150,9 @@ class Collection < ArvadosModel
   def strip_signatures_and_update_replication_confirmed
     if self.manifest_text_changed?
       in_old_manifest = {}
-      if not self.replication_confirmed.nil?
+      # manifest_text_was could be nil when dealing with a freshly created snapshot,
+      # so we skip this case because there was no real manifest change. (Bug #18005)
+      if (not self.replication_confirmed.nil?) and (not self.manifest_text_was.nil?)
         self.class.each_manifest_locator(manifest_text_was) do |match|
           in_old_manifest[match[1]] = true
         end

commit 6ad2354ec1cdfa3636c85a63b3c909d62bab22a4
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date:   Wed Aug 11 15:31:11 2021 -0300

    18005: Exposes the bug by simulating a keep-balance run a making a new version.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>

diff --git a/services/api/test/unit/collection_test.rb b/services/api/test/unit/collection_test.rb
index 916ca0958..b977a5bb1 100644
--- a/services/api/test/unit/collection_test.rb
+++ b/services/api/test/unit/collection_test.rb
@@ -185,6 +185,23 @@ class CollectionTest < ActiveSupport::TestCase
       c.reload
       assert_equal 'foobar', c.name
       assert_equal 2, c.version
+      # Simulate a keep-balance run and trigger a new versionable update
+      # This tests bug #18005
+      assert_nil c.replication_confirmed
+      assert_nil c.replication_confirmed_at
+      # Updates without validations/callbacks
+      c.update_column('modified_at', fifteen_min_ago)
+      c.update_column('replication_confirmed_at', Time.now)
+      c.update_column('replication_confirmed', 2)
+      c.reload
+      assert_equal fifteen_min_ago.to_i, c.modified_at.to_i
+      assert_not_nil c.replication_confirmed_at
+      assert_not_nil c.replication_confirmed
+      # Make the versionable update
+      c.update_attributes!({'name' => 'foobarbaz'})
+      c.reload
+      assert_equal 'foobarbaz', c.name
+      assert_equal 3, c.version
     end
   end
 

commit d1e1624bd32e96badb70b4793d3e2372c447acb0
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date:   Mon Aug 9 15:12:48 2021 -0300

    17936: Updates arv-keepdocker to call arv-put with --batch argument.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>

diff --git a/sdk/python/arvados/commands/keepdocker.py b/sdk/python/arvados/commands/keepdocker.py
index 8eb9dd75a..f3df4e3f3 100644
--- a/sdk/python/arvados/commands/keepdocker.py
+++ b/sdk/python/arvados/commands/keepdocker.py
@@ -498,6 +498,9 @@ def main(arguments=None, stdout=sys.stdout, install_sig_handlers=True, api=None)
         arguments = [i for i in arguments if i not in (args.image, args.tag, image_repo_tag)]
         put_args = keepdocker_parser.parse_known_args(arguments)[1]
 
+        # Don't fail when cached manifest is invalid, just ignore the cache.
+        put_args += ['--batch']
+
         if args.name is None:
             put_args += ['--name', collection_name]
 

commit 7acab3f1a4eeeb7dc2e965b44f0d3b46572f3a8b
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date:   Mon Aug 9 14:58:41 2021 -0300

    17936: Adds --batch implementation, making the new test pass.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>

diff --git a/sdk/python/arvados/commands/put.py b/sdk/python/arvados/commands/put.py
index d307ddaf9..fce89464f 100644
--- a/sdk/python/arvados/commands/put.py
+++ b/sdk/python/arvados/commands/put.py
@@ -444,7 +444,7 @@ class ArvPutUploadJob(object):
     }
 
     def __init__(self, paths, resume=True, use_cache=True, reporter=None,
-                 name=None, owner_uuid=None, api_client=None,
+                 name=None, owner_uuid=None, api_client=None, batch_mode=False,
                  ensure_unique_name=False, num_retries=None,
                  put_threads=None, replication_desired=None, filename=None,
                  update_time=60.0, update_collection=None, storage_classes=None,
@@ -454,6 +454,7 @@ class ArvPutUploadJob(object):
         self.paths = paths
         self.resume = resume
         self.use_cache = use_cache
+        self.batch_mode = batch_mode
         self.update = False
         self.reporter = reporter
         # This will set to 0 before start counting, if no special files are going
@@ -915,7 +916,12 @@ class ArvPutUploadJob(object):
                 # No cache file, set empty state
                 self._state = copy.deepcopy(self.EMPTY_STATE)
             if not self._cached_manifest_valid():
-                raise ResumeCacheInvalidError()
+                if not self.batch_mode:
+                    raise ResumeCacheInvalidError()
+                else:
+                    self.logger.info("Invalid signatures on cache file '{}' while being run in 'batch mode' -- continuing anyways.".format(self._cache_file.name))
+                    self.use_cache = False # Don't overwrite preexisting cache file.
+                    self._state = copy.deepcopy(self.EMPTY_STATE)
             # Load the previous manifest so we can check if files were modified remotely.
             self._local_collection = arvados.collection.Collection(
                 self._state['manifest'],
@@ -1260,6 +1266,7 @@ def main(arguments=None, stdout=sys.stdout, stderr=sys.stderr,
         writer = ArvPutUploadJob(paths = args.paths,
                                  resume = args.resume,
                                  use_cache = args.use_cache,
+                                 batch_mode= args.batch,
                                  filename = args.filename,
                                  reporter = reporter,
                                  api_client = api_client,
@@ -1288,7 +1295,8 @@ def main(arguments=None, stdout=sys.stdout, stderr=sys.stderr,
             "         or been created with another Arvados user's credentials.",
             "         Switch user or use one of the following options to restart upload:",
             "         --no-resume to start a new resume cache.",
-            "         --no-cache to disable resume cache."]))
+            "         --no-cache to disable resume cache.",
+            "         --batch to ignore the resume cache if invalid."]))
         sys.exit(1)
     except (CollectionUpdateError, PathDoesNotExistError) as error:
         logger.error("\n".join([
diff --git a/sdk/python/tests/test_arv_put.py b/sdk/python/tests/test_arv_put.py
index bcbedc050..de58d5f3e 100644
--- a/sdk/python/tests/test_arv_put.py
+++ b/sdk/python/tests/test_arv_put.py
@@ -1103,6 +1103,9 @@ class ArvPutIntegrationTest(run_test_server.TestCaseWithServers,
                     r'ERROR: arv-put: Resume cache contains invalid signature.*')
                 self.assertEqual(p.returncode, 1)
             else:
+                self.assertRegex(
+                    err.decode(),
+                    r'Invalid signatures on cache file \'.*\' while being run in \'batch mode\' -- continuing anyways.*')
                 self.assertEqual(p.returncode, 0)
 
     def test_single_expired_signature_reuploads_file(self):

commit b6b79755119983a5ca000205207a8fe24aaed550
Author: Lucas Di Pentima <lucas.dipentima at curii.com>
Date:   Mon Aug 9 13:56:44 2021 -0300

    17936: Adds --batch argument to arv-put, updates test.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima at curii.com>

diff --git a/sdk/python/arvados/commands/put.py b/sdk/python/arvados/commands/put.py
index f5129d333..d307ddaf9 100644
--- a/sdk/python/arvados/commands/put.py
+++ b/sdk/python/arvados/commands/put.py
@@ -215,6 +215,12 @@ Do not print any debug messages to console. (Any error messages will
 still be displayed.)
 """)
 
+run_opts.add_argument('--batch', action='store_true', default=False,
+                      help="""
+Retries with '--no-resume --no-cache' if cached state contains invalid/expired
+block signatures.
+""")
+
 _group = run_opts.add_mutually_exclusive_group()
 _group.add_argument('--resume', action='store_true', default=True,
                     help="""
diff --git a/sdk/python/tests/test_arv_put.py b/sdk/python/tests/test_arv_put.py
index eb97ebcfa..bcbedc050 100644
--- a/sdk/python/tests/test_arv_put.py
+++ b/sdk/python/tests/test_arv_put.py
@@ -1060,43 +1060,50 @@ class ArvPutIntegrationTest(run_test_server.TestCaseWithServers,
             r'INFO: Cache expired, starting from scratch.*')
         self.assertEqual(p.returncode, 0)
 
-    def test_invalid_signature_invalidates_cache(self):
-        self.authorize_with('active')
-        tmpdir = self.make_tmpdir()
-        with open(os.path.join(tmpdir, 'somefile.txt'), 'w') as f:
-            f.write('foo')
-        # Upload a directory and get the cache file name
-        p = subprocess.Popen([sys.executable, arv_put.__file__, tmpdir],
-                             stdout=subprocess.PIPE,
-                             stderr=subprocess.PIPE,
-                             env=self.ENVIRON)
-        (_, err) = p.communicate()
-        self.assertRegex(err.decode(), r'INFO: Creating new cache file at ')
-        self.assertEqual(p.returncode, 0)
-        cache_filepath = re.search(r'INFO: Creating new cache file at (.*)',
-                                   err.decode()).groups()[0]
-        self.assertTrue(os.path.isfile(cache_filepath))
-        # Load the cache file contents and modify the manifest to simulate
-        # an invalid access token
-        with open(cache_filepath, 'r') as c:
-            cache = json.load(c)
-        self.assertRegex(cache['manifest'], r'\+A\S+\@')
-        cache['manifest'] = re.sub(
-            r'\+A.*\@',
-            "+Aabcdef0123456789abcdef0123456789abcdef01@",
-            cache['manifest'])
-        with open(cache_filepath, 'w') as c:
-            c.write(json.dumps(cache))
-        # Re-run the upload and expect to get an invalid cache message
-        p = subprocess.Popen([sys.executable, arv_put.__file__, tmpdir],
-                             stdout=subprocess.PIPE,
-                             stderr=subprocess.PIPE,
-                             env=self.ENVIRON)
-        (_, err) = p.communicate()
-        self.assertRegex(
-            err.decode(),
-            r'ERROR: arv-put: Resume cache contains invalid signature.*')
-        self.assertEqual(p.returncode, 1)
+    def test_invalid_signature_in_cache(self):
+        for batch_mode in [False, True]:
+            self.authorize_with('active')
+            tmpdir = self.make_tmpdir()
+            with open(os.path.join(tmpdir, 'somefile.txt'), 'w') as f:
+                f.write('foo')
+            # Upload a directory and get the cache file name
+            arv_put_args = [tmpdir]
+            if batch_mode:
+                arv_put_args = ['--batch'] + arv_put_args
+            p = subprocess.Popen([sys.executable, arv_put.__file__] + arv_put_args,
+                                stdout=subprocess.PIPE,
+                                stderr=subprocess.PIPE,
+                                env=self.ENVIRON)
+            (_, err) = p.communicate()
+            self.assertRegex(err.decode(), r'INFO: Creating new cache file at ')
+            self.assertEqual(p.returncode, 0)
+            cache_filepath = re.search(r'INFO: Creating new cache file at (.*)',
+                                    err.decode()).groups()[0]
+            self.assertTrue(os.path.isfile(cache_filepath))
+            # Load the cache file contents and modify the manifest to simulate
+            # an invalid access token
+            with open(cache_filepath, 'r') as c:
+                cache = json.load(c)
+            self.assertRegex(cache['manifest'], r'\+A\S+\@')
+            cache['manifest'] = re.sub(
+                r'\+A.*\@',
+                "+Aabcdef0123456789abcdef0123456789abcdef01@",
+                cache['manifest'])
+            with open(cache_filepath, 'w') as c:
+                c.write(json.dumps(cache))
+            # Re-run the upload and expect to get an invalid cache message
+            p = subprocess.Popen([sys.executable, arv_put.__file__] + arv_put_args,
+                                stdout=subprocess.PIPE,
+                                stderr=subprocess.PIPE,
+                                env=self.ENVIRON)
+            (_, err) = p.communicate()
+            if not batch_mode:
+                self.assertRegex(
+                    err.decode(),
+                    r'ERROR: arv-put: Resume cache contains invalid signature.*')
+                self.assertEqual(p.returncode, 1)
+            else:
+                self.assertEqual(p.returncode, 0)
 
     def test_single_expired_signature_reuploads_file(self):
         self.authorize_with('active')

commit 20f8d664211ed80fb64289ec8a866cf7d741e6f3
Author: Peter Amstutz <peter.amstutz at curii.com>
Date:   Thu Aug 5 10:25:00 2021 -0400

    17952: Remove tests for presence of nodes / disks menu item
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>

diff --git a/apps/workbench/test/integration/application_layout_test.rb b/apps/workbench/test/integration/application_layout_test.rb
index 7d34c43de..35a141521 100644
--- a/apps/workbench/test/integration/application_layout_test.rb
+++ b/apps/workbench/test/integration/application_layout_test.rb
@@ -251,9 +251,7 @@ class ApplicationLayoutTest < ActionDispatch::IntegrationTest
     ['SSH keys', nil, 'public_key'],
     ['Links', nil, 'link_class'],
     ['Groups', nil, 'All users'],
-    ['Compute nodes', nil, 'ping_secret'],
     ['Keep services', nil, 'service_ssl_flag'],
-    ['Keep disks', nil, 'bytes_free'],
   ].each do |page_name, add_button_text, look_for|
     test "test system menu #{page_name} link" do
       visit page_with_token('admin')

commit 6139d2a76538a9872399cf3329e4b001f9b7d8a9
Author: Peter Amstutz <peter.amstutz at curii.com>
Date:   Wed Aug 4 17:38:09 2021 -0400

    17952: Remove links to obsolete "nodes" and "keep disks"
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz at curii.com>

diff --git a/apps/workbench/app/views/layouts/body.html.erb b/apps/workbench/app/views/layouts/body.html.erb
index 9da55cbeb..ed19d51d2 100644
--- a/apps/workbench/app/views/layouts/body.html.erb
+++ b/apps/workbench/app/views/layouts/body.html.erb
@@ -161,15 +161,9 @@ SPDX-License-Identifier: AGPL-3.0 %>
                   <li role="menuitem"><a href="/groups">
                       <i class="fa fa-lg fa-users fa-fw"></i> Groups
                   </a></li>
-                  <li role="menuitem"><a href="/nodes">
-                      <i class="fa fa-lg fa-cloud fa-fw"></i> Compute nodes
-                  </a></li>
                   <li role="menuitem"><a href="/keep_services">
                       <i class="fa fa-lg fa-exchange fa-fw"></i> Keep services
                   </a></li>
-                  <li role="menuitem"><a href="/keep_disks">
-                      <i class="fa fa-lg fa-hdd-o fa-fw"></i> Keep disks
-                  </a></li>
                 </ul>
               </li>
             <% end %>

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list