Merge branch 'mk/little-things' into 'master'

Extract refactoring from !10755

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

config/initializers/1_settings.rb

@@ -258,7 +258,6 @@ Settings.pages.admin['certificate'] ||= ''
 #
 # Geo
 #
-Settings.gitlab['geo_status_timeout'] ||= 10
 
 #
 # External merge request diffs

doc/user/admin_area/geo_nodes.md

@@ -11,37 +11,37 @@ All Geo nodes have the following settings:
 
 | Setting | Description |
 | --------| ----------- |
-| Primary | This marks a Geo Node as primary. There can be only one primary, make sure that you first add the primary node and then all the others. |
+| 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). |
 
 The node you're reading from is indicated with a green `Current node` label, and
-the primary is given a blue `Primary` label. Remember that you can only make
-changes on the primary!
+the **primary** node is given a blue `Primary` label. Remember that you can only make
+changes on the **primary** node!
 
-## Secondary node settings
+## **Secondary** node settings
 
-Secondaries have a number of additional settings available:
+**Secondary** nodes have a number of additional settings available:
 
 | Setting                   | Description |
 |---------------------------|-------------|
-| Selective synchronization | Enable Geo [selective sync](../../administration/geo/replication/configuration.md#selective-synchronization) for this secondary. |
-| Repository sync capacity  | Number of concurrent requests this secondary will make to the primary when backfilling repositories. |
-| File sync capacity        | Number of concurrent requests this secondary will make to the primary when backfilling files. |
+| Selective synchronization | Enable Geo [selective sync](../../administration/geo/replication/configuration.md#selective-synchronization) for this **secondary** node. |
+| Repository sync capacity  | Number of concurrent requests this **secondary** node will make to the **primary** node when backfilling repositories. |
+| File sync capacity        | Number of concurrent requests this **secondary** node will make to the **primary** node when backfilling files. |
 
 ## Geo backfill
 
-Secondaries are notified of changes to repositories and files by the primary,
+**Secondary** nodes are notified of changes to repositories and files by the **primary** node,
 and will always attempt to synchronize those changes as quickly as possible.
 
-Backfill is the act of populating the secondary with repositories and files that
-existed *before* the secondary was added to the database. Since there may be
+Backfill is the act of populating the **secondary** node with repositories and files that
+existed *before* the **secondary** node was added to the database. Since there may be
 extremely large numbers of repositories and files, it's infeasible to attempt to
 download them all at once, so GitLab places an upper limit on the concurrency of
 these operations.
 
 How long the backfill takes is a function of the maximum concurrency, but higher
-values place more strain on the primary node. From [GitLab 10.2](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/3107),
-the limits are configurable - if your primary node has lots of surplus capacity,
+values place more strain on the **primary** node. From [GitLab 10.2](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/3107),
+the limits are configurable. If your **primary** node has lots of surplus capacity,
 you can increase the values to complete backfill in a shorter time. If it's
 under heavy load and backfill is reducing its availability for normal requests,
 you can decrease them.

ee/app/models/geo_node.rb

@@ -15,9 +15,6 @@ class GeoNode < ApplicationRecord
   has_many :namespaces, through: :geo_node_namespace_links
   has_one :status, class_name: 'GeoNodeStatus'
 
-  default_values url: ->(record) { record.class.current_node_url },
-                 primary: false
-
   validates :url, presence: true, uniqueness: { case_sensitive: false }, addressable_url: true
   validates :internal_url, addressable_url: true, allow_blank: true, allow_nil: true
 
@@ -288,6 +285,8 @@ class GeoNode < ApplicationRecord
   end
 
   def update_oauth_application!
+    return unless uri
+
     self.build_oauth_application if oauth_application.nil?
     self.oauth_application.name = "Geo node: #{self.url}"
     self.oauth_application.redirect_uri = oauth_callback_url

ee/app/services/geo/repositories_changed_event_store.rb

@@ -15,12 +15,5 @@ module Geo
     def build_event
       Geo::RepositoriesChangedEvent.new(geo_node: geo_node)
     end
-
-    def log_params
-      {
-        geo_node_id: geo_node.id,
-        geo_node_url: geo_node.url
-      }
-    end
   end
 end

ee/spec/controllers/ee/sessions_controller_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-describe SessionsController do
+describe SessionsController, :geo do
   include DeviseHelpers
   include EE::GeoHelpers
 

ee/spec/controllers/oauth/geo_auth_controller_spec.rb

 require 'spec_helper'
 
-describe Oauth::GeoAuthController do
+describe Oauth::GeoAuthController, :geo do
   include EE::GeoHelpers
 
   # The Geo OAuth workflow depends on the OAuth application and the URL
@@ -92,7 +92,7 @@ describe Oauth::GeoAuthController do
 
         context 'with a tampered HOST header' do
           before do
-            request.headers['HOST'] = 'http://this.is.not.my.host'
+            request.headers['HOST'] = 'this.is.not.my.host'
           end
 
           it_behaves_like 'a valid redirect to redirect_url'
@@ -100,7 +100,7 @@ describe Oauth::GeoAuthController do
 
         context 'with a tampered X-Forwarded-Host header' do
           before do
-            request.headers['X-Forwarded-Host'] = 'http://this.is.not.my.host'
+            request.headers['X-Forwarded-Host'] = 'this.is.not.my.host'
           end
 
           it_behaves_like 'a valid redirect to redirect_url'

ee/spec/factories/geo_nodes.rb

@@ -10,6 +10,8 @@ FactoryBot.define do
       uri.to_s
     end
 
+    primary false
+
     trait :primary do
       primary true
       minimum_reverification_interval 7

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

 require 'spec_helper'
 
-describe 'admin Geo Nodes', :js do
+describe 'admin Geo Nodes', :js, :geo do
   let!(:geo_node) { create(:geo_node) }
 
   before do

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

@@ -5,8 +5,10 @@ require 'spec_helper'
 describe Gitlab::Geo::GitPushSSHProxy, :geo do
   include ::EE::GeoHelpers
 
-  set(:primary_node) { create(:geo_node, :primary) }
-  set(:secondary_node) { create(:geo_node) }
+  # TODO This spec doesn't work with a relative_url_root https://gitlab.com/gitlab-org/gitlab-ee/issues/11173
+  # TODO This spec doesn't work with non-localhost
+  set(:primary_node) { create(:geo_node, :primary, url: 'http://localhost:3001') }
+  set(:secondary_node) { create(:geo_node, url: 'http://localhost:3002') }
 
   let(:current_node) { nil }
   let(:project) { create(:project, :repository) }

ee/spec/lib/gitlab/geo/oauth/session_spec.rb

@@ -2,10 +2,11 @@
 
 require 'spec_helper'
 
-describe Gitlab::Geo::Oauth::Session do
+describe Gitlab::Geo::Oauth::Session, :geo do
   include EE::GeoHelpers
 
-  let!(:primary_node) { create(:geo_node, :primary) }
+  # This spec doesn't work with a relative_url_root https://gitlab.com/gitlab-org/gitlab-ee/issues/11261
+  let!(:primary_node) { create(:geo_node, :primary, url: 'http://primary') }
   let(:secondary_node) { create(:geo_node) }
   let(:oauth_application) { secondary_node.oauth_application }
   let(:access_token) { create(:doorkeeper_access_token, application: oauth_application) }

ee/spec/models/geo_node_spec.rb

@@ -28,6 +28,7 @@ describe GeoNode, :geo, type: :model do
     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(:url) }
 
     context 'when validating primary node' do
       it 'cannot be disabled' do
@@ -85,8 +86,6 @@ describe GeoNode, :geo, type: :model do
 
   context 'default values' do
     where(:attribute, :value) do
-      :url                | Gitlab::Routing.url_helpers.root_url
-      :primary            | false
       :repos_max_capacity | 25
       :files_max_capacity | 10
     end
@@ -332,7 +331,7 @@ describe GeoNode, :geo, type: :model do
   end
 
   describe '#uri' do
-    context 'when all fields are filled' do
+    context 'when url is set' do
       it 'returns an URI object' do
         expect(new_node.uri).to be_a URI
       end
@@ -344,9 +343,9 @@ describe GeoNode, :geo, type: :model do
       end
     end
 
-    context 'when required fields are not filled' do
-      it 'returns an URI object' do
-        expect(empty_node.uri).to be_a URI
+    context 'when url is not yet set' do
+      it 'returns nil' do
+        expect(empty_node.uri).to be_nil
       end
     end
   end
@@ -360,14 +359,6 @@ describe GeoNode, :geo, type: :model do
       expected_url = 'https://localhost:3000/gitlab/'
       expect(new_node.url).to eq(expected_url)
     end
-
-    it 'defaults to existing HTTPS and relative URL with a terminating / if present' do
-      stub_config_setting(port: 443)
-      stub_config_setting(protocol: 'https')
-      stub_config_setting(relative_url_root: '/gitlab')
-
-      expect(empty_node.url).to eq('https://localhost/gitlab/')
-    end
   end
 
   describe '#url=' do