fix(providers): execute rclone with non-cancelling context (#5040)

- test rclone after canceling starting context
- execute rclone with non-cancelling context
- create initial SFTP connection with non-canceling context
- nit: use context.WithoutCancel instead of Background
- Fixes #5039
- Ref #4972

internal/testutil/dockertestutil.go

@@ -50,7 +50,7 @@ func RunContainerAndKillOnCloseOrSkip(t *testing.T, args ...string) string {
 
 	t.Cleanup(func() {
 		// t.Context() is canceled by the time cleanup executes, so it cannot be used here
-		runDockerAndGetOutputOrSkip(context.Background(), t, "kill", containerID)
+		runDockerAndGetOutputOrSkip(context.WithoutCancel(t.Context()), t, "kill", containerID)
 	})
 
 	return containerID

repo/blob/rclone/rclone_storage.go

@@ -339,7 +339,7 @@ func New(ctx context.Context, opt *Options, isCreate bool) (blob.Storage, error)
 		"--vfs-write-back=0s", // disable write-back, critical for correctness
 	)
 
-	r.cmd = exec.CommandContext(ctx, rcloneExe, arguments...) //nolint:gosec
+	r.cmd = exec.CommandContext(context.WithoutCancel(ctx), rcloneExe, arguments...) //nolint:gosec
 	r.cmd.Env = append(r.cmd.Env, opt.RCloneEnv...)
 
 	// https://github.com/kopia/kopia/issues/1934

repo/blob/rclone/rclone_storage_test.go

@@ -62,6 +62,38 @@ func mustGetRcloneExeOrSkip(t *testing.T) string {
 	return rcloneExe
 }
 
+func TestRCloneStorageCancelContext(t *testing.T) {
+	t.Parallel()
+	testutil.ProviderTest(t)
+
+	rcloneExe := mustGetRcloneExeOrSkip(t)
+	dataDir := testutil.TempDirectory(t)
+	ctx := testlogging.Context(t)
+
+	// use context that gets canceled after opening storage to ensure it's not used beyond New().
+	newCtx, cancel := context.WithCancel(ctx)
+	st, err := rclone.New(newCtx, &rclone.Options{
+		// pass local file as remote path.
+		RemotePath: dataDir,
+		RCloneExe:  rcloneExe,
+	}, true)
+
+	cancel()
+
+	require.NoError(t, err, "unable to connect to rclone backend")
+	require.NotNil(t, st, "unable to connect to rclone backend")
+
+	t.Cleanup(func() {
+		st.Close(testlogging.ContextForCleanup(t))
+	})
+
+	var tmp gather.WriteBuffer
+	defer tmp.Close()
+
+	err = st.GetBlob(ctx, blob.ID(uuid.New().String()), 0, -1, &tmp)
+	require.ErrorIs(t, err, blob.ErrBlobNotFound, "unexpected error when downloading non-existent blob")
+}
+
 func TestRCloneStorage(t *testing.T) {
 	t.Parallel()
 	testutil.ProviderTest(t)
@@ -72,8 +104,8 @@ func TestRCloneStorage(t *testing.T) {
 	dataDir := testutil.TempDirectory(t)
 
 	// use context that gets canceled after opening storage to ensure it's not used beyond New().
-	newctx, cancel := context.WithCancel(ctx)
-	st, err := rclone.New(newctx, &rclone.Options{
+	newCtx, cancel := context.WithCancel(ctx)
+	st, err := rclone.New(newCtx, &rclone.Options{
 		// pass local file as remote path.
 		RemotePath: dataDir,
 		RCloneExe:  rcloneExe,

repo/blob/sftp/sftp_storage.go

@@ -553,7 +553,9 @@ func New(ctx context.Context, opts *Options, isCreate bool) (blob.Storage, error
 
 	impl.rec = connection.NewReconnector(impl)
 
-	conn, err := impl.rec.GetOrOpenConnection(ctx)
+	// removing cancelation from ctx since ctx is likely to be canceled after
+	// New returns, causing the initial connection to be closed and not reused
+	conn, err := impl.rec.GetOrOpenConnection(context.WithoutCancel(ctx))
 	if err != nil {
 		return nil, errors.Wrap(err, "unable to open SFTP storage")
 	}

repo/blob/sftp/sftp_storage_test.go

@@ -212,8 +212,9 @@ func TestSFTPStorageValid(t *testing.T) {
 
 	t.Run("PasswordCreds", func(t *testing.T) {
 		ctx := testlogging.Context(t)
+		newctx, cancel := context.WithCancel(ctx)
 
-		st, err := createSFTPStorage(ctx, t, sftp.Options{
+		st, err := createSFTPStorage(newctx, t, sftp.Options{
 			Path:           "/upload2",
 			Host:           host,
 			Username:       sftpUsernameWithPasswordAuth,
@@ -226,6 +227,8 @@ func TestSFTPStorageValid(t *testing.T) {
 			t.Fatalf("unable to connect to SSH: %v", err)
 		}
 
+		cancel()
+
 		deleteBlobs(ctx, t, st)
 
 		blobtesting.VerifyStorage(ctx, t, st, blob.PutOptions{})