[ARVADOS] updated: 1.1.3-167-g0460fa8

Git user git at public.curoverse.com
Fri Mar 9 13:57:37 EST 2018


Summary of changes:
 build/build.list                                   |  3 +-
 build/run-build-packages.sh                        | 19 +------
 build/run-tests.sh                                 | 10 ++--
 doc/install/migrate-docker19.html.textile.liquid   |  2 +-
 lib/dispatchcloud/node_size.go                     | 43 +++++++--------
 sdk/cwl/arvados_cwl/__init__.py                    | 25 ++++++---
 sdk/cwl/arvados_cwl/arvcontainer.py                |  4 +-
 sdk/cwl/arvados_cwl/runner.py                      |  3 +-
 sdk/cwl/tests/test_container.py                    | 20 +++----
 sdk/cwl/tests/test_submit.py                       | 26 +++++++--
 sdk/python/arvados/__init__.py                     |  6 +--
 sdk/python/arvados/commands/migrate19.py           |  2 +-
 sdk/python/arvados/commands/put.py                 | 24 ++++++++-
 sdk/python/tests/test_arv_put.py                   | 63 ++++++++++++++++++----
 .../crunch-dispatch-slurm_test.go                  | 18 ++++---
 services/crunch-dispatch-slurm/slurm.go            |  9 +++-
 services/crunch-dispatch-slurm/squeue.go           | 32 +++++++++--
 services/crunch-dispatch-slurm/squeue_test.go      | 59 ++++++++++++++++++--
 services/nodemanager/arvnodeman/jobqueue.py        |  4 +-
 19 files changed, 266 insertions(+), 106 deletions(-)

  discards  2fa20d60cae47148250cb58b44f520b2cb866644 (commit)
       via  0460fa8b94a6499aff1b816625d65101834b7a97 (commit)
       via  6d1ebf894a02151f751686003dc67ed4788d6c10 (commit)
       via  98c6c6990061c546b9995ad70766589499fb4844 (commit)
       via  e8a18e3ebbcaf24812673f67e14edac7f8c28e41 (commit)
       via  4ad88f00a73da6b4cbe11e2bde8d7a055ba6b60e (commit)
       via  19ee2e029443d4d6de3df4a4e8bcb6ae9700475a (commit)
       via  fdafe3f18cda89136d84a5127309047ad3fab584 (commit)
       via  1a3fb9d7f272d18a974b3a180be7940f5a161030 (commit)
       via  92474d902612ad28b3f9e5b5c21de08162d23ffb (commit)
       via  dd6db47bbbbafd95b7026039057064662de82dac (commit)
       via  8d65322676bee8adc01ec1f0fc3f1339b0441535 (commit)
       via  bab7938e562be6ce1305c15faaffb43aef67ff77 (commit)
       via  55573affe3611054b88a17d3cc89c21b8bfa14e7 (commit)
       via  e1f97dcf68d197525228781e2a861cc3e64a0231 (commit)
       via  f7d0830ae819e2a62115642be449fa79f2fc8152 (commit)
       via  f56ca98bda57d61b6b9b26b5fa1e845f8d567e5b (commit)
       via  53dc92af621bb2064e922b82e88572f5180c503a (commit)
       via  a67b550e12bcf12a8f67ef1472b2dce013747712 (commit)
       via  96f657210ce401a84eafc4a575f56694f2b39a0b (commit)
       via  c04e0a81076b9e0c71bdd4f9b07b7ececf4d2685 (commit)
       via  d4e992482026d418900b066337dfd6273175b66e (commit)
       via  409f48a7689b948a96959ea5c7ec508f7b713912 (commit)
       via  a58a26365472453863010f349d87f9d4fc214daa (commit)
       via  cb191006b7daf0a36f46349ebb9994cd30261a07 (commit)
       via  6906707ffcbf0f063ab2a802b567f1f91a2fe84a (commit)
       via  298fa8a8436596348086b42c8d31eba22609145a (commit)
       via  f03f97c8d629667d460a95c746de4cdb7092a740 (commit)
       via  c6e1f628cc07731cd5fdee991fbd49564dab44b7 (commit)
       via  8682bb87e37e3ef9831dc83167a6620594fc8b52 (commit)

This update added new revisions after undoing existing revisions.  That is
to say, the old revision is not a strict subset of the new revision.  This
situation occurs when you --force push a change and generate a repository
containing something like this:

 * -- * -- B -- O -- O -- O (2fa20d60cae47148250cb58b44f520b2cb866644)
            \
             N -- N -- N (0460fa8b94a6499aff1b816625d65101834b7a97)

