Merge branch 'mk/decouple-geo-node-identity-from-external-url' into 'master'

Allow multiple secondary nodes behind a load balancer

See merge request gitlab-org/gitlab-ee!10755

config/gitlab.yml.example

@@ -407,6 +407,16 @@ production: &base
     # dsn: https://<key>@sentry.io/<project>
     # environment: 'production' # e.g. development, staging, production
 
+  ## Geo
+  # NOTE: These settings will only take effect if Geo is enabled
+  geo:
+    # This is an optional identifier which Geo nodes can use to identify themselves.
+    # For example, if external_url is the same for two secondaries, you must specify
+    # a unique Geo node name for those secondaries.
+    #
+    # If it is blank, it defaults to external_url.
+    node_name: ''
+
   #
   # 2. GitLab CI settings
   # ==========================

config/initializers/1_settings.rb

@@ -268,6 +268,9 @@ Settings.pages.admin['certificate'] ||= ''
 #
 # Geo
 #
+Settings['geo'] ||= Settingslogic.new({})
+# For backwards compatibility, default to gitlab_url and if so, ensure it ends with "/"
+Settings.geo['node_name'] = Settings.geo['node_name'].presence || Settings.gitlab['url'].chomp('/').concat('/')
 
 #
 # External merge request diffs

db/schema.rb

@@ -1385,9 +1385,10 @@ ActiveRecord::Schema.define(version: 20190426180107) do
     t.integer "verification_max_capacity", default: 100, null: false
     t.integer "minimum_reverification_interval", default: 7, null: false
     t.string "internal_url"
+    t.string "name", null: false
     t.index ["access_key"], name: "index_geo_nodes_on_access_key", using: :btree
+    t.index ["name"], name: "index_geo_nodes_on_name", unique: true, using: :btree
     t.index ["primary"], name: "index_geo_nodes_on_primary", using: :btree
-    t.index ["url"], name: "index_geo_nodes_on_url", unique: true, using: :btree
   end
 
   create_table "geo_repositories_changed_events", force: :cascade do |t|

doc/api/geo_nodes.md

