stages: Run() returns an error

Rework Run() to return an error instead of bool

config/shared/errors/errors.go

@@ -91,7 +91,6 @@ var (
 	ErrHashWrongSize       = errors.New("incorrect size for hash sum")
 	ErrHashUnrecognized    = errors.New("unrecognized hash function")
 	ErrEngineConfiguration = errors.New("engine incorrectly configured")
-	ErrValidFetchEmptyData = errors.New("fetch successful but fetched data empty")
 )
 
 // NewNoInstallSectionError produces an error indicating the given unit, named

internal/distro/distro.go

@@ -59,7 +59,7 @@ func DiskByPartUUIDDir() string { return diskByPartUUIDDir }
 func OEMDevicePath() string     { return fromEnv("OEM_DEVICE", oemDevicePath) }
 
 func KernelCmdlinePath() string { return kernelCmdlinePath }
-func SystemConfigDir() string   { return fromEnv("SYSTEM_CONFIG_DIR", "/home/core/") }
+func SystemConfigDir() string   { return fromEnv("SYSTEM_CONFIG_DIR", systemConfigDir) }
 func OEMLookasideDir() string   { return fromEnv("OEM_LOOKASIDE_DIR", oemLookasideDir) }
 
 func ChrootCmd() string   { return chrootCmd }

internal/exec/engine.go

