[arvados] updated: 2.5.0-13-g6477db8a1

git repository hosting git at public.arvados.org
Wed Jan 18 19:54:16 UTC 2023


Summary of changes:
 .../methods/container_requests.html.textile.liquid |  2 +-
 doc/api/methods/containers.html.textile.liquid     |  2 +-
 lib/crunchrun/crunchrun.go                         | 30 ++++++++++++++--------
 lib/crunchrun/crunchrun_test.go                    | 17 +++++++-----
 4 files changed, 33 insertions(+), 18 deletions(-)

       via  6477db8a1f5ed0b2f81cf743bbea32c681c7c10c (commit)
       via  5d6589c1674133460e1bf39d19dbfc6fd075e498 (commit)
       via  6345bcd48d437dfd7f6b1d0ed706985f9c08fde1 (commit)
       via  5ea5c3a5e509f32d607899c5584a5c811b8e2cec (commit)
       via  e6fe9c21160d4bbd16b5fac2c95a68bf987a6152 (commit)
      from  499848b567ebecadbdff1fcbbdb4683dfbf097da (commit)

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 6477db8a1f5ed0b2f81cf743bbea32c681c7c10c
Author: Brett Smith <brett.smith at curii.com>
Date:   Wed Jan 18 14:51:05 2023 -0500

    19886: Update API documentation notes about container request logs
    
    The existing note was already out-of-date: crunch-run would record logs
    about 30 minutes after starting the container. With recent changes,
    crunch-run now records logs shortly after starting the container. Update
    the note accordingly.
    
    Arvados-DCO-1.1-Signed-off-by: Brett Smith <brett.smith at curii.com>

diff --git a/doc/api/methods/container_requests.html.textile.liquid b/doc/api/methods/container_requests.html.textile.liquid
index 11f4f34fc..d694ec778 100644
--- a/doc/api/methods/container_requests.html.textile.liquid
+++ b/doc/api/methods/container_requests.html.textile.liquid
@@ -54,7 +54,7 @@ table(table table-bordered table-condensed).
 |priority|integer|Range 0-1000.  Indicate scheduling order preference.|Clients are expected to submit container requests with zero priority in order to preview the container that will be used to satisfy it. Priority can be null if and only if state!="Committed".  See "below for more details":#priority .|
 |expires_at|datetime|After this time, priority is considered to be zero.|Not yet implemented.|
 |use_existing|boolean|If possible, use an existing (non-failed) container to satisfy the request instead of creating a new one.|Default is true|
-|log_uuid|string|Log collection containing log messages provided by the scheduler and crunch processes.|Null if the container has not yet completed.|
+|log_uuid|string|Log collection containing log messages provided by the scheduler and crunch processes.|Null if the container has not yet started running.|
 |output_uuid|string|Output collection created when the container finished successfully.|Null if the container has failed or not yet completed.|
 |filters|string|Additional constraints for satisfying the container_request, given in the same form as the filters parameter accepted by the container_requests.list API.|
 |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".|

commit 5d6589c1674133460e1bf39d19dbfc6fd075e498
Author: Brett Smith <brett.smith at curii.com>
Date:   Wed Jan 18 14:44:55 2023 -0500

    19886: Correct API documentation notes about container logs
    
    The documented limitation does not exist. In fact, there's a limitation
    in the API server that if you want a container's logs propagated to its
    associated container request(s), you *must* pass in a portable data
    hash. crunch-run consistently updates container records with a portable
    data hash in the log field for this reason. Update the note to reflect
    this.
    
    Arvados-DCO-1.1-Signed-off-by: Brett Smith <brett.smith at curii.com>

diff --git a/doc/api/methods/containers.html.textile.liquid b/doc/api/methods/containers.html.textile.liquid
index 6117823d4..d3eb7445b 100644
--- a/doc/api/methods/containers.html.textile.liquid
+++ b/doc/api/methods/containers.html.textile.liquid
@@ -28,7 +28,7 @@ table(table table-bordered table-condensed).
 |state|string|The allowed states are "Queued", "Locked", "Running", "Cancelled" and "Complete".|See "Container states":#container_states for more details.|
 |started_at|datetime|When this container started running.|Null if container has not yet started.|
 |finished_at|datetime|When this container finished.|Null if container has not yet finished.|
