https://dev.arvados.org/https://dev.arvados.org/favicon.ico?15576888422019-03-20T15:15:05ZArvadosArvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=726422019-03-20T15:15:05ZTom Cleggtom@curii.com
<ul><li><strong>Target version</strong> set to <i>To Be Groomed</i></li></ul> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=726482019-03-20T15:35:10ZTom Cleggtom@curii.com
<ul><li><strong>Description</strong> updated (<a title="View differences" href="/journals/72648/diff?detail_id=69613">diff</a>)</li><li><strong>Story points</strong> set to <i>3.0</i></li></ul> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=726562019-03-20T17:29:43ZTom Cleggtom@curii.com
<ul><li><strong>Blocks</strong> <i><a class="issue tracker-6 status-3 priority-4 priority-default closed" href="/issues/13648">Idea #13648</a>: [Epic] Use one cluster configuration file for all components</i> added</li></ul> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=730092019-04-03T14:07:45ZTom Morristfmorris@veritasgenetics.com
<ul><li><strong>Target version</strong> changed from <i>To Be Groomed</i> to <i>Arvados Future Sprints</i></li></ul> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=732282019-04-10T16:17:05ZTom Morristfmorris@veritasgenetics.com
<ul><li><strong>Assigned To</strong> set to <i>Tom Clegg</i></li><li><strong>Target version</strong> changed from <i>Arvados Future Sprints</i> to <i>2019-04-24 Sprint</i></li></ul> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=733292019-04-16T15:08:07ZTom Cleggtom@curii.com
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>In Progress</i></li></ul> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=735932019-04-24T13:24:38ZTom Cleggtom@curii.com
<ul></ul>15003-preprocess-config @ <a class="changeset" title="15003: Update service names. Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg@veritasgenetics.com>" href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/cb13593decb097f501c0a1d64510a653b3233395">cb13593decb097f501c0a1d64510a653b3233395</a> <a class="external" href="https://ci.curoverse.com/view/Developer/job/developer-run-tests/1209/">https://ci.curoverse.com/view/Developer/job/developer-run-tests/1209/</a>
<ul>
<li>embeds the default config data in the arvados-server binary</li>
<li>adds "arvados-server config-dump" and "arvados-server config-check" commands</li>
</ul> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=736112019-04-24T15:30:39ZTom Cleggtom@curii.com
<ul><li><strong>Target version</strong> changed from <i>2019-04-24 Sprint</i> to <i>2019-05-08 Sprint</i></li></ul> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=738122019-04-26T17:51:57ZTom Cleggtom@curii.com
<ul></ul><p>merged master</p>
<p>15003-preprocess-config @ <a class="changeset" title="15003: Merge branch 'master' into 15003-preprocess-config Arvados-DCO-1.1-Signed-off-by: Tom Cle..." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/689901263ebfdd996da3711236615038e6245db3">689901263ebfdd996da3711236615038e6245db3</a> -- <a class="external" href="https://ci.curoverse.com/view/Developer/job/developer-run-tests/1218/">https://ci.curoverse.com/view/Developer/job/developer-run-tests/1218/</a></p> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=740432019-05-03T14:45:08ZLucas Di Pentimalucas.dipentima@curii.com
<ul></ul><p>This looks good AFAICT, the only thing I think it would be needed is documentation, both on the Admin site & the sub-commands' help text.</p> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=742312019-05-08T17:22:07ZTom Cleggtom@curii.com
<ul><li><strong>Target version</strong> changed from <i>2019-05-08 Sprint</i> to <i>2019-05-22 Sprint</i></li></ul> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=742462019-05-08T17:35:34ZTom Cleggtom@curii.com
<ul><li><strong>Related to</strong> <i><a class="issue tracker-1 status-5 priority-4 priority-default closed" href="/issues/10282">Bug #10282</a>: [Go programs]Warn or error when unrecognized keys appear in config files</i> added</li></ul> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=744362019-05-15T14:14:05ZTom Cleggtom@curii.com
<ul></ul><p>15003-preprocess-config @ <a class="changeset" title="15003: Merge branch 'master' into 15003-preprocess-config Arvados-DCO-1.1-Signed-off-by: Tom Cle..." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/42b483f3a722ad8a506040160e54d0080a197318">42b483f3a722ad8a506040160e54d0080a197318</a> -- <a class="external" href="https://ci.curoverse.com/view/Developer/job/developer-run-tests/1235/">https://ci.curoverse.com/view/Developer/job/developer-run-tests/1235/</a></p> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=747282019-05-22T17:03:35ZTom Cleggtom@curii.com
<ul><li><strong>Target version</strong> changed from <i>2019-05-22 Sprint</i> to <i>2019-06-05 Sprint</i></li></ul> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=747292019-05-22T17:15:27ZTom Cleggtom@curii.com
<ul><li><strong>Story points</strong> changed from <i>3.0</i> to <i>1.0</i></li></ul> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=747362019-05-22T17:19:41ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><p>Warn if Services → Workbench2 → ExternalURL is missing or empty.</p> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=748302019-05-23T14:45:47ZTom Cleggtom@curii.com
<ul></ul><p>15003-disable-config-warnings @ <a class="changeset" title="15003: Temporarily disable config warnings. Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg@ver..." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/311d552b31fbb97d10cdba7e248279d197751f79">311d552b31fbb97d10cdba7e248279d197751f79</a> -- <a class="external" href="https://ci.curoverse.com/view/Developer/job/developer-run-tests/1254/">https://ci.curoverse.com/view/Developer/job/developer-run-tests/1254/</a></p>
<p>Disables warnings until we fill in the rest of the default/template file.</p>
<p>(Jenkins tests were bitten by <a class="issue tracker-1 status-3 priority-4 priority-default closed parent" title="Bug: [API] Fix unreliable test (Resolved)" href="https://dev.arvados.org/issues/14878">#14878</a>)</p> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=748332019-05-23T15:05:38ZLucas Di Pentimalucas.dipentima@curii.com
<ul></ul><p>This LGTM, thanks!</p> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=748782019-05-24T19:34:24ZTom Cleggtom@curii.com
<ul></ul><p>in progress...</p>
<p>15003-new-config-struct @ <a class="changeset" title="15003: Remove NodeProfiles section from integration test config. Arvados-DCO-1.1-Signed-off-by: ..." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/d6358ef9fc0d8474827830a7ea0a451832e1fbec">d6358ef9fc0d8474827830a7ea0a451832e1fbec</a> -- <a class="external" href="https://ci.curoverse.com/view/Developer/job/developer-run-tests/1258/">https://ci.curoverse.com/view/Developer/job/developer-run-tests/1258/</a></p> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=750032019-05-30T15:40:21ZTom Cleggtom@curii.com
<ul></ul>15003-new-config-struct @ <a class="changeset" title="15003: Ignore SAMPLE entries in default config file. Arvados-DCO-1.1-Signed-off-by: Tom Clegg <t..." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/60560d144b7abf7ff98eb12ca3a591211df19ce9">60560d144b7abf7ff98eb12ca3a591211df19ce9</a> -- <a class="external" href="https://ci.curoverse.com/view/Developer/job/developer-run-tests/1266/">https://ci.curoverse.com/view/Developer/job/developer-run-tests/1266/</a>
<ul>
<li>Remove NodeProfiles from config struct/docs/examples (migration allows old configs to continue working)</li>
<li>Update dispatch configs to new config structure (mostly, CloudVMs.* and Dispatch.* move to Containers.CloudVMs.*)</li>
<li>Add SAMPLE entries to default config file so "unknown key" checks work on sections like RemoteClusters and InstanceTypes</li>
<li>Use the "service command" library for the health check aggregator (remove duplicated logging/httpserver stuff)</li>
</ul> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=750152019-05-30T20:02:29ZLucas Di Pentimalucas.dipentima@curii.com
<ul></ul><p>Just a couple of comments:</p>
<ul>
<li>Code comment at <code>sdk/go/arvados/config.go:L252</code> got outdated</li>
<li>Couldn’t the map from <code>lib/service/cmd.go:L148</code> be used from the <code>Map()</code> func at <code>sdk/go/arvados/config.go</code>?</li>
</ul>
<p>With that, it LGTM.</p> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=750422019-05-31T17:56:54ZTom Cleggtom@curii.com
<ul></ul><p>Ah yes, the lib/service map was a subset of Map(). Fixed both, thanks.</p> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=750832019-06-03T19:32:22ZTom Cleggtom@curii.com
<ul></ul><p>15003-duration-format @ <a class="changeset" title="15003: Format durations as "336h", not "336h0m0s". Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tcl..." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/6e73eff3926a2e7345333edd02531e8e6fbe15ef">6e73eff3926a2e7345333edd02531e8e6fbe15ef</a> -- <a class="external" href="https://ci.curoverse.com/view/Developer/job/developer-run-tests/1272/">https://ci.curoverse.com/view/Developer/job/developer-run-tests/1272/</a></p>
<p>Spell durations like "336h" instead of "336h0m0s" when dumping config.</p> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=750892019-06-03T20:15:40ZLucas Di Pentimalucas.dipentima@curii.com
<ul></ul><p>Interesting string replacing strategy :) LGTM, thanks.</p> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=751832019-06-05T15:00:47ZTom Cleggtom@curii.com
<ul><li><strong>Target version</strong> changed from <i>2019-06-05 Sprint</i> to <i>2019-06-19 Sprint</i></li></ul> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=752622019-06-05T21:10:43ZTom Cleggtom@curii.com
<ul></ul><p>On dev clusters, Services entries seem to be incorrectly flagged as deprecated/unknown:</p>
<pre>
deprecated or unknown config entry: Clusters.4xphq.Services.Controller.InternalURLs.http://localhost:9004
deprecated or unknown config entry: Clusters.4xphq.Services.DispatchCloud.InternalURLs.http://4xphq.arvadosapi.com:9006
deprecated or unknown config entry: Clusters.4xphq.Services.RailsAPI.InternalURLs.http://localhost:8000
</pre> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=753602019-06-10T14:15:00ZWard Vandewegeward@curii.com
<ul><li><strong>Release</strong> set to <i>22</i></li></ul> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=754892019-06-12T20:30:08ZTom Cleggtom@curii.com
<ul></ul>15003-config-default-location @ <a class="changeset" title="15003: Load config from default location or -config arg, not stdin. Arvados-DCO-1.1-Signed-off-b..." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/49ca4b9e9d96d35f704216bda401cc20cd972ca8">49ca4b9e9d96d35f704216bda401cc20cd972ca8</a> -- <a class="external" href="https://ci.curoverse.com/view/Developer/job/developer-run-tests/1302/">https://ci.curoverse.com/view/Developer/job/developer-run-tests/1302/</a>
<ul>
<li>Instead of stdin, load config from <code>/etc/arvados/config.yml</code> by default, or use <code>-config</code> arg to override.</li>
</ul> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=755162019-06-13T14:23:40ZLucas Di Pentimalucas.dipentima@curii.com
<ul></ul><p>15003-config-default-location LGTM, thanks!</p> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=756472019-06-19T15:09:19ZTom Cleggtom@curii.com
<ul><li><strong>Status</strong> changed from <i>In Progress</i> to <i>Resolved</i></li></ul> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=762112019-07-10T19:08:02ZTom Cleggtom@curii.com
<ul><li><strong>Status</strong> changed from <i>Resolved</i> to <i>In Progress</i></li><li><strong>Target version</strong> changed from <i>2019-06-19 Sprint</i> to <i>2019-07-17 Sprint</i></li></ul> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=762122019-07-10T19:11:03ZTom Cleggtom@curii.com
<ul></ul><p>15003-real-configs-flagged-unknown @ <a class="changeset" title="15003: Fix errant warnings on PostgreSQL params and InternalURLs. Arvados-DCO-1.1-Signed-off-by:..." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/54d059bfc6d84dbc20613ddd17d812d571e93eda">54d059bfc6d84dbc20613ddd17d812d571e93eda</a> -- <a class="external" href="https://ci.curoverse.com/view/Developer/job/developer-run-tests/1381/">https://ci.curoverse.com/view/Developer/job/developer-run-tests/1381/</a></p> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=762182019-07-10T20:56:26ZLucas Di Pentimalucas.dipentima@curii.com
<ul></ul><p>I believe there're at least a couple more places on the defaul config yaml file that the SAMPLE trick is needed. I tried adding to the <code>TestCheckNoDeprecatedKeys</code>'s test config the following:</p>
<pre>
Workbench:
UserProfileFormField:
testProperty:
Type: text
ApplicationMimetypesWithViewIcon:
ruby: {}
</pre>
<p>... then ran <code>lib/config</code> tests and got:</p>
<pre>
FAIL: cmd_test.go:38: CommandSuite.TestCheckNoDeprecatedKeys
cmd_test.go:60:
c.Check(code, check.Equals, 0)
... obtained int = 1
... expected int = 0
cmd_test.go:62:
c.Check(stderr.String(), check.Equals, "")
... obtained string = "" +
... "deprecated or unknown config entry: Clusters.z1234.Workbench.ApplicationMimetypesWithViewIcon.ruby\n" +
... "deprecated or unknown config entry: Clusters.z1234.Workbench.UserProfileFormField\n"
... expected string = ""
OOPS: 18 passed, 1 FAILED
</pre>
<p>This will surely bite us in the future, do you think there's an automatic way of validating new config items?</p> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=762292019-07-11T16:07:13ZTom Cleggtom@curii.com
<ul></ul><p>I've added SAMPLE entries for those two.</p>
<p>As for automating this, perhaps we could load the defaults file as if it were a real config, then use reflection to detect any map[string]anything that doesn't have a SAMPLE key?</p>
<p>15003-real-configs-flagged-unknown @ <a class="changeset" title="15003: Fix errant warnings on Workbench configs. Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tcleg..." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/93cfe7c262708fb09eda5aad1839c832816d4591">93cfe7c262708fb09eda5aad1839c832816d4591</a> -- <a class="external" href="https://ci.curoverse.com/view/Developer/job/developer-run-tests/1386/">https://ci.curoverse.com/view/Developer/job/developer-run-tests/1386/</a></p> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=762302019-07-11T16:40:50ZLucas Di Pentimalucas.dipentima@curii.com
<ul></ul><p>Tom Clegg wrote:</p>
<p>[...]</p>
<blockquote>
<p>As for automating this, perhaps we could load the defaults file as if it were a real config, then use reflection to detect any map[string]anything that doesn't have a SAMPLE key?</p>
</blockquote>
<p>Yes, I think it would be very useful. Other possibility is to not print any warning on those empty <code>map[string]anything</code> because it's assumed that's a 'list' config item? That would make unnecessary to add <code>SAMPLE: {}</code> entries on the default config that are somewhat confusing IMO.</p> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=762322019-07-11T17:20:20ZTom Cleggtom@curii.com
<ul></ul><p>Lucas Di Pentima wrote:</p>
<blockquote>
<p>Yes, I think it would be very useful. Other possibility is to not print any warning on those empty <code>map[string]anything</code> because it's assumed that's a 'list' config item? That would make unnecessary to add <code>SAMPLE: {}</code> entries on the default config that are somewhat confusing IMO.</p>
</blockquote>
<p>We'd still want to check the fields inside map[string]RemoteCluster, so we'd need samples for things like that. A map[string]struct{} can always be treated as if it had {SAMPLE: {}}, whether or not an example is given in the defaults file, so we could potentially drop those. However, I don't think it's especially easy for the checker to figure out when it's looking at a map[string]struct{} entry -- the extra-key check happens before loading into the config struct.</p> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=762352019-07-11T17:29:11ZLucas Di Pentimalucas.dipentima@curii.com
<ul></ul><p>Ok, so if the SAMPLE key check is simple enough to do in this story, It'll be enough to avoid future issues like this.</p> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=762382019-07-11T17:44:17ZTom Cleggtom@curii.com
<ul></ul><p>I think the easiest way to prevent these from going undetected is a test that loads the defaults file as if it were a real config, and traverses the resulting Config object to confirm that every map[string]anything has a SAMPLE entry.</p> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=762502019-07-11T19:55:14ZTom Cleggtom@curii.com
<ul></ul><p>15003-ensure-all-sample-keys @ <a class="changeset" title="15003: Ensure all map[string]* have sample keys. Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tcleg..." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/cfb6b08b94d423a768862481b629cfb21fcc70a2">cfb6b08b94d423a768862481b629cfb21fcc70a2</a> -- <a class="external" href="https://ci.curoverse.com/view/Developer/job/developer-run-tests/1388/">https://ci.curoverse.com/view/Developer/job/developer-run-tests/1388/</a></p>
<p>This test also revealed that ManagedProperties was a map[string]interface{} with no type checks for its entries, so I fixed that.</p>
<pre>
FAIL: load_test.go:131: LoadSuite.TestDefaultConfigHasAllSAMPLEKeys
load_test.go:134:
s.checkSAMPLEKeys(c, "", cfg)
load_test.go:118:
c.Errorf("%s is a map with string keys (type %T) but config.default.yml has no SAMPLE key", path, x)
... Error: .Clusters.xxxxx.Collections.ManagedProperties.SAMPLE is a map with string keys (type map[string]interface {}) but config.default.yml has no SAMPLE key
</pre> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=762512019-07-11T20:20:45ZLucas Di Pentimalucas.dipentima@curii.com
<ul></ul><p>This LGTM, thanks!</p> Arvados - Idea #15003: [config] Go package & tool for preprocessing cluster config filehttps://dev.arvados.org/issues/15003?journal_id=762532019-07-11T21:08:07ZTom Cleggtom@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>88</i> to <i>100</i></li></ul><p>Applied in changeset <a class="changeset" title="Merge branch '15003-ensure-all-sample-keys' closes #15003 Arvados-DCO-1.1-Signed-off-by: Tom Cl..." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/055512c1f803e70cb9c426a8683aa9e8ddc8170c">arvados|055512c1f803e70cb9c426a8683aa9e8ddc8170c</a>.</p>