Story #13996

[API] Migrate RailsAPI to new cluster config file

Added by Tom Clegg 8 months ago. Updated about 4 hours ago.

Status:
New
Priority:
Normal
Assigned To:
Category:
API
Target version:
Start date:
03/26/2019
Due date:
% Done:

50%

Estimated time:
(Total: 0.00 h)
Story points:
3.0

Description

This will enable an operator to move their configs from their existing RailsAPI configuration file (/etc/arvados/api/application.yml) to their cluster config file (/etc/arvados/config.yml), then delete the RailsAPI-specific file.

Features needed:

  • The old Rails-only configuration file has precedence (otherwise, merely upgrading to this version could cause unanticipated config changes).
  • The operator can easily find out whether deleting their legacy config file would affect their runtime configuration.
  • The operator can delete their legacy config file.

Suggested implementation:

At startup:
  1. Load new cluster config defaults from static file included in package (this can be in the RailsAPI subtree for now, although in the future all arvados components will ship with a copy of it)
  2. Load new cluster config from /etc/arvados/config.yml
  3. Load legacy config /etc/arvados/api/application.yml / config/application.yml
  4. Copy legacy config keys to equivalent cluster config keys

Update Rails.configuration references to use the corresponding keys in the new config.

Provide script(s) or rake task(s) for "dump" and "diff" to assist with config migration:
  • "dump": load configuration (as above) and write the resulting config to stdout/file. This is the quickest way for the operator to make the legacy config file superfluous so it can be deleted.
  • "diff": show the changes made by the "copy legacy config keys" step. This suits an operator who wants to update their new config incrementally and/or preserve comments in their new config.

NOT included here:

  • mechanism to split config into multiple files/fragments ("include")
  • mechanism to overlay multiple configs (like the legacy config does with "common" + "production" sections)
  • mechanism to avoid storing secrets in the config file
  • update other components to read RailsAPI configs from the cluster config file instead of the discovery document

Subtasks

Task #14981: Review 13996-new-api-configResolvedPeter Amstutz

Task #15042: API server packaging includes its own copy of config.defaults.ymlFeedbackWard Vandewege


Related issues

Related to Arvados - Story #13648: [Epic] Use one cluster configuration file for all componentsNew

Blocked by Arvados - Feature #13646: [API] Rails API server loads cluster configuration fileRejected

Blocks Arvados - Feature #14811: [SSO] Use cluster configNew

Blocks Arvados - Feature #14812: [Workbench1] Use cluster configNew

Associated revisions

Revision 3c879467
Added by Peter Amstutz 1 day ago

Merge branch '13996-new-api-config' refs #13996

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <>

Revision 3d878236 (diff)
Added by Peter Amstutz about 7 hours ago

Set secrets.secret_key_base refs #13996

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <>

Revision 196f347c (diff)
Added by Peter Amstutz about 4 hours ago

Merge, don't replace RemoteClusters refs #13996

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <>

History

#1 Updated by Tom Clegg 8 months ago

  • Category set to API
  • Target version set to To Be Groomed

#2 Updated by Tom Clegg 8 months ago

  • Blocked by Feature #13646: [API] Rails API server loads cluster configuration file added

#3 Updated by Peter Amstutz 3 months ago

  • Description updated (diff)

#4 Updated by Tom Clegg 2 months ago

  • Related to Story #13648: [Epic] Use one cluster configuration file for all components added

#5 Updated by Tom Morris 2 months ago

  • Description updated (diff)
  • Target version changed from To Be Groomed to Arvados Future Sprints
  • Story points set to 3.0

#6 Updated by Tom Clegg about 1 month ago

  • Subject changed from [API] [Controller] Move API configs from Rails config file to cluster config file to [API] Migrate RailsAPI to new cluster config file
  • Description updated (diff)

#7 Updated by Peter Amstutz about 1 month ago

  • Assigned To set to Peter Amstutz
  • Target version changed from Arvados Future Sprints to 2019-03-27 Sprint

#8 Updated by Peter Amstutz about 1 month ago

#9 Updated by Peter Amstutz about 1 month ago

#10 Updated by Peter Amstutz 24 days ago

13996-new-api-config @ 5f08fc8fcda25dbaa184dfc63a06cb6633af5877

https://ci.curoverse.com/view/Developer/job/developer-run-tests/1152/

https://ci.curoverse.com/view/CWL/job/arvados-cwl-conformance-tests/69/

  • Loads new config object
  • Loads legacy config, migrates keys into new config object
  • Performs typechecking/coercion for Durations, URIs and essential file paths
  • "rake config:dump" to get active config
  • "rake config:diff" to get new config migrated from legacy config
  • Update all the API server code to use new config parameters
  • Fix all the tests

#11 Updated by Peter Amstutz 23 days ago

  • Target version changed from 2019-03-27 Sprint to 2019-04-10 Sprint

#12 Updated by Peter Amstutz 21 days ago

I realized this doesn't cover database.yml so I need to look into that.

#13 Updated by Lucas Di Pentima 10 days ago

Comments/questions:

  • If I try to start the console or do a "rake config:dump” from 023015bacfded08503d0240d3f71838204d8dcb7 I get an the error 'missing Git.Repositories’, but if I run it from 6e9fcde0422b33d081e2985975e3104eb2434957, it works ok.
  • When running "rake config:check" from 6e9fcde0422b33d081e2985975e3104eb2434957, I get the following error: "NoMethodError: undefined method `sort' for nil:NilClass" (from config_check.rake line 9)
  • When dumping the new config, some keys are listed the old way. Example: 'whiny_nils: true'. Are these rails-only configs needed in the dump?
  • The secret_token key is not being translated on config:dump as RailsSessionSecretToken (on the TO-DO list of the google doc), is that on purpose?
  • Maybe some upgrade notes describing the existence of the new rake commands would be needed?

