manage entire config.toml #18

KlavsKlavsen · 2018-08-24T13:54:40Z

for some reason I've hit a docker issue with DNS lookups.. so I need to add:
extra_hosts = ["mygiturl:mygithostip"] - to config.toml - under runners.docker.

The module does not support this currently.. and does not handle if config.toml file is missing :(

I'd also like to set priviledged to true.

(issue migrated from voxpupuli/puppet-gitlab#121 )

LongLiveCHIEF · 2018-08-30T22:22:53Z

Thanks for re-opening. I know it can be a pain, but when it's feasible, i prefer the original authorship of the issue to be retained.

baurmatt · 2019-01-04T10:41:28Z

I was about to implement this when I discovered the suboptimal registering process of Gitlab runners...

Get a registration token from your Gitlab instance
Call the gitlab-runner binary with the register argument
The gitlab-runner binary will go to your Gitlab instance and register the Runner. By this it will get a runner token from Gitlab which gets then placed in the config file.

As we can never really know the runner token when configuring it in Puppet, we need to do the registration process ourself as explained in https://gitlab.com/gitlab-org/gitlab-runner/issues/985#note_3175185

KlavsKlavsen · 2019-01-04T10:48:12Z

@baurmatt we COULD have a fact that pulls the token from what the current sets up in config.. and then WHEN we have something in that fact.. trigger a rewrite of the config toml file ?

baurmatt · 2019-01-04T10:55:28Z

I don't like to abuse facts for this especially since Puppet runs aren't idempotent anymore.

I think we should go with the self registration of the runner --> https://docs.gitlab.com/ee/api/runners.html#register-a-new-runner

I see two options here:

Doing the registration process on the Puppetmaster
- Pro:
  - Only a Puppet function is required - easier to implement
- Contra:
  - The Puppetmaster needs to have access to Gitlab
Doing the registration process on the Gitlab Runner server
- Pro:
  - The Puppetmaster doesn't need access Gitlab
  - With Puppet 6 we could implement this as a Deferred function as well
- Contra:
  - As we probably can't make this module Puppet 6 only, we need to implement this in a type/provider.

KlavsKlavsen · 2019-01-04T11:04:51Z

As we probably can't make this module Puppet 6 only, we need to implement this in a type/provider.

It could be an optional feature - that happens to depend on puppet6 (if that can be done cleanly)

Deferred values are cleaner - but not much different from using a fact for the task (and they are from puppet 5.6.. )

baurmatt · 2019-01-04T12:05:54Z

Hmm thought about this over lunch... This isn't going to work as the function/type would register a new runner on every run :(

Dan33l · 2019-02-13T13:21:04Z

If more people show interest to get this issue solved, we will got a path:
https://gitlab.com/gitlab-org/gitlab-runner/issues/3022

Please consider to add comment there.

baurmatt · 2019-02-13T13:33:26Z

@Dan33l as we're a paying customer of Gitlab Enterprise, I've directly contacted the official support channel. Perhaps this draw's some attention on this issue :)

baurmatt · 2020-01-09T17:53:18Z

Looks like this isn't going to happen. How about this: We're dropping the support of register/unregister a runner but expect people to give gitlab_ci_runner::runner a already valid runner token.

We could provide a simple script (Bolt task?) which does the registration/unregistration in the first place.

This would allow us to drop the exec foo in gitlab_ci_runner::runner and would make it possible to manage the whole config.toml.

Looking forward to your opinions! :)

baurmatt · 2020-01-10T15:26:35Z

I've completely refactored gitlab_ci_runner::runner to welcome it in a world where we don't have Puppet 3 anymore. But it still uses exec's to (un)register the runner.

I've also added Bolt tasks in #73 because however we decide, I think they're good to have.

Now its your turn: Please decide if you think it is ok to drop the (un)registration from gitlab_ci_runner::runner to allow us the complete management of config.toml or if you prefer to keep it as it is.

This uses the foundation laid in dea2342 and moves the runner configuration from execs to concat::fragments. This was done to be able to change (parts) of the runner configuration after the inital Puppet run which is not possible with the exec because there is no config update logic in the gitlab-runner binary. Sadly this drops the support for autoregistering a runner on a Gitlab instance as discussed in voxpupuli#18. The registration can be done with Bolt task provide in voxpupuli#73.

baurmatt · 2020-01-14T13:41:51Z

I've build this in #75. Take a look and tell me what you think! :)

KlavsKlavsen · 2020-01-14T13:55:57Z

I love this solution.. very cool using the defined params in puppets class, and just convert directly to toml for the file.. no template actually needed :)
It does mean we should probably extend the runners part from a Hash to an actual list of supported params by https://docs.gitlab.com/runner/configuration/advanced-configuration.html - but current addition is no worse than what currently exists.. I'm hoping @LongLiveCHIEF also likes it :)

