Revert "Merge branch 'pedropombeiro/390261/remove-feature-flag' into 'master'"

This reverts merge request !113252

config/feature_flags/development/create_runner_machine.yml

+---
+name: create_runner_machine
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/109983
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/390261
+milestone: '15.9'
+type: development
+group: group::runner
+default_enabled: false

lib/api/ci/helpers/runner.rb

@@ -53,6 +53,8 @@ def current_runner
         end
 
         def current_runner_machine
+          return if Feature.disabled?(:create_runner_machine)
+
           strong_memoize(:current_runner_machine) do
             system_xid = params.fetch(:system_id, LEGACY_SYSTEM_XID)
             current_runner&.ensure_machine(system_xid) { |m| m.contacted_at = Time.current }

spec/lib/api/ci/helpers/runner_spec.rb

@@ -73,38 +73,68 @@
 
     subject(:current_runner_machine) { helper.current_runner_machine }
 
-    context 'when runner machine already exists' do
+    context 'with create_runner_machine FF enabled' do
       before do
-        allow(helper).to receive(:params).and_return(token: runner.token, system_id: runner_machine.system_xid)
+        stub_feature_flags(create_runner_machine: true)
       end
 
-      it { is_expected.to eq(runner_machine) }
+      context 'when runner machine already exists' do
+        before do
+          allow(helper).to receive(:params).and_return(token: runner.token, system_id: runner_machine.system_xid)
+        end
 
-      it 'does not update the contacted_at field' do
-        expect(current_runner_machine.contacted_at).to eq 1.hour.ago
+        it { is_expected.to eq(runner_machine) }
+
+        it 'does not update the contacted_at field' do
+          expect(current_runner_machine.contacted_at).to eq 1.hour.ago
+        end
       end
-    end
 
-    context 'when runner machine cannot be found' do
-      it 'creates a new runner machine', :aggregate_failures do
-        allow(helper).to receive(:params).and_return(token: runner.token, system_id: 'new_system_id')
+      context 'when runner machine cannot be found' do
+        it 'creates a new runner machine', :aggregate_failures do
+          allow(helper).to receive(:params).and_return(token: runner.token, system_id: 'new_system_id')
+
+          expect { current_runner_machine }.to change { Ci::RunnerMachine.count }.by(1)
+
+          expect(current_runner_machine).not_to be_nil
+          expect(current_runner_machine.system_xid).to eq('new_system_id')
+          expect(current_runner_machine.contacted_at).to eq(Time.current)
+          expect(current_runner_machine.runner).to eq(runner)
+        end
+
+        it 'creates a new <legacy> runner machine if system_id is not specified', :aggregate_failures do
+          allow(helper).to receive(:params).and_return(token: runner.token)
+
+          expect { current_runner_machine }.to change { Ci::RunnerMachine.count }.by(1)
 
-        expect { current_runner_machine }.to change { Ci::RunnerMachine.count }.by(1)
+          expect(current_runner_machine).not_to be_nil
+          expect(current_runner_machine.system_xid).to eq(::API::Ci::Helpers::Runner::LEGACY_SYSTEM_XID)
+          expect(current_runner_machine.runner).to eq(runner)
+        end
+      end
+    end
 
-        expect(current_runner_machine).not_to be_nil
-        expect(current_runner_machine.system_xid).to eq('new_system_id')
-        expect(current_runner_machine.contacted_at).to eq(Time.current)
-        expect(current_runner_machine.runner).to eq(runner)
+    context 'with create_runner_machine FF disabled' do
+      before do
+        stub_feature_flags(create_runner_machine: false)
       end
 
-      it 'creates a new <legacy> runner machine if system_id is not specified', :aggregate_failures do
+      it 'does not return runner machine if no system_id specified' do
         allow(helper).to receive(:params).and_return(token: runner.token)
 
-        expect { current_runner_machine }.to change { Ci::RunnerMachine.count }.by(1)
+        is_expected.to be_nil
+      end
+
+      context 'when runner machine can not be found' do
+        before do
+          allow(helper).to receive(:params).and_return(token: runner.token, system_id: 'new_system_id')
+        end
+
+        it 'does not create a new runner machine', :aggregate_failures do
+          expect { current_runner_machine }.not_to change { Ci::RunnerMachine.count }
 
