fix: read error from upload artifacts execution.

What does this MR do?

Fixes a bug when an error occurs during the "artifacts upload" stage.

Why was this MR needed?

During a failure in the upload artifacts step of a build jobs were not failing. This is a problem.

I have tested my patch with a local build and it makes the job fail if an error occurs during artifact uploading execution:

• Example before patch https://gitlab.com/paulrbr/paulrbr.gitlab.io/-/jobs/28482190 (forcing to always return an error on upload artifact)

FATAL: invalid argument                            
Job succeeded

• Example after patch https://gitlab.com/paulrbr/paulrbr.gitlab.io/-/jobs/28483043 (forcing to always return an error on upload artifact)

FATAL: invalid argument                            
ERROR: Job failed: exit code 1

Are there points in the code the reviewer needs to double check?

Help would be appreciated to create a test case for failed uploads builds.

Does this MR meet the acceptance criteria?

• Documentation created/updated
• Tests
  • Added for this feature/bug
  • All builds are passing
• Branch has no merge conflicts with master (if you do - rebase it please)

What are the relevant issue numbers?

Fixes #2584 (closed)

common/build.go

@@ -9,7 +9,6 @@ import (
 	"path"
 	"strconv"
 	"strings"
-	"sync"
 	"time"
 
 	"github.com/Sirupsen/logrus"
@@ -174,42 +173,17 @@ func (b *Build) executeStage(ctx context.Context, buildStage BuildStage, executo
 }
 
 func (b *Build) executeUploadArtifacts(ctx context.Context, state error, executor Executor) (err error) {
-	jobState := state
-	var wait sync.WaitGroup
-	var uploadErrors []error
-
-	// Execute upload stages in parallel
-	for _, artifact := range b.Artifacts {
-		when := artifact.When
-		if state == nil {
-			// Previous stages were successful
-			if when == "" || when == ArtifactWhenOnSuccess || when == ArtifactWhenAlways {
-				wait.Add(1)
-				go func() {
-					uploadErrors = append(uploadErrors, b.executeStage(ctx, BuildStageUploadArtifacts, executor))
-					wait.Done()
-				}()
-			}
-		} else {
-			// Previous stage did fail
-			if when == ArtifactWhenOnFailure || when == ArtifactWhenAlways {
-				wait.Add(1)
-				go func() {
-					uploadErrors = append(uploadErrors, b.executeStage(ctx, BuildStageUploadArtifacts, executor))
-					wait.Done()
-				}()
-			}
+	var uploadError error
+
+	for _, artifact := range b.JobResponse.Artifacts {
+		if artifact.ShouldUpload(state) {
+			uploadError = b.executeStage(ctx, BuildStageUploadArtifacts, executor)
+		}
+		if uploadError != nil {
+			err = uploadError
 		}
 	}
-	wait.Wait()
 
-	// Use job's error if set
-	if jobState != nil {
-		err = jobState
-	}
-	if err == nil {
-		err = uploadErrors[0]
-	}
 	return
 }
 
@@ -242,7 +216,14 @@ func (b *Build) executeScript(ctx context.Context, executor Executor) error {
 	if err == nil {
 		err = b.executeStage(ctx, BuildStageArchiveCache, executor)
 	}
-	err = b.executeUploadArtifacts(ctx, err, executor)
+
+	jobState := err
+	err = b.executeUploadArtifacts(ctx, jobState, executor)
+
+	// Use job's error if set
+	if jobState != nil {
+		err = jobState
+	}
 	return err
 }