Core: originate errors from modules in system lifecycle (#2007) (#2013)

core/engine.go

@@ -92,12 +92,13 @@ func (system *System) Start() error {
 	var err error
 	return system.VisitEnginesE(func(engine Engine) error {
 		if m, ok := engine.(Runnable); ok {
-			coreLogger.Infof("Starting %s...", engineName(engine))
+			name := engineName(engine)
+			coreLogger.Infof("Starting %s...", name)
 			err = m.Start()
 			if err != nil {
-				return fmt.Errorf("failed to start %s: %w", engineName(engine), err)
+				return fmt.Errorf("unable to start %s: %w", name, err)
 			}
-			coreLogger.Infof("Started %s", engineName(engine))
+			coreLogger.Infof("Started %s", name)
 		}
 		return err
 	})
@@ -113,11 +114,12 @@ func (system *System) Shutdown() error {
 	})
 	for i := len(engines) - 1; i >= 0; i-- {
 		curr := engines[i]
-		coreLogger.Infof("Stopping %s...", engineName(curr))
+		name := engineName(curr)
+		coreLogger.Infof("Stopping %s...", name)
 		if err := curr.Shutdown(); err != nil {
-			return err
+			return fmt.Errorf("unable to shutdown %s: %w", name, err)
 		}
-		coreLogger.Infof("Stopped %s", engineName(curr))
+		coreLogger.Infof("Stopped %s", name)
 	}
 	return nil
 }
@@ -131,10 +133,14 @@ func (system *System) Configure() error {
 	}
 	return system.VisitEnginesE(func(engine Engine) error {
 		// only if Engine is dynamically configurable
+		name := engineName(engine)
 		if m, ok := engine.(Configurable); ok {
-			coreLogger.Debugf("Configuring %s", engineName(engine))
+			coreLogger.Debugf("Configuring %s", name)
 			err = m.Configure(*system.Config)
-			coreLogger.Debugf("Configured %s", engineName(engine))
+			coreLogger.Debugf("Configured %s", name)
+		}
+		if err != nil {
+			err = fmt.Errorf("unable to configure %s: %w", name, err)
 		}
 		return err
 	})
@@ -146,9 +152,13 @@ func (system *System) Migrate() error {
 	return system.VisitEnginesE(func(engine Engine) error {
 		// only if Engine is migratable
 		if m, ok := engine.(Migratable); ok {
-			coreLogger.Debugf("Migrating %s", engineName(engine))
+			name := engineName(engine)
+			coreLogger.Debugf("Migrating %s", name)
 			err = m.Migrate()
-			coreLogger.Debugf("Migrated %s", engineName(engine))
+			if err != nil {
+				return fmt.Errorf("unable to migrate %s: %w", name, err)
+			}
+			coreLogger.Debugf("Migrated %s", name)
 		}
 		return err
 	})

core/engine_test.go

@@ -36,15 +36,28 @@ func TestNewSystem(t *testing.T) {
 }
 
 func TestSystem_Start(t *testing.T) {
-	ctrl := gomock.NewController(t)
+	t.Run("ok", func(t *testing.T) {
+		ctrl := gomock.NewController(t)
 
-	r := NewMockRunnable(ctrl)
-	r.EXPECT().Start()
+		r := NewMockRunnable(ctrl)
+		r.EXPECT().Start()
 
-	system := NewSystem()
-	system.RegisterEngine(TestEngine{})
-	system.RegisterEngine(r)
-	assert.NoError(t, system.Start())
+		system := NewSystem()
+		system.RegisterEngine(TestEngine{})
+		system.RegisterEngine(r)
+		assert.NoError(t, system.Start())
+	})
+	t.Run("returns error", func(t *testing.T) {
+		ctrl := gomock.NewController(t)
+
+		r := NewMockRunnable(ctrl)
+		r.EXPECT().Start().Return(errors.New("failure"))
+
+		system := NewSystem()
+		system.RegisterEngine(r)
+
+		assert.EqualError(t, system.Start(), "unable to start *core.MockRunnable: failure")
+	})
 }
 
 func TestSystem_Shutdown(t *testing.T) {
@@ -57,7 +70,7 @@ func TestSystem_Shutdown(t *testing.T) {
 		system := NewSystem()
 		system.RegisterEngine(r)
 
-		assert.EqualError(t, system.Shutdown(), "failure")
+		assert.EqualError(t, system.Shutdown(), "unable to shutdown *core.MockRunnable: failure")
 	})
 	t.Run("start and shutdown are called in opposite order", func(t *testing.T) {
 		ctrl := gomock.NewController(t)
@@ -98,6 +111,18 @@ func TestSystem_Configure(t *testing.T) {
 		assert.NoError(t, system.Load(FlagSet()))
 		assert.Nil(t, system.Configure())
 	})
+	t.Run("returns error", func(t *testing.T) {
+		ctrl := gomock.NewController(t)
+
+		r := NewMockConfigurable(ctrl)
+		r.EXPECT().Configure(gomock.Any()).Return(errors.New("failure"))
+
+		system := NewSystem()
+		system.RegisterEngine(r)
+		assert.NoError(t, system.Load(FlagSet()))
+
+		assert.EqualError(t, system.Configure(), "unable to configure *core.MockConfigurable: failure")
+	})
 	t.Run("unable to create datadir", func(t *testing.T) {
 		system := NewSystem()
 		system.Config.Datadir = "engine_test.go"

crypto/cmd/cmd.go

@@ -77,7 +77,7 @@ func fs2ExternalStore() *cobra.Command {
 			config := instance.Config().(*cryptoEngine.Config)
 			targetStorage, err := external.NewAPIClient(config.External)
 			if err != nil {
-				return err
+				return fmt.Errorf("unable to set up external crypto API client: %w", err)
 			}
 
 			directory := args[0]

crypto/crypto.go

@@ -106,7 +106,7 @@ func (client *Crypto) setupStorageAPIBackend() error {
 	log.Logger().Debug("Setting up StorageAPI backend for storage of private key material.")
 	apiBackend, err := external.NewAPIClient(client.config.External)
 	if err != nil {
-		return err
+		return fmt.Errorf("unable to set up external crypto API client: %w", err)
 	}
 	client.storage = spi.NewValidatedKIDBackendWrapper(apiBackend, kidPattern)
 	return nil