Merge branch 'master' into 8019-crunchrun-log-throttle
authorradhika <radhika@curoverse.com>
Fri, 5 May 2017 16:02:36 +0000 (12:02 -0400)
committerradhika <radhika@curoverse.com>
Fri, 5 May 2017 16:02:36 +0000 (12:02 -0400)
24 files changed:
apps/workbench/app/controllers/container_requests_controller.rb
apps/workbench/app/helpers/provenance_helper.rb
apps/workbench/app/models/container_work_unit.rb
apps/workbench/app/views/container_requests/_extra_tab_line_buttons.html.erb
apps/workbench/app/views/users/_show_activity.html.erb
apps/workbench/test/controllers/container_requests_controller_test.rb
apps/workbench/test/unit/work_unit_test.rb
doc/install/install-manual-prerequisites.html.textile.liquid
sdk/cwl/arvados_cwl/arvcontainer.py
sdk/cwl/tests/test_container.py
sdk/go/arvados/container.go
sdk/python/arvados/commands/put.py
sdk/python/tests/test_arv_put.py
services/api/config/application.rb
services/api/config/initializers/noop_deep_munge.rb [deleted file]
services/api/test/fixtures/container_requests.yml
services/api/test/fixtures/containers.yml
services/api/test/integration/noop_deep_munge_test.rb [moved from services/api/test/integration/noop_deep_munge.rb with 63% similarity]
services/crunch-dispatch-slurm/crunch-dispatch-slurm.go
services/crunch-dispatch-slurm/crunch-dispatch-slurm_test.go
services/crunch-run/crunchrun.go
services/crunch-run/crunchrun_test.go
services/nodemanager/arvnodeman/jobqueue.py
services/nodemanager/tests/test_jobqueue.py

index ef7665b34d9f46dac52ac0c4c88f84c4127164ea..fd29cd3f7088b5ab2ca4011d7d6d8b9ab26b6182 100644 (file)
@@ -87,7 +87,26 @@ class ContainerRequestsController < ApplicationController
 
     @object = ContainerRequest.new
 
-    @object.command = src.command
+    # By default the copied CR won't be reusing containers, unless use_existing=true
+    # param is passed.
+    command = src.command
+    if params[:use_existing]
+      @object.use_existing = true
+      # Pass the correct argument to arvados-cwl-runner command.
+      if src.command[0] == 'arvados-cwl-runner'
+        command = src.command - ['--disable-reuse']
+        command.insert(1, '--enable-reuse')
+      end
+    else
+      @object.use_existing = false
+      # Pass the correct argument to arvados-cwl-runner command.
+      if src.command[0] == 'arvados-cwl-runner'
+        command = src.command - ['--enable-reuse']
+        command.insert(1, '--disable-reuse')
+      end
+    end
+
+    @object.command = command
     @object.container_image = src.container_image
     @object.cwd = src.cwd
     @object.description = src.description
@@ -100,7 +119,6 @@ class ContainerRequestsController < ApplicationController
     @object.runtime_constraints = src.runtime_constraints
     @object.scheduling_parameters = src.scheduling_parameters
     @object.state = 'Uncommitted'
-    @object.use_existing = false
 
     # set owner_uuid to that of source, provided it is a project and writable by current user
     current_project = Group.find(src.owner_uuid) rescue nil
index 44acc18a019759342f94c15c3e1bafd9207be758..782639beddabdd0f8685451e14a089681fd3efe5 100644 (file)
@@ -112,23 +112,20 @@ module ProvenanceHelper
       input_obj = cr[:mounts].andand[:"/var/lib/cwl/cwl.input.json"].andand[:content] || cr[:mounts] || {}
       if input_obj
         ProvenanceHelper::find_collections input_obj, 'input' do |col_hash, col_uuid, key|
-          if col_uuid
-            gr += describe_node(col_uuid)
-            gr += edge(col_uuid, uuid, {:label => key})
-          else
+          # Only include input PDHs
+          if col_hash
             gr += describe_node(col_hash)
             gr += edge(col_hash, uuid, {:label => key})
           end
         end
       end
 
-      [
-        [:output_uuid, 'output'],
-        [:log_uuid, 'log']
-      ].each do |attr, label|
-        if cr[attr]
-          gr += describe_node(cr[attr])
-          gr += edge(uuid, cr[attr], {label: label})
+      # Add CR outputs by PDH so they connect with the child CR's inputs.
+      if cr[:output_uuid]
+        output_pdh = Collection.find(cr[:output_uuid])[:portable_data_hash]
+        if output_pdh
+          gr += describe_node(output_pdh)
+          gr += edge(uuid, output_pdh, {label: 'output'})
         end
       end
 
@@ -228,7 +225,7 @@ module ProvenanceHelper
               child_crs = ContainerRequest.where(requesting_container_uuid: cr[:container_uuid])
               child_crs.each do |child|
                 gr += generate_provenance_edges(child[:uuid])
-                gr += edge(uuid, child[:uuid], {label: 'child'})
+                gr += edge(child[:uuid], uuid, {label: 'child'})
               end
             end
           end
index ed82f18036c1025bffb16e5267701045039bb167..16462f77d817b89aa2c24853caf2dd628772f235 100644 (file)
@@ -81,8 +81,11 @@ class ContainerWorkUnit < ProxyWorkUnit
   def state_label
     ec = exit_code
     return "Failed" if (ec && ec != 0)
+
     state = get_combined(:state)
-    return "Ready" if ((priority == 0) and (["Queued", "Locked"].include?(state)))
+
+    return "Queued" if state == "Locked"
+    return "Cancelled" if ((priority == 0) and (state == "Queued"))
     state
   end
 