When this happens we assume that you've already had alert emails for all
of the O revisions, and so we here report only the revisions in the N
branch from the common base, B.

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 0460fa8b94a6499aff1b816625d65101834b7a97
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Thu Mar 1 09:21:12 2018 -0500

    13143: Add secret_mounts field to containers and container requests.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/services/api/app/controllers/arvados/v1/containers_controller.rb b/services/api/app/controllers/arvados/v1/containers_controller.rb
index 8c63ea7..6ec92b0 100644
--- a/services/api/app/controllers/arvados/v1/containers_controller.rb
+++ b/services/api/app/controllers/arvados/v1/containers_controller.rb
@@ -60,4 +60,14 @@ class Arvados::V1::ContainersController < ApplicationController
       end
     end
   end
+
+  def secret_mounts
+    if @object &&
+       @object.auth_uuid &&
+       @object.auth_uuid == Thread.current[:api_client_authorization].uuid
+      send_json({"secret_mounts" => @object.secret_mounts})
+    else
+      send_error("Token is not associated with this container.", status: 403)
+    end
+  end
 end
diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index b013776..a92dad7 100644
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -20,6 +20,7 @@ class Container < ArvadosModel
   serialize :runtime_constraints, Hash
   serialize :command, Array
   serialize :scheduling_parameters, Hash
+  serialize :secret_mounts, Hash
 
   before_validation :fill_field_defaults, :if => :new_record?
   before_validation :set_timestamps
@@ -121,6 +122,7 @@ class Container < ArvadosModel
       mounts: resolve_mounts(req.mounts),
       runtime_constraints: resolve_runtime_constraints(req.runtime_constraints),
       scheduling_parameters: req.scheduling_parameters,
+      secret_mounts: req.secret_mounts,
     }
     act_as_system_user do
       if req.use_existing && (reusable = find_reusable(c_attrs))
@@ -378,7 +380,8 @@ class Container < ArvadosModel
     if self.new_record?
       permitted.push(:owner_uuid, :command, :container_image, :cwd,
                      :environment, :mounts, :output_path, :priority,
-                     :runtime_constraints, :scheduling_parameters)
+                     :runtime_constraints, :scheduling_parameters,
+                     :secret_mounts)
     end
 
     case self.state
diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb
index bcca407..e1253c7 100644
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -18,6 +18,7 @@ class ContainerRequest < ArvadosModel
   serialize :runtime_constraints, Hash
   serialize :command, Array
   serialize :scheduling_parameters, Hash
+  serialize :secret_mounts, Hash
 
   before_validation :fill_field_defaults, :if => :new_record?
   before_validation :validate_runtime_constraints
@@ -79,7 +80,7 @@ class ContainerRequest < ArvadosModel
   :container_image, :cwd, :environment, :filters, :mounts,
   :output_path, :priority, :properties, :requesting_container_uuid,
   :runtime_constraints, :state, :container_uuid, :use_existing,
-  :scheduling_parameters, :output_name, :output_ttl]
+  :scheduling_parameters, :secret_mounts, :output_name, :output_ttl]
 
   def self.limit_index_columns_read
     ["mounts"]
@@ -216,7 +217,7 @@ class ContainerRequest < ArvadosModel
 
     if self.new_record? || self.state_was == Uncommitted
       # Allow create-and-commit in a single operation.
-      permitted.push *AttrsPermittedBeforeCommit
+      permitted.push(*AttrsPermittedBeforeCommit)
     end
 
     case self.state
diff --git a/services/api/config/initializers/lograge.rb b/services/api/config/initializers/lograge.rb
index 564f31a..db9b225 100644
--- a/services/api/config/initializers/lograge.rb
+++ b/services/api/config/initializers/lograge.rb
@@ -15,6 +15,18 @@ Server::Application.configure do
     }
     exceptions = %w(controller action format id)
     params = event.payload[:params].except(*exceptions)
+
+    # Omit secret_mounts field if supplied in create/update request
+    # body.
+    [
+      ['container', 'secret_mounts'],
+      ['container_request', 'secret_mounts'],
+    ].each do |resource, field|
+      if params[resource].is_a? Hash
+        params[resource] = params[resource].except(field)
+      end
+    end
+
     params_s = SafeJSON.dump(params)
     if params_s.length > Rails.configuration.max_request_log_params_size
       payload[:params_truncated] = params_s[0..Rails.configuration.max_request_log_params_size] + "[...]"
diff --git a/services/api/config/routes.rb b/services/api/config/routes.rb
index fcd5c34..ad2406a 100644
--- a/services/api/config/routes.rb
+++ b/services/api/config/routes.rb
@@ -39,6 +39,7 @@ Server::Application.routes.draw do
         get 'auth', on: :member
         post 'lock', on: :member
         post 'unlock', on: :member
+        get 'secret_mounts', on: :member
         get 'current', on: :collection
       end
       resources :container_requests
