Snappy

Merge lp:~mvo/snappy/experimental-simplify-the-upgrader-script into lp:~snappy-dev/snappy/snappy-moved-to-github

experimental-simplify-the-upgrader-script
Merge into snappy-moved-to-github

Proposed by Michael Vogt on 2015-01-29

Status:	Superseded
Proposed branch:	lp:~mvo/snappy/experimental-simplify-the-upgrader-script
Merge into:	lp:~snappy-dev/snappy/snappy-moved-to-github
Prerequisite:	lp:~mvo/snappy/si-progress
Diff against target:	440 lines (+174/-44) 5 files modified snappy/progress.go (+22/-8) snappy/progress_test.go (+32/-0) snappy/snapp.go (+1/-1) snappy/systemimage.go (+62/-21) snappy/systemimage_test.go (+57/-14)
To merge this branch:	bzr merge lp:~mvo/snappy/experimental-simplify-the-upgrader-script
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Michael Vogt			Pending
Review via email: mp+248023@code.launchpad.net

This proposal supersedes a proposal from 2015-01-29.

Description of the change

This branch might be interessting to simplfy the way that ubuntu-core-upgrader
works. The idea is that the DownloadUpdate() call is run with the other
partition mounted RW (DownloadUpdate will also apply the update so its
probably a good idea to update the name to reflect this fact).

This should in turn make the ubuntu-core-upgrader much simpler and allow
us to remove all the mount/unmount logic in there as this is hanlded by
snappy already and it move the responsibility of all this into a single
component (partition.go). We still need to ensure that "sync_partition()"
is called of course in ubuntu-core-upgrader (that is done in _cmd_mount()
right now AFAICS).

Unmerged revisions

132. By Michael Vogt on 2015-01-29: merged lp:snappy and resolved conflicts
131. By Michael Vogt on 2015-01-29: Run "DownloadUpdate()" with the other partition mounted RW. This should
allow us to simplify the ubuntu-core-upgrader script significantly by
removing all the mount/umount logic there (making _cmd_mount/_cmd_umount
no-ops). The only catch is that the empty-parititon detection needs to
be done still of course.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Alan Meekins

Huang Jin

Michael Vogt

Snappy Developers

kaxing

 === modified file 'snappy/progress.go'
 --- snappy/progress.go	2015-01-23 14:55:15 +0000
 +++ snappy/progress.go	2015-01-29 16:50:57 +0000