This uses the foundation laid in 26617fd and moves the runner configuration from execs to concat::fragments. This was done to be able to change (parts) of the runner configuration after the inital Puppet run which is not possible with the exec because there is no config update logic in the gitlab-runner binary. Sadly this drops the support for autoregistering a runner on a Gitlab instance as discussed in voxpupuli#18. The registration can be done with Bolt task provide in voxpupuli#73.

baurmatt · 2020-01-14T14:25:48Z

@KlavsKlavsen Absolutely, I already have some of the data types laying around but they're HUGE. So I thought it's easier to review in an additional pull request.

This uses the foundation laid in 26617fd and moves the runner configuration from execs to concat::fragments. This was done to be able to change (parts) of the runner configuration after the inital Puppet run which is not possible with the exec because there is no config update logic in the gitlab-runner binary. Sadly this drops the support for autoregistering a runner on a Gitlab instance as discussed in voxpupuli#18. The registration can be done with Bolt task provide in voxpupuli#73.

This uses the foundation laid in e99ffe0 and moves the runner configuration from execs to concat::fragments. This was done to be able to change (parts) of the runner configuration after the inital Puppet run which is not possible with the exec because there is no config update logic in the gitlab-runner binary. Sadly this drops the support for autoregistering a runner on a Gitlab instance as discussed in voxpupuli#18. The registration can be done with Bolt task provide in voxpupuli#73.

This uses the foundation laid in 75a21b61ace991d65864721ccd82499c46c5acf6 and moves the runner configuration from execs to concat::fragments. This was done to be able to change (parts) of the runner configuration after the inital Puppet run which is not possible with the exec because there is no config update logic in the gitlab-runner binary. Sadly this drops the support for autoregistering a runner on a Gitlab instance as discussed in voxpupuli#18. The registration can be done with Bolt task provide in voxpupuli#73.

This uses the foundation laid in 9ff7b03 and moves the runner configuration from execs to concat::fragments. This was done to be able to change (parts) of the runner configuration after the inital Puppet run which is not possible with the exec because there is no config update logic in the gitlab-runner binary. Sadly this drops the support for autoregistering a runner on a Gitlab instance as discussed in voxpupuli#18. The registration can be done with Bolt task provide in voxpupuli#73.

baurmatt · 2020-01-30T09:13:17Z

Sooo, after some testing I've realized that, at least in our environment, registering a runner is often crucial and shouldn't be an extra Bolt task.

After playing around with it for some time, it turns out that the best option is to use a Deferred function. Due to this being a Puppet 6 only functionality, there will like be no support for automatically registering runners on Puppet 5 (everything below is already EOL). Users requiring Puppet 5 and auto-registration would need to stay on a version which included the current exec approach.

But this gives us the ability to full support auto registration and full config management. I'm not yet fully satisfied with the code, but if you want to take a look go over here --> https://github.com/syseleven/puppet-gitlab_ci_runner/tree/move-to-concat-deferred

High level overview how it works:

The module generates a concat file containing the global config and runner specific config
The config is rendered with the help of a to_toml function
If the runner config includes a registration-token key, it gets converted to token key which has a Deferrred function as its value. This function will call the Gitlab server, request the auth token and write it to disk (e.g. /etc/gitlab-runner/auth-token-testrunner) where it can be read on every subsequent Puppet run.
As the final value of the token key gets now determined only on the Agent run, we need to delay the config generation until then hence the to_toml function needs to be a Deferred function as well.

