From aee0a117a889461ce8ced6fcf73207fe017f1d99 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Mon, 20 Dec 2021 13:37:47 +0000 Subject: Add latest changes from gitlab-org/gitlab@14-6-stable-ee --- .../commands/metrics_server/metrics_server_spec.rb | 73 +++++++ spec/commands/sidekiq_cluster/cli_spec.rb | 213 +++++++++++++++++++-- 2 files changed, 273 insertions(+), 13 deletions(-) create mode 100644 spec/commands/metrics_server/metrics_server_spec.rb (limited to 'spec/commands') diff --git a/spec/commands/metrics_server/metrics_server_spec.rb b/spec/commands/metrics_server/metrics_server_spec.rb new file mode 100644 index 00000000000..f3936e6b346 --- /dev/null +++ b/spec/commands/metrics_server/metrics_server_spec.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +require 'spec_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 + let(:config_file) { Tempfile.new('gitlab.yml') } + let(:config) do + { + 'test' => { + 'monitoring' => { + 'sidekiq_exporter' => { + 'address' => 'localhost', + 'enabled' => true, + 'port' => 3807 + } + } + } + } + end + + context 'with a running server' do + before do + # We need to send a request to localhost + WebMock.allow_net_connect! + + config_file.write(YAML.dump(config)) + config_file.close + + env = { + 'GITLAB_CONFIG' => config_file.path, + 'METRICS_SERVER_TARGET' => 'sidekiq', + 'WIPE_METRICS_DIR' => '1' + } + @pid = Process.spawn(env, 'bin/metrics-server', pgroup: true) + end + + after do + webmock_enable! + + if @pid + pgrp = Process.getpgid(@pid) + + Timeout.timeout(5) do + Process.kill('TERM', -pgrp) + Process.waitpid(@pid) + end + + expect(Gitlab::ProcessManagement.process_alive?(@pid)).to be(false) + end + rescue Errno::ESRCH => _ + # 'No such process' means the process died before + ensure + config_file.unlink + end + + it 'serves /metrics endpoint' do + expect do + Timeout.timeout(5) 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 + end + end +end diff --git a/spec/commands/sidekiq_cluster/cli_spec.rb b/spec/commands/sidekiq_cluster/cli_spec.rb index baa4a2b4ec3..148b8720740 100644 --- a/spec/commands/sidekiq_cluster/cli_spec.rb +++ b/spec/commands/sidekiq_cluster/cli_spec.rb @@ -12,8 +12,23 @@ RSpec.describe Gitlab::SidekiqCluster::CLI do # rubocop:disable RSpec/FilePath { env: 'test', directory: Dir.pwd, max_concurrency: 50, min_concurrency: 0, dryrun: false, timeout: timeout } end + let(:sidekiq_exporter_enabled) { false } + let(:sidekiq_exporter_port) { '3807' } + let(:sidekiq_health_checks_port) { '3807' } + before do stub_env('RAILS_ENV', 'test') + stub_config( + monitoring: { + sidekiq_exporter: { + enabled: sidekiq_exporter_enabled, + port: sidekiq_exporter_port + }, + sidekiq_health_checks: { + port: sidekiq_health_checks_port + } + } + ) end describe '#run' do @@ -241,12 +256,184 @@ RSpec.describe Gitlab::SidekiqCluster::CLI do # rubocop:disable RSpec/FilePath end end end + + context 'metrics server' do + let(:trapped_signals) { described_class::TERMINATE_SIGNALS + described_class::FORWARD_SIGNALS } + let(:metrics_dir) { Dir.mktmpdir } + + before do + stub_env('prometheus_multiproc_dir', metrics_dir) + end + + after do + FileUtils.rm_rf(metrics_dir, secure: true) + end + + context 'starting the server' do + context 'without --dryrun' do + context 'when there are no sidekiq_health_checks settings set' do + before do + stub_config( + monitoring: { + sidekiq_exporter: { + enabled: true, + port: sidekiq_exporter_port + } + } + ) + + allow(Gitlab::SidekiqCluster).to receive(:start) + allow(cli).to receive(:write_pid) + allow(cli).to receive(:trap_signals) + allow(cli).to receive(:start_loop) + end + + it 'does not start a sidekiq metrics server' do + expect(MetricsServer).not_to receive(:spawn) + + cli.run(%w(foo)) + end + + it 'rescues Settingslogic::MissingSetting' do + expect { cli.run(%w(foo)) }.not_to raise_error(Settingslogic::MissingSetting) + end + end + + context 'when the sidekiq_exporter.port setting is not set' do + before do + stub_config( + monitoring: { + sidekiq_exporter: { + enabled: true + }, + sidekiq_health_checks: { + port: sidekiq_health_checks_port + } + } + ) + + allow(Gitlab::SidekiqCluster).to receive(:start) + allow(cli).to receive(:write_pid) + allow(cli).to receive(:trap_signals) + allow(cli).to receive(:start_loop) + end + + it 'does not start a sidekiq metrics server' do + expect(MetricsServer).not_to receive(:spawn) + + cli.run(%w(foo)) + end + + it 'rescues Settingslogic::MissingSetting' do + expect { cli.run(%w(foo)) }.not_to raise_error(Settingslogic::MissingSetting) + end + end + + context 'when sidekiq_exporter.enabled setting is not set' do + before do + stub_config( + monitoring: { + sidekiq_exporter: {}, + sidekiq_health_checks: { + port: sidekiq_health_checks_port + } + } + ) + + allow(Gitlab::SidekiqCluster).to receive(:start) + allow(cli).to receive(:write_pid) + allow(cli).to receive(:trap_signals) + allow(cli).to receive(:start_loop) + end + + it 'does not start a sidekiq metrics server' do + expect(MetricsServer).not_to receive(:spawn) + + cli.run(%w(foo)) + end + end + + context 'with valid settings' do + using RSpec::Parameterized::TableSyntax + + 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 + end + + with_them do + before do + allow(Gitlab::SidekiqCluster).to receive(:start) + allow(cli).to receive(:write_pid) + allow(cli).to receive(:trap_signals) + allow(cli).to receive(:start_loop) + end + + specify do + if start_metrics_server + expect(MetricsServer).to receive(:spawn).with('sidekiq', metrics_dir: metrics_dir, wipe_metrics_dir: true, trapped_signals: trapped_signals) + else + expect(MetricsServer).not_to receive(:spawn) + end + + cli.run(%w(foo)) + end + end + end + end + + context 'with --dryrun set' do + let(:sidekiq_exporter_enabled) { true } + + it 'does not start the server' do + expect(MetricsServer).not_to receive(:spawn) + + cli.run(%w(foo --dryrun)) + end + end + end + + context 'supervising the server' do + let(:sidekiq_exporter_enabled) { true } + let(:sidekiq_health_checks_port) { '3907' } + + before do + allow(cli).to receive(:sleep).with(a_kind_of(Numeric)) + allow(MetricsServer).to receive(:spawn).and_return(99) + cli.start_metrics_server + end + + it 'stops the metrics server when one of the processes has been terminated' do + allow(Gitlab::ProcessManagement).to receive(:process_died?).and_return(false) + allow(Gitlab::ProcessManagement).to receive(:all_alive?).with(an_instance_of(Array)).and_return(false) + allow(Gitlab::ProcessManagement).to receive(:signal_processes).with(an_instance_of(Array), :TERM) + + expect(Process).to receive(:kill).with(:TERM, 99) + + cli.start_loop + end + + it 'starts the metrics server when it is down' do + allow(Gitlab::ProcessManagement).to receive(:process_died?).and_return(true) + allow(Gitlab::ProcessManagement).to receive(:all_alive?).with(an_instance_of(Array)).and_return(false) + allow(cli).to receive(:stop_metrics_server) + + expect(MetricsServer).to receive(:spawn).with( + 'sidekiq', metrics_dir: metrics_dir, wipe_metrics_dir: false, trapped_signals: trapped_signals + ) + + cli.start_loop + end + end + end end describe '#write_pid' do context 'when a PID is specified' do it 'writes the PID to a file' do - expect(Gitlab::SidekiqCluster).to receive(:write_pid).with('/dev/null') + expect(Gitlab::ProcessManagement).to receive(:write_pid).with('/dev/null') cli.option_parser.parse!(%w(-P /dev/null)) cli.write_pid @@ -255,7 +442,7 @@ RSpec.describe Gitlab::SidekiqCluster::CLI do # rubocop:disable RSpec/FilePath context 'when no PID is specified' do it 'does not write a PID' do - expect(Gitlab::SidekiqCluster).not_to receive(:write_pid) + expect(Gitlab::ProcessManagement).not_to receive(:write_pid) cli.write_pid end @@ -264,13 +451,13 @@ RSpec.describe Gitlab::SidekiqCluster::CLI do # rubocop:disable RSpec/FilePath describe '#wait_for_termination' do it 'waits for termination of all sub-processes and succeeds after 3 checks' do - expect(Gitlab::SidekiqCluster).to receive(:any_alive?) + expect(Gitlab::ProcessManagement).to receive(:any_alive?) .with(an_instance_of(Array)).and_return(true, true, true, false) - expect(Gitlab::SidekiqCluster).to receive(:pids_alive) + expect(Gitlab::ProcessManagement).to receive(:pids_alive) .with([]).and_return([]) - expect(Gitlab::SidekiqCluster).to receive(:signal_processes) + expect(Gitlab::ProcessManagement).to receive(:signal_processes) .with([], "-KILL") stub_const("Gitlab::SidekiqCluster::CHECK_TERMINATE_INTERVAL_SECONDS", 0.1) @@ -292,13 +479,13 @@ RSpec.describe Gitlab::SidekiqCluster::CLI do # rubocop:disable RSpec/FilePath .with([['foo']], default_options) .and_return(worker_pids) - expect(Gitlab::SidekiqCluster).to receive(:any_alive?) + expect(Gitlab::ProcessManagement).to receive(:any_alive?) .with(worker_pids).and_return(true).at_least(10).times - expect(Gitlab::SidekiqCluster).to receive(:pids_alive) + expect(Gitlab::ProcessManagement).to receive(:pids_alive) .with(worker_pids).and_return([102]) - expect(Gitlab::SidekiqCluster).to receive(:signal_processes) + expect(Gitlab::ProcessManagement).to receive(:signal_processes) .with([102], "-KILL") cli.run(%w(foo)) @@ -312,9 +499,9 @@ RSpec.describe Gitlab::SidekiqCluster::CLI do # rubocop:disable RSpec/FilePath end describe '#trap_signals' do - it 'traps the termination and forwarding signals' do - expect(Gitlab::SidekiqCluster).to receive(:trap_terminate) - expect(Gitlab::SidekiqCluster).to receive(:trap_forward) + it 'traps termination and sidekiq specific signals' do + expect(Gitlab::ProcessManagement).to receive(:trap_signals).with(%i[INT TERM]) + expect(Gitlab::ProcessManagement).to receive(:trap_signals).with(%i[TTIN USR1 USR2 HUP]) cli.trap_signals end @@ -324,10 +511,10 @@ RSpec.describe Gitlab::SidekiqCluster::CLI do # rubocop:disable RSpec/FilePath it 'runs until one of the processes has been terminated' do allow(cli).to receive(:sleep).with(a_kind_of(Numeric)) - expect(Gitlab::SidekiqCluster).to receive(:all_alive?) + expect(Gitlab::ProcessManagement).to receive(:all_alive?) .with(an_instance_of(Array)).and_return(false) - expect(Gitlab::SidekiqCluster).to receive(:signal_processes) + expect(Gitlab::ProcessManagement).to receive(:signal_processes) .with(an_instance_of(Array), :TERM) cli.start_loop -- cgit v1.2.3