-        expect(current_runner_machine).not_to be_nil
-        expect(current_runner_machine.system_xid).to eq(::API::Ci::Helpers::Runner::LEGACY_SYSTEM_XID)
-        expect(current_runner_machine.runner).to eq(runner)
+          expect(current_runner_machine).to be_nil
+        end
       end
     end
   end

spec/requests/api/ci/runner/jobs_request_post_spec.rb

@@ -122,33 +122,56 @@
         context 'when system_id parameter is specified' do
           subject(:request) { request_job(**args) }
 
-          context 'when ci_runner_machines with same system_xid does not exist' do
-            let(:args) { { system_id: 's_some_system_id' } }
+          context 'with create_runner_machine FF enabled' do
+            before do
+              stub_feature_flags(create_runner_machine: true)
+            end
 
-            it 'creates respective ci_runner_machines record', :freeze_time do
-              expect { request }.to change { runner.runner_machines.reload.count }.from(0).to(1)
+            context 'when ci_runner_machines with same system_xid does not exist' do
+              let(:args) { { system_id: 's_some_system_id' } }
 
-              machine = runner.runner_machines.last
-              expect(machine.system_xid).to eq args[:system_id]
-              expect(machine.runner).to eq runner
-              expect(machine.contacted_at).to eq Time.current
+              it 'creates respective ci_runner_machines record', :freeze_time do
+                expect { request }.to change { runner.runner_machines.reload.count }.from(0).to(1)
+
+                machine = runner.runner_machines.last
+                expect(machine.system_xid).to eq args[:system_id]
+                expect(machine.runner).to eq runner
+                expect(machine.contacted_at).to eq Time.current
+              end
             end
-          end
 
-          context 'when ci_runner_machines with same system_xid already exists', :freeze_time do
-            let(:args) { { system_id: 's_existing_system_id' } }
-            let!(:runner_machine) do
-              create(:ci_runner_machine, runner: runner, system_xid: args[:system_id], contacted_at: 1.hour.ago)
+            context 'when ci_runner_machines with same system_xid already exists', :freeze_time do
+              let(:args) { { system_id: 's_existing_system_id' } }
+              let!(:runner_machine) do
+                create(:ci_runner_machine, runner: runner, system_xid: args[:system_id], contacted_at: 1.hour.ago)
+              end
+
+              it 'does not create new ci_runner_machines record' do
+                expect { request }.not_to change { Ci::RunnerMachine.count }
+              end
+
+              it 'updates the contacted_at field' do
+                request
+
+                expect(runner_machine.reload.contacted_at).to eq Time.current
+              end
             end
+          end
 
-            it 'does not create new ci_runner_machines record' do
-              expect { request }.not_to change { Ci::RunnerMachine.count }
+          context 'with create_runner_machine FF disabled' do
+            before do
+              stub_feature_flags(create_runner_machine: false)
             end
 
-            it 'updates the contacted_at field' do
-              request
+            context 'when ci_runner_machines with same system_xid does not exist' do
+              let(:args) { { system_id: 's_some_system_id' } }
+
+              it 'does not create respective ci_runner_machines record', :freeze_time, :aggregate_failures do
+                expect { request }.not_to change { runner.runner_machines.reload.count }
 
-              expect(runner_machine.reload.contacted_at).to eq Time.current
+                expect(response).to have_gitlab_http_status(:created)
+                expect(runner.runner_machines).to be_empty
+              end
             end
           end
         end

spec/requests/api/ci/runner/runners_verify_post_spec.rb

@@ -45,12 +45,33 @@
       context 'when valid token is provided' do
         let(:params) { { token: runner.token } }
 
