From 949cc27ac4fc0d20fc1428c3b13ba9c66abcb55f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Thu, 8 Nov 2018 13:52:19 +0100 Subject: Strictly require a pipeline to merge --- app/models/merge_request.rb | 1 - 1 file changed, 1 deletion(-) (limited to 'app/models') diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index df5678ec2f1..92add079a02 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -966,7 +966,6 @@ class MergeRequest < ActiveRecord::Base def mergeable_ci_state? return true unless project.only_allow_merge_if_pipeline_succeeds? - return true unless head_pipeline actual_head_pipeline&.success? || actual_head_pipeline&.skipped? end -- cgit v1.2.1 From 4ee8bd11fa317a99dc0f99ea8d33271e4c9d1a47 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Thu, 15 Nov 2018 16:55:51 +0100 Subject: Backport ServiceHook#execute from EE In EE this method takes an additional argument that specifies the name of the hook to trigger. There is no particular reason to not backport this to CE, since by default the behaviour remains the same. By backporting this code we remove the need for prepending ServiceHook with a module in EE. --- app/models/hooks/service_hook.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'app/models') diff --git a/app/models/hooks/service_hook.rb b/app/models/hooks/service_hook.rb index 7d9f6d89d44..8f305dd7c22 100644 --- a/app/models/hooks/service_hook.rb +++ b/app/models/hooks/service_hook.rb @@ -5,8 +5,8 @@ class ServiceHook < WebHook validates :service, presence: true # rubocop: disable CodeReuse/ServiceClass - def execute(data) - WebHookService.new(self, data, 'service_hook').execute + def execute(data, hook_name = 'service_hook') + WebHookService.new(self, data, hook_name).execute end # rubocop: enable CodeReuse/ServiceClass end -- cgit v1.2.1 From 590ff80f839179c1e178e941cd57946e9bb91b79 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Tue, 13 Nov 2018 17:29:14 +0100 Subject: Refactor how a few ActiveRecord enums are defined In a few models we define ActiveRecord enums that are redefined in EE using the following pattern: enum :some_enum, { ... }.merge(EE_ENUM_VALUES) This particular approach is problematic to deal with, because it requires that we `prepend` and EE module _before_ defining the enum. This typically translates to the `prepend` being the first line in the model in EE, but this can easily lead to merge conflicts when developers add more `include` and/or `prepend` lines. As part of https://gitlab.com/gitlab-org/gitlab-ee/issues/8244 and https://gitlab.com/gitlab-org/gitlab-ee/issues/8241 we are moving `prepend` to the last line in a file, reducing the chances of running into merge conflicts. This poses a bit of a problem with the pattern above, because this pattern does not allow us to move the `prepend` further down a file. To resolve this problem, we simply move the Hash value of the enum to a separate class method. This method is defined in a separate module where necessary, allowing us to use it like so: enum :failure_reasons, ::SomeModelEnums.failure_reasons The method in turn is defined in a very straightforward manner: module SomeModelEnums def self.failure_reasons { ... } end end This makes it easy for EE to add values without requiring the `prepend` to be placed before the `enum` is defined. For more information, see the following issues and merge requests: * https://gitlab.com/gitlab-org/gitlab-ee/issues/8244 * https://gitlab.com/gitlab-org/gitlab-ee/issues/8241 * https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/8424 --- app/models/ci/pipeline.rb | 19 ++++++------------- app/models/ci/pipeline_enums.rb | 28 ++++++++++++++++++++++++++++ app/models/commit_status.rb | 15 +++------------ app/models/commit_status_enums.rb | 20 ++++++++++++++++++++ app/models/user_callout.rb | 8 +++----- app/models/user_callout_enums.rb | 16 ++++++++++++++++ 6 files changed, 76 insertions(+), 30 deletions(-) create mode 100644 app/models/ci/pipeline_enums.rb create mode 100644 app/models/commit_status_enums.rb create mode 100644 app/models/user_callout_enums.rb (limited to 'app/models') diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 56010e899a4..9512ba42f67 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -58,15 +58,9 @@ module Ci after_create :keep_around_commits, unless: :importing? - enum_with_nil source: { - unknown: nil, - push: 1, - web: 2, - trigger: 3, - schedule: 4, - api: 5, - external: 6 - } + # We use `Ci::PipelineEnums.sources` here so that EE can more easily extend + # this `Hash` with new values. + enum_with_nil source: ::Ci::PipelineEnums.sources enum_with_nil config_source: { unknown_source: nil, @@ -74,10 +68,9 @@ module Ci auto_devops_source: 2 } - enum failure_reason: { - unknown_failure: 0, - config_error: 1 - } + # We use `Ci::PipelineEnums.failure_reasons` here so that EE can more easily + # extend this `Hash` with new values. + enum failure_reason: ::Ci::PipelineEnums.failure_reasons state_machine :status, initial: :created do event :enqueue do diff --git a/app/models/ci/pipeline_enums.rb b/app/models/ci/pipeline_enums.rb new file mode 100644 index 00000000000..8d8d16e2ec1 --- /dev/null +++ b/app/models/ci/pipeline_enums.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +module Ci + module PipelineEnums + # Returns the `Hash` to use for creating the `failure_reason` enum for + # `Ci::Pipeline`. + def self.failure_reasons + { + unknown_failure: 0, + config_error: 1 + } + end + + # Returns the `Hash` to use for creating the `sources` enum for + # `Ci::Pipeline`. + def self.sources + { + unknown: nil, + push: 1, + web: 2, + trigger: 3, + schedule: 4, + api: 5, + external: 6 + } + end + end +end diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index 755f8bd4d06..0f50bd39131 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -42,18 +42,9 @@ class CommitStatus < ActiveRecord::Base scope :retried_ordered, -> { retried.ordered.includes(project: :namespace) } scope :after_stage, -> (index) { where('stage_idx > ?', index) } - enum_with_nil failure_reason: { - unknown_failure: nil, - script_failure: 1, - api_failure: 2, - stuck_or_timeout_failure: 3, - runner_system_failure: 4, - missing_dependency_failure: 5, - runner_unsupported: 6, - stale_schedule: 7, - job_execution_timeout: 8, - archived_failure: 9 - } + # We use `CommitStatusEnums.failure_reasons` here so that EE can more easily + # extend this `Hash` with new values. + enum_with_nil failure_reason: ::CommitStatusEnums.failure_reasons ## # We still create some CommitStatuses outside of CreatePipelineService. diff --git a/app/models/commit_status_enums.rb b/app/models/commit_status_enums.rb new file mode 100644 index 00000000000..152105d9429 --- /dev/null +++ b/app/models/commit_status_enums.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +module CommitStatusEnums + # Returns the Hash to use for creating the `failure_reason` enum for + # `CommitStatus`. + def self.failure_reasons + { + unknown_failure: nil, + script_failure: 1, + api_failure: 2, + stuck_or_timeout_failure: 3, + runner_system_failure: 4, + missing_dependency_failure: 5, + runner_unsupported: 6, + stale_schedule: 7, + job_execution_timeout: 8, + archived_failure: 9 + } + end +end diff --git a/app/models/user_callout.rb b/app/models/user_callout.rb index 1cd05cf3aac..76e7bc06b4e 100644 --- a/app/models/user_callout.rb +++ b/app/models/user_callout.rb @@ -3,11 +3,9 @@ class UserCallout < ActiveRecord::Base belongs_to :user - enum feature_name: { - gke_cluster_integration: 1, - gcp_signup_offer: 2, - cluster_security_warning: 3 - } + # We use `UserCalloutEnums.feature_names` here so that EE can more easily + # extend this `Hash` with new values. + enum feature_name: ::UserCalloutEnums.feature_names validates :user, presence: true validates :feature_name, diff --git a/app/models/user_callout_enums.rb b/app/models/user_callout_enums.rb new file mode 100644 index 00000000000..b9373ae6166 --- /dev/null +++ b/app/models/user_callout_enums.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module UserCalloutEnums + # Returns the `Hash` to use for the `feature_name` enum in the `UserCallout` + # model. + # + # This method is separate from the `UserCallout` model so that it can be + # extended by EE. + def self.feature_names + { + gke_cluster_integration: 1, + gcp_signup_offer: 2, + cluster_security_warning: 3 + } + end +end -- cgit v1.2.1 From fe1469e12f7a1895cbf534f9ab17fd32af0e954c Mon Sep 17 00:00:00 2001 From: Dylan Griffith Date: Wed, 14 Nov 2018 12:38:08 +0000 Subject: Upgrade helm to 2.11.0 and upgrade on every install --- app/models/clusters/concerns/application_status.rb | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'app/models') diff --git a/app/models/clusters/concerns/application_status.rb b/app/models/clusters/concerns/application_status.rb index 93bdf9c223d..712dc901795 100644 --- a/app/models/clusters/concerns/application_status.rb +++ b/app/models/clusters/concerns/application_status.rb @@ -64,6 +64,10 @@ module Clusters status_reason = transition.args.first app_status.status_reason = status_reason if status_reason end + + before_transition any => [:installed, :updated] do |app_status, _| + app_status.cluster.application_helm.update!(version: Gitlab::Kubernetes::Helm::HELM_VERSION) + end end end -- cgit v1.2.1 From a71b3f6a7c13fdc0978a1e9d0151fe15399b8b59 Mon Sep 17 00:00:00 2001 From: Dylan Griffith Date: Thu, 15 Nov 2018 14:13:32 +0000 Subject: Extract Helm::ClientCommand for shared commands --- app/models/clusters/concerns/application_status.rb | 3 +++ 1 file changed, 3 insertions(+) (limited to 'app/models') diff --git a/app/models/clusters/concerns/application_status.rb b/app/models/clusters/concerns/application_status.rb index 712dc901795..0e74cce29b7 100644 --- a/app/models/clusters/concerns/application_status.rb +++ b/app/models/clusters/concerns/application_status.rb @@ -66,6 +66,9 @@ module Clusters end before_transition any => [:installed, :updated] do |app_status, _| + # When installing any application we are also performing an update + # of tiller (see Gitlab::Kubernetes::Helm::ClientCommand) so + # therefore we need to reflect that in the database. app_status.cluster.application_helm.update!(version: Gitlab::Kubernetes::Helm::HELM_VERSION) end end -- cgit v1.2.1