#14 Updated by Peter Amstutz 9 days ago

  • Target version changed from 2019-04-10 Sprint to 2019-04-24 Sprint

#15 Updated by Peter Amstutz 8 days ago

Lucas Di Pentima wrote:

Comments/questions:

This was due to a missing symlink to config.default.yml. Fixed (and now also reports if the defaults file is missing.)

config:check is gone, because the configuration is always checked on load, and most of the code in that task doesn't make sense for the new system.

  • When dumping the new config, some keys are listed the old way. Example: 'whiny_nils: true'. Are these rails-only configs needed in the dump?

There are now three commands:

rake config:diff                               # Print values from application.yml that differ from system config.yml
rake config:dump                               # Print configuration as accessed through Rails.configuration
rake config:migrate                            # Print config.yml after merging with legacy application.yml

config:dump is intended for debugging so it includes any other Rails configuration options we know about, which are not settable in config.yml but may differ between test, development and production configurations.

  • The secret_token key is not being translated on config:dump as RailsSessionSecretToken (on the TO-DO list of the google doc), is that on purpose?

Only that it was in the TO-DO section. Added it to API.

  • Maybe some upgrade notes describing the existence of the new rake commands would be needed?

Yep, added two new documentation pages.

13996-new-api-config @ b05be6e2d1db4e63074fc28e978c40a271376f1f

https://ci.curoverse.com/view/Developer/job/developer-run-tests/1187/

#17 Updated by Lucas Di Pentima 8 days ago

Pretty neat the usage of OrderedOptions! This LGTM, but I had some issues trying to start arvbox, but I'm not sure if it's just my local instance.

#18 Updated by Lucas Di Pentima 8 days ago

I've just tried to render the documentation, this is what I got:

[lucas:~/Devel/git/arvados/doc] 8b5dff309+ 9s ± rake run
/Users/lucas/.rvm/gems/ruby-2.3.8@deleteme/gems/liquid-2.6.1/lib/liquid/htmltags.rb:43: warning: key "index0" is duplicated and overwritten on line 46
[2019-04-11 18:58:54] INFO  WEBrick 1.3.1
[2019-04-11 18:58:54] INFO  ruby 2.3.8 (2018-10-18) [x86_64-darwin18]
[2019-04-11 18:58:54] INFO  WEBrick::HTTPServer#start: pid=11719 port=8000
[2019-04-11 18:58:55] ERROR bad URI `=\x0F��܅qZK��,�����HF���^2m����p\x004\x13\x03\x13\x01\x13\x02�,�+�$�#�'.
127.0.0.1 - - [11/Apr/2019:18:58:55 -03] "\x16\x03\x01\x02\x00\x01\x00\x01�\x03\x03u��\x15�I�\x14�FK���F\x1C\x04\x1E1��\x18|��f\x17��� =\x0F��܅qZK��,�����HF���^2m����p\x004\x13\x03\x13\x01\x13\x02�,�+�$�#�" 400 340
- -> =\x0F��܅qZK��,�����HF���^2m����p\x004\x13\x03\x13\x01\x13\x02�,�+�$�#�
[2019-04-11 18:58:55] ERROR bad Request-Line `\x16\x03\x01\x00�\x01\x00\x00�\x03\x03��I\x02n=�z��\\�5\x1A�\x0E\x13��t\eV�8#��6�x\x00\x00<�,�+�$�#�'.
127.0.0.1 - - [11/Apr/2019:18:58:55 -03] "\x16\x03\x01\x00�\x01\x00\x00�\x03\x03��I\x02n=�z��\\�5\x1A�\x0E\x13��t\eV�8#��6�x\x00\x00<�,�+�$�#�" 400 358
- ->
[2019-04-11 18:58:55] ERROR bad Request-Line `\x16\x03\x01\x00�\x01\x00\x00�\x03\x01\x1C~��\f \x14دx\r�o/��y�\x04V5Nٶ\x19DŽ�Y�5\x00\x00\x14�'.
127.0.0.1 - - [11/Apr/2019:18:58:55 -03] "\x16\x03\x01\x00�\x01\x00\x00�\x03\x01\x1C~��\f \x14دx\r�o/��y�\x04V5Nٶ\x19DŽ�Y�5\x00\x00\x14�" 400 347
- ->

...I also get test failure when running doc tests, but didn't failed on jenkins. I'm not sure what's happening.

#19 Updated by Peter Amstutz 7 days ago

Lucas Di Pentima wrote:

I've just tried to render the documentation, this is what I got:

[...]

That looks to me like you tried to connect with HTTPS to a plain HTTP server.

...I also get test failure when running doc tests, but didn't failed on jenkins. I'm not sure what's happening.

I think that's #15043 ? I don't think the new docs add any new errors.

#20 Updated by Lucas Di Pentima 7 days ago

Thanks, you were right about HTTPS vs HTTP.

The docs looks good, the only addition I think would be useful is a reference to the migration section on the upgrade notes, "current master branch" section, so that it gets included on the future release upgrade notes.

With that, LGTM.

Also available in: Atom PDF