index fd7953551729d722b8cff678feb73baadc81f7d2..049be759beed84778bf4bf35bb474f3e83e7a0cd 100644 (file)
@@ -1,10 +1,44 @@
 <% if @object.state == 'Final' %>
-  <%= link_to(copy_container_request_path('id' => @object.uuid),
-      class: 'btn btn-sm btn-primary',
-      title: 'Re-run',
-      data: {toggle: :tooltip, placement: :top}, title: 'This will make a copy and take you there. You can then make any needed changes and run it',
-      method: :post,
-      ) do %>
-    <i class="fa fa-fw fa-play"></i> Re-run
-  <% end %>
+<script type="application/javascript">
+  function reset_form_cr_reuse() {
+    $('#use_existing').removeAttr('checked');
+  }
+</script>
+
+  <%= link_to raw('<i class="fa fa-fw fa-play"></i> Re-run...'),
+      "#",
+      {class: 'btn btn-sm btn-primary', 'data-toggle' => 'modal',
+       'data-target' => '#clone-and-edit-modal-window',
+       title: 'This will make a copy and take you there. You can then make any needed changes and run it'}  %>
+
+<div id="clone-and-edit-modal-window" class="modal fade" role="dialog"
+     aria-labelledby="myModalLabel" aria-hidden="true">
+  <div class="modal-dialog">
+    <div class="modal-content">
+
+    <%= form_tag copy_container_request_path do |f| %>
+
+      <div class="modal-header">
+        <button type="button" class="close" onClick="reset_form_cr_reuse()" data-dismiss="modal" aria-hidden="true">&times;</button>
+        <div>
+          <div class="col-sm-6"> <h4 class="modal-title">Re-run container request</h4> </div>
+        </div>
+        <br/>
+      </div>
+
+      <div class="modal-body">
+              <%= check_box_tag(:use_existing, "true", false) %>
+              <%= label_tag(:use_existing, "Enable container reuse") %>
+      </div>
+
+      <div class="modal-footer">
+        <button class="btn btn-default" onClick="reset_form_cr_reuse()" data-dismiss="modal" aria-hidden="true">Cancel</button>
+        <button type="submit" class="btn btn-primary" name="container_request[state]" value="Uncommitted">Copy and edit inputs</button>
+      </div>
+
+    </div>
+    <% end %>
+  </div>
+</div>
+
 <% end %>
index 9f714be97515608f0f8ea88a6fcd78ab288d6b0d..ea53307ed58713d1ecea5cbfa5e33c62479bd7fb 100644 (file)
@@ -1,4 +1,4 @@
 <p>
-  As an admin user, you can <%= link_to "view recent user activity", activity_users_url %> and <%= link_to "view user storage activity", storage_users_url %>.
+  As an admin user, you can <%= link_to "view recent user activity", activity_users_url %>.
 </p>
 
index 6f5a6daa100a83974526d5f3892ccbec4a491813..bd2f6beb6b45a6270825ba16eb34b2a0b00120ad 100644 (file)
@@ -38,23 +38,52 @@ class ContainerRequestsControllerTest < ActionController::TestCase
     get :show, {id: uuid}, session_for(:active)
     assert_response :success
 
-   assert_includes @response.body, "href=\"/container_requests/#{uuid}/copy\""
+   assert_includes @response.body, "action=\"/container_requests/#{uuid}/copy\""
   end
 
-  test "container request copy" do
-    completed_cr = api_fixture('container_requests')['completed']
-    post(:copy,
-         {
-           id: completed_cr['uuid']
-         },
-         session_for(:active))
-    assert_response 302
-    copied_cr = assigns(:object)
-    assert_not_nil copied_cr
-    assert_equal 'Uncommitted', copied_cr[:state]
-    assert_equal "Copy of #{completed_cr['name']}", copied_cr['name']
-    assert_equal completed_cr['cmd'], copied_cr['cmd']
-    assert_equal completed_cr['runtime_constraints']['ram'], copied_cr['runtime_constraints'][:ram]
+  [
+    ['completed', false, false],
+    ['completed', true, false],
+    ['completed-older', false, true],
+    ['completed-older', true, true],
+  ].each do |cr_fixture, reuse_enabled, uses_acr|
+    test "container request #{uses_acr ? '' : 'not'} using arvados-cwl-runner copy #{reuse_enabled ? 'with' : 'without'} reuse enabled" do
+      completed_cr = api_fixture('container_requests')[cr_fixture]
+      # Set up post request params
+      copy_params = {id: completed_cr['uuid']}
+      if reuse_enabled
+        copy_params.merge!({use_existing: true})
+      end
+      post(:copy, copy_params, session_for(:active))
+      assert_response 302
+      copied_cr = assigns(:object)
+      assert_not_nil copied_cr
+      assert_equal 'Uncommitted', copied_cr[:state]
+      assert_equal "Copy of #{completed_cr['name']}", copied_cr['name']
+      assert_equal completed_cr['cmd'], copied_cr['cmd']
+      assert_equal completed_cr['runtime_constraints']['ram'], copied_cr['runtime_constraints'][:ram]
+      if reuse_enabled
+        assert copied_cr[:use_existing]
+      else
+        refute copied_cr[:use_existing]
+      end
+      # If the CR's command is arvados-cwl-runner, the appropriate flag should
+      # be passed to it
+      if uses_acr
+        if reuse_enabled
+          # arvados-cwl-runner's default behavior is to enable reuse
+          assert_includes copied_cr['command'], 'arvados-cwl-runner'
+          assert_not_includes copied_cr['command'], '--disable-reuse'
+        else
+          assert_includes copied_cr['command'], 'arvados-cwl-runner'
+          assert_includes copied_cr['command'], '--disable-reuse'
+          assert_not_includes copied_cr['command'], '--enable-reuse'
+        end
+      else
+        # If no arvados-cwl-runner is being used, the command should be left alone
+        assert_equal completed_cr['command'], copied_cr['command']
+      end
+    end
   end
 
   [
index 8bbbb5cf26a993b3e408f923f0899097b971d8eb..1932b754544f69e772db69921d1df1c9aed5236d 100644 (file)
@@ -16,8 +16,8 @@ class WorkUnitTest < ActiveSupport::TestCase
     [Container, 'requester', 'cwu', 1, "Complete", true, 1.0],
     [ContainerRequest, 'cr_for_requester', 'cwu', 1, "Complete", true, 1.0],
     [ContainerRequest, 'queued', 'cwu', 0, "Queued", nil, 0.0],   # priority 1
-    [ContainerRequest, 'canceled_with_queued_container', 'cwu', 0, "Ready", nil, 0.0],
-    [ContainerRequest, 'canceled_with_locked_container', 'cwu', 0, "Ready", nil, 0.0],
+    [ContainerRequest, 'canceled_with_queued_container', 'cwu', 0, "Cancelled", false, 0.0],
+    [ContainerRequest, 'canceled_with_locked_container', 'cwu', 0, "Queued", nil, 0.0],
     [ContainerRequest, 'canceled_with_running_container', 'cwu', 1, "Running", nil, 0.0],
   ].each do |type, fixture, label, num_children, state, success, progress|
     test "children of #{fixture}" do
