[ARVADOS] updated: 2.1.0-1509-g067a68b5e

Git user git at public.arvados.org
Mon Oct 25 15:29:23 UTC 2021


Summary of changes:
 lib/config/config.default.yml                      | 17 ++++++---
 lib/config/generated_config.go                     | 17 ++++++---
 lib/lsf/dispatch.go                                | 41 ++++++++++++++--------
 lib/lsf/dispatch_test.go                           |  9 +++--
 .../api/lib/tasks/manage_long_lived_tokens.rake    |  4 +--
 services/api/script/get_anonymous_user_token.rb    |  3 ++
 services/login-sync/bin/arvados-login-sync         | 37 +++++++++++++++++--
 7 files changed, 96 insertions(+), 32 deletions(-)

       via  067a68b5e9dfa1c7d5e68fd64553e0ced89cad36 (commit)
       via  d415db42e227d2f309d942486b7d2fcb431da628 (commit)
       via  308c90af198f5dd6b25ac284fe24aa8e648bc6d8 (commit)
       via  0b8994f341459e4e6f3ed7cfb9e38109529d632e (commit)
       via  2e921a511f4c5fb93f5bd1299b7a66b830440a8e (commit)
       via  1b95927d6b17cfa2a4c8a8f20bee7dafa59e3d34 (commit)
      from  1e008042ac7a5b7dfe4a11a8f33f71c57ee2666a (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 067a68b5e9dfa1c7d5e68fd64553e0ced89cad36
Author: Ward Vandewege <ward at jhvc.com>
Date:   Mon Oct 25 10:15:21 2021 -0400

    18290: address review comments.
    
    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 8b51a85d9..52e35d83f 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -1026,7 +1026,7 @@ Clusters:
         # Template variables starting with % will be substituted as follows:
         #
         # %U uuid
-        # %C number of cpus
+        # %C number of VCPUs
         # %M memory in MB
         # %T tmp in MB
         #
diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index 519d1a8e5..c58bbe178 100644
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -1032,7 +1032,7 @@ Clusters:
         # Template variables starting with % will be substituted as follows:
         #
         # %U uuid
-        # %C number of cpus
+        # %C number of VCPUs
         # %M memory in MB
         # %T tmp in MB
         #
diff --git a/lib/lsf/dispatch.go b/lib/lsf/dispatch.go
index d17c458e8..6e35b7de9 100644
--- a/lib/lsf/dispatch.go
+++ b/lib/lsf/dispatch.go
@@ -270,9 +270,7 @@ func (disp *dispatcher) bkill(ctr arvados.Container) {
 }
 
 func (disp *dispatcher) bsubArgs(container arvados.Container) ([]string, error) {
-	tmpArgs := []string{}
 	args := []string{"bsub"}
-	tmpArgs = append(tmpArgs, disp.Cluster.Containers.LSF.BsubArgumentsList...)
 
 	tmp := int64(math.Ceil(float64(dispatchcloud.EstimateScratchSpace(&container)) / 1048576))
 	vcpus := container.RuntimeConstraints.VCPUs
@@ -280,16 +278,27 @@ func (disp *dispatcher) bsubArgs(container arvados.Container) ([]string, error)
 		container.RuntimeConstraints.KeepCacheRAM+
 		int64(disp.Cluster.Containers.ReserveExtraRAM)) / 1048576))
 
