[lxc-devel] [lxd/master] Fix remote images in projects

freeekanayaka on Github lxc-bot at linuxcontainers.org
Thu Oct 11 10:19:53 UTC 2018


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 746 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20181011/ddfc54ed/attachment.bin>
-------------- next part --------------
From b4d2bc4f83b0b830b91dc1b354425a17de0b2117 Mon Sep 17 00:00:00 2001
From: Free Ekanayaka <free.ekanayaka at canonical.com>
Date: Thu, 11 Oct 2018 10:55:06 +0200
Subject: [PATCH 1/3] Link an image to a project when downloading it to init a
 container

Signed-off-by: Free Ekanayaka <free.ekanayaka at canonical.com>
---
 lxd/containers_post.go    |  2 +-
 lxd/daemon_images.go      | 10 +++++-----
 lxd/daemon_images_test.go |  2 +-
 lxd/images.go             | 20 ++++++++++----------
 4 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/lxd/containers_post.go b/lxd/containers_post.go
index 031b1153ef..0594d7f174 100644
--- a/lxd/containers_post.go
+++ b/lxd/containers_post.go
@@ -112,7 +112,7 @@ func createFromImage(d *Daemon, project string, req *api.ContainersPost) Respons
 			}
 			info, err = d.ImageDownload(
 				op, req.Source.Server, req.Source.Protocol, req.Source.Certificate,
-				req.Source.Secret, hash, true, autoUpdate, "", true)
+				req.Source.Secret, hash, true, autoUpdate, "", true, project)
 			if err != nil {
 				return err
 			}
diff --git a/lxd/daemon_images.go b/lxd/daemon_images.go
index 6258273c3b..3198bf5389 100644
--- a/lxd/daemon_images.go
+++ b/lxd/daemon_images.go
@@ -96,7 +96,7 @@ func imageLoadStreamCache(d *Daemon) error {
 }
 
 // ImageDownload resolves the image fingerprint and if not in the database, downloads it
