From 036e730c093a3ddefc150f353bc22900ad42b8d9 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Thu, 25 Oct 2018 09:20:14 +0200 Subject: [PATCH 1/2] Fix error messages Should start with a lowercase Signed-off-by: David Gageot --- pkg/skaffold/build/tag/custom.go | 7 +++++-- pkg/skaffold/build/tag/sha256.go | 5 +++-- pkg/skaffold/deploy/labels.go | 2 +- pkg/skaffold/docker/client.go | 10 +++++----- pkg/skaffold/docker/parse_test.go | 2 +- pkg/skaffold/kubernetes/client.go | 2 +- pkg/skaffold/kubernetes/port_forward.go | 2 +- pkg/skaffold/runner/runner.go | 4 ++-- pkg/skaffold/util/util.go | 2 +- testutil/cmd_helper.go | 8 ++++---- testutil/util.go | 9 +++++---- 11 files changed, 29 insertions(+), 24 deletions(-) diff --git a/pkg/skaffold/build/tag/custom.go b/pkg/skaffold/build/tag/custom.go index 28c96a6c96a..5fa56ce3bd9 100644 --- a/pkg/skaffold/build/tag/custom.go +++ b/pkg/skaffold/build/tag/custom.go @@ -17,6 +17,7 @@ limitations under the License. package tag import ( + "errors" "fmt" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants" @@ -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 } diff --git a/pkg/skaffold/build/tag/sha256.go b/pkg/skaffold/build/tag/sha256.go index dc6e029b035..4587ef8fb8f 100644 --- a/pkg/skaffold/build/tag/sha256.go +++ b/pkg/skaffold/build/tag/sha256.go @@ -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 @@ -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:", digest) + return "", fmt.Errorf("digest wrong format: %s, expected sha256:", digest) } return fmt.Sprintf("%s:%s", opts.ImageName, sha256), nil diff --git a/pkg/skaffold/deploy/labels.go b/pkg/skaffold/deploy/labels.go index 52de5bff64e..78e22933b4d 100644 --- a/pkg/skaffold/deploy/labels.go +++ b/pkg/skaffold/deploy/labels.go @@ -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) { diff --git a/pkg/skaffold/docker/client.go b/pkg/skaffold/docker/client.go index a6df23d22b3..d810e09446a 100644 --- a/pkg/skaffold/docker/client.go +++ b/pkg/skaffold/docker/client.go @@ -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()) @@ -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 } @@ -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] } @@ -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) } } diff --git a/pkg/skaffold/docker/parse_test.go b/pkg/skaffold/docker/parse_test.go index 16723b03a55..b31f25efa27 100644 --- a/pkg/skaffold/docker/parse_test.go +++ b/pkg/skaffold/docker/parse_test.go @@ -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) { diff --git a/pkg/skaffold/kubernetes/client.go b/pkg/skaffold/kubernetes/client.go index 088b9025227..9b3727d9a84 100644 --- a/pkg/skaffold/kubernetes/client.go +++ b/pkg/skaffold/kubernetes/client.go @@ -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 } diff --git a/pkg/skaffold/kubernetes/port_forward.go b/pkg/skaffold/kubernetes/port_forward.go index ace63a48330..6d9239530dd 100644 --- a/pkg/skaffold/kubernetes/port_forward.go +++ b/pkg/skaffold/kubernetes/port_forward.go @@ -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") diff --git a/pkg/skaffold/runner/runner.go b/pkg/skaffold/runner/runner.go index 53c54837ae4..bb2e6d9193b 100644 --- a/pkg/skaffold/runner/runner.go +++ b/pkg/skaffold/runner/runner.go @@ -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) } } @@ -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) } } diff --git a/pkg/skaffold/util/util.go b/pkg/skaffold/util/util.go index c9dea209b7a..bb9a13e3df9 100644 --- a/pkg/skaffold/util/util.go +++ b/pkg/skaffold/util/util.go @@ -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 { diff --git a/testutil/cmd_helper.go b/testutil/cmd_helper.go index 1ccc250927b..d8c5674c045 100644 --- a/testutil/cmd_helper.go +++ b/testutil/cmd_helper.go @@ -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 @@ -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 diff --git a/testutil/util.go b/testutil/util.go index cc5a044bd64..24764fdeed9 100644 --- a/testutil/util.go +++ b/testutil/util.go @@ -17,6 +17,7 @@ limitations under the License. package testutil import ( + "errors" "fmt" "net/http" "net/http/httptest" @@ -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 @@ -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 } From 5f9be2d14c95fefb8bf4c9661119f5c1e3824ec4 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Thu, 25 Oct 2018 09:27:00 +0200 Subject: [PATCH 2/2] Shorter error messages Signed-off-by: David Gageot --- pkg/skaffold/runner/runner.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/skaffold/runner/runner.go b/pkg/skaffold/runner/runner.go index bb2e6d9193b..da05aac9f5f 100644 --- a/pkg/skaffold/runner/runner.go +++ b/pkg/skaffold/runner/runner.go @@ -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)