@@ -7,18 +7,23 @@
+ )
  type ProgressMeter interface {
--	Start(total int64)
--	Increment()
++	Start(total float64)
++	Set(current float64)
 	Finished()
++	// FIXME: better name?
++	// Show progress via a spinner
++	Spin(msg string)
++
 	// interface for writer
 	Write(p []byte) (n int, err error)
+ }
  type TextProgress struct {
 	ProgressMeter
--	pbar *pb.ProgressBar
--	pkg string
++	pbar *pb.ProgressBar
++	pkg string
++	spinStep int
+ }
  func NewTextProgress(pkg string) *TextProgress {
@@ -27,14 +32,14 @@
 	return &t
+ }
--func (t *TextProgress) Start(total int64) {
++func (t *TextProgress) Start(total float64) {
 	fmt.Println("Starting download of", t.pkg)
--	t.pbar.Total = total
++	t.pbar.Total = int64(total)
 	t.pbar.Start()
+ }
--func (t *TextProgress) Increment() {
--	t.pbar.Increment()
++func (t *TextProgress) Set(current float64) {
++	t.pbar.Set(int(current))
+ }
  func (t *TextProgress) Finished() {
@@ -44,3 +49,12 @@
  func (t *TextProgress) Write(p []byte) (n int, err error) {
 	return t.pbar.Write(p)
+ }
++
++func (t *TextProgress) Spin(msg string) {
++	states := `|/-\`
++	fmt.Printf("\r%s[%c]", msg, states[t.spinStep])
++	t.spinStep++
++	if t.spinStep >= len(states) {
++	t.spinStep = 0
++	}
++}
 === added file 'snappy/progress_test.go'
 --- snappy/progress_test.go	1970-01-01 00:00:00 +0000
 +++ snappy/progress_test.go	2015-01-29 16:50:57 +0000
@@ -0,0 +1,32 @@
++package snappy
++
++import (
++	"io/ioutil"
++	"os"
++
++	. "gopkg.in/check.v1"
++)
++
++type ProgressTestSuite struct{}
++
++var _ = Suite(&ProgressTestSuite{})
++
++func (ts *ProgressTestSuite) TestSpin(c *C) {
++	f, err := ioutil.TempFile("", "progress-")
++	c.Assert(err, IsNil)
++	defer os.Remove(f.Name())
++	oldStdout := os.Stdout
++	os.Stdout = f
++
++	t := NewTextProgress("no-pkg")
++	for i := 0; i < 6; i++ {
++	t.Spin("m")
++	}
++
++	os.Stdout = oldStdout
++	f.Sync()
++	f.Seek(0, 0)
++	progress, err := ioutil.ReadAll(f)
++	c.Assert(err, IsNil)
++	c.Assert(string(progress), Equals, "\rm[|]\rm[/]\rm[-]\rm[\\]\rm[|]\rm[/]")
++}
 === modified file 'snappy/snapp.go'
 --- snappy/snapp.go	2015-01-27 23:32:36 +0000
 +++ snappy/snapp.go	2015-01-29 16:50:57 +0000
@@ -269,7 +269,7 @@
 	defer resp.Body.Close()
 	if pbar != nil {
--	pbar.Start(resp.ContentLength)
++	pbar.Start(float64(resp.ContentLength))
 	mw := io.MultiWriter(w, pbar)
 	_, err = io.Copy(mw, resp.Body)
 	pbar.Finished()
 === modified file 'snappy/systemimage.go'
 --- snappy/systemimage.go	2015-01-29 15:28:06 +0000
 +++ snappy/systemimage.go	2015-01-29 16:50:57 +0000
@@ -11,6 +11,7 @@
 	"fmt"
 	"log"
 	"os"
++	"path"
 	"runtime"
 	"strings"
 	"time"
@@ -93,6 +94,41 @@
  func (s *SystemImagePart) Install(pb ProgressMeter) (err error) {
++	quitCh := make(chan int)
++	if pb != nil {
++	// FIXME: we need to find a way to stop this watcher
++	updateProgressCh, err := s.proxy.makeWatcher("UpdateProgress")
++	if err != nil {
++	log.Panic(fmt.Sprintf("ERROR: %v", err))
++	return nil
++	}
++
++	defer func() {
++	quitCh <- 1
++	}()
++	pb.Start(100.0)
++	go func() {
++	for {
++	var percent int32
++	var eta float64
++	select {
++	case msg := <-updateProgressCh:
++	err := msg.Args(&percent, &eta)
++	if err == nil {
++	if percent >= 0 {
++	pb.Set(float64(percent))
++	} else {
++	pb.Spin("Applying")
++	}
++	}
++	case <-quitCh:
++	pb.Finished()
++	break
++	}
++	}
++	}()
++	}
++
 	// Ensure there is always a kernel + initrd to boot with, even
 	// if the update does not provide new versions.
 	err = s.partition.SyncBootloaderFiles()
@@ -100,14 +136,21 @@
 	return err
+	}
--	err = s.proxy.DownloadUpdate()
++	err = s.partition.RunWithOther(partition.RW, func(o string) (err error) {
++	err = s.proxy.DownloadUpdate()
++	return
++	})
 	if err != nil {
 	return err
+	}
 	// FIXME: switch s-i daemon back to current partition
++	err = s.partition.UpdateBootloader()
--	return s.partition.UpdateBootloader()
++	if pb != nil {
++	quitCh <- 1
++	}
++	return err
+ }
  func (s *SystemImagePart) Uninstall() (err error) {
@@ -160,6 +203,7 @@
 	updateApplied chan *dbus.Message
 	updateDownloaded chan *dbus.Message
 	updateFailed chan *dbus.Message
++	quitCh chan int // can be used to stop the watchers
+ }
  // this functions only exists to make testing easier, i.e. the testsuite
@@ -184,6 +228,7 @@
 	return nil
+	}
++	p.quitCh = make(chan int)
 	p.updateAvailableStatus, err = p.makeWatcher("UpdateAvailableStatus")
 	if err != nil {
 	log.Panic(fmt.Sprintf("ERROR: %v", err))
@@ -261,8 +306,11 @@
 	received = make(chan *dbus.Message)
 	go func() {
--	for msg := range watch.C {
--	received <- msg
++	for {
++	select {
++	case msg := <-watch.C:
++	received <- msg
++	}
+	}
 	}()
@@ -314,7 +362,7 @@
 	// so once the D-Bus call completes, it no longer cares
 	// about configFile.
 	return s.partition.RunWithOther(partition.RO, func(otherRoot string) (err error) {
--	configFile := otherRoot + systemImageClientConfig
++	configFile := path.Join(otherRoot, systemImageClientConfig)
 	// FIXME: replace with FileExists() call once it's in a utility
 	// package.
 	_, err = os.Stat(configFile)
@@ -336,6 +384,7 @@
 	if err = s.ReloadConfiguration(false); err != nil {
 	return us, err
+	}
++	// FIXME: we can not switch back or DownloadUpdate is unhappy
 	callName := "CheckForUpdate"
 	_, err = s.proxy.Call(systemImageBusName, callName)
@@ -360,12 +409,6 @@
 	systemImageTimeoutSecs))
+	}
--	// switch back to using the current rootfs's system-image
--	// configuration.
--	if err = s.ReloadConfiguration(true); err != nil {
--	return us, err
--	}
--
 	return s.us, err
+ }
@@ -418,7 +461,7 @@
+ }
  func (s *SystemImageRepository) currentPart() Part {
--	configFile := s.myroot + systemImageChannelConfig
++	configFile := path.Join(s.myroot, systemImageChannelConfig)
 	part, err := s.makePartFromSystemImageConfigFile(configFile, true)
 	if err != nil {
 	log.Printf("Can not make system-image part for %s: %s", configFile, err)
@@ -462,17 +505,15 @@
 	if _, err = s.proxy.CheckForUpdate(); err != nil {
 	return parts, err
+	}
--	info, err := s.proxy.Information()
--	if err != nil {
--	return parts, err
--	}
--	if VersionCompare(info["current_build_number"], info["target_build_number"]) < 0 {
--	version := info["target_build_number"]
++	current := s.currentPart()
++	current_version := current.Version()
++	target_version := s.proxy.us.available_version
++	if VersionCompare(current_version, target_version) < 0 {
 	parts = append(parts, &SystemImagePart{
 	proxy: s.proxy,
--	version: version,
--	versionDetails: info["version_details"],
--	channelName: info["channel_name"],
++	version: target_version,
++	versionDetails: "?",
++	channelName: current.(*SystemImagePart).channelName,
 	partition: s.partition})
+	}
 === modified file 'snappy/systemimage_test.go'
 --- snappy/systemimage_test.go	2015-01-28 10:31:55 +0000
 +++ snappy/systemimage_test.go	2015-01-29 16:50:57 +0000
@@ -109,10 +109,11 @@
 	service *DBusService
 	partition *partition.Partition
--	info map[string]string
++	fakeAvailableVersion string
++	info map[string]string
+ }
--func NewMockSystemImage() *MockSystemImage {
++func newMockSystemImage() *MockSystemImage {
 	msi := new(MockSystemImage)
 	msi.info = make(map[string]string)
@@ -140,9 +141,9 @@
 	// again instead
 	var size int32 = 1234
 	sig.AppendArgs(
--	true, // is_available
--	false, // downloading
--	"3.14", // available_version
++	true, // is_available
++	false, // downloading
++	m.fakeAvailableVersion, // available_version
 	size, // update_size
 	"late_update_date", // laste update date
 	"") // error_reason
@@ -155,6 +156,20 @@
+ }
  func (m *MockSystemImage) DownloadUpdate() error {
++	// send progress
++	for i := 1; i <= 5; i++ {
++	sig := dbus.NewSignalMessage(systemImageObjectPath, systemImageInterface, "UpdateProgress")
++	sig.AppendArgs(
++	int32(20*i), // percent (int32)
++	float64(100.0-(20.0*i)), // eta (double)
++	)
++	if err := m.service.SendSignal(sig); err != nil {
++	// FIXME: do something with the error
++	panic(err)
++	}
++	}
++
++	// send done
 	sig := dbus.NewSignalMessage(systemImageObjectPath, systemImageInterface, "UpdateDownloaded")
 	sig.AppendArgs(
@@ -192,7 +207,7 @@
 	s.conn, err = dbus.Connect(dbus.SessionBus)
 	c.Assert(err, IsNil)
--	s.mockSystemImage = NewMockSystemImage()
++	s.mockSystemImage = newMockSystemImage()
 	s.mockService = NewDBusService(s.conn, systemImageInterface, systemImageObjectPath, systemImageBusName, s.mockSystemImage)
 	c.Assert(s.mockService, NotNil)
@@ -202,9 +217,9 @@
 	tmpdir, err := ioutil.TempDir("", "si-root-")
 	c.Assert(err, IsNil)
 	s.systemImage.myroot = tmpdir
--	makeFakeSystemImageChannelConfig(c, tmpdir+systemImageChannelConfig, "2.71")
++	makeFakeSystemImageChannelConfig(c, path.Join(tmpdir, systemImageChannelConfig), "2.71")
 	// setup fake /other partition
--	makeFakeSystemImageChannelConfig(c, tmpdir+"/other/"+systemImageChannelConfig, "3.14")
++	makeFakeSystemImageChannelConfig(c, path.Join(tmpdir, "other", systemImageChannelConfig), "3.14")
 	s.tmpdir = tmpdir
+ }
@@ -214,8 +229,8 @@
+ }
  func makeFakeSystemImageChannelConfig(c *C, cfgPath, buildNumber string) {
--	os.MkdirAll(path.Dir(cfgPath), 0777)
--	f, err := os.OpenFile(cfgPath, os.O_CREATE|os.O_RDWR, 0666)
++	os.MkdirAll(path.Dir(cfgPath), 0775)
++	f, err := os.OpenFile(cfgPath, os.O_CREATE|os.O_RDWR, 0664)
 	c.Assert(err, IsNil)
 	defer f.Close()
 	f.Write([]byte(fmt.Sprintf(`
@@ -263,6 +278,7 @@
+ }
  func (s *SITestSuite) TestUpdateNoUpdate(c *C) {
++	s.mockSystemImage.fakeAvailableVersion = "2.71"
 	parts, err := s.systemImage.Updates()
 	c.Assert(err, IsNil)
 	c.Assert(len(parts), Equals, 0)
@@ -270,7 +286,7 @@
  func (s *SITestSuite) TestUpdateHasUpdate(c *C) {
 	// add a update
--	s.mockSystemImage.info["target_build_number"] = "3.14"
++	s.mockSystemImage.fakeAvailableVersion = "3.14"
 	parts, err := s.systemImage.Updates()
 	c.Assert(err, IsNil)
 	c.Assert(len(parts), Equals, 1)
@@ -305,23 +321,50 @@
 	return f("/other")
+ }
++type MockProgressMeter struct {
++	total float64
++	progress []float64
++	finished bool
++	spin bool
++}
++
++func (m *MockProgressMeter) Start(total float64) {
++	m.total = total
++}
++func (m *MockProgressMeter) Set(current float64) {
++	m.progress = append(m.progress, current)
++}
++func (m *MockProgressMeter) Spin(msg string) {
++	m.spin = true
++}
++func (m *MockProgressMeter) Write(buf []byte) (n int, err error) {
++	return len(buf), err
++}
++func (m *MockProgressMeter) Finished() {
++	m.finished = true
++}
++
  func (s *SITestSuite) TestSystemImagePartInstallUpdatesPartition(c *C) {
 	// add a update
--	s.mockSystemImage.info["target_build_number"] = "3.14"
++	s.mockSystemImage.fakeAvailableVersion = "3.14"
 	parts, err := s.systemImage.Updates()
 	sp := parts[0].(*SystemImagePart)
 	mockPartition := MockPartition{}
 	sp.partition = &mockPartition
--	err = sp.Install(nil)
++	pb := &MockProgressMeter{}
++	err = sp.Install(pb)
 	c.Assert(err, IsNil)
 	c.Assert(mockPartition.updateBootloaderCalled, Equals, true)
++	c.Assert(pb.total, Equals, 100.0)
++	c.Assert(pb.finished, Equals, true)
++	c.Assert(pb.progress, DeepEquals, []float64{20.0, 40.0, 60.0, 80.0, 100.0})
+ }
  func (s *SITestSuite) TestSystemImagePartInstall(c *C) {
 	// add a update
--	s.mockSystemImage.info["target_build_number"] = "3.14"
++	s.mockSystemImage.fakeAvailableVersion = "3.14"
 	parts, err := s.systemImage.Updates()
 	sp := parts[0].(*SystemImagePart)