diff options
| author | Shinya Maeda <shinya@gitlab.com> | 2018-11-01 14:22:17 +0900 |
|---|---|---|
| committer | Shinya Maeda <shinya@gitlab.com> | 2018-11-02 10:17:47 +0900 |
| commit | 36ac13f345f5ef25725c2236a791a40a3a9e6126 (patch) | |
| tree | 9d951d2dd94578917e96c1de894676b86a11779e | |
| parent | 88224ccea67290e1bfa600379715ab278984ccdf (diff) | |
| download | gitlab-ce-36ac13f345f5ef25725c2236a791a40a3a9e6126.tar.gz | |
Squashed commit of the following:
commit ba9aede922e1643db3f06c56736d46d6d86d356b
Author: Shinya Maeda <shinya@gitlab.com>
Date: Thu Nov 1 14:21:33 2018 +0900
Fix ambiguious factory specification in update deployment service spec
commit 013afb5668cb30dc4ca5b21945c17b341e7ea7f9
Author: Shinya Maeda <shinya@gitlab.com>
Date: Thu Nov 1 14:10:24 2018 +0900
Fix spec
commit 78793670d049e2dfb5fc98177eb4d10f20b9310b
Author: Shinya Maeda <shinya@gitlab.com>
Date: Wed Oct 31 18:26:12 2018 +0900
Fix spec
commit 73d27e87c66698f2e3a817bb8728f02475b7ba4f
Author: Shinya Maeda <shinya@gitlab.com>
Date: Wed Oct 31 16:22:14 2018 +0900
Fix index
commit 8580a226ea68bf5e49b35bfb5f404968bbfaf8e9
Author: Shinya Maeda <shinya@gitlab.com>
Date: Wed Oct 31 15:34:57 2018 +0900
Fix deployment relationships in Ci::Build
commit d6d28b55afd1179200b4f5188e0b53079ff3c1a7
Author: Shinya Maeda <shinya@gitlab.com>
Date: Wed Oct 31 15:27:53 2018 +0900
Fix spec
commit 94eb754e2e1bb9a1fe627f86823f571a8298d27b
Author: Shinya Maeda <shinya@gitlab.com>
Date: Wed Oct 31 14:07:11 2018 +0900
Fix spec
commit 0b30f80bcd08a7a06bdde3378ec1733f865284be
Author: Shinya Maeda <shinya@gitlab.com>
Date: Tue Oct 30 20:15:31 2018 +0900
Fix spec
commit 466bdcdb6af8cdb475c9fa16bd7d1dff23b11e40
Author: Shinya Maeda <shinya@gitlab.com>
Date: Tue Oct 30 19:28:51 2018 +0900
Fix spec
commit a7c3caac99139e70fe3f1f3d14856939fa25c527
Author: Shinya Maeda <shinya@gitlab.com>
Date: Tue Oct 30 17:33:47 2018 +0900
Fix factory
commit cea28ae100532e6711ce1d22676719a94e2da8a0
Author: Shinya Maeda <shinya@gitlab.com>
Date: Tue Oct 30 16:28:18 2018 +0900
Drop leagacy success
commit 3785d685eabc10b6597cf3db67bf08385ccf298a
Author: Shinya Maeda <shinya@gitlab.com>
Date: Tue Oct 30 15:37:28 2018 +0900
Remove unnecessary migration file
commit 0d597fa46eeffdbb9a4afb53005a8183e433c6bf
Author: Shinya Maeda <shinya@gitlab.com>
Date: Tue Oct 30 15:35:53 2018 +0900
Fix schema.rb
commit ec3c2abc6944e09f6410468ae5e356865ec7b02b
Author: Shinya Maeda <shinya@gitlab.com>
Date: Tue Oct 30 15:34:21 2018 +0900
Rename post migration file
commit 0e7281885a84656acf95f0f423732680f8fec076
Author: Shinya Maeda <shinya@gitlab.com>
Date: Tue Oct 30 15:31:01 2018 +0900
Remove include EnumWithNil
commit b3846d59c07e07275126c70361bde7f30810729e
Author: Shinya Maeda <shinya@gitlab.com>
Date: Tue Oct 30 15:05:50 2018 +0900
Decouple action
commit c9f9ba4eae9ca1edc7d8751e1d2e0572cb222d9c
Author: Shinya Maeda <shinya@gitlab.com>
Date: Tue Oct 30 14:23:29 2018 +0900
Remove status mock
commit d95bfea1ca67b3a27a3226a669c2b1266d696682
Author: Shinya Maeda <shinya@gitlab.com>
Date: Tue Oct 30 14:17:14 2018 +0900
Add action
commit 0cec39e0f76c22a18498f46d65ad7226fb30c3f8
Author: Shinya Maeda <shinya@gitlab.com>
Date: Tue Oct 30 13:44:07 2018 +0900
Remove unnecessary line in schema.rb
commit 7b4c5f8e1b00dd8e6aa944352f9d8a9f3ae6f1c7
Author: Shinya Maeda <shinya@gitlab.com>
Date: Mon Oct 29 19:59:41 2018 +0900
Revert build success worker
commit 0c52ffa4a23eea488c187317e8b400369846f399
Author: Shinya Maeda <shinya@gitlab.com>
Date: Mon Oct 29 19:11:47 2018 +0900
Use add_column_with_default properly
commit ba9bae357da5dfd2f6ec05f7f9db9d0b31224f48
Author: Shinya Maeda <shinya@gitlab.com>
Date: Mon Oct 29 18:40:55 2018 +0900
Fix with_status
commit 75dffc97b9c5f6fa73d9d09b125c8f849fa2caae
Author: Shinya Maeda <shinya@gitlab.com>
Date: Mon Oct 29 16:26:56 2018 +0900
Remove unnecessary line in schema.rb
commit 25188ccc52fb29ca63b9205c4d95ffc2e0afadee
Author: Shinya Maeda <shinya@gitlab.com>
Date: Mon Oct 29 16:26:17 2018 +0900
Set default values in regular migration
commit 98ea037fbf39c8d9f0db77fb50e2d08382425158
Author: Shinya Maeda <shinya@gitlab.com>
Date: Fri Oct 26 17:27:49 2018 +0900
Fix static analysis
commit e7d1765f77f9ff9b94a34985a7855bdaab1da675
Author: Shinya Maeda <shinya@gitlab.com>
Date: Fri Oct 26 16:37:10 2018 +0900
Remove empty spec
commit 0033f521ed1eae8117dba231961aa47c068bbcfb
Author: Shinya Maeda <shinya@gitlab.com>
Date: Fri Oct 26 16:34:55 2018 +0900
Simplify spec changes
commit 0be4c6b3ade6d9a8bf28bcd177c66ebd7bb7d20a
Author: Shinya Maeda <shinya@gitlab.com>
Date: Fri Oct 26 16:32:45 2018 +0900
Simplify spec changes
commit a93d25d79df7e25bdf688fc938c712922f9dc4df
Author: Shinya Maeda <shinya@gitlab.com>
Date: Fri Oct 26 16:02:31 2018 +0900
Fix flaky spec
commit 339ad50cf471ca706b29f008ccd2bb881dd5b776
Author: Shinya Maeda <shinya@gitlab.com>
Date: Fri Oct 26 15:06:22 2018 +0900
Rename Deployments Success worker
commit bd69c78085adcb9b0f8ff9b7041ae355953ad7ab
Author: Shinya Maeda <shinya@gitlab.com>
Date: Fri Oct 26 14:43:03 2018 +0900
Fix coding offence
commit 004748b2a9c5236ec13eb01289418f3d6571c92c
Author: Shinya Maeda <shinya@gitlab.com>
Date: Thu Oct 25 20:09:10 2018 +0900
Rename to update deployment service
commit b04a85e761de501f030f3844fd485a2b9e46f7f7
Author: Shinya Maeda <shinya@gitlab.com>
Date: Thu Oct 25 18:46:52 2018 +0900
Add spec for Project
commit 548af23a5a07f0c20b72849d03aa0b98a0b49134
Author: Shinya Maeda <shinya@gitlab.com>
Date: Thu Oct 25 18:43:25 2018 +0900
Fix spec
commit c977e4d3f17194c46a1bf857b473017ce21ef7e9
Author: Shinya Maeda <shinya@gitlab.com>
Date: Thu Oct 25 17:58:07 2018 +0900
Add spec for Environment
commit 73feb9010f8d8093bee4b46e56d30cfef3e8e34a
Author: Shinya Maeda <shinya@gitlab.com>
Date: Thu Oct 25 17:39:24 2018 +0900
Add spec for Deployment model
commit 9a3cfbf766f402571588839375cf311bb9807035
Author: Shinya Maeda <shinya@gitlab.com>
Date: Thu Oct 25 17:18:02 2018 +0900
Fix statis analysis
commit a30d28dbc631a29855883ca89c592a10c012f1d2
Author: Shinya Maeda <shinya@gitlab.com>
Date: Thu Oct 25 17:17:32 2018 +0900
Ignore nil instance
commit fa6fdd89f380e588a6bcf14b1f9aef0d14d3854b
Author: Shinya Maeda <shinya@gitlab.com>
Date: Thu Oct 25 16:20:40 2018 +0900
Add spec for deployable concern
commit aa91186821dc671df2c7a641e37586dd5dfc1008
Author: Shinya Maeda <shinya@gitlab.com>
Date: Thu Oct 25 15:37:23 2018 +0900
Clean up deployable
commit 34d3e18731f7906a3db250b105a64d1db83c2fca
Author: Shinya Maeda <shinya@gitlab.com>
Date: Thu Oct 25 15:13:05 2018 +0900
Fix 17 cycle analytics
commit 8dc9e00408f9b390175e7d5ea743eed4fb9e3f79
Author: Shinya Maeda <shinya@gitlab.com>
Date: Thu Oct 25 13:56:51 2018 +0900
Fix static analysys
commit 5c4175807a537bafc4b889b0a97e8f96f0e483cd
Author: Shinya Maeda <shinya@gitlab.com>
Date: Wed Oct 24 15:05:05 2018 +0900
Skip unnecessary sidekiq worker
commit 9d8b5d423f49cc247c96ce3767d03b4af305809f
Author: Shinya Maeda <shinya@gitlab.com>
Date: Wed Oct 24 14:53:13 2018 +0900
Add changelog
commit c8cabba496722240cadf7c161c80bceb09727cba
Author: Shinya Maeda <shinya@gitlab.com>
Date: Wed Oct 24 14:44:45 2018 +0900
Squashed commit of the following:
commit f7643885ac2329e18d690a4e4f2d7614b732c793
Author: Shinya Maeda <shinya@gitlab.com>
Date: Tue Oct 23 19:38:45 2018 +0900
Fix deployment widget specs
commit 03bd04b5c98b634dff6a0ab4292c150a9031995c
Author: Shinya Maeda <shinya@gitlab.com>
Date: Tue Oct 23 17:50:16 2018 +0900
Fix env status spec
commit 4a49c6502b161a12f0f62d5ec167dff777047dab
Author: Shinya Maeda <shinya@gitlab.com>
Date: Tue Oct 23 17:48:59 2018 +0900
Fix environment spec
commit 4044822887987e20a703990ff20352a532eeb965
Author: Shinya Maeda <shinya@gitlab.com>
Date: Tue Oct 23 17:47:17 2018 +0900
Fix environment spec
commit 9939d44b7eb9da371de74c0f04fed1eb3db37ad3
Author: Shinya Maeda <shinya@gitlab.com>
Date: Tue Oct 23 15:45:43 2018 +0900
Add a new spec for deployment success worker
commit f61c4d3657b5ef13b5da171460da68a6643ad4b5
Author: Shinya Maeda <shinya@gitlab.com>
Date: Tue Oct 23 15:38:11 2018 +0900
Fix cycle analytics helper
commit b6242615e8298fb7fc047c8df8006c25ad717c70
Author: Shinya Maeda <shinya@gitlab.com>
Date: Tue Oct 23 14:41:54 2018 +0900
Fix cycle analysis helper
commit 9a001cb4c4ed6f3b87dc612bdffc60a6b2b0a132
Author: Shinya Maeda <shinya@gitlab.com>
Date: Tue Oct 23 14:37:08 2018 +0900
Ignore coding offence in build success worker's spec
commit 1fb88583025bac8a56172cbd59be04258ea4c5f3
Author: Shinya Maeda <shinya@gitlab.com>
Date: Tue Oct 23 14:33:11 2018 +0900
Added more spec for deployments
commit 1a6ba97ababbf62e8dd0ae0c56d75ab1268fd0ce
Author: Shinya Maeda <shinya@gitlab.com>
Date: Mon Oct 22 19:36:50 2018 +0900
Move after create hookd into success worker
commit 09de5fed5d6f108423779cf9d9e7f1d21f3c1c91
Author: Shinya Maeda <shinya@gitlab.com>
Date: Mon Oct 22 19:30:06 2018 +0900
Fix build spec
commit 73a55cbcabbb1e928eca3e53e8ff75dec178bc90
Author: Shinya Maeda <shinya@gitlab.com>
Date: Mon Oct 22 19:08:43 2018 +0900
Fix update_deployment_metrics_service_spec.rb
commit ee05136a02ae9fa348b4b89b9a69937ebb9697dd
Author: Shinya Maeda <shinya@gitlab.com>
Date: Mon Oct 22 17:32:05 2018 +0900
Remove unnecessary degelate
commit e246ddeebc01a807ccc36fdb484c3e72ad91e680
Author: Shinya Maeda <shinya@gitlab.com>
Date: Mon Oct 22 16:07:39 2018 +0900
Remove unnecessary optimistic locking
commit dcc225c8237b90e3bc8dcc3dc2e3252e0b0be093
Author: Shinya Maeda <shinya@gitlab.com>
Date: Mon Oct 22 16:00:22 2018 +0900
Simplify status replication
commit 13a5fd7afb67ba2712fcaecaea5fedf05f9ad177
Author: Shinya Maeda <shinya@gitlab.com>
Date: Mon Oct 22 15:29:24 2018 +0900
Fix sidekiq queue names
commit dcc796f48d523538e1c91b9cd3e1c7065e5329b1
Author: Shinya Maeda <shinya@gitlab.com>
Date: Mon Oct 22 15:23:55 2018 +0900
Revert success check in update_merge_request_metrics
commit 129ef083d637d4acb8c97a6d9ab96deb2ff6efcd
Author: Shinya Maeda <shinya@gitlab.com>
Date: Mon Oct 22 15:18:31 2018 +0900
Fix queue name of deployment success worker
commit 10fe5a6484f4f02322ce5bb16844fc7b1d565963
Author: Shinya Maeda <shinya@gitlab.com>
Date: Mon Oct 22 15:09:42 2018 +0900
Introduce deployable module
commit d91260bbe105bf46f6c06d9e9593c8c4cd5139cf
Author: Shinya Maeda <shinya@gitlab.com>
Date: Mon Oct 22 14:05:31 2018 +0900
Add database index for successful deployments
commit 74274147263de4b60870065a19935498ce662e30
Author: Shinya Maeda <shinya@gitlab.com>
Date: Mon Oct 22 13:51:59 2018 +0900
Fix invalid state transition
commit ff18463cc847bf3cf5a3e49f3651eedfdf67c7e6
Author: Shinya Maeda <shinya@gitlab.com>
Date: Fri Oct 19 20:05:15 2018 +0900
Fix coding style offence
commit 0202c0f5b631601edab7b359b087b307f5eb7ba3
Author: Shinya Maeda <shinya@gitlab.com>
Date: Fri Oct 19 18:34:07 2018 +0900
Target only successful deployments from other relations
commit 1f2758cb030dec1df5dda30f6bc3e25b6d0841c9
Author: Shinya Maeda <shinya@gitlab.com>
Date: Fri Oct 19 18:21:28 2018 +0900
Add namespace explicitly
commit 3d9227b6e5642cecde88d4edac925125f6474b11
Author: Shinya Maeda <shinya@gitlab.com>
Date: Fri Oct 19 17:42:30 2018 +0900
Fix spec in DeleteInconsistentInternalIdRecords
commit 3e0cc99ff6c5c7188511618228a6ec027752ce69
Author: Shinya Maeda <shinya@gitlab.com>
Date: Fri Oct 19 16:10:16 2018 +0900
Fixed spec
commit 8de09b8bb31f7b9f24ecdf9f2dd8ef358a260263
Author: Shinya Maeda <shinya@gitlab.com>
Date: Fri Oct 19 14:22:35 2018 +0900
Fix create deployment service
commit 31957570b4444492eeb412e765f96a56416c25f3
Author: Shinya Maeda <shinya@gitlab.com>
Date: Thu Oct 18 20:21:26 2018 +0900
Move CreateDeploymentService. Fix Cycle analytics spec and fixture.
commit d2eb433a1bb9710c0d4778c4f34c12b6b64f60e6
Author: Shinya Maeda <shinya@gitlab.com>
Date: Thu Oct 18 20:11:22 2018 +0900
Fix build success worker
commit 25e6cd87138bcdb69de8785ca367e479c8dbcc59
Author: Shinya Maeda <shinya@gitlab.com>
Date: Thu Oct 18 19:49:13 2018 +0900
Fix create deployment service spec
commit d268bf410bf65e86c81eb76d50aa8e145b32d249
Author: Shinya Maeda <shinya@gitlab.com>
Date: Thu Oct 18 19:01:23 2018 +0900
Fix cycle analysys spec's deployment
commit 525ade8aa1e4394ed8a759bb0437e407fbe74a35
Author: Shinya Maeda <shinya@gitlab.com>
Date: Thu Oct 18 18:24:04 2018 +0900
Fix factory to set legacy status by default
commit c6a990821ac0a1ffa49e20e2d78d94b8ce075914
Author: Shinya Maeda <shinya@gitlab.com>
Date: Thu Oct 18 17:25:40 2018 +0900
Remove unnecessary lib from deployment
commit a6107e0e85ac26ee09da3316ebc11de32f067d82
Author: Shinya Maeda <shinya@gitlab.com>
Date: Wed Oct 17 17:38:58 2018 +0900
Fix recursive call
commit 15c5f3b64061a75af3c3039ca7f49b1cc4ff3068
Author: Shinya Maeda <shinya@gitlab.com>
Date: Wed Oct 17 17:30:44 2018 +0900
Add finished_at
commit c8d3d70366f694d78acb7e30d342c7697798b922
Author: Shinya Maeda <shinya@gitlab.com>
Date: Wed Oct 17 15:55:31 2018 +0900
Fix last_deployment methods as it used to return successful deployment always
commit 96bbe8670cece021766fde95fe573cbbe23d1e55
Author: Shinya Maeda <shinya@gitlab.com>
Date: Wed Oct 17 15:49:57 2018 +0900
Redefine statuses
commit c86a9d0bd2ab3e7a00bf61f094a96ee99b76b289
Author: Shinya Maeda <shinya@gitlab.com>
Date: Wed Oct 17 14:50:27 2018 +0900
Fix schema.rb
commit 9ff5f0eaafbc08795018c7bb282b19f6327dee21
Author: Shinya Maeda <shinya@gitlab.com>
Date: Wed Oct 17 14:18:04 2018 +0900
Default status nil to success
commit 5928bd9bb94e1e8908ed1561e01595be84d5f4ec
Author: Shinya Maeda <shinya@gitlab.com>
Date: Tue Oct 16 15:13:48 2018 +0900
Add status to Deployment
51 files changed, 1029 insertions, 386 deletions
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index d73c02ba5d7..15130308e10 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -9,19 +9,18 @@ module Ci include Presentable include Importable include Gitlab::Utils::StrongMemoize + include Deployable belongs_to :project, inverse_of: :builds belongs_to :runner belongs_to :trigger_request belongs_to :erased_by, class_name: 'User' - has_many :deployments, as: :deployable - RUNNER_FEATURES = { upload_multiple_artifacts: -> (build) { build.publishes_artifacts_reports? } }.freeze - has_one :last_deployment, -> { order('deployments.id DESC') }, as: :deployable, class_name: 'Deployment' + has_one :deployment, as: :deployable, class_name: 'Deployment' has_many :trace_sections, class_name: 'Ci::BuildTraceSection' has_many :trace_chunks, class_name: 'Ci::BuildTraceChunk', foreign_key: :build_id @@ -195,6 +194,8 @@ module Ci end after_transition pending: :running do |build| + build.deployment&.run + build.run_after_commit do BuildHooksWorker.perform_async(id) end @@ -207,14 +208,18 @@ module Ci end after_transition any => [:success] do |build| + build.deployment&.succeed + build.run_after_commit do - BuildSuccessWorker.perform_async(id) PagesWorker.perform_async(:deploy, id) if build.pages_generator? end end before_transition any => [:failed] do |build| next unless build.project + + build.deployment&.drop + next if build.retries_max.zero? if build.retries_count < build.retries_max @@ -233,6 +238,10 @@ module Ci after_transition running: any do |build| Ci::BuildRunnerSession.where(build: build).delete_all end + + after_transition any => [:skipped, :canceled] do |build| + build.deployment&.cancel + end end def ensure_metadata @@ -323,8 +332,12 @@ module Ci self.options.fetch(:environment, {}).fetch(:action, 'start') if self.options end + def has_deployment? + !!self.deployment + end + def outdated_deployment? - success? && !last_deployment.try(:last?) + success? && !deployment.try(:last?) end def depends_on_builds @@ -339,6 +352,10 @@ module Ci user == current_user end + def on_stop + options&.dig(:environment, :on_stop) + end + # A slugified version of the build ref, suitable for inclusion in URLs and # domain names. Rules: # @@ -723,9 +740,9 @@ module Ci end def successful_deployment_status - if success? && last_deployment&.last? + if success? && deployment&.last? return :last - elsif success? && last_deployment.present? + elsif success? && deployment.present? return :out_of_date end diff --git a/app/models/concerns/deployable.rb b/app/models/concerns/deployable.rb new file mode 100644 index 00000000000..ba3abf28ab0 --- /dev/null +++ b/app/models/concerns/deployable.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module Deployable + extend ActiveSupport::Concern + + included do + after_create :create_deployment + + def create_deployment + return unless has_environment? && !has_deployment? + + environment = project.environments.find_or_create_by( + name: expanded_environment_name + ) + + environment.deployments.create!( + project_id: environment.project_id, + environment: environment, + ref: ref, + tag: tag, + sha: sha, + user: user, + deployable: self, + on_stop: on_stop).tap do |_| + self.reload # Reload relationships + end + end + end +end diff --git a/app/models/deployment.rb b/app/models/deployment.rb index ee5b96e7454..eecb927ad01 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -3,6 +3,7 @@ class Deployment < ActiveRecord::Base include AtomicInternalId include IidRoutes + include AfterCommitQueue belongs_to :project, required: true belongs_to :environment, required: true @@ -16,11 +17,44 @@ class Deployment < ActiveRecord::Base delegate :name, to: :environment, prefix: true - after_create :create_ref - after_create :invalidate_cache - scope :for_environment, -> (environment) { where(environment_id: environment) } + state_machine :status, initial: :created do + event :run do + transition created: :running + end + + event :succeed do + transition any - [:success] => :success + end + + event :drop do + transition any - [:failed] => :failed + end + + event :cancel do + transition any - [:canceled] => :canceled + end + + before_transition any => [:success, :failed, :canceled] do |deployment| + deployment.finished_at = Time.now + end + + after_transition any => :success do |deployment| + deployment.run_after_commit do + Deployments::SuccessWorker.perform_async(id) + end + end + end + + enum status: { + created: 0, + running: 1, + success: 2, + failed: 3, + canceled: 4 + } + def self.last_for_environment(environment) ids = self .for_environment(environment) @@ -65,15 +99,15 @@ class Deployment < ActiveRecord::Base end def update_merge_request_metrics! - return unless environment.update_merge_request_metrics? + return unless environment.update_merge_request_metrics? && success? merge_requests = project.merge_requests .joins(:metrics) .where(target_branch: self.ref, merge_request_metrics: { first_deployed_to_production_at: nil }) - .where("merge_request_metrics.merged_at <= ?", self.created_at) + .where("merge_request_metrics.merged_at <= ?", finished_at) if previous_deployment - merge_requests = merge_requests.where("merge_request_metrics.merged_at >= ?", previous_deployment.created_at) + merge_requests = merge_requests.where("merge_request_metrics.merged_at >= ?", previous_deployment.finished_at) end # Need to use `map` instead of `select` because MySQL doesn't allow `SELECT`ing from the same table @@ -87,7 +121,7 @@ class Deployment < ActiveRecord::Base MergeRequest::Metrics .where(merge_request_id: merge_request_ids, first_deployed_to_production_at: nil) - .update_all(first_deployed_to_production_at: self.created_at) + .update_all(first_deployed_to_production_at: finished_at) end def previous_deployment @@ -105,8 +139,18 @@ class Deployment < ActiveRecord::Base @stop_action ||= manual_actions.find_by(name: on_stop) end + def finished_at + read_attribute(:finished_at) || legacy_finished_at + end + + def deployed_at + return unless success? + + finished_at + end + def formatted_deployment_time - created_at.to_time.in_time_zone.to_s(:medium) + deployed_at&.to_time&.in_time_zone&.to_s(:medium) end def has_metrics? @@ -114,21 +158,17 @@ class Deployment < ActiveRecord::Base end def metrics - return {} unless has_metrics? + return {} unless has_metrics? && success? metrics = prometheus_adapter.query(:deployment, self) - metrics&.merge(deployment_time: created_at.to_i) || {} + metrics&.merge(deployment_time: finished_at.to_i) || {} end def additional_metrics - return {} unless has_metrics? + return {} unless has_metrics? && success? metrics = prometheus_adapter.query(:additional_metrics_deployment, self) - metrics&.merge(deployment_time: created_at.to_i) || {} - end - - def status - 'success' + metrics&.merge(deployment_time: finished_at.to_i) || {} end private @@ -140,4 +180,8 @@ class Deployment < ActiveRecord::Base def ref_path File.join(environment.ref_path, 'deployments', iid.to_s) end + + def legacy_finished_at + self.created_at if success? && !read_attribute(:finished_at) + end end diff --git a/app/models/environment.rb b/app/models/environment.rb index 1c31c01eb9f..7d104bb0c25 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -8,9 +8,9 @@ class Environment < ActiveRecord::Base belongs_to :project, required: true - has_many :deployments, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent + has_many :deployments, -> { success }, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent - has_one :last_deployment, -> { order('deployments.id DESC') }, class_name: 'Deployment' + has_one :last_deployment, -> { success.order('deployments.id DESC') }, class_name: 'Deployment' before_validation :nullify_external_url before_validation :generate_slug, if: ->(env) { env.slug.blank? } diff --git a/app/models/environment_status.rb b/app/models/environment_status.rb index a84871f7253..c9bc9c18dfd 100644 --- a/app/models/environment_status.rb +++ b/app/models/environment_status.rb @@ -8,8 +8,8 @@ class EnvironmentStatus delegate :id, to: :environment delegate :name, to: :environment delegate :project, to: :environment - delegate :deployed_at, to: :deployment, allow_nil: true delegate :status, to: :deployment + delegate :deployed_at, to: :deployment def self.for_merge_request(mr, user) build_environments_status(mr, user, mr.head_pipeline) @@ -33,10 +33,6 @@ class EnvironmentStatus end end - def deployed_at - deployment&.created_at - end - def changes return [] if project.route_map_for(sha).nil? diff --git a/app/models/project.rb b/app/models/project.rb index e2e309e8496..df7f7895624 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -254,7 +254,7 @@ class Project < ActiveRecord::Base has_many :variables, class_name: 'Ci::Variable' has_many :triggers, class_name: 'Ci::Trigger' has_many :environments - has_many :deployments + has_many :deployments, -> { success } has_many :pipeline_schedules, class_name: 'Ci::PipelineSchedule' has_many :project_deploy_tokens has_many :deploy_tokens, through: :project_deploy_tokens diff --git a/app/services/create_deployment_service.rb b/app/services/create_deployment_service.rb deleted file mode 100644 index bb3f605da28..00000000000 --- a/app/services/create_deployment_service.rb +++ /dev/null @@ -1,74 +0,0 @@ -# frozen_string_literal: true - -class CreateDeploymentService - attr_reader :job - - delegate :expanded_environment_name, - :variables, - :project, - to: :job - - def initialize(job) - @job = job - end - - def execute - return unless executable? - - ActiveRecord::Base.transaction do - environment.external_url = expanded_environment_url if - expanded_environment_url - - environment.fire_state_event(action) - - break unless environment.save - break if environment.stopped? - - deploy.tap(&:update_merge_request_metrics!) - end - end - - private - - def executable? - project && job.environment.present? && environment - end - - def deploy - project.deployments.create( - environment: environment, - ref: job.ref, - tag: job.tag, - sha: job.sha, - user: job.user, - deployable: job, - on_stop: on_stop) - end - - def environment - @environment ||= job.persisted_environment - end - - def environment_options - @environment_options ||= job.options&.dig(:environment) || {} - end - - def expanded_environment_url - return @expanded_environment_url if defined?(@expanded_environment_url) - - @expanded_environment_url = - ExpandVariables.expand(environment_url, variables) if environment_url - end - - def environment_url - environment_options[:url] - end - - def on_stop - environment_options[:on_stop] - end - - def action - environment_options[:action] || 'start' - end -end diff --git a/app/services/update_deployment_service.rb b/app/services/update_deployment_service.rb new file mode 100644 index 00000000000..aa7fcca1e2a --- /dev/null +++ b/app/services/update_deployment_service.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +class UpdateDeploymentService + attr_reader :deployment + attr_reader :deployable + + delegate :environment, to: :deployment + delegate :variables, to: :deployable + + def initialize(deployment) + @deployment = deployment + @deployable = deployment.deployable + end + + def execute + deployment.create_ref + deployment.invalidate_cache + + ActiveRecord::Base.transaction do + environment.external_url = expanded_environment_url if + expanded_environment_url + + environment.fire_state_event(action) + + break unless environment.save + break if environment.stopped? + + deployment.tap(&:update_merge_request_metrics!) + end + end + + private + + def environment_options + @environment_options ||= deployable.options&.dig(:environment) || {} + end + + def expanded_environment_url + return @expanded_environment_url if defined?(@expanded_environment_url) + return unless environment_url + + @expanded_environment_url = + ExpandVariables.expand(environment_url, variables) + end + + def environment_url + environment_options[:url] + end + + def action + environment_options[:action] || 'start' + end +end diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml index f21789de37d..39436680540 100644 --- a/app/workers/all_queues.yml +++ b/app/workers/all_queues.yml @@ -72,6 +72,8 @@ - pipeline_processing:update_head_pipeline_for_merge_request - pipeline_processing:ci_build_schedule +- deployment:deployments_success + - repository_check:repository_check_clear - repository_check:repository_check_batch - repository_check:repository_check_single_repository diff --git a/app/workers/build_success_worker.rb b/app/workers/build_success_worker.rb index c17608f7378..f3530317090 100644 --- a/app/workers/build_success_worker.rb +++ b/app/workers/build_success_worker.rb @@ -16,7 +16,14 @@ class BuildSuccessWorker private + ## + # Deprecated: + # As of 11.5, we started creating a deployment record when ci_builds record is created. + # Therefore we no longer need to create a deployment, after a build succeeded. + # We're leaving this code for the transition period, but we can remove this code in 11.6. def create_deployment(build) - CreateDeploymentService.new(build).execute + build.create_deployment.try do |deployment| + deployment.succeed + end end end diff --git a/app/workers/deployments/success_worker.rb b/app/workers/deployments/success_worker.rb new file mode 100644 index 00000000000..fabd6aa12a4 --- /dev/null +++ b/app/workers/deployments/success_worker.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module Deployments + class SuccessWorker + include ApplicationWorker + + queue_namespace :deployment + + def perform(deployment_id) + Deployment.find_by_id(deployment_id).try do |deployment| + UpdateDeploymentService.new(deployment).execute + end + end + end +end diff --git a/changelogs/unreleased/stateful_deployments.yml b/changelogs/unreleased/stateful_deployments.yml new file mode 100644 index 00000000000..4caa5ad77b8 --- /dev/null +++ b/changelogs/unreleased/stateful_deployments.yml @@ -0,0 +1,5 @@ +--- +title: Add status to Deployment +merge_request: 22380 +author: +type: changed diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml index 0e723cdeb9c..53e1c8778b6 100644 --- a/config/sidekiq_queues.yml +++ b/config/sidekiq_queues.yml @@ -29,6 +29,7 @@ - [pipeline_creation, 4] - [pipeline_default, 3] - [pipeline_cache, 3] + - [deployment, 3] - [pipeline_hooks, 2] - [gitlab_shell, 2] - [email_receiver, 2] diff --git a/db/fixtures/development/17_cycle_analytics.rb b/db/fixtures/development/17_cycle_analytics.rb index 285436f4324..7a86fe2eb7c 100644 --- a/db/fixtures/development/17_cycle_analytics.rb +++ b/db/fixtures/development/17_cycle_analytics.rb @@ -180,11 +180,8 @@ class Gitlab::Seeder::CycleAnalytics ref: "refs/heads/#{merge_request.source_branch}") pipeline = service.execute(:push, ignore_skip_ci: true, save_on_errors: false) - pipeline.run! - Timecop.travel rand(1..6).hours.from_now - pipeline.succeed! - - PipelineMetricsWorker.new.perform(pipeline.id) + pipeline.builds.map(&:run!) + pipeline.update_status end end @@ -204,7 +201,8 @@ class Gitlab::Seeder::CycleAnalytics job = merge_request.head_pipeline.builds.where.not(environment: nil).last - CreateDeploymentService.new(job).execute + job.success! + pipeline.update_status end end end diff --git a/db/migrate/20181015155839_add_finished_at_to_deployments.rb b/db/migrate/20181015155839_add_finished_at_to_deployments.rb new file mode 100644 index 00000000000..1a061bb0f5f --- /dev/null +++ b/db/migrate/20181015155839_add_finished_at_to_deployments.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class AddFinishedAtToDeployments < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def up + add_column :deployments, :finished_at, :datetime_with_timezone + end + + def down + remove_column :deployments, :finished_at, :datetime_with_timezone + end +end diff --git a/db/migrate/20181016141739_add_status_to_deployments.rb b/db/migrate/20181016141739_add_status_to_deployments.rb new file mode 100644 index 00000000000..321172696b4 --- /dev/null +++ b/db/migrate/20181016141739_add_status_to_deployments.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +class AddStatusToDeployments < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DEPLOYMENT_STATUS_SUCCESS = 2 # Equivalent to Deployment.state_machine.states['success'].value + + DOWNTIME = false + + disable_ddl_transaction! + + ## + # NOTE: + # Ideally, `status` column should not have default value because it should be leveraged by state machine (i.e. application level). + # However, we have to use the default value for avoiding `NOT NULL` violation during the transition period. + # The default value should be removed in the future release. + def up + add_column_with_default(:deployments, + :status, + :integer, + limit: 2, + default: DEPLOYMENT_STATUS_SUCCESS, + allow_null: false) + end + + def down + remove_column(:deployments, :status) + end +end diff --git a/db/migrate/20181022135539_add_index_on_status_to_deployments.rb b/db/migrate/20181022135539_add_index_on_status_to_deployments.rb new file mode 100644 index 00000000000..2eed20aa855 --- /dev/null +++ b/db/migrate/20181022135539_add_index_on_status_to_deployments.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class AddIndexOnStatusToDeployments < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + add_concurrent_index :deployments, [:project_id, :status] + add_concurrent_index :deployments, [:environment_id, :status] + end + + def down + remove_concurrent_index :deployments, [:project_id, :status] + remove_concurrent_index :deployments, [:environment_id, :status] + end +end diff --git a/db/post_migrate/20181030135124_fill_empty_finished_at_in_deployments.rb b/db/post_migrate/20181030135124_fill_empty_finished_at_in_deployments.rb new file mode 100644 index 00000000000..fd330619a88 --- /dev/null +++ b/db/post_migrate/20181030135124_fill_empty_finished_at_in_deployments.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +class FillEmptyFinishedAtInDeployments < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + class Deployments < ActiveRecord::Base + self.table_name = 'deployments' + + include EachBatch + end + + def up + FillEmptyFinishedAtInDeployments::Deployments + .where('finished_at IS NULL') + .each_batch(of: 10_000) do |relation| + relation.update_all('finished_at=created_at') + end + end + + def down + # no-op + end +end diff --git a/db/schema.rb b/db/schema.rb index 22474916034..fdb2a2c2134 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -824,13 +824,17 @@ ActiveRecord::Schema.define(version: 20181031190559) do t.datetime "created_at" t.datetime "updated_at" t.string "on_stop" + t.integer "status", limit: 2, default: 2, null: false + t.datetime_with_timezone "finished_at" end add_index "deployments", ["created_at"], name: "index_deployments_on_created_at", using: :btree add_index "deployments", ["deployable_type", "deployable_id"], name: "index_deployments_on_deployable_type_and_deployable_id", using: :btree add_index "deployments", ["environment_id", "id"], name: "index_deployments_on_environment_id_and_id", using: :btree add_index "deployments", ["environment_id", "iid", "project_id"], name: "index_deployments_on_environment_id_and_iid_and_project_id", using: :btree + add_index "deployments", ["environment_id", "status"], name: "index_deployments_on_environment_id_and_status", using: :btree add_index "deployments", ["project_id", "iid"], name: "index_deployments_on_project_id_and_iid", unique: true, using: :btree + add_index "deployments", ["project_id", "status"], name: "index_deployments_on_project_id_and_status", using: :btree create_table "emails", force: :cascade do |t| t.integer "user_id", null: false diff --git a/lib/gitlab/ci/pipeline/chain/create.rb b/lib/gitlab/ci/pipeline/chain/create.rb index c882241ef6a..aa627bdb009 100644 --- a/lib/gitlab/ci/pipeline/chain/create.rb +++ b/lib/gitlab/ci/pipeline/chain/create.rb @@ -7,26 +7,11 @@ module Gitlab class Create < Chain::Base include Chain::Helpers - # rubocop: disable CodeReuse/ActiveRecord def perform! - ::Ci::Pipeline.transaction do - pipeline.save! - - ## - # Create environments before the pipeline starts. - # - pipeline.builds.each do |build| - if build.has_environment? - project.environments.find_or_create_by( - name: build.expanded_environment_name - ) - end - end - end + pipeline.save! rescue ActiveRecord::RecordInvalid => e error("Failed to persist the pipeline: #{e}") end - # rubocop: enable CodeReuse/ActiveRecord def break? !pipeline.persisted? diff --git a/spec/controllers/projects/deployments_controller_spec.rb b/spec/controllers/projects/deployments_controller_spec.rb index d1c960e895d..5b7da81b6a1 100644 --- a/spec/controllers/projects/deployments_controller_spec.rb +++ b/spec/controllers/projects/deployments_controller_spec.rb @@ -15,9 +15,9 @@ describe Projects::DeploymentsController do describe 'GET #index' do it 'returns list of deployments from last 8 hours' do - create(:deployment, environment: environment, created_at: 9.hours.ago) - create(:deployment, environment: environment, created_at: 7.hours.ago) - create(:deployment, environment: environment) + create(:deployment, :success, environment: environment, created_at: 9.hours.ago) + create(:deployment, :success, environment: environment, created_at: 7.hours.ago) + create(:deployment, :success, environment: environment) get :index, deployment_params(after: 8.hours.ago) @@ -27,7 +27,7 @@ describe Projects::DeploymentsController do end it 'returns a list with deployments information' do - create(:deployment, environment: environment) + create(:deployment, :success, environment: environment) get :index, deployment_params @@ -37,7 +37,7 @@ describe Projects::DeploymentsController do end describe 'GET #metrics' do - let(:deployment) { create(:deployment, project: project, environment: environment) } + let(:deployment) { create(:deployment, :success, project: project, environment: environment) } before do allow(controller).to receive(:deployment).and_return(deployment) @@ -110,7 +110,7 @@ describe Projects::DeploymentsController do end describe 'GET #additional_metrics' do - let(:deployment) { create(:deployment, project: project, environment: environment) } + let(:deployment) { create(:deployment, :success, project: project, environment: environment) } before do allow(controller).to receive(:deployment).and_return(deployment) diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 7463586621c..dc08692eec2 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -754,7 +754,7 @@ describe Projects::MergeRequestsController do let(:environment) { create(:environment, project: forked) } let(:pipeline) { create(:ci_pipeline, sha: sha, project: forked) } let(:build) { create(:ci_build, pipeline: pipeline) } - let!(:deployment) { create(:deployment, environment: environment, sha: sha, ref: 'master', deployable: build) } + let!(:deployment) { create(:deployment, :succeed, environment: environment, sha: sha, ref: 'master', deployable: build) } let(:merge_request) do create(:merge_request, source_project: forked, target_project: project, target_branch: 'master', head_pipeline: pipeline) @@ -779,7 +779,7 @@ describe Projects::MergeRequestsController do let(:merge_commit_sha) { project.repository.merge(user, forked.commit.id, merge_request, "merged in test") } let(:post_merge_pipeline) { create(:ci_pipeline, sha: merge_commit_sha, project: project) } let(:post_merge_build) { create(:ci_build, pipeline: post_merge_pipeline) } - let!(:source_deployment) { create(:deployment, environment: source_environment, sha: merge_commit_sha, ref: 'master', deployable: post_merge_build) } + let!(:source_deployment) { create(:deployment, :succeed, environment: source_environment, sha: merge_commit_sha, ref: 'master', deployable: post_merge_build) } before do merge_request.update!(merge_commit_sha: merge_commit_sha) diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index 85ba7d4097d..5ef2497cf78 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -94,6 +94,30 @@ FactoryBot.define do url: 'http://staging.example.com/$CI_JOB_NAME' } end + trait :deploy_to_production do + environment 'production' + + options environment: { name: 'production', + url: 'http://prd.example.com/$CI_JOB_NAME' } + end + + trait :start_review_app do + environment 'review/$CI_COMMIT_REF_NAME' + + options environment: { name: 'review/$CI_COMMIT_REF_NAME', + url: 'http://staging.example.com/$CI_JOB_NAME', + on_stop: 'stop_review_app' } + end + + trait :stop_review_app do + name 'stop_review_app' + environment 'review/$CI_COMMIT_REF_NAME' + + options environment: { name: 'review/$CI_COMMIT_REF_NAME', + url: 'http://staging.example.com/$CI_JOB_NAME', + action: 'stop' } + end + trait :allowed_to_fail do allow_failure true end diff --git a/spec/factories/deployments.rb b/spec/factories/deployments.rb index 90d6a338479..a68f385d222 100644 --- a/spec/factories/deployments.rb +++ b/spec/factories/deployments.rb @@ -21,5 +21,31 @@ FactoryBot.define do sha { TestEnv::BRANCH_SHA['pages-deploy'] } ref 'pages-deploy' end + + trait :running do + status { Deployment.state_machine.states['running'].value } + end + + trait :success do + status { Deployment.state_machine.states['success'].value } + finished_at { Time.now } + end + + trait :failed do + status { Deployment.state_machine.states['failed'].value } + finished_at { Time.now } + end + + trait :canceled do + status { Deployment.state_machine.states['canceled'].value } + finished_at { Time.now } + end + + # This trait hooks the state maechine's events + trait :succeed do + after(:create) do |deployment, evaluator| + deployment.succeed! + end + end end end diff --git a/spec/factories/environments.rb b/spec/factories/environments.rb index b5db57d5148..9d9e3d693b8 100644 --- a/spec/factories/environments.rb +++ b/spec/factories/environments.rb @@ -22,6 +22,7 @@ FactoryBot.define do pipeline: pipeline) deployment = create(:deployment, + :success, environment: environment, project: environment.project, deployable: deployable, diff --git a/spec/features/merge_request/user_sees_deployment_widget_spec.rb b/spec/features/merge_request/user_sees_deployment_widget_spec.rb index a298ead43db..cbd130b9740 100644 --- a/spec/features/merge_request/user_sees_deployment_widget_spec.rb +++ b/spec/features/merge_request/user_sees_deployment_widget_spec.rb @@ -11,7 +11,7 @@ describe 'Merge request > User sees deployment widget', :js do let(:sha) { project.commit(ref).id } let(:pipeline) { create(:ci_pipeline_without_jobs, sha: sha, project: project, ref: ref) } let(:build) { create(:ci_build, :success, pipeline: pipeline) } - let!(:deployment) { create(:deployment, environment: environment, sha: sha, ref: ref, deployable: build) } + let!(:deployment) { create(:deployment, :succeed, environment: environment, sha: sha, ref: ref, deployable: build) } let!(:manual) { } before do diff --git a/spec/features/merge_request/user_sees_merge_widget_spec.rb b/spec/features/merge_request/user_sees_merge_widget_spec.rb index 0c610edd6d1..a703823cb33 100644 --- a/spec/features/merge_request/user_sees_merge_widget_spec.rb +++ b/spec/features/merge_request/user_sees_merge_widget_spec.rb @@ -45,7 +45,8 @@ describe 'Merge request > User sees merge widget', :js do let(:build) { create(:ci_build, :success, pipeline: pipeline) } let!(:deployment) do - create(:deployment, environment: environment, + create(:deployment, :succeed, + environment: environment, ref: merge_request.source_branch, deployable: build, sha: sha) diff --git a/spec/features/projects/environments/environment_spec.rb b/spec/features/projects/environments/environment_spec.rb index 70e0879dd81..7826d26ff1b 100644 --- a/spec/features/projects/environments/environment_spec.rb +++ b/spec/features/projects/environments/environment_spec.rb @@ -33,7 +33,7 @@ describe 'Environment' do context 'with deployments' do context 'when there is no related deployable' do let(:deployment) do - create(:deployment, environment: environment, deployable: nil) + create(:deployment, :success, environment: environment, deployable: nil) end it 'does show deployment SHA' do @@ -48,7 +48,7 @@ describe 'Environment' do let(:build) { create(:ci_build, pipeline: pipeline) } let(:deployment) do - create(:deployment, environment: environment, deployable: build) + create(:deployment, :success, environment: environment, deployable: build) end it 'does show build name' do @@ -97,7 +97,7 @@ describe 'Environment' do context 'with external_url' do let(:environment) { create(:environment, project: project, external_url: 'https://git.gitlab.com') } let(:build) { create(:ci_build, pipeline: pipeline) } - let(:deployment) { create(:deployment, environment: environment, deployable: build) } + let(:deployment) { create(:deployment, :success, environment: environment, deployable: build) } it 'does show an external link button' do expect(page).to have_link(nil, href: environment.external_url) @@ -158,7 +158,7 @@ describe 'Environment' do end let(:deployment) do - create(:deployment, environment: environment, + create(:deployment, :success, environment: environment, deployable: build, on_stop: 'close_app') end diff --git a/spec/features/projects/jobs_spec.rb b/spec/features/projects/jobs_spec.rb index 5cb3f7c732f..5e7bcb862cd 100644 --- a/spec/features/projects/jobs_spec.rb +++ b/spec/features/projects/jobs_spec.rb @@ -397,7 +397,7 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do context 'job is successful and has deployment' do let(:build) { create(:ci_build, :success, :trace_live, environment: environment.name, pipeline: pipeline) } - let!(:deployment) { create(:deployment, environment: environment, project: environment.project, deployable: build) } + let!(:deployment) { create(:deployment, :success, environment: environment, project: environment.project, deployable: build) } it 'shows a link for the job' do expect(page).to have_link environment.name @@ -456,6 +456,8 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do describe 'environment info in job view', :js do before do + allow_any_instance_of(Ci::Build).to receive(:create_deployment) + visit project_job_path(project, job) wait_for_requests end @@ -464,8 +466,8 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do let(:job) { create(:ci_build, :success, :trace_artifact, environment: 'staging', pipeline: pipeline) } let(:second_build) { create(:ci_build, :success, :trace_artifact, environment: 'staging', pipeline: pipeline) } let(:environment) { create(:environment, name: 'staging', project: project) } - let!(:first_deployment) { create(:deployment, environment: environment, deployable: job) } - let!(:second_deployment) { create(:deployment, environment: environment, deployable: second_build) } + let!(:first_deployment) { create(:deployment, :success, environment: environment, deployable: job) } + let!(:second_deployment) { create(:deployment, :success, environment: environment, deployable: second_build) } it 'shows deployment message' do expected_text = 'This job is an out-of-date deployment ' \ @@ -505,7 +507,7 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do end context 'when it has deployment' do - let!(:deployment) { create(:deployment, environment: environment) } + let!(:deployment) { create(:deployment, :success, environment: environment) } it 'shows that deployment will be overwritten' do expected_text = 'This job is creating a deployment to staging' diff --git a/spec/features/projects/view_on_env_spec.rb b/spec/features/projects/view_on_env_spec.rb index a48ad94e9fa..7bfcd46713e 100644 --- a/spec/features/projects/view_on_env_spec.rb +++ b/spec/features/projects/view_on_env_spec.rb @@ -44,7 +44,7 @@ describe 'View on environment', :js do context 'and an active deployment' do let(:sha) { project.commit(branch_name).sha } let(:environment) { create(:environment, project: project, name: 'review/feature', external_url: 'http://feature.review.example.com') } - let!(:deployment) { create(:deployment, environment: environment, ref: branch_name, sha: sha) } + let!(:deployment) { create(:deployment, :success, environment: environment, ref: branch_name, sha: sha) } context 'when visiting the diff of a merge request for the branch' do let(:merge_request) { create(:merge_request, :simple, source_project: project, source_branch: branch_name) } diff --git a/spec/finders/environments_finder_spec.rb b/spec/finders/environments_finder_spec.rb index 3cd421f22eb..25835bb4d94 100644 --- a/spec/finders/environments_finder_spec.rb +++ b/spec/finders/environments_finder_spec.rb @@ -12,7 +12,7 @@ describe EnvironmentsFinder do context 'tagged deployment' do before do - create(:deployment, environment: environment, ref: 'v1.1.0', tag: true, sha: project.commit.id) + create(:deployment, :success, environment: environment, ref: 'v1.1.0', tag: true, sha: project.commit.id) end it 'returns environment when with_tags is set' do @@ -33,7 +33,7 @@ describe EnvironmentsFinder do context 'branch deployment' do before do - create(:deployment, environment: environment, ref: 'master', sha: project.commit.id) + create(:deployment, :success, environment: environment, ref: 'master', sha: project.commit.id) end it 'returns environment when ref is set' do @@ -59,7 +59,7 @@ describe EnvironmentsFinder do context 'commit deployment' do before do - create(:deployment, environment: environment, ref: 'master', sha: project.commit.id) + create(:deployment, :success, environment: environment, ref: 'master', sha: project.commit.id) end it 'returns environment' do @@ -71,7 +71,7 @@ describe EnvironmentsFinder do context 'recently updated' do context 'when last deployment to environment is the most recent one' do before do - create(:deployment, environment: environment, ref: 'feature') + create(:deployment, :success, environment: environment, ref: 'feature') end it 'finds recently updated environment' do @@ -82,8 +82,8 @@ describe EnvironmentsFinder do context 'when last deployment to environment is not the most recent' do before do - create(:deployment, environment: environment, ref: 'feature') - create(:deployment, environment: environment, ref: 'master') + create(:deployment, :success, environment: environment, ref: 'feature') + create(:deployment, :success, environment: environment, ref: 'master') end it 'does not find environment' do @@ -96,8 +96,8 @@ describe EnvironmentsFinder do let(:second_environment) { create(:environment, project: project) } before do - create(:deployment, environment: environment, ref: 'feature') - create(:deployment, environment: second_environment, ref: 'feature') + create(:deployment, :success, environment: environment, ref: 'feature') + create(:deployment, :success, environment: second_environment, ref: 'feature') end it 'finds both environments' do diff --git a/spec/lib/gitlab/cycle_analytics/stage_summary_spec.rb b/spec/lib/gitlab/cycle_analytics/stage_summary_spec.rb index 2e67c1c7f78..f8009709ce2 100644 --- a/spec/lib/gitlab/cycle_analytics/stage_summary_spec.rb +++ b/spec/lib/gitlab/cycle_analytics/stage_summary_spec.rb @@ -44,15 +44,15 @@ describe Gitlab::CycleAnalytics::StageSummary do describe "#deploys" do it "finds the number of deploys made created after the 'from date'" do - Timecop.freeze(5.days.ago) { create(:deployment, project: project) } - Timecop.freeze(5.days.from_now) { create(:deployment, project: project) } + Timecop.freeze(5.days.ago) { create(:deployment, :success, project: project) } + Timecop.freeze(5.days.from_now) { create(:deployment, :success, project: project) } expect(subject.third[:value]).to eq(1) end it "doesn't find commits from other projects" do Timecop.freeze(5.days.from_now) do - create(:deployment, project: create(:project, :repository)) + create(:deployment, :success, project: create(:project, :repository)) end expect(subject.third[:value]).to eq(0) diff --git a/spec/lib/gitlab/slash_commands/command_spec.rb b/spec/lib/gitlab/slash_commands/command_spec.rb index 194cae8c645..eceacac58af 100644 --- a/spec/lib/gitlab/slash_commands/command_spec.rb +++ b/spec/lib/gitlab/slash_commands/command_spec.rb @@ -44,7 +44,7 @@ describe Gitlab::SlashCommands::Command do let!(:build) { create(:ci_build, pipeline: pipeline) } let!(:pipeline) { create(:ci_pipeline, project: project) } let!(:staging) { create(:environment, name: 'staging', project: project) } - let!(:deployment) { create(:deployment, environment: staging, deployable: build) } + let!(:deployment) { create(:deployment, :success, environment: staging, deployable: build) } let!(:manual) do create(:ci_build, :manual, pipeline: pipeline, diff --git a/spec/lib/gitlab/slash_commands/deploy_spec.rb b/spec/lib/gitlab/slash_commands/deploy_spec.rb index 0d57334aa4c..25f3e8a0409 100644 --- a/spec/lib/gitlab/slash_commands/deploy_spec.rb +++ b/spec/lib/gitlab/slash_commands/deploy_spec.rb @@ -31,7 +31,7 @@ describe Gitlab::SlashCommands::Deploy do let!(:staging) { create(:environment, name: 'staging', project: project) } let!(:pipeline) { create(:ci_pipeline, project: project) } let!(:build) { create(:ci_build, pipeline: pipeline) } - let!(:deployment) { create(:deployment, environment: staging, deployable: build) } + let!(:deployment) { create(:deployment, :success, environment: staging, deployable: build) } context 'without actions' do it 'does not execute an action' do diff --git a/spec/migrations/delete_inconsistent_internal_id_records_spec.rb b/spec/migrations/delete_inconsistent_internal_id_records_spec.rb index becb71cf427..4af51217031 100644 --- a/spec/migrations/delete_inconsistent_internal_id_records_spec.rb +++ b/spec/migrations/delete_inconsistent_internal_id_records_spec.rb @@ -65,6 +65,21 @@ describe DeleteInconsistentInternalIdRecords, :migration do context 'for deployments' do let(:scope) { :deployment } + let(:deployments) { table(:deployments) } + let(:internal_ids) { table(:internal_ids) } + + before do + internal_ids.create!(project_id: project1.id, usage: 2, last_value: 2) + internal_ids.create!(project_id: project2.id, usage: 2, last_value: 2) + internal_ids.create!(project_id: project3.id, usage: 2, last_value: 2) + end + + let(:create_models) do + 3.times { |i| deployments.create!(project_id: project1.id, iid: i, environment_id: 1, ref: 'master', sha: 'a', tag: false) } + 3.times { |i| deployments.create!(project_id: project2.id, iid: i, environment_id: 1, ref: 'master', sha: 'a', tag: false) } + 3.times { |i| deployments.create!(project_id: project3.id, iid: i, environment_id: 1, ref: 'master', sha: 'a', tag: false) } + end + it_behaves_like 'deleting inconsistent internal_id records' end diff --git a/spec/migrations/fill_empty_finished_at_in_deployments_spec.rb b/spec/migrations/fill_empty_finished_at_in_deployments_spec.rb new file mode 100644 index 00000000000..afe07effc41 --- /dev/null +++ b/spec/migrations/fill_empty_finished_at_in_deployments_spec.rb @@ -0,0 +1,49 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20181030135124_fill_empty_finished_at_in_deployments') + +describe FillEmptyFinishedAtInDeployments, :migration do + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + let(:environments) { table(:environments) } + let(:deployments) { table(:deployments) } + + context 'when a deployment row does not have a value on finished_at' do + before do + namespaces.create!(id: 123, name: 'gitlab1', path: 'gitlab1') + projects.create!(id: 1, name: 'gitlab1', path: 'gitlab1', namespace_id: 123) + environments.create!(id: 1, name: 'production', slug: 'production', project_id: 1) + deployments.create!(id: 1, iid: 1, project_id: 1, environment_id: 1, ref: 'master', sha: 'xxx', tag: false) + end + + it 'correctly migrates nullified file_store/store column' do + expect(deployments.last.created_at).not_to be_nil + expect(deployments.last.finished_at).to be_nil + + migrate! + + expect(deployments.last.created_at).not_to be_nil + expect(deployments.last.finished_at).to eq(deployments.last.created_at) + end + end + + context 'when a deployment row does has a value on finished_at' do + let(:finished_at) { '2018-10-30 11:12:02 UTC' } + + before do + namespaces.create!(id: 123, name: 'gitlab1', path: 'gitlab1') + projects.create!(id: 1, name: 'gitlab1', path: 'gitlab1', namespace_id: 123) + environments.create!(id: 1, name: 'production', slug: 'production', project_id: 1) + deployments.create!(id: 1, iid: 1, project_id: 1, environment_id: 1, ref: 'master', sha: 'xxx', tag: false, finished_at: finished_at) + end + + it 'does not affect existing value' do + expect(deployments.last.created_at).not_to be_nil + expect(deployments.last.finished_at).not_to be_nil + + migrate! + + expect(deployments.last.created_at).not_to be_nil + expect(deployments.last.finished_at).to eq(finished_at) + end + end +end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 65e06f27f35..1be1602fe36 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -17,7 +17,6 @@ describe Ci::Build do it { is_expected.to belong_to(:runner) } it { is_expected.to belong_to(:trigger_request) } it { is_expected.to belong_to(:erased_by) } - it { is_expected.to have_many(:deployments) } it { is_expected.to have_many(:trace_sections)} it { is_expected.to have_one(:runner_session)} it { is_expected.to validate_presence_of(:ref) } @@ -811,33 +810,154 @@ describe Ci::Build do end end + describe '.deployment' do + subject { build.deployment } + + context 'when there is an old deployment record' do + before do + create(:deployment, deployable: build, project: project) + end + + context 'when there is a deployment record with created status' do + let!(:deployment) { create(:deployment, deployable: build, project: project) } + + it 'returns the record' do + is_expected.to eq(deployment) + end + end + + context 'when there is a deployment record with running status' do + let!(:deployment) { create(:deployment, :running, deployable: build, project: project) } + + it 'returns the record' do + is_expected.to eq(deployment) + end + end + + context 'when there is a deployment record with success status' do + let!(:deployment) { create(:deployment, :success, deployable: build, project: project) } + + it 'returns the record' do + is_expected.to eq(deployment) + end + end + end + end + + describe 'state transition as a deployable' do + let!(:build) { create(:ci_build, :start_review_app) } + let(:deployment) { build.deployment } + let(:environment) { deployment.environment } + + it 'has deployments record with created status' do + expect(deployment).to be_created + expect(environment.name).to eq('review/master') + end + + context 'when transits to running' do + before do + build.run! + end + + it 'transits deployment status to running' do + expect(deployment).to be_running + end + end + + context 'when transits to success' do + before do + allow(Deployments::SuccessWorker).to receive(:perform_async) + build.success! + end + + it 'transits deployment status to success' do + expect(deployment).to be_success + end + end + + context 'when transits to failed' do + before do + build.drop! + end + + it 'transits deployment status to failed' do + expect(deployment).to be_failed + end + end + + context 'when transits to skipped' do + before do + build.skip! + end + + it 'transits deployment status to canceled' do + expect(deployment).to be_canceled + end + end + + context 'when transits to canceled' do + before do + build.cancel! + end + + it 'transits deployment status to canceled' do + expect(deployment).to be_canceled + end + end + end + + describe '#on_stop' do + subject { build.on_stop } + + context 'when a job has a specification that it can be stopped from the other job' do + let(:build) { create(:ci_build, :start_review_app) } + + it 'returns the other job name' do + is_expected.to eq('stop_review_app') + end + end + end + describe 'deployment' do - describe '#last_deployment' do - subject { build.last_deployment } + describe '#deployment' do + subject { build.deployment } - context 'when multiple deployments are created' do - let!(:deployment1) { create(:deployment, deployable: build) } - let!(:deployment2) { create(:deployment, deployable: build) } + context 'when a deployment is created' do + let!(:deployment) { create(:deployment, deployable: build) } - it 'returns the latest one' do - is_expected.to eq(deployment2) + it 'returns the deployment' do + is_expected.to eq(deployment) end end end + describe '#has_deployment?' do + subject { build.has_deployment? } + + context 'when build has a deployment' do + let!(:deployment) { create(:deployment, deployable: build) } + + it { is_expected.to be_truthy } + end + + context 'when build does not have a deployment' do + it { is_expected.to be_falsy } + end + end + describe '#outdated_deployment?' do subject { build.outdated_deployment? } context 'when build succeeded' do let(:build) { create(:ci_build, :success) } - let!(:deployment) { create(:deployment, deployable: build) } + let!(:deployment) { create(:deployment, :success, deployable: build) } context 'current deployment is latest' do it { is_expected.to be_falsey } end context 'current deployment is not latest on environment' do - let!(:deployment2) { create(:deployment, environment: deployment.environment) } + let!(:deployment2) { create(:deployment, :success, environment: deployment.environment) } it { is_expected.to be_truthy } end @@ -3157,10 +3277,14 @@ describe Ci::Build do end describe '#deployment_status' do + before do + allow_any_instance_of(described_class).to receive(:create_deployment) + end + context 'when build is a last deployment' do let(:build) { create(:ci_build, :success, environment: 'production') } let(:environment) { create(:environment, name: 'production', project: build.project) } - let!(:deployment) { create(:deployment, environment: environment, project: environment.project, deployable: build) } + let!(:deployment) { create(:deployment, :success, environment: environment, project: environment.project, deployable: build) } it { expect(build.deployment_status).to eq(:last) } end @@ -3168,8 +3292,8 @@ describe Ci::Build do context 'when there is a newer build with deployment' do let(:build) { create(:ci_build, :success, environment: 'production') } let(:environment) { create(:environment, name: 'production', project: build.project) } - let!(:deployment) { create(:deployment, environment: environment, project: environment.project, deployable: build) } - let!(:last_deployment) { create(:deployment, environment: environment, project: environment.project) } + let!(:deployment) { create(:deployment, :success, environment: environment, project: environment.project, deployable: build) } + let!(:last_deployment) { create(:deployment, :success, environment: environment, project: environment.project) } it { expect(build.deployment_status).to eq(:out_of_date) } end @@ -3177,7 +3301,7 @@ describe Ci::Build do context 'when build with deployment has failed' do let(:build) { create(:ci_build, :failed, environment: 'production') } let(:environment) { create(:environment, name: 'production', project: build.project) } - let!(:deployment) { create(:deployment, environment: environment, project: environment.project, deployable: build) } + let!(:deployment) { create(:deployment, :success, environment: environment, project: environment.project, deployable: build) } it { expect(build.deployment_status).to eq(:failed) } end @@ -3185,7 +3309,7 @@ describe Ci::Build do context 'when build with deployment is running' do let(:build) { create(:ci_build, environment: 'production') } let(:environment) { create(:environment, name: 'production', project: build.project) } - let!(:deployment) { create(:deployment, environment: environment, project: environment.project, deployable: build) } + let!(:deployment) { create(:deployment, :success, environment: environment, project: environment.project, deployable: build) } it { expect(build.deployment_status).to eq(:creating) } end diff --git a/spec/models/concerns/deployable_spec.rb b/spec/models/concerns/deployable_spec.rb new file mode 100644 index 00000000000..28c9fb98872 --- /dev/null +++ b/spec/models/concerns/deployable_spec.rb @@ -0,0 +1,36 @@ +require 'rails_helper' + +describe Deployable do + describe '#create_deployment' do + let(:deployment) { job.deployment } + let(:environment) { deployment&.environment } + + context 'when the deployable object will deploy to production' do + let!(:job) { create(:ci_build, :start_review_app) } + + before do + job.reload + end + + it 'creates a deployment and environment record' do + expect(deployment.project).to eq(job.project) + expect(deployment.ref).to eq(job.ref) + expect(deployment.tag).to eq(job.tag) + expect(deployment.sha).to eq(job.sha) + expect(deployment.user).to eq(job.user) + expect(deployment.deployable).to eq(job) + expect(deployment.on_stop).to eq('stop_review_app') + expect(environment.name).to eq('review/master') + end + end + + context 'when the deployable object will not deploy' do + let!(:job) { create(:ci_build) } + + it 'does not create a deployment and environment record' do + expect(deployment).to be_nil + expect(environment).to be_nil + end + end + end +end diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index b8364e0cf88..1e2f19fd9e3 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -26,16 +26,174 @@ describe Deployment do end end - describe 'after_create callbacks' do - let(:environment) { create(:environment) } - let(:store) { Gitlab::EtagCaching::Store.new } + describe '.success' do + subject { described_class.success } - it 'invalidates the environment etag cache' do - old_value = store.get(environment.etag_cache_key) + context 'when deployment status is success' do + let(:deployment) { create(:deployment, :success) } - create(:deployment, environment: environment) + it { is_expected.to eq([deployment]) } + end + + context 'when deployment status is created' do + let(:deployment) { create(:deployment, :created) } + + it { is_expected.to be_empty } + end + + context 'when deployment status is running' do + let(:deployment) { create(:deployment, :running) } + + it { is_expected.to be_empty } + end + end + + describe 'state machine' do + context 'when deployment runs' do + let(:deployment) { create(:deployment) } + + before do + deployment.run! + end + + it 'starts running' do + Timecop.freeze do + expect(deployment).to be_running + expect(deployment.finished_at).to be_nil + end + end + end + + context 'when deployment succeeded' do + let(:deployment) { create(:deployment, :running) } + + it 'has correct status' do + Timecop.freeze do + deployment.succeed! + + expect(deployment).to be_success + expect(deployment.finished_at).to eq(Time.now) + end + end + + it 'executes Deployments::SuccessWorker asynchronously' do + expect(Deployments::SuccessWorker) + .to receive(:perform_async).with(deployment.id) - expect(store.get(environment.etag_cache_key)).not_to eq(old_value) + deployment.succeed! + end + end + + context 'when deployment failed' do + let(:deployment) { create(:deployment, :running) } + + it 'has correct status' do + Timecop.freeze do + deployment.drop! + + expect(deployment).to be_failed + expect(deployment.finished_at).to eq(Time.now) + end + end + end + + context 'when deployment was canceled' do + let(:deployment) { create(:deployment, :running) } + + it 'has correct status' do + Timecop.freeze do + deployment.cancel! + + expect(deployment).to be_canceled + expect(deployment.finished_at).to eq(Time.now) + end + end + end + end + + describe '#success?' do + subject { deployment.success? } + + context 'when deployment status is success' do + let(:deployment) { create(:deployment, :success) } + + it { is_expected.to be_truthy } + end + + context 'when deployment status is failed' do + let(:deployment) { create(:deployment, :failed) } + + it { is_expected.to be_falsy } + end + end + + describe '#status_name' do + subject { deployment.status_name } + + context 'when deployment status is success' do + let(:deployment) { create(:deployment, :success) } + + it { is_expected.to eq(:success) } + end + + context 'when deployment status is failed' do + let(:deployment) { create(:deployment, :failed) } + + it { is_expected.to eq(:failed) } + end + end + + describe '#finished_at' do + subject { deployment.finished_at } + + context 'when deployment status is created' do + let(:deployment) { create(:deployment) } + + it { is_expected.to be_nil } + end + + context 'when deployment status is success' do + let(:deployment) { create(:deployment, :success) } + + it { is_expected.to eq(deployment.read_attribute(:finished_at)) } + end + + context 'when deployment status is success' do + let(:deployment) { create(:deployment, :success, finished_at: nil) } + + before do + deployment.update_column(:finished_at, nil) + end + + it { is_expected.to eq(deployment.read_attribute(:created_at)) } + end + + context 'when deployment status is running' do + let(:deployment) { create(:deployment, :running) } + + it { is_expected.to be_nil } + end + end + + describe '#deployed_at' do + subject { deployment.deployed_at } + + context 'when deployment status is created' do + let(:deployment) { create(:deployment) } + + it { is_expected.to be_nil } + end + + context 'when deployment status is success' do + let(:deployment) { create(:deployment, :success) } + + it { is_expected.to eq(deployment.read_attribute(:finished_at)) } + end + + context 'when deployment status is running' do + let(:deployment) { create(:deployment, :running) } + + it { is_expected.to be_nil } end end @@ -96,7 +254,7 @@ describe Deployment do end describe '#metrics' do - let(:deployment) { create(:deployment) } + let(:deployment) { create(:deployment, :success) } let(:prometheus_adapter) { double('prometheus_adapter', can_query?: true) } subject { deployment.metrics } @@ -125,7 +283,7 @@ describe Deployment do describe '#additional_metrics' do let(:project) { create(:project, :repository) } - let(:deployment) { create(:deployment, project: project) } + let(:deployment) { create(:deployment, :succeed, project: project) } subject { deployment.additional_metrics } diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 1de95d881a7..e121369f6ac 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -95,7 +95,7 @@ describe Environment do context 'with a last deployment' do let!(:deployment) do - create(:deployment, environment: environment, sha: project.commit('master').id) + create(:deployment, :success, environment: environment, sha: project.commit('master').id) end context 'in the same branch' do @@ -136,8 +136,8 @@ describe Environment do describe '#first_deployment_for' do let(:project) { create(:project, :repository) } - let!(:deployment) { create(:deployment, environment: environment, ref: commit.parent.id) } - let!(:deployment1) { create(:deployment, environment: environment, ref: commit.id) } + let!(:deployment) { create(:deployment, :succeed, environment: environment, ref: commit.parent.id) } + let!(:deployment1) { create(:deployment, :succeed, environment: environment, ref: commit.id) } let(:head_commit) { project.commit } let(:commit) { project.commit.parent } @@ -181,7 +181,8 @@ describe Environment do let(:build) { create(:ci_build) } let!(:deployment) do - create(:deployment, environment: environment, + create(:deployment, :success, + environment: environment, deployable: build, on_stop: 'close_app') end @@ -249,7 +250,8 @@ describe Environment do let(:build) { create(:ci_build, pipeline: pipeline) } let!(:deployment) do - create(:deployment, environment: environment, + create(:deployment, :success, + environment: environment, deployable: build, on_stop: 'close_app') end @@ -304,7 +306,7 @@ describe Environment do context 'when last deployment to environment is the most recent one' do before do - create(:deployment, environment: environment, ref: 'feature') + create(:deployment, :success, environment: environment, ref: 'feature') end it { is_expected.to be true } @@ -312,8 +314,8 @@ describe Environment do context 'when last deployment to environment is not the most recent' do before do - create(:deployment, environment: environment, ref: 'feature') - create(:deployment, environment: environment, ref: 'master') + create(:deployment, :success, environment: environment, ref: 'feature') + create(:deployment, :success, environment: environment, ref: 'master') end it { is_expected.to be false } @@ -321,7 +323,7 @@ describe Environment do end describe '#actions_for' do - let(:deployment) { create(:deployment, environment: environment) } + let(:deployment) { create(:deployment, :success, environment: environment) } let(:pipeline) { deployment.deployable.pipeline } let!(:review_action) { create(:ci_build, :manual, name: 'review-apps', pipeline: pipeline, environment: 'review/$CI_COMMIT_REF_NAME' )} let!(:production_action) { create(:ci_build, :manual, name: 'production', pipeline: pipeline, environment: 'production' )} @@ -331,6 +333,70 @@ describe Environment do end end + describe '.deployments' do + subject { environment.deployments } + + context 'when there is a deployment record with created status' do + let(:deployment) { create(:deployment, :created, environment: environment) } + + it 'does not return the record' do + is_expected.to be_empty + end + end + + context 'when there is a deployment record with running status' do + let(:deployment) { create(:deployment, :running, environment: environment) } + + it 'does not return the record' do + is_expected.to be_empty + end + end + + context 'when there is a deployment record with success status' do + let(:deployment) { create(:deployment, :success, environment: environment) } + + it 'returns the record' do + is_expected.to eq([deployment]) + end + end + end + + describe '.last_deployment' do + subject { environment.last_deployment } + + before do + allow_any_instance_of(Deployment).to receive(:create_ref) + end + + context 'when there is an old deployment record' do + let!(:previous_deployment) { create(:deployment, :success, environment: environment) } + + context 'when there is a deployment record with created status' do + let!(:deployment) { create(:deployment, environment: environment) } + + it 'returns the previous deployment' do + is_expected.to eq(previous_deployment) + end + end + + context 'when there is a deployment record with running status' do + let!(:deployment) { create(:deployment, :running, environment: environment) } + + it 'returns the previous deployment' do + is_expected.to eq(previous_deployment) + end + end + + context 'when there is a deployment record with success status' do + let!(:deployment) { create(:deployment, :success, environment: environment) } + + it 'returns the latest successful deployment' do + is_expected.to eq(deployment) + end + end + end + end + describe '#has_terminals?' do subject { environment.has_terminals? } @@ -338,7 +404,7 @@ describe Environment do context 'with a deployment service' do shared_examples 'same behavior between KubernetesService and Platform::Kubernetes' do context 'and a deployment' do - let!(:deployment) { create(:deployment, environment: environment) } + let!(:deployment) { create(:deployment, :success, environment: environment) } it { is_expected.to be_truthy } end diff --git a/spec/models/environment_status_spec.rb b/spec/models/environment_status_spec.rb index e7805d52d75..52b98552184 100644 --- a/spec/models/environment_status_spec.rb +++ b/spec/models/environment_status_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe EnvironmentStatus do - let(:deployment) { create(:deployment, :review_app) } + let(:deployment) { create(:deployment, :succeed, :review_app) } let(:environment) { deployment.environment} let(:project) { deployment.project } let(:merge_request) { create(:merge_request, :deployed_review_app, deployment: deployment) } @@ -12,7 +12,7 @@ describe EnvironmentStatus do it { is_expected.to delegate_method(:id).to(:environment) } it { is_expected.to delegate_method(:name).to(:environment) } it { is_expected.to delegate_method(:project).to(:environment) } - it { is_expected.to delegate_method(:deployed_at).to(:deployment).as(:created_at) } + it { is_expected.to delegate_method(:deployed_at).to(:deployment) } it { is_expected.to delegate_method(:status).to(:deployment) } describe '#project' do diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 85a4ebac66c..4c5d80a4561 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1836,8 +1836,8 @@ describe MergeRequest do let(:environments) { create_list(:environment, 3, project: project) } before do - create(:deployment, environment: environments.first, ref: 'master', sha: project.commit('master').id) - create(:deployment, environment: environments.second, ref: 'feature', sha: project.commit('feature').id) + create(:deployment, :success, environment: environments.first, ref: 'master', sha: project.commit('master').id) + create(:deployment, :success, environment: environments.second, ref: 'feature', sha: project.commit('feature').id) end it 'selects deployed environments' do @@ -1857,7 +1857,7 @@ describe MergeRequest do let(:source_environment) { create(:environment, project: source_project) } before do - create(:deployment, environment: source_environment, ref: 'feature', sha: merge_request.diff_head_sha) + create(:deployment, :success, environment: source_environment, ref: 'feature', sha: merge_request.diff_head_sha) end it 'selects deployed environments' do @@ -1868,7 +1868,7 @@ describe MergeRequest do let(:target_environment) { create(:environment, project: project) } before do - create(:deployment, environment: target_environment, tag: true, sha: merge_request.diff_head_sha) + create(:deployment, :success, environment: target_environment, tag: true, sha: merge_request.diff_head_sha) end it 'selects deployed environments' do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index d4b9a4c8cd6..5c64fa27046 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -3963,6 +3963,40 @@ describe Project do end end + describe '.deployments' do + subject { project.deployments } + + let(:project) { create(:project) } + + before do + allow_any_instance_of(Deployment).to receive(:create_ref) + end + + context 'when there is a deployment record with created status' do + let(:deployment) { create(:deployment, :created, project: project) } + + it 'does not return the record' do + is_expected.to be_empty + end + end + + context 'when there is a deployment record with running status' do + let(:deployment) { create(:deployment, :running, project: project) } + + it 'does not return the record' do + is_expected.to be_empty + end + end + + context 'when there is a deployment record with success status' do + let(:deployment) { create(:deployment, :success, project: project) } + + it 'returns the record' do + is_expected.to eq([deployment]) + end + end + end + def rugged_config rugged_repo(project.repository).config end diff --git a/spec/requests/api/deployments_spec.rb b/spec/requests/api/deployments_spec.rb index 61ae053cea7..ab63bf832ef 100644 --- a/spec/requests/api/deployments_spec.rb +++ b/spec/requests/api/deployments_spec.rb @@ -10,9 +10,9 @@ describe API::Deployments do describe 'GET /projects/:id/deployments' do let(:project) { create(:project) } - let!(:deployment_1) { create(:deployment, project: project, iid: 11, ref: 'master', created_at: Time.now) } - let!(:deployment_2) { create(:deployment, project: project, iid: 12, ref: 'feature', created_at: 1.day.ago) } - let!(:deployment_3) { create(:deployment, project: project, iid: 8, ref: 'feature', created_at: 2.days.ago) } + let!(:deployment_1) { create(:deployment, :success, project: project, iid: 11, ref: 'master', created_at: Time.now) } + let!(:deployment_2) { create(:deployment, :success, project: project, iid: 12, ref: 'feature', created_at: 1.day.ago) } + let!(:deployment_3) { create(:deployment, :success, project: project, iid: 8, ref: 'feature', created_at: 2.days.ago) } context 'as member of the project' do it 'returns projects deployments sorted by id asc' do @@ -76,7 +76,7 @@ describe API::Deployments do describe 'GET /projects/:id/deployments/:deployment_id' do let(:project) { deployment.environment.project } - let!(:deployment) { create(:deployment) } + let!(:deployment) { create(:deployment, :success) } context 'as a member of the project' do it 'returns the projects deployment' do diff --git a/spec/serializers/environment_serializer_spec.rb b/spec/serializers/environment_serializer_spec.rb index 0f0ab5ac796..87493a28d1f 100644 --- a/spec/serializers/environment_serializer_spec.rb +++ b/spec/serializers/environment_serializer_spec.rb @@ -14,7 +14,8 @@ describe EnvironmentSerializer do let(:project) { create(:project, :repository) } let(:deployable) { create(:ci_build) } let(:deployment) do - create(:deployment, deployable: deployable, + create(:deployment, :success, + deployable: deployable, user: user, project: project, sha: project.commit.id) diff --git a/spec/serializers/environment_status_entity_spec.rb b/spec/serializers/environment_status_entity_spec.rb index 1b4d8b70aa6..962ec919092 100644 --- a/spec/serializers/environment_status_entity_spec.rb +++ b/spec/serializers/environment_status_entity_spec.rb @@ -4,8 +4,8 @@ describe EnvironmentStatusEntity do let(:user) { create(:user) } let(:request) { double('request') } - let(:deployment) { create(:deployment, :review_app) } - let(:environment) { deployment.environment} + let(:deployment) { create(:deployment, :succeed, :review_app) } + let(:environment) { deployment.environment } let(:project) { deployment.project } let(:merge_request) { create(:merge_request, :deployed_review_app, deployment: deployment) } diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb index 368abded448..e779675744c 100644 --- a/spec/services/ci/retry_build_service_spec.rb +++ b/spec/services/ci/retry_build_service_spec.rb @@ -32,7 +32,7 @@ describe Ci::RetryBuildService do IGNORE_ACCESSORS = %i[type lock_version target_url base_tags trace_sections - commit_id deployments erased_by_id last_deployment project_id + commit_id deployment erased_by_id project_id runner_id tag_taggings taggings tags trigger_request_id user_id auto_canceled_by_id retried failure_reason artifacts_file_store artifacts_metadata_store diff --git a/spec/services/create_deployment_service_spec.rb b/spec/services/update_deployment_service_spec.rb index b9bfbb11511..3a34bceae23 100644 --- a/spec/services/create_deployment_service_spec.rb +++ b/spec/services/update_deployment_service_spec.rb @@ -1,50 +1,57 @@ require 'spec_helper' -describe CreateDeploymentService do +describe UpdateDeploymentService do let(:user) { create(:user) } - let(:options) { nil } + let(:options) { { name: 'production' } } let(:job) do create(:ci_build, ref: 'master', tag: false, environment: 'production', - options: { environment: options }) + options: { environment: options }, + project: project) end - let(:project) { job.project } - - let!(:environment) do - create(:environment, project: project, name: 'production') - end - - let(:service) { described_class.new(job) } + let(:project) { create(:project, :repository) } + let(:environment) { deployment.environment } + let(:deployment) { job.deployment } + let(:service) { described_class.new(deployment) } before do - allow_any_instance_of(Deployment).to receive(:create_ref) + job.success! # Create/Succeed deployment end describe '#execute' do subject { service.execute } - context 'when environment exists' do - it 'creates a deployment' do - expect(subject).to be_persisted - end + let(:store) { Gitlab::EtagCaching::Store.new } + + it 'invalidates the environment etag cache' do + old_value = store.get(environment.etag_cache_key) + + subject + + expect(store.get(environment.etag_cache_key)).not_to eq(old_value) end - context 'when environment does not exist' do - let(:environment) {} + it 'creates ref' do + expect_any_instance_of(Repository) + .to receive(:create_ref) + .with(deployment.ref, deployment.send(:ref_path)) - it 'does not create a deployment' do - expect do - expect(subject).to be_nil - end.not_to change { Deployment.count } - end + subject + end + + it 'updates merge request metrics' do + expect_any_instance_of(Deployment) + .to receive(:update_merge_request_metrics!) + + subject end context 'when start action is defined' do - let(:options) { { action: 'start' } } + let(:options) { { name: 'production', action: 'start' } } context 'and environment is stopped' do before do @@ -56,15 +63,11 @@ describe CreateDeploymentService do expect(environment.reload).to be_available end - - it 'creates a deployment' do - expect(subject).to be_persisted - end end end context 'when stop action is defined' do - let(:options) { { action: 'stop' } } + let(:options) { { name: 'production', action: 'stop' } } context 'and environment is available' do before do @@ -76,10 +79,6 @@ describe CreateDeploymentService do expect(environment.reload).to be_stopped end - - it 'does not create a deployment' do - expect(subject).to be_nil - end end end @@ -94,10 +93,6 @@ describe CreateDeploymentService do job.update(environment: 'review-apps/$CI_COMMIT_REF_NAME') end - it 'creates a new deployment' do - expect(subject).to be_persisted - end - it 'does not create a new environment' do expect { subject }.not_to change { Environment.count } end @@ -109,21 +104,6 @@ describe CreateDeploymentService do expect(subject.environment.external_url).to eq('http://master.review-apps.gitlab.com') end end - - context 'when project was removed' do - let(:environment) {} - - before do - job.update(project: nil) - end - - it 'does not create deployment or environment' do - expect { subject }.not_to raise_error - - expect(Environment.count).to be_zero - expect(Deployment.count).to be_zero - end - end end describe '#expanded_environment_url' do @@ -133,7 +113,9 @@ describe CreateDeploymentService do let(:job) do create(:ci_build, ref: 'master', - options: { environment: { url: 'http://review/$CI_COMMIT_REF_NAME' } }) + environment: 'production', + project: project, + options: { environment: { name: 'production', url: 'http://review/$CI_COMMIT_REF_NAME' } }) end it { is_expected.to eq('http://review/master') } @@ -143,16 +125,9 @@ describe CreateDeploymentService do let(:job) do create(:ci_build, ref: 'master', - environment: 'production', - options: { environment: { url: 'http://review/$CI_ENVIRONMENT_SLUG' } }) - end - - let!(:environment) do - create(:environment, - project: job.project, - name: 'production', - slug: 'prod-slug', - external_url: 'http://review/old') + environment: 'prod-slug', + project: project, + options: { environment: { name: 'prod-slug', url: 'http://review/$CI_ENVIRONMENT_SLUG' } }) end it { is_expected.to eq('http://review/prod-slug') } @@ -162,18 +137,16 @@ describe CreateDeploymentService do let(:job) do create(:ci_build, yaml_variables: [{ key: :APP_HOST, value: 'host' }], - options: { environment: { url: 'http://review/$APP_HOST' } }) + environment: 'production', + project: project, + options: { environment: { name: 'production', url: 'http://review/$APP_HOST' } }) end it { is_expected.to eq('http://review/host') } end context 'when yaml environment does not have url' do - let(:job) { create(:ci_build, environment: 'staging') } - - let!(:environment) do - create(:environment, project: job.project, name: job.environment) - end + let(:job) { create(:ci_build, environment: 'staging', project: project) } it 'returns the external_url from persisted environment' do is_expected.to be_nil @@ -181,92 +154,12 @@ describe CreateDeploymentService do end end - describe 'processing of builds' do - shared_examples 'does not create deployment' do - it 'does not create a new deployment' do - expect { subject }.not_to change { Deployment.count } - end - - it 'does not call a service' do - expect_any_instance_of(described_class).not_to receive(:execute) - - subject - end - end - - shared_examples 'creates deployment' do - it 'creates a new deployment' do - expect { subject }.to change { Deployment.count }.by(1) - end - - it 'calls a service' do - expect_any_instance_of(described_class).to receive(:execute) - - subject - end - - it 'is set as deployable' do - subject - - expect(Deployment.last.deployable).to eq(deployable) - end - - it 'updates environment URL' do - subject - - expect(Deployment.last.environment.external_url).not_to be_nil - end - end - - context 'without environment specified' do - let(:job) { create(:ci_build) } - - it_behaves_like 'does not create deployment' do - subject { job.success } - end - end - - context 'when environment is specified' do - let(:deployable) { job } - - let(:options) do - { environment: { name: 'production', url: 'http://gitlab.com' } } - end - - context 'when job succeeds' do - it_behaves_like 'creates deployment' do - subject { job.success } - end - end - - context 'when job fails' do - it_behaves_like 'does not create deployment' do - subject { job.drop } - end - end - - context 'when job is retried' do - it_behaves_like 'creates deployment' do - before do - stub_not_protect_default_branch - - project.add_developer(user) - end - - let(:deployable) { Ci::Build.retry(job, user) } - - subject { deployable.success } - end - end - end - end - describe "merge request metrics" do let(:merge_request) { create(:merge_request, target_branch: 'master', source_branch: 'feature', source_project: project) } context "while updating the 'first_deployed_to_production_at' time" do before do - merge_request.metrics.update!(merged_at: Time.now) + merge_request.metrics.update!(merged_at: 10.minutes.ago) end context "for merge requests merged before the current deploy" do @@ -277,12 +170,21 @@ describe CreateDeploymentService do expect(merge_request.reload.metrics.first_deployed_to_production_at).to be_like_time(time) end - it "doesn't set the time if the deploy's environment is not 'production'" do - job.update(environment: 'staging') - service = described_class.new(job) - service.execute + context 'when job deploys to staging' do + let(:job) do + create(:ci_build, + ref: 'master', + tag: false, + environment: 'staging', + options: { environment: { name: 'staging' } }, + project: project) + end + + it "doesn't set the time if the deploy's environment is not 'production'" do + service.execute - expect(merge_request.reload.metrics.first_deployed_to_production_at).to be_nil + expect(merge_request.reload.metrics.first_deployed_to_production_at).to be_nil + end end it 'does not raise errors if the merge request does not have a metrics record' do @@ -303,7 +205,6 @@ describe CreateDeploymentService do expect(merge_request.reload.metrics.first_deployed_to_production_at).to be_like_time(time) # Current deploy - service = described_class.new(job) Timecop.freeze(time + 12.hours) { service.execute } expect(merge_request.reload.metrics.first_deployed_to_production_at).to be_like_time(time) @@ -318,15 +219,12 @@ describe CreateDeploymentService do expect(merge_request.reload.metrics.merged_at).to be < merge_request.reload.metrics.first_deployed_to_production_at - merge_request.reload.metrics.update(first_deployed_to_production_at: nil) - - expect(merge_request.reload.metrics.first_deployed_to_production_at).to be_nil + previous_time = merge_request.reload.metrics.first_deployed_to_production_at # Current deploy - service = described_class.new(job) Timecop.freeze(time + 12.hours) { service.execute } - expect(merge_request.reload.metrics.first_deployed_to_production_at).to be_nil + expect(merge_request.reload.metrics.first_deployed_to_production_at).to eq(previous_time) end end end diff --git a/spec/support/helpers/cycle_analytics_helpers.rb b/spec/support/helpers/cycle_analytics_helpers.rb index 83035788a56..aaca21957a8 100644 --- a/spec/support/helpers/cycle_analytics_helpers.rb +++ b/spec/support/helpers/cycle_analytics_helpers.rb @@ -85,7 +85,7 @@ module CycleAnalyticsHelpers raise ArgumentError end - CreateDeploymentService.new(dummy_job).execute + dummy_job.success! end def dummy_production_job(user, project) @@ -97,7 +97,7 @@ module CycleAnalyticsHelpers end def dummy_pipeline(project) - Ci::Pipeline.new( + create(:ci_pipeline, sha: project.repository.commit('master').sha, ref: 'master', source: :push, @@ -106,9 +106,7 @@ module CycleAnalyticsHelpers end def new_dummy_job(user, project, environment) - project.environments.find_or_create_by(name: environment) - - Ci::Build.new( + create(:ci_build, project: project, user: user, environment: environment, diff --git a/spec/workers/build_success_worker_spec.rb b/spec/workers/build_success_worker_spec.rb index dba70883130..05ae03566ca 100644 --- a/spec/workers/build_success_worker_spec.rb +++ b/spec/workers/build_success_worker_spec.rb @@ -2,26 +2,40 @@ require 'spec_helper' describe BuildSuccessWorker do describe '#perform' do + subject { described_class.new.perform(build.id) } + context 'when build exists' do - context 'when build belogs to the environment' do - let!(:build) { create(:ci_build, environment: 'production') } + context 'when deployment was not created when build was created because of transition period' do + before do + allow_any_instance_of(Deployment).to receive(:create_ref) + end + + context 'when build belogs to the environment' do + let!(:build) { create(:ci_build, :deploy_to_production) } - it 'executes deployment service' do - expect_any_instance_of(CreateDeploymentService) - .to receive(:execute) + before do + Deployment.delete_all + end - described_class.new.perform(build.id) + it 'creates a successful deployment' do + expect(build).not_to be_has_deployment + + subject + + build.reload + expect(build).to be_has_deployment + expect(build.deployment).to be_success + end end - end - context 'when build is not associated with project' do - let!(:build) { create(:ci_build, project: nil) } + context 'when build is not associated with project' do + let!(:build) { create(:ci_build, project: nil) } - it 'does not create deployment' do - expect_any_instance_of(CreateDeploymentService) - .not_to receive(:execute) + it 'does not create deployment' do + subject - described_class.new.perform(build.id) + expect(build.reload).not_to be_has_deployment + end end end end diff --git a/spec/workers/deployments/success_worker_spec.rb b/spec/workers/deployments/success_worker_spec.rb new file mode 100644 index 00000000000..91e323f202b --- /dev/null +++ b/spec/workers/deployments/success_worker_spec.rb @@ -0,0 +1,26 @@ +require 'spec_helper' + +describe Deployments::SuccessWorker do + subject { described_class.new.perform(deployment&.id) } + + context 'when deploy record exists' do + let(:deployment) { create(:deployment) } + + it 'executes UpdateDeploymentService' do + expect(UpdateDeploymentService) + .to receive(:new).with(deployment).and_call_original + + subject + end + end + + context 'when deploy record does not exist' do + let(:deployment) { nil } + + it 'executes UpdateDeploymentService' do + expect(UpdateDeploymentService).not_to receive(:new) + + subject + end + end +end |