index 97a346bc29fa4ddf6b81b6ba6e4167a33aadc1de..86713f1bcad5599a1b588f83981c9591a0194fa3 100644 (file)
@@ -27,8 +27,9 @@ table(table table-bordered table-condensed).
 |_Distribution_|_State_|_Last supported version_|
 |CentOS 7|Supported|Latest|
 |Debian 8 ("jessie")|Supported|Latest|
-|Ubuntu 12.04 ("precise")|Supported|Latest|
 |Ubuntu 14.04 ("trusty")|Supported|Latest|
+|Ubuntu 16.04 ("xenial")|Supported|Latest|
+|Ubuntu 12.04 ("precise")|EOL|8ed7b6dd5d4df93a3f37096afe6d6f81c2a7ef6e (2017-05-03)|
 |Debian 7 ("wheezy")|EOL|997479d1408139e96ecdb42a60b4f727f814f6c9 (2016-12-28)|
 |CentOS 6 |EOL|997479d1408139e96ecdb42a60b4f727f814f6c9 (2016-12-28)|
 
@@ -53,7 +54,7 @@ baseurl=http://rpm.arvados.org/CentOS/$releasever/os/$basearch/
 
 h3. Debian and Ubuntu
 
-Packages are available for Debian 8 ("jessie"), Ubuntu 12.04 ("precise"), and Ubuntu 14.04 ("trusty").
+Packages are available for Debian 8 ("jessie"), Ubuntu 14.04 ("trusty") and Ubuntu 16.04 ("xenial").
 
 First, register the Curoverse signing key in apt's database:
 
@@ -64,12 +65,12 @@ Configure apt to retrieve packages from the Arvados package repository. This com
 table(table table-bordered table-condensed).
 |OS version|Command|
 |Debian 8 ("jessie")|<notextile><code><span class="userinput">echo "deb http://apt.arvados.org/ jessie main" &#x7c; sudo tee /etc/apt/sources.list.d/arvados.list</span></code></notextile>|
-|Ubuntu 12.04 ("precise")|<notextile><code><span class="userinput">echo "deb http://apt.arvados.org/ precise main" &#x7c; sudo tee /etc/apt/sources.list.d/arvados.list</span></code></notextile>|
-|Ubuntu 14.04 ("trusty")|<notextile><code><span class="userinput">echo "deb http://apt.arvados.org/ trusty main" &#x7c; sudo tee /etc/apt/sources.list.d/arvados.list</span></code></notextile>|
+|Ubuntu 14.04 ("trusty")[1]|<notextile><code><span class="userinput">echo "deb http://apt.arvados.org/ trusty main" &#x7c; sudo tee /etc/apt/sources.list.d/arvados.list</span></code></notextile>|
+|Ubuntu 16.04 ("xenial")[1]|<notextile><code><span class="userinput">echo "deb http://apt.arvados.org/ xenial main" &#x7c; sudo tee /etc/apt/sources.list.d/arvados.list</span></code></notextile>|
 
 {% include 'notebox_begin' %}
 
-Arvados packages for Ubuntu may depend on third-party packages in Ubuntu's "universe" repository.  If you're installing on Ubuntu, make sure you have the universe sources uncommented in @/etc/apt/sources.list@.
+fn1. Arvados packages for Ubuntu may depend on third-party packages in Ubuntu's "universe" repository.  If you're installing on Ubuntu, make sure you have the universe sources uncommented in @/etc/apt/sources.list@.
 
 {% include 'notebox_end' %}
 
index 657d5927d0328025eb948f6e8ee62ba215486769..0b302b6280e960fe6dfed0b8f1365f7dc3c6693c 100644 (file)
@@ -45,12 +45,20 @@ class ArvadosContainer(object):
             "properties": {}
         }
         runtime_constraints = {}
