Merge lp:~mvo/snappy/snappy-snapfs-refactor-ftw into lp:~snappy-dev/snappy/snappy-moved-to-github

[Michael Vogt]

[lp:~mvo/snappy/snappy-snapfs-refactor-ftw]

786. By Michael Vogt on 2015-10-20

more snap types

787. By Michael Vogt on 2015-10-20

start refactor code out of generic and into OemSnap

[John Lenton (chipaca)]

review: Needs Fixing

[lp:~mvo/snappy/snappy-snapfs-refactor-ftw]

788. By Michael Vogt on 2015-10-20

more SnapIF work

789. By Michael Vogt on 2015-10-20

move kernel unpack into kernel snap

Unmerged revisions

789. By Michael Vogt on 2015-10-20

move kernel unpack into kernel snap

788. By Michael Vogt on 2015-10-20

more SnapIF work

787. By Michael Vogt on 2015-10-20

start refactor code out of generic and into OemSnap

786. By Michael Vogt on 2015-10-20

more snap types

785. By Michael Vogt on 2015-10-20

start refactor to have OemSnap etc

784. By Michael Vogt on 2015-10-20

add boot-ok for new-style all-snaps

783. By Michael Vogt on 2015-10-20

disallow removal of the active kernel and os

782. By Michael Vogt on 2015-10-20

remove kernel assets in /boot/ on snap removal

781. By Michael Vogt on 2015-10-20

show a reboot required message (not complete yet)

780. By Michael Vogt on 2015-10-20

go fmt

=== modified file 'pkg/clickdeb/deb.go'
--- pkg/clickdeb/deb.go	2015-10-20 14:48:11 +0000
+++ pkg/clickdeb/deb.go	2015-10-20 14:48:11 +0000
@@ -30,6 +30,7 @@
 	"os"
 	"os/exec"
 	"path/filepath"
+	"runtime"
 	"strings"
 	"time"
 
@@ -127,13 +128,19 @@
 	file *os.File
 }
 
+func clickDebFinalizer(d *ClickDeb) {
+	d.file.Close()
+}
+
 // Open calls os.Open and uses that file for the backing file.
 func Open(path string) (*ClickDeb, error) {
 	f, err := os.Open(path)
 	if err != nil {
 		return nil, err
 	}
-	return &ClickDeb{f}, nil
+	deb := &ClickDeb{f}
+	runtime.SetFinalizer(deb, clickDebFinalizer)
+	return deb, nil
 }
 
 // Create calls os.Create and uses that file for the backing file.
@@ -142,7 +149,9 @@
 	if err != nil {
 		return nil, err
 	}
-	return &ClickDeb{f}, nil
+	deb := &ClickDeb{f}
+	runtime.SetFinalizer(deb, clickDebFinalizer)
+	return deb, nil
 }
 
 // Name returns the Name of the backing file

=== modified file 'pkg/lightweight/lightweight_test.go'
--- pkg/lightweight/lightweight_test.go	2015-10-10 09:48:49 +0000
+++ pkg/lightweight/lightweight_test.go	2015-10-20 14:48:11 +0000
@@ -388,7 +388,7 @@
 	c.Check(oem.ActiveIndex(), check.Equals, -1)
 	p, err := oem.Load(0)
 	c.Check(err, check.IsNil)
-	c.Check(p, check.FitsTypeOf, new(snappy.SnapPart))
+	c.Check(p, check.FitsTypeOf, new(snappy.OemSnap))
 	c.Check(p.Version(), check.Equals, "3")
 }
 

=== modified file 'snappy/click.go'
--- snappy/click.go	2015-10-15 07:32:34 +0000
+++ snappy/click.go	2015-10-20 14:48:11 +0000
@@ -771,7 +771,6 @@
 	if err != nil {
 		return "", err
 	}
-	defer part.deb.Close()
 
 	return part.Install(inter, flags)
 }

