From 8b0a220a0e056571e810415fabefb5c02ba75090 Mon Sep 17 00:00:00 2001 From: Tiger Date: Fri, 13 Jun 2025 13:21:22 +1200 Subject: [PATCH] Store the most recent deployment job's auto_stop_in on the environment When an environment is configured with an auto stop time, CI jobs for the environment that use the `prepare` or `access` action will cause the stop time to be recalculated using the value that was originally used to set it. This value is stored as metadata (`Ci::BuildMetadata`) associated with the most recent deployment job. With this change the value is now stored on the environment record itself. This means the auto stop time can be recalculated without having to fetch metadata from an old job, which in turn allows the metadata to be removed without losing the ability to recalculate the timer. Changelog: other --- app/models/environment.rb | 8 ++++++++ .../recalculate_auto_stop_service.rb | 11 +++++++++- ...add_latest_auto_stop_in_to_environments.rb | 20 +++++++++++++++++++ db/schema_migrations/20250613003344 | 1 + db/structure.sql | 2 ++ spec/models/environment_spec.rb | 2 ++ .../recalculate_auto_stop_service_spec.rb | 18 +++++++++++++++++ 7 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20250613003344_add_latest_auto_stop_in_to_environments.rb create mode 100644 db/schema_migrations/20250613003344 diff --git a/app/models/environment.rb b/app/models/environment.rb index 71f421fb55d6b3..af9f45cd787e22 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -84,6 +84,10 @@ class Environment < ApplicationRecord allow_nil: true, if: :description_html_changed? + validates :latest_configured_auto_stop_in, + length: { maximum: 255 }, + allow_nil: true + validates :kubernetes_namespace, allow_nil: true, length: 1..63, @@ -527,6 +531,10 @@ def auto_stop_in=(value) parser = ::Gitlab::Ci::Build::DurationParser.new(value) + # The raw value is stored to allow the auto stop + # time to be recalculated without needing to fetch + # the original value from the CI job that set it. + self.latest_configured_auto_stop_in = value self.auto_stop_at = parser.seconds_from_now rescue ChronicDuration::DurationParseError => ex Gitlab::ErrorTracking.track_exception(ex, project_id: self.project_id, environment_id: self.id) diff --git a/app/services/environments/recalculate_auto_stop_service.rb b/app/services/environments/recalculate_auto_stop_service.rb index 8bcd7cac68f456..c82eb05508285d 100644 --- a/app/services/environments/recalculate_auto_stop_service.rb +++ b/app/services/environments/recalculate_auto_stop_service.rb @@ -13,13 +13,22 @@ def execute return unless can_set_auto_stop? && environment.present? auto_stop_in = deployable.expanded_auto_stop_in - auto_stop_in ||= last_successful_deployable&.expanded_auto_stop_in if can_reset_timer? + auto_stop_in ||= last_deployable_auto_stop_in if can_reset_timer? environment.update!(auto_stop_in: auto_stop_in) if auto_stop_in.present? end private + # TODO: We are transitioning from using the value stored on the last + # successful deployment job to the one stored on the environment. The + # fallback can be removed once the transition is complete (at this + # point the value will no longer exist at the old location). + # See https://gitlab.com/gitlab-org/gitlab/-/issues/545659. + def last_deployable_auto_stop_in + environment.latest_configured_auto_stop_in || last_successful_deployable&.expanded_auto_stop_in + end + # Jobs that start an environment (using `action: start`) can also # specify a stop time, however this is handled by the deployment # process. Actions other than `start` do not create deployments, diff --git a/db/migrate/20250613003344_add_latest_auto_stop_in_to_environments.rb b/db/migrate/20250613003344_add_latest_auto_stop_in_to_environments.rb new file mode 100644 index 00000000000000..47c48a3839ffef --- /dev/null +++ b/db/migrate/20250613003344_add_latest_auto_stop_in_to_environments.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +class AddLatestAutoStopInToEnvironments < Gitlab::Database::Migration[2.3] + disable_ddl_transaction! + milestone '18.1' + + def up + with_lock_retries do + add_column :environments, :latest_configured_auto_stop_in, :text, if_not_exists: true + end + + add_text_limit :environments, :latest_configured_auto_stop_in, 255 + end + + def down + with_lock_retries do + remove_column :environments, :latest_configured_auto_stop_in + end + end +end diff --git a/db/schema_migrations/20250613003344 b/db/schema_migrations/20250613003344 new file mode 100644 index 00000000000000..007e0c7a02b8d8 --- /dev/null +++ b/db/schema_migrations/20250613003344 @@ -0,0 +1 @@ +4f6c23fb39965b5fc4a4485003f45294b9998d396f35a100f819c246ba78ec21 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 0d3e95a71c26f1..cc62b10bcc3eba 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -14224,7 +14224,9 @@ CREATE TABLE environments ( description_html text, cached_markdown_version integer, auto_stop_setting smallint DEFAULT 0 NOT NULL, + latest_configured_auto_stop_in text, CONSTRAINT check_23b1eb18a2 CHECK ((char_length(flux_resource_path) <= 255)), + CONSTRAINT check_4a037337eb CHECK ((char_length(latest_configured_auto_stop_in) <= 255)), CONSTRAINT check_ad5e1ed5e1 CHECK ((char_length(description) <= 10000)), CONSTRAINT check_b5373a1804 CHECK ((char_length(kubernetes_namespace) <= 63)), CONSTRAINT check_d26221226f CHECK ((char_length(description_html) <= 50000)) diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 5e4a66ee1d4364..a685e109f80bc6 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -43,6 +43,7 @@ it { is_expected.to validate_length_of(:flux_resource_path).is_at_most(255) } it { is_expected.to validate_length_of(:description).is_at_most(10000) } it { is_expected.to validate_length_of(:description_html).is_at_most(50000) } + it { is_expected.to validate_length_of(:latest_configured_auto_stop_in).is_at_most(255) } describe 'validation' do it 'does not become invalid record when external_url is empty' do @@ -1978,6 +1979,7 @@ def create_deployment_with_stop_action(status, pipeline, stop_action_name) if expected_result.is_a?(Integer) || expected_result.nil? subject + expect(environment.latest_configured_auto_stop_in).to eq(value) expect(environment.auto_stop_in).to eq(expected_result) else expect { subject }.to raise_error(expected_result) diff --git a/spec/services/environments/recalculate_auto_stop_service_spec.rb b/spec/services/environments/recalculate_auto_stop_service_spec.rb index 4443ce8a85f7fa..32c1ceb82990b8 100644 --- a/spec/services/environments/recalculate_auto_stop_service_spec.rb +++ b/spec/services/environments/recalculate_auto_stop_service_spec.rb @@ -90,6 +90,24 @@ it 'does not update the environment' do expect { recalculate }.not_to change { environment.reload.auto_stop_at } end + + context 'when the latest_configured_auto_stop_in is stored on the environment' do + before do + environment.update!(latest_configured_auto_stop_in: '1 week') + end + + if can_reset_timer + it 'updates the environment auto_stop_at' do + expected_stop_at = be_like_time(1.week.from_now) + + expect { recalculate }.to change { environment.reload.auto_stop_at }.from(nil).to(expected_stop_at) + end + else + it 'does not update the environment' do + expect { recalculate }.not_to change { environment.reload.auto_stop_at } + end + end + end end end end -- GitLab