Only problem with this is, that the to_toml function requires the toml-rb gem to be available on the Agent before the Deferred function gets called. As Deferred functions are currently evaluated before anything is applied, there is basically no good way to install the toml-rb gem before the function uses it. I'm currently considering to include a TOML dumper functionality directly in the module to work around this problem.

I will polish that branch over the next weeks and submit a PR after my current PRs are reviewed - don't want to overload the PR queue :)

This uses the foundation laid in 9ff7b03 and moves the runner configuration from execs to concat::fragments. This was done to be able to change (parts) of the runner configuration after the inital Puppet run which is not possible with the exec because there is no config update logic in the gitlab-runner binary. Sadly this drops the support for autoregistering a runner on a Gitlab instance as discussed in voxpupuli#18. The registration can be done with Bolt task provide in voxpupuli#73.

Before this, the complete hash with all runners was injected into gitlab_ci_runner::runner. Based on the title of the resource it was then decided which runner should be extracted from the global hash and merge with defaults ect. This was completely refactored in a way that a single gitlab_ci_runner::runner resource now only holds all data which is need to configure it but no other. This also changes the default of gitlab_ci_runner::runners(_defaults) to an empty hash because I don't see why we need to force user to configure it. Move global options to toml generation This drop the 'builds_dir' and cache_dir because they are runner specific parameter rather than global ones. The 'metrics_server' parameter gets dropped as well because it was deprecated almost 2 years ago. Furthermore, this commit introduce all missing global configuration options which are 'log_level', 'log_format' and 'check_interval'. Move runner configuration from exec to concat::fragment This uses the foundation laid in 9ff7b03 and moves the runner configuration from execs to concat::fragments. This was done to be able to change (parts) of the runner configuration after the inital Puppet run which is not possible with the exec because there is no config update logic in the gitlab-runner binary. Sadly this drops the support for autoregistering a runner on a Gitlab instance as discussed in voxpupuli#18. The registration can be done with Bolt task provide in voxpupuli#73. move-to-concat - Move TOML creation to its own function move-to-concat-deferred - Refactor to use a library move-to-concat-deferred - bolt-tasks - Add register/unregister Puppet functions move-to-concat-deferred - Allow $config['token'] to be a deferred function move-to-concat-deferred - Allow auth and reg token to be retrieved by a Deferred function move-to-concat-deferred - Don't depend on toml-rb, included the dumper.rb from their move-to-concat-deferred - Save

alexjfisher · 2021-07-08T16:40:31Z

@baurmatt Sorry your PR didn't get the attention it deserved at the time. Are you interested in reviving this work now that we can drop support for Puppet 5 if we want?

baurmatt · 2021-07-09T06:18:05Z

@alexjfisher I would absolutely be interested in getting #75 merged! If you're up to reviewing the PR, I'm happy to rebase it and solve all merge conflicts. As I'm currently not working very much with Puppet anymore, I need to find some time to get back into the code again.

alexjfisher · 2021-07-14T13:57:12Z

I'm also thinking I need autoregistration and full management of the config file. Which branch of yours am I best looking at?

move-to-concat-deferred or clean-move-to-concat-deferred?

Only problem with this is, that the to_toml function requires the toml-rb gem to be available on the Agent before the Deferred function gets called. As Deferred functions are currently evaluated before anything is applied, there is basically no good way to install the toml-rb gem before the function uses it. I'm currently considering to include a TOML dumper functionality directly in the module to work around this problem.

Maybe just a toml-rb-gem-version fact or similar would be ok for this? (Even if it does then take 2 puppet runs)

alexjfisher · 2021-07-14T15:14:35Z

I'm liking your clean-move-to-concat-deferred branch. Seems to work well. In my environment, I had to hack it to get it to register via an http proxy, but I think to keep it small (or at least not make the change any bigger!), I could submit proxy support changes in a separate PR.

(FWIW, in the current module version, I could hack proxy support during registration in my profile)

# outrageous hack to get runner registration to use proxy
Exec <| title == 'Register_runner_testrpms' |> {
  environment => [
    "HTTPS_PROXY=http://${proxy_host}:${proxy_port}",
    "https_proxy=http://${proxy_host}:${proxy_port}",
  ],
}

