From 5745eb74a9054ce7853a03d801e86050b8f7b2ec Mon Sep 17 00:00:00 2001 From: Pam Artiaga Date: Tue, 12 Dec 2023 11:20:55 +1100 Subject: [PATCH 1/4] update models --- app/models/ci/pipeline.rb | 1 + app/models/deployment.rb | 25 +++++++++++++++++++++++++ app/models/environment.rb | 5 +++++ 3 files changed, 31 insertions(+) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 6331b5880e0cef..449ee85b086abc 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -102,6 +102,7 @@ class Pipeline < Ci::ApplicationRecord not_expired.or(where_exists(Ci::Pipeline.artifacts_locked.where("#{Ci::Pipeline.quoted_table_name}.id = #{Ci::Build.quoted_table_name}.commit_id"))).downloadable.with_job end, through: :latest_builds, source: :job_artifacts has_many :latest_successful_jobs, ->(pipeline) { in_partition(pipeline).latest.success.with_project_and_metadata(pipeline.project) }, foreign_key: :commit_id, inverse_of: :pipeline, class_name: 'Ci::Processable' + has_many :latest_finished_jobs, ->(pipeline) { in_partition(pipeline).latest.finished.with_project_and_metadata(pipeline.project) }, foreign_key: :commit_id, inverse_of: :pipeline, class_name: 'Ci::Processable' has_many :messages, class_name: 'Ci::PipelineMessage', inverse_of: :pipeline diff --git a/app/models/deployment.rb b/app/models/deployment.rb index 36f4a0ef426ce8..aa672e379881ed 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -51,6 +51,7 @@ class Deployment < ApplicationRecord scope :for_projects, -> (projects) { where(project: projects) } scope :visible, -> { where(status: VISIBLE_STATUSES) } + scope :finished, -> { where(status: FINISHED_STATUSES) } scope :stoppable, -> { where.not(on_stop: nil).where.not(deployable_id: nil).success } scope :active, -> { where(status: %i[created running]) } scope :upcoming, -> { where(status: %i[blocked running]) } @@ -208,6 +209,30 @@ def self.last_deployment_group_for_environment(env) end end + def self.last_finished_deployment_group_for_environment(env) + return self.none unless env.last_finished_deployment_pipeline&.latest_finished_jobs&.present? + + BatchLoader.for(env).batch(default_value: self.none) do |environments, loader| + latest_finished_job_ids = [] + environments_hash = {} + + environments.each do |environment| + environments_hash[environment.id] = environment + + # Refer comment note above, if not preloaded this can lead to N+1. + latest_finished_job_ids << environment.last_finished_deployment_pipeline.latest_finished_jobs.map(&:id) + end + + Deployment + .where(deployable_type: 'CommitStatus', deployable_id: latest_finished_job_ids.flatten) + .preload(last_deployment_group_associations) + .group_by { |deployment| deployment.environment_id } + .each do |env_id, deployment_group| + loader.call(environments_hash[env_id], deployment_group) + end + end + end + def self.find_successful_deployment!(iid) success.find_by!(iid: iid) end diff --git a/app/models/environment.rb b/app/models/environment.rb index 4f76fae24ebfa2..1e78dda5f63eec 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -32,6 +32,7 @@ class Environment < ApplicationRecord # NOTE: If you preload multiple last deployments of environments, use Preloaders::Environments::DeploymentPreloader. has_one :last_deployment, -> { success.ordered }, class_name: 'Deployment', inverse_of: :environment has_one :last_visible_deployment, -> { visible.order(id: :desc) }, inverse_of: :environment, class_name: 'Deployment' + has_one :last_finished_deployment, -> { finished.order(id: :desc) }, class_name: 'Deployment', inverse_of: :environment has_one :upcoming_deployment, -> { upcoming.order(id: :desc) }, class_name: 'Deployment', inverse_of: :environment Deployment::FINISHED_STATUSES.each do |status| @@ -271,6 +272,10 @@ def last_deployment_pipeline last_deployable&.pipeline end + def last_finished_deployment_pipeline + last_finished_deployment&.deployable&.pipeline + end + # This method returns the deployment records of the last deployment pipeline, that successfully executed to this environment. # e.g. # A pipeline contains -- GitLab From 633c911fd3215a2dcb5be467aa0eb9610d10d6ee Mon Sep 17 00:00:00 2001 From: Pam Artiaga Date: Tue, 12 Dec 2023 16:38:22 +1100 Subject: [PATCH 2/4] Introduce FF and modify stop_actions --- app/models/environment.rb | 14 ++++++++++++-- ...op_actions_include_all_finished_deployments.yml | 8 ++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) create mode 100644 config/feature_flags/development/environment_stop_actions_include_all_finished_deployments.yml diff --git a/app/models/environment.rb b/app/models/environment.rb index 1e78dda5f63eec..9dbc595da10e19 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -387,11 +387,21 @@ def stop_with_actions!(current_user) end def stop_actions - strong_memoize(:stop_actions) do - last_deployment_group.map(&:stop_action).compact + if Feature.enabled?(:environment_stop_actions_include_all_finished_deployments) + strong_memoize(:stop_actions_for_all_finished_deployments) do + last_finished_deployment_group.map(&:stop_action).compact + end + else + strong_memoize(:stop_actions) do + last_deployment_group.map(&:stop_action).compact + end end end + def last_finished_deployment_group + Deployment.last_finished_deployment_group_for_environment(self) + end + def last_deployment_group Deployment.last_deployment_group_for_environment(self) end diff --git a/config/feature_flags/development/environment_stop_actions_include_all_finished_deployments.yml b/config/feature_flags/development/environment_stop_actions_include_all_finished_deployments.yml new file mode 100644 index 00000000000000..6b8a066ff3e971 --- /dev/null +++ b/config/feature_flags/development/environment_stop_actions_include_all_finished_deployments.yml @@ -0,0 +1,8 @@ +--- +name: environment_stop_actions_include_all_finished_deployments +introduced_by_url: +rollout_issue_url: +milestone: '17.0' +type: development +group: group::environments +default_enabled: false -- GitLab From 68e79b10bccdce398b9e6113ecff10ad5d964d51 Mon Sep 17 00:00:00 2001 From: Pam Artiaga Date: Wed, 13 Dec 2023 12:27:59 +1100 Subject: [PATCH 3/4] Add comments in all affected parts --- app/controllers/projects/environments_controller.rb | 2 ++ app/models/environment.rb | 11 +++++++++++ app/serializers/environment_serializer.rb | 8 ++++++-- app/workers/environments/auto_recover_worker.rb | 1 + 4 files changed, 20 insertions(+), 2 deletions(-) diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index 8cdd6efa7c5652..412791f820cd2d 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -113,6 +113,8 @@ def update end end + # called when 'stop' -> 'stop environment' button is clicked + # see spec/features/projects/environments/environment_spec.rb:'when user has ability to stop environment' example def stop return render_404 unless @environment.available? diff --git a/app/models/environment.rb b/app/models/environment.rb index 9dbc595da10e19..2092815336f348 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -345,6 +345,13 @@ def formatted_external_url external_url.gsub(%r{\A.*?://}, '') end + # Used in: + # 1- EnvironmentPolicy (app/policies/environment_policy.rb) to determine the :stop_environment permission + # the :stop_environment permission is checked for various authorizations, + # ie: if the enviroments/stop request is authorized in either the Environments Controller or GraphQL request + # 2- EnvironmentEntity (app/serializers/environment_entity.rb), exposed as `has_stop_action` + # this is used in determining if the stop environment modal should have a warning that the environment + # does not have stop actions (app/assets/javascripts/environments/components/stop_environment_modal.vue) def stop_actions_available? available? && stop_actions.present? end @@ -359,10 +366,14 @@ def cancel_deployment_jobs! end end + # This is used to determine the correct environment state transition when an environment is stopped + # - if an environment does not have stop_actions, the environment state immediately transitions to :stopped + # - if an environment has stop_actions, the environment state transitions to :stopping -> then :stopped once all the stop jobs are run def wait_for_stop? stop_actions.present? end + # Called to stop an environment; this can be from the EnvironmentsController, Environments::StopService, or Environments::AutoStopWorker def stop_with_actions!(current_user) return unless available? diff --git a/app/serializers/environment_serializer.rb b/app/serializers/environment_serializer.rb index 8f3aeea2eedc55..b105925170f092 100644 --- a/app/serializers/environment_serializer.rb +++ b/app/serializers/environment_serializer.rb @@ -68,8 +68,12 @@ def batch_load(resource) environment.last_deployment&.commit&.try(:lazy_author) environment.upcoming_deployment&.commit&.try(:lazy_author) - # Batch loading last_deployment_group which is called later by environment.stop_actions - environment.last_deployment_group + # Batch loading last_deployment_group or last_finished_deployment_group which is called later by environment.stop_actions + if Feature.enabled?(:environment_stop_actions_include_all_finished_deployments) + environment.last_finished_deployment_group + else + environment.last_deployment_group + end end end end diff --git a/app/workers/environments/auto_recover_worker.rb b/app/workers/environments/auto_recover_worker.rb index 75e86e38f1a09b..98e719005f78ec 100644 --- a/app/workers/environments/auto_recover_worker.rb +++ b/app/workers/environments/auto_recover_worker.rb @@ -9,6 +9,7 @@ class AutoRecoverWorker idempotent! feature_category :continuous_delivery + # This is also affected by the change in `environment.stop_actions` def perform(environment_id, _params = {}) Environment.find_by_id(environment_id).try do |environment| next unless environment.long_stopping? -- GitLab From 661c821f4241cbf1daa33a12bcf537933a58ba8d Mon Sep 17 00:00:00 2001 From: Pam Artiaga Date: Wed, 13 Dec 2023 17:38:07 +1100 Subject: [PATCH 4/4] Check FF by project --- app/models/environment.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/environment.rb b/app/models/environment.rb index 2092815336f348..2a7659814ea8fd 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -398,7 +398,7 @@ def stop_with_actions!(current_user) end def stop_actions - if Feature.enabled?(:environment_stop_actions_include_all_finished_deployments) + if Feature.enabled?(:environment_stop_actions_include_all_finished_deployments, project) strong_memoize(:stop_actions_for_all_finished_deployments) do last_finished_deployment_group.map(&:stop_action).compact end -- GitLab