+
+        resources = self.builder.resources
+        if resources is not None:
+            runtime_constraints["vcpus"] = resources.get("cores", 1)
+            runtime_constraints["ram"] = resources.get("ram") * 2**20
+
         mounts = {
             self.outdir: {
-                "kind": "tmp"
+                "kind": "tmp",
+                "capacity": resources.get("outdirSize", 0) * 2**20
             },
             self.tmpdir: {
-                "kind": "tmp"
+                "kind": "tmp",
+                "capacity": resources.get("tmpdirSize", 0) * 2**20
             }
         }
         scheduling_parameters = {}
@@ -139,11 +147,6 @@ class ArvadosContainer(object):
                                                                      pull_image,
                                                                      self.arvrunner.project_uuid)
 
-        resources = self.builder.resources
-        if resources is not None:
-            runtime_constraints["vcpus"] = resources.get("cores", 1)
-            runtime_constraints["ram"] = resources.get("ram") * 2**20
-
         api_req, _ = get_feature(self, "http://arvados.org/cwl#APIRequirement")
         if api_req:
             runtime_constraints["API"] = True
@@ -152,6 +155,15 @@ class ArvadosContainer(object):
         if runtime_req:
             if "keep_cache" in runtime_req:
                 runtime_constraints["keep_cache_ram"] = runtime_req["keep_cache"] * 2**20
+            if "outputDirType" in runtime_req:
+                if runtime_req["outputDirType"] == "local_output_dir":
+                    # Currently the default behavior.
+                    pass
+                elif runtime_req["outputDirType"] == "keep_output_dir":
+                    mounts[self.outdir]= {
+                        "kind": "collection",
+                        "writable": True
+                    }
 
         partition_req, _ = get_feature(self, "http://arvados.org/cwl#PartitionRequirement")
         if partition_req:
index b06eae8105aad06d35ece8611f3b7a5103ab838c..af05773e0c62f186294e64be242653242f403338 100644 (file)
@@ -63,8 +63,11 @@ class TestContainer(unittest.TestCase):
                         'use_existing': enable_reuse,
                         'priority': 1,
                         'mounts': {
-                            '/tmp': {'kind': 'tmp'},
-                            '/var/spool/cwl': {'kind': 'tmp'}
+                            '/tmp': {'kind': 'tmp',
+                                     "capacity": 1073741824
+                                 },
+                            '/var/spool/cwl': {'kind': 'tmp',
+                                               "capacity": 1073741824 }
                         },
                         'state': 'Committed',
                         'owner_uuid': 'zzzzz-8i9sb-zzzzzzzzzzzzzzz',
@@ -97,7 +100,8 @@ class TestContainer(unittest.TestCase):
                 "class": "ResourceRequirement",
                 "coresMin": 3,
                 "ramMin": 3000,
