CI: Improve error handling in gcloud copy operations (#73474)

pkg/build/gcloud/storage/gsutil.go

@@ -102,27 +102,37 @@ func (client *Client) CopyLocalDir(ctx context.Context, dir string, bucket *stor
 	}
 	log.Printf("Number or files to be copied over: %d\n", len(files))
 
+	errs := make([]error, 0, 10)
+	errsLock := sync.Mutex{}
+
 	for _, chunk := range asChunks(files, maxThreads) {
 		var wg sync.WaitGroup
 		for _, f := range chunk {
 			wg.Add(1)
 			go func(file File) {
 				defer wg.Done()
-				err = client.Copy(ctx, file, bucket, bucketPath, trim)
+				err := client.Copy(ctx, file, bucket, bucketPath, trim)
 				if err != nil {
 					log.Printf("failed to copy objects, err: %s\n", err.Error())
+					errsLock.Lock()
+					errs = append(errs, err)
+					errsLock.Unlock()
 				}
 			}(f)
 		}
 		wg.Wait()
 	}
 
+	if len(errs) > 0 {
+		return fmt.Errorf("copy operation failed: %s", errs)
+	}
+
 	return nil
 }
 
 // Copy copies a single local file into the bucket at the provided path.
 // trim variable should be set to true if the full object path is needed - false otherwise.
-func (client *Client) Copy(ctx context.Context, file File, bucket *storage.BucketHandle, remote string, trim bool) error {
+func (client *Client) Copy(ctx context.Context, file File, bucket *storage.BucketHandle, remote string, trim bool) (resultErr error) {
 	if bucket == nil {
 		return ErrorNilBucket
 	}
@@ -152,11 +162,16 @@ func (client *Client) Copy(ctx context.Context, file File, bucket *storage.Bucke
 	defer func() {
 		if err := wc.Close(); err != nil {
 			log.Println("failed to close writer", "err", err)
+			// Keep the original error intact if there was one:
+			if resultErr == nil {
+				resultErr = err
+			}
 		}
 	}()
 
 	if _, err = io.Copy(wc, localFile); err != nil {
-		return fmt.Errorf("failed to copy to Cloud Storage: %w", err)
+		resultErr = fmt.Errorf("failed to copy to Cloud Storage: %w", err)
+		return
 	}
 
 	return nil

pkg/build/gcloud/storage/gsutil_test.go

@@ -1,9 +1,14 @@
 package storage
 
 import (
+	"context"
+	"os"
+	"path/filepath"
 	"testing"
 
+	"cloud.google.com/go/storage"
 	"github.com/stretchr/testify/require"
+	"google.golang.org/api/option"
 )
 
 func Test_asChunks(t *testing.T) {
@@ -157,3 +162,64 @@ func Test_asChunks(t *testing.T) {
 		})
 	}
 }
+
+func TestCopyLocalDir(t *testing.T) {
+	t.Parallel()
+
+	tmpDir := t.TempDir()
+	testFiles := []string{"file1.txt", "file2.txt"}
+	for _, testFile := range testFiles {
+		path := filepath.Join(tmpDir, testFile)
+		require.NoError(t, os.WriteFile(path, []byte{}, 0600))
+	}
+
+	// If an upload fails then the whole copy operation should return an error.
+	t.Run("failure-should-error", func(t *testing.T) {
+		t.Parallel()
+
+		// Assemble:
+		ctx := context.Background()
+		client := createAnonymousClient(t, ctx)
+		testBucket := client.Bucket("grafana-testing-repo")
+
+		// Act:
+		err := client.CopyLocalDir(ctx, tmpDir, testBucket, "test-path", false)
+
+		// Assert:
+		// This should fail as the client has no access to the bucket to upload to:
+		require.Error(t, err)
+	})
+}
+
+func TestCopyRemoteDir(t *testing.T) {
+	t.Parallel()
+
+	t.Run("failure-should-error", func(t *testing.T) {
+		t.Parallel()
+
+		// Assemble:
+		ctx := context.Background()
+		client := createAnonymousClient(t, ctx)
+		testFromBucket := client.Bucket("grafana-testing-repo")
+		testToBucket := client.Bucket("grafana-testing-repo")
+
+		// Act:
+		err := client.CopyRemoteDir(ctx, testFromBucket, "test-from", testToBucket, "test-to")
+
+		// Assert:
+		// This should fail as the client has no access to the bucket to copy from/to. Unfortunately, this does not yet
+		// cover if a single file fails to get transferred.
+		require.Error(t, err)
+	})
+}
+
+func createAnonymousClient(t *testing.T, ctx context.Context) *Client {
+	t.Helper()
+	storageClient, err := storage.NewClient(ctx, option.WithoutAuthentication())
+	require.NoError(t, err)
+	client := &Client{
+		Client: *storageClient,
+	}
+	require.NoError(t, err)
+	return client
+}