Story #13996

[API] Migrate RailsAPI to new cluster config file

Added by Tom Clegg over 1 year ago. Updated 7 months ago.

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

100%

Estimated time:
(Total: 0.00 h)
Story points:
3.0
Release relationship:
Auto

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.ymlResolvedWard Vandewege


Related issues

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

Related to Arvados - Bug #15225: [API] installation gives error: Creating git_internal_dir ''...mkdir: cannot create directory ‘’: No such file or directoryResolved

Related to Arvados - Bug #15273: [API] accept lowercase postgresql connection paramsResolved

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] Load configuration from cluster config fileResolved07/02/2019

Associated revisions

Revision 3c879467
Added by Peter Amstutz 8 months 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 8 months ago

Set secrets.secret_key_base refs #13996

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

Revision 196f347c (diff)
Added by Peter Amstutz 8 months ago

Merge, don't replace RemoteClusters refs #13996

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

Revision 709cd2fa
Added by Peter Amstutz 8 months ago

Merge branch '15119-api-config-fixes'

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

refs #13996
refs #15119

Revision d541d3e4 (diff)
Added by Peter Amstutz 8 months ago

Handle unrecognized uuid_prefix of remote host refs #13996

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

Revision 0d7f05d0 (diff)
Added by Peter Amstutz 7 months ago

Fix mistake setting container_count_max from wrong config parameter

refs #13996

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

Revision 390c0199 (diff)
Added by Ward Vandewege 6 months ago

Do not blow up when Rails.configuration.Users.UserProfileNotificationAddress is
set to the empty string, which is the default since #13996 (it defaulted to a
dummy e-mail adress before).

refs #15286
refs #13996

Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <>

Revision 1a18a297 (diff)
Added by Ward Vandewege 6 months ago

Do not blow up when Rails.configuration.Users.UserProfileNotificationAddress is
set to the empty string, which is the default since #13996 (it defaulted to a
dummy e-mail adress before).

refs #15286
refs #13996

Arvados-DCO-1.1-Signed-off-by: Ward Vandewege <>

History

#1 Updated by Tom Clegg over 1 year ago

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

#2 Updated by Tom Clegg over 1 year ago

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

#3 Updated by Peter Amstutz 11 months ago

  • Description updated (diff)

#4 Updated by Tom Clegg 10 months ago

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

#5 Updated by Tom Morris 10 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 9 months 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 9 months ago

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

#8 Updated by Peter Amstutz 9 months ago

#9 Updated by Peter Amstutz 9 months ago

  • Blocks Feature #14812: [Workbench1] Load configuration from cluster config file added

#10 Updated by Peter Amstutz 9 months 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 9 months ago

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

#12 Updated by Peter Amstutz 9 months ago

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

#13 Updated by Lucas Di Pentima 8 months 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 8 months ago

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

#15 Updated by Peter Amstutz 8 months 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 months 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 months 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 8 months 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 8 months 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.

#21 Updated by Peter Amstutz 8 months ago

  • Status changed from New to Resolved

#22 Updated by Ward Vandewege 7 months ago

  • Related to Bug #15225: [API] installation gives error: Creating git_internal_dir ''...mkdir: cannot create directory ‘’: No such file or directory added

#23 Updated by Tom Morris 7 months ago

  • Release set to 15

#24 Updated by Tom Clegg 7 months ago

  • Related to Bug #15273: [API] accept lowercase postgresql connection params added

Also available in: Atom PDF