-                "tmpdirMin": 4000
+                "tmpdirMin": 4000,
+                "outdirMin": 5000
             }, {
                 "class": "http://arvados.org/cwl#RuntimeConstraints",
                 "keep_cache": 512
@@ -136,8 +140,10 @@ class TestContainer(unittest.TestCase):
                 'use_existing': True,
                 'priority': 1,
                 'mounts': {
-                    '/tmp': {'kind': 'tmp'},
-                    '/var/spool/cwl': {'kind': 'tmp'}
+                    '/tmp': {'kind': 'tmp',
+                             "capacity": 4194304000 },
+                    '/var/spool/cwl': {'kind': 'tmp',
+                                       "capacity": 5242880000 }
                 },
                 'state': 'Committed',
                 'owner_uuid': 'zzzzz-8i9sb-zzzzzzzzzzzzzzz',
@@ -242,8 +248,10 @@ class TestContainer(unittest.TestCase):
                 'use_existing': True,
                 'priority': 1,
                 'mounts': {
-                    '/tmp': {'kind': 'tmp'},
-                    '/var/spool/cwl': {'kind': 'tmp'},
+                    '/tmp': {'kind': 'tmp',
+                             "capacity": 1073741824 },
+                    '/var/spool/cwl': {'kind': 'tmp',
+                                       "capacity": 1073741824 },
                     '/var/spool/cwl/foo': {
                         'kind': 'collection',
                         'path': 'foo',
@@ -328,8 +336,10 @@ class TestContainer(unittest.TestCase):
                     'use_existing': True,
                     'priority': 1,
                     'mounts': {
-                        '/tmp': {'kind': 'tmp'},
-                        '/var/spool/cwl': {'kind': 'tmp'},
+                        '/tmp': {'kind': 'tmp',
+                                 "capacity": 1073741824 },
+                        '/var/spool/cwl': {'kind': 'tmp',
+                                           "capacity": 1073741824 },
                         "stderr": {
                             "kind": "file",
                             "path": "/var/spool/cwl/stderr.txt"
@@ -460,8 +470,10 @@ class TestContainer(unittest.TestCase):
                             "kind": "collection",
                             "portable_data_hash": "99999999999999999999999999999994+44"
                         },
-                        '/tmp': {'kind': 'tmp'},
-                        '/var/spool/cwl': {'kind': 'tmp'}
+                        '/tmp': {'kind': 'tmp',
+                                 "capacity": 1073741824 },
+                        '/var/spool/cwl': {'kind': 'tmp',
+                                           "capacity": 1073741824 }
                     },
                     'state': 'Committed',
                     'owner_uuid': 'zzzzz-8i9sb-zzzzzzzzzzzzzzz',
index 36d9af1a269c0ae96858155ef10cbb3b4b029245..d59a57c25c15666e37edece0aa2c36b399c5b280 100644 (file)
@@ -27,6 +27,7 @@ type Mount struct {
        Path              string      `json:"path"`
        Content           interface{} `json:"content"`
        ExcludeFromOutput bool        `json:"exclude_from_output"`
+       Capacity          int64       `json:capacity`
 }
 
 // RuntimeConstraints specify a container's compute resources (RAM,
index 42510754aba4724bb7b91aa061125fce52a1c1a0..6836d803886291a57dcc39b34c3dcb2c96a8c515 100644 (file)
@@ -185,6 +185,16 @@ _group.add_argument('--no-resume', action='store_false', dest='resume',
 Do not continue interrupted uploads from cached state.
 """)
 
+_group = run_opts.add_mutually_exclusive_group()
+_group.add_argument('--follow-links', action='store_true', default=True,
+                    dest='follow_links', help="""
+Follow file and directory symlinks (default).
+""")
+_group.add_argument('--no-follow-links', action='store_false', dest='follow_links',
+                    help="""
+Do not follow file and directory symlinks.
+""")
+
 _group = run_opts.add_mutually_exclusive_group()
 _group.add_argument('--cache', action='store_true', dest='use_cache', default=True,
                     help="""
@@ -236,6 +246,10 @@ def parse_arguments(arguments):
     return args
 
 
+class PathDoesNotExistError(Exception):
+    pass
+
+
 class CollectionUpdateError(Exception):
     pass
 
@@ -361,7 +375,8 @@ class ArvPutUploadJob(object):
                  ensure_unique_name=False, num_retries=None,
                  put_threads=None, replication_desired=None,
                  filename=None, update_time=60.0, update_collection=None,
-                 logger=logging.getLogger('arvados.arv_put'), dry_run=False):
+                 logger=logging.getLogger('arvados.arv_put'), dry_run=False,
+                 follow_links=True):
         self.paths = paths
         self.resume = resume
         self.use_cache = use_cache
@@ -394,6 +409,7 @@ class ArvPutUploadJob(object):
         self.logger = logger
         self.dry_run = dry_run
         self._checkpoint_before_quit = True
+        self.follow_links = follow_links
 
         if not self.use_cache and self.resume:
             raise ArvPutArgumentConflict('resume cannot be True when use_cache is False')
@@ -418,13 +434,15 @@ class ArvPutUploadJob(object):
                     if self.dry_run:
                         raise ArvPutUploadIsPending()
                     self._write_stdin(self.filename or 'stdin')
+                elif not os.path.exists(path):
+                     raise PathDoesNotExistError("file or directory '{}' does not exist.".format(path))
                 elif os.path.isdir(path):
                     # Use absolute paths on cache index so CWD doesn't interfere
                     # with the caching logic.
                     prefixdir = path = os.path.abspath(path)
                     if prefixdir != '/':
                         prefixdir += '/'
-                    for root, dirs, files in os.walk(path):
+                    for root, dirs, files in os.walk(path, followlinks=self.follow_links):
                         # Make os.walk()'s dir traversing order deterministic
                         dirs.sort()
                         files.sort()
@@ -456,7 +474,10 @@ class ArvPutUploadJob(object):
             # Note: We're expecting SystemExit instead of KeyboardInterrupt because
             #   we have a custom signal handler in place that raises SystemExit with
             #   the catched signal's code.
-            if not isinstance(e, SystemExit) or e.code != -2:
+            if isinstance(e, PathDoesNotExistError):
+                # We aren't interested in the traceback for this case
+                pass
+            elif not isinstance(e, SystemExit) or e.code != -2:
                 self.logger.warning("Abnormal termination:\n{}".format(traceback.format_exc(e)))
             raise
         finally:
@@ -562,7 +583,12 @@ class ArvPutUploadJob(object):
         output.close()
 
     def _check_file(self, source, filename):
-        """Check if this file needs to be uploaded"""
+        """
+        Check if this file needs to be uploaded
+        """
+        # Ignore symlinks when requested
+        if (not self.follow_links) and os.path.islink(source):
+            return
         resume_offset = 0
         should_upload = False
         new_file_in_cache = False
@@ -798,14 +824,20 @@ class ArvPutUploadJob(object):
         return datablocks
 
 
-def expected_bytes_for(pathlist):
+def expected_bytes_for(pathlist, follow_links=True):
     # Walk the given directory trees and stat files, adding up file sizes,
     # so we can display progress as percent
     bytesum = 0
     for path in pathlist:
         if os.path.isdir(path):
-            for filename in arvados.util.listdir_recursive(path):
-                bytesum += os.path.getsize(os.path.join(path, filename))
+            for root, dirs, files in os.walk(path, followlinks=follow_links):
+                # Sum file sizes
+                for f in files:
+                    filepath = os.path.join(root, f)
+                    # Ignore symlinked files when requested
+                    if (not follow_links) and os.path.islink(filepath):
+                        continue
+                    bytesum += os.path.getsize(filepath)
         elif not os.path.isfile(path):
             return None
         else:
@@ -893,7 +925,7 @@ def main(arguments=None, stdout=sys.stdout, stderr=sys.stderr):
     # uploaded, the expected bytes calculation can take a moment.
     if args.progress and any([os.path.isdir(f) for f in args.paths]):
         logger.info("Calculating upload size, this could take some time...")
-    bytes_expected = expected_bytes_for(args.paths)
+    bytes_expected = expected_bytes_for(args.paths, follow_links=args.follow_links)
 
     try:
         writer = ArvPutUploadJob(paths = args.paths,
@@ -910,7 +942,8 @@ def main(arguments=None, stdout=sys.stdout, stderr=sys.stderr):
                                  ensure_unique_name = True,
                                  update_collection = args.update_collection,
                                  logger=logger,
-                                 dry_run=args.dry_run)
+                                 dry_run=args.dry_run,
+                                 follow_links=args.follow_links)
     except ResumeCacheConflict:
         logger.error("\n".join([
             "arv-put: Another process is already uploading this data.",
@@ -952,6 +985,10 @@ def main(arguments=None, stdout=sys.stdout, stderr=sys.stderr):
     except ArvPutUploadNotPending:
         # No files pending for upload
         sys.exit(0)
+    except PathDoesNotExistError as error:
+        logger.error("\n".join([
+            "arv-put: %s" % str(error)]))
+        sys.exit(1)
 
     if args.progress:  # Print newline to split stderr from stdout for humans.
         logger.info("\n")
index 286a22e36a559779fd190a96201479d5ed81413d..320189104ab555dc97be4c618e83adc8d6acdd7f 100644 (file)
@@ -17,6 +17,7 @@ import yaml
 import threading
 import hashlib
 import random
+import uuid
 
 from cStringIO import StringIO
 
@@ -268,12 +269,39 @@ class ArvPutUploadJobTest(run_test_server.TestCaseWithServers,
             with open(os.path.join(self.small_files_dir, str(i)), 'w') as f:
                 f.write(data + str(i))
         self.arvfile_write = getattr(arvados.arvfile.ArvadosFileWriter, 'write')
+        # Temp dir to hold a symlink to other temp dir
+        self.tempdir_with_symlink = tempfile.mkdtemp()
+        os.symlink(self.tempdir, os.path.join(self.tempdir_with_symlink, 'linkeddir'))
+        os.symlink(os.path.join(self.tempdir, '1'),
+                   os.path.join(self.tempdir_with_symlink, 'linkedfile'))
 
     def tearDown(self):
         super(ArvPutUploadJobTest, self).tearDown()
         shutil.rmtree(self.tempdir)
         os.unlink(self.large_file_name)
         shutil.rmtree(self.small_files_dir)
+        shutil.rmtree(self.tempdir_with_symlink)
+
+    def test_symlinks_are_followed_by_default(self):
+        cwriter = arv_put.ArvPutUploadJob([self.tempdir_with_symlink])
+        cwriter.start(save_collection=False)
+        self.assertIn('linkeddir', cwriter.manifest_text())
+        self.assertIn('linkedfile', cwriter.manifest_text())
+        cwriter.destroy_cache()
+
+    def test_symlinks_are_not_followed_when_requested(self):
+        cwriter = arv_put.ArvPutUploadJob([self.tempdir_with_symlink],
+                                          follow_links=False)
+        cwriter.start(save_collection=False)
+        self.assertNotIn('linkeddir', cwriter.manifest_text())
+        self.assertNotIn('linkedfile', cwriter.manifest_text())
+        cwriter.destroy_cache()
+
+    def test_passing_nonexistant_path_raise_exception(self):
+        uuid_str = str(uuid.uuid4())
+        cwriter = arv_put.ArvPutUploadJob(["/this/path/does/not/exist/{}".format(uuid_str)])
+        with self.assertRaises(arv_put.PathDoesNotExistError):
+            cwriter.start(save_collection=False)
 
     def test_writer_works_without_cache(self):
         cwriter = arv_put.ArvPutUploadJob(['/dev/null'], resume=False)
index 0d6aa1d9f933bcd5b89bc7f03cdd3212c58cfe22..38c5ae107167468a90375f3670e61d5f1234d331 100644 (file)
@@ -55,6 +55,8 @@ module Server
 
     config.active_support.test_order = :sorted
 
+    config.action_dispatch.perform_deep_munge = false
+
     I18n.enforce_available_locales = false
 
     # Before using the filesystem backend for Rails.cache, check
diff --git a/services/api/config/initializers/noop_deep_munge.rb b/services/api/config/initializers/noop_deep_munge.rb
deleted file mode 100644 (file)
index b1f84e9..0000000
+++ /dev/null
@@ -1,10 +0,0 @@
-module ActionDispatch
-  class Request < Rack::Request
-    # This Rails method messes with valid JSON, for example turning the empty
-    # array [] into 'nil'.  We don't want that, so turn it into a no-op.
-    remove_method :deep_munge
-    def deep_munge(hash)
-      hash
-    end
-  end
-end
index 76f59c29f87a3ebbad2672a42a5f1502883c2b13..d5bb47f6ff30e1bcae98944a47844b01f4d98c99 100644 (file)
@@ -109,7 +109,7 @@ completed-older:
   container_image: test
   cwd: test
   output_path: test
-  command: ["echo", "hello"]
+  command: ["arvados-cwl-runner", "echo", "hello"]
   container_uuid: zzzzz-dz642-compltcontainr2
   runtime_constraints:
     vcpus: 1
index d1f4c7bdc8fac1248aa80d3b0a88d674900cc8e8..07ccb134287e081914ac6a226c7b7ff433bbd40c 100644 (file)
@@ -12,6 +12,13 @@ queued:
   runtime_constraints:
     ram: 12000000000
     vcpus: 4
+  mounts:
+    /tmp:
+      kind: tmp
+      capacity: 24000000000
+    /var/spool/cwl:
+      kind: tmp
+      capacity: 24000000000
 
 running:
   uuid: zzzzz-dz642-runningcontainr
similarity index 63%
rename from services/api/test/integration/noop_deep_munge.rb
rename to services/api/test/integration/noop_deep_munge_test.rb
index 6305fe58b8a238ca6ad02dafd5fc2395f8af104f..11ffa51f190cba6c3fccf71d23d3af7dfefd1276 100644 (file)
@@ -1,8 +1,21 @@
 require 'test_helper'
 
-class NoopDeepMunge < ActionDispatch::IntegrationTest
+class NoopDeepMungeTest < ActionDispatch::IntegrationTest
+  test "empty array" do
+    check({"foo" => []})
+  end
+
+  test "null in array" do
+    check({"foo" => ["foo", nil]})
+  end
 
-  test "that empty list round trips properly" do
+  test "array of nulls" do
+    check({"foo" => [nil, nil, nil]})
+  end
+
+  protected
+
+  def check(val)
     post "/arvados/v1/container_requests",
          {
            :container_request => {
@@ -14,10 +27,7 @@ class NoopDeepMunge < ActionDispatch::IntegrationTest
              :mounts => {
                :foo => {
                  :kind => "json",
-                 :content => {
-                   :a => [],
-                   :b => {}
-                 }
+                 :content => JSON.parse(SafeJSON.dump(val)),
                }
              }
            }
@@ -25,11 +35,6 @@ class NoopDeepMunge < ActionDispatch::IntegrationTest
                     'CONTENT_TYPE' => 'application/json'}
     assert_response :success
     assert_equal "arvados#containerRequest", json_response['kind']
-    content = {
-      "a" => [],
-      "b" => {}
-    }
-    assert_equal content, json_response['mounts']['foo']['content']
-
+    assert_equal val, json_response['mounts']['foo']['content']
   end
 end
index d84d461c30184e12b45cd87611580d03004a343b..cca8b3f1505de624c3b9f5775cec95c33e6b2b58 100644 (file)
@@ -149,14 +149,22 @@ func checkSqueueForOrphans(dispatcher *dispatch.Dispatcher, sqCheck *SqueueCheck
 
 // sbatchCmd
 func sbatchFunc(container arvados.Container) *exec.Cmd {
-       memPerCPU := math.Ceil(float64(container.RuntimeConstraints.RAM) / (float64(container.RuntimeConstraints.VCPUs) * 1048576))
+       mem := int64(math.Ceil(float64(container.RuntimeConstraints.RAM+container.RuntimeConstraints.KeepCacheRAM) / float64(1048576)))
+
+       var disk int64
+       for _, m := range container.Mounts {
+               if m.Kind == "tmp" {
+                       disk += m.Capacity
+               }
+       }
+       disk = int64(math.Ceil(float64(disk) / float64(1048576)))
 
        var sbatchArgs []string
-       sbatchArgs = append(sbatchArgs, "--share")
        sbatchArgs = append(sbatchArgs, theConfig.SbatchArguments...)
        sbatchArgs = append(sbatchArgs, fmt.Sprintf("--job-name=%s", container.UUID))
-       sbatchArgs = append(sbatchArgs, fmt.Sprintf("--mem-per-cpu=%d", int(memPerCPU)))
+       sbatchArgs = append(sbatchArgs, fmt.Sprintf("--mem=%d", mem))
        sbatchArgs = append(sbatchArgs, fmt.Sprintf("--cpus-per-task=%d", container.RuntimeConstraints.VCPUs))
+       sbatchArgs = append(sbatchArgs, fmt.Sprintf("--tmp=%d", disk))
        if len(container.SchedulingParameters.Partitions) > 0 {
                sbatchArgs = append(sbatchArgs, fmt.Sprintf("--partition=%s", strings.Join(container.SchedulingParameters.Partitions, ",")))
        }
index e6abb1650ec1007d6f44906d7fdeba7c977318f9..1c366a0e5a59ecaa17233354e14cd882d1470deb 100644 (file)
@@ -118,10 +118,11 @@ func (s *TestSuite) TestIntegrationCancel(c *C) {
 }
 
 func (s *TestSuite) TestIntegrationMissingFromSqueue(c *C) {
-       container := s.integrationTest(c, func() *exec.Cmd { return exec.Command("echo") }, []string{"sbatch", "--share",
+       container := s.integrationTest(c, func() *exec.Cmd { return exec.Command("echo") }, []string{"sbatch",
                fmt.Sprintf("--job-name=%s", "zzzzz-dz642-queuedcontainer"),
-               fmt.Sprintf("--mem-per-cpu=%d", 2862),
-               fmt.Sprintf("--cpus-per-task=%d", 4)},
+               fmt.Sprintf("--mem=%d", 11445),
+               fmt.Sprintf("--cpus-per-task=%d", 4),
+               fmt.Sprintf("--tmp=%d", 45777)},
                func(dispatcher *dispatch.Dispatcher, container arvados.Container) {
                        dispatcher.UpdateState(container.UUID, dispatch.Running)
                        time.Sleep(3 * time.Second)
@@ -327,9 +328,9 @@ func testSbatchFuncWithArgs(c *C, args []string) {
        sbatchCmd := sbatchFunc(container)
 
        var expected []string
-       expected = append(expected, "sbatch", "--share")
+       expected = append(expected, "sbatch")
        expected = append(expected, theConfig.SbatchArguments...)
-       expected = append(expected, "--job-name=123", "--mem-per-cpu=120", "--cpus-per-task=2")
+       expected = append(expected, "--job-name=123", "--mem=239", "--cpus-per-task=2", "--tmp=0")
 
        c.Check(sbatchCmd.Args, DeepEquals, expected)
 }
@@ -340,8 +341,8 @@ func (s *MockArvadosServerSuite) TestSbatchPartition(c *C) {
        sbatchCmd := sbatchFunc(container)
 
        var expected []string
-       expected = append(expected, "sbatch", "--share")
-       expected = append(expected, "--job-name=123", "--mem-per-cpu=239", "--cpus-per-task=1", "--partition=blurb,b2")
+       expected = append(expected, "sbatch")
+       expected = append(expected, "--job-name=123", "--mem=239", "--cpus-per-task=1", "--tmp=0", "--partition=blurb,b2")
 
        c.Check(sbatchCmd.Args, DeepEquals, expected)
 }
index 812525db6904ba1201a54502c5fd781686b0188b..07937291724c8cde9f6a938414924e205e63c0ef 100644 (file)
@@ -430,24 +430,25 @@ func (runner *ContainerRunner) SetupMounts() (err error) {
                        }
                        collectionPaths = append(collectionPaths, src)
 
-               case mnt.Kind == "tmp" && bind == runner.Container.OutputPath:
-                       runner.HostOutputDir, err = runner.MkTempDir("", "")
+               case mnt.Kind == "tmp":
+                       var tmpdir string
+                       tmpdir, err = runner.MkTempDir("", "")
                        if err != nil {
                                return fmt.Errorf("While creating mount temp dir: %v", err)
                        }
-                       st, staterr := os.Stat(runner.HostOutputDir)
+                       st, staterr := os.Stat(tmpdir)
                        if staterr != nil {
                                return fmt.Errorf("While Stat on temp dir: %v", staterr)
                        }
-                       err = os.Chmod(runner.HostOutputDir, st.Mode()|os.ModeSetgid|0777)
+                       err = os.Chmod(tmpdir, st.Mode()|os.ModeSetgid|0777)
                        if staterr != nil {
                                return fmt.Errorf("While Chmod temp dir: %v", err)
                        }
-                       runner.CleanupTempDir = append(runner.CleanupTempDir, runner.HostOutputDir)
-                       runner.Binds = append(runner.Binds, fmt.Sprintf("%s:%s", runner.HostOutputDir, bind))
-
-               case mnt.Kind == "tmp":
-                       runner.Volumes[bind] = struct{}{}
+                       runner.CleanupTempDir = append(runner.CleanupTempDir, tmpdir)
+                       runner.Binds = append(runner.Binds, fmt.Sprintf("%s:%s", tmpdir, bind))
+                       if bind == runner.Container.OutputPath {
+                               runner.HostOutputDir = tmpdir
+                       }
 
                case mnt.Kind == "json":
                        jsondata, err := json.Marshal(mnt.Content)
index 5bfbf796f045c2d90fe45495dc7c645151c190ca..c8427563cb9447ceada0af2d44a3ef30586672ab 100644 (file)
@@ -977,6 +977,22 @@ func (s *TestSuite) TestSetupMounts(c *C) {
                checkEmpty()
        }
 
+       {
+               i = 0
+               cr.ArvMountPoint = ""
+               cr.Container.Mounts = make(map[string]arvados.Mount)
+               cr.Container.Mounts["/out"] = arvados.Mount{Kind: "tmp"}
+               cr.Container.Mounts["/tmp"] = arvados.Mount{Kind: "tmp"}
+               cr.OutputPath = "/out"
+
+               err := cr.SetupMounts()
+               c.Check(err, IsNil)
+               c.Check(am.Cmd, DeepEquals, []string{"--foreground", "--allow-other", "--read-write", "--mount-by-pdh", "by_id", realTemp + "/keep1"})
+               c.Check(cr.Binds, DeepEquals, []string{realTemp + "/2:/out", realTemp + "/3:/tmp"})
+               cr.CleanupDirs()
+               checkEmpty()
+       }
+
        {
                i = 0
                cr.ArvMountPoint = ""
index f6e9249ebb812b3e33610479d28dd27f28c000fe..66cf73acb357f4e5cd78dfcd20793cf697749be5 100644 (file)
@@ -25,7 +25,9 @@ class ServerCalculator(object):
                          'extra']:
                 setattr(self, name, getattr(self.real, name))
             self.cores = kwargs.pop('cores')
-            self.scratch = self.disk
+            # libcloud disk sizes are in GB, Arvados/SLURM are in MB
+            # multiply by 1000 instead of 1024 to err on low side
+            self.scratch = self.disk * 1000
             self.ram = int(self.ram * node_mem_scaling)
             for name, override in kwargs.iteritems():
                 if not hasattr(self, name):
@@ -53,6 +55,10 @@ class ServerCalculator(object):
         self.logger = logging.getLogger('arvnodeman.jobqueue')
         self.logged_jobs = set()
 
+        self.logger.info("Using cloud node sizes:")
+        for s in self.cloud_sizes:
+            self.logger.info(str(s.__dict__))
+
     @staticmethod
     def coerce_int(x, fallback):
         try:
index c20313913bbd79a18f8d6969d0e18ad57afa9db2..08a813185ed6f52fbf90c22430435f4b25075c8d 100644 (file)
@@ -62,7 +62,7 @@ class ServerCalculatorTestCase(unittest.TestCase):
                                   {'min_ram_mb_per_node': 256},
                                   {'min_nodes': 6},
                                   {'min_nodes': 12},
-                                  {'min_scratch_mb_per_node': 200})
+                                  {'min_scratch_mb_per_node': 300000})
         self.assertEqual(6, len(servlist))
 
     def test_ignore_too_expensive_jobs(self):