Merge lp://qastaging/~mvo/snappy/snappy-lp1460152-workaround into lp://qastaging/~snappy-dev/snappy/snappy-moved-to-github

Proposed by Michael Vogt
Status: Work in progress
Proposed branch: lp://qastaging/~mvo/snappy/snappy-lp1460152-workaround
Merge into: lp://qastaging/~snappy-dev/snappy/snappy-moved-to-github
Diff against target: 157 lines (+78/-6)
5 files modified
helpers/helpers.go (+17/-0)
helpers/helpers_test.go (+11/-1)
snappy/dirs.go (+4/-0)
snappy/systemimage.go (+16/-5)
snappy/systemimage_test.go (+30/-0)
To merge this branch: bzr merge lp://qastaging/~mvo/snappy/snappy-lp1460152-workaround
Reviewer Review Type Date Requested Status
Ricardo Salveti (community) Needs Information
Review via email: mp+261144@code.qastaging.launchpad.net

Commit message

This branch adds a workaround for LP: #1460152 to prevent that the
apparmor profile and cache get out of sync.

Description of the change

This branch adds a workaround for LP: #1460152 to prevent that the
apparmor profile and cache get out of sync.

To post a comment you must log in.
Revision history for this message
Sergio Schvezov (sergiusens) wrote :

I linked the bug, we can add a task for a proper implementation.

Revision history for this message
Ricardo Salveti (rsalveti) wrote :

Wouldn't this only be triggered after doing a second update? The major problem we have is that we're currently over a stable/older image, and we want to update to edge, and make sure that the new cache is not broken on the next reboot, but we're still updating to edge using the tools available at stable (not using the extra logic you added here).

So the workaround would need to be either something triggered by the update process coming from stable, or something done as part of the first boot after updating the image.

review: Needs Information
Revision history for this message
Sergio Schvezov (sergiusens) wrote :

Tears aside ;-) this is lacking some sort of systemd unit to trigger apparmor cache cleaning on boot (we can probably leverage the one apparmor has itself).

Revision history for this message
Michael Vogt (mvo) wrote :

@Ricardo: Indeed you are right :/ Sorry for overlooking this, at some point we need to add the ability for the upgrader to upgrade itself. So the best way seems to be going a systemd unit route (as Sergio suggested).

@Sergio: I pushed an alternative solution based on systemd to http://bazaar.launchpad.net/~mvo/ubuntu/vivid/ubuntu-core-config/lp1460152-workaround/revision/28 and then http://bazaar.launchpad.net/~mvo/ubuntu/vivid/ubuntu-core-config/lp1460152-workaround/revision/29. My original mtime based approach did not work, the md5sum one looks promising though.

The full diff is here:
https://code.launchpad.net/~mvo/ubuntu/vivid/ubuntu-core-config/lp1460152-workaround/+merge/261179

If the approach looks good this needs to go to the ppa and get a real-world test (i.e. 15.04 stable->edge upgrade). Please keep me updated via mail or telegram as I won't be on irc today.

Unmerged revisions

486. By Michael Vogt

Remove /etc/apparmor.d/cache/* on upgrade to workaround lp1460152

This works around the issue that the way apparmor creates the cache
is based on the mtime of the profile. So if the mtime of the profile
is older than the mtime of the cache file the cache is not re-generated.

This is a problem because:
- boot stable, /etc/apparmor.d/cache/usr.bin.ubuntu-core-launcher is mtime of now because we generate the cache on boot
- upgrade to edge, /etc/apparmor.d/usr.bin.ubuntu-core-launcher is updated and has the mtime of T (yesterday) when the file was put into the package
- on the next reboot the apparmor_parser compares the mtime of the cache/usr.bin.ubuntu-core-launcher (very very recent) with the mtime of the souce usr.bin.ubuntu-core-launcher (much older)
-> cache does is *not* re-generate

The real fix is IMO that apparmor adds the mtime of the profile into
the header of the cache file (or makes the mtime of the cache file)
the mtime of the profile and re-generated if they get out of sync
(instead of checking for newer).

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches