diff --git a/app/helpers/ci/runners_helper.rb b/app/helpers/ci/runners_helper.rb index 6de589286d520e002929e89a87dc10bd21920b55..72a1b3cab39dc94533848093ce56c11939c89360 100644 --- a/app/helpers/ci/runners_helper.rb +++ b/app/helpers/ci/runners_helper.rb @@ -21,7 +21,13 @@ def runner_status_icon(runner, size: 16, icon_class: '') title = s_("Runners|Runner has never contacted this instance") icon = 'warning-solid' when :offline - title = s_("Runners|Runner is offline; last contact was %{runner_contact} ago") % { runner_contact: time_ago_in_words(contacted_at) } + title = + if contacted_at + s_("Runners|Runner is offline; last contact was %{runner_contact} ago") % { runner_contact: time_ago_in_words(contacted_at) } + else + s_("Runners|Runner is offline; it has never contacted this instance") + end + icon = 'status-waiting' span_class = 'gl-text-gray-500' when :stale diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index f4d8f4690a88bf07c555b982c8a935bf35dfc48f..111b032f4a38d0e0c6b6e102688f9071a9a1390e 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -34,6 +34,11 @@ class Runner < Ci::ApplicationRecord project_type: 3 } + enum creation_state: { + started: 0, + finished: 100 + }, _suffix: true + enum registration_type: { registration_token: 0, authenticated_user: 1 @@ -454,7 +459,11 @@ def heartbeat(values, update_contacted_at: true) # ::Gitlab::Database::LoadBalancing::Session.without_sticky_writes do values = values&.slice(:version, :revision, :platform, :architecture, :ip_address, :config, :executor) || {} - values[:contacted_at] = Time.current if update_contacted_at + + if update_contacted_at + values.merge!(contacted_at: Time.current, creation_state: :finished) + end + if values.include?(:executor) values[:executor_type] = EXECUTOR_NAME_TO_TYPES.fetch(values.delete(:executor), :unknown) end diff --git a/app/models/ci/runner_manager.rb b/app/models/ci/runner_manager.rb index 44fe1bdd67db115b7df3026e01fb13b4bd1d94e0..69b8bd1cbef68872b94490e64888ccee662ddb96 100644 --- a/app/models/ci/runner_manager.rb +++ b/app/models/ci/runner_manager.rb @@ -17,6 +17,11 @@ class RunnerManager < Ci::ApplicationRecord belongs_to :runner + enum creation_state: { + started: 0, + finished: 100 + }, _suffix: true + has_many :runner_manager_builds, inverse_of: :runner_manager, foreign_key: :runner_machine_id, class_name: 'Ci::RunnerManagerBuild' has_many :builds, through: :runner_manager_builds, class_name: 'Ci::Build' @@ -109,7 +114,9 @@ def heartbeat(values, update_contacted_at: true) # ::Gitlab::Database::LoadBalancing::Session.without_sticky_writes do values = values&.slice(:version, :revision, :platform, :architecture, :ip_address, :config, :executor) || {} - values[:contacted_at] = Time.current if update_contacted_at + + values.merge!(contacted_at: Time.current, creation_state: :finished) if update_contacted_at + if values.include?(:executor) values[:executor_type] = Ci::Runner::EXECUTOR_NAME_TO_TYPES.fetch(values.delete(:executor), :unknown) end diff --git a/app/models/concerns/ci/has_runner_status.rb b/app/models/concerns/ci/has_runner_status.rb index f6fb9940b44ee6ceb2e4cbfb81bd5be4ed5bcdd5..4615e9573ff41b2b3c4ca42a8b693919d059df6d 100644 --- a/app/models/concerns/ci/has_runner_status.rb +++ b/app/models/concerns/ci/has_runner_status.rb @@ -32,7 +32,7 @@ def stale_deadline def status return :stale if stale? - return :never_contacted unless contacted_at + return :never_contacted unless finished_creation_state? online? ? :online : :offline end diff --git a/db/migrate/20240408103150_add_runner_creation_status_to_ci_runner.rb b/db/migrate/20240408103150_add_runner_creation_status_to_ci_runner.rb new file mode 100644 index 0000000000000000000000000000000000000000..b1a78c5eb9ba38b92fdfb86254933baa825851de --- /dev/null +++ b/db/migrate/20240408103150_add_runner_creation_status_to_ci_runner.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddRunnerCreationStatusToCiRunner < Gitlab::Database::Migration[2.2] + milestone '16.11' + + def change + add_column :ci_runners, :creation_state, :integer, limit: 2, default: 100, null: false + end +end diff --git a/db/migrate/20240408103358_update_runner_creation_status_default_to_zero.rb b/db/migrate/20240408103358_update_runner_creation_status_default_to_zero.rb new file mode 100644 index 0000000000000000000000000000000000000000..4f09bdd58e474bb0b752b7f8f1e27beba3d2c49b --- /dev/null +++ b/db/migrate/20240408103358_update_runner_creation_status_default_to_zero.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class UpdateRunnerCreationStatusDefaultToZero < Gitlab::Database::Migration[2.2] + milestone '16.11' + + def change + change_column_default :ci_runners, :creation_state, from: 100, to: 0 + end +end diff --git a/db/migrate/20240408103457_add_runner_creation_status_to_ci_runner_machines.rb b/db/migrate/20240408103457_add_runner_creation_status_to_ci_runner_machines.rb new file mode 100644 index 0000000000000000000000000000000000000000..73c11016b972ac796310afcab03f2ba6be9acf19 --- /dev/null +++ b/db/migrate/20240408103457_add_runner_creation_status_to_ci_runner_machines.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddRunnerCreationStatusToCiRunnerMachines < Gitlab::Database::Migration[2.2] + milestone '16.11' + + def change + add_column :ci_runner_machines, :creation_state, :integer, limit: 2, default: 100, null: false + end +end diff --git a/db/migrate/20240408103529_update_ci_runner_machines_runner_creation_status_default_to_zero.rb b/db/migrate/20240408103529_update_ci_runner_machines_runner_creation_status_default_to_zero.rb new file mode 100644 index 0000000000000000000000000000000000000000..2bc69fb1fd4df6b25dada8dddfcdb9af881a7a32 --- /dev/null +++ b/db/migrate/20240408103529_update_ci_runner_machines_runner_creation_status_default_to_zero.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class UpdateCiRunnerMachinesRunnerCreationStatusDefaultToZero < Gitlab::Database::Migration[2.2] + milestone '16.11' + + def change + change_column_default :ci_runner_machines, :creation_state, from: 100, to: 0 + end +end diff --git a/db/schema_migrations/20240408103150 b/db/schema_migrations/20240408103150 new file mode 100644 index 0000000000000000000000000000000000000000..28c509156280a63498c0cb82139c21b6f26948cd --- /dev/null +++ b/db/schema_migrations/20240408103150 @@ -0,0 +1 @@ +31a9c402bac291e3be11363976e10223991afe14868941c91d6d96ab7283bcf5 \ No newline at end of file diff --git a/db/schema_migrations/20240408103358 b/db/schema_migrations/20240408103358 new file mode 100644 index 0000000000000000000000000000000000000000..f51553bf814db3a47c86e8f7bda4404d4f0d8918 --- /dev/null +++ b/db/schema_migrations/20240408103358 @@ -0,0 +1 @@ +1474b0983a8d7b469c6969a5949d8a185be263d23e23f1d9b5da21fb861e6674 \ No newline at end of file diff --git a/db/schema_migrations/20240408103457 b/db/schema_migrations/20240408103457 new file mode 100644 index 0000000000000000000000000000000000000000..eb887f2d5b6e2015d6ed2481d3665a2ed68b268f --- /dev/null +++ b/db/schema_migrations/20240408103457 @@ -0,0 +1 @@ +c1e3ee0f7f7dcb8577372d0649b84c0157da0e76b96ed0a23f4bdfcd319e5ee9 \ No newline at end of file diff --git a/db/schema_migrations/20240408103529 b/db/schema_migrations/20240408103529 new file mode 100644 index 0000000000000000000000000000000000000000..e091bee45ca32a6c6006cdbe092d2fd9cae5548d --- /dev/null +++ b/db/schema_migrations/20240408103529 @@ -0,0 +1 @@ +9a0f82dab091ebbd9ec2146d96efed4c0a46e18021e5378913c9dba9244f9d55 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index fdde49d90db1a7f6f5a98852777baa47e13dd30d..4273c2e54aeb29d60f176bc2359dcec6b167c695 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -6645,6 +6645,7 @@ CREATE TABLE ci_runner_machines ( ip_address text, config jsonb DEFAULT '{}'::jsonb NOT NULL, system_xid text, + creation_state smallint DEFAULT 0 NOT NULL, CONSTRAINT check_1537c1f66f CHECK ((char_length(platform) <= 255)), CONSTRAINT check_5253913ae9 CHECK ((char_length(system_xid) <= 64)), CONSTRAINT check_6f45a91da7 CHECK ((char_length(version) <= 2048)), @@ -6730,6 +6731,7 @@ CREATE TABLE ci_runners ( allowed_plans text[] DEFAULT '{}'::text[] NOT NULL, registration_type smallint DEFAULT 0 NOT NULL, creator_id bigint, + creation_state smallint DEFAULT 0 NOT NULL, CONSTRAINT check_ce275cee06 CHECK ((char_length(maintainer_note) <= 1024)) ); diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 769bfcfccd2d30621b492b198ae211ee7f1cbd6e..e5e307aca200ccfbe590b8bdd3dd93cef28c3e42 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -44070,6 +44070,9 @@ msgstr "" msgid "Runners|Runner is locked and available for currently assigned projects only. Only administrators can change the assigned projects." msgstr "" +msgid "Runners|Runner is offline; it has never contacted this instance" +msgstr "" + msgid "Runners|Runner is offline; last contact was %{runner_contact} ago" msgstr "" diff --git a/spec/factories/ci/runner_managers.rb b/spec/factories/ci/runner_managers.rb index 7a2b0c3721520b6bc01ddb4f730f78d1848a8992..9f9e3adb4ded8f2f27b468fd814a15a4063f9832 100644 --- a/spec/factories/ci/runner_managers.rb +++ b/spec/factories/ci/runner_managers.rb @@ -5,6 +5,13 @@ runner factory: :ci_runner system_xid { "r_#{SecureRandom.hex.slice(0, 10)}" } + creation_state { :finished } + + trait :unregistered do + contacted_at { nil } + creation_state { :started } + end + trait :stale do created_at { 1.year.ago } contacted_at { Ci::RunnerManager::STALE_TIMEOUT.ago } diff --git a/spec/factories/ci/runners.rb b/spec/factories/ci/runners.rb index 63e8cec82e6c066ec7f048b075075d3c0328ae81..474a4dad7c251b819b566821f14c136244cbc1d0 100644 --- a/spec/factories/ci/runners.rb +++ b/spec/factories/ci/runners.rb @@ -10,6 +10,8 @@ runner_type { :instance_type } + creation_state { :finished } + transient do groups { [] } projects { [] } @@ -41,6 +43,11 @@ runner_type { :instance_type } end + trait :unregistered do + contacted_at { nil } + creation_state { :started } + end + trait :group do runner_type { :group_type } diff --git a/spec/features/admin/admin_runners_spec.rb b/spec/features/admin/admin_runners_spec.rb index 4609f4fa111f1b8a0e1effab838fa2436ac71da9..4b63a73c3e6dc4f7fad4183af6f4be615a0c6984 100644 --- a/spec/features/admin/admin_runners_spec.rb +++ b/spec/features/admin/admin_runners_spec.rb @@ -258,7 +258,7 @@ describe 'filter by status' do let_it_be(:never_contacted) do - create(:ci_runner, :instance, description: 'runner-never-contacted', contacted_at: nil) + create(:ci_runner, :instance, :unregistered, description: 'runner-never-contacted') end before_all do @@ -614,7 +614,7 @@ describe "Runner edit page" do let_it_be(:project1) { create(:project) } let_it_be(:project2) { create(:project) } - let_it_be(:project_runner) { create(:ci_runner, :project) } + let_it_be(:project_runner) { create(:ci_runner, :unregistered, :project) } before do visit edit_admin_runner_path(project_runner) diff --git a/spec/finders/ci/runner_managers_finder_spec.rb b/spec/finders/ci/runner_managers_finder_spec.rb index 0581330e65b000c2e6b209320079258298879bd1..893469ca25702858a5bdbeb4b1a43200ac14a7ee 100644 --- a/spec/finders/ci/runner_managers_finder_spec.rb +++ b/spec/finders/ci/runner_managers_finder_spec.rb @@ -18,7 +18,7 @@ let_it_be(:offline_runner_manager) { create(:ci_runner_machine, runner: runner, contacted_at: 2.hours.ago) } let_it_be(:online_runner_manager) { create(:ci_runner_machine, runner: runner, contacted_at: 1.second.ago) } - let_it_be(:never_contacted_runner_manager) { create(:ci_runner_machine, runner: runner, contacted_at: nil) } + let_it_be(:never_contacted_runner_manager) { create(:ci_runner_machine, :unregistered, runner: runner) } let_it_be(:stale_runner_manager) do create( :ci_runner_machine, diff --git a/spec/frontend/fixtures/runner.rb b/spec/frontend/fixtures/runner.rb index 3b03a03cb96c1098be03952d9f67569d0130fa0f..f5c80df4692ec4ecc86929dd921122b8f015e42c 100644 --- a/spec/frontend/fixtures/runner.rb +++ b/spec/frontend/fixtures/runner.rb @@ -13,13 +13,18 @@ let_it_be(:project) { create(:project, :repository, :public) } let_it_be(:project_2) { create(:project, :repository, :public) } - let_it_be(:runner) { create(:ci_runner, :instance, description: 'My Runner', creator: admin, version: '1.0.0') } + let_it_be(:runner) do + create(:ci_runner, :instance, :unregistered, description: 'My Runner', creator: admin, version: '1.0.0') + end + let_it_be(:runner_manager_1) { create(:ci_runner_machine, runner: runner, contacted_at: Time.current) } let_it_be(:runner_manager_2) { create(:ci_runner_machine, runner: runner, contacted_at: Time.current) } let_it_be(:group_runner) { create(:ci_runner, :group, groups: [group], version: '2.0.0') } let_it_be(:group_runner_2) { create(:ci_runner, :group, groups: [group], version: '2.0.0') } - let_it_be(:project_runner) { create(:ci_runner, :project, projects: [project, project_2], version: '2.0.0') } + let_it_be(:project_runner) do + create(:ci_runner, :project, :unregistered, projects: [project, project_2], version: '2.0.0') + end let_it_be(:build) { create(:ci_build, runner: runner) } diff --git a/spec/helpers/ci/runners_helper_spec.rb b/spec/helpers/ci/runners_helper_spec.rb index 583bbba1b6d72a8c512beb0289667584233d8a09..9252f765579fb9e7b58b78ea281143907540b9f0 100644 --- a/spec/helpers/ci/runners_helper_spec.rb +++ b/spec/helpers/ci/runners_helper_spec.rb @@ -16,7 +16,7 @@ end it "returns never contacted" do - runner = create(:ci_runner) + runner = create(:ci_runner, :unregistered) expect(helper.runner_status_icon(runner)).to include("never contacted") end @@ -32,7 +32,7 @@ end it "returns stale text, when runner never contacted" do - runner = create(:ci_runner, created_at: 4.months.ago) + runner = create(:ci_runner, :unregistered, created_at: 4.months.ago) expect(helper.runner_status_icon(runner)).to include("is stale") expect(helper.runner_status_icon(runner)).to include("never contacted") end diff --git a/spec/models/ci/runner_manager_spec.rb b/spec/models/ci/runner_manager_spec.rb index 4fdfbae997d847369198313714b13bfd3b0fcbe0..bc479036b0062783f68518747fa5e2581219ba90 100644 --- a/spec/models/ci/runner_manager_spec.rb +++ b/spec/models/ci/runner_manager_spec.rb @@ -46,7 +46,7 @@ let_it_be(:offline_runner_manager) { create(:ci_runner_machine, runner: runner, contacted_at: 2.hours.ago) } let_it_be(:online_runner_manager) { create(:ci_runner_machine, runner: runner, contacted_at: 1.second.ago) } - let_it_be(:never_contacted_runner_manager) { create(:ci_runner_machine, runner: runner, contacted_at: nil) } + let_it_be(:never_contacted_runner_manager) { create(:ci_runner_machine, :unregistered, runner: runner) } describe '.online' do subject(:runner_managers) { described_class.online } @@ -329,46 +329,34 @@ end describe '#status', :freeze_time do - let(:runner_manager) { build(:ci_runner_machine, created_at: 8.days.ago) } - subject { runner_manager.status } context 'if never connected' do - before do - runner_manager.contacted_at = nil - end + let(:runner_manager) { build(:ci_runner_machine, :unregistered, created_at: 8.days.ago) } it { is_expected.to eq(:stale) } context 'if created recently' do - before do - runner_manager.created_at = 1.day.ago - end + let(:runner_manager) { build(:ci_runner_machine, :unregistered, created_at: 1.day.ago) } it { is_expected.to eq(:never_contacted) } end end context 'if contacted 1s ago' do - before do - runner_manager.contacted_at = 1.second.ago - end + let(:runner_manager) { build(:ci_runner_machine, contacted_at: 1.second.ago) } it { is_expected.to eq(:online) } end context 'if contacted recently' do - before do - runner_manager.contacted_at = 2.hours.ago - end + let(:runner_manager) { build(:ci_runner_machine, contacted_at: 2.hours.ago) } it { is_expected.to eq(:offline) } end context 'if contacted long time ago' do - before do - runner_manager.contacted_at = 7.days.ago - end + let(:runner_manager) { build(:ci_runner_machine, created_at: 8.days.ago, contacted_at: 7.days.ago) } it { is_expected.to eq(:stale) } end @@ -436,7 +424,7 @@ end it 'updates only ip_address' do - expect_redis_update(values.merge(contacted_at: Time.current)) + expect_redis_update(values.merge(contacted_at: Time.current, creation_state: :finished)) heartbeat end diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index 46f5952cf6b67472a734befc6ddc6e7e762e8df8..4be5f57c554d4817f3e9dbc35f0bfe4101750faa 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -493,7 +493,7 @@ let!(:runner3) { create(:ci_runner, :instance, contacted_at: 1.month.ago, created_at: 2.months.ago) } let!(:runner4) { create(:ci_runner, :instance, contacted_at: 1.month.ago, created_at: 3.months.ago) } - it { is_expected.to eq([runner1, runner3, runner4]) } + it { is_expected.to contain_exactly(runner1, runner3, runner4) } end describe '.active' do @@ -887,55 +887,40 @@ def stub_redis_runner_contacted_at(value) end describe '#status', :freeze_time do - let(:runner) { build(:ci_runner, :instance, created_at: 3.months.ago) } - subject { runner.status } context 'never connected' do - before do - runner.contacted_at = nil - end + let(:runner) { build(:ci_runner, :instance, :unregistered, created_at: 3.months.ago) } it { is_expected.to eq(:stale) } context 'created recently' do - before do - runner.created_at = 1.day.ago - end + let(:runner) { build(:ci_runner, :instance, :unregistered, created_at: 1.day.ago) } it { is_expected.to eq(:never_contacted) } end end context 'inactive but online' do - before do - runner.contacted_at = 1.second.ago - runner.active = false - end + let(:runner) { build(:ci_runner, :instance, active: false, contacted_at: 1.second.ago) } it { is_expected.to eq(:online) } end context 'contacted 1s ago' do - before do - runner.contacted_at = 1.second.ago - end + let(:runner) { build(:ci_runner, :instance, contacted_at: 1.second.ago) } it { is_expected.to eq(:online) } end context 'contacted recently' do - before do - runner.contacted_at = (3.months - 1.second).ago - end + let(:runner) { build(:ci_runner, :instance, contacted_at: (3.months - 1.second).ago) } it { is_expected.to eq(:offline) } end context 'contacted long time ago' do - before do - runner.contacted_at = 3.months.ago - end + let(:runner) { build(:ci_runner, :instance, created_at: 3.months.ago, contacted_at: 3.months.ago) } it { is_expected.to eq(:stale) } end @@ -1100,7 +1085,7 @@ def value_in_queues end it 'updates only ip_address' do - expect_redis_update(values.merge(contacted_at: Time.current)) + expect_redis_update(values.merge(contacted_at: Time.current, creation_state: :finished)) heartbeat end @@ -2150,7 +2135,7 @@ def does_db_update describe 'status scopes' do let_it_be(:online_runner) { create(:ci_runner, :instance, contacted_at: 1.second.ago) } let_it_be(:offline_runner) { create(:ci_runner, :instance, contacted_at: 2.hours.ago) } - let_it_be(:never_contacted_runner) { create(:ci_runner, :instance, contacted_at: nil) } + let_it_be(:never_contacted_runner) { create(:ci_runner, :instance, :unregistered) } describe '.online' do subject(:runners) { described_class.online } diff --git a/spec/requests/api/graphql/ci/runner_spec.rb b/spec/requests/api/graphql/ci/runner_spec.rb index ec12498a0451e98a3ba79e392450e6abeca96179..032c167a2c8526077889e91be2846ea7924670f5 100644 --- a/spec/requests/api/graphql/ci/runner_spec.rb +++ b/spec/requests/api/graphql/ci/runner_spec.rb @@ -708,7 +708,7 @@ end let_it_be(:never_contacted_instance_runner) do - create(:ci_runner, description: 'Missing runner 1', created_at: 1.month.ago, contacted_at: nil) + create(:ci_runner, :unregistered, description: 'Missing runner 1', created_at: 1.month.ago) end let(:query) do diff --git a/spec/services/ci/runners/register_runner_service_spec.rb b/spec/services/ci/runners/register_runner_service_spec.rb index 991de6a449d666fbcce4b019fa692556067f633c..645bec580da77a922d63ae2fa03a2bc4e2b0fdaf 100644 --- a/spec/services/ci/runners/register_runner_service_spec.rb +++ b/spec/services/ci/runners/register_runner_service_spec.rb @@ -242,7 +242,7 @@ context 'when it exceeds the application limits' do before do - create(:ci_runner, runner_type: :group_type, groups: [group], contacted_at: nil, created_at: 1.month.ago) + create(:ci_runner, :unregistered, runner_type: :group_type, groups: [group], created_at: 1.month.ago) create(:plan_limits, :default_plan, ci_registered_group_runners: 1) end @@ -261,7 +261,7 @@ context 'when abandoned runners cause application limits to not be exceeded' do before do create(:ci_runner, runner_type: :group_type, groups: [group], created_at: 4.months.ago, contacted_at: 3.months.ago) - create(:ci_runner, runner_type: :group_type, groups: [group], contacted_at: nil, created_at: 4.months.ago) + create(:ci_runner, :unregistered, runner_type: :group_type, groups: [group], created_at: 4.months.ago) create(:plan_limits, :default_plan, ci_registered_group_runners: 1) end