Merge pull request #742 from Azure/dev

10.3.2 Release

ChangeLog.md

@@ -1,6 +1,24 @@
 
 # Change Log
 
+## Version 10.3.2
+
+### Bug fixes
+
+1. Jobs could not be cancelled while scanning was still in progress.
+1. Downloading large managed disks (8 TB and above) failed with errors.
+1. Downloading large page blobs might make no progress for the first 15 or 20 minutes.
+1. There was a rare error where the final output could under-report the total number of files in the job. That error has been fixed.
+1. When using JSON output mode, the output from the rm command on ADLS Gen2 was inconsistent with the output from other commands
+1. After authentication errors, files in progress were not cleaned up (deleted) at the destination. If there was an
+   authentication failure during a job (e.g. a SAS token expired while in use) this could result in files being left
+   behind that had incomplete contents (even though their size looked correct).
+1. The AUTO concurrency option, for automatically tuning concurrency as AzCopy runs, started working too late if scanning (aka enumeration) took a long time. This resulted in reduced throughput when using this setting.
+1. It was not possible to access the root of Windows drives with lowercase drive letters. E.g. d:\
+1. Service to Service transfers would fail when using environment variable to specify OAuth authentication.
+1. Certain errors parsing URLs were not reported clearly.
+1. When downloading to NUL (/dev/null on Linux), files of zero length no longer trigger errors. (Downloads to NUL can be used in performance testing and bulk MD5 checking.
+
 ## Version 10.3.1
 
 ### New features

azure-pipelines.yml

@@ -17,7 +17,7 @@ jobs:
     steps:
       - task: GoTool@0
         inputs:
-          version: '1.12'
+          version: '1.13'
       - script: |
           GOARCH=amd64 GOOS=linux go build -o "$(Build.ArtifactStagingDirectory)/azcopy_linux_amd64"
           GOARCH=amd64 GOOS=linux go build -tags "se_integration" -o "$(Build.ArtifactStagingDirectory)/azcopy_linux_se_amd64"
@@ -35,7 +35,7 @@ jobs:
     steps:
       - task: GoTool@0
         inputs:
-          version: '1.12'
+          version: '1.13'
       - script: |
           go build -o "$(Build.ArtifactStagingDirectory)/azcopy_darwin_amd64"
         displayName: 'Generate builds'
@@ -59,7 +59,7 @@ jobs:
       - task: GoTool@0
         name: 'Set_up_Golang'
         inputs:
-          version: '1.12'
+          version: '1.13'
       - script: |
           pip install azure-storage-blob==12.0.0b3
           # the recent release 1.0.0b4 has a breaking change

cmd/benchmark.go

@@ -160,7 +160,7 @@ func (raw rawBenchmarkCmdArgs) appendVirtualDir(target, virtualDir string) (stri
 
 	u, err := url.Parse(target)
 	if err != nil {
-		return "", fmt.Errorf("error parsing the url %s. Failed with error %s", u.String(), err.Error())
+		return "", fmt.Errorf("error parsing the url %s. Failed with error %s", target, err.Error())
 	}
 
 	var result url.URL

cmd/copy.go

@@ -852,6 +852,9 @@ func (cca *cookedCopyCmdArgs) processRedirectionUpload(blobUrl string, blockSize
 func (cca *cookedCopyCmdArgs) processCopyJobPartOrders() (err error) {
 	ctx := context.WithValue(context.TODO(), ste.ServiceAPIVersionOverride, ste.DefaultServiceApiVersion)
 
+	// Note: credential info here is only used by remove at the moment.
+	// TODO: Get the entirety of remove into the new copyEnumeratorInit script so we can remove this
+	//       and stop having two places in copy that we get credential info
 	// verifies credential type and initializes credential info.
 	// Note: Currently, only one credential type is necessary for source and destination.
 	// For upload&download, only one side need credential.
@@ -988,14 +991,7 @@ func (cca *cookedCopyCmdArgs) processCopyJobPartOrders() (err error) {
 
 	case common.EFromTo.BlobFSTrash():
 		// TODO merge with BlobTrash case
-		msg, err := removeBfsResources(cca)
-		if err == nil {
-			glcm.Exit(func(format common.OutputFormat) string {
-				return msg
-			}, common.EExitCode.Success())
-		}
-
-		return err
+		err = removeBfsResources(cca)
 
 	// TODO: Hide the File to Blob direction temporarily, as service support on-going.
 	// case common.EFromTo.FileBlob():

cmd/credentialUtil.go

@@ -258,7 +258,9 @@ type rawFromToInfo struct {
 }
 
 func getCredentialInfoForLocation(ctx context.Context, location common.Location, resource, resourceSAS string, isSource bool) (credInfo common.CredentialInfo, isPublic bool, err error) {
-	if credInfo.CredentialType = GetCredTypeFromEnvVar(); credInfo.CredentialType == common.ECredentialType.Unknown() {
+	if resourceSAS != "" {
+		credInfo.CredentialType = common.ECredentialType.Anonymous()
+	} else if credInfo.CredentialType = GetCredTypeFromEnvVar(); credInfo.CredentialType == common.ECredentialType.Unknown() {
 		switch location {
 		case common.ELocation.Local(), common.ELocation.Benchmark():
 			credInfo.CredentialType = common.ECredentialType.Anonymous()

cmd/removeEnumerator.go

@@ -22,6 +22,7 @@ package cmd
 
 import (
 	"context"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"net/url"
@@ -119,29 +120,29 @@ func (s *directoryStack) Pop() (*azfile.DirectoryURL, bool) {
 // TODO move after ADLS/Blob interop goes public
 // TODO this simple remove command is only here to support the scenario temporarily
 // Ultimately, this code can be merged into the newRemoveEnumerator
-func removeBfsResources(cca *cookedCopyCmdArgs) (successMessage string, err error) {
+func removeBfsResources(cca *cookedCopyCmdArgs) (err error) {
 	ctx := context.Background()
 
 	// return an error if the unsupported options are passed in
 	if len(cca.includePatterns)+len(cca.excludePatterns) > 0 {
-		return "", errors.New("include/exclude options are not supported")
+		return errors.New("include/exclude options are not supported")
 	}
 
 	// patterns are not supported
 	if strings.Contains(cca.source, "*") {
-		return "", errors.New("pattern matches are not supported in this command")
+		return errors.New("pattern matches are not supported in this command")
 	}
 
 	// create bfs pipeline
 	p, err := createBlobFSPipeline(ctx, cca.credentialInfo)
 	if err != nil {
-		return "", err
+		return err
 	}
 
 	// attempt to parse the source url
 	sourceURL, err := url.Parse(cca.source)
 	if err != nil {
-		return "", errors.New("cannot parse source URL")
+		return errors.New("cannot parse source URL")
 	}
 
 	// append the SAS query to the newly parsed URL
@@ -150,29 +151,82 @@ func removeBfsResources(cca *cookedCopyCmdArgs) (successMessage string, err erro
 	// parse the given source URL into parts, which separates the filesystem name and directory/file path
 	urlParts := azbfs.NewBfsURLParts(*sourceURL)
 
-	// list of files is given, record the parent path
-	parentPath := urlParts.DirectoryOrFilePath
-	successCount := 0
-
 	if cca.listOfFilesChannel == nil {
-		return removeSingleBfsResource(urlParts, p, ctx, cca.recursive)
+		successMsg, err := removeSingleBfsResource(urlParts, p, ctx, cca.recursive)
+		if err != nil {
+			return err
+		}
+
+		glcm.Exit(func(format common.OutputFormat) string {
+			if format == common.EOutputFormat.Json() {
+				summary := common.ListJobSummaryResponse{
+					JobStatus:          common.EJobStatus.Completed(),
+					TotalTransfers:     1,
+					TransfersCompleted: 1,
+					PercentComplete:    100,
+				}
+				jsonOutput, err := json.Marshal(summary)
+				common.PanicIfErr(err)
+				return string(jsonOutput)
+			}
+
+			return successMsg
+		}, common.EExitCode.Success())
+
+		// explicitly exit, since in our tests Exit might be mocked away
+		return nil
 	}
 
+	// list of files is given, record the parent path
+	parentPath := urlParts.DirectoryOrFilePath
+	successCount := uint32(0)
+	failedTransfers := make([]common.TransferDetail, 0)
+
 	// read from the list of files channel to find out what needs to be deleted.
 	childPath, ok := <-cca.listOfFilesChannel
 	for ; ok; childPath, ok = <-cca.listOfFilesChannel {
 		// remove the child path
 		urlParts.DirectoryOrFilePath = common.GenerateFullPath(parentPath, childPath)
 		successMessage, err := removeSingleBfsResource(urlParts, p, ctx, cca.recursive)
 		if err != nil {
+			// the specific error is not included in the details, since it doesn't have a field for full error message
+			failedTransfers = append(failedTransfers, common.TransferDetail{Src: childPath, TransferStatus: common.ETransferStatus.Failed()})
 			glcm.Info(fmt.Sprintf("Skipping %s due to error %s", childPath, err))
 		} else {
 			glcm.Info(successMessage)
 			successCount += 1
 		}
 	}
 
-	return fmt.Sprintf("Successfully removed %v entities.", successCount), nil
+	glcm.Exit(func(format common.OutputFormat) string {
+		if format == common.EOutputFormat.Json() {
+			status := common.EJobStatus.Completed()
+			if len(failedTransfers) > 0 {
+				status = common.EJobStatus.CompletedWithErrors()
+
+				// if nothing got deleted
+				if successCount == 0 {
+					status = common.EJobStatus.Failed()
+				}
+			}
+
+			summary := common.ListJobSummaryResponse{
+				JobStatus:          status,
+				TotalTransfers:     successCount + uint32(len(failedTransfers)),
+				TransfersCompleted: successCount,
+				TransfersFailed:    uint32(len(failedTransfers)),
+				PercentComplete:    100,
+				FailedTransfers:    failedTransfers,
+			}
+			jsonOutput, err := json.Marshal(summary)
+			common.PanicIfErr(err)
+			return string(jsonOutput)
+		}
+
+		return fmt.Sprintf("Successfully removed %v entities.", successCount)
+	}, common.EExitCode.Success())
+
+	return nil
 }
 
 // TODO move after ADLS/Blob interop goes public

cmd/root.go

@@ -25,6 +25,7 @@ import (
 	"context"
 	"net/url"
 	"os"
+	"runtime"
 	"strings"
 	"time"
 
@@ -57,6 +58,22 @@ var rootCmd = &cobra.Command{
 			return err
 		}
 
+		// warn Windows users re quoting (since our docs all use single quotes, but CMD needs double)
+		// Single ones just come through as part of the args, in CMD.
+		// Ideally, for usability, we'd ideally have this info come back in the result of url.Parse. But that's hard to
+		// arrange. So we check it here.
+		if runtime.GOOS == "windows" {
+			for _, a := range args {
+				a = strings.ToLower(a)
+				if strings.HasPrefix(a, "'http") { // note the single quote
+					glcm.Info("")
+					glcm.Info("*** When running from CMD, surround URLs with double quotes. Only using single quotes from PowerShell. ***")
+					glcm.Info("")
+					break
+				}
+			}
+		}
+
 		// currently, we only automatically do auto-tuning when benchmarking
 		preferToAutoTuneGRs := cmd == benchCmd // TODO: do we have a better way to do this than making benchCmd global?
 		providePerformanceAdvice := cmd == benchCmd

cmd/zc_enumerator.go

@@ -179,8 +179,6 @@ func initResourceTraverser(resource string, location common.Location, ctx *conte
 	if listofFilesChannel != nil {
 		sas := ""
 		if location.IsRemote() {
-			// note to future self: this will cause a merge conflict.
-			// rename source to resource and delete this comment.
 			var err error
 			resource, sas, err = SplitAuthTokenFromResource(resource, location)
 

cmd/zc_traverser_local_test.go

@@ -42,6 +42,8 @@ func (s *localTraverserTestSuite) TestCleanLocalPathForWindows(c *chk.C) {
 		`\\foo\bar\`:  `\\foo\bar`,  // network share
 		`C:\`:         `C:\`,        // special case, the slash after colon is actually required
 		`D:`:          `D:\`,        // special case, the slash after colon is actually required
+		`c:\`:         `c:\`,        // special case, the slash after colon is actually required
+		`c:`:          `c:\`,        // special case, the slash after colon is actually required
 	}
 
 	for orig, expected := range testCases {

common/LongPathHandler.go

@@ -36,19 +36,27 @@ import (
 
 // ToExtendedPath converts short paths to an extended path.
 func ToExtendedPath(short string) string {
+	// filepath.Abs has an issue where if the path is just the drive indicator of your CWD, it just returns the CWD. So, we append the / to show that yes, we really mean C: or whatever.
+	if runtime.GOOS == "windows" && len(short) == 2 && RootDriveRegex.MatchString(short) {
+		short += "/"
+	}
+
 	short, err := filepath.Abs(short)
 	PanicIfErr(err) //TODO: Handle errors better?
 
 	// ex. C:/dir/file.txt -> \\?\C:\dir\file.txt
 	// ex. \\share\dir\file.txt -> \\?\UNC\share\dir\file.txt
 	if runtime.GOOS == "windows" { // Only do this on Windows
-		if strings.HasPrefix(short, EXTENDED_PATH_PREFIX) {
+		if strings.HasPrefix(short, EXTENDED_PATH_PREFIX) { // already an extended path \\?\C:\folder\file.txt or \\?\UNC\sharename\folder\file.txt
 			return strings.Replace(short, `/`, `\`, -1) // Just ensure it has all backslashes-- Windows can't handle forward-slash anymore in this format.
-		} else if strings.HasPrefix(short, `\\`) {
+		} else if strings.HasPrefix(short, `\\`) { // this is a file share (//sharename/folder/file.txt)
 			// Steal the first backslash, and then append the prefix. Enforce \.
 			return strings.Replace(EXTENDED_UNC_PATH_PREFIX+short[1:], `/`, `\`, -1) // convert to extended UNC path
-		} else {
-			// Just append the prefix. Enforce \.
+		} else { // this is coming from a drive-- capitalize the drive prefix. (C:/folder/file.txt)
+			if len(short) >= 2 && RootDriveRegex.MatchString(short[:2]) {
+				short = strings.Replace(short, short[:2], strings.ToUpper(short[:2]), 1)
+			}
+			// Then append the prefix. Enforce \.
 			return strings.Replace(EXTENDED_PATH_PREFIX+short, `/`, `\`, -1) // Just append the prefix
 		}
 	}

common/LongPathHandler_test.go

@@ -10,6 +10,9 @@ var _ = chk.Suite(&pathHandlerSuite{})
 
 func (p *pathHandlerSuite) TestShortToLong(c *chk.C) {
 	if OS_PATH_SEPARATOR == `\` {
+		c.Assert(ToExtendedPath(`c:`), chk.Equals, `\\?\C:\`)
+		c.Assert(ToExtendedPath(`c:/`), chk.Equals, `\\?\C:\`)
+		c.Assert(ToExtendedPath(`c:/myPath`), chk.Equals, `\\?\C:\myPath`)
 		c.Assert(ToExtendedPath(`C:\myPath`), chk.Equals, `\\?\C:\myPath`)
 		c.Assert(ToExtendedPath(`\\myHost\myPath`), chk.Equals, `\\?\UNC\myHost\myPath`)
 		c.Assert(ToExtendedPath(`\\?\C:\myPath`), chk.Equals, `\\?\C:\myPath`)

common/version.go

@@ -1,6 +1,6 @@
 package common
 
-const AzcopyVersion = "10.3.1"
+const AzcopyVersion = "10.3.2"
 const UserAgent = "AzCopy/" + AzcopyVersion
 const S3ImportUserAgent = "S3Import " + UserAgent
 const BenchmarkUserAgent = "Benchmark " + UserAgent

common/writeThoughFile.go

@@ -32,7 +32,7 @@ import (
 // //myShare
 // //myShare/
 // demonstrated at: https://regexr.com/4mf6l
-var RootDriveRegex = regexp.MustCompile(`(^[A-Z]:\/?$)`)
+var RootDriveRegex = regexp.MustCompile(`(?i)(^[A-Z]:\/?$)`)
 var RootShareRegex = regexp.MustCompile(`(^\/\/[^\/]*\/?$)`)
 
 func CreateParentDirectoryIfNotExist(destinationPath string) error {

ste/JobsAdmin.go

@@ -115,7 +115,7 @@ func initJobsAdmin(appCtx context.Context, concurrency ConcurrencySettings, targ
 	cpuMon := common.NewNullCpuMonitor()
 	// One day, we might monitor CPU as the app runs in all cases (and report CPU as possible constraint like we do with disk).
 	// But for now, we only monitor it when tuning the GR pool size.
-	if concurrency.AutoTuneMainPool() && concurrency.CheckCpuWhenTuing.Value {
+	if concurrency.AutoTuneMainPool() && concurrency.CheckCpuWhenTuning.Value {
 		// let our CPU monitor self-calibrate BEFORE we start doing any real work TODO: remove if we switch to gopsutil
 		cpuMon = common.NewCalibratedCpuUsageMonitor()
 	}
@@ -341,6 +341,7 @@ func (ja *jobsAdmin) poolSizer(tuner ConcurrencyTuner) {
 	initialMonitoringInterval := time.Duration(4 * time.Second)
 	expandedMonitoringInterval := time.Duration(8 * time.Second)
 	throughputMonitoringInterval := initialMonitoringInterval
+	slowTuneCh := ja.poolSizingChannels.requestSlowTuneCh
 
 	// get initial pool size
 	targetConcurrency, reason := tuner.GetRecommendedConcurrency(-1, ja.cpuMonitor.CPUContentionExists())
@@ -368,10 +369,11 @@ func (ja *jobsAdmin) poolSizer(tuner ConcurrencyTuner) {
 			// worker has exited
 			actualConcurrency--
 			atomic.StoreInt32(&ja.atomicCurrentMainPoolSize, int32(actualConcurrency))
-		case <-ja.poolSizingChannels.requestSlowTuneCh:
+		case <-slowTuneCh:
 			// we've been asked to tune more slowly
 			// TODO: confirm we don't need this: expandedMonitoringInterval *= 2
 			throughputMonitoringInterval = expandedMonitoringInterval
+			slowTuneCh = nil // so we won't keep running this case at the expense of others)
 		case <-time.After(throughputMonitoringInterval):
 			if actualConcurrency == targetConcurrency { // scalebacks can take time. Don't want to do any tuning if actual is not yet aligned to target
 				bytesOnWire := ja.BytesOverWire()

ste/concurrency.go

@@ -115,8 +115,8 @@ type ConcurrencySettings struct {
 	MaxOpenDownloadFiles int
 	// TODO: consider whether we should also use this (renamed to( MaxOpenFiles) for uploads, somehow (see command above). Is there any actual value in that? Maybe only highly handle-constrained Linux environments?
 
-	// CheckCpuWhenTuing determines whether CPU usage should be taken into account when auto-tuning
-	CheckCpuWhenTuing *ConfiguredBool
+	// CheckCpuWhenTuning determines whether CPU usage should be taken into account when auto-tuning
+	CheckCpuWhenTuning *ConfiguredBool
 }
 
 // AutoTuneMainPool says whether the main pool size should by dynamically tuned
@@ -139,7 +139,7 @@ func NewConcurrencySettings(maxFileAndSocketHandles int, requestAutoTuneGRs bool
 		MaxMainPoolSize:            maxMainPoolSize,
 		TransferInitiationPoolSize: getTransferInitiationPoolSize(),
 		MaxOpenDownloadFiles:       getMaxOpenPayloadFiles(maxFileAndSocketHandles, maxMainPoolSize.Value),
-		CheckCpuWhenTuing:          getCheckCpuUsageWhenTuning(),
+		CheckCpuWhenTuning:          getCheckCpuUsageWhenTuning(),
 	}
 
 	// Set the max idle connections that we allow. If there are any more idle connections