From 36a59d088eca61b834191dacea009677a96c052f Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 19 May 2022 07:33:21 +0000 Subject: Add latest changes from gitlab-org/gitlab@15-0-stable-ee --- .../commands/metrics_server/metrics_server_spec.rb | 89 +++++++++++++----- spec/commands/sidekiq_cluster/cli_spec.rb | 102 +++++---------------- 2 files changed, 89 insertions(+), 102 deletions(-) (limited to 'spec/commands') diff --git a/spec/commands/metrics_server/metrics_server_spec.rb b/spec/commands/metrics_server/metrics_server_spec.rb index 217aa185767..f93be1d9f88 100644 --- a/spec/commands/metrics_server/metrics_server_spec.rb +++ b/spec/commands/metrics_server/metrics_server_spec.rb @@ -1,44 +1,83 @@ # frozen_string_literal: true require 'spec_helper' +require 'rake_helper' require_relative '../../../metrics_server/metrics_server' # End-to-end tests for the metrics server process we use to serve metrics # from forking applications (Sidekiq, Puma) to the Prometheus scraper. -RSpec.describe 'bin/metrics-server', :aggregate_failures do +RSpec.describe 'GitLab metrics server', :aggregate_failures do let(:config_file) { Tempfile.new('gitlab.yml') } + let(:address) { '127.0.0.1' } + let(:port) { 3807 } let(:config) do { 'test' => { 'monitoring' => { 'web_exporter' => { - 'address' => 'localhost', + 'address' => address, 'enabled' => true, - 'port' => 3807 + 'port' => port }, 'sidekiq_exporter' => { - 'address' => 'localhost', + 'address' => address, 'enabled' => true, - 'port' => 3807 + 'port' => port } } } } end - %w(puma sidekiq).each do |target| - context "with a running server targeting #{target}" do + before(:all) do + Rake.application.rake_require 'tasks/gitlab/metrics_exporter' + + @exporter_path = Rails.root.join('tmp', 'test', 'gme') + + run_rake_task('gitlab:metrics_exporter:install', @exporter_path) + end + + after(:all) do + FileUtils.rm_rf(@exporter_path) + end + + shared_examples 'serves metrics endpoint' do + it 'serves /metrics endpoint' do + start_server! + + expect do + Timeout.timeout(10) do + http_ok = false + until http_ok + sleep 1 + response = Gitlab::HTTP.try_get("http://#{address}:#{port}/metrics", allow_local_requests: true) + http_ok = response&.success? + end + end + end.not_to raise_error + end + end + + shared_examples 'spawns a server' do |target, use_golang_server| + context "targeting #{target} when using Golang server is #{use_golang_server}" do let(:metrics_dir) { Dir.mktmpdir } + subject(:start_server!) do + @pid = MetricsServer.spawn(target, metrics_dir: metrics_dir, path: @exporter_path.join('bin')) + end + before do + if use_golang_server + stub_env('GITLAB_GOLANG_METRICS_SERVER', '1') + allow(Settings).to receive(:monitoring).and_return(config.dig('test', 'monitoring')) + else + config_file.write(YAML.dump(config)) + config_file.close + stub_env('GITLAB_CONFIG', config_file.path) + end # We need to send a request to localhost WebMock.allow_net_connect! - - config_file.write(YAML.dump(config)) - config_file.close - - @pid = MetricsServer.spawn(target, metrics_dir: metrics_dir, gitlab_config: config_file.path, wipe_metrics_dir: true) end after do @@ -54,25 +93,25 @@ RSpec.describe 'bin/metrics-server', :aggregate_failures do expect(Gitlab::ProcessManagement.process_alive?(@pid)).to be(false) end - rescue Errno::ESRCH => _ - # 'No such process' means the process died before + rescue Errno::ESRCH, Errno::ECHILD => _ + # 'No such process' or 'No child processes' means the process died before ensure config_file.unlink FileUtils.rm_rf(metrics_dir, secure: true) end - it 'serves /metrics endpoint' do - expect do - Timeout.timeout(10) do - http_ok = false - until http_ok - sleep 1 - response = Gitlab::HTTP.try_get("http://localhost:3807/metrics", allow_local_requests: true) - http_ok = response&.success? - end - end - end.not_to raise_error + it_behaves_like 'serves metrics endpoint' + + context 'when using Pathname instance as target directory' do + let(:metrics_dir) { Pathname.new(Dir.mktmpdir) } + + it_behaves_like 'serves metrics endpoint' end end end + + it_behaves_like 'spawns a server', 'puma', true + it_behaves_like 'spawns a server', 'puma', false + it_behaves_like 'spawns a server', 'sidekiq', true + it_behaves_like 'spawns a server', 'sidekiq', false end diff --git a/spec/commands/sidekiq_cluster/cli_spec.rb b/spec/commands/sidekiq_cluster/cli_spec.rb index bbf5f2bc4d9..223d0c3b0ec 100644 --- a/spec/commands/sidekiq_cluster/cli_spec.rb +++ b/spec/commands/sidekiq_cluster/cli_spec.rb @@ -18,7 +18,6 @@ RSpec.describe Gitlab::SidekiqCluster::CLI, stub_settings_source: true do # rubo let(:sidekiq_exporter_enabled) { false } let(:sidekiq_exporter_port) { '3807' } - let(:sidekiq_health_checks_port) { '3807' } let(:config_file) { Tempfile.new('gitlab.yml') } let(:config) do @@ -29,11 +28,6 @@ RSpec.describe Gitlab::SidekiqCluster::CLI, stub_settings_source: true do # rubo 'address' => 'localhost', 'enabled' => sidekiq_exporter_enabled, 'port' => sidekiq_exporter_port - }, - 'sidekiq_health_checks' => { - 'address' => 'localhost', - 'enabled' => sidekiq_exporter_enabled, - 'port' => sidekiq_health_checks_port } } } @@ -310,63 +304,37 @@ RSpec.describe Gitlab::SidekiqCluster::CLI, stub_settings_source: true do # rubo cli.run(%w(foo)) end - context 'when there are no sidekiq_health_checks settings set' do - let(:sidekiq_exporter_enabled) { true } - - it 'does not start a sidekiq metrics server' do - expect(MetricsServer).not_to receive(:fork) - - cli.run(%w(foo)) - end - end - - context 'when the sidekiq_exporter.port setting is not set' do - let(:sidekiq_exporter_enabled) { true } - - it 'does not start a sidekiq metrics server' do - expect(MetricsServer).not_to receive(:fork) - - cli.run(%w(foo)) - end - end - - context 'when sidekiq_exporter.enabled setting is not set' do + context 'when sidekiq_exporter is not set up' do let(:config) do { 'test' => { 'monitoring' => { - 'sidekiq_exporter' => {}, - 'sidekiq_health_checks' => { - 'address' => 'localhost', - 'enabled' => sidekiq_exporter_enabled, - 'port' => sidekiq_health_checks_port - } + 'sidekiq_exporter' => {} } } } end it 'does not start a sidekiq metrics server' do - expect(MetricsServer).not_to receive(:fork) + expect(MetricsServer).not_to receive(:start_for_sidekiq) cli.run(%w(foo)) end end - context 'with a blank sidekiq_exporter setting' do + context 'with missing sidekiq_exporter setting' do let(:config) do { 'test' => { 'monitoring' => { - 'sidekiq_exporter' => nil, - 'sidekiq_health_checks' => nil + 'sidekiq_exporter' => nil } } } end it 'does not start a sidekiq metrics server' do - expect(MetricsServer).not_to receive(:fork) + expect(MetricsServer).not_to receive(:start_for_sidekiq) cli.run(%w(foo)) end @@ -376,26 +344,21 @@ RSpec.describe Gitlab::SidekiqCluster::CLI, stub_settings_source: true do # rubo end end - context 'with valid settings' do - using RSpec::Parameterized::TableSyntax + context 'when sidekiq_exporter is disabled' do + it 'does not start a sidekiq metrics server' do + expect(MetricsServer).not_to receive(:start_for_sidekiq) - where(:sidekiq_exporter_enabled, :sidekiq_exporter_port, :sidekiq_health_checks_port, :start_metrics_server) do - true | '3807' | '3907' | true - true | '3807' | '3807' | false - false | '3807' | '3907' | false - false | '3807' | '3907' | false + cli.run(%w(foo)) end + end + + context 'when sidekiq_exporter is enabled' do + let(:sidekiq_exporter_enabled) { true } - with_them do - specify do - if start_metrics_server - expect(MetricsServer).to receive(:fork).with('sidekiq', metrics_dir: metrics_dir, reset_signals: trapped_signals) - else - expect(MetricsServer).not_to receive(:fork) - end + it 'starts the metrics server' do + expect(MetricsServer).to receive(:start_for_sidekiq).with(metrics_dir: metrics_dir, reset_signals: trapped_signals) - cli.run(%w(foo)) - end + cli.run(%w(foo)) end end @@ -421,7 +384,7 @@ RSpec.describe Gitlab::SidekiqCluster::CLI, stub_settings_source: true do # rubo let(:sidekiq_exporter_enabled) { true } it 'does not start the server' do - expect(MetricsServer).not_to receive(:fork) + expect(MetricsServer).not_to receive(:start_for_sidekiq) cli.run(%w(foo --dryrun)) end @@ -431,7 +394,6 @@ RSpec.describe Gitlab::SidekiqCluster::CLI, stub_settings_source: true do # rubo context 'supervising the cluster' do let(:sidekiq_exporter_enabled) { true } - let(:sidekiq_health_checks_port) { '3907' } let(:metrics_server_pid) { 99 } let(:sidekiq_worker_pids) { [2, 42] } @@ -440,32 +402,18 @@ RSpec.describe Gitlab::SidekiqCluster::CLI, stub_settings_source: true do # rubo end it 'stops the entire process cluster if one of the workers has been terminated' do - expect(supervisor).to receive(:alive).and_return(true) - expect(supervisor).to receive(:supervise).and_yield([2]) - expect(MetricsServer).to receive(:fork).once.and_return(metrics_server_pid) - expect(Gitlab::ProcessManagement).to receive(:signal_processes).with([42, 99], :TERM) + expect(MetricsServer).to receive(:start_for_sidekiq).once.and_return(metrics_server_pid) + expect(supervisor).to receive(:supervise).and_yield([2, 99]) + expect(supervisor).to receive(:shutdown) cli.run(%w(foo)) end - context 'when the supervisor is alive' do - it 'restarts the metrics server when it is down' do - expect(supervisor).to receive(:alive).and_return(true) - expect(supervisor).to receive(:supervise).and_yield([metrics_server_pid]) - expect(MetricsServer).to receive(:fork).twice.and_return(metrics_server_pid) + it 'restarts the metrics server when it is down' do + expect(supervisor).to receive(:supervise).and_yield([metrics_server_pid]) + expect(MetricsServer).to receive(:start_for_sidekiq).twice.and_return(metrics_server_pid) - cli.run(%w(foo)) - end - end - - context 'when the supervisor is shutting down' do - it 'does not restart the metrics server' do - expect(supervisor).to receive(:alive).and_return(false) - expect(supervisor).to receive(:supervise).and_yield([metrics_server_pid]) - expect(MetricsServer).to receive(:fork).once.and_return(metrics_server_pid) - - cli.run(%w(foo)) - end + cli.run(%w(foo)) end end end -- cgit v1.2.3