[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