https://dev.arvados.org/https://dev.arvados.org/favicon.ico?15576888422016-01-20T17:35:05ZArvadosArvados - Feature #8015: [Crunch2] Keep-backed mount points: tmp/output dir, read-only collection, read-only by-id dirhttps://dev.arvados.org/issues/8015?journal_id=344952016-01-20T17:35:05ZPeter Amstutzpeter.amstutz@curii.com
<ul><li><strong>Description</strong> updated (<a title="View differences" href="/journals/34495/diff?detail_id=33869">diff</a>)</li></ul> Arvados - Feature #8015: [Crunch2] Keep-backed mount points: tmp/output dir, read-only collection, read-only by-id dirhttps://dev.arvados.org/issues/8015?journal_id=346742016-01-25T22:20:03ZTom Cleggtom@curii.com
<ul><li><strong>Description</strong> updated (<a title="View differences" href="/journals/34674/diff?detail_id=34039">diff</a>)</li></ul> Arvados - Feature #8015: [Crunch2] Keep-backed mount points: tmp/output dir, read-only collection, read-only by-id dirhttps://dev.arvados.org/issues/8015?journal_id=347272016-01-27T14:56:50ZTom Cleggtom@curii.com
<ul><li><strong>Subject</strong> changed from <i>[Crunch2] Set up mount points</i> to <i>[Crunch2] Keep-backed mount points: tmp/output dir, read-only collection, read-only by-id dir</i></li></ul> Arvados - Feature #8015: [Crunch2] Keep-backed mount points: tmp/output dir, read-only collection, read-only by-id dirhttps://dev.arvados.org/issues/8015?journal_id=347292016-01-27T15:10:07ZTom Cleggtom@curii.com
<ul><li><strong>Description</strong> updated (<a title="View differences" href="/journals/34729/diff?detail_id=34089">diff</a>)</li><li><strong>Category</strong> set to <i>Crunch</i></li></ul> Arvados - Feature #8015: [Crunch2] Keep-backed mount points: tmp/output dir, read-only collection, read-only by-id dirhttps://dev.arvados.org/issues/8015?journal_id=347322016-01-27T18:20:48ZBrett Smithbrett.smith@curii.com
<ul><li><strong>Story points</strong> set to <i>2.0</i></li></ul> Arvados - Feature #8015: [Crunch2] Keep-backed mount points: tmp/output dir, read-only collection, read-only by-id dirhttps://dev.arvados.org/issues/8015?journal_id=348902016-02-02T19:58:32ZTom Cleggtom@curii.com
<ul><li><strong>Target version</strong> set to <i>2016-02-17 Sprint</i></li></ul> Arvados - Feature #8015: [Crunch2] Keep-backed mount points: tmp/output dir, read-only collection, read-only by-id dirhttps://dev.arvados.org/issues/8015?journal_id=349702016-02-03T20:17:28ZPeter Amstutzpeter.amstutz@curii.com
<ul><li><strong>Assigned To</strong> set to <i>Peter Amstutz</i></li></ul> Arvados - Feature #8015: [Crunch2] Keep-backed mount points: tmp/output dir, read-only collection, read-only by-id dirhttps://dev.arvados.org/issues/8015?journal_id=351802016-02-12T03:15:08ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><p>Note that <code>kind=collection, writable=false, uuid=X</code> will make the job impure, because the collection could change. I thought we were trying to avoid that sort of thing?</p> Arvados - Feature #8015: [Crunch2] Keep-backed mount points: tmp/output dir, read-only collection, read-only by-id dirhttps://dev.arvados.org/issues/8015?journal_id=352592016-02-16T16:23:51ZBrett Smithbrett.smith@curii.com
<ul><li><strong>Target version</strong> deleted (<del><i>2016-02-17 Sprint</i></del>)</li><li><strong>Assigned To</strong> deleted (<del><i>Peter Amstutz</i></del>)</li></ul> Arvados - Feature #8015: [Crunch2] Keep-backed mount points: tmp/output dir, read-only collection, read-only by-id dirhttps://dev.arvados.org/issues/8015?journal_id=352752016-02-16T18:30:18ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><p>This is ready for review on @ 8015-crunch2-mount</p> Arvados - Feature #8015: [Crunch2] Keep-backed mount points: tmp/output dir, read-only collection, read-only by-id dirhttps://dev.arvados.org/issues/8015?journal_id=352792016-02-16T18:32:28ZBrett Smithbrett.smith@curii.com
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>In Progress</i></li><li><strong>Assigned To</strong> set to <i>Peter Amstutz</i></li><li><strong>Target version</strong> set to <i>2016-02-17 Sprint</i></li></ul> Arvados - Feature #8015: [Crunch2] Keep-backed mount points: tmp/output dir, read-only collection, read-only by-id dirhttps://dev.arvados.org/issues/8015?journal_id=353142016-02-16T20:56:45ZTom Cleggtom@curii.com
<ul></ul><p>Peter Amstutz wrote:</p>
<blockquote>
<p>Note that <code>kind=collection, writable=false, uuid=X</code> will make the job impure, because the collection could change. I thought we were trying to avoid that sort of thing?</p>
</blockquote>
We want to
<ul>
<li>permit impure containers</li>
<li>make it harder to <em>accidentally</em> and <em>unknowingly</em> run impure containers</li>
</ul>
<p>Use of this kind of mount is an example of how clients like Workbench (and job re-use features) can detect/predict/guess that a container is impure. But I don't think crunch-run needs to care one way or the other...</p> Arvados - Feature #8015: [Crunch2] Keep-backed mount points: tmp/output dir, read-only collection, read-only by-id dirhttps://dev.arvados.org/issues/8015?journal_id=354052016-02-17T20:13:26ZBrett Smithbrett.smith@curii.com
<ul><li><strong>Target version</strong> changed from <i>2016-02-17 Sprint</i> to <i>2016-03-02 sprint</i></li><li><strong>Story points</strong> changed from <i>2.0</i> to <i>0.5</i></li></ul> Arvados - Feature #8015: [Crunch2] Keep-backed mount points: tmp/output dir, read-only collection, read-only by-id dirhttps://dev.arvados.org/issues/8015?journal_id=354582016-02-17T21:40:31ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><p>A couple notes:</p>
<p>This branch ended up bringing <a class="issue tracker-2 status-3 priority-4 priority-default closed" title="Feature: [Crunch2] Save output of container execution to collection (Resolved)" href="https://dev.arvados.org/issues/8020">#8020</a> (handle outputs) along for the ride, so some of the bulk is from adding the ability to upload results (in addition to using writable keep). The uploading code was adapted from the existing crunchrunner code so it's not completely new.</p>
<p>I after the actual arv-mount handling, most of the rest of the changes are from general improvements/refactoring around logging. I went to the Docker API and figured how to correctly handle stdout/stderr multiplexed on the single stream.</p> Arvados - Feature #8015: [Crunch2] Keep-backed mount points: tmp/output dir, read-only collection, read-only by-id dirhttps://dev.arvados.org/issues/8015?journal_id=359142016-02-29T19:41:42ZRadhika Chippadaradhika@curoverse.com
<ul></ul><p>crunchrun.go</p>
<ul>
<li>It does not appear that SetupSignals would return any non-nil error?</li>
</ul>
<ul>
<li>Should runner.CrunchLog.Printf be using closeerr instead of readerr in the following? (one more like this in the next if statement as well)<br /> if closeerr != nil {<br /> runner.CrunchLog.Printf("While closing stdout logs: %v", readerr)<br /> }</li>
</ul>
<ul>
<li>AttachStreams comment says “AttachLogs”</li>
</ul>
<ul>
<li>Line 644 (CrunchLog.Close()): Would this result in writing the log (with any errors in steps 8 and 9) to keep again ? Would this result in changes to collection such as pdh?</li>
</ul>
<ul>
<li>Line 650: Should this be “err = waiterr” instead of runerr?</li>
</ul>
<p>upload.go</p>
<ul>
<li>func ReadFrom reads from reader and writes to keep. Should it be better named to reflect this? ReadAndWrite or something like that</li>
</ul>
<ul>
<li>The comment “CollectionWriter makes implements creating new Keep collections” seems like too many verbs makes and implements</li>
</ul>
<ul>
<li>6 tests are failing (it could be my env; if they are passing for you, you can ignore this comment)</li>
</ul> Arvados - Feature #8015: [Crunch2] Keep-backed mount points: tmp/output dir, read-only collection, read-only by-id dirhttps://dev.arvados.org/issues/8015?journal_id=359952016-03-01T18:46:15ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><p>Radhika Chippada wrote:</p>
<blockquote>
<p>crunchrun.go</p>
<ul>
<li>It does not appear that SetupSignals would return any non-nil error?</li>
</ul>
</blockquote>
<p>Fixed to not return anything now (because signal.Notify() doesn't return an error either).</p>
<blockquote>
<ul>
<li>Should runner.CrunchLog.Printf be using closeerr instead of readerr in the following? (one more like this in the next if statement as well)<br />if closeerr != nil {<br />runner.CrunchLog.Printf("While closing stdout logs: %v", readerr)<br />}</li>
</ul>
</blockquote>
<p>Fixed.</p>
<blockquote>
<ul>
<li>AttachStreams comment says “AttachLogs”</li>
</ul>
</blockquote>
<p>Fixed. Also fixed error handling around that code path.</p>
<blockquote>
<ul>
<li>Line 644 (CrunchLog.Close()): Would this result in writing the log (with any errors in steps 8 and 9) to keep again ? Would this result in changes to collection such as pdh?</li>
</ul>
</blockquote>
<p>The code is correct, after committing to keep in CommitLogs() it creates a new logger (so that any late shutdown errors can be logged to the API server) but without a CollectionFileWriter attached.</p>
<blockquote>
<ul>
<li>Line 650: Should this be “err = waiterr” instead of runerr?</li>
</ul>
</blockquote>
<p>Fixed.</p>
<blockquote>
<p>upload.go</p>
<ul>
<li>func ReadFrom reads from reader and writes to keep. Should it be better named to reflect this? ReadAndWrite or something like that</li>
</ul>
</blockquote>
<p>It's named that way to fulfill the "ReaderFrom" interface (<a class="external" href="https://golang.org/pkg/io/">https://golang.org/pkg/io/</a>)</p>
<blockquote>
<ul>
<li>The comment “CollectionWriter makes implements creating new Keep collections” seems like too many verbs makes and implements</li>
</ul>
</blockquote>
<p>Fixed.</p>
<blockquote>
<ul>
<li>6 tests are failing (it could be my env; if they are passing for you, you can ignore this comment)</li>
</ul>
</blockquote>
<p>With <code>arvbox restart test --only services/crunch-run</code> I get 1 error. Looking into it.</p> Arvados - Feature #8015: [Crunch2] Keep-backed mount points: tmp/output dir, read-only collection, read-only by-id dirhttps://dev.arvados.org/issues/8015?journal_id=360322016-03-01T20:51:52ZRadhika Chippadaradhika@curoverse.com
<ul></ul><p>I am still not using arvbox. All tests passed for me. LGTM</p> Arvados - Feature #8015: [Crunch2] Keep-backed mount points: tmp/output dir, read-only collection, read-only by-id dirhttps://dev.arvados.org/issues/8015?journal_id=360332016-03-01T21:00:14ZPeter Amstutzpeter.amstutz@curii.com
<ul><li><strong>Status</strong> changed from <i>In Progress</i> to <i>Resolved</i></li><li><strong>% Done</strong> changed from <i>67</i> to <i>100</i></li></ul><p>Applied in changeset arvados|commit:0f8dc3d824f03b82b8db9f61bbcc0592b62b998f.</p>