@@ -19,6 +19,7 @@ Example response:
 [
   {
     "id": 1,
+    "name": "us-node",
     "url": "https://primary.example.com/",
     "internal_url": "https://internal.example.com/",
     "primary": true,
@@ -31,6 +32,7 @@ Example response:
   },
   {
     "id": 2,
+    "name": "cn-node",
     "url": "https://secondary.example.com/",
     "internal_url": "https://secondary.example.com/",
     "primary": false,
@@ -59,6 +61,7 @@ Example response:
 ```json
 {
   "id": 1,
+  "name": "us-node",
   "url": "https://primary.example.com/",
   "internal_url": "https://primary.example.com/",
   "primary": true,
@@ -85,7 +88,8 @@ PUT /geo_nodes/:id
 |----------------------|---------|-----------|---------------------------------------------------------------------------|
 | `id`                 | integer | yes       | The ID of the Geo node.                                                   |
 | `enabled`            | boolean | no        | Flag indicating if the Geo node is enabled.                               |
-| `url`                | string  | no        | The URL to connect to the Geo node.                                       |
+| `name`               | string  | yes       | The unique identifier for the Geo node. Must match `geo_node_name` if it is set in gitlab.rb, otherwise it must match `external_url`. |
+| `url`                | string  | yes       | The user-facing URL of the Geo node. |
 | `internal_url`       | string  | no        | The URL defined on the primary node that secondary nodes should use to contact it. Returns `url` if not set.|
 | `files_max_capacity` | integer | no        | Control the maximum concurrency of LFS/attachment backfill for this secondary node. |
 | `repos_max_capacity` | integer | no        | Control the maximum concurrency of repository backfill for this secondary node.     |
@@ -96,6 +100,7 @@ Example response:
 ```json
 {
   "id": 1,
+  "name": "cn-node",
   "url": "https://secondary.example.com/",
   "internal_url": "https://secondary.example.com/",
   "primary": false,
@@ -138,6 +143,7 @@ Example response:
 ```json
 {
   "id": 1,
+  "name": "us-node",
   "url": "https://primary.example.com/",
   "internal_url": "https://primary.example.com/",
   "primary": true,

doc/user/admin_area/geo_nodes.md

@@ -12,7 +12,8 @@ All Geo nodes have the following settings:
 | Setting | Description |
 | --------| ----------- |
 | Primary | This marks a Geo Node as **primary** node. There can be only one **primary** node; make sure that you first add the **primary** node and then all the others. |
-| URL     | The instance's full URL, in the same way it is configured in  `/etc/gitlab/gitlab.rb` (Omnibus GitLab installations) or `gitlab.yml` (source based installations). |
+| Name    | The unique identifier for the Geo node. Must match the setting `gitlab_rails[geo_node_name]` in `/etc/gitlab/gitlab.rb`. The setting defaults to `external_url` with a trailing slash. |
+| URL     | The instance's user-facing URL. |
 
 The node you're reading from is indicated with a green `Current node` label, and
 the **primary** node is given a blue `Primary` label. Remember that you can only make
@@ -55,3 +56,15 @@ which is used by users. Internal URL does not need to be a private address.
 
 Internal URL defaults to External URL, but you can customize it under
 **Admin area > Geo Nodes**.
+
+## Multiple secondary nodes behind a load balancer
+
+In GitLab 11.11, **secondary** nodes can use identical external URLs as long as
+a unique `name` is set for each Geo node. The `gitlab.rb` setting
+`gitlab_rails[geo_node_name]` must:
+
+- Be set for each GitLab instance that runs `unicorn`, `sidekiq`, or `geo_logcursor`.
+- Match a Geo node name.
+
+The load balancer must use sticky sessions in order to avoid authentication
+failures and cross site request errors.

ee/app/controllers/admin/geo/nodes_controller.rb

@@ -47,6 +47,7 @@ class Admin::Geo::NodesController < Admin::Geo::ApplicationController
 
   def geo_node_params
     params.require(:geo_node).permit(
+      :name,
       :url,
       :internal_url,
       :primary,

ee/app/controllers/ee/sessions_controller.rb

@@ -14,7 +14,7 @@ module EE
       return super if signed_in?
 
       if ::Gitlab::Geo.secondary_with_primary?
-        redirect_to oauth_geo_auth_url(host: ::Gitlab::Geo.current_node.url, state: geo_login_state.encode)
+        redirect_to oauth_geo_auth_url(host: GeoNode.current_node_url, state: geo_login_state.encode)
       else
         super
       end

ee/app/models/geo_node.rb

@@ -3,6 +3,7 @@
 class GeoNode < ApplicationRecord
   include Presentable
   include Geo::SelectiveSync
+  include StripAttribute
 
   SELECTIVE_SYNC_TYPES = %w[namespaces shards].freeze
 
@@ -15,7 +16,8 @@ class GeoNode < ApplicationRecord
   has_many :namespaces, through: :geo_node_namespace_links
   has_one :status, class_name: 'GeoNodeStatus'
 
-  validates :url, presence: true, uniqueness: { case_sensitive: false }, addressable_url: true
+  validates :name, presence: true, uniqueness: { case_sensitive: false }, length: { maximum: 255 }
+  validates :url, presence: true, addressable_url: true
   validates :internal_url, addressable_url: true, allow_blank: true, allow_nil: true
 
   validates :primary, uniqueness: { message: 'node already exists' }, if: :primary
@@ -54,22 +56,27 @@ class GeoNode < ApplicationRecord
                  mode: :per_attribute_iv,
                  encode: true
 
+  strip_attributes :name
+
   class << self
+    # Set in gitlab.rb as external_url
     def current_node_url
       RequestStore.fetch('geo_node:current_node_url') do
-        cfg = Gitlab.config.gitlab
-
-        uri = URI.parse("#{cfg.protocol}://#{cfg.host}:#{cfg.port}#{cfg.relative_url_root}")
-        uri.path += '/' unless uri.path.end_with?('/')
+        Gitlab.config.gitlab.url
+      end
+    end
 
-        uri.to_s
+    # Set in gitlab.rb as geo_node_name
+    def current_node_name
+      RequestStore.fetch('geo_node:current_node_name') do
+        Gitlab.config.geo.node_name
       end
     end
 
     def current_node
-      return unless column_names.include?('url')
+      return unless column_names.include?('name')
 
-      GeoNode.find_by(url: current_node_url)
+      GeoNode.find_by(name: current_node_name)
     end
 
     def primary_node
@@ -107,7 +114,7 @@ class GeoNode < ApplicationRecord
   end
 
   def current?
-    self.class.current_node_url == url
+    self.class.current_node_name == name
   end
 
   def secondary?
@@ -275,7 +282,7 @@ class GeoNode < ApplicationRecord
 
   # Prevent locking yourself out
   def check_not_adding_primary_as_secondary
-    if url == self.class.current_node_url
+    if name == self.class.current_node_name
       errors.add(:base, 'Current node must be the primary node or you will be locking yourself out')
     end
   end

ee/app/services/geo/metrics_update_service.rb

@@ -70,7 +70,16 @@ module Geo
     end
 
     def metric_labels(node)
-      { url: node.url }
+      labels = { name: node.name }
+
+      # Installations that existed before 11.11 were using the `url` label. This
+      # line preserves continuity of metrics.
+      #
+      # This can be removed in 12.0+ since there will have been at least one
+      # release worth of data labeled with `name`.
+      labels[:url] = node.name
+
+      labels
     end
 
     def prometheus_enabled?

ee/app/views/admin/geo/nodes/_form.html.haml

 = form_errors(geo_node)
 .form-row.form-group
+  .form-group.col-sm-6
+    = form.label :name, _('Name'), class: 'font-weight-bold'
+    = form.text_field :name, class: 'form-control'
+    .form-text.text-muted= _('The unique identifier for the Geo node. Must match `geo_node_name` if it is set in gitlab.rb, otherwise it must match `external_url`')
+
   .form-group.col-sm-6
     = form.label :url, s_('Geo|URL'), class: 'font-weight-bold'
     = form.text_field :url, class: 'form-control'
+    .form-text.text-muted= _('The user-facing URL of the Geo node.')
 
   .form-group.col-sm-6.js-internal-url{ class: ('hidden' unless geo_node.primary?) }
     = form.label :internal_url, s_('Geo|Internal URL'), class: 'font-weight-bold'

ee/changelogs/unreleased/mk-decouple-geo-node-identity-from-external-url.yml

+---
+title: Allow multiple secondary nodes behind a load balancer
+merge_request: 10755
+author:
+type: added

ee/db/migrate/20190409224933_add_name_to_geo_nodes.rb

+# frozen_string_literal: true
+
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddNameToGeoNodes < ActiveRecord::Migration[5.0]
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  def up
+    add_column :geo_nodes, :name, :string
+
+    # url is also unique, and its type and size is identical to the name column,
+    # so this is safe.
+    execute "UPDATE geo_nodes SET name = url;"
+
+    # url is also `null: false`, so this is safe.
+    change_column :geo_nodes, :name, :string, null: false
+  end
+
+  def down
+    remove_column :geo_nodes, :name
+  end
+end

ee/db/migrate/20190410173409_add_name_index_to_geo_nodes.rb

+# frozen_string_literal: true
+
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddNameIndexToGeoNodes < ActiveRecord::Migration[5.0]
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :geo_nodes, :name, unique: true
+  end
+
+  def down
+    remove_concurrent_index :geo_nodes, :name
+  end
+end

ee/db/migrate/20190412183653_remove_url_index_from_geo_nodes.rb

+# frozen_string_literal: true
+
+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class RemoveUrlIndexFromGeoNodes < ActiveRecord::Migration[5.0]
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    remove_concurrent_index :geo_nodes, :url
+  end
+
+  def down
+    add_concurrent_index :geo_nodes, :url, unique: true
+  end
+end

ee/lib/api/geo_nodes.rb

@@ -143,7 +143,8 @@ module API
         end
         params do
           optional :enabled, type: Boolean, desc: 'Flag indicating if the Geo node is enabled'
-          optional :url, type: String, desc: 'The URL to connect to the Geo node'
+          optional :name, type: String, desc: 'The unique identifier for the Geo node. Must match `geo_node_name` if it is set in gitlab.rb, otherwise it must match `external_url`'
+          optional :url, type: String, desc: 'The user-facing URL of the Geo node'
           optional :internal_url, type: String, desc: 'The URL defined on the primary node that secondary nodes should use to contact it. Defaults to url'
           optional :files_max_capacity, type: Integer, desc: 'Control the maximum concurrency of LFS/attachment backfill for this secondary node'
           optional :repos_max_capacity, type: Integer, desc: 'Control the maximum concurrency of repository backfill for this secondary node'

ee/lib/ee/api/entities.rb

@@ -456,6 +456,7 @@ module EE
         include ::API::Helpers::RelatedResourcesHelpers
 
         expose :id
+        expose :name
         expose :url
         expose :internal_url
         expose :primary?, as: :primary

ee/lib/gitlab/geo/geo_tasks.rb

@@ -6,8 +6,8 @@ module Gitlab
       extend self
 
       def set_primary_geo_node
-        node = GeoNode.new(primary: true, url: GeoNode.current_node_url)
-        $stdout.puts "Saving primary Geo node with URL #{node.url} ..."
+        node = GeoNode.new(primary: true, name: GeoNode.current_node_name, url: GeoNode.current_node_url)
+        $stdout.puts "Saving primary Geo node with name #{node.name} and URL #{node.url} ..."
         node.save
 
         if node.persisted?

ee/lib/tasks/geo.rake

@@ -217,7 +217,8 @@ namespace :geo do
     end
 
     puts
-    puts GeoNode.current_node_url
+    puts "Name: #{GeoNode.current_node_name}"
+    puts "URL: #{GeoNode.current_node_url}"
     puts '-----------------------------------------------------'.color(:yellow)
 
     unless Gitlab::Database.postgresql_minimum_supported_version?

ee/spec/controllers/ee/sessions_controller_spec.rb

@@ -27,7 +27,7 @@ describe SessionsController, :geo do
           redirect_params = CGI.parse(redirect_uri.query)
 
           expect(response).to have_gitlab_http_status(302)
-          expect(response).to redirect_to %r(\A#{primary_node.url}oauth/geo/auth)
+          expect(response).to redirect_to %r(\A#{Gitlab.config.gitlab.url}/oauth/geo/auth)
           expect(redirect_params['state'].first).to end_with(':')
         end
       end

ee/spec/controllers/oauth/geo_auth_controller_spec.rb

@@ -82,7 +82,7 @@ describe Oauth::GeoAuthController, :geo do
           it "redirects to primary node's oauth endpoint" do
             get :callback, params: { state: login_state }
 
-            expect(response).to redirect_to('/')
+            expect(response).to redirect_to(secondary_node.uri.path)
           end
         end
 

ee/spec/factories/geo_nodes.rb

 FactoryBot.define do
   factory :geo_node do
-    # Start at a number higher than the current port to avoid the GeoNode
-    # "lock out" validation
-    sequence(:url, Gitlab.config.gitlab.port + 1) do |port|
-      uri = URI.parse("http://#{Gitlab.config.gitlab.host}:#{Gitlab.config.gitlab.relative_url_root}")
-      uri.port = port
-      uri.path += '/' unless uri.path.end_with?('/')
+    sequence(:url) do |n|
+      "http://node#{n}.example.com/gitlab"
+    end
 
-      uri.to_s
+    sequence(:name) do |n|
+      "node_name_#{n}"
     end
 
     primary false
@@ -15,14 +13,6 @@ FactoryBot.define do
     trait :primary do
       primary true
       minimum_reverification_interval 7
-
-      url do
-        uri = URI.parse("http://#{Gitlab.config.gitlab.host}:#{Gitlab.config.gitlab.relative_url_root}")
-        uri.port = Gitlab.config.gitlab.port
-        uri.path += '/' unless uri.path.end_with?('/')
-
-        uri.to_s
-      end
     end
   end
 end

ee/spec/features/admin/geo/admin_geo_nodes_spec.rb

@@ -77,6 +77,7 @@ describe 'admin Geo Nodes', :js, :geo do
 
     it 'creates a new Geo Node' do
       check 'This is a primary node'
+      fill_in 'geo_node_name', with: 'a node name'
       fill_in 'geo_node_url', with: 'https://test.gitlab.com'
       click_button 'Add Node'
 

ee/spec/fixtures/api/schemas/public_api/v4/geo_node.json

@@ -2,6 +2,7 @@
   "type": "object",
   "required" : [
     "id",
+    "name",
     "url",
     "primary",
     "enabled",
@@ -14,6 +15,7 @@
   ],
   "properties" : {
     "id": { "type": "integer" },
+    "name": { "type": ["string", "null"] },
     "url": { "type": ["string", "null"] },
     "primary": { "type": "boolean" },
     "internal_url": { "type": ["string", "null"] },

ee/spec/lib/gitlab/geo/geo_tasks_spec.rb

@@ -3,6 +3,7 @@ require 'spec_helper'
 describe Gitlab::Geo::GeoTasks do
   describe '.set_primary_geo_node' do
     before do
+      allow(GeoNode).to receive(:current_node_name).and_return('https://primary.geo.example.com')
       allow(GeoNode).to receive(:current_node_url).and_return('https://primary.geo.example.com')
     end
 

ee/spec/lib/gitlab/geo/jwt_request_decoder_spec.rb

 require 'spec_helper'
 
 describe Gitlab::Geo::JwtRequestDecoder do
+  include EE::GeoHelpers
+
   let!(:primary_node) { FactoryBot.create(:geo_node, :primary) }
   let(:data) { { input: 123 } }
   let(:request) { Gitlab::Geo::TransferRequest.new(data) }
 
   subject { described_class.new(request.headers['Authorization']) }
 
+  before do
+    stub_current_geo_node(primary_node)
+  end
+
   describe '#decode' do
     it 'decodes correct data' do
       expect(subject.decode).to eq(data)

ee/spec/lib/gitlab/geo_spec.rb

@@ -18,6 +18,8 @@ describe Gitlab::Geo, :geo, :request_store do
 
   describe '.current_node' do
     it 'returns a GeoNode instance' do
+      expect(GeoNode).to receive(:current_node).and_return(primary_node)
+
       expect(described_class.current_node).to eq(primary_node)
     end
 
@@ -42,6 +44,10 @@ describe Gitlab::Geo, :geo, :request_store do
 
   describe '.primary?' do
     context 'when current node is a primary node' do
+      before do
+        stub_current_geo_node(primary_node)
+      end
+
       it 'returns true' do
         expect(described_class.primary?).to be_truthy
       end

ee/spec/models/ci/job_artifact_spec.rb

@@ -3,11 +3,15 @@
 require 'spec_helper'
 
 describe Ci::JobArtifact do
+  include EE::GeoHelpers
+
   describe '#destroy' do
     set(:primary) { create(:geo_node, :primary) }
     set(:secondary) { create(:geo_node) }
 
     it 'creates a JobArtifactDeletedEvent' do
+      stub_current_geo_node(primary)
+
       job_artifact = create(:ci_job_artifact, :archive)
 
       expect do

ee/spec/models/geo_node_spec.rb

@@ -23,12 +23,17 @@ describe GeoNode, :geo, type: :model do
   end
 
   context 'validations' do
+    subject { build(:geo_node) }
+
     it { is_expected.to validate_inclusion_of(:selective_sync_type).in_array([nil, *GeoNode::SELECTIVE_SYNC_TYPES]) }
     it { is_expected.to validate_numericality_of(:repos_max_capacity).is_greater_than_or_equal_to(0) }
     it { is_expected.to validate_numericality_of(:files_max_capacity).is_greater_than_or_equal_to(0) }
     it { is_expected.to validate_numericality_of(:verification_max_capacity).is_greater_than_or_equal_to(0) }
     it { is_expected.to validate_numericality_of(:minimum_reverification_interval).is_greater_than_or_equal_to(1) }
+    it { is_expected.to validate_presence_of(:name) }
     it { is_expected.to validate_presence_of(:url) }
+    it { is_expected.to validate_uniqueness_of(:name).case_insensitive }
+    it { is_expected.to validate_length_of(:name).is_at_most(255) }
 
     context 'when validating primary node' do
       it 'cannot be disabled' do
@@ -59,6 +64,13 @@ describe GeoNode, :geo, type: :model do
 
         it { is_expected.not_to be_valid }
       end
+
+      context 'when an existing GeoNode has the same url but different name' do
+        let!(:existing) { new_node }
+        let(:url) { new_node.url }
+
+        it { is_expected.to be_valid }
+      end
     end
 
     context 'when validating internal_url' do
@@ -97,7 +109,8 @@ describe GeoNode, :geo, type: :model do
 
   context 'prevent locking yourself out' do
     it 'does not accept adding a non primary node with same details as current_node' do
-      node = build(:geo_node, :primary, primary: false)
+      stub_geo_setting(node_name: 'foo')
+      node = build(:geo_node, :primary, primary: false, name: 'foo')
 
       expect(node).not_to be_valid
       expect(node.errors.full_messages.count).to eq(1)
@@ -318,13 +331,13 @@ describe GeoNode, :geo, type: :model do
 
   describe '#current?' do
     it 'returns true when node is the current node' do
-      node = described_class.new(url: described_class.current_node_url)
+      node = described_class.new(name: described_class.current_node_name)
 
       expect(node.current?).to be_truthy
     end
 
     it 'returns false when node is not the current node' do
-      node = described_class.new(url: 'http://another.node.com:8080/foo')
+      node = described_class.new(name: 'some other node')
 
       expect(node.current?).to be_falsy
     end
@@ -658,4 +671,16 @@ describe GeoNode, :geo, type: :model do
       is_expected.to be_falsy
     end
   end
+
+  describe '#name=' do
+    context 'before validation' do
+      it 'strips leading and trailing whitespace' do
+        node = build(:geo_node)
+        node.name = " foo\n\n "
+        node.valid?
+
+        expect(node.name).to eq('foo')
+      end
+    end
+  end
 end

ee/spec/models/lfs_object_spec.rb

 require 'spec_helper'
 
 describe LfsObject do
+  include EE::GeoHelpers
+
   describe '#destroy' do
     subject { create(:lfs_object, :with_file) }
 
@@ -9,6 +11,8 @@ describe LfsObject do
       set(:secondary) { create(:geo_node) }
 
       it 'logs an event to the Geo event log' do
+        stub_current_geo_node(primary)
+
         expect { subject.destroy }.to change(Geo::LfsObjectDeletedEvent, :count).by(1)
       end
     end

ee/spec/models/namespace_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 describe Namespace do
+  include EE::GeoHelpers
+
   let!(:namespace) { create(:namespace) }
   let!(:free_plan) { create(:free_plan) }
   let!(:bronze_plan) { create(:bronze_plan) }
@@ -148,6 +150,8 @@ describe Namespace do
         allow(gitlab_shell).to receive(:mv_namespace)
           .with(project_legacy.repository_storage, full_path_was, new_path)
           .and_return(true)
+
+        stub_current_geo_node(primary)
       end
 
       it 'logs the Geo::RepositoryRenamedEvent for each project inside namespace' do

ee/spec/models/upload_spec.rb

 require 'spec_helper'
 
 describe Upload do
+  include EE::GeoHelpers
+
   describe '#destroy' do
     subject { create(:upload, checksum: '8710d2c16809c79fee211a9693b64038a8aae99561bc86ce98a9b46b45677fe4') }
 
@@ -9,6 +11,8 @@ describe Upload do
       set(:secondary) { create(:geo_node) }
 
       it 'logs an event to the Geo event log' do
+        stub_current_geo_node(primary)
+
         expect { subject.destroy }.to change(Geo::UploadDeletedEvent, :count).by(1)
       end
     end

ee/spec/requests/api/geo_nodes_spec.rb

@@ -170,7 +170,8 @@ describe API::GeoNodes, :geo, :prometheus, api: true do
     end
 
     it 'returns 200 for the primary node' do
-      expect(GeoNodeStatus).to receive(:fast_current_node_status).and_return(secondary_status)
+      set_current_geo_node!(primary)
+      create(:geo_node_status, :healthy, geo_node: primary)
 
       post api("/geo_nodes/#{primary.id}/repair", admin)
 

ee/spec/requests/api/geo_spec.rb

@@ -17,6 +17,10 @@ describe API::Geo do
     Gitlab::Geo::TransferRequest.new(transfer.request_data.merge(file_id: 100000)).headers
   end
 
+  before do
+    stub_current_geo_node(primary_node)
+  end
+
   shared_examples 'with terms enforced' do
     before do
       enforce_terms

ee/spec/requests/git_http_geo_spec.rb

@@ -12,7 +12,7 @@ describe "Git HTTP requests (Geo)", :geo do
   set(:secondary) { create(:geo_node) }
 
   # Ensure the token always comes from the real time of the request
-  let!(:auth_token) { Gitlab::Geo::BaseRequest.new(scope: project.full_path).authorization }
+  let(:auth_token) { Gitlab::Geo::BaseRequest.new(scope: project.full_path).authorization }
   let!(:user) { create(:user) }
   let!(:user_without_any_access) { create(:user) }
   let!(:user_without_push_access) { create(:user) }
@@ -29,6 +29,9 @@ describe "Git HTTP requests (Geo)", :geo do
 
     stub_licensed_features(geo: true)
     stub_current_geo_node(current_node)
+
+    # Current Geo node must be stubbed before this is instantiated
+    auth_token
   end
 
   shared_examples_for 'Geo request' do

ee/spec/services/geo/file_upload_service_spec.rb

 require 'spec_helper'
 
 describe Geo::FileUploadService do
+  include EE::GeoHelpers
+
   set(:node) { create(:geo_node, :primary) }
 
   let(:transfer_request) { Gitlab::Geo::TransferRequest.new(request_data) }
   let(:req_header) { transfer_request.headers['Authorization'] }
 
+  before do
+    stub_current_geo_node(node)
+  end
+
   shared_examples 'no authorization header' do
     it 'returns nil' do
       service = described_class.new(params, nil)

ee/spec/services/geo/metrics_update_service_spec.rb

@@ -116,9 +116,10 @@ describe Geo::MetricsUpdateService, :geo, :prometheus do
 
         expect(Gitlab::Metrics.registry.get(:geo_db_replication_lag_seconds).values.count).to eq(2)
         expect(Gitlab::Metrics.registry.get(:geo_repositories).values.count).to eq(3)
-        expect(Gitlab::Metrics.registry.get(:geo_repositories).get({ url: secondary.url })).to eq(10)
-        expect(Gitlab::Metrics.registry.get(:geo_repositories).get({ url: another_secondary.url })).to eq(10)
-        expect(Gitlab::Metrics.registry.get(:geo_repositories).get({ url: primary.url })).to eq(10)
+
+        expect(Gitlab::Metrics.registry.get(:geo_repositories).get({ name: secondary.name, url: secondary.name })).to eq(10)
+        expect(Gitlab::Metrics.registry.get(:geo_repositories).get({ name: another_secondary.name, url: another_secondary.name })).to eq(10)
+        expect(Gitlab::Metrics.registry.get(:geo_repositories).get({ name: primary.name, url: primary.name })).to eq(10)
       end
 
       it 'updates the GeoNodeStatus entry' do
@@ -202,7 +203,7 @@ describe Geo::MetricsUpdateService, :geo, :prometheus do
       end
 
       def metric_value(metric_name)
-        Gitlab::Metrics.registry.get(metric_name)&.get({ url: secondary.url })
+        Gitlab::Metrics.registry.get(metric_name)&.get({ name: secondary.name, url: secondary.name })
       end
     end
   end

ee/spec/services/geo/node_create_service_spec.rb

@@ -3,32 +3,32 @@ require 'spec_helper'
 describe Geo::NodeCreateService do
   describe '#execute' do
     it 'creates a new node with valid params' do
-      service = described_class.new(url: 'http://example.com')
+      service = described_class.new(name: 'foo', url: 'http://example.com')
 
       expect { service.execute }.to change(GeoNode, :count).by(1)
     end
 
     it 'does not create a node with invalid params' do
-      service = described_class.new(url: 'ftp://example.com')
+      service = described_class.new(name: 'foo', url: 'ftp://example.com')
 
       expect { service.execute }.not_to change(GeoNode, :count)
     end
 
     it 'returns true when creation succeeds' do
-      service = described_class.new(url: 'http://example.com')
+      service = described_class.new(name: 'foo', url: 'http://example.com')
 
       expect(service.execute.persisted?).to eq true
     end
 
     it 'returns false when creation fails' do
-      service = described_class.new(url: 'ftp://example.com')
+      service = described_class.new(name: 'foo', url: 'ftp://example.com')
 
       expect(service.execute.persisted?).to eq false
     end
 
     it 'parses the namespace_ids when node have namespace restrictions' do
       groups = create_list(:group, 2)
-      params = { url: 'http://example.com', namespace_ids: groups.map(&:id).join(',') }
+      params = { name: 'foo', url: 'http://example.com', namespace_ids: groups.map(&:id).join(',') }
       service = described_class.new(params)
 
       service.execute

ee/spec/services/geo/node_status_post_service_spec.rb

@@ -10,6 +10,10 @@ describe Geo::NodeStatusPostService, :geo do
   subject { described_class.new }
 
   describe '#execute' do
+    before do
+      stub_current_geo_node(primary)
+    end
+
     it 'parses a 401 response' do
       response = double(success?: false,
                         code: 401,
@@ -59,8 +63,6 @@ describe Geo::NodeStatusPostService, :geo do
     end
 
     it 'does not include id in the payload' do
-      stub_current_geo_node(primary)
-
       expect(Gitlab::HTTP).to receive(:post)
                                 .with(
                                   primary.status_url,
@@ -76,8 +78,6 @@ describe Geo::NodeStatusPostService, :geo do
     end
 
     it 'sends geo_node_id in the request' do
-      stub_current_geo_node(primary)
-
       expect(Gitlab::HTTP).to receive(:post)
                                 .with(
                                   primary.status_url,

ee/spec/services/geo/node_update_service_spec.rb

 require 'spec_helper'
 
 describe Geo::NodeUpdateService do
+  include EE::GeoHelpers
+
   set(:primary) { create(:geo_node, :primary) }
 
   let(:geo_node) { create(:geo_node) }
   let(:groups) { create_list(:group, 2) }
   let(:namespace_ids) { groups.map(&:id).join(',') }
 
+  before do
+    stub_current_geo_node(primary)
+  end
+
   describe '#execute' do
     it 'updates the node' do
       params = { url: 'http://example.com' }

ee/spec/services/projects/after_rename_service_spec.rb

@@ -3,6 +3,8 @@
 require 'spec_helper'
 
 describe Projects::AfterRenameService do
+  include EE::GeoHelpers
+
   describe '#execute' do
     context 'when running on a primary node' do
       set(:primary) { create(:geo_node, :primary) }
@@ -12,6 +14,10 @@ describe Projects::AfterRenameService do
       let!(:full_path_before_rename) { project.full_path }
       let(:path_after_rename) { "#{project.path}-renamed" }
 
+      before do
+        stub_current_geo_node(primary)
+      end
+
       it 'logs the Geo::RepositoryRenamedEvent for project backed by hashed storage' do
         expect { service_execute }.to change(Geo::RepositoryRenamedEvent, :count)
       end

ee/spec/services/projects/create_service_spec.rb

 require 'spec_helper'
 
 describe Projects::CreateService, '#execute' do
+  include EE::GeoHelpers
+
   let(:user) { create :user }
   let(:opts) do
     {
@@ -197,6 +199,10 @@ describe Projects::CreateService, '#execute' do
     set(:primary) { create(:geo_node, :primary) }
     set(:secondary) { create(:geo_node) }
 
+    before do
+      stub_current_geo_node(primary)
+    end
+
     it 'logs an event to the Geo event log' do
       expect { create_project(user, opts) }.to change(Geo::RepositoryCreatedEvent, :count).by(1)
     end

ee/spec/services/projects/destroy_service_spec.rb

 require 'spec_helper'
 
 describe Projects::DestroyService do
+  include EE::GeoHelpers
+
   let!(:user) { create(:user) }
   let!(:project) { create(:project, :repository, namespace: user.namespace) }
   let!(:project_id) { project.id }
@@ -33,6 +35,10 @@ describe Projects::DestroyService do
     set(:primary) { create(:geo_node, :primary) }
     set(:secondary) { create(:geo_node) }
 
+    before do
+      stub_current_geo_node(primary)
+    end
+
     it 'logs an event to the Geo event log' do
       # Run Sidekiq immediately to check that renamed repository will be removed
       Sidekiq::Testing.inline! do

ee/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb

 require 'spec_helper'
 
 describe Projects::HashedStorage::MigrateAttachmentsService do
+  include EE::GeoHelpers
+
   let(:project) { create(:project, storage_version: 1) }
   let(:legacy_storage) { Storage::LegacyProject.new(project) }
   let(:hashed_storage) { Storage::HashedProject.new(project) }
   let(:old_attachments_path) { legacy_storage.disk_path }
   let(:new_attachments_path) { hashed_storage.disk_path }
   let(:service) { described_class.new(project, old_attachments_path) }
+  set(:primary) { create(:geo_node, :primary) }
+  set(:secondary) { create(:geo_node) }
 
-  describe '#execute' do
-    set(:primary) { create(:geo_node, :primary) }
-    set(:secondary) { create(:geo_node) }
+  before do
+    stub_current_geo_node(primary)
+  end
 
+  describe '#execute' do
     context 'on success' do
       before do
         TestEnv.clean_test_path

ee/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb

 require 'spec_helper'
 
 describe Projects::HashedStorage::MigrateRepositoryService do
+  include EE::GeoHelpers
+
   let(:project) { create(:project, :empty_repo, :wiki_repo, :legacy_storage) }
   let(:legacy_storage) { Storage::LegacyProject.new(project) }
   let(:hashed_storage) { Storage::HashedProject.new(project) }
@@ -12,6 +14,7 @@ describe Projects::HashedStorage::MigrateRepositoryService do
 
     before do
       TestEnv.clean_test_path
+      stub_current_geo_node(primary)
     end
 
     it 'creates a Geo::HashedStorageMigratedEvent on success' do

ee/spec/services/projects/transfer_service_spec.rb

 require 'spec_helper'
 
 describe Projects::TransferService do
+  include EE::GeoHelpers
+
   let(:user) { create(:user) }
   let(:group) { create(:group) }
   let(:project) { create(:project, :repository, namespace: user.namespace) }
@@ -16,6 +18,8 @@ describe Projects::TransferService do
     set(:secondary) { create(:geo_node) }
 
     it 'logs an event to the Geo event log' do
+      stub_current_geo_node(primary)
+
       expect { subject.execute(group) }.to change(Geo::RepositoryRenamedEvent, :count).by(1)
     end
   end

ee/spec/support/helpers/ee/geo_helpers.rb

 module EE
   module GeoHelpers
+    # Actually sets the specified node to be the current one, so it works on new
+    # instances of GeoNode, unlike stub_current_geo_node. But this is slower.
+    def set_current_geo_node!(node)
+      node.name = GeoNode.current_node_name
+      node.save!(validate: false)
+    end
+
     def stub_current_geo_node(node)
       allow(::Gitlab::Geo).to receive(:current_node).and_return(node)
       allow(node).to receive(:current?).and_return(true) unless node.nil?

ee/spec/support/helpers/ee/stub_configuration.rb

@@ -25,5 +25,9 @@ module EE
         allow(object).to receive_message_chain(:current_application_settings, setting) { value }
       end
     end
+
+    def stub_geo_setting(messages)
+      allow(::Gitlab.config.geo).to receive_messages(to_settings(messages))
+    end
   end
 end

ee/spec/tasks/geo_rake_spec.rb

@@ -10,7 +10,8 @@ describe 'geo rake tasks', :geo do
 
   describe 'set_primary_node task' do
     before do
-      stub_config_setting(protocol: 'https')
+      stub_config_setting(url: 'https://example.com:1234/relative_part')
+      stub_geo_setting(node_name: 'Region 1 node')
     end
 
     it 'creates a GeoNode' do
@@ -22,7 +23,9 @@ describe 'geo rake tasks', :geo do
 
       node = GeoNode.first
 
+      expect(node.name).to eq('Region 1 node')
       expect(node.uri.scheme).to eq('https')
+      expect(node.url).to eq('https://example.com:1234/relative_part/')
       expect(node.primary).to be_truthy
     end
   end

ee/spec/workers/remove_unreferenced_lfs_objects_worker_spec.rb

 require 'spec_helper'
 
 describe RemoveUnreferencedLfsObjectsWorker do
+  include EE::GeoHelpers
+
   describe '#perform' do
     context 'when running in a Geo primary node' do
       set(:primary) { create(:geo_node, :primary) }
       set(:secondary) { create(:geo_node) }
 
       it 'logs an event to the Geo event log for every unreferenced LFS objects' do
+        stub_current_geo_node(primary)
         unreferenced_lfs_object_1 = create(:lfs_object, :with_file)
         unreferenced_lfs_object_2 = create(:lfs_object, :with_file)
         referenced_lfs_object = create(:lfs_object)

locale/gitlab.pot

@@ -11964,6 +11964,9 @@ msgstr ""
 msgid "The time taken by each data entry gathered by that stage."
 msgstr ""
 
+msgid "The unique identifier for the Geo node. Must match `geo_node_name` if it is set in gitlab.rb, otherwise it must match `external_url`"
+msgstr ""
+
 msgid "The update action will time out after %{number_of_minutes} minutes. For big repositories, use a clone/push combination."
 msgstr ""
 
@@ -11985,6 +11988,9 @@ msgstr ""
 msgid "The user map is a mapping of the FogBugz users that participated on your projects to the way their email address and usernames will be imported into GitLab. You can change this by populating the table below."
 msgstr ""
 
+msgid "The user-facing URL of the Geo node."
+msgstr ""
+
 msgid "The value lying at the midpoint of a series of observed values. E.g., between 3, 5, 9, the median is 5. Between 3, 5, 7, 8, the median is (5+7)/2 = 6."
 msgstr ""