Fix environments are stopped incorrectly in merge requests
What does this MR do and why?
This MR fixes the regression that was accidentally introduced by !71591 (merged). The MergeRequest#environments
is supposed to fetch only environments that were started by the MR, however, currently it includes environments even if it's not started by the MR.
See this comment for more analysis on this problem and customer impact.
Fix #346152 (closed)
Screenshots or screen recordings
These are strongly recommended to assist reviewers and reduce the time to merge your change.
How to set up and validate locally
Run a pipeline with the following .gitlab-ci.yml
start review:
script: echo
environment:
name: review/$CI_COMMIT_REF_NAME
on_stop: stop review
stop review:
script: echo
environment:
name: review/$CI_COMMIT_REF_NAME
action: stop
when: manual
prepare for dev:
script: echo
environment:
name: dev
action: prepare
Before
[2] pry(main)> MergeRequest.last.environments_in_head_pipeline
MergeRequest Load (0.8ms) SELECT "merge_requests".* FROM "merge_requests" ORDER BY "merge_requests"."id" DESC LIMIT 1 /*application:console,db_config_name:main,line:/devkitkat/services/rails/cache/ruby/2.7.0/gems/marginalia-1.10.0/lib/marginalia/comment.rb:25:in `block in construct_comment'*/
Project Load (0.8ms) SELECT "projects"."id", "projects"."name", "projects"."path", "projects"."description", "projects"."created_at", "projects"."updated_at", "projects"."creator_id", "projects"."namespace_id", "projects"."last_activity_at", "projects"."import_url", "projects"."visibility_level", "projects"."archived", "projects"."avatar", "projects"."merge_requests_template", "projects"."star_count", "projects"."merge_requests_rebase_enabled", "projects"."import_type", "projects"."import_source", "projects"."approvals_before_merge", "projects"."reset_approvals_on_push", "projects"."merge_requests_ff_only_enabled", "projects"."issues_template", "projects"."mirror", "projects"."mirror_user_id", "projects"."shared_runners_enabled", "projects"."runners_token", "projects"."build_coverage_regex", "projects"."build_allow_git_fetch", "projects"."build_timeout", "projects"."mirror_trigger_builds", "projects"."pending_delete", "projects"."public_builds", "projects"."last_repository_check_failed", "projects"."last_repository_check_at", "projects"."only_allow_merge_if_pipeline_succeeds", "projects"."has_external_issue_tracker", "projects"."repository_storage", "projects"."repository_read_only", "projects"."request_access_enabled", "projects"."has_external_wiki", "projects"."ci_config_path", "projects"."lfs_enabled", "projects"."description_html", "projects"."only_allow_merge_if_all_discussions_are_resolved", "projects"."repository_size_limit", "projects"."printing_merge_request_link_enabled", "projects"."auto_cancel_pending_pipelines", "projects"."service_desk_enabled", "projects"."cached_markdown_version", "projects"."delete_error", "projects"."last_repository_updated_at", "projects"."disable_overriding_approvers_per_merge_request", "projects"."storage_version", "projects"."resolve_outdated_diff_discussions", "projects"."remote_mirror_available_overridden", "projects"."only_mirror_protected_branches", "projects"."pull_mirror_available_overridden", "projects"."jobs_cache_index", "projects"."external_authorization_classification_label", "projects"."mirror_overwrites_diverged_branches", "projects"."pages_https_only", "projects"."external_webhook_token", "projects"."packages_enabled", "projects"."merge_requests_author_approval", "projects"."pool_repository_id", "projects"."runners_token_encrypted", "projects"."bfg_object_map", "projects"."detected_repository_languages", "projects"."merge_requests_disable_committers_approval", "projects"."require_password_to_approve", "projects"."emails_disabled", "projects"."max_pages_size", "projects"."max_artifacts_size", "projects"."remove_source_branch_after_merge", "projects"."marked_for_deletion_at", "projects"."marked_for_deletion_by_user_id", "projects"."autoclose_referenced_issues", "projects"."suggestion_commit_message", "projects"."project_namespace_id", "projects"."hidden" FROM "projects" WHERE "projects"."id" = 21 LIMIT 1 /*application:console,db_config_name:main,line:/devkitkat/services/rails/cache/ruby/2.7.0/gems/marginalia-1.10.0/lib/marginalia/comment.rb:25:in `block in construct_comment'*/
Ci::Pipeline Load (0.8ms) SELECT "ci_pipelines".* FROM "ci_pipelines" WHERE "ci_pipelines"."id" = 95 LIMIT 1 /*application:console,db_config_name:ci,line:/devkitkat/services/rails/cache/ruby/2.7.0/gems/marginalia-1.10.0/lib/marginalia/comment.rb:25:in `block in construct_comment'*/
MergeRequestDiff Load (0.2ms) SELECT "merge_request_diffs".* FROM "merge_request_diffs" WHERE "merge_request_diffs"."id" = 73 LIMIT 1 /*application:console,db_config_name:main,line:/devkitkat/services/rails/cache/ruby/2.7.0/gems/marginalia-1.10.0/lib/marginalia/comment.rb:25:in `block in construct_comment'*/
(0.7ms) SELECT DISTINCT "expanded_environment_name" FROM "ci_builds" INNER JOIN "ci_builds_metadata" ON "ci_builds_metadata"."build_id" = "ci_builds"."id" WHERE "ci_builds"."type" = 'Ci::Build' AND ("ci_builds"."retried" = FALSE OR "ci_builds"."retried" IS NULL) AND "ci_builds"."commit_id" = 95 AND "ci_builds_metadata"."expanded_environment_name" IS NOT NULL LIMIT 100 /*application:console,db_config_name:ci,line:/devkitkat/services/rails/cache/ruby/2.7.0/gems/marginalia-1.10.0/lib/marginalia/comment.rb:25:in `block in construct_comment'*/
Environment Load (0.3ms) SELECT "environments".* FROM "environments" WHERE "environments"."project_id" = 21 AND "environments"."name" IN ('dev', 'review/feature-1') /*application:console,db_config_name:main,line:/devkitkat/services/rails/cache/ruby/2.7.0/gems/marginalia-1.10.0/lib/marginalia/comment.rb:25:in `block in construct_comment'*/
=> [#<Environment:0x000055a157b92898
id: 34,
project_id: 21,
name: "dev",
created_at: Thu, 31 Mar 2022 05:52:50.764661000 UTC +00:00,
updated_at: Thu, 31 Mar 2022 05:52:50.764661000 UTC +00:00,
external_url: nil,
environment_type: nil,
state: "available",
slug: "dev",
auto_stop_at: nil,
auto_delete_at: nil,
tier: "development">,
#<Environment:0x000055a157b927d0
id: 35,
project_id: 21,
name: "review/feature-1",
created_at: Thu, 31 Mar 2022 05:53:30.290572000 UTC +00:00,
updated_at: Thu, 31 Mar 2022 05:53:30.290572000 UTC +00:00,
external_url: nil,
environment_type: "review",
state: "available",
slug: "review-feature-1-1q251u",
auto_stop_at: nil,
auto_delete_at: nil,
tier: "development">]
After
[1] pry(main)> MergeRequest.last.environments_in_head_pipeline
MergeRequest Load (1.0ms) SELECT "merge_requests".* FROM "merge_requests" ORDER BY "merge_requests"."id" DESC LIMIT 1 /*application:console,db_config_name:main,line:/devkitkat/services/rails/cache/ruby/2.7.0/gems/marginalia-1.10.0/lib/marginalia/comment.rb:25:in `block in construct_comment'*/
Project Load (1.5ms) SELECT "projects"."id", "projects"."name", "projects"."path", "projects"."description", "projects"."created_at", "projects"."updated_at", "projects"."creator_id", "projects"."namespace_id", "projects"."last_activity_at", "projects"."import_url", "projects"."visibility_level", "projects"."archived", "projects"."avatar", "projects"."merge_requests_template", "projects"."star_count", "projects"."merge_requests_rebase_enabled", "projects"."import_type", "projects"."import_source", "projects"."approvals_before_merge", "projects"."reset_approvals_on_push", "projects"."merge_requests_ff_only_enabled", "projects"."issues_template", "projects"."mirror", "projects"."mirror_user_id", "projects"."shared_runners_enabled", "projects"."runners_token", "projects"."build_coverage_regex", "projects"."build_allow_git_fetch", "projects"."build_timeout", "projects"."mirror_trigger_builds", "projects"."pending_delete", "projects"."public_builds", "projects"."last_repository_check_failed", "projects"."last_repository_check_at", "projects"."only_allow_merge_if_pipeline_succeeds", "projects"."has_external_issue_tracker", "projects"."repository_storage", "projects"."repository_read_only", "projects"."request_access_enabled", "projects"."has_external_wiki", "projects"."ci_config_path", "projects"."lfs_enabled", "projects"."description_html", "projects"."only_allow_merge_if_all_discussions_are_resolved", "projects"."repository_size_limit", "projects"."printing_merge_request_link_enabled", "projects"."auto_cancel_pending_pipelines", "projects"."service_desk_enabled", "projects"."cached_markdown_version", "projects"."delete_error", "projects"."last_repository_updated_at", "projects"."disable_overriding_approvers_per_merge_request", "projects"."storage_version", "projects"."resolve_outdated_diff_discussions", "projects"."remote_mirror_available_overridden", "projects"."only_mirror_protected_branches", "projects"."pull_mirror_available_overridden", "projects"."jobs_cache_index", "projects"."external_authorization_classification_label", "projects"."mirror_overwrites_diverged_branches", "projects"."pages_https_only", "projects"."external_webhook_token", "projects"."packages_enabled", "projects"."merge_requests_author_approval", "projects"."pool_repository_id", "projects"."runners_token_encrypted", "projects"."bfg_object_map", "projects"."detected_repository_languages", "projects"."merge_requests_disable_committers_approval", "projects"."require_password_to_approve", "projects"."emails_disabled", "projects"."max_pages_size", "projects"."max_artifacts_size", "projects"."remove_source_branch_after_merge", "projects"."marked_for_deletion_at", "projects"."marked_for_deletion_by_user_id", "projects"."autoclose_referenced_issues", "projects"."suggestion_commit_message", "projects"."project_namespace_id", "projects"."hidden" FROM "projects" WHERE "projects"."id" = 21 LIMIT 1 /*application:console,db_config_name:main,line:/devkitkat/services/rails/cache/ruby/2.7.0/gems/marginalia-1.10.0/lib/marginalia/comment.rb:25:in `block in construct_comment'*/
Feature::FlipperGate Load (0.5ms) SELECT "feature_gates".* FROM "feature_gates" WHERE "feature_gates"."feature_key" = 'fix_related_environments_for_merge_requests' /*application:console,db_config_name:main,line:/devkitkat/services/rails/cache/ruby/2.7.0/gems/marginalia-1.10.0/lib/marginalia/comment.rb:25:in `block in construct_comment'*/
Ci::Pipeline Load (0.7ms) SELECT "ci_pipelines".* FROM "ci_pipelines" WHERE "ci_pipelines"."id" = 95 LIMIT 1 /*application:console,db_config_name:ci,line:/devkitkat/services/rails/cache/ruby/2.7.0/gems/marginalia-1.10.0/lib/marginalia/comment.rb:25:in `block in construct_comment'*/
Creating scope :verification_succeeded. Overwriting existing method MergeRequestDiff.verification_succeeded.
Creating scope :verification_failed. Overwriting existing method MergeRequestDiff.verification_failed.
Creating scope :available_replicables. Overwriting existing method MergeRequestDiff.available_replicables.
Creating scope :available_verifiables. Overwriting existing method MergeRequestDiff.available_verifiables.
Creating scope :with_verification_state. Overwriting existing method MergeRequestDiff.with_verification_state.
Creating scope :checksummed. Overwriting existing method MergeRequestDiff.checksummed.
Creating scope :not_checksummed. Overwriting existing method MergeRequestDiff.not_checksummed.
MergeRequestDiff Load (0.4ms) SELECT "merge_request_diffs".* FROM "merge_request_diffs" WHERE "merge_request_diffs"."id" = 73 LIMIT 1 /*application:console,db_config_name:main,line:/devkitkat/services/rails/cache/ruby/2.7.0/gems/marginalia-1.10.0/lib/marginalia/comment.rb:25:in `block in construct_comment'*/
(2.1ms) SELECT DISTINCT "expanded_environment_name" FROM "ci_builds" INNER JOIN "ci_builds_metadata" ON "ci_builds_metadata"."build_id" = "ci_builds"."id" WHERE "ci_builds"."type" = 'Ci::Build' AND ("ci_builds"."retried" = FALSE OR "ci_builds"."retried" IS NULL) AND "ci_builds"."commit_id" IN (WITH RECURSIVE "base_and_descendants" AS ((SELECT "ci_pipelines".* FROM "ci_pipelines" WHERE "ci_pipelines"."id" = 95)
UNION
(SELECT "ci_pipelines".* FROM "ci_pipelines", "base_and_descendants", "ci_sources_pipelines" WHERE "ci_sources_pipelines"."pipeline_id" = "ci_pipelines"."id" AND "ci_sources_pipelines"."source_pipeline_id" = "base_and_descendants"."id" AND "ci_sources_pipelines"."source_project_id" = "ci_sources_pipelines"."project_id")) SELECT id FROM "base_and_descendants" AS "ci_pipelines") AND "ci_builds_metadata"."expanded_environment_name" IS NOT NULL LIMIT 100 /*application:console,db_config_name:ci,line:/devkitkat/services/rails/cache/ruby/2.7.0/gems/marginalia-1.10.0/lib/marginalia/comment.rb:25:in `block in construct_comment'*/
Project Load (0.6ms) SELECT "projects"."id", "projects"."name", "projects"."path", "projects"."description", "projects"."created_at", "projects"."updated_at", "projects"."creator_id", "projects"."namespace_id", "projects"."last_activity_at", "projects"."import_url", "projects"."visibility_level", "projects"."archived", "projects"."avatar", "projects"."merge_requests_template", "projects"."star_count", "projects"."merge_requests_rebase_enabled", "projects"."import_type", "projects"."import_source", "projects"."approvals_before_merge", "projects"."reset_approvals_on_push", "projects"."merge_requests_ff_only_enabled", "projects"."issues_template", "projects"."mirror", "projects"."mirror_user_id", "projects"."shared_runners_enabled", "projects"."runners_token", "projects"."build_coverage_regex", "projects"."build_allow_git_fetch", "projects"."build_timeout", "projects"."mirror_trigger_builds", "projects"."pending_delete", "projects"."public_builds", "projects"."last_repository_check_failed", "projects"."last_repository_check_at", "projects"."only_allow_merge_if_pipeline_succeeds", "projects"."has_external_issue_tracker", "projects"."repository_storage", "projects"."repository_read_only", "projects"."request_access_enabled", "projects"."has_external_wiki", "projects"."ci_config_path", "projects"."lfs_enabled", "projects"."description_html", "projects"."only_allow_merge_if_all_discussions_are_resolved", "projects"."repository_size_limit", "projects"."printing_merge_request_link_enabled", "projects"."auto_cancel_pending_pipelines", "projects"."service_desk_enabled", "projects"."cached_markdown_version", "projects"."delete_error", "projects"."last_repository_updated_at", "projects"."disable_overriding_approvers_per_merge_request", "projects"."storage_version", "projects"."resolve_outdated_diff_discussions", "projects"."remote_mirror_available_overridden", "projects"."only_mirror_protected_branches", "projects"."pull_mirror_available_overridden", "projects"."jobs_cache_index", "projects"."external_authorization_classification_label", "projects"."mirror_overwrites_diverged_branches", "projects"."pages_https_only", "projects"."external_webhook_token", "projects"."packages_enabled", "projects"."merge_requests_author_approval", "projects"."pool_repository_id", "projects"."runners_token_encrypted", "projects"."bfg_object_map", "projects"."detected_repository_languages", "projects"."merge_requests_disable_committers_approval", "projects"."require_password_to_approve", "projects"."emails_disabled", "projects"."max_pages_size", "projects"."max_artifacts_size", "projects"."remove_source_branch_after_merge", "projects"."marked_for_deletion_at", "projects"."marked_for_deletion_by_user_id", "projects"."autoclose_referenced_issues", "projects"."suggestion_commit_message", "projects"."project_namespace_id", "projects"."hidden" FROM "projects" WHERE "projects"."id" = 21 LIMIT 1 /*application:console,db_config_name:main,line:/devkitkat/services/rails/cache/ruby/2.7.0/gems/marginalia-1.10.0/lib/marginalia/comment.rb:25:in `block in construct_comment'*/
Environment Load (1.2ms) SELECT "environments".* FROM "environments" WHERE "environments"."project_id" = 21 AND "environments"."name" IN ('dev', 'review/feature-1') AND (EXISTS (SELECT 1 FROM "deployments" WHERE (deployments.environment_id = environments.id) AND "deployments"."sha" = 'a429fe6bcb00f017f28b1dc4b6fc1a056450dc8c')) /*application:console,db_config_name:main,line:/devkitkat/services/rails/cache/ruby/2.7.0/gems/marginalia-1.10.0/lib/marginalia/comment.rb:25:in `block in construct_comment'*/
=> [#<Environment:0x000055d0a7fc3848
id: 35,
project_id: 21,
name: "review/feature-1",
created_at: Thu, 31 Mar 2022 05:53:30.290572000 UTC +00:00,
updated_at: Thu, 31 Mar 2022 05:53:30.290572000 UTC +00:00,
external_url: nil,
environment_type: "review",
state: "available",
slug: "review-feature-1-1q251u",
auto_stop_at: nil,
auto_delete_at: nil,
tier: "development">]
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Edited by Shinya Maeda