Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added new option to cancel and ignore completed job error, and output a summary instead #2519

Merged
merged 11 commits into from
Jan 18, 2024
21 changes: 16 additions & 5 deletions cmd/cancel.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@ package cmd
import (
"errors"
"fmt"

"github.com/Azure/azure-storage-azcopy/v10/common"
"github.com/spf13/cobra"
)

// TODO should this command be removed? Previously AzCopy was supposed to have an independent backend (out of proc)
// TODO but that's not the plan anymore
type rawCancelCmdArgs struct {
jobID string
jobID string
ignoreCompletedJobError bool
}

func (raw rawCancelCmdArgs) cook() (cookedCancelCmdArgs, error) {
Expand All @@ -42,11 +42,12 @@ func (raw rawCancelCmdArgs) cook() (cookedCancelCmdArgs, error) {
return cookedCancelCmdArgs{}, fmt.Errorf("invalid jobId string passed: %q", raw.jobID)
}

return cookedCancelCmdArgs{jobID: jobID}, nil
return cookedCancelCmdArgs{jobID: jobID, ignoreCompletedJobError: raw.ignoreCompletedJobError}, nil
}

type cookedCancelCmdArgs struct {
jobID common.JobID
jobID common.JobID
ignoreCompletedJobError bool
}

// handles the cancel command
Expand All @@ -55,6 +56,14 @@ func (cca cookedCancelCmdArgs) process() error {
var cancelJobResponse common.CancelPauseResumeResponse
Rpc(common.ERpcCmd.CancelJob(), cca.jobID, &cancelJobResponse)
if !cancelJobResponse.CancelledPauseResumed {
if cca.ignoreCompletedJobError && cancelJobResponse.JobStatus == common.EJobStatus.Completed() {
glcm.Info(cancelJobResponse.ErrorMsg)
resp := common.ListJobSummaryResponse{}
rpcCmd := common.ERpcCmd.ListJobSummary()
Rpc(rpcCmd, &cca.jobID, &resp)
PrintJobProgressSummary(resp)
return nil
}
return errors.New(cancelJobResponse.ErrorMsg)
}
return nil
Expand Down Expand Up @@ -88,7 +97,7 @@ func init() {

err = cooked.process()
if err != nil {
glcm.Error("failed to perform copy command due to error " + err.Error())
glcm.Error("failed to perform cancel command due to error " + err.Error())
}

glcm.Exit(nil, common.EExitCode.Success())
Expand All @@ -98,4 +107,6 @@ func init() {
Hidden: true,
}
rootCmd.AddCommand(cancelCmd)

cancelCmd.PersistentFlags().BoolVar(&raw.ignoreCompletedJobError, "ignore-error-if-completed", false, "")
}
13 changes: 7 additions & 6 deletions common/rpc-models.go
Original file line number Diff line number Diff line change
Expand Up @@ -339,14 +339,14 @@ type ListJobTransfersRequest struct {
}

type ResumeJobRequest struct {
JobID JobID
SourceSAS string
DestinationSAS string
JobID JobID
SourceSAS string
DestinationSAS string
SrcServiceClient *ServiceClient
DstServiceClient *ServiceClient
IncludeTransfer map[string]int
ExcludeTransfer map[string]int
CredentialInfo CredentialInfo
IncludeTransfer map[string]int
ExcludeTransfer map[string]int
CredentialInfo CredentialInfo
}

// represents the Details and details of a single transfer
Expand All @@ -362,6 +362,7 @@ type TransferDetail struct {
type CancelPauseResumeResponse struct {
ErrorMsg string
CancelledPauseResumed bool
JobStatus JobStatus
}

// represents the list of Details and details of number of transfers
Expand Down
12 changes: 8 additions & 4 deletions e2etest/declarativeHelpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,9 @@ type params struct {
includeAttributes string
excludePath string
excludePattern string
excludeAttributes string
forceIfReadOnly bool
capMbps float32
excludeAttributes string
forceIfReadOnly bool
capMbps float32
blockSizeMB float32
deleteDestination common.DeleteDestination // Manual validation is needed.
s2sSourceChangeValidation bool
Expand Down Expand Up @@ -179,7 +179,7 @@ type params struct {
destNull bool

disableParallelTesting bool
trailingDot common.TrailingDotOption
trailingDot common.TrailingDotOption
// looks like this for a folder transfer:
/*
INFO: source: /New folder/New Text Document.txt dest: /Test/New folder/New Text Document.txt
Expand All @@ -189,6 +189,9 @@ type params struct {
/*
INFO: source: dest: /New Text Document.txt
*/

// Options for cancel
ignoreErrorIfCompleted bool
}

// we expect folder transfers to be allowed (between folder-aware resources) if there are no filters that act at file level
Expand All @@ -208,6 +211,7 @@ func (Operation) Sync() Operation { return Operation(1 << 1) }
func (Operation) CopyAndSync() Operation { return eOperation.Copy() | eOperation.Sync() }
func (Operation) Remove() Operation { return Operation(1 << 2) }
func (Operation) Resume() Operation { return Operation(1 << 7) } // Resume should only ever be combined with Copy or Sync, and is a mid-job cancel/resume.
func (Operation) Cancel() Operation { return Operation(1 << 3) }

func (o Operation) String() string {
return enum.StringInt(o, reflect.TypeOf(o))
Expand Down
3 changes: 2 additions & 1 deletion e2etest/declarativeRunner.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ func RunScenarios(
// construct all the scenarios
scenarios := make([]scenario, 0)
for _, op := range operations.getValues() {
if op == eOperation.Resume() {
if op == eOperation.Resume() || op == eOperation.Cancel() {
continue
}

Expand Down Expand Up @@ -180,6 +180,7 @@ func RunScenarios(
hs: hsToUse,
fs: fs.DeepCopy(),
needResume: operations&eOperation.Resume() != 0,
needCancel: operations&eOperation.Cancel() != 0,
stripTopDir: p.stripTopDir,
}

Expand Down
33 changes: 33 additions & 0 deletions e2etest/declarativeScenario.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ type scenario struct {
a asserter
state scenarioState // TODO: does this really need to be a separate struct?
needResume bool
needCancel bool
chToStdin chan string
}

Expand Down Expand Up @@ -152,6 +153,14 @@ func (s *scenario) Run() {
return // resume failed. No point in running validation
}

// cancel if needed
if s.needCancel {
s.cancelAzCopy(azcopyDir)
}
if s.a.Failed() {
return // resume failed. No point in running validation
}

// check
s.validateTransferStates(azcopyDir)
if s.a.Failed() {
Expand Down Expand Up @@ -330,6 +339,30 @@ func (s *scenario) runAzCopy(logDirectory string) {
s.state.result = &result
}

func (s *scenario) cancelAzCopy(logDir string) {
r := newTestRunner()
r.SetAllFlags(s.p, eOperation.Cancel())

afterStart := func() string { return "" }
result, wasClean, err := r.ExecuteAzCopyCommand(
eOperation.Cancel(),
s.state.result.jobID.String(),
"",
false,
false,
s.fromTo,
afterStart,
s.chToStdin,
logDir,
)

if !wasClean {
s.a.AssertNoErr(err, "running AzCopy")
}

s.state.result = &result
}

func (s *scenario) resumeAzCopy(logDir string) {
s.chToStdin = make(chan string) // unubuffered seems the most predictable for our usages
defer close(s.chToStdin)
Expand Down
64 changes: 36 additions & 28 deletions e2etest/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,34 +80,36 @@ func (t *TestRunner) SetAllFlags(p params, o Operation) {
}

// TODO: TODO: nakulkar-msft there will be many more to add here
set("recursive", p.recursive, false)
set("as-subdir", !p.invertedAsSubdir, true)
set("include-path", p.includePath, "")
set("exclude-path", p.excludePath, "")
set("include-pattern", p.includePattern, "")
set("exclude-pattern", p.excludePattern, "")
set("include-after", p.includeAfter, "")
set("include-pattern", p.includePattern, "")
set("exclude-path", p.excludePath, "")
set("exclude-pattern", p.excludePattern, "")
set("cap-mbps", p.capMbps, float32(0))
set("block-size-mb", p.blockSizeMB, float32(0))
set("s2s-detect-source-changed", p.s2sSourceChangeValidation, false)
set("metadata", p.metadata, "")
set("cancel-from-stdin", p.cancelFromStdin, false)
set("preserve-smb-info", p.preserveSMBInfo, nil)
set("preserve-smb-permissions", p.preserveSMBPermissions, false)
set("backup", p.backupMode, false)
set("blob-tags", p.blobTags, "")
set("blob-type", p.blobType, "")
set("s2s-preserve-blob-tags", p.s2sPreserveBlobTags, false)
set("cpk-by-name", p.cpkByName, "")
set("cpk-by-value", p.cpkByValue, false)
set("is-object-dir", p.isObjectDir, false)
set("debug-skip-files", strings.Join(p.debugSkipFiles, ";"), "")
set("check-md5", p.checkMd5.String(), "FailIfDifferent")
set("trailing-dot", p.trailingDot.String(), "Enable")
set("force-if-read-only", p.forceIfReadOnly, false)
if o != eOperation.Cancel() {
siminsavani-msft marked this conversation as resolved.
Show resolved Hide resolved
set("recursive", p.recursive, false)
set("as-subdir", !p.invertedAsSubdir, true)
set("include-path", p.includePath, "")
set("exclude-path", p.excludePath, "")
set("include-pattern", p.includePattern, "")
set("exclude-pattern", p.excludePattern, "")
set("include-after", p.includeAfter, "")
set("include-pattern", p.includePattern, "")
set("exclude-path", p.excludePath, "")
set("exclude-pattern", p.excludePattern, "")
set("cap-mbps", p.capMbps, float32(0))
set("block-size-mb", p.blockSizeMB, float32(0))
set("s2s-detect-source-changed", p.s2sSourceChangeValidation, false)
set("metadata", p.metadata, "")
set("cancel-from-stdin", p.cancelFromStdin, false)
set("preserve-smb-info", p.preserveSMBInfo, nil)
set("preserve-smb-permissions", p.preserveSMBPermissions, false)
set("backup", p.backupMode, false)
set("blob-tags", p.blobTags, "")
set("blob-type", p.blobType, "")
set("s2s-preserve-blob-tags", p.s2sPreserveBlobTags, false)
set("cpk-by-name", p.cpkByName, "")
set("cpk-by-value", p.cpkByValue, false)
set("is-object-dir", p.isObjectDir, false)
set("debug-skip-files", strings.Join(p.debugSkipFiles, ";"), "")
set("check-md5", p.checkMd5.String(), "FailIfDifferent")
set("trailing-dot", p.trailingDot.String(), "Enable")
set("force-if-read-only", p.forceIfReadOnly, false)
}

if o == eOperation.Copy() {
set("s2s-preserve-access-tier", p.s2sPreserveAccessTier, true)
Expand All @@ -125,6 +127,8 @@ func (t *TestRunner) SetAllFlags(p params, o Operation) {
set("compare-hash", p.compareHash.String(), "None")
set("local-hash-storage-mode", p.hashStorageMode.String(), common.EHashStorageMode.Default().String())
set("hash-meta-dir", p.hashStorageDir, "")
} else if o == eOperation.Cancel() {
set("ignore-error-if-completed", p.ignoreErrorIfCompleted, "")
}
}

Expand Down Expand Up @@ -231,6 +235,8 @@ func (t *TestRunner) ExecuteAzCopyCommand(operation Operation, src, dst string,
verb = "remove"
case eOperation.Resume():
verb = "jobs resume"
case eOperation.Cancel():
verb = "cancel"
default:
panic("unsupported operation type")
}
Expand All @@ -240,6 +246,8 @@ func (t *TestRunner) ExecuteAzCopyCommand(operation Operation, src, dst string,
args = args[:2]
} else if operation == eOperation.Resume() {
args = args[:3]
} else if operation == eOperation.Cancel() {
args = args[:2]
}
args = append(args, t.computeArgs()...)
if needsFromTo {
Expand Down
61 changes: 61 additions & 0 deletions e2etest/zt_cancel_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// Copyright © Microsoft <wastore@microsoft.com>
//
// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files (the "Software"), to deal
// in the Software without restriction, including without limitation the rights
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
// copies of the Software, and to permit persons to whom the Software is
// furnished to do so, subject to the following conditions:
//
// The above copyright notice and this permission notice shall be included in
// all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.

package e2etest

import (
"github.com/Azure/azure-storage-azcopy/v10/common"
"testing"
)

func TestCancel_CompletedJobCopySync(t *testing.T) {
RunScenarios(t, eOperation.CopyAndSync()|eOperation.Cancel(), eTestFromTo.Other(common.EFromTo.BlobBlob()), eValidate.Auto(), anonymousAuthOnly, anonymousAuthOnly, params{
recursive: true,
ignoreErrorIfCompleted: true,
}, nil, testFiles{
defaultSize: "1K",
shouldTransfer: []interface{}{
folder(""),
f("file1.txt"),
},
}, EAccountType.Standard(), EAccountType.Standard(), "")
}

func TestCancel_CompletedJobRemove(t *testing.T) {
blobRemove := TestFromTo{
desc: "BlobRemove",
useAllTos: true,
froms: []common.Location{
common.ELocation.Blob(),
},
tos: []common.Location{
common.ELocation.Unknown(),
},
}
RunScenarios(t, eOperation.Remove()|eOperation.Cancel(), blobRemove, eValidate.Auto(), anonymousAuthOnly, anonymousAuthOnly, params{
ignoreErrorIfCompleted: true,
}, nil, testFiles{
defaultSize: "1K",
shouldTransfer: []interface{}{
folder(""),
f("file1.txt"),
},
}, EAccountType.Standard(), EAccountType.Standard(), "")
}