Merge branch 'ashmckenzie/make-console-messages-consistent' into 'master'

Geo: make console messages consistent

See merge request gitlab-org/gitlab!16584

doc/administration/geo/replication/using_a_geo_server.md

@@ -10,8 +10,12 @@ Example of the output you will see when pushing to a **secondary** node:
 
 ```bash
 $ git push
-> GitLab: You're pushing to a Geo secondary.
-> GitLab: We'll help you by proxying this request to the primary: ssh://git@primary.geo/user/repo.git
+remote:
+remote: You're pushing to a Geo secondary. We'll help you by proxying this
+remote: request to the primary:
+remote:
+remote:   ssh://git@primary.geo/user/repo.git
+remote:
 Everything up-to-date
 ```
 

ee/lib/ee/gitlab/geo_git_access.rb

@@ -21,7 +21,7 @@ module EE
       override :check_for_console_messages
       def check_for_console_messages(cmd)
         super.push(
-          *current_replication_lag_message(cmd)
+          *current_replication_lag_message
         )
       end
 
@@ -33,18 +33,6 @@ module EE
 
       private
 
-      def current_replication_lag_message(cmd)
-        return unless upload_pack?(cmd) # git fetch / pull
-        return unless ::Gitlab::Database.read_only?
-        return unless current_replication_lag > 0
-
-        "Current replication lag: #{current_replication_lag} seconds"
-      end
-
-      def current_replication_lag
-        @current_replication_lag ||= ::Gitlab::Geo::HealthCheck.new.db_replication_lag_seconds
-      end
-
       def custom_action_for?(cmd)
         return unless receive_pack?(cmd) # git push
         return unless ::Gitlab::Database.read_only?
@@ -58,13 +46,21 @@ module EE
         payload = {
           'action' => 'geo_proxy_to_primary',
           'data' => {
-            'info_message' => proxying_to_primary_message,
             'api_endpoints' => custom_action_api_endpoints,
             'primary_repo' => primary_http_repo_url
           }
         }
 
-        ::Gitlab::GitAccessResult::CustomAction.new(payload, 'Attempting to proxy to primary.')
+        ::Gitlab::GitAccessResult::CustomAction.new(payload, messages)
+      end
+
+      def messages
+        messages = proxying_to_primary_message
+        lag_message = current_replication_lag_message
+
+        return messages unless lag_message
+
+        messages + ['', lag_message]
       end
 
       def push_to_read_only_message
@@ -95,7 +91,30 @@ module EE
       end
 
       def proxying_to_primary_message
-        "You're pushing to a Geo secondary.\nWe'll help you by proxying this request to the primary: #{primary_ssh_url_to_repo}"
+        # This is formatted like this to fit into the console 'box', e.g.
+        #
+        # remote:
+        # remote: You're pushing to a Geo secondary! We'll help you by proxying this
+        # remote: request to the primary:
+        # remote:
+        # remote:   ssh://<user>@<host>:<port>/<group>/<repo>.git
+        # remote:
+        <<~STR.split("\n")
+          You're pushing to a Geo secondary! We'll help you by proxying this
+          request to the primary:
+
+            #{primary_ssh_url_to_repo}
+        STR
+      end
+
+      def current_replication_lag_message
+        return if ::Gitlab::Database.read_write? || current_replication_lag.zero?
+
+        "Current replication lag: #{current_replication_lag} seconds"
+      end
+
+      def current_replication_lag
+        @current_replication_lag ||= ::Gitlab::Geo::HealthCheck.new.db_replication_lag_seconds
       end
 
       def custom_action_api_endpoints

ee/spec/support/shared_examples/git_access_shared_examples.rb

@@ -8,8 +8,9 @@ shared_examples 'a read-only GitLab instance' do
   end
 
   context 'for a Geo setup' do
+    let(:primary_node) { create(:geo_node, :primary, url: 'https://localhost:3000/gitlab') }
+
     before do
-      primary_node = create(:geo_node, :primary, url: 'https://localhost:3000/gitlab')
       allow(Gitlab::Geo).to receive(:primary).and_return(primary_node)
       allow(Gitlab::Geo).to receive(:secondary_with_primary?).and_return(secondary_with_primary)
     end
@@ -31,19 +32,26 @@ shared_examples 'a read-only GitLab instance' do
         {
           'action' => 'geo_proxy_to_primary',
           'data' => {
-            'info_message' => "You're pushing to a Geo secondary.\nWe'll help you by proxying this request to the primary: #{primary_repo_ssh_url}",
             'api_endpoints' => %w{/api/v4/geo/proxy_git_push_ssh/info_refs /api/v4/geo/proxy_git_push_ssh/push},
             'primary_repo' => primary_repo_url
           }
         }
       end
+      let(:console_messages) do
+        [
+          "You're pushing to a Geo secondary! We'll help you by proxying this",
+          "request to the primary:",
+          "",
+          "  #{primary_repo_ssh_url}"
+        ]
+      end
 
       it 'attempts to proxy to the primary' do
         project.add_maintainer(user)
 
         expect(push_changes).to be_a(Gitlab::GitAccessResult::CustomAction)
-        expect(push_changes.message).to eql('Attempting to proxy to primary.')
         expect(push_changes.payload).to eql(payload)
+        expect(push_changes.console_messages).to include(*console_messages)
       end
     end
   end

lib/api/internal/base.rb

@@ -77,7 +77,7 @@ module API
 
             response_with_status(**payload)
           when ::Gitlab::GitAccessResult::CustomAction
-            response_with_status(code: 300, message: check_result.message, payload: check_result.payload)
+            response_with_status(code: 300, payload: check_result.payload, gl_console_messages: check_result.console_messages)
           else
             response_with_status(code: 500, success: false, message: UNKNOWN_CHECK_RESULT_ERROR)
           end

lib/gitlab/git_access_result/custom_action.rb

@@ -3,7 +3,7 @@
 module Gitlab
   module GitAccessResult
     class CustomAction
-      attr_reader :payload, :message
+      attr_reader :payload, :console_messages
 
       # Example of payload:
       #
@@ -16,9 +16,9 @@ module Gitlab
       #   }
       # }
       #
-      def initialize(payload, message)
+      def initialize(payload, console_messages)
         @payload = payload
-        @message = message
+        @console_messages = console_messages
       end
     end
   end

spec/requests/api/internal/base_spec.rb

@@ -407,7 +407,6 @@ describe API::Internal::Base do
 
     context "custom action" do
       let(:access_checker) { double(Gitlab::GitAccess) }
-      let(:message) { 'CustomActionError message' }
       let(:payload) do
         {
           'action' => 'geo_proxy_to_primary',
@@ -418,8 +417,8 @@ describe API::Internal::Base do
           }
         }
       end
-
-      let(:custom_action_result) { Gitlab::GitAccessResult::CustomAction.new(payload, message) }
+      let(:console_messages) { ['informational message'] }
+      let(:custom_action_result) { Gitlab::GitAccessResult::CustomAction.new(payload, console_messages) }
 
       before do
         project.add_guest(user)
@@ -446,8 +445,8 @@ describe API::Internal::Base do
 
           expect(response).to have_gitlab_http_status(300)
           expect(json_response['status']).to be_truthy
-          expect(json_response['message']).to eql(message)
           expect(json_response['payload']).to eql(payload)
+          expect(json_response['gl_console_messages']).to eql(console_messages)
           expect(user.reload.last_activity_on).to be_nil
         end
       end