diff options
author | Rémy Coutable <remy@rymai.me> | 2016-10-13 16:33:19 +0000 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2016-10-13 16:33:19 +0000 |
commit | 9a7afd2a63ae9f9d02a403ecf6d1beea74ac13b7 (patch) | |
tree | e4ed189372f0772d6c45db885e16d4308a9a6a67 | |
parent | 626d5e555a5634abd4ab61cf942c36025aed60f4 (diff) | |
parent | 776cea4c00d883cafc2bc5381f3b61b146a93976 (diff) | |
download | gitlab-ce-9a7afd2a63ae9f9d02a403ecf6d1beea74ac13b7.tar.gz |
Merge branch '22655-deployments-don-t-always-have-keep-around-refs' into 'master'
Handle case where deployment ref no longer exists
## What does this MR do?
In 8.9, we didn't create keep-around refs for deployments. So it's possible that someone created a deployment (say, for testing), and then deleted the branch and all other references to that commit. That commit could then get GCed, and trying to view MRs on 8.11+ will show a 500. See https://gitlab.com/gitlab-org/gitlab-ce/issues/22655#note_16575020 for more details.
## Why was this MR needed?
If someone created a deployment on 8.9, then deleted all references to the commit for that deployment, we will throw an exception when checking if the deployment includes a commit.
Closes #22655.
See merge request !6855
-rw-r--r-- | app/models/deployment.rb | 9 | ||||
-rw-r--r-- | spec/models/deployment_spec.rb | 9 |
2 files changed, 17 insertions, 1 deletions
diff --git a/app/models/deployment.rb b/app/models/deployment.rb index 82b27b78229..f63cc179b9e 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -40,7 +40,14 @@ class Deployment < ActiveRecord::Base def includes_commit?(commit) return false unless commit - project.repository.is_ancestor?(commit.id, sha) + # Before 8.10, deployments didn't have keep-around refs. Any deployment + # created before then could have a `sha` referring to a commit that no + # longer exists in the repository, so just ignore those. + begin + project.repository.is_ancestor?(commit.id, sha) + rescue Rugged::OdbError + false + end end def update_merge_request_metrics! diff --git a/spec/models/deployment_spec.rb b/spec/models/deployment_spec.rb index bfff639ad78..01a4a53a264 100644 --- a/spec/models/deployment_spec.rb +++ b/spec/models/deployment_spec.rb @@ -38,5 +38,14 @@ describe Deployment, models: true do expect(deployment.includes_commit?(commit)).to be true end end + + context 'when the SHA for the deployment does not exist in the repo' do + it 'returns false' do + deployment.update(sha: Gitlab::Git::BLANK_SHA) + commit = project.commit + + expect(deployment.includes_commit?(commit)).to be false + end + end end end |