From 91f27f885beefbaee1e8d6e5ebb1f6ddbe6cb417 Mon Sep 17 00:00:00 2001
From: Tiffany Rea <trea@gitlab.com>
Date: Fri, 2 Dec 2022 04:47:00 +0000
Subject: [PATCH] Refactor runner resource class

---
 qa/qa/resource/runner.rb                      | 146 ++++++++++--------
 .../api/4_verify/remove_runner_spec.rb        |  11 +-
 2 files changed, 85 insertions(+), 72 deletions(-)

diff --git a/qa/qa/resource/runner.rb b/qa/qa/resource/runner.rb
index da4021f89b760..77a7e06bb2031 100644
--- a/qa/qa/resource/runner.rb
+++ b/qa/qa/resource/runner.rb
@@ -3,10 +3,22 @@
 module QA
   module Resource
     class Runner < Base
-      attr_writer :name, :tags, :image, :executor, :executor_image
-      attr_accessor :config, :token, :run_untagged
+      attributes :id,
+                 :active,
+                 :paused,
+                 :runner_type,
+                 :online,
+                 :status,
+                 :ip_address,
+                 :token,
+                 :tags,
+                 :config,
+                 :run_untagged,
+                 :name, # This attribute == runner[:description]
+                 :image,
+                 :executor,
+                 :executor_image
 
-      attribute :id
       attribute :project do
         Project.fabricate_via_api! do |resource|
           resource.name = 'project-with-ci-cd'
@@ -14,81 +26,41 @@ class Runner < Base
         end
       end
 
-      def name
-        @name || "qa-runner-#{SecureRandom.hex(4)}"
-      end
-
-      def image
-        @image || 'registry.gitlab.com/gitlab-org/gitlab-runner:alpine'
-      end
-
-      def executor
-        @executor || :shell
-      end
-
-      def executor_image
-        @executor_image || 'registry.gitlab.com/gitlab-org/gitlab-build-images:gitlab-qa-alpine-ruby-2.7'
+      def initialize
+        @tags = nil
+        @config = nil
+        @run_untagged = nil
+        @name = "qa-runner-#{SecureRandom.hex(4)}"
+        @image = 'registry.gitlab.com/gitlab-org/gitlab-runner:alpine'
+        @executor = :shell
+        @executor_image = 'registry.gitlab.com/gitlab-org/gitlab-build-images:gitlab-qa-alpine-ruby-2.7'
       end
 
+      # Start container and register runner
+      # Fetch via API and populate attributes
+      #
       def fabricate_via_api!
-        @docker_container = Service::DockerRun::GitlabRunner.new(name).tap do |runner|
-          QA::Support::Retrier.retry_on_exception(sleep_interval: 5) do
-            runner.pull
-          end
-
-          runner.token = @token ||= project.runners_token
-          runner.address = Runtime::Scenario.gitlab_address
-          runner.tags = @tags if @tags
-          runner.image = image
-          runner.config = config if config
-          runner.executor = executor
-          runner.executor_image = executor_image if executor == :docker
-          runner.run_untagged = run_untagged if run_untagged
-          runner.register!
-        end
+        start_container_and_register
+        populate_runner_attributes
       end
 
       def remove_via_api!
-        runners = list_of_runners(tag_list: @tags)
-
-        # If we have no runners, print the logs from the runner docker container in case they show why it isn't running.
-        if runners.blank?
-          dump_logs
-
-          return
-        end
-
-        this_runner = runners.find { |runner| runner[:description] == name }
-
-        # As above, but now we should have a specific runner. If not, print the logs from the runner docker container
-        # to see if we can find out why the runner isn't running.
-        unless this_runner
-          dump_logs
-
-          raise "Project #{project.path_with_namespace} does not have a runner with a description matching #{name} #{"or tags #{@tags}" if @tags&.any?}. Runners available: #{runners}"
-        end
-
-        @id = this_runner[:id]
-
         super
       ensure
