rename metrics_server to listen_address

[tipmg]

According to official documentation - https://docs.gitlab.com/runner/monitoring/README.html#configuration-of-the-metrics-http-server

Pull Request (PR) description

This Pull Request (PR) fixes the following issues

[LongLiveCHIEF]

@tipmg can you fix the failing tests? Looks like it is just some linting issues. Other than that, this one LGTM 👍

[Dan33l]

Hi @tipmg . Thank you for the PR.
Can you have a look to inline comments ?

[Dan33l]

This change does not look related to this PR.
If this line is changed we needs to call the fact via $facts and remove leading ::.
IMO this should be in an other PR.

[tipmg]

Yes, you are right. Thanks. I fixed it too.

[tipmg]

It is interesting case. Test will be fail. I returned ${::osfamily}
I recommend to use $facts['os']['family']
It is decision by author.

[Dan33l]

It is strange when i run tests locally with fail("gitlab_ci_runner::manage_repo parameter for ${facts['os']['family']} is not supported.") the command bundle exec rake test does not fail.

Perhaps if we want to be more preservative without structured facts, we can use $facts['osfamily']

What was the problem you encountered ?

[tipmg]

Hello, @LongLiveCHIEF I fixed tests.

[tipmg]

Hello, @Dan33l Please review changes.

[alexjfisher]

Not sure about this one. The option was called metrics_server up until gitlab runner v11.
See https://gitlab.com/gitlab-org/gitlab-runner/merge_requests/838 and https://gitlab.com/gitlab-org/gitlab-ce/issues/46527

This change would make the module incompatible with v10 of the runner. (in v11 both options currently work, but metrics_server is deprecated).

Would it be better to add listen_address as a new parameter, but keep metrics_server for now?
Perhaps throw an error if a user tries to set both?

[bastelfreak]

Also this looks like a breaking-change. I'm not sure if that move is required.

[LongLiveCHIEF]

@alexjfisher @bastelfreak We deal with this a bit in the puppet-gitlab module also, and so we came up with the following strategy, taken from puppet-gitlab/README.md/#description:

This module is designed to support the most recent versions of the gitlab-omnibus package (both ce and ee). Gitlab will support and release patches for the last 3 releases. This module can typically support the most recent major version, as well as the previous major version, but is currently only tested in the gitlab-supported versions of the module.

One of the reasons I broke this out into its own module however, was to give flexibility to make different support strategy decisions for the runners. Just because someone is using this module, does not mean they're using the puppet-gitlab module, and it is very likely someone could use a v10 runner with v11 of GitLab. (I've actually seen it a lot).

This opens up the discussion of what the support strategy should be for this module.

I will open a discussion issue for that discussion, but in the meantime, I don't think we can leave both, and make a note in the README that users should be aware and use the correct parameters for their version.

@tipmg can you ammend the PR to leave in the metrics_server param, while still adding the listen_address?

[alexjfisher]

can you amend the PR to leave in the metrics_server param, while still adding the listen_address?

+1

[dhollinger]

I think we should keep the metrics_server param in for now, but revisit it soon. After spending some time in the Gitlab docs, they seem to imply that (aside from critical security fixes and certain pay levels) they primarily only support latest stable, which is a monthly release.

While I don't think we should be that aggressive, we should discourage the use of major releases older than 18-24 months (or less). Each Gitlab major release is yearly, so that should be simple enough. Additionally, we should only support latest stable minor release, aside from critical bugfixes (that shouldn't be difficult since Gitlab only usually introduces breaking changes in major releases)

[baurmatt]

Any update on this? :)

[pccibot]

Dear @tipmg, thanks for the PR!

This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

[pccibot]

Dear @tipmg, thanks for the PR!

This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks