Merge branch 'fix-resource-group-stuck-issue' into 'master'

Fix Resource Groups are not refreshed when a pipeline is canceled

See merge request gitlab-org/gitlab!71538

app/models/ci/processable.rb

@@ -58,7 +58,8 @@ module Ci
 
       after_transition any => ::Ci::Processable.completed_statuses do |processable|
         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
           Ci::ResourceGroups::AssignResourceFromResourceGroupWorker

doc/ci/resource_groups/index.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).
 
-<!-- ## Troubleshooting
+## Troubleshooting
 
-Include any troubleshooting steps that you can foresee. If you know beforehand what issues
-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.
+### Avoid dead locks in pipeline configurations
 
-Each scenario can be a third-level heading, e.g. `### Getting error message X`.
-If you have none to add when creating a doc, leave this section in place
-but commented out to help encourage others to add to it in the future. -->
+Since [`oldest_first` process mode](#process-modes) enforces the jobs to be executed in a pipeline order,
+there is a case that it doesn't work well with the other CI features.
+
+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
+```

spec/models/ci/processable_spec.rb

@@ -147,13 +147,20 @@ RSpec.describe Ci::Processable do
         end
 
         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)
 
           build.enqueue_waiting_for_resource!
           build.success!
         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
           before do
             allow(build).to receive(:any_unmet_prerequisites?) { true }