-|log|string|UUID or portable data hash of a collection containing the log messages produced when executing the container.|PDH after the container is finished, otherwise UUID or null.|
+|log|string|UUID or portable data hash of a collection containing the log messages produced when executing the container.|If a running container is updated with a portable data hash in its log field, that collection will be propagated to the logs of associated container request(s). This behaivor may be extended to UUID updates in the future.|
 |environment|hash|Environment variables and values that should be set in the container environment (@docker run --env@). This augments and (when conflicts exist) overrides environment variables given in the image's Dockerfile.|Must be equal to a ContainerRequest's environment in order to satisfy the ContainerRequest.|
 |cwd|string|Initial working directory.|Must be equal to a ContainerRequest's cwd in order to satisfy the ContainerRequest|
 |command|array of strings|Command to execute.| Must be equal to a ContainerRequest's command in order to satisfy the ContainerRequest.|

commit 6345bcd48d437dfd7f6b1d0ed706985f9c08fde1
Author: Brett Smith <brett.smith at curii.com>
Date:   Wed Jan 18 14:35:58 2023 -0500

    19886: crunch-run records initial log with PDH
    
    The API server will only propagate a container's log collection to
    container requests when it is specified with a portable data hash. See
    the top of ContainerRequest#update_collections.
    
    Arvados-DCO-1.1-Signed-off-by: Brett Smith <brett.smith at curii.com>

diff --git a/lib/crunchrun/crunchrun.go b/lib/crunchrun/crunchrun.go
index d322a3ac3..6384f6a1c 100644
--- a/lib/crunchrun/crunchrun.go
+++ b/lib/crunchrun/crunchrun.go
@@ -1427,18 +1427,25 @@ func (runner *ContainerRunner) saveLogCollection(final bool) (response arvados.C
 }
 
 // UpdateContainerRunning updates the container state to "Running"
