Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

Commit

Permalink
Cleanup error usage (#3386)
Browse files Browse the repository at this point in the history
  • Loading branch information
@cpuguy83 @jackfrancis
cpuguy83 authored and jackfrancis committed Jul 3, 2018
1 parent 20b95b4 commit 9993e58
Show file tree
Hide file tree
Showing 33 changed files with 2,113 additions and 194 deletions.
32 changes: 16 additions & 16 deletions cmd/dcos-upgrade.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package cmd

import (
"encoding/json"
"fmt"
"io/ioutil"
"os"
"path"
Expand All @@ -15,6 +14,7 @@ import (
"github.com/Azure/acs-engine/pkg/i18n"
"github.com/Azure/acs-engine/pkg/operations/dcosupgrade"
"github.com/leonelquinteros/gotext"
"github.com/pkg/errors"

log "github.com/sirupsen/logrus"
"github.com/spf13/cobra"
Expand Down Expand Up @@ -76,40 +76,40 @@ func (uc *dcosUpgradeCmd) validate(cmd *cobra.Command) error {

uc.locale, err = i18n.LoadTranslations()
if err != nil {
return fmt.Errorf("error loading translation files: %s", err.Error())
return errors.Wrap(err, "error loading translation files")
}

if len(uc.resourceGroupName) == 0 {
cmd.Usage()
return fmt.Errorf("--resource-group must be specified")
return errors.New("--resource-group must be specified")
}

if len(uc.location) == 0 {
cmd.Usage()
return fmt.Errorf("--location must be specified")
return errors.New("--location must be specified")
}
uc.location = helpers.NormalizeAzureRegion(uc.location)

if len(uc.upgradeVersion) == 0 {
cmd.Usage()
return fmt.Errorf("--upgrade-version must be specified")
return errors.New("--upgrade-version must be specified")
}

if len(uc.deploymentDirectory) == 0 {
cmd.Usage()
return fmt.Errorf("--deployment-dir must be specified")
return errors.New("--deployment-dir must be specified")
}

if len(uc.sshPrivateKeyPath) == 0 {
uc.sshPrivateKeyPath = filepath.Join(uc.deploymentDirectory, "id_rsa")
}
if uc.sshPrivateKey, err = ioutil.ReadFile(uc.sshPrivateKeyPath); err != nil {
cmd.Usage()
return fmt.Errorf("ssh-private-key-path must be specified: %s", err)
return errors.Wrap(err, "ssh-private-key-path must be specified")
}

if err = uc.authArgs.validateAuthArgs(); err != nil {
return fmt.Errorf("%s", err)
return err
}
return nil
}
Expand All @@ -118,19 +118,19 @@ func (uc *dcosUpgradeCmd) loadCluster(cmd *cobra.Command) error {
var err error

if uc.client, err = uc.authArgs.getClient(); err != nil {
return fmt.Errorf("Failed to get client: %s", err)
return errors.Wrap(err, "Failed to get client")
}

_, err = uc.client.EnsureResourceGroup(uc.resourceGroupName, uc.location, nil)
if err != nil {
return fmt.Errorf("Error ensuring resource group: %s", err)
return errors.Wrap(err, "Error ensuring resource group")
}

// load apimodel from the deployment directory
apiModelPath := path.Join(uc.deploymentDirectory, "apimodel.json")

if _, err = os.Stat(apiModelPath); os.IsNotExist(err) {
return fmt.Errorf("specified api model does not exist (%s)", apiModelPath)
return errors.Errorf("specified api model does not exist (%s)", apiModelPath)
}

apiloader := &api.Apiloader{
Expand All @@ -140,24 +140,24 @@ func (uc *dcosUpgradeCmd) loadCluster(cmd *cobra.Command) error {
}
uc.containerService, uc.apiVersion, err = apiloader.LoadContainerServiceFromFile(apiModelPath, true, true, nil)
if err != nil {
return fmt.Errorf("error parsing the api model: %s", err.Error())
return errors.Wrap(err, "error parsing the api model")
}
uc.currentDcosVersion = uc.containerService.Properties.OrchestratorProfile.OrchestratorVersion

if uc.currentDcosVersion == uc.upgradeVersion {
return fmt.Errorf("already running DCOS %s", uc.upgradeVersion)
return errors.Errorf("already running DCOS %s", uc.upgradeVersion)
}

if len(uc.containerService.Location) == 0 {
uc.containerService.Location = uc.location
} else if uc.containerService.Location != uc.location {
return fmt.Errorf("--location does not match api model location")
return errors.New("--location does not match api model location")
}

// get available upgrades for container service
orchestratorInfo, err := api.GetOrchestratorVersionProfile(uc.containerService.Properties.OrchestratorProfile)
if err != nil {
return fmt.Errorf("error getting list of available upgrades: %s", err.Error())
return errors.Wrap(err, "error getting list of available upgrades")
}
// add the current version if upgrade has failed
orchestratorInfo.Upgrades = append(orchestratorInfo.Upgrades, &api.OrchestratorProfile{
Expand All @@ -174,7 +174,7 @@ func (uc *dcosUpgradeCmd) loadCluster(cmd *cobra.Command) error {
}
}
if !found {
return fmt.Errorf("upgrade to DCOS %s is not supported", uc.upgradeVersion)
return errors.Errorf("upgrade to DCOS %s is not supported", uc.upgradeVersion)
}

// Read name suffix to identify nodes in the resource group that belong
Expand Down
44 changes: 22 additions & 22 deletions cmd/deploy.go
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package cmd

import (
"errors"
"fmt"
"io/ioutil"
"math/rand"
Expand All @@ -24,6 +23,7 @@ import (
"github.com/Azure/acs-engine/pkg/i18n"
"github.com/Azure/azure-sdk-for-go/arm/graphrbac"
"github.com/Azure/go-autorest/autorest/to"
"github.com/pkg/errors"
)

const (
Expand Down Expand Up @@ -74,10 +74,10 @@ func newDeployCmd() *cobra.Command {
Long: deployLongDescription,
RunE: func(cmd *cobra.Command, args []string) error {
if err := dc.validateArgs(cmd, args); err != nil {
log.Fatalf(fmt.Sprintf("error validating deployCmd: %s", err.Error()))
log.Fatalf("error validating deployCmd: %s", err.Error())
}
if err := dc.mergeAPIModel(); err != nil {
log.Fatalf(fmt.Sprintf("error merging API model in deployCmd: %s", err.Error()))
log.Fatalf("error merging API model in deployCmd: %s", err.Error())
}
if err := dc.loadAPIModel(cmd, args); err != nil {
log.Fatalln("failed to load apimodel: %s", err.Error())
Expand Down Expand Up @@ -111,27 +111,27 @@ func (dc *deployCmd) validateArgs(cmd *cobra.Command, args []string) error {

dc.locale, err = i18n.LoadTranslations()
if err != nil {
return fmt.Errorf(fmt.Sprintf("error loading translation files: %s", err.Error()))
return errors.Wrap(err, "error loading translation files")
}

if dc.apimodelPath == "" {
if len(args) == 1 {
dc.apimodelPath = args[0]
} else if len(args) > 1 {
cmd.Usage()
return fmt.Errorf(fmt.Sprintf("too many arguments were provided to 'deploy'"))
return errors.New("too many arguments were provided to 'deploy'")
} else {
cmd.Usage()
return fmt.Errorf(fmt.Sprintf("--api-model was not supplied, nor was one specified as a positional argument"))
return errors.New("--api-model was not supplied, nor was one specified as a positional argument")
}
}

if _, err := os.Stat(dc.apimodelPath); os.IsNotExist(err) {
return fmt.Errorf(fmt.Sprintf("specified api model does not exist (%s)", dc.apimodelPath))
return errors.Errorf("specified api model does not exist (%s)", dc.apimodelPath)
}

if dc.location == "" {
return fmt.Errorf(fmt.Sprintf("--location must be specified"))
return errors.New("--location must be specified")
}
dc.location = helpers.NormalizeAzureRegion(dc.location)

Expand All @@ -149,7 +149,7 @@ func (dc *deployCmd) mergeAPIModel() error {
// overrides the api model and generates a new file
dc.apimodelPath, err = transform.MergeValuesWithAPIModel(dc.apimodelPath, m)
if err != nil {
return fmt.Errorf(fmt.Sprintf("error merging --set values with the api model: %s", err.Error()))
return errors.Wrap(err, "error merging --set values with the api model: %s")
}

log.Infoln(fmt.Sprintf("new api model file has been generated during merge: %s", dc.apimodelPath))
Expand All @@ -172,7 +172,7 @@ func (dc *deployCmd) loadAPIModel(cmd *cobra.Command, args []string) error {
// do not validate when initially loading the apimodel, validation is done later after autofilling values
dc.containerService, dc.apiVersion, err = apiloader.LoadContainerServiceFromFile(dc.apimodelPath, false, false, nil)
if err != nil {
return fmt.Errorf(fmt.Sprintf("error parsing the api model: %s", err.Error()))
return errors.Wrap(err, "error parsing the api model")
}

if dc.outputDirectory == "" {
Expand All @@ -190,10 +190,10 @@ func (dc *deployCmd) loadAPIModel(cmd *cobra.Command, args []string) error {

if dc.caCertificatePath != "" {
if caCertificateBytes, err = ioutil.ReadFile(dc.caCertificatePath); err != nil {
return fmt.Errorf(fmt.Sprintf("failed to read CA certificate file: %s", err.Error()))
return errors.Wrap(err, "failed to read CA certificate file")
}
if caKeyBytes, err = ioutil.ReadFile(dc.caPrivateKeyPath); err != nil {
return fmt.Errorf(fmt.Sprintf("failed to read CA private key file: %s", err.Error()))
return errors.Wrap(err, "failed to read CA private key file")
}

prop := dc.containerService.Properties
Expand All @@ -207,16 +207,16 @@ func (dc *deployCmd) loadAPIModel(cmd *cobra.Command, args []string) error {
if dc.containerService.Location == "" {
dc.containerService.Location = dc.location
} else if dc.containerService.Location != dc.location {
return fmt.Errorf(fmt.Sprintf("--location does not match api model location"))
return errors.New("--location does not match api model location")
}

if err = dc.authArgs.validateAuthArgs(); err != nil {
return fmt.Errorf("%s", err)
return err
}

dc.client, err = dc.authArgs.getClient()
if err != nil {
return fmt.Errorf("failed to get client: %s", err.Error())
return errors.Wrap(err, "failed to get client")
}

if err = autofillApimodel(dc); err != nil {
Expand All @@ -239,11 +239,11 @@ func autofillApimodel(dc *deployCmd) error {
}

if dc.dnsPrefix != "" && dc.containerService.Properties.MasterProfile.DNSPrefix != "" {
return fmt.Errorf("invalid configuration: the apimodel masterProfile.dnsPrefix and --dns-prefix were both specified")
return errors.New("invalid configuration: the apimodel masterProfile.dnsPrefix and --dns-prefix were both specified")
}
if dc.containerService.Properties.MasterProfile.DNSPrefix == "" {
if dc.dnsPrefix == "" {
return fmt.Errorf("apimodel: missing masterProfile.dnsPrefix and --dns-prefix was not specified")
return errors.New("apimodel: missing masterProfile.dnsPrefix and --dns-prefix was not specified")
}
log.Warnf("apimodel: missing masterProfile.dnsPrefix will use %q", dc.dnsPrefix)
dc.containerService.Properties.MasterProfile.DNSPrefix = dc.dnsPrefix
Expand All @@ -259,15 +259,15 @@ func autofillApimodel(dc *deployCmd) error {
}

if _, err := os.Stat(dc.outputDirectory); !dc.forceOverwrite && err == nil {
return fmt.Errorf("Output directory already exists and forceOverwrite flag is not set: %s", dc.outputDirectory)
return errors.Errorf("Output directory already exists and forceOverwrite flag is not set: %s", dc.outputDirectory)
}

if dc.resourceGroup == "" {
dnsPrefix := dc.containerService.Properties.MasterProfile.DNSPrefix
log.Warnf("--resource-group was not specified. Using the DNS prefix from the apimodel as the resource group name: %s", dnsPrefix)
dc.resourceGroup = dnsPrefix
if dc.location == "" {
return fmt.Errorf("--resource-group was not specified. --location must be specified in case the resource group needs creation")
return errors.New("--resource-group was not specified. --location must be specified in case the resource group needs creation")
}
}

Expand All @@ -279,7 +279,7 @@ func autofillApimodel(dc *deployCmd) error {
}
_, publicKey, err := acsengine.CreateSaveSSH(dc.containerService.Properties.LinuxProfile.AdminUsername, dc.outputDirectory, translator)
if err != nil {
return fmt.Errorf("Failed to generate SSH Key: %s", err.Error())
return errors.Wrap(err, "Failed to generate SSH Key")
}

dc.containerService.Properties.LinuxProfile.SSH.PublicKeys = []api.PublicKey{{KeyData: publicKey}}
Expand Down Expand Up @@ -321,15 +321,15 @@ func autofillApimodel(dc *deployCmd) error {
}
applicationID, servicePrincipalObjectID, secret, err := dc.client.CreateApp(appName, appURL, replyURLs, requiredResourceAccess)
if err != nil {
return fmt.Errorf("apimodel invalid: ServicePrincipalProfile was empty, and we failed to create valid credentials: %q", err)
return errors.Wrap(err, "apimodel invalid: ServicePrincipalProfile was empty, and we failed to create valid credentials")
}
log.Warnf("created application with applicationID (%s) and servicePrincipalObjectID (%s).", applicationID, servicePrincipalObjectID)

log.Warnln("apimodel: ServicePrincipalProfile was empty, assigning role to application...")

err = dc.client.CreateRoleAssignmentSimple(dc.resourceGroup, servicePrincipalObjectID)
if err != nil {
return fmt.Errorf("apimodel: could not create or assign ServicePrincipal: %q", err)
return errors.Wrap(err, "apimodel: could not create or assign ServicePrincipal")

}

Expand Down
14 changes: 7 additions & 7 deletions cmd/generate.go
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package cmd

import (
"errors"
"fmt"
"io/ioutil"
"os"
Expand All @@ -12,6 +11,7 @@ import (
"github.com/Azure/acs-engine/pkg/api"
"github.com/Azure/acs-engine/pkg/i18n"
"github.com/leonelquinteros/gotext"
"github.com/pkg/errors"
log "github.com/sirupsen/logrus"
"github.com/spf13/cobra"
)
Expand Down Expand Up @@ -80,7 +80,7 @@ func (gc *generateCmd) validate(cmd *cobra.Command, args []string) error {

gc.locale, err = i18n.LoadTranslations()
if err != nil {
return fmt.Errorf(fmt.Sprintf("error loading translation files: %s", err.Error()))
return errors.Wrap(err, "error loading translation files")
}

if gc.apimodelPath == "" {
Expand All @@ -96,7 +96,7 @@ func (gc *generateCmd) validate(cmd *cobra.Command, args []string) error {
}

if _, err := os.Stat(gc.apimodelPath); os.IsNotExist(err) {
return fmt.Errorf(fmt.Sprintf("specified api model does not exist (%s)", gc.apimodelPath))
return errors.Errorf("specified api model does not exist (%s)", gc.apimodelPath)
}

return nil
Expand All @@ -113,7 +113,7 @@ func (gc *generateCmd) mergeAPIModel() error {
// overrides the api model and generates a new file
gc.apimodelPath, err = transform.MergeValuesWithAPIModel(gc.apimodelPath, m)
if err != nil {
return fmt.Errorf(fmt.Sprintf("error merging --set values with the api model: %s", err.Error()))
return errors.Wrap(err, "error merging --set values with the api model")
}

log.Infoln(fmt.Sprintf("new api model file has been generated during merge: %s", gc.apimodelPath))
Expand All @@ -134,7 +134,7 @@ func (gc *generateCmd) loadAPIModel(cmd *cobra.Command, args []string) error {
}
gc.containerService, gc.apiVersion, err = apiloader.LoadContainerServiceFromFile(gc.apimodelPath, true, false, nil)
if err != nil {
return fmt.Errorf(fmt.Sprintf("error parsing the api model: %s", err.Error()))
return errors.Wrap(err, "error parsing the api model")
}

if gc.outputDirectory == "" {
Expand All @@ -152,10 +152,10 @@ func (gc *generateCmd) loadAPIModel(cmd *cobra.Command, args []string) error {
}
if gc.caCertificatePath != "" {
if caCertificateBytes, err = ioutil.ReadFile(gc.caCertificatePath); err != nil {
return fmt.Errorf(fmt.Sprintf("failed to read CA certificate file: %s", err.Error()))
return errors.Wrap(err, "failed to read CA certificate file")
}
if caKeyBytes, err = ioutil.ReadFile(gc.caPrivateKeyPath); err != nil {
return fmt.Errorf(fmt.Sprintf("failed to read CA private key file: %s", err.Error()))
return errors.Wrap(err, "failed to read CA private key file")
}

prop := gc.containerService.Properties
Expand Down
Loading

0 comments on commit 9993e58

Please sign in to comment.