Merge pull request #13296 from gabrielmougard/fix/set-cached-false-pr…

…e-existing-image

lxd/image: set `Cached: false` for pre-existing cached image

lxd/daemon_images.go

@@ -45,6 +45,7 @@ type ImageDownloadArgs struct {
 	StoragePool       string
 	Budget            int64
 	SourceProjectName string
+	UserRequested     bool
 }
 
 // imageOperationLock acquires a lock for operating on an image and returns the unlock function.
@@ -252,32 +253,45 @@ func ImageDownload(r *http.Request, s *state.State, op *operations.Operation, ar
 		ctxMap = logger.Ctx{"fingerprint": info.Fingerprint}
 		logger.Debug("Image already exists in the DB", ctxMap)
 
-		// If not requested in a particular pool, we're done.
-		if args.StoragePool == "" {
-			return info, nil
-		}
-
-		ctxMap["pool"] = args.StoragePool
-
 		var poolID int64
 		var poolIDs []int64
-
 		err = s.DB.Cluster.Transaction(context.TODO(), func(ctx context.Context, tx *db.ClusterTx) error {
-			// Get the ID of the target storage pool.
-			poolID, err = tx.GetStoragePoolID(ctx, args.StoragePool)
-			if err != nil {
-				return err
+			// If the image already exists, is cached and that it is
+			// requested to be downloaded from an explicit `image copy` operation, then disable its `cache` parameter
+			// so that it won't be candidate for auto removal.
+			if imgInfo.Cached && args.UserRequested {
+				err = tx.UnsetImageCached(ctx, args.ProjectName, imgInfo.Fingerprint)
+				if err != nil {
+					return err
+				}
 			}
 
-			// Check if the image is already in the pool.
-			poolIDs, err = tx.GetPoolsWithImage(ctx, info.Fingerprint)
+			if args.StoragePool != "" {
+				ctxMap["pool"] = args.StoragePool
 
-			return err
+				// Get the ID of the target storage pool.
+				poolID, err = tx.GetStoragePoolID(ctx, args.StoragePool)
+				if err != nil {
+					return err
+				}
+
+				// Check if the image is already in the pool.
+				poolIDs, err = tx.GetPoolsWithImage(ctx, info.Fingerprint)
+
+				return err
+			}
+
+			return nil
 		})
 		if err != nil {
 			return nil, err
 		}
 
+		// If not requested in a particular pool, we're done.
+		if args.StoragePool == "" {
+			return info, nil
+		}
+
 		if shared.ValueInSlice(poolID, poolIDs) {
 			logger.Debug("Image already exists on storage pool", ctxMap)
 			return info, nil

lxd/db/images.go

@@ -773,6 +773,15 @@ func (c *ClusterTx) SetImageCachedAndLastUseDate(ctx context.Context, projectNam
 	return err
 }
 
+// UnsetImageCached unsets the cached field of the image with the given fingerprint.
+func (c *ClusterTx) UnsetImageCached(ctx context.Context, projectName string, fingerprint string) error {
+	stmt := `UPDATE images SET cached=0 WHERE fingerprint=? AND project_id = (SELECT id FROM projects WHERE name = ? LIMIT 1)`
+
+	_, err := c.tx.ExecContext(ctx, stmt, fingerprint, projectName)
+
+	return err
+}
+
 // UpdateImage updates the image with the given ID.
 func (c *ClusterTx) UpdateImage(ctx context.Context, id int, fname string, sz int64, public bool, autoUpdate bool, architecture string, createdAt time.Time, expiresAt time.Time, properties map[string]string, project string, profileIDs []int64) error {
 	arch, err := osarch.ArchitectureId(architecture)

lxd/images.go

@@ -427,6 +427,7 @@ func imgPostRemoteInfo(s *state.State, r *http.Request, req api.ImagesPost, op *
 		ProjectName:       project,
 		Budget:            budget,
 		SourceProjectName: req.Source.Project,
+		UserRequested:     true,
 	})
 	if err != nil {
 		return nil, err
@@ -529,13 +530,14 @@ func imgPostURLInfo(s *state.State, r *http.Request, req api.ImagesPost, op *ope
 
 	// Import the image
 	info, err := ImageDownload(r, s, op, &ImageDownloadArgs{
-		Server:      url,
-		Protocol:    "direct",
-		Alias:       hash,
-		AutoUpdate:  req.AutoUpdate,
-		Public:      req.Public,
-		ProjectName: project,
-		Budget:      budget,
+		Server:        url,
+		Protocol:      "direct",
+		Alias:         hash,
+		AutoUpdate:    req.AutoUpdate,
+		Public:        req.Public,
+		ProjectName:   project,
+		Budget:        budget,
+		UserRequested: true,
 	})
 	if err != nil {
 		return nil, err

test/suites/remote.sh

@@ -339,6 +339,36 @@ test_remote_usage() {
   lxc_remote image delete "lxd2:${sum}"
   lxc_remote profile delete lxd2:foo
 
+  lxc_remote image copy localhost:testimage lxd2: --alias bar
+  # Get the `cached` and `aliases` fields for the image `bar` in lxd2
+  cached=$(lxc_remote image info lxd2:bar | awk '/Cached/ { print $2 }')
+  alias=$(lxc_remote image info lxd2:bar | grep -A 1 "Aliases:" | tail -n1 | awk '{print $2}')
+
+  # Check that image is not cached
+  [ "${cached}" = "no" ]
+  # Check that the alias is correct
+  [ "${alias}" = "bar" ]
+
+  # Now, lets delete the image and observe that when its downloaded implicitly as part of an instance create,
+  # the image becomes `cached` and has no alias.
+  fingerprint=$(lxc_remote image info lxd2:bar | awk '/Fingerprint/ { print $2 }')
+  lxc_remote image delete lxd2:bar
+  lxc_remote init localhost:testimage lxd2:c1
+  cached=$(lxc_remote image info "lxd2:${fingerprint}" | awk '/Cached/ { print $2 }')
+  # The `cached` field should be set to `yes` since the image was implicitly downloaded by the instance create operation
+  [ "${cached}" = "yes" ]
+  # There should be no alias for the image
+  ! lxc_remote image info "lxd2:${fingerprint}" | grep -q "Aliases:"
+
+  # Finally, lets copy the remote image explicitly to the local server with an alias like we did before
+  lxc_remote image copy localhost:testimage lxd2: --alias bar
+  cached=$(lxc_remote image info lxd2:bar | awk '/Cached/ { print $2 }')
+  alias=$(lxc_remote image info lxd2:bar | grep -A 1 "Aliases:" | tail -n1 | awk '{print $2}')
+  # The `cached` field should be set to `no` since the image was explicitly copied.
+  [ "${cached}" = "no" ]
+  # The alias should be set to `bar`.
+  [ "${alias}" = "bar" ]
+
   lxc_remote image alias delete localhost:foo
 
   lxc_remote remote remove lxd2