Move gitlab.rb config library to the packages cookbook
In this MR I had several goals.
Practical:
parse_variables
libraries for things like roles and secrets
1. Establish a way in the config that we can use our My intent for roles it to create new roles library files under a libaries/roles
folder, and call into those files using a predefined method like we do with parse_variables
. That change will be in a seperate MR, but while working on it, I was working on roles in the package cookbook, but having our config still all handled by the gitlab cookbook seemed off, so I moved the gitlab.rb into packages.
I decided to create a new folder called config
, and move our services in there, as well as the gitlab.rb. The intent is that when users go to add a new setting/service that they will remember to check to see if they need to update both files if they co-exist next to each other there in the config folder.
In order to use the other cookbooks libraries from the packages cookbook (which loads first), I define them in Procs, similar to the lazy syntax we are familiar with in chef.
2. Reduce the need to duplicate declaring the same config name multiple times when adding a new config object
While writing up the 'how to add new services' document, it became clear that it would be easier if we just used a method that we could define the gitlab attribute once.
So I added an attribute
method that takes care of setting the mixlib::config Mash for the attributes, and handles using the attribute when generating the Hash to pass back to the chef node.
Sugar:
3. Come up with a terse syntax for declaring the properties to minimise the perceived complexity when trying to add a new service
I have gone through a couple iterations already that achieved my other goals, but still looked very complicated as an observer after the fact. So I have added some additional "sugar" so it also looks less complex to use.
- The Proc for the handler is set at the same time as the attribute, so in the docs we only have to document one place you need to add changes for your attribute, even if it needs parsing.
- The attributes that need to be set on
node['gitlab']
instead of justnode
are wrapped in a block that set that for you, while still using the same syntax
One of the previous iterations I had us declaring attributes similar to chef resources, where you passed a block, and within that block you could call methods for setting the parser etc. But this resulted in the list being a lot longer (because we couldn't one-line it) and it gave the impression of looking complicated to add an attribute. So I dropped it in favour of this one-line approach.
Fixes: https://gitlab.com/gitlab-org/omnibus-gitlab/issues/2662
Merge request reports
Activity
added Technical Debt label
assigned to @marin
added 16 commits
- 3a58a0fd...80d738a3 - 8 commits from branch
master
- aed7ac75 - Try to clean up how we define our GitLab config object
- 6d335fd3 - Fix rubocop whitespace failure
- e1a3a2df - Comment the settings_helper library
- 1a0d6c7a - Add gitlab settings helper tests
- 07f16fc2 - Sequence shell parsing before rails parsing
- a1c204dd - Remove duplicate workhorse attribute
- ca373821 - Registry external url attribute was missing
- fcff6a37 - Update dev services documentation with the new config changes
Toggle commit list- 3a58a0fd...80d738a3 - 8 commits from branch
added 1 commit
- aa30b071 - Fix issue where were weren't always ignoring disabled attributes
@marin this is ready for review
added 38 commits
-
aa30b071...2e975477 - 36 commits from branch
master
- 74039aa4 - Merge remote-tracking branch 'origin/master' into cleanup-config-settings
- 4ee1697e - Update branch with latest changes from master
-
aa30b071...2e975477 - 36 commits from branch
mentioned in issue #2615 (closed)
- Resolved by DJ Mountney
- Resolved by DJ Mountney
- Resolved by DJ Mountney
@twk3 I have few minor nitpicks, but the rest looks good. We should be able to merge this after the comments are addressed :)
assigned to @twk3
changed milestone to %10.0
added 58 commits
-
4ee1697e...c84f79dc - 57 commits from branch
master
- 72df11df - Merge remote-tracking branch 'origin/master' into cleanup-config-settings
-
4ee1697e...c84f79dc - 57 commits from branch
added 1 commit
- 6e991e5b - Use a new way of determining whether we are running the GitLab EE
assigned to @marin
@marin updated and ready for review again
mentioned in merge request !1887 (merged)
- Resolved by DJ Mountney
@twk3 one remaining comment, add a Changelog item and we can merge.
@marin updated
added 58 commits
- 4e02ad34...67210a1c - 57 commits from branch
master
- 50feabc9 - Merge branch 'master' into 'cleanup-config-settings'
- 4e02ad34...67210a1c - 57 commits from branch
mentioned in commit 3ba0f802