From 82eab00e02e7698cd7557164891095dc590a627d Mon Sep 17 00:00:00 2001 From: Shinya Maeda Date: Mon, 11 Jan 2021 14:04:43 +0700 Subject: [PATCH] PoC: Support Resource Group For Cross-Project pipelines This commit is PoC --- app/models/ci/bridge.rb | 2 +- app/models/ci/build.rb | 36 -------------- app/models/ci/processable.rb | 49 +++++++++++++++++++ app/models/ci/resource.rb | 6 +-- app/models/ci/resource_group.rb | 10 ++-- ...gn_resource_from_resource_group_service.rb | 4 +- lib/gitlab/ci/config/entry/job.rb | 6 +-- lib/gitlab/ci/config/entry/processable.rb | 8 +-- lib/gitlab/ci/pipeline/seed/build.rb | 5 +- .../ci/pipeline/seed/build/resource_group.rb | 10 ++-- lib/gitlab/ci/status/bridge/factory.rb | 1 + .../ci/status/bridge/waiting_for_resource.rb | 12 +++++ 12 files changed, 89 insertions(+), 60 deletions(-) create mode 100644 lib/gitlab/ci/status/bridge/waiting_for_resource.rb diff --git a/app/models/ci/bridge.rb b/app/models/ci/bridge.rb index f1f99771b4077c..645cc3109d71c5 100644 --- a/app/models/ci/bridge.rb +++ b/app/models/ci/bridge.rb @@ -27,7 +27,7 @@ class Bridge < Ci::Processable # rubocop:enable Cop/ActiveRecordSerialize state_machine :status do - after_transition [:created, :manual] => :pending do |bridge| + after_transition [:created, :manual, :waiting_for_resource] => :pending do |bridge| next unless bridge.downstream_project bridge.run_after_commit do diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index efe5789e49a845..553b3d96115730 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -20,7 +20,6 @@ class Build < Ci::Processable belongs_to :runner belongs_to :trigger_request belongs_to :erased_by, class_name: 'User' - belongs_to :resource_group, class_name: 'Ci::ResourceGroup', inverse_of: :builds belongs_to :pipeline, class_name: 'Ci::Pipeline', foreign_key: :commit_id RUNNER_FEATURES = { @@ -38,7 +37,6 @@ class Build < Ci::Processable DEGRADATION_THRESHOLD_VARIABLE_NAME = 'DEGRADATION_THRESHOLD' has_one :deployment, as: :deployable, class_name: 'Deployment' - has_one :resource, class_name: 'Ci::Resource', inverse_of: :build has_one :pending_state, class_name: 'Ci::BuildPendingState', inverse_of: :build has_many :trace_sections, class_name: 'Ci::BuildTraceSection' has_many :trace_chunks, class_name: 'Ci::BuildTraceChunk', foreign_key: :build_id, inverse_of: :build @@ -236,19 +234,16 @@ def with_preloads state_machine :status do event :enqueue do - transition [:created, :skipped, :manual, :scheduled] => :waiting_for_resource, if: :requires_resource? transition [:created, :skipped, :manual, :scheduled] => :preparing, if: :any_unmet_prerequisites? end event :enqueue_scheduled do - transition scheduled: :waiting_for_resource, if: :requires_resource? transition scheduled: :preparing, if: :any_unmet_prerequisites? transition scheduled: :pending end event :enqueue_waiting_for_resource do transition waiting_for_resource: :preparing, if: :any_unmet_prerequisites? - transition waiting_for_resource: :pending end event :enqueue_preparing do @@ -279,23 +274,6 @@ def with_preloads build.scheduled_at = build.options_scheduled_at end - before_transition any => :waiting_for_resource do |build| - build.waiting_for_resource_at = Time.current - end - - before_transition on: :enqueue_waiting_for_resource do |build| - next unless build.requires_resource? - - build.resource_group.assign_resource_to(build) # If false is returned, it stops the transition - end - - after_transition any => :waiting_for_resource do |build| - build.run_after_commit do - Ci::ResourceGroups::AssignResourceFromResourceGroupWorker - .perform_async(build.resource_group_id) - end - end - before_transition on: :enqueue_preparing do |build| !build.any_unmet_prerequisites? # If false is returned, it stops the transition end @@ -328,16 +306,6 @@ def with_preloads end end - after_transition any => ::Ci::Build.completed_statuses do |build| - next unless build.resource_group_id.present? - next unless build.resource_group.release_resource_from(build) - - build.run_after_commit do - Ci::ResourceGroups::AssignResourceFromResourceGroupWorker - .perform_async(build.resource_group_id) - end - end - after_transition any => [:success, :failed, :canceled] do |build| build.run_after_commit do build.run_status_commit_hooks! @@ -505,10 +473,6 @@ def expanded_kubernetes_namespace end end - def requires_resource? - self.resource_group_id.present? - end - def has_environment? environment.present? end diff --git a/app/models/ci/processable.rb b/app/models/ci/processable.rb index 6aaf6ac530ba77..c3fd7f68f24c5f 100644 --- a/app/models/ci/processable.rb +++ b/app/models/ci/processable.rb @@ -4,6 +4,10 @@ module Ci class Processable < ::CommitStatus include Gitlab::Utils::StrongMemoize + has_one :resource, class_name: 'Ci::Resource', foreign_key: 'build_id', inverse_of: :processable + + belongs_to :resource_group, class_name: 'Ci::ResourceGroup', inverse_of: :processables + accepts_nested_attributes_for :needs scope :preload_needs, -> { preload(:needs) } @@ -20,6 +24,47 @@ class Processable < ::CommitStatus where('NOT EXISTS (?)', needs) end + state_machine :status do + event :enqueue do + transition [:created, :skipped, :manual, :scheduled] => :waiting_for_resource, if: :requires_resource? + end + + event :enqueue_scheduled do + transition scheduled: :waiting_for_resource, if: :requires_resource? + end + + event :enqueue_waiting_for_resource do + transition waiting_for_resource: :pending + end + + before_transition any => :waiting_for_resource do |processable| + processable.waiting_for_resource_at = Time.current + end + + before_transition on: :enqueue_waiting_for_resource do |processable| + next unless processable.requires_resource? + + processable.resource_group.assign_resource_to(processable) + end + + after_transition any => :waiting_for_resource do |processable| + processable.run_after_commit do + Ci::ResourceGroups::AssignResourceFromResourceGroupWorker + .perform_async(processable.resource_group_id) + end + end + + after_transition any => ::Ci::Processable.completed_statuses do |processable| + next unless processable.resource_group_id.present? + next unless processable.resource_group.release_resource_from(processable) + + processable.run_after_commit do + Ci::ResourceGroups::AssignResourceFromResourceGroupWorker + .perform_async(processable.resource_group_id) + end + end + end + def self.select_with_aggregated_needs(project) aggregated_needs_names = Ci::BuildNeed .scoped_build @@ -77,6 +122,10 @@ def scoped_variables_hash raise NotImplementedError end + def requires_resource? + self.resource_group_id.present? + end + # Overriding scheduling_type enum's method for nil `scheduling_type`s def scheduling_type_dag? scheduling_type.nil? ? find_legacy_scheduling_type == :dag : super diff --git a/app/models/ci/resource.rb b/app/models/ci/resource.rb index ee5b6546165bb4..419c7448368ddb 100644 --- a/app/models/ci/resource.rb +++ b/app/models/ci/resource.rb @@ -5,9 +5,9 @@ class Resource < ApplicationRecord extend Gitlab::Ci::Model belongs_to :resource_group, class_name: 'Ci::ResourceGroup', inverse_of: :resources - belongs_to :build, class_name: 'Ci::Build', inverse_of: :resource + belongs_to :processable, class_name: 'Ci::Processable', foreign_key: 'build_id', inverse_of: :resource - scope :free, -> { where(build: nil) } - scope :retained_by, -> (build) { where(build: build) } + scope :free, -> { where(build_id: nil) } + scope :retained_by, -> (processable) { where(build_id: processable.id) } end end diff --git a/app/models/ci/resource_group.rb b/app/models/ci/resource_group.rb index eb18f3da0bf2fb..85fbe03e1c9fe9 100644 --- a/app/models/ci/resource_group.rb +++ b/app/models/ci/resource_group.rb @@ -7,7 +7,7 @@ class ResourceGroup < ApplicationRecord belongs_to :project, inverse_of: :resource_groups has_many :resources, class_name: 'Ci::Resource', inverse_of: :resource_group - has_many :builds, class_name: 'Ci::Build', inverse_of: :resource_group + has_many :processables, class_name: 'Ci::Processable', inverse_of: :resource_group validates :key, length: { maximum: 255 }, @@ -19,12 +19,12 @@ class ResourceGroup < ApplicationRecord ## # NOTE: This is concurrency-safe method that the subquery in the `UPDATE` # works as explicit locking. - def assign_resource_to(build) - resources.free.limit(1).update_all(build_id: build.id) > 0 + def assign_resource_to(processable) + resources.free.limit(1).update_all(build_id: processable.id) > 0 end - def release_resource_from(build) - resources.retained_by(build).update_all(build_id: nil) > 0 + def release_resource_from(processable) + resources.retained_by(processable).update_all(build_id: nil) > 0 end private diff --git a/app/services/ci/resource_groups/assign_resource_from_resource_group_service.rb b/app/services/ci/resource_groups/assign_resource_from_resource_group_service.rb index a4bcca8e8b36ee..9e3e6de3928e57 100644 --- a/app/services/ci/resource_groups/assign_resource_from_resource_group_service.rb +++ b/app/services/ci/resource_groups/assign_resource_from_resource_group_service.rb @@ -7,8 +7,8 @@ class AssignResourceFromResourceGroupService < ::BaseService def execute(resource_group) free_resources = resource_group.resources.free.count - resource_group.builds.waiting_for_resource.take(free_resources).each do |build| - build.enqueue_waiting_for_resource + resource_group.processables.waiting_for_resource.take(free_resources).each do |processable| + processable.enqueue_waiting_for_resource end end # rubocop: enable CodeReuse/ActiveRecord diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb index 85e3514499caf4..6eb35bc9e1ec27 100644 --- a/lib/gitlab/ci/config/entry/job.rb +++ b/lib/gitlab/ci/config/entry/job.rb @@ -14,7 +14,7 @@ class Job < ::Gitlab::Config::Entry::Node ALLOWED_KEYS = %i[tags script type image services start_in artifacts cache dependencies before_script after_script environment coverage retry parallel interruptible timeout - resource_group release secrets].freeze + release secrets].freeze REQUIRED_BY_NEEDS = %i[stage].freeze @@ -30,7 +30,6 @@ class Job < ::Gitlab::Config::Entry::Node } validates :dependencies, array_of_strings: true - validates :resource_group, type: String validates :allow_failure, hash_or_boolean: true end @@ -124,7 +123,7 @@ class Job < ::Gitlab::Config::Entry::Node attributes :script, :tags, :when, :dependencies, :needs, :retry, :parallel, :start_in, - :interruptible, :timeout, :resource_group, + :interruptible, :timeout, :release, :allow_failure def self.matching?(name, config) @@ -174,7 +173,6 @@ def value ignore: ignored?, allow_failure_criteria: allow_failure_criteria, needs: needs_defined? ? needs_value : nil, - resource_group: resource_group, scheduling_type: needs_defined? ? :dag : :stage ).compact end diff --git a/lib/gitlab/ci/config/entry/processable.rb b/lib/gitlab/ci/config/entry/processable.rb index 5ef8cfbddb7667..9584d19bdecfd1 100644 --- a/lib/gitlab/ci/config/entry/processable.rb +++ b/lib/gitlab/ci/config/entry/processable.rb @@ -15,7 +15,7 @@ module Processable include ::Gitlab::Config::Entry::Inheritable PROCESSABLE_ALLOWED_KEYS = %i[extends stage only except rules variables - inherit allow_failure when needs].freeze + inherit allow_failure when needs resource_group].freeze included do validations do @@ -32,6 +32,7 @@ module Processable with_options allow_nil: true do validates :extends, array_of_strings_or_string: true validates :rules, array_of_hashes: true + validates :resource_group, type: String end end @@ -64,7 +65,7 @@ module Processable inherit: false, default: {} - attributes :extends, :rules + attributes :extends, :rules, :resource_group end def compose!(deps = nil) @@ -125,7 +126,8 @@ def value rules: rules_value, variables: root_and_job_variables_value, only: only_value, - except: except_value }.compact + except: except_value, + resource_group: resource_group }.compact end def root_and_job_variables_value diff --git a/lib/gitlab/ci/pipeline/seed/build.rb b/lib/gitlab/ci/pipeline/seed/build.rb index 2271915a72b279..fef81b31c64165 100644 --- a/lib/gitlab/ci/pipeline/seed/build.rb +++ b/lib/gitlab/ci/pipeline/seed/build.rb @@ -78,8 +78,11 @@ def to_resource else ::Ci::Build.new(attributes).tap do |build| build.assign_attributes(self.class.environment_attributes_for(build)) - build.resource_group = Seed::Build::ResourceGroup.new(build, @resource_group_key).to_resource end + end.tap do |processable| + processable.resource_group = + Seed::Build::ResourceGroup.new(processable, @resource_group_key) + .to_resource end end end diff --git a/lib/gitlab/ci/pipeline/seed/build/resource_group.rb b/lib/gitlab/ci/pipeline/seed/build/resource_group.rb index c0641d9ff0a4ab..794bd06be25ec8 100644 --- a/lib/gitlab/ci/pipeline/seed/build/resource_group.rb +++ b/lib/gitlab/ci/pipeline/seed/build/resource_group.rb @@ -8,17 +8,17 @@ class Build class ResourceGroup < Seed::Base include Gitlab::Utils::StrongMemoize - attr_reader :build, :resource_group_key + attr_reader :processable, :resource_group_key - def initialize(build, resource_group_key) - @build = build + def initialize(processable, resource_group_key) + @processable = processable @resource_group_key = resource_group_key end def to_resource return unless resource_group_key.present? - resource_group = build.project.resource_groups + resource_group = processable.project.resource_groups .safe_find_or_create_by(key: expanded_resource_group_key) resource_group if resource_group.persisted? @@ -28,7 +28,7 @@ def to_resource def expanded_resource_group_key strong_memoize(:expanded_resource_group_key) do - ExpandVariables.expand(resource_group_key, -> { build.simple_variables }) + ExpandVariables.expand(resource_group_key, -> { processable.simple_variables }) end end end diff --git a/lib/gitlab/ci/status/bridge/factory.rb b/lib/gitlab/ci/status/bridge/factory.rb index b9bd66cee71bac..4d5a94a3bebaa1 100644 --- a/lib/gitlab/ci/status/bridge/factory.rb +++ b/lib/gitlab/ci/status/bridge/factory.rb @@ -8,6 +8,7 @@ class Factory < Status::Factory def self.extended_statuses [[Status::Bridge::Failed], [Status::Bridge::Manual], + [Status::Bridge::WaitingForResource], [Status::Bridge::Play], [Status::Bridge::Action]] end diff --git a/lib/gitlab/ci/status/bridge/waiting_for_resource.rb b/lib/gitlab/ci/status/bridge/waiting_for_resource.rb new file mode 100644 index 00000000000000..a063c47fadf9f4 --- /dev/null +++ b/lib/gitlab/ci/status/bridge/waiting_for_resource.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +module Gitlab + module Ci + module Status + module Bridge + class WaitingForResource < Status::Build::WaitingForResource + end + end + end + end +end -- GitLab