[ARVADOS] created: 1.3.0-6-g7c85f3289

Git user git at public.curoverse.com
Wed Dec 5 14:22:25 EST 2018


        at  7c85f328995e20060d9a55460f49eef413980f27 (commit)


commit 7c85f328995e20060d9a55460f49eef413980f27
Author: Peter Amstutz <pamstutz at veritasgenetics.com>
Date:   Wed Dec 5 14:18:54 2018 -0500

    14584: Remove runtime_user_uuid and runtime_auth_scopes checks for reuse.
    
    Also fix Container.resolve to call find_reusable as the current user,
    not the system user, so the "is output collection readable" check is
    against the correct user.
    
    Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>

diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index ac67040ed..b5f9ba0c4 100644
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -179,10 +179,10 @@ class Container < ArvadosModel
         runtime_auth_scopes: runtime_auth_scopes
       }
     end
-    act_as_system_user do
-      if req.use_existing && (reusable = find_reusable(c_attrs))
-        reusable
-      else
+    if req.use_existing && (reusable = find_reusable(c_attrs))
+      reusable
+    else
+      act_as_system_user do
         Container.create!(c_attrs)
       end
     end
@@ -265,7 +265,7 @@ class Container < ArvadosModel
     candidates = candidates.where('output_path = ?', attrs[:output_path])
     log_reuse_info(candidates) { "after filtering on output_path #{attrs[:output_path].inspect}" }
 
-    image = resolve_container_image(attrs[:container_image])
+    image = attrs[:container_image]
     candidates = candidates.where('container_image = ?', image)
     log_reuse_info(candidates) { "after filtering on container_image #{image.inspect} (resolved from #{attrs[:container_image].inspect})" }
 
@@ -279,14 +279,6 @@ class Container < ArvadosModel
     candidates = candidates.where_serialized(:runtime_constraints, resolve_runtime_constraints(attrs[:runtime_constraints]), md5: true)
     log_reuse_info(candidates) { "after filtering on runtime_constraints #{attrs[:runtime_constraints].inspect}" }
 
-    candidates = candidates.where('runtime_user_uuid = ? or (runtime_user_uuid is NULL and runtime_auth_scopes is NULL)',
-                                  attrs[:runtime_user_uuid])
-    log_reuse_info(candidates) { "after filtering on runtime_user_uuid #{attrs[:runtime_user_uuid].inspect}" }
-
-    candidates = candidates.where('runtime_auth_scopes = ? or (runtime_user_uuid is NULL and runtime_auth_scopes is NULL)',
-                                  SafeJSON.dump(attrs[:runtime_auth_scopes].sort))
-    log_reuse_info(candidates) { "after filtering on runtime_auth_scopes #{attrs[:runtime_auth_scopes].inspect}" }
-
     log_reuse_info { "checking for state=Complete with readable output and log..." }
 
     select_readable_pdh = Collection.
diff --git a/services/api/test/unit/container_test.rb b/services/api/test/unit/container_test.rb
index 90b4f13bf..2a9ff5bf4 100644
--- a/services/api/test/unit/container_test.rb
+++ b/services/api/test/unit/container_test.rb
@@ -558,7 +558,8 @@ class ContainerTest < ActiveSupport::TestCase
     c1, _ = minimal_new(common_attrs.merge({runtime_token: api_client_authorizations(:active).token}))
     assert_equal Container::Queued, c1.state
     reused = Container.find_reusable(common_attrs.merge(runtime_token_attr(:container_runtime_token)))
-    assert_nil reused
+    # See #14584
+    assert_equal c1.uuid, reused.uuid
   end
 
   test "find_reusable method with nil runtime_token, then runtime_token with different user" do
@@ -567,7 +568,8 @@ class ContainerTest < ActiveSupport::TestCase
     c1, _ = minimal_new(common_attrs.merge({runtime_token: nil}))
     assert_equal Container::Queued, c1.state
     reused = Container.find_reusable(common_attrs.merge(runtime_token_attr(:container_runtime_token)))
-    assert_nil reused
+    # See #14584
+    assert_equal c1.uuid, reused.uuid
   end
 
   test "find_reusable method with different runtime_token, different scope, same user" do
@@ -576,7 +578,8 @@ class ContainerTest < ActiveSupport::TestCase
     c1, _ = minimal_new(common_attrs.merge({runtime_token: api_client_authorizations(:runtime_token_limited_scope).token}))
     assert_equal Container::Queued, c1.state
     reused = Container.find_reusable(common_attrs.merge(runtime_token_attr(:container_runtime_token)))
-    assert_nil reused
+    # See #14584
+    assert_equal c1.uuid, reused.uuid
   end
 
   test "Container running" do

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list