cmd/sysota,service: allow service save state or config

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@huawei.com>

cmd/sysotad/cmdsysotad/sysotad.go

@@ -14,6 +14,7 @@ import (
 	"time"
 
 	"booting.oniroproject.org/distro/components/sysota/dbusutil"
+	"booting.oniroproject.org/distro/components/sysota/errutil"
 	"booting.oniroproject.org/distro/components/sysota/ota"
 	"booting.oniroproject.org/distro/components/sysota/service"
 )
@@ -42,46 +43,38 @@ func DefaultOptions() *Options {
 	}
 }
 
-// Run implements the sysotad binary.
-func Run(opts *Options) (err error) {
-	// Load configuration.
-	conf, err := ota.LoadConfig(opts.ConfigFiles)
-	if err != nil {
-		return err
-	}
+func (opts *Options) LoadConfig() (*ota.Config, error) {
+	return ota.LoadConfig(opts.ConfigFiles)
+}
 
-	// Load state and defer state save on return.
-	state, err := ota.LoadState(opts.StateFile)
-	if err != nil {
-		return err
-	}
+func (opts *Options) LoadState() (*ota.SystemState, error) {
+	return ota.LoadState(opts.StateFile)
+}
 
-	defer func() {
-		e := ota.SaveState(state, opts.StateFile)
+func (opts *Options) SaveConfig(cfg *ota.Config) error {
+	return ota.SaveConfig(cfg, ota.DefaultStatefulConfigFile())
+}
 
-		// Ignore save errors if the state directory does not exist.
-		var pathErr *os.PathError
-		if errors.As(e, &pathErr) && os.IsNotExist(pathErr) {
-			return
-		}
+func (opts *Options) SaveState(st *ota.SystemState) error {
+	err := ota.SaveState(st, opts.StateFile)
 
-		// Did we save the state successfully?
-		if e == nil {
-			_, _ = fmt.Fprintf(Stdout, "System state saved to %s\n", opts.StateFile)
-			return
-		}
+	// Ignore save errors if the state directory does not exist.
+	var pathErr *os.PathError
+	if errors.As(err, &pathErr) && os.IsNotExist(pathErr) {
+		return nil
+	}
 
-		// We failed to save the state. What shall we do with the error (e)?
-		if err == nil {
-			// Propagate state save error if possible.
-			err = e
-		} else {
-			// Log state save error if it cannot be propagated (the service
-			// will exit with a failure code in both cases).
-			_, _ = fmt.Fprintf(Stderr, "%v\n", e)
-		}
-	}()
+	// Did we save the state successfully?
+	if err == nil {
+		_, _ = fmt.Fprintf(Stdout, "System state saved to %s\n", opts.StateFile)
+		return nil
+	}
+
+	return err
+}
 
+// Run implements the sysotad binary.
+func Run(opts *Options) (err error) {
 	// Connect to the system bus and defer disconnect on return.
 	conn, err := dbusutil.SystemBus()
 	if err != nil {
@@ -89,18 +82,21 @@ func Run(opts *Options) (err error) {
 	}
 
 	defer func() {
-		e := conn.Close()
-		if err == nil {
-			err = e
-		}
+		errutil.Forward(&err, conn.Close())
 	}()
 
 	// Create the SystemOTA service
-	svc, err := service.New(conf, state, conn)
+	svc, err := service.New(opts, conn)
 	if err != nil {
 		return err
 	}
 
+	// TODO(zyga): save the state only when it is modified and do so immediately
+	// when the service needs to, allowing this call to go away.
+	defer func() {
+		errutil.Forward(&err, opts.SaveState(svc.State()))
+	}()
+
 	// Export the service to D-Bus having finished configuration.
 	if err := svc.Export(); err != nil {
 		return err

cmd/sysotad/cmdsysotad/sysotad_test.go

@@ -106,33 +106,3 @@ func (s *sysotadSuite) TestStateLoadSaveDance(c *C) {
 	c.Assert(err, IsNil)
 	c.Check(data, DeepEquals, []byte("[System]\nBootMode=try\n"))
 }
-
-func (s *sysotadSuite) TestBrokenDBusButWhatAboutState(c *C) {
-	restore, err := dbustest.SetDBusSystemBusAddress("potato")
-	c.Assert(err, IsNil)
-
-	defer func() {
-		c.Assert(restore(), IsNil)
-	}()
-
-	d := c.MkDir()
-	stateFile := filepath.Join(d, "state.ini")
-	err = ioutil.WriteFile(stateFile, []byte("[System]\nBootMode=try\n"), 0600)
-	c.Assert(err, IsNil)
-
-	opts := cmdsysotad.DefaultOptions()
-	opts.IdleDuration = time.Second
-	opts.StateFile = stateFile
-
-	// The service fails to start but doesn't forget to save the state.
-	err = cmdsysotad.Run(opts)
-	c.Assert(err, ErrorMatches, `dbus: invalid bus address \(no transport\)`)
-
-	c.Check(s.stdout.String(), Equals, ""+
-		"System state saved to "+stateFile+"\n")
-	c.Check(s.stderr.String(), Equals, "")
-
-	data, err := ioutil.ReadFile(stateFile)
-	c.Assert(err, IsNil)
-	c.Check(data, DeepEquals, []byte("[System]\nBootMode=try\n"))
-}

service/service.go

@@ -38,6 +38,13 @@ func RequestBusName(conn *dbus.Conn) error {
 	return nil
 }
 
+type Stater interface {
+	LoadConfig() (*ota.Config, error)
+	LoadState() (*ota.SystemState, error)
+	SaveConfig(*ota.Config) error
+	SaveState(*ota.SystemState) error
+}
+
 // Service implements the D-Bus service capable of downloading system updates.
 type Service struct {
 	m sync.RWMutex
@@ -86,46 +93,8 @@ func configuredBootProtocol(config *ota.Config) (boot.Protocol, error) {
 	return nil, unsupportedBootLoaderError(config.BootLoaderType)
 }
 
-type compat struct {
-	config *ota.Config
-	state  *ota.SystemState
-}
-
-func (c *compat) LoadConfig() (*ota.Config, error) {
-	return c.config, nil
-}
-
-func (c *compat) SaveConfig(config *ota.Config) error {
-	return nil
-}
-
-func (c *compat) LoadState() (*ota.SystemState, error) {
-	return c.state, nil
-}
-
-func (c *compat) SaveState(state *ota.SystemState) error {
-	return nil
-}
-
 // New returns a new service.
-func New(config *ota.Config, state *ota.SystemState, conn *dbus.Conn) (*Service, error) {
-	if config == nil {
-		panic("config cannot be nil")
-	}
-
-	if state == nil {
-		panic("state cannot be nil")
-	}
-
-	if conn == nil {
-		panic("conn cannot be nil")
-	}
-
-	return NewStater(&compat{config: config, state: state}, conn)
-}
-
-// New returns a new service.
-func NewStater(stater Stater, conn *dbus.Conn) (*Service, error) {
+func New(stater Stater, conn *dbus.Conn) (*Service, error) {
 	if stater == nil {
 		panic("stater cannot be nil")
 	}
@@ -175,11 +144,9 @@ func NewStater(stater Stater, conn *dbus.Conn) (*Service, error) {
 	return svc, nil
 }
 
-type Stater interface {
-	LoadConfig() (*ota.Config, error)
-	LoadState() (*ota.SystemState, error)
-	SaveConfig(*ota.Config) error
-	SaveState(*ota.SystemState) error
+// State returns the system state as it was loaded from the stater.
+func (svc *Service) State() *ota.SystemState {
+	return svc.state
 }
 
 // BootMode implements raucadapter.BootModeHolder and returns the current boot mode.

service/service_test.go

@@ -44,13 +44,34 @@ func (s *serviceSuite) EnsureConn(c *C) *dbus.Conn {
 	return s.conn
 }
 
+type stater struct {
+	config *ota.Config
+	state  *ota.SystemState
+}
+
+func (c *stater) LoadConfig() (*ota.Config, error) {
+	return c.config, nil
+}
+
+func (c *stater) SaveConfig(config *ota.Config) error {
+	return nil
+}
+
+func (c *stater) LoadState() (*ota.SystemState, error) {
+	return c.state, nil
+}
+
+func (c *stater) SaveState(state *ota.SystemState) error {
+	return nil
+}
+
 func (s *serviceSuite) EnsureService(c *C) {
 	var err error
 
 	if s.svc == nil {
 		s.EnsureConn(c)
 
-		s.svc, err = service.New(&s.config, &s.state, s.conn)
+		s.svc, err = service.New(&stater{config: &s.config, state: &s.state}, s.conn)
 		c.Assert(err, IsNil)
 
 		err := s.svc.Export()