diff options
Diffstat (limited to 'app')
-rw-r--r-- | app/controllers/projects/merge_requests/diffs_controller.rb | 5 | ||||
-rw-r--r-- | app/models/concerns/issuable.rb | 1 | ||||
-rw-r--r-- | app/models/concerns/mentionable.rb | 17 | ||||
-rw-r--r-- | app/models/deployment.rb | 1 | ||||
-rw-r--r-- | app/models/environment.rb | 39 | ||||
-rw-r--r-- | app/models/issue.rb | 2 | ||||
-rw-r--r-- | app/models/merge_request.rb | 6 | ||||
-rw-r--r-- | app/models/note.rb | 5 | ||||
-rw-r--r-- | app/models/snippet.rb | 4 | ||||
-rw-r--r-- | app/serializers/merge_request_diff_entity.rb | 8 | ||||
-rw-r--r-- | app/services/ci/stop_environments_service.rb | 16 | ||||
-rw-r--r-- | app/services/environments/auto_stop_service.rb | 38 | ||||
-rw-r--r-- | app/services/issuable_base_service.rb | 4 | ||||
-rw-r--r-- | app/services/notes/create_service.rb | 4 | ||||
-rw-r--r-- | app/services/notes/update_service.rb | 2 | ||||
-rw-r--r-- | app/services/snippets/create_service.rb | 2 | ||||
-rw-r--r-- | app/services/snippets/update_service.rb | 2 | ||||
-rw-r--r-- | app/views/clusters/clusters/gcp/_form.html.haml | 2 | ||||
-rw-r--r-- | app/views/clusters/clusters/show.html.haml | 2 | ||||
-rw-r--r-- | app/workers/all_queues.yml | 6 | ||||
-rw-r--r-- | app/workers/environments/auto_stop_cron_worker.rb | 16 |
21 files changed, 166 insertions, 16 deletions
diff --git a/app/controllers/projects/merge_requests/diffs_controller.rb b/app/controllers/projects/merge_requests/diffs_controller.rb index 72e76f56540..872497992e1 100644 --- a/app/controllers/projects/merge_requests/diffs_controller.rb +++ b/app/controllers/projects/merge_requests/diffs_controller.rb @@ -111,6 +111,11 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic end end + if Gitlab::Utils.to_boolean(params[:diff_head]) && @merge_request.diffable_merge_ref? + return CompareService.new(@project, @merge_request.merge_ref_head.sha) + .execute(@project, @merge_request.target_branch) + end + if @start_sha @merge_request_diff.compare_with(@start_sha) else diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index f3e03ed22d7..78d815e5858 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -91,6 +91,7 @@ module Issuable validate :description_max_length_for_new_records_is_valid, on: :update before_validation :truncate_description_on_import! + after_save :store_mentions!, if: :any_mentionable_attributes_changed? scope :authored, ->(user) { where(author_id: user) } scope :recent, -> { reorder(id: :desc) } diff --git a/app/models/concerns/mentionable.rb b/app/models/concerns/mentionable.rb index b43b91699ab..d157404f7bc 100644 --- a/app/models/concerns/mentionable.rb +++ b/app/models/concerns/mentionable.rb @@ -99,18 +99,23 @@ module Mentionable # threw the `ActiveRecord::RecordNotUnique` exception in first place. self.class.safe_ensure_unique(retries: 1) do user_mention = model_user_mention + + # this may happen due to notes polymorphism, so noteable_id may point to a record that no longer exists + # as we cannot have FK on noteable_id + break if user_mention.blank? + user_mention.mentioned_users_ids = references[:mentioned_users_ids] user_mention.mentioned_groups_ids = references[:mentioned_groups_ids] user_mention.mentioned_projects_ids = references[:mentioned_projects_ids] if user_mention.has_mentions? user_mention.save! - elsif user_mention.persisted? + else user_mention.destroy! end - - true end + + true end def referenced_users @@ -218,6 +223,12 @@ module Mentionable source.select { |key, val| mentionable.include?(key) } end + def any_mentionable_attributes_changed? + self.class.mentionable_attrs.any? do |attr| + saved_changes.key?(attr.first) + end + end + # Determine whether or not a cross-reference Note has already been created between this Mentionable and # the specified target. def cross_reference_exists?(target) diff --git a/app/models/deployment.rb b/app/models/deployment.rb index 0c679a3075b..5450357fe1b 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -39,6 +39,7 @@ class Deployment < ApplicationRecord scope :for_status, -> (status) { where(status: status) } scope :visible, -> { where(status: %i[running success failed canceled]) } + scope :stoppable, -> { where.not(on_stop: nil).where.not(deployable_id: nil).success } state_machine :status, initial: :created do event :run do diff --git a/app/models/environment.rb b/app/models/environment.rb index 973f1243e6b..4635b05fcc7 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -61,6 +61,7 @@ class Environment < ApplicationRecord scope :in_review_folder, -> { where(environment_type: "review") } scope :for_name, -> (name) { where(name: name) } scope :preload_cluster, -> { preload(last_deployment: :cluster) } + scope :auto_stoppable, -> (limit) { available.where('auto_stop_at < ?', Time.zone.now).limit(limit) } ## # Search environments which have names like the given query. @@ -107,6 +108,44 @@ class Environment < ApplicationRecord find_or_create_by(name: name) end + class << self + ## + # This method returns stop actions (jobs) for multiple environments within one + # query. It's useful to avoid N+1 problem. + # + # NOTE: The count of environments should be small~medium (e.g. < 5000) + def stop_actions + cte = cte_for_deployments_with_stop_action + ci_builds = Ci::Build.arel_table + + inner_join_stop_actions = ci_builds.join(cte.table).on( + ci_builds[:project_id].eq(cte.table[:project_id]) + .and(ci_builds[:ref].eq(cte.table[:ref])) + .and(ci_builds[:name].eq(cte.table[:on_stop])) + ).join_sources + + pipeline_ids = ci_builds.join(cte.table).on( + ci_builds[:id].eq(cte.table[:deployable_id]) + ).project(:commit_id) + + Ci::Build.joins(inner_join_stop_actions) + .with(cte.to_arel) + .where(ci_builds[:commit_id].in(pipeline_ids)) + .where(status: HasStatus::BLOCKED_STATUS) + .preload_project_and_pipeline_project + .preload(:user, :metadata, :deployment) + end + + private + + def cte_for_deployments_with_stop_action + Gitlab::SQL::CTE.new(:deployments_with_stop_action, + Deployment.where(environment_id: select(:id)) + .distinct_on_environment + .stoppable) + end + end + def clear_prometheus_reactive_cache!(query_name) cluster_prometheus_adapter&.clear_prometheus_reactive_cache!(query_name, self) end diff --git a/app/models/issue.rb b/app/models/issue.rb index 684108b1a7a..d01aa78a2c1 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -45,7 +45,7 @@ class Issue < ApplicationRecord has_many :issue_assignees has_many :assignees, class_name: "User", through: :issue_assignees has_many :zoom_meetings - has_many :user_mentions, class_name: "IssueUserMention" + has_many :user_mentions, class_name: "IssueUserMention", dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent has_one :sentry_issue accepts_nested_attributes_for :sentry_issue diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index b8f51b58fd3..14aa6ac066e 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -77,7 +77,7 @@ class MergeRequest < ApplicationRecord has_many :merge_request_assignees has_many :assignees, class_name: "User", through: :merge_request_assignees - has_many :user_mentions, class_name: "MergeRequestUserMention" + has_many :user_mentions, class_name: "MergeRequestUserMention", dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent has_many :deployment_merge_requests @@ -840,6 +840,10 @@ class MergeRequest < ApplicationRecord end # rubocop: enable CodeReuse/ServiceClass + def diffable_merge_ref? + Feature.enabled?(:diff_compare_with_head, target_project) && can_be_merged? && merge_ref_head.present? + end + # Returns boolean indicating the merge_status should be rechecked in order to # switch to either can_be_merged or cannot_be_merged. def recheck_merge_status? diff --git a/app/models/note.rb b/app/models/note.rb index 8af650e27aa..97e84bb79f6 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -157,6 +157,7 @@ class Note < ApplicationRecord after_save :expire_etag_cache, unless: :importing? after_save :touch_noteable, unless: :importing? after_destroy :expire_etag_cache + after_save :store_mentions!, if: :any_mentionable_attributes_changed? class << self def model_name @@ -498,6 +499,8 @@ class Note < ApplicationRecord end def user_mentions + return Note.none unless noteable.present? + noteable.user_mentions.where(note: self) end @@ -506,6 +509,8 @@ class Note < ApplicationRecord # Using this method followed by a call to `save` may result in ActiveRecord::RecordNotUnique exception # in a multithreaded environment. Make sure to use it within a `safe_ensure_unique` block. def model_user_mention + return if user_mentions.is_a?(ActiveRecord::NullRelation) + user_mentions.first_or_initialize end diff --git a/app/models/snippet.rb b/app/models/snippet.rb index e2b72dfde7a..4ba8e6a94e6 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -41,7 +41,7 @@ class Snippet < ApplicationRecord belongs_to :project has_many :notes, as: :noteable, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent - has_many :user_mentions, class_name: "SnippetUserMention" + has_many :user_mentions, class_name: "SnippetUserMention", dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent has_one :snippet_repository, inverse_of: :snippet delegate :name, :email, to: :author, prefix: true, allow_nil: true @@ -66,6 +66,8 @@ class Snippet < ApplicationRecord validates :visibility_level, inclusion: { in: Gitlab::VisibilityLevel.values } + after_save :store_mentions!, if: :any_mentionable_attributes_changed? + # Scopes scope :are_internal, -> { where(visibility_level: Snippet::INTERNAL) } scope :are_private, -> { where(visibility_level: Snippet::PRIVATE) } diff --git a/app/serializers/merge_request_diff_entity.rb b/app/serializers/merge_request_diff_entity.rb index 5c79b165ee9..aa0ac7d2a7e 100644 --- a/app/serializers/merge_request_diff_entity.rb +++ b/app/serializers/merge_request_diff_entity.rb @@ -34,6 +34,14 @@ class MergeRequestDiffEntity < Grape::Entity merge_request_version_path(project, merge_request, merge_request_diff) end + expose :head_version_path do |merge_request_diff| + project = merge_request.target_project + + next unless project && merge_request.diffable_merge_ref? + + diffs_project_merge_request_path(project, merge_request, diff_head: true) + end + expose :version_path do |merge_request_diff| start_sha = options[:start_sha] project = merge_request.target_project diff --git a/app/services/ci/stop_environments_service.rb b/app/services/ci/stop_environments_service.rb index d9a800791f2..14ef744ada1 100644 --- a/app/services/ci/stop_environments_service.rb +++ b/app/services/ci/stop_environments_service.rb @@ -16,6 +16,22 @@ module Ci merge_request.environments.each { |environment| stop(environment) } end + ## + # This method is for stopping multiple environments in a batch style. + # The maximum acceptable count of environments is roughly 5000. Please + # apply acceptable `LIMIT` clause to the `environments` relation. + def self.execute_in_batch(environments) + stop_actions = environments.stop_actions.load + + environments.update_all(auto_stop_at: nil, state: 'stopped') + + stop_actions.each do |stop_action| + stop_action.play(stop_action.user) + rescue => e + Gitlab::ErrorTracking.track_error(e, deployable_id: stop_action.id) + end + end + private def environments diff --git a/app/services/environments/auto_stop_service.rb b/app/services/environments/auto_stop_service.rb new file mode 100644 index 00000000000..6eef8138493 --- /dev/null +++ b/app/services/environments/auto_stop_service.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module Environments + class AutoStopService + include ::Gitlab::ExclusiveLeaseHelpers + include ::Gitlab::LoopHelpers + + BATCH_SIZE = 100 + LOOP_TIMEOUT = 45.minutes + LOOP_LIMIT = 1000 + EXCLUSIVE_LOCK_KEY = 'environments:auto_stop:lock' + LOCK_TIMEOUT = 50.minutes + + ## + # Stop expired environments on GitLab instance + # + # This auto stop process cannot run for more than 45 minutes. This is for + # preventing multiple `AutoStopCronWorker` CRON jobs run concurrently, + # which is scheduled at every hour. + def execute + in_lock(EXCLUSIVE_LOCK_KEY, ttl: LOCK_TIMEOUT, retries: 1) do + loop_until(timeout: LOOP_TIMEOUT, limit: LOOP_LIMIT) do + stop_in_batch + end + end + end + + private + + def stop_in_batch + environments = Environment.auto_stoppable(BATCH_SIZE) + + return false unless environments.exists? && Feature.enabled?(:auto_stop_environments) + + Ci::StopEnvironmentsService.execute_in_batch(environments) + end + end +end diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index b59bb803778..830afbf4a43 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -168,7 +168,7 @@ class IssuableBaseService < BaseService before_create(issuable) issuable_saved = issuable.with_transaction_returning_status do - issuable.save && issuable.store_mentions! + issuable.save end if issuable_saved @@ -233,7 +233,7 @@ class IssuableBaseService < BaseService ensure_milestone_available(issuable) issuable_saved = issuable.with_transaction_returning_status do - issuable.save(touch: should_touch) && issuable.store_mentions! + issuable.save(touch: should_touch) end if issuable_saved diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index 50dc98b88e9..4a0d85038ee 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -2,7 +2,6 @@ module Notes class CreateService < ::Notes::BaseService - # rubocop:disable Metrics/CyclomaticComplexity def execute note = Notes::BuildService.new(project, current_user, params.except(:merge_request_diff_head_sha)).execute @@ -34,7 +33,7 @@ module Notes end note_saved = note.with_transaction_returning_status do - !only_commands && note.save && note.store_mentions! + !only_commands && note.save end if note_saved @@ -67,7 +66,6 @@ module Notes note end - # rubocop:enable Metrics/CyclomaticComplexity private diff --git a/app/services/notes/update_service.rb b/app/services/notes/update_service.rb index c8b0dc30209..3070e7b0e53 100644 --- a/app/services/notes/update_service.rb +++ b/app/services/notes/update_service.rb @@ -10,7 +10,7 @@ module Notes note.assign_attributes(params.merge(updated_by: current_user)) note.with_transaction_returning_status do - note.save && note.store_mentions! + note.save end only_commands = false diff --git a/app/services/snippets/create_service.rb b/app/services/snippets/create_service.rb index 250e99c466a..51860adca77 100644 --- a/app/services/snippets/create_service.rb +++ b/app/services/snippets/create_service.rb @@ -24,7 +24,7 @@ module Snippets spam_check(snippet, current_user) snippet_saved = snippet.with_transaction_returning_status do - snippet.save && snippet.store_mentions! + snippet.save end if snippet_saved diff --git a/app/services/snippets/update_service.rb b/app/services/snippets/update_service.rb index 8d2c8cac148..c0c0aec2050 100644 --- a/app/services/snippets/update_service.rb +++ b/app/services/snippets/update_service.rb @@ -21,7 +21,7 @@ module Snippets spam_check(snippet, current_user) snippet_saved = snippet.with_transaction_returning_status do - snippet.save && snippet.store_mentions! + snippet.save end if snippet_saved diff --git a/app/views/clusters/clusters/gcp/_form.html.haml b/app/views/clusters/clusters/gcp/_form.html.haml index 66da76d7a43..e83bf61ab9b 100644 --- a/app/views/clusters/clusters/gcp/_form.html.haml +++ b/app/views/clusters/clusters/gcp/_form.html.haml @@ -70,7 +70,7 @@ label_class: 'label-bold' } .form-text.text-muted = s_('ClusterIntegration|Uses the Cloud Run, Istio, and HTTP Load Balancing addons for this cluster.') - = link_to _('More information'), help_page_path('user/project/clusters/index.md', anchor: 'cloud-run-on-gke'), target: '_blank' + = link_to _('More information'), help_page_path('user/project/clusters/add_remove_clusters.md', anchor: 'cloud-run-for-anthos'), target: '_blank' .form-group = field.check_box :managed, { label: s_('ClusterIntegration|GitLab-managed cluster'), diff --git a/app/views/clusters/clusters/show.html.haml b/app/views/clusters/clusters/show.html.haml index 59ce28d2d26..e1f011a3225 100644 --- a/app/views/clusters/clusters/show.html.haml +++ b/app/views/clusters/clusters/show.html.haml @@ -34,7 +34,7 @@ environments_help_path: help_page_path('ci/environments', anchor: 'defining-environments'), clusters_help_path: help_page_path('user/project/clusters/index.md', anchor: 'deploying-to-a-kubernetes-cluster'), deploy_boards_help_path: help_page_path('user/project/deploy_boards.html', anchor: 'enabling-deploy-boards'), - cloud_run_help_path: help_page_path('user/project/clusters/index.md', anchor: 'cloud-run-on-gke'), + cloud_run_help_path: help_page_path('user/project/clusters/add_remove_clusters.md', anchor: 'cloud-run-for-anthos'), manage_prometheus_path: manage_prometheus_path, cluster_id: @cluster.id } } diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index 3bf46675ad5..5ff1a331b09 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -75,6 +75,12 @@ :latency_sensitive: :resource_boundary: :unknown :weight: 1 +- :name: cronjob:environments_auto_stop_cron + :feature_category: :continuous_delivery + :has_external_dependencies: + :latency_sensitive: + :resource_boundary: :unknown + :weight: 1 - :name: cronjob:expire_build_artifacts :feature_category: :continuous_integration :has_external_dependencies: diff --git a/app/workers/environments/auto_stop_cron_worker.rb b/app/workers/environments/auto_stop_cron_worker.rb new file mode 100644 index 00000000000..8fcda35b414 --- /dev/null +++ b/app/workers/environments/auto_stop_cron_worker.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module Environments + class AutoStopCronWorker + include ApplicationWorker + include CronjobQueue # rubocop:disable Scalability/CronWorkerContext + + feature_category :continuous_delivery + + def perform + return unless Feature.enabled?(:auto_stop_environments) + + AutoStopService.new.execute + end + end +end |