Skip to content

Commit

Permalink
Merge pull request #400 from abhinavdahiya/wrap_errors
Browse files Browse the repository at this point in the history
Wrap all errors with context
  • Loading branch information
openshift-merge-robot committed Oct 4, 2018
2 parents 93e9292 + 47aa8f8 commit 44b2220
Show file tree
Hide file tree
Showing 41 changed files with 685 additions and 172 deletions.
9 changes: 9 additions & 0 deletions Gopkg.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions Gopkg.toml
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,7 @@ ignored = ["github.com/openshift/installer/tests*"]
[[constraint]]
name = "k8s.io/client-go"
version = "6.0.0"

[[constraint]]
name = "github.com/pkg/errors"
version = "0.8.0"
2 changes: 1 addition & 1 deletion cmd/openshift-install/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func main() {
for _, asset := range targetAssets {
st, err := assetStore.Fetch(asset)
if err != nil {
logrus.Fatalf("Failed to generate asset: %v", err)
logrus.Fatalf("Failed to generate %s: %v", asset.Name(), err)
}

if err := st.PersistToFile(*dirFlag); err != nil {
Expand Down
7 changes: 4 additions & 3 deletions pkg/asset/asset.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
package asset

import (
"fmt"
"path/filepath"

"github.com/pkg/errors"
)

// Asset used to install OpenShift.
Expand All @@ -22,13 +23,13 @@ type Asset interface {
func GetDataByFilename(a Asset, parents map[Asset]*State, filename string) ([]byte, error) {
st, ok := parents[a]
if !ok {
return nil, fmt.Errorf("failed to find %T in parents", a)
return nil, errors.Errorf("failed to find %T in parents", a)
}

for _, c := range st.Contents {
if filepath.Base(c.Name) == filename {
return c.Data, nil
}
}
return nil, fmt.Errorf("failed to find data in %v with filename == %q", st, filename)
return nil, errors.Errorf("failed to find data in %v with filename == %q", st, filename)
}
14 changes: 7 additions & 7 deletions pkg/asset/cluster/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ package cluster

import (
"encoding/json"
"fmt"
"io/ioutil"
"os"
"path/filepath"

"github.com/pkg/errors"
"github.com/sirupsen/logrus"

"github.com/openshift/installer/data"
Expand Down Expand Up @@ -45,23 +45,23 @@ func (c *Cluster) Dependencies() []asset.Asset {
func (c *Cluster) Generate(parents map[asset.Asset]*asset.State) (*asset.State, error) {
state, ok := parents[c.tfvars]
if !ok {
return nil, fmt.Errorf("failed to get terraform.tfvar state in the parent asset states")
return nil, errors.Errorf("failed to get terraform.tfvars from parent")
}

// Copy the terraform.tfvars to a temp directory where the terraform will be invoked within.
tmpDir, err := ioutil.TempDir(os.TempDir(), "openshift-install-")
if err != nil {
return nil, fmt.Errorf("failed to create temp dir: %v", err)
return nil, errors.Wrap(err, "failed to create temp dir for terraform execution")
}
defer os.RemoveAll(tmpDir)

if err := ioutil.WriteFile(filepath.Join(tmpDir, state.Contents[0].Name), state.Contents[0].Data, 0600); err != nil {
return nil, fmt.Errorf("failed to write terraform.tfvars file: %v", err)
return nil, errors.Wrap(err, "failed to write terraform.tfvars file")
}

var tfvars config.Cluster
if err := json.Unmarshal(state.Contents[0].Data, &tfvars); err != nil {
return nil, fmt.Errorf("failed to unmarshal terraform tfvars file: %v", err)
return nil, errors.Wrap(err, "failed to Unmarshal terraform.tfvars file")
}

platform := string(tfvars.Platform)
Expand All @@ -78,12 +78,12 @@ func (c *Cluster) Generate(parents map[asset.Asset]*asset.State) (*asset.State,
// This runs the terraform in a temp directory, the tfstate file will be returned
// to the asset store to persist it on the disk.
if err := terraform.Init(tmpDir); err != nil {
return nil, err
return nil, errors.Wrap(err, "failed to initialize terraform")
}

stateFile, err := terraform.Apply(tmpDir)
if err != nil {
err = fmt.Errorf("terraform failed: %v", err)
err = errors.Wrap(err, "failed to run terraform")
}

data, err2 := ioutil.ReadFile(stateFile)
Expand Down
9 changes: 4 additions & 5 deletions pkg/asset/cluster/tfvars.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
package cluster

import (
"fmt"

"github.com/openshift/installer/pkg/asset"
"github.com/openshift/installer/pkg/asset/installconfig"
"github.com/openshift/installer/pkg/types/config"
"github.com/pkg/errors"
)

const (
Expand Down Expand Up @@ -39,7 +38,7 @@ func (t *TerraformVariables) Dependencies() []asset.Asset {
func (t *TerraformVariables) Generate(parents map[asset.Asset]*asset.State) (*asset.State, error) {
installCfg, err := installconfig.GetInstallConfig(t.installConfig, parents)
if err != nil {
return nil, fmt.Errorf("failed to get install config state in the parent asset states")
return nil, errors.Wrap(err, "failed to get InstallConfig from parents")
}

contents := map[asset.Asset][]string{}
Expand All @@ -51,7 +50,7 @@ func (t *TerraformVariables) Generate(parents map[asset.Asset]*asset.State) (*as
} {
state, ok := parents[ign]
if !ok {
return nil, fmt.Errorf("failed to get the ignition state for %v in the parent asset states", ign)
return nil, errors.Errorf("failed to get the ignition state for %v in the parent asset states", ign)
}

for _, content := range state.Contents {
Expand All @@ -72,7 +71,7 @@ func (t *TerraformVariables) Generate(parents map[asset.Asset]*asset.State) (*as

data, err := cluster.TFVars()
if err != nil {
return nil, err
return nil, errors.Wrap(err, "failed to get Tfvars")
}

return &asset.State{
Expand Down
9 changes: 5 additions & 4 deletions pkg/asset/ignition/bootstrap/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

"github.com/coreos/ignition/config/util"
igntypes "github.com/coreos/ignition/config/v2_2/types"
"github.com/pkg/errors"
log "github.com/sirupsen/logrus"

"github.com/openshift/installer/pkg/asset"
Expand Down Expand Up @@ -133,12 +134,12 @@ func (a *bootstrap) Dependencies() []asset.Asset {
func (a *bootstrap) Generate(dependencies map[asset.Asset]*asset.State) (*asset.State, error) {
installConfig, err := installconfig.GetInstallConfig(a.installConfig, dependencies)
if err != nil {
return nil, err
return nil, errors.Wrap(err, "failed to get InstallConfig from parents")
}

templateData, err := a.getTemplateData(installConfig)
if err != nil {
return nil, err
return nil, errors.Wrap(err, "failed to get bootstrap templates")
}

config := igntypes.Config{
Expand Down Expand Up @@ -166,7 +167,7 @@ func (a *bootstrap) Generate(dependencies map[asset.Asset]*asset.State) (*asset.

data, err := json.Marshal(config)
if err != nil {
return nil, err
return nil, errors.Wrap(err, "failed to Marshal Ignition config")
}

return &asset.State{
Expand All @@ -186,7 +187,7 @@ func (a *bootstrap) Name() string {
func (a *bootstrap) getTemplateData(installConfig *types.InstallConfig) (*bootstrapTemplateData, error) {
clusterDNSIP, err := installconfig.ClusterDNSIP(installConfig)
if err != nil {
return nil, err
return nil, errors.Wrap(err, "failed to get ClusterDNSIP from InstallConfig")
}
etcdEndpoints := make([]string, installConfig.MasterCount())
for i := range etcdEndpoints {
Expand Down
4 changes: 3 additions & 1 deletion pkg/asset/ignition/machine/master.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package machine
import (
"fmt"

"github.com/pkg/errors"

"github.com/openshift/installer/pkg/asset"
"github.com/openshift/installer/pkg/asset/installconfig"
"github.com/openshift/installer/pkg/asset/tls"
Expand Down Expand Up @@ -39,7 +41,7 @@ func (a *master) Dependencies() []asset.Asset {
func (a *master) Generate(dependencies map[asset.Asset]*asset.State) (*asset.State, error) {
installConfig, err := installconfig.GetInstallConfig(a.installConfig, dependencies)
if err != nil {
return nil, err
return nil, errors.Wrap(err, "failed to get InstallConfig from parents")
}

state := &asset.State{
Expand Down
4 changes: 3 additions & 1 deletion pkg/asset/ignition/machine/worker.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package machine

import (
"github.com/pkg/errors"

"github.com/openshift/installer/pkg/asset"
"github.com/openshift/installer/pkg/asset/installconfig"
"github.com/openshift/installer/pkg/asset/tls"
Expand Down Expand Up @@ -37,7 +39,7 @@ func (a *worker) Dependencies() []asset.Asset {
func (a *worker) Generate(dependencies map[asset.Asset]*asset.State) (*asset.State, error) {
installConfig, err := installconfig.GetInstallConfig(a.installConfig, dependencies)
if err != nil {
return nil, err
return nil, errors.Wrap(err, "failed to get InstallConfig from parents")
}

return &asset.State{
Expand Down
9 changes: 4 additions & 5 deletions pkg/asset/installconfig/installconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (

"github.com/apparentlymart/go-cidr/cidr"
"github.com/ghodss/yaml"

"github.com/pkg/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/openshift/installer/pkg/asset"
Expand Down Expand Up @@ -131,7 +131,7 @@ func (a *installConfig) Generate(dependencies map[asset.Asset]*asset.State) (*as

data, err := yaml.Marshal(installConfig)
if err != nil {
return nil, err
return nil, errors.Wrap(err, "failed to Marshal InstallConfig")
}

return &asset.State{
Expand All @@ -155,13 +155,12 @@ func GetInstallConfig(installConfig asset.Asset, parents map[asset.Asset]*asset.

st, ok := parents[installConfig]
if !ok {
return nil, fmt.Errorf("failed to find %T in parents", installConfig)
return nil, errors.Errorf("%T does not exist in parents", installConfig)
}

if err := yaml.Unmarshal(st.Contents[0].Data, &cfg); err != nil {
return nil, fmt.Errorf("failed to unmarshal the installconfig: %v", err)
return nil, errors.Wrap(err, "failed to Unmarshal the installconfig")
}

return &cfg, nil
}

Expand Down
21 changes: 15 additions & 6 deletions pkg/asset/installconfig/platform.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"sort"
"strings"

"github.com/pkg/errors"
survey "gopkg.in/AlecAivazis/survey.v1"

"github.com/openshift/installer/pkg/asset"
Expand Down Expand Up @@ -100,6 +101,7 @@ func (a *Platform) Name() string {
func (a *Platform) queryUserForPlatform() (string, error) {
sort.Strings(validPlatforms)
prompt := asset.UserProvided{
AssetName: "Platform",
Question: &survey.Question{
Prompt: &survey.Select{
Message: "Platform",
Expand All @@ -109,7 +111,7 @@ func (a *Platform) queryUserForPlatform() (string, error) {
choice := ans.(string)
i := sort.SearchStrings(validPlatforms, choice)
if i == len(validPlatforms) || validPlatforms[i] != choice {
return fmt.Errorf("invalid platform %q", choice)
return errors.Errorf("invalid platform %q", choice)
}
return nil
}),
Expand Down Expand Up @@ -142,6 +144,7 @@ func (a *Platform) awsPlatform() (*asset.State, error) {
sort.Strings(longRegions)
sort.Strings(shortRegions)
prompt := asset.UserProvided{
AssetName: "AWS Region",
Question: &survey.Question{
Prompt: &survey.Select{
Message: "Region",
Expand All @@ -153,7 +156,7 @@ func (a *Platform) awsPlatform() (*asset.State, error) {
choice := regionTransform(ans).(string)
i := sort.SearchStrings(shortRegions, choice)
if i == len(shortRegions) || shortRegions[i] != choice {
return fmt.Errorf("invalid region %q", choice)
return errors.Errorf("invalid region %q", choice)
}
return nil
}),
Expand All @@ -169,13 +172,13 @@ func (a *Platform) awsPlatform() (*asset.State, error) {

if value, ok := os.LookupEnv("_CI_ONLY_STAY_AWAY_OPENSHIFT_INSTALL_AWS_USER_TAGS"); ok {
if err := json.Unmarshal([]byte(value), &platform.UserTags); err != nil {
return nil, fmt.Errorf("_CI_ONLY_STAY_AWAY_OPENSHIFT_INSTALL_AWS_USER_TAGS contains invalid JSON: %s (%v)", value, err)
return nil, errors.Wrapf(err, "_CI_ONLY_STAY_AWAY_OPENSHIFT_INSTALL_AWS_USER_TAGS contains invalid JSON: %s", value)
}
}

data, err := json.Marshal(platform)
if err != nil {
return nil, err
return nil, errors.Wrapf(err, "failed to Marshal %s platform", AWSPlatformType)
}

return &asset.State{
Expand All @@ -197,6 +200,7 @@ func (a *Platform) openstackPlatform() (*asset.State, error) {
NetworkCIDRBlock: defaultVPCCIDR,
}
prompt := asset.UserProvided{
AssetName: "OpenStack Region",
Question: &survey.Question{
Prompt: &survey.Select{
Message: "Region",
Expand All @@ -217,6 +221,7 @@ func (a *Platform) openstackPlatform() (*asset.State, error) {
}
platform.Region = string(region.Contents[0].Data)
prompt2 := asset.UserProvided{
AssetName: "OpenStack Image",
Question: &survey.Question{
Prompt: &survey.Select{
Message: "Image",
Expand All @@ -237,6 +242,7 @@ func (a *Platform) openstackPlatform() (*asset.State, error) {
}
platform.BaseImage = string(image.Contents[0].Data)
prompt3 := asset.UserProvided{
AssetName: "OpenStack Cloud",
Question: &survey.Question{
Prompt: &survey.Select{
Message: "Cloud",
Expand All @@ -256,6 +262,7 @@ func (a *Platform) openstackPlatform() (*asset.State, error) {
}
platform.Cloud = string(cloud.Contents[0].Data)
prompt4 := asset.UserProvided{
AssetName: "OpenStack External Network",
Question: &survey.Question{
Prompt: &survey.Select{
Message: "ExternalNetwork",
Expand All @@ -277,7 +284,7 @@ func (a *Platform) openstackPlatform() (*asset.State, error) {

data, err := json.Marshal(platform)
if err != nil {
return nil, err
return nil, errors.Wrapf(err, "failed to Marshal %s platform", OpenStackPlatformType)
}

return &asset.State{
Expand Down Expand Up @@ -307,6 +314,7 @@ func (a *Platform) libvirtPlatform() (*asset.State, error) {
}

uriPrompt := asset.UserProvided{
AssetName: "Libvirt Connection URI",
Question: &survey.Question{
Prompt: &survey.Input{
Message: "Libvirt Connection URI",
Expand All @@ -323,6 +331,7 @@ func (a *Platform) libvirtPlatform() (*asset.State, error) {
}

imagePrompt := asset.UserProvided{
AssetName: "Libvirt Image",
Question: &survey.Question{
Prompt: &survey.Input{
Message: "Image",
Expand All @@ -342,7 +351,7 @@ func (a *Platform) libvirtPlatform() (*asset.State, error) {

data, err := json.Marshal(platform)
if err != nil {
return nil, err
return nil, errors.Wrapf(err, "failed to Marshal %s platform", LibvirtPlatformType)
}

return &asset.State{
Expand Down
Loading

0 comments on commit 44b2220

Please sign in to comment.