The following pull request was submitted through Github. It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/3318
This e-mail was sent by the LXC bot, direct replies will not reach the author unless they happen to be subscribed to this list. === Description (from pull-request) === From what I understand, it seems that this commit: https://github.com/lxc/lxd/commit/7d4869527f1cef579015a06258c93a33e76cc865 introduced a regression in the auto-update logic. The code that was setting the AutoUpdate flag in the *api.Image structure in line 541: https://github.com/lxc/lxd/commit/7d4869527f1cef579015a06258c93a33e76cc865#diff-6182827642cd00e061c6ffe52891acc5L541 was moved down in the function: https://github.com/lxc/lxd/commit/7d4869527f1cef579015a06258c93a33e76cc865#diff-6182827642cd00e061c6ffe52891acc5R482 however the new location is after the call to dbImageInsert: ``` err = dbImageInsert(d.db, info.Fingerprint, info.Filename, info.Size, info.Public, info.AutoUpdate, info.Architecture, info.CreatedAt, info.ExpiresAt, info.Properties) ``` that actually makes use of such setting. This results in the auto_update column in the database always being 0, no matter what, and the server logic to auto-update images will never kick in. This commit should also fix another (non fatal) regression introduced by: https://github.com/lxc/lxd/commit/806d55c88562ab82b32d2d8563c20a8ec9bdac98 where appending the empty string "" to the poolNames to be processed ends up triggering a call to ```doDeleteImageFromPool()``` from ``autoUpdateImage``: https://github.com/lxc/lxd/blob/471604191b9793ffe479036dab8e527cb743cee1/lxd/images.go#L975 which as far as I understand will always end up with an error in case poolName is the empty string, so a spurious error will be logged. Note that auto-update regression affects the latest 2.13 release, but not 2.12. Signed-off-by: Free Ekanayaka <[email protected]>
From e51906bbe8bf8e1439d9b3ead18e85498e14affc Mon Sep 17 00:00:00 2001 From: Free Ekanayaka <[email protected]> Date: Mon, 15 May 2017 08:37:03 +0200 Subject: [PATCH] Fix regression in image auto-update logic From what I understand, it seems that this commit: https://github.com/lxc/lxd/commit/7d4869527f1cef579015a06258c93a33e76cc865 introduced a regression in the auto-update logic. The code that was setting the AutoUpdate flag in the *api.Image structure in line 541: https://github.com/lxc/lxd/commit/7d4869527f1cef579015a06258c93a33e76cc865#diff-6182827642cd00e061c6ffe52891acc5L541 was moved down in the function: https://github.com/lxc/lxd/commit/7d4869527f1cef579015a06258c93a33e76cc865#diff-6182827642cd00e061c6ffe52891acc5R482 however the new location is after the call to dbImageInsert: ``` err = dbImageInsert(d.db, info.Fingerprint, info.Filename, info.Size, info.Public, info.AutoUpdate, info.Architecture, info.CreatedAt, info.ExpiresAt, info.Properties) ``` that actually makes use of such setting. This results in the auto_update column in the database always being 0, no matter what, and the server logic to auto-update images will never kick in. This commit should also fix another (non fatal) regression introduced by: https://github.com/lxc/lxd/commit/806d55c88562ab82b32d2d8563c20a8ec9bdac98 where appending the empty string "" to the poolNames to be processed ends up triggering a call to ```doDeleteImageFromPool()``` from ``autoUpdateImage``: https://github.com/lxc/lxd/blob/471604191b9793ffe479036dab8e527cb743cee1/lxd/images.go#L975 which as far as I understand will always end up with an error in case poolName is the empty string, so a spurious error will be logged. Note that auto-update regression affects the latest 2.13 release, but not 2.12. Signed-off-by: Free Ekanayaka <[email protected]> --- lxd/daemon_images.go | 9 +++++-- lxd/images.go | 10 +++++--- test/main.sh | 1 + test/suites/image_auto_update.sh | 51 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 66 insertions(+), 5 deletions(-) create mode 100644 test/suites/image_auto_update.sh diff --git a/lxd/daemon_images.go b/lxd/daemon_images.go index 859dbb6..40bf44b 100644 --- a/lxd/daemon_images.go +++ b/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 diff --git a/lxd/images.go b/lxd/images.go index 4021479..bc0640c 100644 --- a/lxd/images.go +++ b/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}) + } } } diff --git a/test/main.sh b/test/main.sh index d73597d..06ae511 100755 --- a/test/main.sh +++ b/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" diff --git a/test/suites/image_auto_update.sh b/test/suites/image_auto_update.sh new file mode 100644 index 0000000..a77152a --- /dev/null +++ b/test/suites/image_auto_update.sh @@ -0,0 +1,51 @@ +test_image_auto_update() { + 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}" + + # The first image got deleted from the local storage + if lxc image info "${fp1}"; 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" +}
_______________________________________________ lxc-devel mailing list [email protected] http://lists.linuxcontainers.org/listinfo/lxc-devel
