From fa033e985ea637bbed62333285430ff65d016834 Mon Sep 17 00:00:00 2001 From: Matt Ellis Date: Tue, 29 Aug 2017 18:15:53 -0700 Subject: [PATCH] Adopt error at API boundary --- pkg/engine/config_delete.go | 2 +- pkg/engine/config_set.go | 4 ++-- pkg/engine/deploy.go | 2 +- pkg/engine/env.go | 44 ++++++++++++++++++++----------------- pkg/engine/env_info.go | 6 ++--- pkg/engine/env_list.go | 4 ++-- pkg/engine/env_remove.go | 3 +-- 7 files changed, 34 insertions(+), 31 deletions(-) diff --git a/pkg/engine/config_delete.go b/pkg/engine/config_delete.go index 2aa87cef9..c06bb2e39 100644 --- a/pkg/engine/config_delete.go +++ b/pkg/engine/config_delete.go @@ -15,7 +15,7 @@ func (eng *Engine) DeleteConfig(envName string, key string) error { if config != nil { delete(config, tokens.Token(key)) - if !eng.saveEnv(info.Target, info.Snapshot) { + if err = eng.saveEnv(info.Target, info.Snapshot); err != nil { return errors.Errorf("could not save configuration value") } } diff --git a/pkg/engine/config_set.go b/pkg/engine/config_set.go index ab410ccb7..7014ea860 100644 --- a/pkg/engine/config_set.go +++ b/pkg/engine/config_set.go @@ -20,7 +20,7 @@ func (eng *Engine) SetConfig(envName string, key string, value string) error { config[tokens.Token(key)] = value - if !eng.saveEnv(info.Target, info.Snapshot) { + if err = eng.saveEnv(info.Target, info.Snapshot); err != nil { return errors.Errorf("could not save configuration value") } @@ -43,7 +43,7 @@ func (eng *Engine) ReplaceConfig(envName string, newConfig map[string]string) er info.Target.Config = config - if !eng.saveEnv(info.Target, info.Snapshot) { + if err = eng.saveEnv(info.Target, info.Snapshot); err != nil { return errors.Errorf("could not save configuration value") } diff --git a/pkg/engine/deploy.go b/pkg/engine/deploy.go index e31e06718..d2d7458a3 100644 --- a/pkg/engine/deploy.go +++ b/pkg/engine/deploy.go @@ -104,7 +104,7 @@ func (eng *Engine) deployLatest(info *envCmdInfo, opts deployOptions) error { // Now save the updated snapshot Notee that if a failure has occurred, the Apply routine above will // have returned a safe checkpoint. targ := result.Info.Target - eng.saveEnv(targ, summary.Snap()) + _ = eng.saveEnv(targ, summary.Snap()) fmt.Fprint(eng.Stdout, colors.Colorize(&footer)) return err diff --git a/pkg/engine/env.go b/pkg/engine/env.go index 23761615d..3669e26f1 100644 --- a/pkg/engine/env.go +++ b/pkg/engine/env.go @@ -36,8 +36,8 @@ func (eng *Engine) initEnvCmdName(name tokens.QName, pkgarg string) (*envCmdInfo } // Read in the deployment information, bailing if an IO error occurs. - target, snapshot, checkpoint := eng.readEnv(name) - if checkpoint == nil { + target, snapshot, checkpoint, err := eng.readEnv(name) + if err != nil { return nil, goerr.Errorf("could not read environment information") } @@ -61,7 +61,7 @@ type envCmdInfo struct { // createEnv just creates a new empty environment without deploying anything into it. func (eng *Engine) createEnv(name tokens.QName) { env := &deploy.Target{Name: name} - if success := eng.saveEnv(env, nil); success { + if err := eng.saveEnv(env, nil); err == nil { fmt.Fprintf(eng.Stdout, "Environment '%v' initialized; see `lumi deploy` to deploy into it\n", name) eng.setCurrentEnv(name, false) } @@ -93,7 +93,7 @@ func (eng *Engine) getCurrentEnv() tokens.QName { // setCurrentEnv changes the current environment to the given environment name, issuing an error if it doesn't exist. func (eng *Engine) setCurrentEnv(name tokens.QName, verify bool) { if verify { - if _, _, checkpoint := eng.readEnv(name); checkpoint == nil { + if _, _, _, err := eng.readEnv(name); err != nil { return // no environment by this name exists, bail out. } } @@ -110,11 +110,14 @@ func (eng *Engine) setCurrentEnv(name tokens.QName, verify bool) { } // removeTarget permanently deletes the environment's information from the local workstation. -func (eng *Engine) removeTarget(env *deploy.Target) { - deleteTarget(env) +func (eng *Engine) removeTarget(env *deploy.Target) error { + if err := deleteTarget(env); err != nil { + return err + } msg := fmt.Sprintf("%sEnvironment '%s' has been removed!%s\n", colors.SpecAttention, env.Name, colors.Reset) fmt.Fprint(eng.Stdout, colors.ColorizeText(msg)) + return nil } // backupTarget makes a backup of an existing file, in preparation for writing a new one. Instead of a copy, it @@ -127,15 +130,16 @@ func backupTarget(file string) { } // deleteTarget removes an existing snapshot file, leaving behind a backup. -func deleteTarget(env *deploy.Target) { +func deleteTarget(env *deploy.Target) error { contract.Require(env != nil, "env") // Just make a backup of the file and don't write out anything new. file := workspace.EnvPath(env.Name) backupTarget(file) + return nil } // readEnv reads in an existing snapshot file, issuing an error and returning nil if something goes awry. -func (eng *Engine) readEnv(name tokens.QName) (*deploy.Target, *deploy.Snapshot, *environment.Checkpoint) { +func (eng *Engine) readEnv(name tokens.QName) (*deploy.Target, *deploy.Snapshot, *environment.Checkpoint, error) { contract.Require(name != "", "name") file := workspace.EnvPath(name) @@ -143,7 +147,7 @@ func (eng *Engine) readEnv(name tokens.QName) (*deploy.Target, *deploy.Snapshot, m, ext := encoding.Detect(file) if m == nil { glog.Errorf("Resource deserialization failed; illegal markup extension: '%v'", ext) - return nil, nil, nil + return nil, nil, nil, fmt.Errorf("resource deserialization failed; illegal markup extension: '%v'", ext) } // Now read the whole file into a byte blob. @@ -154,14 +158,14 @@ func (eng *Engine) readEnv(name tokens.QName) (*deploy.Target, *deploy.Snapshot, } else { glog.Errorf("An IO error occurred during the current operation: %v", err) } - return nil, nil, nil + return nil, nil, nil, err } // Unmarshal the contents into a checkpoint structure. var checkpoint environment.Checkpoint if err = m.Unmarshal(b, &checkpoint); err != nil { glog.Errorf("Could not read deployment file '%v': %v", file, err) - return nil, nil, nil + return nil, nil, nil, err } // Next, use the mapping infrastructure to validate the contents. @@ -169,7 +173,7 @@ func (eng *Engine) readEnv(name tokens.QName) (*deploy.Target, *deploy.Snapshot, var obj map[string]interface{} if err = m.Unmarshal(b, &obj); err != nil { glog.Errorf("Could not read deployment file '%v': %v", file, err) - return nil, nil, nil + return nil, nil, nil, err } if obj["latest"] != nil { @@ -181,16 +185,16 @@ func (eng *Engine) readEnv(name tokens.QName) (*deploy.Target, *deploy.Snapshot, var ignore environment.Checkpoint // just for errors. if err = md.Decode(obj, &ignore); err != nil { glog.Errorf("Could not read deployment file '%v': %v", file, err) - return nil, nil, nil + return nil, nil, nil, err } target, snapshot := environment.DeserializeCheckpoint(&checkpoint) contract.Assert(target != nil) - return target, snapshot, &checkpoint + return target, snapshot, &checkpoint, nil } // saveEnv saves a new snapshot at the given location, backing up any existing ones. -func (eng *Engine) saveEnv(env *deploy.Target, snap *deploy.Snapshot) bool { +func (eng *Engine) saveEnv(env *deploy.Target, snap *deploy.Snapshot) error { contract.Require(env != nil, "env") file := workspace.EnvPath(env.Name) @@ -198,7 +202,7 @@ func (eng *Engine) saveEnv(env *deploy.Target, snap *deploy.Snapshot) bool { m, ext := encoding.Detect(file) if m == nil { glog.Errorf("Resource serialization failed; illegal markup extension: '%v'", ext) - return false + return fmt.Errorf("resource serialization failed; illegal markup extension: '%v'", ext) } if filepath.Ext(file) == "" { file = file + ext @@ -207,7 +211,7 @@ func (eng *Engine) saveEnv(env *deploy.Target, snap *deploy.Snapshot) bool { b, err := m.Marshal(dep) if err != nil { glog.Errorf("An IO error occurred during the current operation: %v", err) - return false + return err } // Back up the existing file if it already exists. @@ -216,14 +220,14 @@ func (eng *Engine) saveEnv(env *deploy.Target, snap *deploy.Snapshot) bool { // Ensure the directory exists. if err = os.MkdirAll(filepath.Dir(file), 0700); err != nil { glog.Errorf("An IO error occurred during the current operation: %v", err) - return false + return err } // And now write out the new snapshot file, overwriting that location. if err = ioutil.WriteFile(file, b, 0600); err != nil { glog.Errorf("An IO error occurred during the current operation: %v", err) - return false + return err } - return true + return nil } diff --git a/pkg/engine/env_info.go b/pkg/engine/env_info.go index 6804284be..54ec8a418 100644 --- a/pkg/engine/env_info.go +++ b/pkg/engine/env_info.go @@ -17,9 +17,9 @@ func (eng *Engine) EnvInfo(showIDs bool, showURNs bool) error { } fmt.Fprintf(eng.Stdout, "Current environment is %v\n", curr) fmt.Fprintf(eng.Stdout, " (use `lumi env select` to change environments; `lumi env ls` lists known ones)\n") - target, snapshot, checkpoint := eng.readEnv(curr) - if checkpoint == nil { - return errors.Errorf("could not read environment information") + target, snapshot, checkpoint, err := eng.readEnv(curr) + if err != nil { + return err } if checkpoint.Latest != nil { fmt.Fprintf(eng.Stdout, "Last deployment at %v\n", checkpoint.Latest.Time) diff --git a/pkg/engine/env_list.go b/pkg/engine/env_list.go index 1cc4a7d77..bb640be09 100644 --- a/pkg/engine/env_list.go +++ b/pkg/engine/env_list.go @@ -38,8 +38,8 @@ func (eng *Engine) ListEnvs() error { // Read in this environment's information. name := tokens.QName(envfn[:len(envfn)-len(ext)]) - target, snapshot, checkpoint := eng.readEnv(name) - if checkpoint == nil { + target, snapshot, checkpoint, err := eng.readEnv(name) + if err != nil { continue // failure reading the environment information. } diff --git a/pkg/engine/env_remove.go b/pkg/engine/env_remove.go index 57160ca5b..7b86696ff 100644 --- a/pkg/engine/env_remove.go +++ b/pkg/engine/env_remove.go @@ -20,6 +20,5 @@ func (eng *Engine) RemoveEnv(envName string, force bool) error { "'%v' still has resources; removal rejected; pass --force to override", info.Target.Name) } - eng.removeTarget(info.Target) - return nil + return eng.removeTarget(info.Target) }