Skip to content

Fix environments are stopped incorrectly in merge requests

Shinya Maeda requested to merge fix-regression-environment-mr-pipelines into master

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.

Edited by Shinya Maeda

Merge request reports