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

[Michael Vogt]

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

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

[Sergio Schvezov (sergiusens)]

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

[Ricardo Salveti (rsalveti)]

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

[Sergio Schvezov (sergiusens)]

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).

[Michael Vogt (mvo)]

@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 on 2015-06-04

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).

=== modified file 'helpers/helpers.go'
--- helpers/helpers.go	2015-06-04 12:55:54 +0000
+++ helpers/helpers.go	2015-06-04 20:40:35 +0000
@@ -26,6 +26,7 @@
 	"encoding/hex"
 	"fmt"
 	"io"
+	"io/ioutil"
 	"math/rand"
 	"os"
 	"os/exec"
@@ -436,3 +437,19 @@
 		"SNAPP_APP_USER_DATA_PATH={{.Home}}{{.AppPath}}",
 	})
 }
+
+// CleanDir cleans all regular files in the given directory
+func CleanDir(dir string) error {
+	dents, err := ioutil.ReadDir(dir)
+	if err != nil {
+		return err
+	}
+	for _, dent := range dents {
+		if dent.Mode().IsRegular() {
+			if err := os.Remove(filepath.Join(dir, dent.Name())); err != nil {
+				return err
+			}
+		}
+	}
+	return nil
+}

=== modified file 'helpers/helpers_test.go'
--- helpers/helpers_test.go	2015-06-04 12:55:54 +0000
+++ helpers/helpers_test.go	2015-06-04 20:40:35 +0000
@@ -326,5 +326,15 @@
 	err = UnpackTar(f, c.MkDir(), nil)
 	c.Assert(err, IsNil)
 	c.Assert(mknodWasCalled, Equals, true)
-
+}
+
+func (ts *HTestSuite) TestCleanDir(c *C) {
+	d := c.MkDir()
+	canary := filepath.Join(d, "foo")
+	err := ioutil.WriteFile(canary, []byte(nil), 0644)
+	c.Assert(err, IsNil)
+
+	err = CleanDir(d)
+	c.Assert(err, IsNil)
+	c.Assert(FileExists(canary), Equals, false)
 }

=== modified file 'snappy/dirs.go'
--- snappy/dirs.go	2015-05-15 13:33:27 +0000
+++ snappy/dirs.go	2015-06-04 20:40:35 +0000
@@ -33,6 +33,8 @@
 	snapSeccompDir   string
 	snapUdevRulesDir string
 
+	systemApparmorProfileCacheDir string
+
 	snapBinariesDir  string
 	snapServicesDir  string
 	snapBusPolicyDir string
@@ -62,4 +64,6 @@
 	cloudMetaDataFile = filepath.Join(rootdir, "/var/lib/cloud/seed/nocloud-net/meta-data")
 
 	snapUdevRulesDir = filepath.Join(rootdir, "/etc/udev/rules.d")
+
+	systemApparmorProfileCacheDir = filepath.Join(rootdir, "/etc/apparmor.d/cache")
 }

=== modified file 'snappy/systemimage.go'
--- snappy/systemimage.go	2015-05-20 17:24:29 +0000
+++ snappy/systemimage.go	2015-06-04 20:40:35 +0000
@@ -222,16 +222,27 @@
 	}
 
 	// Check that the final system state is as expected.
-	if err = s.verifyUpgradeWasApplied(); err != nil {
-		return "", err
-	}
-
-	if err = s.partition.ToggleNextBoot(); err != nil {
+	if err := s.verifyUpgradeWasApplied(); err != nil {
+		return "", err
+	}
+
+	if err := s.postUpgradeHacks(); err != nil {
+		return "", err
+	}
+
+	if err := s.partition.ToggleNextBoot(); err != nil {
 		return "", err
 	}
 	return systemImagePartName, nil
 }
 
+// this makes me cry
+func (s *SystemImagePart) postUpgradeHacks() error {
+	// Apply HACK to deal with bug #1460152 where the apparmor
+	// cache is confused after the upgrade
+	return helpers.CleanDir(systemApparmorProfileCacheDir)
+}
+
 // Ensure the expected version update was applied to the expected partition.
 func (s *SystemImagePart) verifyUpgradeWasApplied() error {
 	// The upgrade has now been applied, so check that the expected

=== modified file 'snappy/systemimage_test.go'
--- snappy/systemimage_test.go	2015-06-02 20:53:10 +0000
+++ snappy/systemimage_test.go	2015-06-04 20:40:35 +0000
@@ -58,6 +58,13 @@
 	// setup fake /other partition
 	makeFakeSystemImageChannelConfig(c, filepath.Join(tempdir, "other", systemImageChannelConfig), "2")
 
+	// fake the apparmor.d/cache
+	systemApparmorProfileCacheDir = filepath.Join(systemImageRoot, "etc", "apparmor.d", "cache")
+	err := os.MkdirAll(systemApparmorProfileCacheDir, 0755)
+	c.Assert(err, IsNil)
+	err = ioutil.WriteFile(filepath.Join(systemApparmorProfileCacheDir, "some-cache"), []byte(nil), 0644)
+	c.Assert(err, IsNil)
+
 	// run test webserver instead of talking to the real one
 	//
 	// The mock webserver versions  "1" and "2"
@@ -192,6 +199,29 @@
 	c.Assert(pb.progress, DeepEquals, []float64{20.0, 40.0, 60.0, 80.0, 100.0})
 }
 
+func (s *SITestSuite) TestSystemImagePartInstalAppliesHackLp1460152(c *C) {
+	// add a update
+	mockSystemImageIndexJSON = fmt.Sprintf(mockSystemImageIndexJSONTemplate, "2")
+	parts, err := s.systemImage.Updates()
+
+	sp := parts[0].(*SystemImagePart)
+	mockPartition := MockPartition{}
+	sp.partition = &mockPartition
+
+	dents, err := ioutil.ReadDir(systemApparmorProfileCacheDir)
+	c.Assert(err, IsNil)
+	c.Assert(dents, HasLen, 1)
+
+	pb := &MockProgressMeter{}
+	// do the install
+	_, err = sp.Install(pb, 0)
+	c.Assert(err, IsNil)
+
+	dents, err = ioutil.ReadDir(systemApparmorProfileCacheDir)
+	c.Assert(err, IsNil)
+	c.Assert(dents, HasLen, 0)
+}
+
 func (s *SITestSuite) TestSystemImagePartInstallUpdatesBroken(c *C) {
 	// fake a broken upgrade
 	scriptContent := `#!/bin/sh