diff --git a/services/api/db/migrate/20180228220311_add_secret_mounts_to_containers.rb b/services/api/db/migrate/20180228220311_add_secret_mounts_to_containers.rb
new file mode 100644
index 0000000..84a77b8
--- /dev/null
+++ b/services/api/db/migrate/20180228220311_add_secret_mounts_to_containers.rb
@@ -0,0 +1,7 @@
+class AddSecretMountsToContainers < ActiveRecord::Migration
+  def change
+    add_column :container_requests, :secret_mounts, :jsonb, default: {}
+    add_column :containers, :secret_mounts, :jsonb, default: {}
+    add_column :containers, :secret_mounts_md5, :string, default: "99914b932bd37a50b983c5e7c90ae93b"
+  end
+end
diff --git a/services/api/db/structure.sql b/services/api/db/structure.sql
index 357e95c..f02ccbf 100644
--- a/services/api/db/structure.sql
+++ b/services/api/db/structure.sql
@@ -299,7 +299,8 @@ CREATE TABLE container_requests (
     output_uuid character varying(255),
     log_uuid character varying(255),
     output_name character varying(255) DEFAULT NULL::character varying,
-    output_ttl integer DEFAULT 0 NOT NULL
+    output_ttl integer DEFAULT 0 NOT NULL,
+    secret_mounts jsonb DEFAULT '{}'::jsonb
 );
 
 
@@ -352,7 +353,9 @@ CREATE TABLE containers (
     exit_code integer,
     auth_uuid character varying(255),
     locked_by_uuid character varying(255),
-    scheduling_parameters text
+    scheduling_parameters text,
+    secret_mounts jsonb DEFAULT '{}'::jsonb,
+    secret_mounts_md5 character varying DEFAULT '99914b932bd37a50b983c5e7c90ae93b'::character varying
 );
 
 
@@ -3054,3 +3057,5 @@ INSERT INTO schema_migrations (version) VALUES ('20171212153352');
 
 INSERT INTO schema_migrations (version) VALUES ('20180216203422');
 
+INSERT INTO schema_migrations (version) VALUES ('20180228220311');
+
diff --git a/services/api/test/fixtures/containers.yml b/services/api/test/fixtures/containers.yml
index 3995791..47b5471 100644
--- a/services/api/test/fixtures/containers.yml
+++ b/services/api/test/fixtures/containers.yml
@@ -39,6 +39,10 @@ running:
   runtime_constraints:
     ram: 12000000000
     vcpus: 4
+  secret_mounts:
+    /secret/6x9:
+      kind: text
+      content: "42\n"
   auth_uuid: zzzzz-gj3su-077z32aux8dg2s2
 
 running_older:
diff --git a/services/api/test/functional/arvados/v1/container_requests_controller_test.rb b/services/api/test/functional/arvados/v1/container_requests_controller_test.rb
index 4ebda97..5423e84 100644
--- a/services/api/test/functional/arvados/v1/container_requests_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/container_requests_controller_test.rb
@@ -23,4 +23,19 @@ class Arvados::V1::ContainerRequestsControllerTest < ActionController::TestCase
     assert_not_nil cr, 'Expected container request'
     assert_equal sp, cr['scheduling_parameters']
   end
+
+  test "secret_mounts not in container API responses" do
+  end
+
+  test "secret_mounts not in container request API responses" do
+  end
+
+  test "create with secret_mounts" do
+  end
+
+  test "update with secret_mounts" do
+  end
+
+  test "update without deleting secret_mounts" do
+  end
 end
diff --git a/services/api/test/functional/arvados/v1/containers_controller_test.rb b/services/api/test/functional/arvados/v1/containers_controller_test.rb
index 5510e90..567e118 100644
--- a/services/api/test/functional/arvados/v1/containers_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/containers_controller_test.rb
@@ -106,7 +106,7 @@ class Arvados::V1::ContainersControllerTest < ActionController::TestCase
     [:running, :lock, 422, 'Running'],
     [:running, :unlock, 422, 'Running'],
   ].each do |fixture, action, response, state|
-    test "state transitions from #{fixture } to #{action}" do
+    test "state transitions from #{fixture} to #{action}" do
       authorize_with :dispatch1
       uuid = containers(fixture).uuid
       post action, {id: uuid}
@@ -133,4 +133,21 @@ class Arvados::V1::ContainersControllerTest < ActionController::TestCase
     assert_response 401
   end
 
+  [
+    [true, :running_container_auth],
+    [false, :dispatch2],
+    [false, :admin],
+    [false, :active],
+  ].each do |expect_success, auth|
+    test "get secret_mounts with #{auth} token" do
+      authorize_with auth
+      get :secret_mounts, {id: containers(:running).uuid}
+      if expect_success
+        assert_response :success
+        assert_equal "42\n", json_response["secret_mounts"]["/secret/6x9"]["content"]
+      else
+        assert_response 403
+      end
+    end
+  end
 end
