[lxc-devel] [lxd/master] Support cancelling image copy operation

albertodonato on Github lxc-bot at linuxcontainers.org
Fri Jun 16 09:43:15 UTC 2017


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 518 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20170616/6e7ee1c2/attachment.bin>
-------------- next part --------------
From 79e94873b7ca7f1f0308980106c95de04ca65cf6 Mon Sep 17 00:00:00 2001
From: Alberto Donato <alberto.donato at canonical.com>
Date: Wed, 14 Jun 2017 10:50:46 +0200
Subject: [PATCH 1/2] Cleanup.

Signed-off-by: Alberto Donato <alberto.donato at canonical.com>
---
 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 <alberto.donato at canonical.com>
Date: Thu, 15 Jun 2017 15:01:42 +0200
Subject: [PATCH 2/2] Add logic to cancel HTTP request.

Signed-off-by: Alberto Donato <alberto.donato at canonical.com>
---
 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"
 }


More information about the lxc-devel mailing list