Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

rename metrics_server to listen_address #46

Closed
wants to merge 11 commits into from
Closed

rename metrics_server to listen_address #46

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
142a194
rename metrics_server to listen_address
tipmg May 31, 2019
7ae1d6a
remove unrequited spaces
tipmg Jun 3, 2019
02f3a20
fix ${osfamily} variable
tipmg Jun 3, 2019
6672eb8
update variable osfamily
tipmg Jun 3, 2019
467f721
try to fix $::osfamily
tipmg Jun 3, 2019
a01c4e8
try to fix tests
tipmg Jun 3, 2019
f0b06d0
fix test
tipmg Jun 3, 2019
06bebc2
update tests
tipmg Jun 3, 2019
1760be3
update tests
tipmg Jun 3, 2019
e2b099c
update test
tipmg Jun 3, 2019
9270d12
fix test
tipmg Jun 3, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 18 additions & 17 deletions manifests/init.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
# The limit on the number of jobs that can run concurrently among
# all runners, or `undef` to leave unmanaged.
#
# [*metrics_server*]
# [*listen_address*]
# Default: `undef`
# [host]:<port> to enable metrics server as described in
# https://docs.gitlab.com/runner/monitoring/README.html#configuration-of-the-metrics-http-server
Expand All @@ -18,24 +18,24 @@
Hash $runners,
Hash $runner_defaults,
String $xz_package_name,
Optional[Integer] $concurrent = undef,
Optional[String] $builds_dir = undef,
Optional[String] $cache_dir = undef,
Optional[Pattern[/.*:.+/]] $metrics_server = undef,
Optional[String] $sentry_dsn = undef,
Boolean $manage_docker = true,
Boolean $manage_repo = true,
String $package_ensure = installed,
String $package_name = 'gitlab-runner',
){
Optional[Integer] $concurrent = undef,
Optional[String] $builds_dir = undef,
Optional[String] $cache_dir = undef,
Optional[Pattern[/.*:.+/]] $listen_address = undef,
Optional[String] $sentry_dsn = undef,
Boolean $manage_docker = true,
Boolean $manage_repo = true,
String $package_ensure = installed,
String $package_name = 'gitlab-runner',
) {
if $manage_docker {
# workaround for cirunner issue #1617
# https://gitlab.com/gitlab-org/gitlab-ci-multi-runner/issues/1617
ensure_packages($xz_package_name)

$docker_images = {
ubuntu_trusty => {
image => 'ubuntu',
image => 'ubuntu',
image_tag => 'trusty',
},
}
Expand Down Expand Up @@ -92,7 +92,7 @@
}
}
default: {
fail ("gitlab_ci_runner::manage_repo parameter for ${::osfamily} is not supported.")
fail("gitlab_ci_runner::manage_repo parameter for ${::osfamily} is not supported.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

@Dan33l Dan33l Jun 4, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ?

}
}
}
Expand All @@ -111,11 +111,12 @@
}
}

if $metrics_server {
file_line { 'gitlab-runner-metrics-server':
if $listen_address {
file_line { 'gitlab-runner-listen-address':
path => '/etc/gitlab-runner/config.toml',
line => "metrics_server = \"${metrics_server}\"",
match => '^metrics_server = .+',
after => '^concurrent',
line => "listen_address = \"${listen_address}\"",
match => '^listen_address = .+',
require => Package[$package_name],
notify => Exec['gitlab-runner-restart'],
}
Expand Down
24 changes: 12 additions & 12 deletions spec/classes/gitlab_ci_runner_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
end
it { is_expected.to contain_gitlab_ci_runner__runner('test_runner').that_requires('Exec[gitlab-runner-restart]') }
it { is_expected.not_to contain_file_line('gitlab-runner-concurrent') }
it { is_expected.not_to contain_file_line('gitlab-runner-metrics-server') }
it { is_expected.not_to contain_file_line('gitlab-runner-listen-address') }
it { is_expected.not_to contain_file_line('gitlab-runner-builds_dir') }
it { is_expected.not_to contain_file_line('gitlab-runner-cache_dir') }

Expand All @@ -49,25 +49,25 @@
it { is_expected.to contain_file_line('gitlab-runner-concurrent').that_notifies('Exec[gitlab-runner-restart]') }
it do
is_expected.to contain_file_line('gitlab-runner-concurrent').with('path' => '/etc/gitlab-runner/config.toml',
'line' => 'concurrent = 10',
'line' => 'concurrent = 10',
'match' => '^concurrent = \d+')
end
end
context 'with metrics_server => localhost:9252' do
context 'with listen-address => localhost:9252' do
let(:params) do
{
'runner_defaults' => {},
'runners' => {},
'metrics_server' => 'localhost:9252'
'listen_address' => 'localhost:9252'
}
end

it { is_expected.to contain_file_line('gitlab-runner-metrics-server').that_requires("Package[#{package_name}]") }
it { is_expected.to contain_file_line('gitlab-runner-metrics-server').that_notifies('Exec[gitlab-runner-restart]') }
it { is_expected.to contain_file_line('gitlab-runner-listen-address').that_requires("Package[#{package_name}]") }
it { is_expected.to contain_file_line('gitlab-runner-listen-address').that_notifies('Exec[gitlab-runner-restart]') }
it do
is_expected.to contain_file_line('gitlab-runner-metrics-server').with('path' => '/etc/gitlab-runner/config.toml',
'line' => 'metrics_server = "localhost:9252"',
'match' => '^metrics_server = .+')
is_expected.to contain_file_line('gitlab-runner-listen-address').with('path' => '/etc/gitlab-runner/config.toml',
'line' => 'listen_address = "localhost:9252"',
'match' => '^listen_address = .+')
end
end
context 'with builds_dir => /tmp/builds_dir' do
Expand All @@ -83,7 +83,7 @@
it { is_expected.to contain_file_line('gitlab-runner-builds_dir').that_notifies('Exec[gitlab-runner-restart]') }
it do
is_expected.to contain_file_line('gitlab-runner-builds_dir').with('path' => '/etc/gitlab-runner/config.toml',
'line' => 'builds_dir = "/tmp/builds_dir"',
'line' => 'builds_dir = "/tmp/builds_dir"',
'match' => '^builds_dir = .+')
end
end
Expand All @@ -100,7 +100,7 @@
it { is_expected.to contain_file_line('gitlab-runner-cache_dir').that_notifies('Exec[gitlab-runner-restart]') }
it do
is_expected.to contain_file_line('gitlab-runner-cache_dir').with('path' => '/etc/gitlab-runner/config.toml',
'line' => 'cache_dir = "/tmp/cache_dir"',
'line' => 'cache_dir = "/tmp/cache_dir"',
'match' => '^cache_dir = .+')
end
end
Expand All @@ -117,7 +117,7 @@
it { is_expected.to contain_file_line('gitlab-runner-sentry_dsn').that_notifies('Exec[gitlab-runner-restart]') }
it do
is_expected.to contain_file_line('gitlab-runner-sentry_dsn').with('path' => '/etc/gitlab-runner/config.toml',
'line' => 'sentry_dsn = "https://123abc@localhost/1"',
'line' => 'sentry_dsn = "https://123abc@localhost/1"',
'match' => '^sentry_dsn = .+')
end
end
Expand Down