[arvados] created: 2.1.0-2618-g40dc04d6f

git repository hosting git at public.arvados.org
Fri Jun 24 14:05:46 UTC 2022


        at  40dc04d6fa45b6b727e94986491c06132db4a582 (commit)


commit 40dc04d6fa45b6b727e94986491c06132db4a582
Author: Tom Clegg <tom at curii.com>
Date:   Fri Jun 24 10:05:17 2022 -0400

    18948: Update test.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/services/api/test/unit/container_test.rb b/services/api/test/unit/container_test.rb
index 685bd15e6..bcf99da2e 100644
--- a/services/api/test/unit/container_test.rb
+++ b/services/api/test/unit/container_test.rb
@@ -870,16 +870,14 @@ class ContainerTest < ActiveSupport::TestCase
     end
   end
 
-  test "Container only set exit code on complete" do
+  test "can only change exit code while running and at completion" do
     set_user_from_auth :active
     c, _ = minimal_new
     set_user_from_auth :dispatch1
     c.lock
+    check_illegal_updates c, [{exit_code: 1}]
     c.update_attributes! state: Container::Running
-
-    check_illegal_updates c, [{exit_code: 1},
-                              {exit_code: 1, state: Container::Cancelled}]
-
+    assert c.update_attributes(exit_code: 1)
     assert c.update_attributes(exit_code: 1, state: Container::Complete)
   end
 

commit 1f0da8d97cfb16e478d5dfed896e86112eb82120
Author: Tom Clegg <tom at curii.com>
Date:   Thu Jun 23 14:26:28 2022 -0400

    18948: Update container record exit_code as soon as it is known.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/doc/api/methods/containers.html.textile.liquid b/doc/api/methods/containers.html.textile.liquid
index 76e5730c9..43163c555 100644
--- a/doc/api/methods/containers.html.textile.liquid
+++ b/doc/api/methods/containers.html.textile.liquid
@@ -52,8 +52,8 @@ Generally this will contain additional keys that are not present in any correspo
 |output|string|Portable data hash of the output collection.|Null if the container is not yet finished.|
 |container_image|string|Portable data hash of a collection containing the docker image used to run the container.||
 |progress|number|A number between 0.0 and 1.0 describing the fraction of work done.||
-|priority|integer|Range 0-1000.  Indicate scheduling order preference.|Currently assigned by the system as the max() of the priorities of all associated ContainerRequests.  See "container request priority":container_requests.html#priority .|
-|exit_code|integer|Process exit code.|Null if state!="Complete"|
+|priority|integer|Range 0-1000.  Indicate scheduling order preference.|Currently assigned by the system as the max() of the priorities of all associated ContainerRequests.  See "container request priority":container_requests.html#priority.|
+|exit_code|integer|Process exit code.|Null if container process has not exited yet.|
 |auth_uuid|string|UUID of a token to be passed into the container itself, used to access Keep-backed mounts, etc.  Automatically assigned.|Null if state∉{"Locked","Running"} or if @runtime_token@ was provided.|
 |locked_by_uuid|string|UUID of a token, indicating which dispatch process changed state to Locked. If null, any token can be used to lock. If not null, only the indicated token can modify this container.|Null if state∉{"Locked","Running"}|
 |runtime_token|string|A v2 token to be passed into the container itself, used to access Keep-backed mounts, etc.|Not returned in API responses.  Reset to null when state is "Complete" or "Cancelled".|
diff --git a/lib/crunchrun/crunchrun.go b/lib/crunchrun/crunchrun.go
index 30871e734..0e86f604a 100644
--- a/lib/crunchrun/crunchrun.go
+++ b/lib/crunchrun/crunchrun.go
@@ -1095,6 +1095,12 @@ func (runner *ContainerRunner) WaitFinish() error {
 		}
 	}
 	runner.CrunchLog.Printf("Container exited with status code %d%s", exitcode, extra)
