Project

General

Profile

Actions

Feature #20319

closed

Move containers/{uuid}/log/ to container_requests/{uuid}/log/

Added by Tom Clegg about 1 year ago. Updated 8 months ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
API
Story points:
2.0
Release relationship:
Auto

Files

20319-doc.png (131 KB) 20319-doc.png Tom Clegg, 04/21/2023 05:55 PM

Subtasks 1 (0 open1 closed)

Task #20331: Review 20319-container-request-logsResolvedTom Clegg04/21/2023Actions

Related issues

Related to Arvados Epics - Idea #16442: Scalable + reliable container loggingResolved03/15/202308/31/2023Actions
Related to Arvados - Feature #19889: access current container logs at /arvados/v1/containers/{uuid}/log/{filename}ResolvedTom Clegg03/23/2023Actions
Actions #1

Updated by Tom Clegg about 1 year ago

  • Related to Idea #16442: Scalable + reliable container logging added
Actions #2

Updated by Tom Clegg about 1 year ago

  • Related to Feature #19889: access current container logs at /arvados/v1/containers/{uuid}/log/{filename} added
Actions #3

Updated by Tom Clegg about 1 year ago

  • Target version changed from Development 2023-04-12 sprint to Development 2023-04-26 sprint
Actions #4

Updated by Tom Clegg about 1 year ago

in progress...

20319-container-request-logs @ 5cc1710b57f98905469225c68d975ad2e3e7e56d -- developer-run-tests: #3614

Actions #5

Updated by Tom Clegg about 1 year ago

This move is convenient in that underneath /arvados/v1/$container_request_uuid/log/ we can use the same "subdirectory per container" layout that we use in the container request log collection.

However this does leave us with a couple of complications.

First: The CR log collection has subdirectories named "log for container $c_uuid", not just $c_uuid. We could:
  • Make the new API /arvados/v1/$container_request_uuid/log/log for $container_uuid/, which seems a bit awkward
  • Change the container request log layout so the subdirectory is just called $c_uuid, which is a little less human-friendly and would probably break some scripts
  • Make the new API /arvados/v1/$container_request_uuid/log/$container_uuid/, and leave the CR collection layout alone, which is potentially a little confusing / more complicated for clients that use both this API and the regular WebDAV endpoint

For now I've done the last option. One reason is that I don't think the layout can ever match perfectly anyway: the top-level log files in the CR log collection ("logs for the last container that ran") don't really have a counterpart here. (If we offered "logs for the current/last container" at the top level, I think we would just be inviting weird behavior when a client doesn't notice the files shrinking/rewriting during a cancel/retry cycle.)

Second: Producing a full directory listing at .../log/ is complicated by the fact that part of the tree comes from the container gateway (the current container) and the rest comes from keep-web. Currently I've avoided this by noting in the docs that the current (running) container's log subdir doesn't show up in directory listings. But we could:
  • make it show up in a depth=1 traversal (intercept PROPFIND and serve it in controller by getting the log collection, splicing an empty dir into it, and putting a webdav handler on that)
  • make its content show up in a depth>1 traversal (as above, but also fetch a dir tree from the container gateway, and splice that on)

Both options (especially the second) seem like a lot of trouble, maybe more trouble than it's worth. Practically speaking, "get container_uuid from the container_request record and stick that in the path" doesn't seem all that painful. After all, clients already need special behavior for the running container's logs because they're getting updated on the fly.

20319-container-request-logs @ 92bf76b1077282126b8a1c446458c6618a36e5d5 -- developer-run-tests: #3615

retry keep-balance developer-run-tests-remainder: #3801

Actions #6

Updated by Tom Clegg about 1 year ago

snapshot of doc update in this branch:

Actions #7

Updated by Lucas Di Pentima 12 months ago

This LGTM, thanks!

Actions #8

Updated by Tom Clegg 12 months ago

  • % Done changed from 0 to 100
  • Status changed from In Progress to Resolved
Actions #9

Updated by Peter Amstutz 8 months ago

  • Release set to 66
Actions

Also available in: Atom PDF