[ARVADOS] created: 1.1.2-119-g9359659

Git user git at public.curoverse.com
Wed Jan 31 10:38:59 EST 2018


        at  9359659d79a0c17265ff8a09e896920243a1b800 (commit)


commit 9359659d79a0c17265ff8a09e896920243a1b800
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Wed Jan 31 10:17:21 2018 -0500

    12902: Offer Cancel button for "on hold" containers.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/apps/workbench/app/models/container_work_unit.rb b/apps/workbench/app/models/container_work_unit.rb
index 103507a..dbc81c5 100644
--- a/apps/workbench/app/models/container_work_unit.rb
+++ b/apps/workbench/app/models/container_work_unit.rb
@@ -58,7 +58,10 @@ class ContainerWorkUnit < ProxyWorkUnit
   end
 
   def can_cancel?
-    @proxied.is_a?(ContainerRequest) && @proxied.state == "Committed" && @proxied.priority > 0 && @proxied.editable?
+    @proxied.is_a?(ContainerRequest) &&
+      @proxied.state == "Committed" &&
+      (@proxied.priority > 0 || get(:state, @container) != 'Running') &&
+      @proxied.editable?
   end
 
   def container_uuid

commit fd228a5ac00d71170eadef15560ced417b1d3909
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Wed Jan 31 10:09:11 2018 -0500

    12902: Say "created at" instead of "started at" for a queued ctr.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/apps/workbench/app/views/projects/_show_dashboard.html.erb b/apps/workbench/app/views/projects/_show_dashboard.html.erb
index 7135826..e51cf53 100644
--- a/apps/workbench/app/views/projects/_show_dashboard.html.erb
+++ b/apps/workbench/app/views/projects/_show_dashboard.html.erb
@@ -125,12 +125,15 @@ SPDX-License-Identifier: AGPL-3.0 %>
               </div>
 
               <div class="clearfix">
-                Started at <%= render_localized_date(wu.started_at || wu.created_at, "noseconds") %>.
-                <% wu_time = Time.now - (wu.started_at || wu.created_at) %>
-                Active for <%= render_runtime(wu_time, false) %>.
-
-                <div class="pull-right">
-                </div>
+                <% if wu.started_at %>
+                  Started at <%= render_localized_date(wu.started_at, "noseconds") %>
+                  Active for <%= render_runtime(Time.now - wu.started_at, false) %>.
+                <% else %>
+                  Created at <%= render_localized_date(wu.created_at, "noseconds") %>.
+                  <% if wu.state_label == 'Queued' %>
+                    Queued for <%= render_runtime(Time.now - wu.created_at, false) %>.
+                  <% end %>
+                <% end %>
               </div>
             </div>
             <% end %>

commit d03facf18d724c52c09604710b074d465febfece
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Wed Jan 31 09:48:03 2018 -0500

    12902: Fix crash on null container_request name.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/apps/workbench/app/views/container_requests/_show_recent_rows.html.erb b/apps/workbench/app/views/container_requests/_show_recent_rows.html.erb
index 32de59c..0212162 100644
--- a/apps/workbench/app/views/container_requests/_show_recent_rows.html.erb
+++ b/apps/workbench/app/views/container_requests/_show_recent_rows.html.erb
@@ -24,7 +24,7 @@ SPDX-License-Identifier: AGPL-3.0 %>
     <td>
       <span class="label label-<%= wu.state_bootstrap_class %>"><%= wu.state_label %></span>
     </td><td>
-      <%= link_to_if_arvados_object obj, friendly_name: true, link_text: if !obj.name.empty? then obj.name else obj.uuid end %>
+      <%= link_to_if_arvados_object obj, friendly_name: true, link_text: if obj.name && !obj.name.empty? then obj.name else obj.uuid end %>
     </td><td>
       <%= obj.description || '' %>
     </td><td>

commit 086f6cc8645e655f75c84268e7e758856a07be87
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Tue Jan 30 22:41:01 2018 -0500

    12902: Label CRs "Cancelled" if finalized with incomplete container.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/apps/workbench/app/models/container_work_unit.rb b/apps/workbench/app/models/container_work_unit.rb
index a5b26f0..103507a 100644
--- a/apps/workbench/app/models/container_work_unit.rb
+++ b/apps/workbench/app/models/container_work_unit.rb
@@ -95,14 +95,29 @@ class ContainerWorkUnit < ProxyWorkUnit
   end
 
   def state_label
