diff --git a/cmd/sysotad/cmdsysotad/sysotad.go b/cmd/sysotad/cmdsysotad/sysotad.go index 7fe792d4ed3f1100d6c9d39124bfd022f7a008ae..1c9895913d0b27c92f34f6becc9dc1e417e9f9f8 100644 --- a/cmd/sysotad/cmdsysotad/sysotad.go +++ b/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 diff --git a/cmd/sysotad/cmdsysotad/sysotad_test.go b/cmd/sysotad/cmdsysotad/sysotad_test.go index e89fcc31f6cb2d53e4e5d842f41743451d567591..9928109541decf4d349bbfc4bca3cc2359493b00 100644 --- a/cmd/sysotad/cmdsysotad/sysotad_test.go +++ b/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")) -} diff --git a/service/service.go b/service/service.go index 8fec22270c9f3fc31ff0feb5046edff2a51c58c4..724bb37d23f31c5194edb253d5b5960508087566 100644 --- a/service/service.go +++ b/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. diff --git a/service/service_test.go b/service/service_test.go index 2369d773eb1d48158a98a516c36607183eb8ca44..abf69b18be5afa9abce1874958d5e78d6e8b443a 100644 --- a/service/service_test.go +++ b/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()