Add latest changes from gitlab-org/gitlab@15-0-stable-eev15.0.0-rc42

2 files changed, 89 insertions, 102 deletions

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