=== modified file 'snappy/kernel.go'
--- snappy/kernel.go	2015-10-20 14:48:11 +0000
+++ snappy/kernel.go	2015-10-20 14:48:11 +0000
@@ -25,47 +25,70 @@
 
 	"launchpad.net/snappy/partition"
 	"launchpad.net/snappy/pkg/snapfs"
+	"launchpad.net/snappy/progress"
 )
 
-func unpackKernel(s *SnapPart) error {
+type KernelSnap struct {
+	SnapPart
+}
+
+// OEM snaps should not be removed as they are a key
+// building block for OEMs. Prunning non active ones
+// is acceptible.
+func (s *KernelSnap) Uninstall(pb progress.Meter) (err error) {
+	if s.IsActive() {
+		return ErrPackageNotRemovable
+	}
+
+	return s.SnapPart.Uninstall(pb)
+}
+
+func (s *KernelSnap) Install(inter progress.Meter, flags InstallFlags) (name string, err error) {
+	// do the generic install
+	name, err = s.SnapPart.Install(inter, flags)
+	if err != nil {
+		return name, err
+	}
+
+	// now do the kernel specific bits
 	bootdir := partition.BootloaderDir()
 	if err := os.MkdirAll(filepath.Join(bootdir, s.Version()), 0755); err !=
 		nil {
-		return err
+		return name, err
 	}
 	blobName := filepath.Base(snapfs.BlobPath(s.basedir))
 	dstDir := filepath.Join(bootdir, blobName)
 	if s.m.Kernel != "" {
 		src := s.m.Kernel
 		if err := s.deb.Unpack(src, dstDir); err != nil {
-			return err
+			return name, err
 		}
 		src = filepath.Join(dstDir, s.m.Kernel)
 		dst := filepath.Join(dstDir, partition.NormalizeKernelInitrdName(s.m.Kernel))
 		if err := os.Rename(src, dst); err != nil {
-			return err
+			return name, err
 		}
 	}
 	if s.m.Initrd != "" {
 		src := s.m.Initrd
 		if err := s.deb.Unpack(src, dstDir); err != nil {
-			return err
+			return name, err
 		}
 		src = filepath.Join(dstDir, s.m.Initrd)
 		dst := filepath.Join(dstDir, partition.NormalizeKernelInitrdName(s.m.Initrd))
 		if err := os.Rename(src, dst); err != nil {
-			return err
+			return name, err
 		}
 	}
 	if s.m.Dtbs != "" {
 		src := s.m.Dtbs
 		dst := filepath.Join(dstDir, s.m.Dtbs)
 		if err := s.deb.Unpack(src, dst); err != nil {
-			return err
+			return name, err
 		}
 	}
 
-	return nil
+	return name, nil
 }
 
 func removeKernel(s *SnapPart) error {

=== modified file 'snappy/oem.go'
--- snappy/oem.go	2015-09-25 15:27:11 +0000
+++ snappy/oem.go	2015-10-20 14:48:11 +0000
@@ -34,8 +34,34 @@
 	"launchpad.net/snappy/dirs"
 	"launchpad.net/snappy/logger"
 	"launchpad.net/snappy/pkg"
+	"launchpad.net/snappy/progress"
 )
 
+type OemSnap struct {
+	SnapPart
+}
+
+func (s *OemSnap) Install(inter progress.Meter, flags InstallFlags) (name string, err error) {
+	name, err = s.SnapPart.Install(inter, flags)
+	if err != nil {
+		return "", err
+	}
+
+	if err := installOemHardwareUdevRules(s.m); err != nil {
+		return "", err
+	}
+
+	return name, nil
+}
+
+func (s *OemSnap) Uninstall(pb progress.Meter) (err error) {
+	if s.IsActive() {
+		return ErrPackageNotRemovable
+	}
+
+	return s.SnapPart.Uninstall(pb)
+}
+
 // OEM represents the structure inside the package.yaml for the oem component
 // of an oem package type.
 type OEM struct {
@@ -129,7 +155,7 @@
 func getOemImpl() (*packageYaml, error) {
 	oems, _ := ActiveSnapsByType(pkg.TypeOem)
 	if len(oems) == 1 {
-		return oems[0].(*SnapPart).m, nil
+		return oems[0].(*OemSnap).m, nil
 	}
 
 	return nil, errors.New("no oem snap")

=== added file 'snappy/os.go'
--- snappy/os.go	1970-01-01 00:00:00 +0000
+++ snappy/os.go	2015-10-20 14:48:11 +0000
@@ -0,0 +1,36 @@
+package snappy
+
+// -*- Mode: Go; indent-tabs-mode: t -*-
+
+/*
+ * Copyright (C) 2014-2015 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+import (
+	"launchpad.net/snappy/progress"
+)
+
+type OsSnap struct {
+	SnapPart
+}
+
+func (s *OsSnap) Uninstall(pb progress.Meter) (err error) {
+	if s.IsActive() {
+		return ErrPackageNotRemovable
+	}
+
+	return s.SnapPart.Uninstall(pb)
+}

=== modified file 'snappy/purge.go'
--- snappy/purge.go	2015-09-25 15:27:11 +0000
+++ snappy/purge.go	2015-10-20 14:48:11 +0000
@@ -48,7 +48,7 @@
 
 	purgeActive := flags&DoPurgeActive != 0
 
-	var active []*SnapPart
+	var active []SnapIF
 
 	for _, datadir := range datadirs {
 		yamlPath := filepath.Join(dirs.SnapAppsDir, datadir.QualifiedName(), datadir.Version, "meta", "package.yaml")

=== modified file 'snappy/purge_test.go'
--- snappy/purge_test.go	2015-10-10 13:27:50 +0000
+++ snappy/purge_test.go	2015-10-20 14:48:11 +0000
@@ -59,7 +59,7 @@
 	c.Check(inter.notified, HasLen, 0)
 }
 
-func (s *purgeSuite) mkpkg(c *C, args ...string) (dataDir string, part *SnapPart) {
+func (s *purgeSuite) mkpkg(c *C, args ...string) (dataDir string, part SnapIF) {
 	version := "1.10"
 	extra := ""
 	switch len(args) {

=== modified file 'snappy/snapp.go'
--- snappy/snapp.go	2015-10-20 14:48:11 +0000
+++ snappy/snapp.go	2015-10-20 14:48:11 +0000
@@ -209,6 +209,16 @@
 	return nil
 }
 
+// FIXME: name suckso
+type SnapIF interface {
+	Part
+	activate(inhibitHooks bool, inter interacter) error
+	deactivate(inhibitHooks bool, inter interacter) error
+	Dir() string
+	remove(inter interacter) error
+	ServiceYamls() []ServiceYaml
+}
+
 // TODO split into payloads per package type composing the common
 // elements for all snaps.
 type packageYaml struct {
@@ -529,17 +539,16 @@
 }
 
 // NewInstalledSnapPart returns a new SnapPart from the given yamlPath
-func NewInstalledSnapPart(yamlPath, origin string) (*SnapPart, error) {
+func NewInstalledSnapPart(yamlPath, origin string) (SnapIF, error) {
 	m, err := parsePackageYamlFile(yamlPath)
 	if err != nil {
 		return nil, err
 	}
 
-	part, err := NewSnapPartFromYaml(yamlPath, origin, m)
+	part, err := newSnapPartFromYaml(yamlPath, origin, m, true)
 	if err != nil {
 		return nil, err
 	}
-	part.isInstalled = true
 
 	return part, nil
 }
@@ -547,7 +556,7 @@
 // NewSnapPartFromSnapFile loads a snap from the given (clickdeb) snap file.
 // Caller should call Close on the pkg.
 // TODO: expose that Close.
-func NewSnapPartFromSnapFile(snapFile string, origin string, unauthOk bool) (*SnapPart, error) {
+func NewSnapPartFromSnapFile(snapFile string, origin string, unauthOk bool) (SnapIF, error) {
 	d, err := pkg.Open(snapFile)
 	if err != nil {
 		return nil, err
@@ -570,41 +579,45 @@
 		return nil, err
 	}
 
-	// FIXME: make a special OemSnap that knows its install dir
-	//        instead of having this special case here
-	targetDir := dirs.SnapAppsDir
-	// the "oem" parts are special
-	if m.Type == pkg.TypeOem {
-		targetDir = dirs.SnapOemDir
-	}
-
 	if origin == SideloadedOrigin {
 		m.Version = helpers.NewSideloadVersion()
 	}
 
 	fullName := m.qualifiedName(origin)
-	instDir := filepath.Join(targetDir, fullName, m.Version)
+	instDir := filepath.Join(dirs.SnapAppsDir, fullName, m.Version)
 
-	baseSnap := &SnapPart{
+	basePart := &SnapPart{
 		basedir: instDir,
 		origin:  origin,
 		m:       m,
 		deb:     d,
 	}
 
-	return baseSnap, nil
+	// FIXME: duplicated code
+	switch m.Type {
+	case pkg.TypeOem:
+		basePart.basedir = filepath.Join(dirs.SnapOemDir, fullName, m.Version)
+		return &OemSnap{SnapPart: *basePart}, nil
+	case pkg.TypeOS:
+		return &OsSnap{SnapPart: *basePart}, nil
+	case pkg.TypeKernel:
+		return &KernelSnap{SnapPart: *basePart}, nil
+	}
+
+	return basePart, nil
 }
 
-// NewSnapPartFromYaml returns a new SnapPart from the given *packageYaml at yamlPath
-func NewSnapPartFromYaml(yamlPath, origin string, m *packageYaml) (*SnapPart, error) {
+// newSnapPartFromYaml returns a new SnapPart from the given *packageYaml at yamlPath
+func newSnapPartFromYaml(yamlPath, origin string, m *packageYaml, installed bool) (SnapIF, error) {
 	if _, err := os.Stat(yamlPath); err != nil {
 		return nil, err
 	}
 
 	part := &SnapPart{
-		basedir: filepath.Dir(filepath.Dir(yamlPath)),
-		origin:  origin,
-		m:       m,
+		basedir:     filepath.Dir(filepath.Dir(yamlPath)),
+		origin:      origin,
+		m:           m,
+		isInstalled: installed,
 	}
 
 	// override the package's idea of its version
@@ -656,6 +669,19 @@
 		part.remoteM = &r
 	}
 
+	// FIXME: duplicated code
+	basePart := part
+	switch m.Type {
+	case pkg.TypeOem:
+		fullName := m.qualifiedName(origin)
+		basePart.basedir = filepath.Join(dirs.SnapOemDir, fullName, m.Version)
+		return &OemSnap{SnapPart: *basePart}, nil
+	case pkg.TypeOS:
+		return &OsSnap{SnapPart: *basePart}, nil
+	case pkg.TypeKernel:
+		return &KernelSnap{SnapPart: *basePart}, nil
+	}
+
 	return part, nil
 }
 
@@ -674,6 +700,10 @@
 	return s.m.Name
 }
 
+func (s *SnapPart) Dir() string {
+	return s.basedir
+}
+
 // Version returns the version
 func (s *SnapPart) Version() string {
 	if s.basedir != "" {
@@ -809,17 +839,10 @@
 		return "", err
 	}
 
-	// the "oem" parts are special
-	if s.Type() == pkg.TypeOem {
-		if err := installOemHardwareUdevRules(s.m); err != nil {
-			return "", err
-		}
-	}
-
 	fullName := QualifiedName(s)
 	dataDir := filepath.Join(dirs.SnapDataDir, fullName, s.Version())
 
-	var oldPart *SnapPart
+	var oldPart SnapIF
 	if currentActiveDir, _ := filepath.EvalSymlinks(filepath.Join(s.basedir, "..", "current")); currentActiveDir != "" {
 		oldPart, err = NewInstalledSnapPart(filepath.Join(currentActiveDir, "meta", "package.yaml"), s.origin)
 		if err != nil {
@@ -854,13 +877,6 @@
 		}
 	}
 
-	// FIXME: move into KernelSnap instead of poluting generic code
-	if s.m.Type == pkg.TypeKernel {
-		if err := unpackKernel(s); err != nil {
-			return "", err
-		}
-	}
-
 	// legacy, the hooks (e.g. apparmor) need this. Once we converted
 	// all hooks this can go away
 	clickMetaDir := filepath.Join(s.basedir, ".click", "info")
@@ -1160,13 +1176,6 @@
 
 // Uninstall remove the snap from the system
 func (s *SnapPart) Uninstall(pb progress.Meter) (err error) {
-	// OEM snaps should not be removed as they are a key
-	// building block for OEMs. Prunning non active ones
-	// is acceptible.
-	if (s.m.Type == pkg.TypeOem || s.m.Type == pkg.TypeKernel || s.m.Type == pkg.TypeOS) && s.IsActive() {
-		return ErrPackageNotRemovable
-	}
-
 	if IsBuiltInSoftware(s.Name()) && s.IsActive() {
 		return ErrPackageNotRemovable
 	}
@@ -1396,12 +1405,12 @@
 }
 
 // RefreshDependentsSecurity refreshes the security policies of dependent snaps
-func (s *SnapPart) RefreshDependentsSecurity(oldPart *SnapPart, inter interacter) (err error) {
+func (s *SnapPart) RefreshDependentsSecurity(oldPart SnapIF, inter interacter) (err error) {
 	oldBaseDir := ""
 	if oldPart != nil {
-		oldBaseDir = oldPart.basedir
+		oldBaseDir = oldPart.Dir()
 	}
-	upPol, upTpl := policy.AppArmorDelta(oldBaseDir, s.basedir, s.Name()+"_")
+	upPol, upTpl := policy.AppArmorDelta(oldBaseDir, s.Dir(), s.Name()+"_")
 
 	deps, err := s.Dependents()
 	if err != nil {
@@ -2066,7 +2075,7 @@
 // The returned environment contains additional SNAP_* variables that
 // are required when calling a meta/hook/ script and that will override
 // any already existing SNAP_* variables in os.Environment()
-func makeSnapHookEnv(part *SnapPart) (env []string) {
+func makeSnapHookEnv(part SnapIF) (env []string) {
 	desc := struct {
 		AppName     string
 		AppArch     string
@@ -2077,7 +2086,7 @@
 	}{
 		part.Name(),
 		helpers.UbuntuArchitecture(),
-		part.basedir,
+		part.Dir(),
 		part.Version(),
 		QualifiedName(part),
 		part.Origin(),

=== modified file 'snappy/snapp_snapfs_test.go'
--- snappy/snapp_snapfs_test.go	2015-10-20 14:48:11 +0000
+++ snappy/snapp_snapfs_test.go	2015-10-20 14:48:11 +0000
@@ -138,7 +138,7 @@
 	c.Assert(err, IsNil)
 
 	// ensure the right backend got picked up
-	c.Assert(part.deb, FitsTypeOf, &snapfs.Snap{})
+	c.Assert(part.(*SnapPart).deb, FitsTypeOf, &snapfs.Snap{})
 }
 
 func (s *SnapfsTestSuite) TestInstallViaSnapfsWorks(c *C) {
@@ -312,7 +312,7 @@
 	c.Assert(err, IsNil)
 	c.Assert(snap.NeedsReboot(), Equals, false)
 
-	snap.isActive = false
+	snap.(*OsSnap).isActive = false
 	mockb.bootvars["snappy_os"] = "ubuntu-core." + testOrigin + "_15.10-1.snap"
 	c.Assert(snap.NeedsReboot(), Equals, true)
 }
@@ -325,7 +325,7 @@
 	c.Assert(err, IsNil)
 	c.Assert(snap.NeedsReboot(), Equals, false)
 
-	snap.isActive = false
+	snap.(*KernelSnap).isActive = false
 	mockb.bootvars["snappy_kernel"] = "ubuntu-kernel." + testOrigin + "_4.0-1.snap"
 	c.Assert(snap.NeedsReboot(), Equals, true)
 }
@@ -350,15 +350,24 @@
 	c.Assert(helpers.FileExists(kernelAssetsDir), Equals, false)
 }
 
-func (s *SnapfsTestSuite) TestActiveKernelOSNotRemovable(c *C) {
-	for _, yaml := range []string{packageKernel, packageOS} {
-		snapYaml, err := makeInstalledMockSnap(dirs.GlobalRootDir, yaml)
-		c.Assert(err, IsNil)
-
-		snap, err := NewInstalledSnapPart(snapYaml, testOrigin)
-		c.Assert(err, IsNil)
-
-		snap.isActive = true
-		c.Assert(snap.Uninstall(&MockProgressMeter{}), Equals, ErrPackageNotRemovable)
-	}
+func (s *SnapfsTestSuite) TestActiveKernelNotRemovable(c *C) {
+	snapYaml, err := makeInstalledMockSnap(dirs.GlobalRootDir, packageKernel)
+	c.Assert(err, IsNil)
+
+	snap, err := NewInstalledSnapPart(snapYaml, testOrigin)
+	c.Assert(err, IsNil)
+
+	snap.(*KernelSnap).isActive = true
+	c.Assert(snap.Uninstall(&MockProgressMeter{}), Equals, ErrPackageNotRemovable)
+}
+
+func (s *SnapfsTestSuite) TestActiveOSNotRemovable(c *C) {
+	snapYaml, err := makeInstalledMockSnap(dirs.GlobalRootDir, packageOS)
+	c.Assert(err, IsNil)
+
+	snap, err := NewInstalledSnapPart(snapYaml, testOrigin)
+	c.Assert(err, IsNil)
+
+	snap.(*OsSnap).isActive = true
+	c.Assert(snap.Uninstall(&MockProgressMeter{}), Equals, ErrPackageNotRemovable)
 }

=== modified file 'snappy/snapp_test.go'
--- snappy/snapp_test.go	2015-10-20 07:03:39 +0000
+++ snappy/snapp_test.go	2015-10-20 14:48:11 +0000
@@ -152,11 +152,11 @@
 	c.Assert(services[0].Name, Equals, "svc1")
 
 	// ensure we get valid Date()
-	st, err := os.Stat(snap.basedir)
+	st, err := os.Stat(snap.Dir())
 	c.Assert(err, IsNil)
 	c.Assert(snap.Date(), Equals, st.ModTime())
 
-	c.Assert(snap.basedir, Equals, filepath.Join(s.tempdir, "apps", helloAppComposedName, "1.10"))
+	c.Assert(snap.Dir(), Equals, filepath.Join(s.tempdir, "apps", helloAppComposedName, "1.10"))
 	c.Assert(snap.InstalledSize(), Not(Equals), -1)
 }
 
@@ -825,11 +825,11 @@
 	c.Assert(services[1].Description, Equals, "Service #2")
 
 	// ensure we get valid Date()
-	st, err := os.Stat(snap.basedir)
+	st, err := os.Stat(snap.Dir())
 	c.Assert(err, IsNil)
 	c.Assert(snap.Date(), Equals, st.ModTime())
 
-	c.Assert(snap.basedir, Equals, filepath.Join(s.tempdir, "apps", helloAppComposedName, "1.10"))
+	c.Assert(snap.Dir(), Equals, filepath.Join(s.tempdir, "apps", helloAppComposedName, "1.10"))
 	c.Assert(snap.InstalledSize(), Not(Equals), -1)
 }