-func (d *Daemon) ImageDownload(op *operation, server string, protocol string, certificate string, secret string, alias string, forContainer bool, autoUpdate bool, storagePool string, preferCached bool) (*api.Image, error) {
+func (d *Daemon) ImageDownload(op *operation, server string, protocol string, certificate string, secret string, alias string, forContainer bool, autoUpdate bool, storagePool string, preferCached bool, project string) (*api.Image, error) {
 	var err error
 	var ctxMap log.Ctx
 
@@ -244,7 +244,7 @@ func (d *Daemon) ImageDownload(op *operation, server string, protocol string, ce
 	}
 
 	// Check if the image already exists (partial hash match)
-	_, imgInfo, err := d.cluster.ImageGet("default", fp, false, true)
+	_, imgInfo, err := d.cluster.ImageGet(project, fp, false, true)
 	if err == nil {
 		logger.Debug("Image already exists in the db", log.Ctx{"image": fp})
 		info = imgInfo
@@ -298,7 +298,7 @@ func (d *Daemon) ImageDownload(op *operation, server string, protocol string, ce
 		<-waitChannel
 
 		// Grab the database entry
-		_, imgInfo, err := d.cluster.ImageGet("default", fp, false, true)
+		_, imgInfo, err := d.cluster.ImageGet(project, fp, false, true)
 		if err != nil {
 			// Other download failed, lets try again
 			logger.Error("Other image download didn't succeed", log.Ctx{"image": fp})
@@ -519,7 +519,7 @@ func (d *Daemon) ImageDownload(op *operation, server string, protocol string, ce
 	}
 
 	// Create the database entry
-	err = d.cluster.ImageInsert("default", info.Fingerprint, info.Filename, info.Size, info.Public, info.AutoUpdate, info.Architecture, info.CreatedAt, info.ExpiresAt, info.Properties)
+	err = d.cluster.ImageInsert(project, info.Fingerprint, info.Filename, info.Size, info.Public, info.AutoUpdate, info.Architecture, info.CreatedAt, info.ExpiresAt, info.Properties)
 	if err != nil {
 		return nil, err
 	}
@@ -545,7 +545,7 @@ func (d *Daemon) ImageDownload(op *operation, server string, protocol string, ce
 
 	// Record the image source
 	if alias != fp {
-		id, _, err := d.cluster.ImageGet("default", fp, false, true)
+		id, _, err := d.cluster.ImageGet(project, fp, false, true)
 		if err != nil {
 			return nil, err
 		}
diff --git a/lxd/daemon_images_test.go b/lxd/daemon_images_test.go
index 91a5854053..fb5acf2bda 100644
--- a/lxd/daemon_images_test.go
+++ b/lxd/daemon_images_test.go
@@ -42,7 +42,7 @@ func (suite *daemonImagesTestSuite) TestUseCachedImagesIfAvailable() {
 	// one we created above.
 	op, err := operationCreate(suite.d.cluster, "default", operationClassTask, db.OperationImageDownload, map[string][]string{}, nil, nil, nil, nil)
 	suite.Req.Nil(err)
-	image, err := suite.d.ImageDownload(op, "img.srv", "simplestreams", "", "", "test", false, false, "", true)
+	image, err := suite.d.ImageDownload(op, "img.srv", "simplestreams", "", "", "test", false, false, "", true, "default")
 	suite.Req.Nil(err)
 	suite.Req.Equal("abcd", image.Fingerprint)
 }
diff --git a/lxd/images.go b/lxd/images.go
index d82c42f998..b8ccb2597d 100644
--- a/lxd/images.go
+++ b/lxd/images.go
@@ -274,7 +274,7 @@ func imgPostContInfo(d *Daemon, r *http.Request, req api.ImagesPost, builddir st
 	return &info, nil
 }
 
-func imgPostRemoteInfo(d *Daemon, req api.ImagesPost, op *operation) (*api.Image, error) {
+func imgPostRemoteInfo(d *Daemon, req api.ImagesPost, op *operation, project string) (*api.Image, error) {
 	var err error
 	var hash string
 
@@ -286,7 +286,7 @@ func imgPostRemoteInfo(d *Daemon, req api.ImagesPost, op *operation) (*api.Image
 		return nil, fmt.Errorf("must specify one of alias or fingerprint for init from image")
 	}
 
-	info, err := d.ImageDownload(op, req.Source.Server, req.Source.Protocol, req.Source.Certificate, req.Source.Secret, hash, false, req.AutoUpdate, "", false)
+	info, err := d.ImageDownload(op, req.Source.Server, req.Source.Protocol, req.Source.Certificate, req.Source.Secret, hash, false, req.AutoUpdate, "", false, project)
 	if err != nil {
 		return nil, err
 	}
@@ -312,7 +312,7 @@ func imgPostRemoteInfo(d *Daemon, req api.ImagesPost, op *operation) (*api.Image
 	return info, nil
 }
 
-func imgPostURLInfo(d *Daemon, req api.ImagesPost, op *operation) (*api.Image, error) {
+func imgPostURLInfo(d *Daemon, req api.ImagesPost, op *operation, project string) (*api.Image, error) {
 	var err error
 
 	if req.Source.URL == "" {
@@ -355,7 +355,7 @@ func imgPostURLInfo(d *Daemon, req api.ImagesPost, op *operation) (*api.Image, e
 	}
 
 	// Import the image
-	info, err := d.ImageDownload(op, url, "direct", "", "", hash, false, req.AutoUpdate, "", false)
+	info, err := d.ImageDownload(op, url, "direct", "", "", hash, false, req.AutoUpdate, "", false, project)
 	if err != nil {
 		return nil, err
 	}
@@ -697,10 +697,10 @@ func imagesPost(d *Daemon, r *http.Request) Response {
 		} else {
 			if req.Source.Type == "image" {
 				/* Processing image copy from remote */
-				info, err = imgPostRemoteInfo(d, req, op)
+				info, err = imgPostRemoteInfo(d, req, op, project)
 			} else if req.Source.Type == "url" {
 				/* Processing image copy from URL */
-				info, err = imgPostURLInfo(d, req, op)
+				info, err = imgPostURLInfo(d, req, op, project)
 			} else {
 				/* Processing image creation from container */
 				imagePublishLock.Lock()
@@ -883,7 +883,7 @@ func autoUpdateImages(ctx context.Context, d *Daemon) {
 		//        goroutine and simply abort when the context expires.
 		ch := make(chan struct{})
 		go func() {
-			autoUpdateImage(d, nil, id, info)
+			autoUpdateImage(d, nil, id, info, "default")
 			ch <- struct{}{}
 		}()
 		select {
@@ -898,7 +898,7 @@ func autoUpdateImages(ctx context.Context, d *Daemon) {
 
 // Update a single image.  The operation can be nil, if no progress tracking is needed.
 // Returns whether the image has been updated.
-func autoUpdateImage(d *Daemon, op *operation, id int, info *api.Image) error {
+func autoUpdateImage(d *Daemon, op *operation, id int, info *api.Image, project string) error {
 	fingerprint := info.Fingerprint
 	_, source, err := d.cluster.ImageSourceGet(id)
 	if err != nil {
@@ -941,7 +941,7 @@ func autoUpdateImage(d *Daemon, op *operation, id int, info *api.Image) error {
 	// Update the image on each pool where it currently exists.
 	hash := fingerprint
 	for _, poolName := range poolNames {
-		newInfo, err := d.ImageDownload(op, source.Server, source.Protocol, source.Certificate, "", source.Alias, false, true, poolName, false)
+		newInfo, err := d.ImageDownload(op, source.Server, source.Protocol, source.Certificate, "", source.Alias, false, true, poolName, false, project)
 
 		if err != nil {
 			logger.Error("Failed to update the image", log.Ctx{"err": err, "fp": fingerprint})
@@ -1870,7 +1870,7 @@ func imageRefresh(d *Daemon, r *http.Request) Response {
 
 	// Begin background operation
 	run := func(op *operation) error {
-		return autoUpdateImage(d, op, imageId, imageInfo)
+		return autoUpdateImage(d, op, imageId, imageInfo, project)
 	}
 
 	op, err := operationCreate(d.cluster, project, operationClassTask, db.OperationImageRefresh, nil, nil, run, nil, nil)

From 9100528e831daf76d22cc116eb5c8aab28b3b61a Mon Sep 17 00:00:00 2001
From: Free Ekanayaka <free.ekanayaka at canonical.com>
Date: Thu, 11 Oct 2018 12:01:56 +0200
Subject: [PATCH 2/3] Avoid downloading an image twice if it's already in
 another project

Signed-off-by: Free Ekanayaka <free.ekanayaka at canonical.com>
---
 lxd/daemon_images.go | 28 +++++++++++++++++++-
 lxd/db/images.go     | 62 ++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 84 insertions(+), 6 deletions(-)

diff --git a/lxd/daemon_images.go b/lxd/daemon_images.go
index 3198bf5389..3a3b22eff0 100644
--- a/lxd/daemon_images.go
+++ b/lxd/daemon_images.go
@@ -16,6 +16,7 @@ import (
 
 	"github.com/lxc/lxd/client"
 	"github.com/lxc/lxd/lxd/cluster"
+	"github.com/lxc/lxd/lxd/db"
 	"github.com/lxc/lxd/lxd/sys"
 	"github.com/lxc/lxd/lxd/util"
 	"github.com/lxc/lxd/shared"
@@ -243,8 +244,33 @@ func (d *Daemon) ImageDownload(op *operation, server string, protocol string, ce
 		}
 	}
 
-	// Check if the image already exists (partial hash match)
+	// Check if the image already exists in this project (partial hash match)
 	_, imgInfo, err := d.cluster.ImageGet(project, fp, false, true)
+	if err == db.ErrNoSuchObject {
+		// Check if the image already exists in some other project.
+		_, imgInfo, err = d.cluster.ImageGetFromAnyProject(fp)
+		if err == nil {
+			// We just need to insert the database data, no actual download necessary.
+			err = d.cluster.ImageInsert(
+				project, imgInfo.Fingerprint, imgInfo.Filename, imgInfo.Size, false,
+				imgInfo.AutoUpdate, imgInfo.Architecture, imgInfo.CreatedAt, imgInfo.ExpiresAt,
+				imgInfo.Properties)
+			if err != nil {
+				return nil, err
+			}
+
+			var id int
+			id, imgInfo, err = d.cluster.ImageGet(project, fp, false, true)
+			if err != nil {
+				return nil, err
+			}
+			err = d.cluster.ImageSourceInsert(id, server, protocol, certificate, alias)
+			if err != nil {
+				return nil, err
+			}
+		}
+	}
+
 	if err == nil {
 		logger.Debug("Image already exists in the db", log.Ctx{"image": fp})
 		info = imgInfo
diff --git a/lxd/db/images.go b/lxd/db/images.go
index 8089ace51e..11a8104416 100644
--- a/lxd/db/images.go
+++ b/lxd/db/images.go
@@ -340,6 +340,58 @@ SELECT COUNT(images.id)
 		}
 	}
 
+	err = c.imageFill(id, &image, create, expire, used, upload, arch)
+	if err != nil {
+		return -1, nil, errors.Wrapf(err, "Fill image details")
+	}
+
+	return id, &image, nil
+}
+
+// ImageGetFromAnyProject returns an image matching the given fingerprint, if
+// it exists in any project.
+func (c *Cluster) ImageGetFromAnyProject(fingerprint string) (int, *api.Image, error) {
+	var create, expire, used, upload *time.Time // These hold the db-returned times
+
+	// The object we'll actually return
+	image := api.Image{}
+	id := -1
+	arch := -1
+
+	// These two humongous things will be filled by the call to DbQueryRowScan
+	outfmt := []interface{}{&id, &image.Fingerprint, &image.Filename,
+		&image.Size, &image.Cached, &image.Public, &image.AutoUpdate, &arch,
+		&create, &expire, &used, &upload}
+
+	inargs := []interface{}{fingerprint}
+	query := `
+        SELECT
+            images.id, fingerprint, filename, size, cached, public, auto_update, architecture,
+            creation_date, expiry_date, last_use_date, upload_date
+        FROM images
+        WHERE fingerprint = ?
+        LIMIT 1`
+
+	err := dbQueryRowScan(c.db, query, inargs, outfmt)
+	if err != nil {
+		if err == sql.ErrNoRows {
+			return -1, nil, ErrNoSuchObject
+		}
+
+		return -1, nil, err // Likely: there are no rows for this fingerprint
+	}
+
+	err = c.imageFill(id, &image, create, expire, used, upload, arch)
+	if err != nil {
+		return -1, nil, errors.Wrapf(err, "Fill image details")
+	}
+
+	return id, &image, nil
+}
+
+// Fill extra image fields such as properties and alias. This is called after
+// fetching a single row from the images table.
+func (c *Cluster) imageFill(id int, image *api.Image, create, expire, used, upload *time.Time, arch int) error {
 	// Some of the dates can be nil in the DB, let's process them.
 	if create != nil {
 		image.CreatedAt = *create
@@ -367,11 +419,11 @@ SELECT COUNT(images.id)
 	// Get the properties
 	q := "SELECT key, value FROM images_properties where image_id=?"
 	var key, value, name, desc string
-	inargs = []interface{}{id}
-	outfmt = []interface{}{key, value}
+	inargs := []interface{}{id}
+	outfmt := []interface{}{key, value}
 	results, err := queryScan(c.db, q, inargs, outfmt)
 	if err != nil {
-		return -1, nil, err
+		return err
 	}
 
 	properties := map[string]string{}
@@ -389,7 +441,7 @@ SELECT COUNT(images.id)
 	outfmt = []interface{}{name, desc}
 	results, err = queryScan(c.db, q, inargs, outfmt)
 	if err != nil {
-		return -1, nil, err
+		return err
 	}
 
 	aliases := []api.ImageAlias{}
@@ -407,7 +459,7 @@ SELECT COUNT(images.id)
 		image.UpdateSource = &source
 	}
 
-	return id, &image, nil
+	return nil
 }
 
 // ImageLocate returns the address of an online node that has a local copy of

From 9db6680000db02d5f5b45ce4e9090023646f4025 Mon Sep 17 00:00:00 2001
From: Free Ekanayaka <free.ekanayaka at canonical.com>
Date: Thu, 11 Oct 2018 12:13:05 +0200
Subject: [PATCH 3/3] Auto-update images also in projects other than the
 default one

Signed-off-by: Free Ekanayaka <free.ekanayaka at canonical.com>
---
 lxd/images.go | 42 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 35 insertions(+), 7 deletions(-)

diff --git a/lxd/images.go b/lxd/images.go
index b8ccb2597d..7dd70da63f 100644
--- a/lxd/images.go
+++ b/lxd/images.go
@@ -861,16 +861,44 @@ func autoUpdateImagesTask(d *Daemon) (task.Func, task.Schedule) {
 func autoUpdateImages(ctx context.Context, d *Daemon) {
 	logger.Infof("Updating images")
 
-	images, err := d.cluster.ImagesGet("default", false)
+	projectNames := []string{}
+	err := d.cluster.Transaction(func(tx *db.ClusterTx) error {
+		projects, err := tx.ProjectList(db.ProjectFilter{})
+		if err != nil {
+			return err
+		}
+
+		for _, project := range projects {
+			projectNames = append(projectNames, project.Name)
+		}
+
+		return nil
+	})
 	if err != nil {
-		logger.Error("Unable to retrieve the list of images", log.Ctx{"err": err})
+		logger.Error("Unable to retrieve project names", log.Ctx{"err": err})
 		return
 	}
 
+	for _, project := range projectNames {
+		err := autoUpdateImagesInProject(ctx, d, project)
+		if err != nil {
+			logger.Errorf("Unable to update images for project %s: %v", project, err)
+		}
+	}
+
+	logger.Infof("Done updating images")
+}
+
+func autoUpdateImagesInProject(ctx context.Context, d *Daemon, project string) error {
+	images, err := d.cluster.ImagesGet(project, false)
+	if err != nil {
+		return errors.Wrap(err, "Unable to retrieve the list of images")
+	}
+
 	for _, fingerprint := range images {
-		id, info, err := d.cluster.ImageGet("default", fingerprint, false, true)
+		id, info, err := d.cluster.ImageGet(project, fingerprint, false, true)
 		if err != nil {
-			logger.Error("Error loading image", log.Ctx{"err": err, "fp": fingerprint})
+			logger.Error("Error loading image", log.Ctx{"err": err, "fp": fingerprint, "project": project})
 			continue
 		}
 
@@ -883,17 +911,17 @@ func autoUpdateImages(ctx context.Context, d *Daemon) {
 		//        goroutine and simply abort when the context expires.
 		ch := make(chan struct{})
 		go func() {
-			autoUpdateImage(d, nil, id, info, "default")
+			autoUpdateImage(d, nil, id, info, project)
 			ch <- struct{}{}
 		}()
 		select {
 		case <-ctx.Done():
-			return
+			return nil
 		case <-ch:
 		}
 	}
 
-	logger.Infof("Done updating images")
+	return nil
 }
 
 // Update a single image.  The operation can be nil, if no progress tracking is needed.


More information about the lxc-devel mailing list