-    ec = exit_code
-    return "Failed" if (ec && ec != 0)
-
-    state = get_combined(:state)
-
-    return "Queued" if state == "Locked"
-    return "Cancelled" if ((priority == 0) and (state == "Queued"))
-    state
+    if get(:state) == 'Final' && get(:state, @container) != 'Complete'
+      # Request was finalized before its container started (or the
+      # container was cancelled)
+      return 'Cancelled'
+    end
+    state = get(:state, @container) || get(:state, @proxied)
+    case state
+    when 'Locked', 'Queued'
+      if priority == 0
+        'On hold'
+      else
+        'Queued'
+      end
+    when 'Complete'
+      if exit_code == 0
+        state
+      else
+        'Failed'
+      end
+    else
+      # Cancelled, Running, or Uncommitted (no container assigned)
+      state
+    end
   end
 
   def exit_code
diff --git a/apps/workbench/test/unit/work_unit_test.rb b/apps/workbench/test/unit/work_unit_test.rb
index 5cf9499..9f660cd 100644
--- a/apps/workbench/test/unit/work_unit_test.rb
+++ b/apps/workbench/test/unit/work_unit_test.rb
@@ -21,7 +21,7 @@ class WorkUnitTest < ActiveSupport::TestCase
     [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, "Cancelled", false, 0.0],