+	err = runner.DispatcherArvClient.Update("containers", runner.Container.UUID, arvadosclient.Dict{
+		"container": arvadosclient.Dict{"exit_code": exitcode},
+	}, nil)
+	if err != nil {
+		runner.CrunchLog.Printf("ignoring error updating exit_code: %s", err)
+	}
 
 	var returnErr error
 	if err = runner.executorStdin.Close(); err != nil {
@@ -1162,10 +1168,9 @@ func (runner *ContainerRunner) updateLogs() {
 			continue
 		}
 
-		var updated arvados.Container
 		err = runner.DispatcherArvClient.Update("containers", runner.Container.UUID, arvadosclient.Dict{
 			"container": arvadosclient.Dict{"log": saved.PortableDataHash},
-		}, &updated)
+		}, nil)
 		if err != nil {
 			runner.CrunchLog.Printf("error updating container log to %s: %s", saved.PortableDataHash, err)
 			continue
@@ -1443,13 +1448,13 @@ func (runner *ContainerRunner) UpdateContainerFinal() error {
 	if runner.LogsPDH != nil {
 		update["log"] = *runner.LogsPDH
 	}
-	if runner.finalState == "Complete" {
-		if runner.ExitCode != nil {
-			update["exit_code"] = *runner.ExitCode
-		}
-		if runner.OutputPDH != nil {
-			update["output"] = *runner.OutputPDH
-		}
+	if runner.ExitCode != nil {
+		update["exit_code"] = *runner.ExitCode
+	} else {
+		update["exit_code"] = nil
+	}
+	if runner.finalState == "Complete" && runner.OutputPDH != nil {
+		update["output"] = *runner.OutputPDH
 	}
 	return runner.DispatcherArvClient.Update("containers", runner.Container.UUID, arvadosclient.Dict{"container": update}, nil)
 }
diff --git a/lib/crunchrun/crunchrun_test.go b/lib/crunchrun/crunchrun_test.go
index 997175789..76289b951 100644
--- a/lib/crunchrun/crunchrun_test.go
+++ b/lib/crunchrun/crunchrun_test.go
@@ -276,7 +276,7 @@ func (client *ArvTestClient) Update(resourceType string, uuid string, parameters
 		if parameters["container"].(arvadosclient.Dict)["state"] == "Running" {
 			client.WasSetRunning = true
 		}
-	} else if resourceType == "collections" {
+	} else if resourceType == "collections" && output != nil {
 		mt := parameters["collection"].(arvadosclient.Dict)["manifest_text"].(string)
 		output.(*arvados.Collection).UUID = uuid
 		output.(*arvados.Collection).PortableDataHash = fmt.Sprintf("%x", md5.Sum([]byte(mt)))
diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index 3a04c5604..08f87bbdb 100644
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -478,8 +478,8 @@ class Container < ArvadosModel
 
   def validate_change
     permitted = [:state]
-    progress_attrs = [:progress, :runtime_status, :log, :output, :output_properties]
-    final_attrs = [:exit_code, :finished_at]
+    progress_attrs = [:progress, :runtime_status, :log, :output, :output_properties, :exit_code]
+    final_attrs = [:finished_at]
 
     if self.new_record?
       permitted.push(:owner_uuid, :command, :container_image, :cwd,
diff --git a/services/api/test/unit/container_test.rb b/services/api/test/unit/container_test.rb
index ac3e6bea4..685bd15e6 100644
--- a/services/api/test/unit/container_test.rb
+++ b/services/api/test/unit/container_test.rb
@@ -933,7 +933,7 @@ class ContainerTest < ActiveSupport::TestCase
   end
 
   ["auth_uuid", "runtime_token"].each do |tok|
-    test "#{tok} can set output, progress, runtime_status, state on running container -- but not log" do
+    test "#{tok} can set output, progress, runtime_status, state, exit_code on running container -- but not log" do
       if tok == "runtime_token"
         set_user_from_auth :spectator
         c, _ = minimal_new(container_image: "9ae44d5792468c58bcf85ce7353c7027+124",
@@ -963,6 +963,7 @@ class ContainerTest < ActiveSupport::TestCase
       assert c.update_attributes(output: collections(:collection_owned_by_active).portable_data_hash)
       assert c.update_attributes(runtime_status: {'warning' => 'something happened'})
       assert c.update_attributes(progress: 0.5)
+      assert c.update_attributes(exit_code: 0)
       refute c.update_attributes(log: collections(:real_log_collection).portable_data_hash)
       c.reload
       assert c.update_attributes(state: Container::Complete, exit_code: 0)

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list