The following pull request was submitted through Github. It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/3420
This e-mail was sent by the LXC bot, direct replies will not reach the author unless they happen to be subscribed to this list. === Description (from pull-request) === Cancellation is not wired in the client yet, but it's possible to perform it via direct API call. An operation is now considered cancellable when the .Cancellable(true) is called, so that the cancel channel is set.
From 79e94873b7ca7f1f0308980106c95de04ca65cf6 Mon Sep 17 00:00:00 2001 From: Alberto Donato <[email protected]> Date: Wed, 14 Jun 2017 10:50:46 +0200 Subject: [PATCH 1/2] Cleanup. Signed-off-by: Alberto Donato <[email protected]> --- lxd/images.go | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/lxd/images.go b/lxd/images.go index 8cdb17009..64da6ad2d 100644 --- a/lxd/images.go +++ b/lxd/images.go @@ -720,35 +720,25 @@ func imagesPost(d *Daemon, r *http.Request) Response { // Setup the cleanup function defer cleanup(builddir, post) - if !imageUpload { + if imageUpload { + /* Processing image upload */ + info, err = getImgPostInfo(d, r, builddir, post) + } else { if req.Source.Type == "image" { /* Processing image copy from remote */ info, err = imgPostRemoteInfo(d, req, op) - if err != nil { - return err - } } else if req.Source.Type == "url" { /* Processing image copy from URL */ info, err = imgPostURLInfo(d, req, op) - if err != nil { - return err - } } else { /* Processing image creation from container */ imagePublishLock.Lock() info, err = imgPostContInfo(d, r, req, builddir) - if err != nil { - imagePublishLock.Unlock() - return err - } imagePublishLock.Unlock() } - } else { - /* Processing image upload */ - info, err = getImgPostInfo(d, r, builddir, post) - if err != nil { - return err - } + } + if err != nil { + return err } // Apply any provided alias From be1bb85f2f9a50a2e3ff2fd3bcbab9789f435ce2 Mon Sep 17 00:00:00 2001 From: Alberto Donato <[email protected]> Date: Thu, 15 Jun 2017 15:01:42 +0200 Subject: [PATCH 2/2] Add logic to cancel HTTP request. Signed-off-by: Alberto Donato <[email protected]> --- client/interfaces.go | 4 ++++ client/lxd_images.go | 4 +++- client/simplestreams_images.go | 8 ++++---- client/util.go | 8 +++++--- lxd/daemon_images.go | 5 ++++- lxd/operations.go | 25 ++++++++++++++++++++----- shared/operation/operation.go | 31 +++++++++++++++++++++++++++++++ test/suites/image.sh | 24 ++++++++++++++++++++++++ 8 files changed, 95 insertions(+), 14 deletions(-) create mode 100644 shared/operation/operation.go diff --git a/client/interfaces.go b/client/interfaces.go index 1d76196b5..dddac03d7 100644 --- a/client/interfaces.go +++ b/client/interfaces.go @@ -6,6 +6,7 @@ import ( "github.com/gorilla/websocket" "github.com/lxc/lxd/shared/api" + "github.com/lxc/lxd/shared/operation" ) // The Server type represents a generic read-only server. @@ -194,6 +195,9 @@ type ImageFileRequest struct { // Progress handler (called whenever some progress is made) ProgressHandler func(progress ProgressData) + + // The cancellable operation that's handling the request + Operation operation.CancellableOperation } // The ImageFileResponse struct is used as the response for image downloads diff --git a/client/lxd_images.go b/client/lxd_images.go index edc7a4530..b7310169a 100644 --- a/client/lxd_images.go +++ b/client/lxd_images.go @@ -16,6 +16,7 @@ import ( "github.com/lxc/lxd/shared" "github.com/lxc/lxd/shared/api" "github.com/lxc/lxd/shared/ioprogress" + "github.com/lxc/lxd/shared/operation" ) // Image handling functions @@ -118,10 +119,11 @@ func (r *ProtocolLXD) GetPrivateImageFile(fingerprint string, secret string, req } // Start the request - response, err := r.http.Do(request) + response, err, doneCh := operation.CancellableDownload(req.Operation, r.http, request) if err != nil { return nil, err } + defer close(doneCh) defer response.Body.Close() if response.StatusCode != http.StatusOK { diff --git a/client/simplestreams_images.go b/client/simplestreams_images.go index d84f8ed09..ef803105f 100644 --- a/client/simplestreams_images.go +++ b/client/simplestreams_images.go @@ -63,11 +63,11 @@ func (r *ProtocolSimpleStreams) GetImageFile(fingerprint string, req ImageFileRe // Try over http url := fmt.Sprintf("http://%s/%s";, strings.TrimPrefix(r.httpHost, "https://";), meta.Path) - size, err := downloadFileSha256(r.http, r.httpUserAgent, req.ProgressHandler, "metadata", url, meta.Sha256, req.MetaFile) + size, err := downloadFileSha256(req.Operation, r.http, r.httpUserAgent, req.ProgressHandler, "metadata", url, meta.Sha256, req.MetaFile) if err != nil { // Try over https url = fmt.Sprintf("%s/%s", r.httpHost, meta.Path) - size, err = downloadFileSha256(r.http, r.httpUserAgent, req.ProgressHandler, "metadata", url, meta.Sha256, req.MetaFile) + size, err = downloadFileSha256(req.Operation, r.http, r.httpUserAgent, req.ProgressHandler, "metadata", url, meta.Sha256, req.MetaFile) if err != nil { return nil, err } @@ -84,11 +84,11 @@ func (r *ProtocolSimpleStreams) GetImageFile(fingerprint string, req ImageFileRe // Try over http url := fmt.Sprintf("http://%s/%s";, strings.TrimPrefix(r.httpHost, "https://";), rootfs.Path) - size, err := downloadFileSha256(r.http, r.httpUserAgent, req.ProgressHandler, "rootfs", url, rootfs.Sha256, req.RootfsFile) + size, err := downloadFileSha256(req.Operation, r.http, r.httpUserAgent, req.ProgressHandler, "rootfs", url, rootfs.Sha256, req.RootfsFile) if err != nil { // Try over https url = fmt.Sprintf("%s/%s", r.httpHost, rootfs.Path) - size, err = downloadFileSha256(r.http, r.httpUserAgent, req.ProgressHandler, "rootfs", url, rootfs.Sha256, req.RootfsFile) + size, err = downloadFileSha256(req.Operation, r.http, r.httpUserAgent, req.ProgressHandler, "rootfs", url, rootfs.Sha256, req.RootfsFile) if err != nil { return nil, err } diff --git a/client/util.go b/client/util.go index 973f1d562..2b35e606b 100644 --- a/client/util.go +++ b/client/util.go @@ -10,6 +10,7 @@ import ( "github.com/lxc/lxd/shared" "github.com/lxc/lxd/shared/ioprogress" + "github.com/lxc/lxd/shared/operation" ) func tlsHTTPClient(tlsClientCert string, tlsClientKey string, tlsCA string, tlsServerCert string, proxy func(req *http.Request) (*url.URL, error)) (*http.Client, error) { @@ -81,7 +82,7 @@ func unixHTTPClient(path string) (*http.Client, error) { return &client, nil } -func downloadFileSha256(httpClient *http.Client, useragent string, progress func(progress ProgressData), filename string, url string, hash string, target io.WriteSeeker) (int64, error) { +func downloadFileSha256(op operation.CancellableOperation, httpClient *http.Client, useragent string, progress func(progress ProgressData), filename string, url string, hash string, target io.WriteSeeker) (int64, error) { // Always seek to the beginning target.Seek(0, 0) @@ -95,12 +96,13 @@ func downloadFileSha256(httpClient *http.Client, useragent string, progress func req.Header.Set("User-Agent", useragent) } - // Start the request - r, err := httpClient.Do(req) + // Perform the request + r, err, doneCh := operation.CancellableDownload(op, httpClient, req) if err != nil { return -1, err } defer r.Body.Close() + defer close(doneCh) if r.StatusCode != http.StatusOK { return -1, fmt.Errorf("Unable to fetch %s: %s", url, r.Status) diff --git a/lxd/daemon_images.go b/lxd/daemon_images.go index b2023d42f..dab78cc9a 100644 --- a/lxd/daemon_images.go +++ b/lxd/daemon_images.go @@ -19,6 +19,7 @@ import ( "github.com/lxc/lxd/shared/api" "github.com/lxc/lxd/shared/ioprogress" "github.com/lxc/lxd/shared/logger" + cancellable_op "github.com/lxc/lxd/shared/operation" "github.com/lxc/lxd/shared/version" log "gopkg.in/inconshreveable/log15.v2" @@ -385,6 +386,7 @@ func (d *Daemon) ImageDownload(op *operation, server string, protocol string, ce MetaFile: io.WriteSeeker(dest), RootfsFile: io.WriteSeeker(destRootfs), ProgressHandler: progress, + Operation: op, } if secret != "" { @@ -418,7 +420,8 @@ func (d *Daemon) ImageDownload(op *operation, server string, protocol string, ce req.Header.Set("User-Agent", version.UserAgent) // Make the request - raw, err := httpClient.Do(req) + raw, err, doneCh := cancellable_op.CancellableDownload(op, httpClient, req) + defer close(doneCh) if err != nil { return nil, err } diff --git a/lxd/operations.go b/lxd/operations.go index 0794bc5d0..d5aa21684 100644 --- a/lxd/operations.go +++ b/lxd/operations.go @@ -54,7 +54,8 @@ type operation struct { onConnect func(*operation, *http.Request, http.ResponseWriter) error // Channels used for error reporting and state tracking of background actions - chanDone chan error + chanDone chan error + chanCancel chan error // Locking for concurent access to the operation lock sync.Mutex @@ -153,17 +154,18 @@ func (op *operation) Cancel() (chan error, error) { return nil, fmt.Errorf("Only running operations can be cancelled") } + op.lock.Lock() if !op.mayCancel() { + op.lock.Unlock() return nil, fmt.Errorf("This operation can't be cancelled") } - chanCancel := make(chan error, 1) - - op.lock.Lock() oldStatus := op.status op.status = api.Cancelling + close(op.chanCancel) op.lock.Unlock() + chanCancel := make(chan error, 1) if op.onCancel != nil { go func(op *operation, oldStatus api.StatusCode, chanCancel chan error) { err := op.onCancel(op) @@ -244,7 +246,20 @@ func (op *operation) Connect(r *http.Request, w http.ResponseWriter) (chan error } func (op *operation) mayCancel() bool { - return op.onCancel != nil || op.class == operationClassToken + return op.chanCancel != nil || op.class == operationClassToken +} + +// Toggle whether the operation is cancellable. If `true` is passed, the +// channel to cancel the operation is returned, otherwise nil. +func (op *operation) Cancellable(flag bool) chan error { + var ch chan error + if flag { + ch = make(chan error) + } + op.lock.Lock() + op.chanCancel = ch + op.lock.Unlock() + return ch } func (op *operation) Render() (string, *api.Operation, error) { diff --git a/shared/operation/operation.go b/shared/operation/operation.go new file mode 100644 index 000000000..b0ee2c0d9 --- /dev/null +++ b/shared/operation/operation.go @@ -0,0 +1,31 @@ +package operation + +import ( + "net/http" +) + +// An operation that can be canceled. +type CancellableOperation interface { + + // Toggle whether the operation is cancellable + Cancellable(flag bool) chan error +} + +func CancellableDownload(op CancellableOperation, client *http.Client, req *http.Request) (*http.Response, error, chan bool) { + chDone := make(chan bool) + + go func() { + chCancel := op.Cancellable(true) + select { + case <-chCancel: + if transport, ok := client.Transport.(*http.Transport); ok { + transport.CancelRequest(req) + } + case <-chDone: + } + op.Cancellable(false) + }() + + resp, err := client.Do(req) + return resp, err, chDone +} diff --git a/test/suites/image.sh b/test/suites/image.sh index 7b5c19101..ecbc269a5 100644 --- a/test/suites/image.sh +++ b/test/suites/image.sh @@ -48,5 +48,29 @@ test_image_list_all_aliases() { # both aliases are listed if the "aliases" column is included in output lxc image list -c L | grep -q testimage lxc image list -c L | grep -q zzz +} +test_image_copy_interrupt() { + # shellcheck disable=2039,2153,2155 + local operation_id=$( + curl -s --unix-socket "${LXD_DIR}"/unix.socket lxd/1.0/images -d ' + { + "auto_update": false, + "public": false, + "source": { + "certificate": "", + "fingerprint": "x", + "mode": "pull", + "protocol": "simplestreams", + "server": "https://cloud-images.ubuntu.com/releases";, + "type": "image" + } + }' | jq -r .metadata.id) + sleep 1 + # cancel the operation and expect a success response + curl -s --unix-socket "${LXD_DIR}"/unix.socket -X DELETE \ + "lxd/1.0/operations/$operation_id" | grep -q 200 + [ "$(lxc image list --format=csv | wc -l)" = 0 ] + # Remove leftover files from image download + rm -rf "${LXD_DIR}/images" }
_______________________________________________ lxc-devel mailing list [email protected] http://lists.linuxcontainers.org/listinfo/lxc-devel
