lxd: don't re-inject the same snaps

[kalikiana]

• Have you followed the guidelines for contributing?
• Have you signed the CLA?
• If this is a bugfix. Have you checked that there is a bug report open for the issue you are trying to fix on bug reports?
• If this is a new feature. Have you discussed the design on the forum?
• Have you successfully run ./runtests.sh static?
• Have you successfully run ./runtests.sh unit?

---

Fixes: bug 1716923

[sergiusens]

can you please rename this _is_same_snap, then when used in conditionals it reads better -> if _is_snap_snap

[kalikiana]

Done

[elopio]

When I tried this PR the first time, I got this printed:
md5sum: /var/lib/snapd/snaps/snapcraft_x1.snap: No such file or directory

We should check if the file exists before running md5sum.

[elopio]

I built and installed the snap from your branch. The first time I run the container build, this is printed:

Installing /run/core_2898.snap
core 16-2.27.6 from 'canonical' installed
Making snapcraft_x2.snap user-accessible
Installing /run/snapcraft_x2.snap
snapcraft 2.34+git49.535b8db installed

The second time, this is printed:

Making snapcraft_x2.snap user-accessible
Installing /run/snapcraft_x2.snap
snapcraft 2.34+git49.535b8db installed

So it seems to me that core is installed only once, but snapcraft is installed also the second time.

[sergiusens]

@elopio that should be fine, a locally built snap would be x versioned

[elopio]

I think I understand what's happening here.
My snapcraft snap is x2 in the host. On the first container build execution, it will be installed as x1 on the container. On the second container build execution, we will check if x2 == x1, and install it again. Then on the third container build execution, we will check if x2 == x2, and skip the installation.

I confirmed this behaviour checking that if I have snapcraft x3 in the host, I require 4 container builds to skip the installation.

It is unfortunate because if I have snapcraft x100 in my host, I will need 100 reinstalls of the same snap in the container. But if I am right on my diagnostic here, then this sounds like a low priority bug, not a blocker for this PR, because people shouldn't be running a snapcraft that doesn't come from the store.

[elopio]

+1 because I confirmed this doesn't reinstall snaps from the store.

[kyrofa]

My snapcraft snap is x2 in the host. On the first container build execution, it will be installed as x1 on the container. On the second container build execution, we will check if x2 == x1, and install it again. Then on the third container build execution, we will check if x2 == x2, and skip the installation.

Isn't this code just comparing hashes? I don't see any revision checking here. This PR looks good, but I can't explain the behavior @elopio is seeing.

[kalikiana]

@kyrofa Yeah, there is just hashes. The issue @elopio ran into is that the x revisions are running numbers so you can have a lower or higher x on the host, and upon installing it you have different filenames for the same file, so the checksum will never "match" since the names are different.

[sergiusens]

Can we use sha384 here instead?

[sergiusens]

@kalikiana can you explain more how a filename change would change the sum for a blob like the squashfs?

[kalikiana]

@sergiusens The problem is that x revisions are arbitrary, so installing the same snap on two systems may lead to different filenames - different filenames won't ever be compared with my code...
After some rubber ducking it occured to me I could try to query the snap in use in the container instead of assuming the names match. Looking into that now.

[sergiusens]

instead of ls, may I suggest readlink /snap/core/current