-        Service::DockerRun::GitlabRunner.new(name).remove!
-      end
-
-      def list_of_runners(tag_list: nil)
-        url = tag_list ? "#{api_post_path}?tag_list=#{tag_list.compact.join(',')}" : api_post_path
-        auto_paginated_response(request_url(url))
+        @docker_container.remove!
       end
 
       def reload!
-        super if method(:running?).super_method.call
+        populate_runner_attributes
       end
 
       def api_delete_path
         "/runners/#{id}"
       end
 
-      def api_get_path; end
+      def api_get_path
+        "/runners"
+      end
 
       def api_post_path
         "/runners"
@@ -96,15 +68,57 @@ def api_post_path
 
       def api_post_body; end
 
+      def not_found_by_tags?
+        url = "#{api_get_path}?tag_list=#{@tags.compact.join(',')}"
+        auto_paginated_response(request_url(url)).empty?
+      end
+
+      def runners_list
+        runners_list = nil
+        url = tags ? "#{api_get_path}?tag_list=#{tags.compact.join(',')}" : api_get_path
+        QA::Runtime::Logger.info('Looking for list of runners via API...')
+        Support::Retrier.retry_until(max_duration: 60, sleep_interval: 1) do
+          runners_list = auto_paginated_response(request_url(url))
+          runners_list.present?
+        end
+
+        runners_list
+      end
+
       private
 
-      def dump_logs
-        if @docker_container.running?
-          @docker_container.logs
-        else
-          QA::Runtime::Logger.debug("No runner container found named #{name}")
+      def start_container_and_register
+        @docker_container = Service::DockerRun::GitlabRunner.new(name).tap do |runner|
+          QA::Support::Retrier.retry_on_exception(sleep_interval: 5) do
+            runner.pull
+          end
+
+          runner.token = @token ||= project.runners_token
+          runner.address = Runtime::Scenario.gitlab_address
+          runner.tags = tags if tags
+          runner.image = image
+          runner.config = config if config
+          runner.executor = executor
+          runner.executor_image = executor_image if executor == :docker
+          runner.run_untagged = run_untagged if run_untagged
+          runner.register!
         end
       end
+
+      def this_runner
+        runners_list.find { |runner| runner[:description] == name }
+      end
+
+      def populate_runner_attributes
+        runner = this_runner
+        @id = runner[:id]
+        @active = runner[:active]
+        @paused = runner[:paused]
+        @runner_type = runner[:typed]
+        @online = runner[:online]
+        @status = runner[:status]
+        @ip_address = runner[:ip_address]
+      end
     end
   end
 end
diff --git a/qa/qa/specs/features/api/4_verify/remove_runner_spec.rb b/qa/qa/specs/features/api/4_verify/remove_runner_spec.rb
index eb1b085c35c6f..7aaaa7137ed55 100644
--- a/qa/qa/specs/features/api/4_verify/remove_runner_spec.rb
+++ b/qa/qa/specs/features/api/4_verify/remove_runner_spec.rb
@@ -17,17 +17,16 @@ module QA
 
       # Removing a runner via the UI is covered by `spec/features/runners_spec.rb``
       it 'removes the runner', testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/354828' do
-        runners = nil
-        expect { (runners = runner.list_of_runners(tag_list: runner_tags)).size }
-          .to eventually_eq(1).within(max_duration: 10, sleep_interval: 1)
-        expect(runners.first[:description]).to eq(executor)
+        runners_list = runner.runners_list
+        expect(runners_list.size).to eq(1)
+        expect(runners_list.first[:description]).to eq(executor)
 
-        request = Runtime::API::Request.new(api_client, "runners/#{runners.first[:id]}")
+        request = Runtime::API::Request.new(api_client, "runners/#{runner.id}")
         response = delete(request.url)
         expect(response.code).to eq(Support::API::HTTP_STATUS_NO_CONTENT)
         expect(response.body).to be_empty
 
-        expect(runner.list_of_runners(tag_list: runner_tags)).to be_empty
+        expect(runner).to be_not_found_by_tags
       end
     end
   end
-- 
GitLab