Commit 1d90f21e authored by Shinya Maeda's avatar Shinya Maeda Committed by Kerri Miller

Fix Resource Groups are not refreshed when a pipeline is canceled

parent b52f21df
...@@ -58,7 +58,8 @@ module Ci ...@@ -58,7 +58,8 @@ module Ci
after_transition any => ::Ci::Processable.completed_statuses do |processable| after_transition any => ::Ci::Processable.completed_statuses do |processable|
next unless processable.with_resource_group? next unless processable.with_resource_group?
next unless processable.resource_group.release_resource_from(processable)
processable.resource_group.release_resource_from(processable)
processable.run_after_commit do processable.run_after_commit do
Ci::ResourceGroups::AssignResourceFromResourceGroupWorker Ci::ResourceGroups::AssignResourceFromResourceGroupWorker
......
...@@ -155,14 +155,53 @@ See the [API documentation](../../api/resource_groups.md). ...@@ -155,14 +155,53 @@ See the [API documentation](../../api/resource_groups.md).
Read more how you can use GitLab for [safe deployments](../environments/deployment_safety.md). Read more how you can use GitLab for [safe deployments](../environments/deployment_safety.md).
<!-- ## Troubleshooting ## Troubleshooting
Include any troubleshooting steps that you can foresee. If you know beforehand what issues ### Avoid dead locks in pipeline configurations
one might have when setting this up, or when something is changed, or on upgrading, it's
important to describe those, too. Think of things that may go wrong and include them here.
This is important to minimize requests for support, and to avoid doc comments with
questions that you know someone might ask.
Each scenario can be a third-level heading, e.g. `### Getting error message X`. Since [`oldest_first` process mode](#process-modes) enforces the jobs to be executed in a pipeline order,
If you have none to add when creating a doc, leave this section in place there is a case that it doesn't work well with the other CI features.
but commented out to help encourage others to add to it in the future. -->
For example, when you run [a child pipeline](../pipelines/parent_child_pipelines.md)
that requires the same resource group with the parent pipeline,
a dead lock could happen. Here is an example of a _bad_ setup:
```yaml
# BAD
test:
stage: test
trigger:
include: child-pipeline-requires-production-resource-group.yml
strategy: depend
deploy:
stage: deploy
script: echo
resource_group: production
```
In a parent pipeline, it runs the `test` job that subsequently runs a child pipeline,
and the [`strategy: depend` option](../yaml/index.md#linking-pipelines-with-triggerstrategy) makes the `test` job wait until the child pipeline has finished.
The parent pipeline runs the `deploy` job in the next stage, that requires a resource from the `production` resource group.
If the process mode is `oldest_first`, it executes the jobs from the oldest pipelines, meaning the `deploy` job is going to be executed next.
However, a child pipeline also requires a resource from the `production` resource group.
Since the child pipeline is newer than the parent pipeline, the child pipeline
waits until the `deploy` job is finished, something that will never happen.
In this case, you should specify the `resource_group` keyword in the parent pipeline configuration instead:
```yaml
# GOOD
test:
stage: test
trigger:
include: child-pipeline.yml
strategy: depend
resource_group: production # Specify the resource group in the parent pipeline
deploy:
stage: deploy
script: echo
resource_group: production
```
...@@ -147,13 +147,20 @@ RSpec.describe Ci::Processable do ...@@ -147,13 +147,20 @@ RSpec.describe Ci::Processable do
end end
it 'releases a resource when build finished' do it 'releases a resource when build finished' do
expect(build.resource_group).to receive(:release_resource_from).with(build).and_call_original expect(build.resource_group).to receive(:release_resource_from).with(build).and_return(true).and_call_original
expect(Ci::ResourceGroups::AssignResourceFromResourceGroupWorker).to receive(:perform_async).with(build.resource_group_id) expect(Ci::ResourceGroups::AssignResourceFromResourceGroupWorker).to receive(:perform_async).with(build.resource_group_id)
build.enqueue_waiting_for_resource! build.enqueue_waiting_for_resource!
build.success! build.success!
end end
it 're-checks the resource group even if the processable does not retain a resource' do
expect(build.resource_group).to receive(:release_resource_from).with(build).and_return(false).and_call_original
expect(Ci::ResourceGroups::AssignResourceFromResourceGroupWorker).to receive(:perform_async).with(build.resource_group_id)
build.success!
end
context 'when build has prerequisites' do context 'when build has prerequisites' do
before do before do
allow(build).to receive(:any_unmet_prerequisites?) { true } allow(build).to receive(:any_unmet_prerequisites?) { true }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment