[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