Merge pull request canonical#3318 from freeekanayaka/fix-regressed-im…

…age-auto-update

Fix regression in image auto-update logic

lxd/daemon_images.go

@@ -460,6 +460,13 @@ func (d *Daemon) ImageDownload(op *operation, server string, protocol string, ce
 	// Override visiblity
 	info.Public = false
 
+	// We want to enable auto-update only if we were passed an
+	// alias name, so we can figure when the associated
+	// fingerprint changes in the remote.
+	if alias != fp {
+		info.AutoUpdate = autoUpdate
+	}
+
 	// Create the database entry
 	err = dbImageInsert(d.db, info.Fingerprint, info.Filename, info.Size, info.Public, info.AutoUpdate, info.Architecture, info.CreatedAt, info.ExpiresAt, info.Properties)
 	if err != nil {
@@ -479,8 +486,6 @@ func (d *Daemon) ImageDownload(op *operation, server string, protocol string, ce
 		if err != nil {
 			return nil, err
 		}
-
-		info.AutoUpdate = autoUpdate
 	}
 
 	// Import into the requested storage pool

lxd/images.go

@@ -972,9 +972,13 @@ func autoUpdateImage(d *Daemon, op *operation, fingerprint string, id int, info
 			continue
 		}
 
-		err = doDeleteImageFromPool(d, fingerprint, poolName)
-		if err != nil {
-			logger.Error("Error deleting image", log.Ctx{"err": err, "fp": fingerprint})
+		// If we do have optimized pools, make sure we remove
+		// the volumes associated with the image.
+		if poolName != "" {
+			err = doDeleteImageFromPool(d, fingerprint, poolName)
+			if err != nil {
+				logger.Error("Error deleting image from pool", log.Ctx{"err": err, "fp": fingerprint})
+			}
 		}
 	}
 

test/main.sh

@@ -599,6 +599,7 @@ run_test test_remote_usage "remote usage"
 run_test test_basic_usage "basic usage"
 run_test test_security "security features"
 run_test test_image_expiry "image expiry"
+run_test test_image_auto_update "image auto-update"
 run_test test_concurrent_exec "concurrent exec"
 run_test test_concurrent "concurrent startup"
 run_test test_snapshots "container snapshots"

test/suites/image_auto_update.sh

@@ -0,0 +1,72 @@
+test_image_auto_update() {
+  # XXX this test appears to be flaky when running on Jenkins
+  # against the LVM backend. Needs further investigation.
+  # shellcheck disable=2153
+  backend=$(storage_backend "$LXD_DIR")
+  if [ "${backend}" = "lvm" ]; then
+      return 0
+  fi
+
+  if lxc image alias list | grep -q "^| testimage\s*|.*$"; then
+      lxc image delete testimage
+  fi
+
+  # shellcheck disable=2039
+  local LXD2_DIR LXD2_ADDR
+  LXD2_DIR=$(mktemp -d -p "${TEST_DIR}" XXX)
+  chmod +x "${LXD2_DIR}"
+  spawn_lxd "${LXD2_DIR}" true
+  LXD2_ADDR=$(cat "${LXD2_DIR}/lxd.addr")
+
+  (LXD_DIR=${LXD2_DIR} deps/import-busybox --alias testimage --public)
+  fp1=$(LXD_DIR=${LXD2_DIR} lxc image info testimage | awk -F: '/^Fingerprint/ { print $2 }' | awk '{ print $1 }')
+
+  lxc remote add l2 "${LXD2_ADDR}" --accept-certificate --password foo
+  lxc init l2:testimage c1
+
+  # Now the first image image is in the local store, since it was
+  # downloaded to create c1.
+  alias=$(lxc image info "${fp1}" | awk -F: '/^    Alias/ { print $2 }' | awk '{ print $1 }')
+  [ "${alias}" = "testimage" ]
+
+  # Delete the first image from the remote store and replace it with a
+  # new one with a different fingerprint (passing "--template create"
+  # will do that).
+  (LXD_DIR=${LXD2_DIR} lxc image delete testimage)
+  (LXD_DIR=${LXD2_DIR} deps/import-busybox --alias testimage --public --template create)
+  fp2=$(LXD_DIR=${LXD2_DIR} lxc image info testimage | awk -F: '/^Fingerprint/ { print $2 }' | awk '{ print $1 }')
+  [ "${fp1}" != "${fp2}" ]
+
+  # Restart the server to force an image refresh immediately
+  # shellcheck disable=2153
+  shutdown_lxd "${LXD_DIR}"
+  respawn_lxd "${LXD_DIR}"
+
+  # Check that the first image got deleted from the local storage
+  #
+  # XXX: Since the auto-update logic runs asynchronously we need to wait
+  #      a little bit before it actually completes.
+  retries=10
+  while [ "${retries}" != "0" ]; do
+    if lxc image info "${fp1}" > /dev/null 2>&1; then
+	sleep 2
+	retries=$((retries-1))
+	continue
+    fi
+    break
+  done
+
+  if [ "${retries}" -eq 0 ]; then
+      echo "First image ${fp1} not deleted from local store"
+      return 1
+  fi
+
+  # The second image replaced the first one in the local storage.
+  alias=$(lxc image info "${fp2}" | awk -F: '/^    Alias/ { print $2 }' | awk '{ print $1 }')
+  [ "${alias}" = "testimage" ]
+
+  lxc delete c1
+  lxc remote remove l2
+  lxc image delete "${fp2}"
+  kill_lxd "$LXD2_DIR"
+}