What are your thoughts? Would you like to rebase and put up a new PR based on this branch? Now that Puppet 5 is EOL and we've already dropped support for it in many modules, feel free to rip out any compatibility code if you think that makes things simpler.

This uses the foundation laid in 9ff7b03 and moves the runner configuration from execs to concat::fragments. This was done to be able to change (parts) of the runner configuration after the inital Puppet run which is not possible with the exec because there is no config update logic in the gitlab-runner binary. Sadly this drops the support for autoregistering a runner on a Gitlab instance as discussed in voxpupuli#18. The registration can be done with Bolt task provide in voxpupuli#73.

baurmatt · 2021-07-15T14:25:14Z

Uff, wow this is really a mess :( Sorry about that! We're currently using the branch move-to-concat-deferred, to be more specific the commit 22ef708204212488b003689bef7fd5cb8434b531.

I could successfully rebase #75 and was also able to fix some unrelated CI issues in #103. Please review and merge #103 first.

This uses the foundation laid in 9ff7b03 and moves the runner configuration from execs to concat::fragments. This was done to be able to change (parts) of the runner configuration after the inital Puppet run which is not possible with the exec because there is no config update logic in the gitlab-runner binary. Sadly this drops the support for autoregistering a runner on a Gitlab instance as discussed in voxpupuli#18. The registration can be done with Bolt task provide in voxpupuli#73.

LongLiveCHIEF added enhancement New feature or request good first issue labels Aug 30, 2018

alexjfisher removed the good first issue label Nov 21, 2018

baurmatt mentioned this issue Jan 10, 2020

Add bolt task to register/unregister a runner #73

Merged

baurmatt mentioned this issue Jan 14, 2020

Move to concat for config.toml #75

Merged

alexjfisher closed this as completed in #75 Jul 22, 2021

alexjfisher added the skip-changelog label Aug 25, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

manage entire config.toml #18

manage entire config.toml #18

KlavsKlavsen commented Aug 24, 2018

LongLiveCHIEF commented Aug 30, 2018

baurmatt commented Jan 4, 2019

KlavsKlavsen commented Jan 4, 2019

baurmatt commented Jan 4, 2019

KlavsKlavsen commented Jan 4, 2019 •

edited

Loading

baurmatt commented Jan 4, 2019

Dan33l commented Feb 13, 2019

baurmatt commented Feb 13, 2019

baurmatt commented Jan 9, 2020 •

edited

Loading

baurmatt commented Jan 10, 2020

baurmatt commented Jan 14, 2020

KlavsKlavsen commented Jan 14, 2020

baurmatt commented Jan 14, 2020

baurmatt commented Jan 30, 2020

alexjfisher commented Jul 8, 2021

baurmatt commented Jul 9, 2021

alexjfisher commented Jul 14, 2021

alexjfisher commented Jul 14, 2021

baurmatt commented Jul 15, 2021

manage entire config.toml #18

manage entire config.toml #18

Comments

KlavsKlavsen commented Aug 24, 2018

LongLiveCHIEF commented Aug 30, 2018

baurmatt commented Jan 4, 2019

KlavsKlavsen commented Jan 4, 2019

baurmatt commented Jan 4, 2019

KlavsKlavsen commented Jan 4, 2019 • edited Loading

baurmatt commented Jan 4, 2019

Dan33l commented Feb 13, 2019

baurmatt commented Feb 13, 2019

baurmatt commented Jan 9, 2020 • edited Loading

baurmatt commented Jan 10, 2020

baurmatt commented Jan 14, 2020

KlavsKlavsen commented Jan 14, 2020

baurmatt commented Jan 14, 2020

baurmatt commented Jan 30, 2020

alexjfisher commented Jul 8, 2021

baurmatt commented Jul 9, 2021

alexjfisher commented Jul 14, 2021

alexjfisher commented Jul 14, 2021

baurmatt commented Jul 15, 2021

KlavsKlavsen commented Jan 4, 2019 •

edited

Loading

baurmatt commented Jan 9, 2020 •

edited

Loading