From 3dcf874e04d18e9ef57f74e1609b95ba851f3ac7 Mon Sep 17 00:00:00 2001 From: Pedro Pombeiro Date: Sat, 8 Jun 2024 09:58:35 +0200 Subject: [PATCH 1/4] Reduce runner stale timeout from 3 months to 7 days Changelog: changed --- app/models/ci/runner.rb | 2 +- doc/administration/instance_limits.md | 2 +- doc/api/graphql/reference/index.md | 4 +-- doc/ci/runners/runners_scope.md | 2 +- .../stale_group_runners_prune_service_spec.rb | 2 +- spec/models/ci/runner_spec.rb | 30 +++++++++---------- 6 files changed, 21 insertions(+), 21 deletions(-) diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index 0de619793473a4..57fbe48297011b 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -67,7 +67,7 @@ class Runner < Ci::ApplicationRecord UPDATE_CONTACT_COLUMN_EVERY = ((40.minutes)..(55.minutes)) # The `STALE_TIMEOUT` constant defines the how far past the last contact or creation date a runner will be considered stale - STALE_TIMEOUT = 3.months + STALE_TIMEOUT = 7.days # Only allow authentication token to be visible for a short while REGISTRATION_AVAILABILITY_TIME = 1.hour diff --git a/doc/administration/instance_limits.md b/doc/administration/instance_limits.md index 8e7993f5cae087..9b5980bcebf25f 100644 --- a/doc/administration/instance_limits.md +++ b/doc/administration/instance_limits.md @@ -675,7 +675,7 @@ of extra Pages deployments permitted for a top-level namespace is 1000. ### Number of registered runners per scope The total number of registered runners is limited at the group and project levels. Each time a new runner is registered, -GitLab checks these limits against runners that have been active in the last 3 months. +GitLab checks these limits against runners that have been active in the last 7 days. A runner's registration fails if it exceeds the limit for the scope determined by the runner registration token. If the limit value is set to zero, the limit is disabled. diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 80b4201a56bdf5..35f2cc66f4e892 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -33298,10 +33298,10 @@ Values for sorting runners. | ----- | ----------- | | `ACTIVE` **{warning-solid}** | **Deprecated** in GitLab 14.6. This was renamed. Use: [`CiRunner.paused`](#cirunnerpaused). | | `NEVER_CONTACTED` | Runner that has never contacted this instance. | -| `OFFLINE` | Runner that has not contacted this instance within the last 2 hours. Will be considered `STALE` if offline for more than 3 months. | +| `OFFLINE` | Runner that has not contacted this instance within the last 2 hours. Will be considered `STALE` if offline for more than 7 days. | | `ONLINE` | Runner that contacted this instance within the last 2 hours. | | `PAUSED` **{warning-solid}** | **Deprecated** in GitLab 14.6. This was renamed. Use: [`CiRunner.paused`](#cirunnerpaused). | -| `STALE` | Runner that has not contacted this instance within the last 3 months. | +| `STALE` | Runner that has not contacted this instance within the last 7 days. | ### `CiRunnerType` diff --git a/doc/ci/runners/runners_scope.md b/doc/ci/runners/runners_scope.md index a853c9978ef97d..78b72d63a2e476 100644 --- a/doc/ci/runners/runners_scope.md +++ b/doc/ci/runners/runners_scope.md @@ -584,7 +584,7 @@ A runner can have one of the following statuses. |---------|-------------| | `online` | The runner has contacted GitLab within the last 2 hours and is available to run jobs. | | `offline` | The runner has not contacted GitLab in more than 2 hours and is not available to run jobs. Check the runner to see if you can bring it online. | -| `stale` | The runner has not contacted GitLab in more than 3 months. If the runner was created more than 3 months ago, but it never contacted the instance, it is also considered **stale**. | +| `stale` | The runner has not contacted GitLab in more than 7 days. If the runner was created more than 7 days ago, but it never contacted the instance, it is also considered **stale**. | | `never_contacted` | The runner has never contacted GitLab. To make the runner contact GitLab, run `gitlab-runner run`. | ## View statistics for runner performance diff --git a/ee/spec/services/ci/runners/stale_group_runners_prune_service_spec.rb b/ee/spec/services/ci/runners/stale_group_runners_prune_service_spec.rb index 73585b7d598c67..c92e8519b27737 100644 --- a/ee/spec/services/ci/runners/stale_group_runners_prune_service_spec.rb +++ b/ee/spec/services/ci/runners/stale_group_runners_prune_service_spec.rb @@ -13,7 +13,7 @@ end let!(:stale_runners) do - create_list(:ci_runner, 3, :group, groups: [group1], created_at: 5.months.ago, contacted_at: 4.months.ago) + create_list(:ci_runner, 3, :group, groups: [group1], created_at: 5.months.ago, contacted_at: 8.days.ago) end let(:group2) { create(:group) } diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index eb440dd7ddf02e..e1b47c6b17d347 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -474,10 +474,10 @@ describe '.recent' do subject { described_class.recent } - let!(:runner1) { create(:ci_runner, :instance, contacted_at: nil, created_at: 2.months.ago) } - let!(:runner2) { create(:ci_runner, :instance, contacted_at: nil, created_at: 3.months.ago) } - 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) } + let!(:runner1) { create(:ci_runner, contacted_at: nil, created_at: 6.days.ago) } + let!(:runner2) { create(:ci_runner, contacted_at: nil, created_at: 7.days.ago) } + let!(:runner3) { create(:ci_runner, contacted_at: 1.day.ago, created_at: 6.days.ago) } + let!(:runner4) { create(:ci_runner, contacted_at: 1.day.ago, created_at: 7.days.ago) } it { is_expected.to contain_exactly(runner1, runner3, runner4) } end @@ -569,11 +569,11 @@ using RSpec::Parameterized::TableSyntax where(:created_at, :contacted_at, :expected_stale?) do - nil | nil | false - 3.months.ago | 3.months.ago | true - 3.months.ago | (3.months - 1.hour).ago | false - 3.months.ago | nil | true - (3.months - 1.hour).ago | nil | false + nil | nil | false + 7.days.ago | 7.days.ago | true + 7.days.ago | (7.days - 1.hour).ago | false + 7.days.ago | nil | true + (7.days - 1.hour).ago | nil | false end with_them do @@ -866,7 +866,7 @@ def stub_redis_runner_contacted_at(value) subject { runner.status } context 'never connected' do - let(:runner) { build(:ci_runner, :instance, :unregistered, created_at: 3.months.ago) } + let(:runner) { build(:ci_runner, :instance, :unregistered, created_at: 7.days.ago) } it { is_expected.to eq(:stale) } @@ -890,13 +890,13 @@ def stub_redis_runner_contacted_at(value) end context 'contacted recently' do - let(:runner) { build(:ci_runner, :instance, contacted_at: (3.months - 1.second).ago) } + let(:runner) { build(:ci_runner, :instance, contacted_at: (7.days - 1.second).ago) } it { is_expected.to eq(:offline) } end context 'contacted long time ago' do - let(:runner) { build(:ci_runner, :instance, created_at: 3.months.ago, contacted_at: 3.months.ago) } + let(:runner) { build(:ci_runner, :instance, created_at: 7.days.ago, contacted_at: 7.days.ago) } it { is_expected.to eq(:stale) } end @@ -925,8 +925,8 @@ def stub_redis_runner_contacted_at(value) context 'contacted long time ago' do before do - runner.created_at = 3.months.ago - runner.contacted_at = 3.months.ago + runner.created_at = 7.days.ago + runner.contacted_at = 7.days.ago end it { is_expected.to eq(:stale) } @@ -2042,7 +2042,7 @@ def does_db_update describe '.stale_deadline', :freeze_time do subject { described_class.stale_deadline } - it { is_expected.to eq(3.months.ago) } + it { is_expected.to eq(7.days.ago) } end describe '.with_runner_type' do -- GitLab From 6fe6d7019f6778e501fa5c77b40e970c21cf575f Mon Sep 17 00:00:00 2001 From: Pedro Pombeiro Date: Mon, 10 Jun 2024 12:22:34 +0200 Subject: [PATCH 2/4] Fix missed specs --- spec/features/admin/admin_runners_spec.rb | 2 +- spec/helpers/ci/runners_helper_spec.rb | 2 +- spec/requests/api/graphql/ci/runner_spec.rb | 2 +- spec/services/ci/runners/register_runner_service_spec.rb | 2 +- spec/support/shared_examples/helpers/runners_shared_examples.rb | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/features/admin/admin_runners_spec.rb b/spec/features/admin/admin_runners_spec.rb index 8eedd53e58f425..ce520adb1de780 100644 --- a/spec/features/admin/admin_runners_spec.rb +++ b/spec/features/admin/admin_runners_spec.rb @@ -62,8 +62,8 @@ context "with multiple runners" do before do create(:ci_runner, :instance, created_at: 1.year.ago, contacted_at: Time.zone.now) + create(:ci_runner, :instance, created_at: 1.year.ago, contacted_at: 1.day.ago) create(:ci_runner, :instance, created_at: 1.year.ago, contacted_at: 1.week.ago) - create(:ci_runner, :instance, created_at: 1.year.ago, contacted_at: 1.year.ago) visit admin_runners_path end diff --git a/spec/helpers/ci/runners_helper_spec.rb b/spec/helpers/ci/runners_helper_spec.rb index 9252f765579fb9..3f928667dad975 100644 --- a/spec/helpers/ci/runners_helper_spec.rb +++ b/spec/helpers/ci/runners_helper_spec.rb @@ -147,7 +147,7 @@ group_full_path: group.full_path, runner_install_help_page: 'https://docs.gitlab.com/runner/install/', online_contact_timeout_secs: 7200, - stale_timeout_secs: 7889238 + stale_timeout_secs: 604800 ) end end diff --git a/spec/requests/api/graphql/ci/runner_spec.rb b/spec/requests/api/graphql/ci/runner_spec.rb index fc5aa59b731cfd..437a56b3cd3380 100644 --- a/spec/requests/api/graphql/ci/runner_spec.rb +++ b/spec/requests/api/graphql/ci/runner_spec.rb @@ -689,7 +689,7 @@ end let_it_be(:never_contacted_instance_runner) do - create(:ci_runner, :unregistered, description: 'Missing runner 1', created_at: 1.month.ago) + create(:ci_runner, :unregistered, description: 'Missing runner 1', created_at: 6.days.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 7c7c27a3d19fa3..1dc6e760c088f3 100644 --- a/spec/services/ci/runners/register_runner_service_spec.rb +++ b/spec/services/ci/runners/register_runner_service_spec.rb @@ -246,7 +246,7 @@ context 'when it exceeds the application limits' do before do - create(:ci_runner, :unregistered, runner_type: :group_type, groups: [group], created_at: 1.month.ago) + create(:ci_runner, :unregistered, runner_type: :group_type, groups: [group], created_at: 6.days.ago) create(:plan_limits, :default_plan, ci_registered_group_runners: 1) end diff --git a/spec/support/shared_examples/helpers/runners_shared_examples.rb b/spec/support/shared_examples/helpers/runners_shared_examples.rb index e509f7a65a5589..01f38d13c06f9a 100644 --- a/spec/support/shared_examples/helpers/runners_shared_examples.rb +++ b/spec/support/shared_examples/helpers/runners_shared_examples.rb @@ -6,7 +6,7 @@ runner_install_help_page: 'https://docs.gitlab.com/runner/install/', registration_token: Gitlab::CurrentSettings.runners_registration_token, online_contact_timeout_secs: 7200, - stale_timeout_secs: 7889238 + stale_timeout_secs: 604800 ) end end -- GitLab From dcdc3807ea7f374a92f5c24c3d7ffb866f9bde68 Mon Sep 17 00:00:00 2001 From: Pedro Pombeiro Date: Mon, 10 Jun 2024 16:00:01 +0200 Subject: [PATCH 3/4] Add history entry to documentation --- doc/administration/instance_limits.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/administration/instance_limits.md b/doc/administration/instance_limits.md index 9b5980bcebf25f..f9d701a280798c 100644 --- a/doc/administration/instance_limits.md +++ b/doc/administration/instance_limits.md @@ -674,6 +674,8 @@ of extra Pages deployments permitted for a top-level namespace is 1000. ### Number of registered runners per scope +> - [Changed](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/155795) in GitLab 17.1 to reduce runner TTL from 3 months to 7 days. + The total number of registered runners is limited at the group and project levels. Each time a new runner is registered, GitLab checks these limits against runners that have been active in the last 7 days. A runner's registration fails if it exceeds the limit for the scope determined by the runner registration token. -- GitLab From ab4b900a58533ad1c4748185efea088ba0b5fce7 Mon Sep 17 00:00:00 2001 From: Pedro Pombeiro Date: Mon, 10 Jun 2024 16:24:54 +0200 Subject: [PATCH 4/4] Apply MR review suggestion --- doc/administration/instance_limits.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/administration/instance_limits.md b/doc/administration/instance_limits.md index f9d701a280798c..99712755bc2242 100644 --- a/doc/administration/instance_limits.md +++ b/doc/administration/instance_limits.md @@ -674,7 +674,7 @@ of extra Pages deployments permitted for a top-level namespace is 1000. ### Number of registered runners per scope -> - [Changed](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/155795) in GitLab 17.1 to reduce runner TTL from 3 months to 7 days. +> - Runner stale timeout [changed](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/155795) from 3 months to 7 days in GitLab 17.1. The total number of registered runners is limited at the group and project levels. Each time a new runner is registered, GitLab checks these limits against runners that have been active in the last 7 days. -- GitLab