Do not fail if the self monitoring project exists64689-add-instance-administrators-group

- Since we save the project_id in application_settings, we can check if
the project exists before trying to create it.
- Also add 8 random characters to the end of the group path so that
there is no chance that a group with the same path already exists.

4 files changed, 44 insertions, 11 deletions

diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 9dbcef8abaa..cb6346421ec 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -10,6 +10,8 @@ class ApplicationSetting < ApplicationRecord
   add_authentication_token_field :runners_registration_token, encrypted: -> { Feature.enabled?(:application_settings_tokens_optional_encryption, default_enabled: true) ? :optional : :required }
   add_authentication_token_field :health_check_access_token
 
+  belongs_to :instance_administration_project, class_name: "Project"
+
   # Include here so it can override methods from
   # `add_authentication_token_field`
   # We don't prepend for now because otherwise we'll need to
diff --git a/app/services/self_monitoring/project/create_service.rb b/app/services/self_monitoring/project/create_service.rb
index d8085daae99..d211e399e11 100644
--- a/app/services/self_monitoring/project/create_service.rb
+++ b/app/services/self_monitoring/project/create_service.rb
@@ -42,6 +42,12 @@ module SelfMonitoring
       end
 
       def create_group
+        if project_created?
+          log_info(_('Instance administrators group already exists'))
+          @group = application_settings.instance_administration_project.owner
+          return success(group: @group)
+        end
+
         admin_user = group_owner
         @group = ::Groups::CreateService.new(admin_user, create_group_params).execute
 
@@ -53,6 +59,12 @@ module SelfMonitoring
       end
 
       def create_project
+        if project_created?
+          log_info(_('Instance administration project already exists'))
+          @project = application_settings.instance_administration_project
+          return success(project: project)
+        end
+
         admin_user = group_owner
         @project = ::Projects::CreateService.new(admin_user, create_project_params).execute
 
@@ -65,6 +77,8 @@ module SelfMonitoring
       end
 
       def save_project_id
+        return success if project_created?
+
         result = ApplicationSettings::UpdateService.new(
           application_settings,
           group_owner,
@@ -131,6 +145,10 @@ module SelfMonitoring
           ::ApplicationSetting.create_from_defaults
       end
 
+      def project_created?
+        application_settings.instance_administration_project.present?
+      end
+
       def parse_url(uri_string)
         Addressable::URI.parse(uri_string)
       rescue Addressable::URI::InvalidURIError, TypeError
@@ -163,7 +181,7 @@ module SelfMonitoring
       def create_group_params
         {
           name: GROUP_NAME,
-          path: GROUP_PATH,
+          path: "#{GROUP_PATH}-#{SecureRandom.hex(4)}",
           visibility_level: VISIBILITY_LEVEL
         }
       end
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 5ecaaa00c7b..071ea86405b 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -5802,6 +5802,12 @@ msgstr ""
 msgid "Instance Statistics visibility"
 msgstr ""
 
+msgid "Instance administration project already exists"
+msgstr ""
+
+msgid "Instance administrators group already exists"
+msgstr ""
+
 msgid "Instance does not support multiple Kubernetes clusters"
 msgstr ""
 
diff --git a/spec/services/self_monitoring/project/create_service_spec.rb b/spec/services/self_monitoring/project/create_service_spec.rb
index e2b60c2b02e..a0eb83340f0 100644
--- a/spec/services/self_monitoring/project/create_service_spec.rb
+++ b/spec/services/self_monitoring/project/create_service_spec.rb
@@ -58,7 +58,8 @@ describe SelfMonitoring::Project::CreateService do
         expect(result[:status]).to eq(:success)
         expect(group).to be_persisted
         expect(group.name).to eq(described_class::GROUP_NAME)
-        expect(group.path).to eq(described_class::GROUP_PATH)
+        expect(group.path).to start_with(described_class::GROUP_PATH)
+        expect(group.path.split('-').last.length).to eq(8)
         expect(group.visibility_level).to eq(described_class::VISIBILITY_LEVEL)
       end
 
@@ -102,16 +103,22 @@ describe SelfMonitoring::Project::CreateService do
         expect(application_setting.instance_administration_project_id).to eq(project.id)
       end
 
-      context 'when saving project id fails' do
-        before do
-          allow(application_setting).to receive(:update) { false }
-        end
+      it 'returns error when saving project ID fails' do
+        allow(application_setting).to receive(:update) { false }
 
-        it 'returns error' do
-          expect(result[:status]).to eq(:error)
-          expect(result[:failed_step]).to eq(:save_project_id)
-          expect(result[:message]).to eq('Could not save project ID')
-        end
+        expect(result[:status]).to eq(:error)
+        expect(result[:failed_step]).to eq(:save_project_id)
+        expect(result[:message]).to eq('Could not save project ID')
+      end
+
+      it 'does not fail when a project already exists' do
+        expect(result[:status]).to eq(:success)
+
+        second_result = subject.execute
+
+        expect(second_result[:status]).to eq(:success)
+        expect(second_result[:project]).to eq(project)
+        expect(second_result[:group]).to eq(group)
       end
 
       context 'when local requests from hooks and services are not allowed' do