diff --git a/services/api/test/helpers/container_test_helper.rb b/services/api/test/helpers/container_test_helper.rb
new file mode 100644
index 0000000..15f0a08
--- /dev/null
+++ b/services/api/test/helpers/container_test_helper.rb
@@ -0,0 +1,11 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+module ContainerTestHelper
+  def assert_no_secrets_logged
+    Log.all.map(&:properties).each do |props|
+      refute_match /secret\/6x9/, SafeJSON.dump(props)
+    end
+  end
+end
diff --git a/services/api/test/unit/container_request_test.rb b/services/api/test/unit/container_request_test.rb
index 0edc0f4..93be0b5 100644
--- a/services/api/test/unit/container_request_test.rb
+++ b/services/api/test/unit/container_request_test.rb
@@ -3,11 +3,13 @@
 # SPDX-License-Identifier: AGPL-3.0
 
 require 'test_helper'
+require 'helpers/container_test_helper'
 require 'helpers/docker_migration_helper'
 
 class ContainerRequestTest < ActiveSupport::TestCase
   include DockerMigrationHelper
   include DbCurrentTime
+  include ContainerTestHelper
 
   def create_minimal_req! attrs={}
     defaults = {
@@ -836,4 +838,58 @@ class ContainerRequestTest < ActiveSupport::TestCase
     end
   end
 
+  test "reuse container with same secret_mounts" do
+    set_user_from_auth :active
+    cr1 = create_minimal_req!(state: "Committed", priority: 1)
+    cr1.save!
+    cr2 = create_minimal_req!(state: "Committed", priority: 1, secret_mounts: {})
+    cr2.save!
+    assert_not_nil cr1.container_uuid
+    assert_equal cr1.container_uuid, cr2.container_uuid
+  end
+
+  secrets = {"/foo" => {"kind" => "binary"}}
+  [
+    [true, nil, nil],
+    [true, nil, {}],
+    [true, {}, {}],
+    [true, secrets, secrets],
+    [false, nil, secrets],
+    [false, {}, secrets],
+  ].each do |expect_reuse, sm1, sm2|
+    test "container reuse secret_mounts #{sm1.inspect}, #{sm2.inspect}" do
+      set_user_from_auth :active
+      cr1 = create_minimal_req!(state: "Committed", priority: 1, secret_mounts: sm1)
+      cr1.save!
+      cr2 = create_minimal_req!(state: "Committed", priority: 1, secret_mounts: sm2)
+      cr2.save!
+      assert_not_nil cr1.container_uuid
+      assert_not_nil cr2.container_uuid
+      if expect_reuse
+        assert_equal cr1.container_uuid, cr2.container_uuid
+      else
+        assert_not_equal cr1.container_uuid, cr2.container_uuid
+      end
+    end
+  end
+
+  test "secret_mounts is null after container request is final" do
+    fail # todo: submit with secrets and finalize
+    assert_no_secrets_logged
+  end
+
+  test "reuse container with identical secret_mounts, even if scrubbed" do
+    fail # todo: run, ensure scrubbed, run identical
+  end
+
+  test "not reuse container with different secret_mounts" do
+    secrets = {"/foo" => {"kind" => "binary"}}
+    set_user_from_auth :active
+    cr1 = create_minimal_req!(state: "Committed", priority: 1, secret_mounts: secrets.dup)
+    cr1.save!
+    cr2 = create_minimal_req!(state: "Committed", priority: 1, secret_mounts: secrets.dup)
+    cr2.save!
+    assert_not_nil cr1.container_uuid
+    assert_equal cr1.container_uuid, cr2.container_uuid
+  end
 end
diff --git a/services/api/test/unit/container_test.rb b/services/api/test/unit/container_test.rb
index 0e13ee9..bd07a88 100644
--- a/services/api/test/unit/container_test.rb
+++ b/services/api/test/unit/container_test.rb
@@ -3,9 +3,11 @@
 # SPDX-License-Identifier: AGPL-3.0
 
 require 'test_helper'
+require 'helpers/container_test_helper'
 
 class ContainerTest < ActiveSupport::TestCase
   include DbCurrentTime
+  include ContainerTestHelper
 
   DEFAULT_ATTRS = {
     command: ['echo', 'foo'],
@@ -621,4 +623,12 @@ class ContainerTest < ActiveSupport::TestCase
     end
   end
 
+  [
+    Container::Complete,
+    Container::Cancelled,
+  ].each do |final_state|
+    test "secret_mounts is null after container is #{final_state}" do
+      assert_no_secrets_logged
+    end
+  end
 end

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list