-        context 'with glrt-prefixed token' do
-          let_it_be(:registration_token) { 'glrt-abcdefg123456' }
-          let_it_be(:registration_type) { :authenticated_user }
-          let_it_be(:runner) do
-            create(:ci_runner, registration_type: registration_type,
-              token: registration_token, token_expires_at: 3.days.from_now)
+        context 'with create_runner_machine FF enabled' do
+          before do
+            stub_feature_flags(create_runner_machine: true)
+          end
+
+          context 'with glrt-prefixed token' do
+            let_it_be(:registration_token) { 'glrt-abcdefg123456' }
+            let_it_be(:registration_type) { :authenticated_user }
+            let_it_be(:runner) do
+              create(:ci_runner, registration_type: registration_type,
+                token: registration_token, token_expires_at: 3.days.from_now)
+            end
+
+            it 'verifies Runner credentials' do
+              verify
+
+              expect(response).to have_gitlab_http_status(:ok)
+              expect(json_response).to eq({
+                'id' => runner.id,
+                'token' => runner.token,
+                'token_expires_at' => runner.token_expires_at.iso8601(3)
+              })
+            end
+
+            it 'does not update contacted_at' do
+              expect { verify }.not_to change { runner.reload.contacted_at }.from(nil)
+            end
           end
 
           it 'verifies Runner credentials' do
@@ -64,29 +85,43 @@
             })
           end
 
-          it 'does not update contacted_at' do
-            expect { verify }.not_to change { runner.reload.contacted_at }.from(nil)
+          it 'updates contacted_at' do
+            expect { verify }.to change { runner.reload.contacted_at }.from(nil).to(Time.current)
           end
-        end
 
-        it 'verifies Runner credentials' do
-          verify
+          context 'with non-expiring runner token' do
+            before do
+              runner.update!(token_expires_at: nil)
+            end
+
+            it 'verifies Runner credentials' do
+              verify
+
+              expect(response).to have_gitlab_http_status(:ok)
+              expect(json_response).to eq({
+                'id' => runner.id,
+                'token' => runner.token,
+                'token_expires_at' => nil
+              })
+            end
+          end
 
-          expect(response).to have_gitlab_http_status(:ok)
-          expect(json_response).to eq({
-            'id' => runner.id,
-            'token' => runner.token,
-            'token_expires_at' => runner.token_expires_at.iso8601(3)
-          })
-        end
+          it_behaves_like 'storing arguments in the application context for the API' do
+            let(:expected_params) { { client_id: "runner/#{runner.id}" } }
+          end
 
-        it 'updates contacted_at' do
-          expect { verify }.to change { runner.reload.contacted_at }.from(nil).to(Time.current)
+          context 'when system_id is provided' do
+            let(:params) { { token: runner.token, system_id: 's_some_system_id' } }
+
+            it 'creates a runner_machine' do
+              expect { verify }.to change { Ci::RunnerMachine.count }.by(1)
+            end
+          end
         end
 
-        context 'with non-expiring runner token' do
+        context 'with create_runner_machine FF disabled' do
           before do
-            runner.update!(token_expires_at: nil)
+            stub_feature_flags(create_runner_machine: false)
           end
 
           it 'verifies Runner credentials' do
@@ -96,20 +131,18 @@
             expect(json_response).to eq({
               'id' => runner.id,
               'token' => runner.token,
-              'token_expires_at' => nil
+              'token_expires_at' => runner.token_expires_at.iso8601(3)
             })
           end
-        end
 
-        it_behaves_like 'storing arguments in the application context for the API' do
-          let(:expected_params) { { client_id: "runner/#{runner.id}" } }
-        end
+          context 'when system_id is provided' do
+            let(:params) { { token: runner.token, system_id: 's_some_system_id' } }
 
-        context 'when system_id is provided' do
-          let(:params) { { token: runner.token, system_id: 's_some_system_id' } }
+            it 'does not create a runner_machine', :aggregate_failures do
+              expect { verify }.not_to change { Ci::RunnerMachine.count }
 
-          it 'creates a runner_machine' do
-            expect { verify }.to change { Ci::RunnerMachine.count }.by(1)
+              expect(response).to have_gitlab_http_status(:ok)
+            end
           end
         end
       end