-	r := regexp.MustCompile(`([^%]|^)%([^%])`)
-	undoubleRE := regexp.MustCompile(`%%`)
-	for _, a := range tmpArgs {
-		tmp := r.ReplaceAllStringFunc(a, func(m string) string {
-			parts := r.FindStringSubmatch(m)
-			return parts[1] + disp.substitute(parts[2], container.UUID, vcpus, mem, tmp)
-		})
-		// handle escaped literal % symbols
-		tmp = undoubleRE.ReplaceAllString(tmp, "%")
-		args = append(args, tmp)
+	repl := map[string]string{
+		"%%": "%",
+		"%C": fmt.Sprintf("%d", vcpus),
+		"%M": fmt.Sprintf("%d", mem),
+		"%T": fmt.Sprintf("%d", tmp),
+		"%U": container.UUID,
+	}
+
+	re := regexp.MustCompile(`%.`)
+	var substitutionErrors string
+	for _, a := range disp.Cluster.Containers.LSF.BsubArgumentsList {
+		args = append(args, re.ReplaceAllStringFunc(a, func(s string) string {
+			subst := repl[s]
+			if len(subst) == 0 {
+				substitutionErrors += fmt.Sprintf("Unknown substitution parameter %s in BsubArgumentsList, ", s)
+			}
+			return subst
+		}))
+	}
+	if len(substitutionErrors) != 0 {
+		return nil, fmt.Errorf("%s", substitutionErrors[:len(substitutionErrors)-2])
 	}
 
 	if u := disp.Cluster.Containers.LSF.BsubSudoUser; u != "" {
@@ -298,23 +307,6 @@ func (disp *dispatcher) bsubArgs(container arvados.Container) ([]string, error)
 	return args, nil
 }
 
-func (disp *dispatcher) substitute(l string, uuid string, vcpus int, mem, tmp int64) string {
-	var arg string
-	switch l {
-	case "C":
-		arg = fmt.Sprintf("%d", vcpus)
-	case "T":
-		arg = fmt.Sprintf("%d", tmp)
-	case "M":
-		arg = fmt.Sprintf("%d", mem)
-	case "U":
-		arg = uuid
-	default:
-		arg = "%" + l
-	}
-	return arg
-}
-
 // Check the next bjobs report, and invoke TrackContainer for all the
 // containers in the report. This gives us a chance to cancel existing
 // Arvados LSF jobs (started by a previous dispatch process) that

commit d415db42e227d2f309d942486b7d2fcb431da628
Author: Ward Vandewege <ward at curii.com>
Date:   Sat Oct 23 11:42:14 2021 -0400

    18290: LSF: make the bsub arguments completely configurable.
    
    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 4e2a0e26d..8b51a85d9 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -1021,14 +1021,23 @@ Clusters:
           AssignNodeHostname: "compute%<slot_number>d"
 
       LSF:
-        # Additional arguments to bsub when submitting Arvados
-        # containers as LSF jobs.
+        # Arguments to bsub when submitting Arvados containers as LSF jobs.
+        #
+        # Template variables starting with % will be substituted as follows:
+        #
+        # %U uuid
+        # %C number of cpus
+        # %M memory in MB
+        # %T tmp in MB
+        #
+        # Use %% to express a literal %. The %%J in the default will be changed
+        # to %J, which is interpreted by bsub itself.
         #
         # Note that the default arguments cause LSF to write two files
         # in /tmp on the compute node each time an Arvados container
         # runs. Ensure you have something in place to delete old files
-        # from /tmp, or adjust these arguments accordingly.
-        BsubArgumentsList: ["-o", "/tmp/crunch-run.%J.out", "-e", "/tmp/crunch-run.%J.err"]
+        # from /tmp, or adjust the "-o" and "-e" arguments accordingly.
+        BsubArgumentsList: ["-o", "/tmp/crunch-run.%%J.out", "-e", "/tmp/crunch-run.%%J.err", "-J", "%U", "-n", "%C", "-D", "%MMB", "-R", "rusage[mem=%MMB:tmp=%TMB] span[hosts=1]"]
 
         # Use sudo to switch to this user account when submitting LSF
         # jobs.
diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index 875939a3e..519d1a8e5 100644
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -1027,14 +1027,23 @@ Clusters:
           AssignNodeHostname: "compute%<slot_number>d"
 
       LSF:
-        # Additional arguments to bsub when submitting Arvados
-        # containers as LSF jobs.
+        # Arguments to bsub when submitting Arvados containers as LSF jobs.
+        #
+        # Template variables starting with % will be substituted as follows:
+        #
+        # %U uuid
+        # %C number of cpus
+        # %M memory in MB
+        # %T tmp in MB
+        #
+        # Use %% to express a literal %. The %%J in the default will be changed
+        # to %J, which is interpreted by bsub itself.
         #
         # Note that the default arguments cause LSF to write two files
         # in /tmp on the compute node each time an Arvados container
         # runs. Ensure you have something in place to delete old files
-        # from /tmp, or adjust these arguments accordingly.
-        BsubArgumentsList: ["-o", "/tmp/crunch-run.%J.out", "-e", "/tmp/crunch-run.%J.err"]
+        # from /tmp, or adjust the "-o" and "-e" arguments accordingly.
+        BsubArgumentsList: ["-o", "/tmp/crunch-run.%%J.out", "-e", "/tmp/crunch-run.%%J.err", "-J", "%U", "-n", "%C", "-D", "%MMB", "-R", "rusage[mem=%MMB:tmp=%TMB] span[hosts=1]"]
 
         # Use sudo to switch to this user account when submitting LSF
         # jobs.
diff --git a/lib/lsf/dispatch.go b/lib/lsf/dispatch.go
index d3ba605ab..d17c458e8 100644
--- a/lib/lsf/dispatch.go
+++ b/lib/lsf/dispatch.go
@@ -270,28 +270,49 @@ func (disp *dispatcher) bkill(ctr arvados.Container) {
 }
 
 func (disp *dispatcher) bsubArgs(container arvados.Container) ([]string, error) {
+	tmpArgs := []string{}
 	args := []string{"bsub"}
-	args = append(args, disp.Cluster.Containers.LSF.BsubArgumentsList...)
-	args = append(args, "-J", container.UUID)
-	args = append(args, disp.bsubConstraintArgs(container)...)
-	if u := disp.Cluster.Containers.LSF.BsubSudoUser; u != "" {
-		args = append([]string{"sudo", "-E", "-u", u}, args...)
-	}
-	return args, nil
-}
+	tmpArgs = append(tmpArgs, disp.Cluster.Containers.LSF.BsubArgumentsList...)
 
-func (disp *dispatcher) bsubConstraintArgs(container arvados.Container) []string {
-	// TODO: propagate container.SchedulingParameters.Partitions
 	tmp := int64(math.Ceil(float64(dispatchcloud.EstimateScratchSpace(&container)) / 1048576))
 	vcpus := container.RuntimeConstraints.VCPUs
 	mem := int64(math.Ceil(float64(container.RuntimeConstraints.RAM+
 		container.RuntimeConstraints.KeepCacheRAM+
 		int64(disp.Cluster.Containers.ReserveExtraRAM)) / 1048576))
-	return []string{
-		"-n", fmt.Sprintf("%d", vcpus),
-		"-D", fmt.Sprintf("%dMB", mem), // ulimit -d (note this doesn't limit the total container memory usage)
-		"-R", fmt.Sprintf("rusage[mem=%dMB:tmp=%dMB] span[hosts=1]", mem, tmp),
+
+	r := regexp.MustCompile(`([^%]|^)%([^%])`)
+	undoubleRE := regexp.MustCompile(`%%`)
+	for _, a := range tmpArgs {
+		tmp := r.ReplaceAllStringFunc(a, func(m string) string {
+			parts := r.FindStringSubmatch(m)
+			return parts[1] + disp.substitute(parts[2], container.UUID, vcpus, mem, tmp)
+		})
+		// handle escaped literal % symbols
+		tmp = undoubleRE.ReplaceAllString(tmp, "%")
+		args = append(args, tmp)
+	}
+
+	if u := disp.Cluster.Containers.LSF.BsubSudoUser; u != "" {
+		args = append([]string{"sudo", "-E", "-u", u}, args...)
+	}
+	return args, nil
+}
+
+func (disp *dispatcher) substitute(l string, uuid string, vcpus int, mem, tmp int64) string {
+	var arg string
+	switch l {
+	case "C":
+		arg = fmt.Sprintf("%d", vcpus)
+	case "T":
+		arg = fmt.Sprintf("%d", tmp)
+	case "M":
+		arg = fmt.Sprintf("%d", mem)
+	case "U":
+		arg = uuid
+	default:
+		arg = "%" + l
 	}
+	return arg
 }
 
 // Check the next bjobs report, and invoke TrackContainer for all the
diff --git a/lib/lsf/dispatch_test.go b/lib/lsf/dispatch_test.go
index 44a1a3d8c..641453e54 100644
--- a/lib/lsf/dispatch_test.go
+++ b/lib/lsf/dispatch_test.go
@@ -72,11 +72,10 @@ func (stub lsfstub) stubCommand(s *suite, c *check.C) func(prog string, args ...
 		switch prog {
 		case "bsub":
 			defaultArgs := s.disp.Cluster.Containers.LSF.BsubArgumentsList
-			c.Assert(len(args) > len(defaultArgs), check.Equals, true)
-			c.Check(args[:len(defaultArgs)], check.DeepEquals, defaultArgs)
-			args = args[len(defaultArgs):]
-
-			c.Check(args[0], check.Equals, "-J")
+			c.Assert(len(args), check.Equals, len(defaultArgs))
+			// %%J must have been rewritten to %J
+			c.Check(args[1], check.Equals, "/tmp/crunch-run.%J.out")
+			args = args[4:]
 			switch args[1] {
 			case arvadostest.LockedContainerUUID:
 				c.Check(args, check.DeepEquals, []string{

commit 308c90af198f5dd6b25ac284fe24aa8e648bc6d8
Author: Ward Vandewege <ward at curii.com>
Date:   Thu Oct 21 14:19:42 2021 -0400

    18281: Rephrase the help text for --force-rotate option.
    
    Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward at curii.com>

diff --git a/services/login-sync/bin/arvados-login-sync b/services/login-sync/bin/arvados-login-sync
index 92b9f9a2c..da8a21efa 100755
--- a/services/login-sync/bin/arvados-login-sync
+++ b/services/login-sync/bin/arvados-login-sync
@@ -21,7 +21,7 @@ end
 options = {}
 OptionParser.new do |parser|
   parser.on('--exclusive', 'Manage SSH keys file exclusively.')
-  parser.on('--rotate-tokens', 'Always create new user tokens. Usually needed with --token-lifetime.')
+  parser.on('--rotate-tokens', 'Force a rotation of all user tokens.')
   parser.on('--skip-missing-users', "Don't try to create any local accounts.")
   parser.on('--token-lifetime SECONDS', 'Create user tokens that expire after SECONDS.', Integer)
   parser.on('--debug', 'Enable debug output')

commit 0b8994f341459e4e6f3ed7cfb9e38109529d632e
Author: Ward Vandewege <ward at curii.com>
Date:   Wed Oct 20 16:28:30 2021 -0400

    18281: make arvados-login-sync smart enough to replace expired tokens.
           Also add a --debug parameter and some debug output.
    
    Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward at curii.com>

diff --git a/services/login-sync/bin/arvados-login-sync b/services/login-sync/bin/arvados-login-sync
index 8e5c6deb5..92b9f9a2c 100755
--- a/services/login-sync/bin/arvados-login-sync
+++ b/services/login-sync/bin/arvados-login-sync
@@ -24,6 +24,7 @@ OptionParser.new do |parser|
   parser.on('--rotate-tokens', 'Always create new user tokens. Usually needed with --token-lifetime.')
   parser.on('--skip-missing-users', "Don't try to create any local accounts.")
   parser.on('--token-lifetime SECONDS', 'Create user tokens that expire after SECONDS.', Integer)
+  parser.on('--debug', 'Enable debug output')
 end.parse!(into: options)
 
 exclusive_banner = "#######################################################################################
@@ -35,6 +36,10 @@ end_banner = "### END Arvados-managed keys -- changes between markers will be ov
 keys = ''
 
 begin
+  debug = false
+  if options[:"debug"]
+    debug = true
+  end
   arv = Arvados.new({ :suppress_ssl_warnings => false })
   logincluster_arv = Arvados.new({ :api_host => (ENV['LOGINCLUSTER_ARVADOS_API_HOST'] || ENV['ARVADOS_API_HOST']),
                                    :api_token => (ENV['LOGINCLUSTER_ARVADOS_API_TOKEN'] || ENV['ARVADOS_API_TOKEN']),
@@ -75,7 +80,7 @@ begin
         end
       else
         if pwnam[l[:username]].uid < uid_min
-          STDERR.puts "Account #{l[:username]} uid #{pwnam[l[:username]].uid} < uid_min #{uid_min}. Skipping"
+          STDERR.puts "Account #{l[:username]} uid #{pwnam[l[:username]].uid} < uid_min #{uid_min}. Skipping" if debug
           true
         end
       end
@@ -85,6 +90,7 @@ begin
 
   # Collect all keys
   logins.each do |l|
+    STDERR.puts("Considering #{l[:username]} ...") if debug
     keys[l[:username]] = Array.new() if not keys.has_key?(l[:username])
     key = l[:public_key]
     if !key.nil?
@@ -197,7 +203,32 @@ begin
     tokenfile = File.join(configarvados, "settings.conf")
 
     begin
-      if !File.exist?(tokenfile) || options[:"rotate-tokens"]
+      STDERR.puts "Processing #{tokenfile} ..." if debug
+      newToken = false
+      if File.exist?(tokenfile)
+        # check if the token is still valid
+        myToken = ENV["ARVADOS_API_TOKEN"]
+        userEnv = IO::read(tokenfile)
+        if (m = /^ARVADOS_API_TOKEN=(.*?\n)/m.match(userEnv))
+          begin
+            tmp_arv = Arvados.new({ :api_host => (ENV['LOGINCLUSTER_ARVADOS_API_HOST'] || ENV['ARVADOS_API_HOST']),
+                                   :api_token => (m[1]),
+                      :suppress_ssl_warnings => false })
+            tmp_arv.user.current
+          rescue Arvados::TransactionFailedError => e
+            if e.to_s =~ /401 Unauthorized/
+              STDERR.puts "Account #{l[:username]} token not valid, creating new token."
+              newToken = true
+            else
+              raise
+            end
+          end
+        end
+      elsif !File.exist?(tokenfile) || options[:"rotate-tokens"]
+        STDERR.puts "Account #{l[:username]} token file not found, creating new token."
+        newToken = true
+      end
+      if newToken
         aca_params = {owner_uuid: l[:user_uuid], api_client_id: 0}
         if options[:"token-lifetime"] && options[:"token-lifetime"] > 0
           aca_params.merge!(expires_at: (Time.now + options[:"token-lifetime"]))

commit 2e921a511f4c5fb93f5bd1299b7a66b830440a8e
Author: Ward Vandewege <ward at curii.com>
Date:   Thu Oct 21 16:27:59 2021 -0400

    18288: when storing the anonymous user token, make sure to clear the
           expires_at field.
    
    Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward at curii.com>

diff --git a/services/api/script/get_anonymous_user_token.rb b/services/api/script/get_anonymous_user_token.rb
index 8775ae595..4c3ca34f0 100755
--- a/services/api/script/get_anonymous_user_token.rb
+++ b/services/api/script/get_anonymous_user_token.rb
@@ -58,6 +58,9 @@ def create_api_client_auth(supplied_token=nil)
 
   api_client_auth = ApiClientAuthorization.where(attr).first
   if !api_client_auth
+    # The anonymous user token should never expire but we are not allowed to
+    # set :expires_at to nil, so we set it to 1000 years in the future.
+    attr[:expires_at] = Time.now + 1000.years
     api_client_auth = ApiClientAuthorization.create!(attr)
   end
   api_client_auth

commit 1b95927d6b17cfa2a4c8a8f20bee7dafa59e3d34
Author: Ward Vandewege <ward at curii.com>
Date:   Thu Oct 21 16:16:04 2021 -0400

    18288: when running the db:check_long_lived_tokens and
           db:fix_long_lived_tokens rake tasks, do not expire the
           anonymouspublic token.
    
    Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <ward at curii.com>

diff --git a/services/api/lib/tasks/manage_long_lived_tokens.rake b/services/api/lib/tasks/manage_long_lived_tokens.rake
index d83c2b603..7a665ff7e 100644
--- a/services/api/lib/tasks/manage_long_lived_tokens.rake
+++ b/services/api/lib/tasks/manage_long_lived_tokens.rake
@@ -29,7 +29,7 @@ namespace :db do
         # skip this token
         next
       end
-      if (auth.user.uuid =~ /-tpzed-000000000000000/).nil?
+      if (auth.user.uuid =~ /-tpzed-000000000000000/).nil? and (auth.user.uuid =~ /-tpzed-anonymouspublic/).nil?
         CurrentApiClientHelper.act_as_system_user do
           auth.update_attributes!(expires_at: exp_date)
         end
@@ -58,7 +58,7 @@ namespace :db do
         # skip this token
         next
       end
-      if not auth.user.nil? and (auth.user.uuid =~ /-tpzed-000000000000000/).nil?
+      if not auth.user.nil? and (auth.user.uuid =~ /-tpzed-000000000000000/).nil? and (auth.user.uuid =~ /-tpzed-anonymouspublic/).nil?
         user_ids.add(auth.user_id)
         token_count += 1
       end

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list