-    [ContainerRequest, 'canceled_with_locked_container', 'cwu', 0, "Queued", nil, 0.0],
+    [ContainerRequest, 'canceled_with_locked_container', 'cwu', 0, "Cancelled", false, 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
diff --git a/services/api/test/fixtures/container_requests.yml b/services/api/test/fixtures/container_requests.yml
index 85e40ff..29ce4f5 100644
--- a/services/api/test/fixtures/container_requests.yml
+++ b/services/api/test/fixtures/container_requests.yml
@@ -219,7 +219,7 @@ canceled_with_queued_container:
   uuid: zzzzz-xvhdp-canceledqueuedc
   owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
   name: canceled with queued container
-  state: Committed
+  state: Final
   priority: 0
   created_at: 2016-01-11 11:11:11.111111111 Z
   updated_at: 2016-01-11 11:11:11.111111111 Z
@@ -238,7 +238,7 @@ canceled_with_locked_container:
   uuid: zzzzz-xvhdp-canceledlocekdc
   owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
   name: canceled with locked container
-  state: Committed
+  state: Final
   priority: 0
   created_at: 2016-01-11 11:11:11.111111111 Z
   updated_at: 2016-01-11 11:11:11.111111111 Z

commit bcb86b69fba693af210e82d40d92854e609157c4
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Tue Jan 30 17:56:13 2018 -0500

    12902: Update state=Final when cancelling a container request.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/apps/workbench/app/controllers/container_requests_controller.rb b/apps/workbench/app/controllers/container_requests_controller.rb
index f61596e..783cafa 100644
--- a/apps/workbench/app/controllers/container_requests_controller.rb
+++ b/apps/workbench/app/controllers/container_requests_controller.rb
@@ -77,6 +77,17 @@ class ContainerRequestsController < ApplicationController
   end
 
   def cancel
+    if @object.container_uuid
+      c = Container.select(['state']).where(uuid: @object.container_uuid).first
+      if c && c.state != 'Running'
+        # If the container hasn't started yet, setting priority=0
+        # leaves our request in "Committed" state and doesn't cancel
+        # the container (even if no other requests are giving it
+        # priority). To avoid showing this container request as "on
+        # hold" after hitting the Cancel button, set state=Final too.
+        @object.state = 'Final'
+      end
+    end
     @object.update_attributes! priority: 0
     if params[:return_to]
       redirect_to params[:return_to]
diff --git a/apps/workbench/test/controllers/container_requests_controller_test.rb b/apps/workbench/test/controllers/container_requests_controller_test.rb
index 206352a..261169c 100644
--- a/apps/workbench/test/controllers/container_requests_controller_test.rb
+++ b/apps/workbench/test/controllers/container_requests_controller_test.rb
@@ -42,7 +42,21 @@ class ContainerRequestsControllerTest < ActionController::TestCase
     get :show, {id: uuid}, session_for(:active)
     assert_response :success
 
-   assert_includes @response.body, "action=\"/container_requests/#{uuid}/copy\""
+    assert_includes @response.body, "action=\"/container_requests/#{uuid}/copy\""
+  end
+
+  test "cancel request for queued container" do
+    cr_fixture = api_fixture('container_requests')['queued']
+    post :cancel, {id: cr_fixture['uuid']}, session_for(:active)
+    assert_response 302
+
+    use_token 'active'
+    cr = ContainerRequest.find(cr_fixture['uuid'])
+    assert_equal 'Final', cr.state
+    assert_equal 0, cr.priority
+    c = Container.find(cr_fixture['container_uuid'])
+    assert_equal 'Queued', c.state
+    assert_equal 0, c.priority
   end
 
   [
diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb
index 0b18914..bcca407 100644
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -239,12 +239,13 @@ class ContainerRequest < ArvadosModel
       end
 
     when Final
-      if self.state_changed? and not current_user.andand.is_admin
-        self.errors.add :state, "of container request can only be set to Final by system."
-      end
-
       if self.state_was == Committed
-        permitted.push :output_uuid, :log_uuid
+        # "Cancel" means setting priority=0, state=Committed
+        permitted.push :priority
+
+        if current_user.andand.is_admin
+          permitted.push :output_uuid, :log_uuid
+        end
       end
 
     end

commit 23995e4bd823ff1a9f4986ac2b852fd35d740940
Author: Tom Clegg <tclegg at veritasgenetics.com>
Date:   Tue Jan 23 10:40:21 2018 -0500

    12902: Leave child CR "cancelled" (not "on hold") when parent exits.
    
    Changing a container's priority to 0 effects "cancel" only after it
    has started. Before that, it just gets held in the queue with priority
    0. The child CRs of a terminated container are presumed abandoned, so
    it makes more sense to finalize them.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg at veritasgenetics.com>

diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index edcb850..b013776 100644
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -90,7 +90,7 @@ class Container < ArvadosModel
       self.priority = ContainerRequest.
         where(container_uuid: uuid,
               state: ContainerRequest::Committed).
-        maximum('priority')
+        maximum('priority') || 0
       self.save!
     end
   end
@@ -515,7 +515,7 @@ class Container < ArvadosModel
             cr.with_lock do
               # Use row locking because this increments container_count
               cr.container_uuid = c.uuid
-              cr.save
+              cr.save!
             end
           end
         end
@@ -526,11 +526,21 @@ class Container < ArvadosModel
           cr.finalize!
         end
 
-        # Try to cancel any outstanding container requests made by this container.
-        ContainerRequest.where(requesting_container_uuid: uuid,
-                               state: ContainerRequest::Committed).each do |cr|
-          cr.priority = 0
-          cr.save
+        # Cancel outstanding container requests made by this container.
+        ContainerRequest.
+          includes(:container).
+          where(requesting_container_uuid: uuid,
+                state: ContainerRequest::Committed).each do |cr|
+          cr.update_attributes!(priority: 0)
+          cr.container.reload
+          if cr.container.state == Container::Queued || cr.container.state == Container::Locked
+            # If the child container hasn't started yet, finalize the
+            # child CR now instead of leaving it "on hold", i.e.,
+            # Queued with priority 0.  (OTOH, if the child is already
+            # running, leave it alone so it can get cancelled the
+            # usual way, get a copy of the log collection, etc.)
+            cr.update_attributes!(state: ContainerRequest::Final)
+          end
         end
       end
     end
diff --git a/services/api/app/models/container_request.rb b/services/api/app/models/container_request.rb
index 3596bf3..0b18914 100644
--- a/services/api/app/models/container_request.rb
+++ b/services/api/app/models/container_request.rb
@@ -10,6 +10,8 @@ class ContainerRequest < ArvadosModel
   include CommonApiTemplate
   include WhitelistUpdate
 
+  belongs_to :container, foreign_key: :container_uuid, primary_key: :uuid
+
   serialize :properties, Hash
   serialize :environment, Hash
   serialize :mounts, Hash
diff --git a/services/api/test/unit/container_test.rb b/services/api/test/unit/container_test.rb
index ed86bef..0e13ee9 100644
--- a/services/api/test/unit/container_test.rb
+++ b/services/api/test/unit/container_test.rb
@@ -473,10 +473,12 @@ class ContainerTest < ActiveSupport::TestCase
   end
 
   test "Container queued cancel" do
-    c, _ = minimal_new
+    c, cr = minimal_new({container_count_max: 1})
     set_user_from_auth :dispatch1
     assert c.update_attributes(state: Container::Cancelled), show_errors(c)
     check_no_change_from_cancelled c
+    cr.reload
+    assert_equal ContainerRequest::Final, cr.state
   end
 
   test "Container queued count" do

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list