Skip to content

Commit

Permalink
Merge pull request #1255 from dgageot/improve-errors
Browse files Browse the repository at this point in the history
Improve errors
  • Loading branch information
dgageot authored Nov 8, 2018
2 parents d1648a8 + 5f9be2d commit a70f7b7
Show file tree
Hide file tree
Showing 11 changed files with 33 additions and 28 deletions.
7 changes: 5 additions & 2 deletions pkg/skaffold/build/tag/custom.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package tag

import (
"errors"
"fmt"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
Expand All @@ -35,11 +36,13 @@ func (c *CustomTag) Labels() map[string]string {
// GenerateFullyQualifiedImageName tags an image with the custom tag
func (c *CustomTag) GenerateFullyQualifiedImageName(workingDir string, opts *Options) (string, error) {
if opts == nil {
return "", fmt.Errorf("Tag options not provided")
return "", errors.New("tag options not provided")
}

tag := c.Tag
if tag == "" {
return "", fmt.Errorf("Custom tag not provided")
return "", errors.New("custom tag not provided")
}

return fmt.Sprintf("%s:%s", opts.ImageName, tag), nil
}
5 changes: 3 additions & 2 deletions pkg/skaffold/build/tag/sha256.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"strings"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
"github.com/pkg/errors"
)

// ChecksumTagger tags an image by the sha256 of the image tarball
Expand All @@ -36,13 +37,13 @@ func (c *ChecksumTagger) Labels() map[string]string {
// GenerateFullyQualifiedImageName tags an image with the supplied image name and the sha256 checksum of the image
func (c *ChecksumTagger) GenerateFullyQualifiedImageName(workingDir string, opts *Options) (string, error) {
if opts == nil {
return "", fmt.Errorf("Tag options not provided")
return "", errors.New("tag options not provided")
}

digest := opts.Digest
sha256 := strings.TrimPrefix(opts.Digest, "sha256:")
if sha256 == digest {
return "", fmt.Errorf("Digest wrong format: %s, expected sha256:<checksum>", digest)
return "", fmt.Errorf("digest wrong format: %s, expected sha256:<checksum>", digest)
}

return fmt.Sprintf("%s:%s", opts.ImageName, sha256), nil
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/deploy/labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ func groupVersionResource(disco discovery.DiscoveryInterface, gvk schema.GroupVe
}
}

return schema.GroupVersionResource{}, fmt.Errorf("Could not find resource for %s", gvk.String())
return schema.GroupVersionResource{}, fmt.Errorf("could not find resource for %s", gvk.String())
}

func copyMap(dest, from map[string]string) {
Expand Down
10 changes: 5 additions & 5 deletions pkg/skaffold/docker/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func newAPIClient(kubeContext string) (APIClient, error) {
func newEnvAPIClient() (APIClient, error) {
cli, err := client.NewClientWithOpts(client.FromEnv)
if err != nil {
return nil, fmt.Errorf("Error getting docker client: %s", err)
return nil, fmt.Errorf("error getting docker client: %s", err)
}
cli.NegotiateAPIVersion(context.Background())

Expand Down Expand Up @@ -144,10 +144,10 @@ func getMiniKubeFilename() (string, error) {
if found, _ := detectWsl(); found {
filename, err := exec.LookPath("minikube.exe")
if err != nil {
return "", fmt.Errorf("Unable to find minikube.exe. Please add it to PATH environment variable")
return "", errors.New("unable to find minikube.exe. Please add it to PATH environment variable")
}
if _, err := os.Stat(filename); os.IsNotExist(err) {
return "", fmt.Errorf("Unable to find minikube.exe. File not found %s", filename)
return "", fmt.Errorf("unable to find minikube.exe. File not found %s", filename)
}
return filename, nil
}
Expand All @@ -172,7 +172,7 @@ func getMinikubeDockerEnv() (map[string]string, error) {
}
kv := strings.Split(line, "=")
if len(kv) != 2 {
return nil, fmt.Errorf("Unable to parse minikube docker-env keyvalue: %s, line: %s, output: %s", kv, line, string(out))
return nil, fmt.Errorf("unable to parse minikube docker-env keyvalue: %s, line: %s, output: %s", kv, line, string(out))
}
env[kv[0]] = kv[1]
}
Expand All @@ -183,7 +183,7 @@ func getMinikubeDockerEnv() (map[string]string, error) {
if err == nil {
env["DOCKER_CERT_PATH"] = strings.TrimRight(string(out), "\n")
} else {
return nil, fmt.Errorf("Can't run wslpath: %s", err)
return nil, fmt.Errorf("can't run wslpath: %s", err)
}
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/docker/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ func (f *fakeImageFetcher) fetch(image string) (*v1.ConfigFile, error) {
}, nil
}

return nil, fmt.Errorf("No image found for %s", image)
return nil, fmt.Errorf("no image found for %s", image)
}

func TestGetDependencies(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/kubernetes/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func getClientConfig() (*restclient.Config, error) {
kubeConfig := clientcmd.NewNonInteractiveDeferredLoadingClientConfig(loadingRules, &clientcmd.ConfigOverrides{})
clientConfig, err := kubeConfig.ClientConfig()
if err != nil {
return nil, fmt.Errorf("Error creating kubeConfig: %s", err)
return nil, fmt.Errorf("error creating kubeConfig: %s", err)
}
return clientConfig, nil
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/kubernetes/port_forward.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func (*kubectlForwarder) Forward(pfe *portForwardEntry) error {
func (*kubectlForwarder) Stop(p *portForwardEntry) error {
logrus.Debugf("Terminating port-forward %s", p)
if p.cmd == nil {
return fmt.Errorf("No port-forward command found for %s", p)
return fmt.Errorf("no port-forward command found for %s", p)
}
if err := p.cmd.Process.Signal(syscall.SIGTERM); err != nil {
return errors.Wrap(err, "terminating port-forward process")
Expand Down
12 changes: 6 additions & 6 deletions pkg/skaffold/runner/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,22 +81,22 @@ func NewForConfig(opts *config.SkaffoldOptions, cfg *latest.SkaffoldPipeline) (*

tagger, err := getTagger(cfg.Build.TagPolicy, opts.CustomTag)
if err != nil {
return nil, errors.Wrap(err, "parsing skaffold tag config")
return nil, errors.Wrap(err, "parsing tag config")
}

builder, err := getBuilder(&cfg.Build, kubeContext)
if err != nil {
return nil, errors.Wrap(err, "parsing skaffold build config")
return nil, errors.Wrap(err, "parsing build config")
}

tester, err := getTester(&cfg.Test)
if err != nil {
return nil, errors.Wrap(err, "parsing skaffold test config")
return nil, errors.Wrap(err, "parsing test config")
}

deployer, err := getDeployer(&cfg.Deploy, kubeContext, opts.Namespace, defaultRepo)
if err != nil {
return nil, errors.Wrap(err, "parsing skaffold deploy config")
return nil, errors.Wrap(err, "parsing deploy config")
}

deployer = deploy.WithLabels(deployer, opts, builder, deployer, tagger)
Expand Down Expand Up @@ -141,7 +141,7 @@ func getBuilder(cfg *latest.BuildConfig, kubeContext string) (build.Builder, err
return acr.NewBuilder(cfg.AzureContainerBuild), nil

default:
return nil, fmt.Errorf("Unknown builder for config %+v", cfg)
return nil, fmt.Errorf("unknown builder for config %+v", cfg)
}
}

Expand Down Expand Up @@ -201,7 +201,7 @@ func getTagger(t latest.TagPolicy, customTag string) (tag.Tagger, error) {
return tag.NewDateTimeTagger(t.DateTimeTagger.Format, t.DateTimeTagger.TimeZone), nil

default:
return nil, fmt.Errorf("Unknown tagger for strategy %+v", t)
return nil, fmt.Errorf("unknown tagger for strategy %+v", t)
}
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func ExpandPathsGlob(workingDir string, paths []string) ([]string, error) {
return nil, errors.Wrap(err, "glob")
}
if files == nil {
return nil, fmt.Errorf("File pattern must match at least one file %s", path)
return nil, fmt.Errorf("file pattern must match at least one file %s", path)
}

for _, f := range files {
Expand Down
8 changes: 4 additions & 4 deletions testutil/cmd_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,11 @@ func NewFakeCmdOut(expectedCommand, stdout string, err error) *FakeCmd {
func (f *FakeCmd) RunCmdOut(cmd *exec.Cmd) ([]byte, error) {
actualCommand := strings.Join(cmd.Args, " ")
if f.expectedCommand != actualCommand {
return nil, fmt.Errorf("Expected: %s. Got: %s", f.expectedCommand, actualCommand)
return nil, fmt.Errorf("expected: %s. Got: %s", f.expectedCommand, actualCommand)
}

if f.stdout == nil {
return nil, fmt.Errorf("Expected RunCmd(%s) to be called. Got RunCmdOut(%s)", f.expectedCommand, actualCommand)
return nil, fmt.Errorf("expected RunCmd(%s) to be called. Got RunCmdOut(%s)", f.expectedCommand, actualCommand)
}

return f.stdout, f.err
Expand All @@ -59,11 +59,11 @@ func (f *FakeCmd) RunCmdOut(cmd *exec.Cmd) ([]byte, error) {
func (f *FakeCmd) RunCmd(cmd *exec.Cmd) error {
actualCommand := strings.Join(cmd.Args, " ")
if f.expectedCommand != actualCommand {
return fmt.Errorf("Expected: %s. Got: %s", f.expectedCommand, actualCommand)
return fmt.Errorf("expected: %s. Got: %s", f.expectedCommand, actualCommand)
}

if f.stdout != nil {
return fmt.Errorf("Expected RunCmdOut(%s) to be called. Got RunCmd(%s)", f.expectedCommand, actualCommand)
return fmt.Errorf("expected RunCmdOut(%s) to be called. Got RunCmd(%s)", f.expectedCommand, actualCommand)
}

return f.err
Expand Down
9 changes: 5 additions & 4 deletions testutil/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package testutil

import (
"errors"
"fmt"
"net/http"
"net/http/httptest"
Expand All @@ -29,11 +30,11 @@ import (

type BadReader struct{}

func (BadReader) Read([]byte) (int, error) { return 0, fmt.Errorf("Bad read") }
func (BadReader) Read([]byte) (int, error) { return 0, errors.New("bad read") }

type BadWriter struct{}

func (BadWriter) Write([]byte) (int, error) { return 0, fmt.Errorf("Bad write") }
func (BadWriter) Write([]byte) (int, error) { return 0, errors.New("bad write") }

type FakeReaderCloser struct {
Err error
Expand Down Expand Up @@ -86,10 +87,10 @@ func CheckError(t *testing.T, shouldErr bool, err error) {

func checkErr(shouldErr bool, err error) error {
if err == nil && shouldErr {
return fmt.Errorf("Expected error, but returned none")
return errors.New("expected error, but returned none")
}
if err != nil && !shouldErr {
return fmt.Errorf("Unexpected error: %s", err)
return fmt.Errorf("unexpected error: %s", err)
}
return nil
}
Expand Down

0 comments on commit a70f7b7

Please sign in to comment.