-func (runner *ContainerRunner) UpdateContainerRunning() error {
+func (runner *ContainerRunner) UpdateContainerRunning(logId string) error {
 	runner.cStateLock.Lock()
 	defer runner.cStateLock.Unlock()
 	if runner.cCancelled {
 		return ErrCancelled
 	}
-	return runner.DispatcherArvClient.Update("containers", runner.Container.UUID,
-		arvadosclient.Dict{"container": arvadosclient.Dict{
-			"state":           "Running",
-			"gateway_address": runner.gateway.Address,
-			"log":             runner.logUUID,
-		}}, nil)
+	updates := arvadosclient.Dict{
+		"gateway_address": runner.gateway.Address,
+		"state":           "Running",
+	}
+	if logId != "" {
+		updates["log"] = logId
+	}
+	return runner.DispatcherArvClient.Update(
+		"containers",
+		runner.Container.UUID,
+		arvadosclient.Dict{"container": updates},
+		nil,
+	)
 }
 
 // ContainerToken returns the api_token the container (and any
@@ -1629,11 +1636,14 @@ func (runner *ContainerRunner) Run() (err error) {
 		return
 	}
 
-	_, err = runner.saveLogCollection(false)
-	if err != nil {
+	logCollection, err := runner.saveLogCollection(false)
+	var logId string
+	if err == nil {
+		logId = logCollection.PortableDataHash
+	} else {
 		runner.CrunchLog.Printf("Error committing initial log collection: %v", err)
 	}
-	err = runner.UpdateContainerRunning()
+	err = runner.UpdateContainerRunning(logId)
 	if err != nil {
 		return
 	}
diff --git a/lib/crunchrun/crunchrun_test.go b/lib/crunchrun/crunchrun_test.go
index 1997d2153..fdb6338cb 100644
--- a/lib/crunchrun/crunchrun_test.go
+++ b/lib/crunchrun/crunchrun_test.go
@@ -595,7 +595,7 @@ func (s *TestSuite) TestUpdateContainerRunning(c *C) {
 	cr, err := NewContainerRunner(s.client, api, kc, "zzzzz-zzzzz-zzzzzzzzzzzzzzz")
 	c.Assert(err, IsNil)
 
-	err = cr.UpdateContainerRunning()
+	err = cr.UpdateContainerRunning("")
 	c.Check(err, IsNil)
 
 	c.Check(api.Content[0]["container"].(arvadosclient.Dict)["state"], Equals, "Running")
@@ -925,7 +925,11 @@ func (s *TestSuite) TestCommitNodeInfoBeforeStart(c *C) {
 	c.Check(manifest_text, Matches, `\. .+ \d+:\d{2,}:node\.json( .+)?\n`)
 
 	c.Assert(container_update, NotNil)
-	c.Check(container_update["container"].(arvadosclient.Dict)["log"], Matches, `zzzzz-4zz18-[0-9a-z]{15}`)
+	// As of Arvados 2.5.0, the container update must specify its log in PDH
+	// format for the API server to propagate it to container requests, which
+	// is what we care about for this test.
+	expect_pdh := fmt.Sprintf("%x+%d", md5.Sum([]byte(manifest_text)), len(manifest_text))
+	c.Check(container_update["container"].(arvadosclient.Dict)["log"], Equals, expect_pdh)
 }
 
 func (s *TestSuite) TestContainerRecordLog(c *C) {

commit 5ea5c3a5e509f32d607899c5584a5c811b8e2cec
Author: Brett Smith <brett.smith at curii.com>
Date:   Wed Jan 18 14:34:00 2023 -0500

    19886: Tighten test regexps for better validation
    
    Arvados-DCO-1.1-Signed-off-by: Brett Smith <brett.smith at curii.com>

diff --git a/lib/crunchrun/crunchrun_test.go b/lib/crunchrun/crunchrun_test.go
index 08bfcdfec..1997d2153 100644
--- a/lib/crunchrun/crunchrun_test.go
+++ b/lib/crunchrun/crunchrun_test.go
@@ -921,8 +921,8 @@ func (s *TestSuite) TestCommitNodeInfoBeforeStart(c *C) {
 	manifest_text := log_collection["manifest_text"].(string)
 	// We check that the file size is at least two digits as an easy way to
 	// check the file isn't empty.
-	c.Check(manifest_text, Matches, `\. .* \d+:\d{2,}:node-info\.txt .*\n`)
-	c.Check(manifest_text, Matches, `\. .* \d+:\d{2,}:node\.json .*\n`)
+	c.Check(manifest_text, Matches, `\. .+ \d+:\d{2,}:node-info\.txt( .+)?\n`)
+	c.Check(manifest_text, Matches, `\. .+ \d+:\d{2,}:node\.json( .+)?\n`)
 
 	c.Assert(container_update, NotNil)
 	c.Check(container_update["container"].(arvadosclient.Dict)["log"], Matches, `zzzzz-4zz18-[0-9a-z]{15}`)

commit e6fe9c21160d4bbd16b5fac2c95a68bf987a6152
Author: Brett Smith <brett.smith at curii.com>
Date:   Wed Jan 18 14:33:41 2023 -0500

    19886: DRY up test code for better readability
    
    Arvados-DCO-1.1-Signed-off-by: Brett Smith <brett.smith at curii.com>

diff --git a/lib/crunchrun/crunchrun_test.go b/lib/crunchrun/crunchrun_test.go
index 20ce8c647..08bfcdfec 100644
--- a/lib/crunchrun/crunchrun_test.go
+++ b/lib/crunchrun/crunchrun_test.go
@@ -916,8 +916,9 @@ func (s *TestSuite) TestCommitNodeInfoBeforeStart(c *C) {
 		})
 
 	c.Assert(collection_create, NotNil)
-	c.Check(collection_create["collection"].(arvadosclient.Dict)["name"], Equals, "logs for zzzzz-dz642-202301121543210")
-	manifest_text := collection_create["collection"].(arvadosclient.Dict)["manifest_text"]
+	log_collection := collection_create["collection"].(arvadosclient.Dict)
+	c.Check(log_collection["name"], Equals, "logs for zzzzz-dz642-202301121543210")
+	manifest_text := log_collection["manifest_text"].(string)
 	// We check that the file size is at least two digits as an easy way to
 	// check the file isn't empty.
 	c.Check(manifest_text, Matches, `\. .* \d+:\d{2,}:node-info\.txt .*\n`)

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list