@@ -53,10 +53,10 @@ type Engine struct {
 
 // Run executes the stage of the given name. It returns true if the stage
 // successfully ran and false if there were any errors.
-func (e Engine) Run(stageName string) (bool, error) {
+func (e Engine) Run(stageName string) error {
 	if e.Fetcher == nil || e.Logger == nil {
 		fmt.Fprintf(os.Stderr, "engine incorrectly configured\n")
-		return false, errors.ErrEngineConfiguration
+		return errors.ErrEngineConfiguration
 	}
 	baseConfig := types.Config{
 		Ignition: types.Ignition{Version: types.MaxVersion.String()},
@@ -72,7 +72,7 @@ func (e Engine) Run(stageName string) (bool, error) {
 	e.logReport(r)
 	if err != nil && err != providers.ErrNoProvider {
 		e.Logger.Crit("failed to acquire system base config: %v", err)
-		return false, err
+		return err
 	}
 
 	cfg, err := e.acquireConfig()
@@ -84,17 +84,23 @@ func (e Engine) Run(stageName string) (bool, error) {
 		e.logReport(r)
 		if err != nil && err != providers.ErrNoProvider {
 			e.Logger.Crit("failed to acquire default config: %v", err)
-			return false, err
+			return err
 		}
 	default:
 		e.Logger.Crit("failed to acquire config: %v", err)
-		return false, err
+		return err
 	}
 
 	e.Logger.PushPrefix(stageName)
 	defer e.Logger.PopPrefix()
 
-	return stages.Get(stageName).Create(e.Logger, e.Root, *e.Fetcher).Run(config.Append(baseConfig, config.Append(systemBaseConfig, cfg))), err
+	if err = stages.Get(stageName).Create(e.Logger, e.Root, *e.Fetcher).Run(config.Append(baseConfig, config.Append(systemBaseConfig, cfg))); err != nil {
+		// e.Logger could be nil
+		fmt.Fprintf(os.Stderr, "%s failed", stageName)
+		return err
+	}
+	e.Logger.Info("passed")
+	return nil
 }
 
 // acquireConfig returns the configuration, first checking a local cache

internal/exec/stages/disks/disks.go

@@ -65,29 +65,26 @@ func (stage) Name() string {
 	return name
 }
 
-func (s stage) Run(config types.Config) bool {
+func (s stage) Run(config types.Config) error {
 	// Interacting with disks/partitions/raids/filesystems in general can cause
 	// udev races. If we do not need to  do anything, we also do not need to
 	// do the udevadm settle and can just return here.
 	if len(config.Storage.Disks) == 0 &&
 		len(config.Storage.Raid) == 0 &&
 		len(config.Storage.Filesystems) == 0 {
-		return true
+		return nil
 	}
 
 	if err := s.createPartitions(config); err != nil {
-		s.Logger.Crit("create partitions failed: %v", err)
-		return false
+		return fmt.Errorf("create partitions failed: %v", err)
 	}
 
 	if err := s.createRaids(config); err != nil {
-		s.Logger.Crit("failed to create raids: %v", err)
-		return false
+		return fmt.Errorf("failed to create raids: %v", err)
 	}
 
 	if err := s.createFilesystems(config); err != nil {
-		s.Logger.Crit("failed to create filesystems: %v", err)
-		return false
+		return fmt.Errorf("failed to create filesystems: %v", err)
 	}
 
 	// udevd registers an IN_CLOSE_WRITE inotify watch on block device
@@ -118,11 +115,10 @@ func (s stage) Run(config types.Config) bool {
 		exec.Command(distro.UdevadmCmd(), "settle"),
 		"waiting for udev to settle",
 	); err != nil {
-		s.Logger.Crit("udevadm settle failed: %v", err)
-		return false
+		return fmt.Errorf("udevadm settle failed: %v", err)
 	}
 
-	return true
+	return nil
 }
 
 // waitOnDevices waits for the devices enumerated in devs as a logged operation

internal/exec/stages/files/files.go

@@ -16,6 +16,7 @@ package files
 
 import (
 	"errors"
+	"fmt"
 
 	"github.com/coreos/ignition/internal/config/types"
 	"github.com/coreos/ignition/internal/exec/stages"
@@ -60,21 +61,18 @@ func (stage) Name() string {
 	return name
 }
 
-func (s stage) Run(config types.Config) bool {
+func (s stage) Run(config types.Config) error {
 	if err := s.createPasswd(config); err != nil {
-		s.Logger.Crit("failed to create users/groups: %v", err)
-		return false
+		return fmt.Errorf("failed to create users/groups: %v", err)
 	}
 
 	if err := s.createFilesystemsEntries(config); err != nil {
-		s.Logger.Crit("failed to create files: %v", err)
-		return false
+		return fmt.Errorf("failed to create files: %v", err)
 	}
 
 	if err := s.createUnits(config); err != nil {
-		s.Logger.Crit("failed to create units: %v", err)
-		return false
+		return fmt.Errorf("failed to create units: %v", err)
 	}
 
-	return true
+	return nil
 }

internal/exec/stages/stages.go

@@ -23,7 +23,7 @@ import (
 
 // Stage is responsible for actually executing a stage of the configuration.
 type Stage interface {
-	Run(config types.Config) bool
+	Run(config types.Config) error
 	Name() string
 }
 

internal/main.go

@@ -92,27 +92,14 @@ func main() {
 		OEMConfig:    oemConfig,
 		Fetcher:      &fetcher,
 	}
-	ignSuccess, err := engine.Run(flags.stage.String())
 
-	if !ignSuccess {
-		logger.Warning("Ignition failed")
-		if err != nil {
-			logger.Err(err.Error())
-		} else {
-			logger.Info("ERRORRRRR IS NIL")
-		}
-		statusErr := engine.OEMConfig.Status(*engine.Fetcher, err)
-		if statusErr != nil {
-			logger.Err("Status error: ", statusErr.Error())
-		}
+	err = engine.Run(flags.stage.String())
+	if statusErr := engine.OEMConfig.Status(flags.stage.String(), *engine.Fetcher, err); statusErr != nil {
+		logger.Err("POST Status error: ", statusErr.Error())
+	}
+	if err != nil {
+		logger.Crit("Ignition failed: %v", err.Error())
 		os.Exit(1)
-
-	} else {
-		logger.Info("Ignition finished successfully")
-		// err should be nil
-		statusErr := engine.OEMConfig.Status(*engine.Fetcher, err)
-		if statusErr != nil {
-			logger.Err("Status error: ", statusErr.Error())
-		}
 	}
+	logger.Info("Ignition finished successfully")
 }

internal/oem/oem.go

@@ -62,15 +62,10 @@ func (c Config) NewFetcherFunc() providers.FuncNewFetcher {
 	}
 }
 
-// Status takes b as whether ignition passes (true) or fails
-// msg is the error or success message
-func (c Config) Status(f resource.Fetcher, statusErr error) error {
+// Status takes a Fetcher and the error from Run (from engine)
+func (c Config) Status(stageName string, f resource.Fetcher, statusErr error) error {
 	if c.status != nil {
-		err := c.status(f, statusErr)
-		if err != nil {
-			f.Logger.Err("Error: ", err)
-		}
-		return err
+		return c.status(stageName, f, statusErr)
 	}
 	return nil
 }

internal/providers/packet/packet.go

@@ -20,17 +20,21 @@ package packet
 import (
 	"bytes"
 	"encoding/json"
+	"errors"
 	"net/http"
 	"net/url"
 	"strings"
 
-	"github.com/coreos/ignition/config/shared/errors"
 	"github.com/coreos/ignition/config/validate/report"
 	"github.com/coreos/ignition/internal/config/types"
 	"github.com/coreos/ignition/internal/providers/util"
 	"github.com/coreos/ignition/internal/resource"
 )
 
+var (
+	ErrValidFetchEmptyData = errors.New("fetch successful but fetched data empty")
+)
+
 var (
 	userdataUrl = url.URL{
 		Scheme: "https",
@@ -63,7 +67,7 @@ func FetchConfig(f resource.Fetcher) (types.Config, report.Report, error) {
 }
 
 // PostStatus posts a message that will show on the Packet Instance Timeline
-func PostStatus(f resource.Fetcher, errMsg error) error {
+func PostStatus(stageName string, f resource.Fetcher, errMsg error) error {
 	f.Logger.Info("POST message to Packet Timeline")
 	// fetch JSON from https://metadata.packet.net/metadata
 	data, err := f.FetchToBuffer(metadataUrl, resource.FetchOptions{
@@ -73,42 +77,29 @@ func PostStatus(f resource.Fetcher, errMsg error) error {
 		return err
 	}
 	if data == nil {
-		return errors.ErrValidFetchEmptyData
+		return ErrValidFetchEmptyData
 	}
-
 	metadata := struct {
 		PhoneHomeURL string `json:"phone_home_url"`
 	}{}
 	err = json.Unmarshal(data, &metadata)
 	if err != nil {
 		return err
-
 	}
 	phonehomeURL := metadata.PhoneHomeURL
 	// to get phonehome IPv4
 	phonehomeURL = strings.TrimSuffix(phonehomeURL, "/phone-home")
 	// POST Message to phonehome IP
 	postMessageURL := phonehomeURL + "/events"
-	if errMsg != nil {
-
-		err = postMessage(errMsg, f, "Ignition error: "+errMsg.Error(), postMessageURL)
-		if err != nil {
-			return err
-		}
 
-	} else {
-
-		err = postMessage(errMsg, f, "Ignition: valid config", postMessageURL)
-		if err != nil {
-			return err
-		}
-
-	}
-	return err
+	return postMessage(stageName, errMsg, postMessageURL)
 }
 
 // postMessage makes a post request with the supplied message to the url
-func postMessage(e error, f resource.Fetcher, message string, url string) error {
+func postMessage(stageName string, e error, url string) error {
+
+	stageName = "[" + stageName + "]"
+
 	type mStruct struct {
 		State   string `json:"state"`
 		Message string `json:"message"`
@@ -117,13 +108,13 @@ func postMessage(e error, f resource.Fetcher, message string, url string) error
 	if e != nil {
 		m = mStruct{
 			State:   "failed",
-			Message: message,
+			Message: stageName + " Ignition error: " + e.Error(),
 		}
 
 	} else {
 		m = mStruct{
 			State:   "succeeded",
-			Message: message,
+			Message: stageName + " Ignition status: finished successfully",
 		}
 	}
 	messageJSON, err := json.Marshal(m)

internal/providers/providers.go

@@ -29,4 +29,4 @@ var (
 
 type FuncFetchConfig func(f resource.Fetcher) (types.Config, report.Report, error)
 type FuncNewFetcher func(logger *log.Logger) (resource.Fetcher, error)
-type FuncPostStatus func(f resource.Fetcher, e error) error
+type FuncPostStatus func(stageName string, f resource.Fetcher, e error) error

internal/resource/url.go

@@ -246,6 +246,7 @@ func (f *Fetcher) FetchFromHTTP(u url.URL, dest *os.File, opts FetchOptions) err
 	default:
 		return ErrFailed
 	}
+
 	return f.decompressCopyHashAndVerify(dest, dataReader, opts)
 }