Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve errors #1255

Merged
merged 2 commits into from
Nov 8, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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