Skip to content

*: use fmt.Errorf with %w/%v for Go 1.13 error wrapping #2355

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

Merged
merged 9 commits into from
Jan 16, 2020
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
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
- Added [`run`](./doc/cli/operator-sdk_alpha_run.md) and [`cleanup`](./doc/cli/operator-sdk_alpha_cleanup.md) subcommands (under the `alpha` subcommand) to manage deployment/deletion of operators. These commands currently interact with OLM via an in-cluster registry-server created using an operator's on-disk manifests and managed by `operator-sdk`. ([#2402](ttps://github.com/operator-framework/operator-sdk/pull/2402))

### Changed
- Changed error wrapping according to Go version 1.13+ [error handling](https://blog.golang.org/go1.13-errors). ([#2355](https://github.com/operator-framework/operator-sdk/pull/2355))

### Deprecated

Expand All @@ -24,7 +25,6 @@
- Added support for override values with environment variable expansion in the `watches.yaml` file for Helm-based operators. ([#2325](https://github.com/operator-framework/operator-sdk/pull/2325))

### Changed

- Replace usage of `github.com/operator-framework/operator-sdk/pkg/restmapper.DynamicRESTMapper` with `sigs.k8s.io/controller-runtime/pkg/client/apiutil.DynamicRESTMapper`. ([#2309](https://github.com/operator-framework/operator-sdk/pull/2309))
- Upgraded Helm operator packages and base image from Helm v2 to Helm v3. Cluster state for pre-existing CRs using Helm v2-based operators will be automatically migrated to Helm v3's new release storage format, and existing releases may be upgraded due to changes in Helm v3's label injection. ([#2080](https://github.com/operator-framework/operator-sdk/pull/2080))
- Fail `operator-sdk olm-catalog gen-csv` if it is not run from a project's root, which the command already assumes is the case. ([#2322](https://github.com/operator-framework/operator-sdk/pull/2322))
Expand Down
11 changes: 5 additions & 6 deletions cmd/operator-sdk/add/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"github.com/operator-framework/operator-sdk/internal/scaffold/input"
"github.com/operator-framework/operator-sdk/internal/util/projutil"

"github.com/pkg/errors"
log "github.com/sirupsen/logrus"
"github.com/spf13/cobra"
)
Expand Down Expand Up @@ -115,7 +114,7 @@ func apiRun(cmd *cobra.Command, args []string) error {
// scaffold a group.go to prevent erroneous gengo parse errors.
group := &scaffold.Group{Resource: r}
if err := scaffoldIfNoPkgFileExists(s, cfg, group); err != nil {
return errors.Wrap(err, "scaffold group file")
return fmt.Errorf("scaffold group file: %v", err)
}

err = s.Execute(cfg,
Expand All @@ -126,12 +125,12 @@ func apiRun(cmd *cobra.Command, args []string) error {
&scaffold.CR{Resource: r},
)
if err != nil {
return fmt.Errorf("api scaffold failed: (%v)", err)
return fmt.Errorf("api scaffold failed: %v", err)
}

// update deploy/role.yaml for the given resource r.
if err := scaffold.UpdateRoleForResource(r, absProjectPath); err != nil {
return fmt.Errorf("failed to update the RBAC manifest for the resource (%v, %v): (%v)", r.APIVersion, r.Kind, err)
return fmt.Errorf("failed to update the RBAC manifest for the resource (%v, %v): %v", r.APIVersion, r.Kind, err)
}

if !skipGeneration {
Expand All @@ -155,12 +154,12 @@ func apiRun(cmd *cobra.Command, args []string) error {
func scaffoldIfNoPkgFileExists(s *scaffold.Scaffold, cfg *input.Config, f input.File) error {
i, err := f.GetInput()
if err != nil {
return errors.Wrapf(err, "error getting file %s input", i.Path)
return fmt.Errorf("error getting file %s input: %v", i.Path, err)
}
groupDir := filepath.Dir(i.Path)
gdInfos, err := ioutil.ReadDir(groupDir)
if err != nil && !os.IsNotExist(err) {
return errors.Wrapf(err, "error reading dir %s", groupDir)
return fmt.Errorf("error reading dir %s: %v", groupDir, err)
}
if err == nil {
for _, info := range gdInfos {
Expand Down
2 changes: 1 addition & 1 deletion cmd/operator-sdk/add/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func controllerRun(cmd *cobra.Command, args []string) error {
&scaffold.AddController{Resource: r},
)
if err != nil {
return fmt.Errorf("controller scaffold failed: (%v)", err)
return fmt.Errorf("controller scaffold failed: %v", err)
}

log.Info("Controller generation complete.")
Expand Down
6 changes: 3 additions & 3 deletions cmd/operator-sdk/add/crd.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func crdFunc(cmd *cobra.Command, args []string) error {
},
)
if err != nil {
return fmt.Errorf("crd scaffold failed: (%v)", err)
return fmt.Errorf("crd scaffold failed: %v", err)
}

// This command does not consider an APIs dir. Instead it adds a plain CRD
Expand All @@ -105,7 +105,7 @@ func crdFunc(cmd *cobra.Command, args []string) error {

// update deploy/role.yaml for the given resource r.
if err := scaffold.UpdateRoleForResource(resource, cfg.AbsProjectPath); err != nil {
return fmt.Errorf("failed to update the RBAC manifest for the resource (%v, %v): (%v)", resource.APIVersion, resource.Kind, err)
return fmt.Errorf("failed to update the RBAC manifest for the resource (%v, %v): %v", resource.APIVersion, resource.Kind, err)
}

log.Info("CRD generation complete.")
Expand Down Expand Up @@ -133,7 +133,7 @@ func verifyCRDFlags() error {
func verifyCRDDeployPath() error {
wd, err := os.Getwd()
if err != nil {
return fmt.Errorf("failed to determine the full path of the current directory: (%v)", err)
return fmt.Errorf("failed to determine the full path of the current directory: %v", err)
}
// check if the deploy sub-directory exist
_, err = os.Stat(filepath.Join(wd, scaffold.DeployDir))
Expand Down
6 changes: 3 additions & 3 deletions cmd/operator-sdk/build/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func createBuildCommand(imageBuilder, context, dockerFile, image string, imageBu
if bargs != "" {
splitArgs, err := shlex.Split(bargs)
if err != nil {
return nil, fmt.Errorf("image-build-args is not parseable: %w", err)
return nil, fmt.Errorf("image-build-args is not parseable: %v", err)
}
args = append(args, splitArgs...)
}
Expand Down Expand Up @@ -120,7 +120,7 @@ func buildFunc(cmd *cobra.Command, args []string) error {
Env: goBuildEnv,
}
if err := projutil.GoBuild(opts); err != nil {
return fmt.Errorf("failed to build operator binary: (%v)", err)
return fmt.Errorf("failed to build operator binary: %v", err)
}
}

Expand All @@ -134,7 +134,7 @@ func buildFunc(cmd *cobra.Command, args []string) error {
}

if err := projutil.ExecCmd(buildCmd); err != nil {
return fmt.Errorf("failed to output build image %s: (%v)", image, err)
return fmt.Errorf("failed to output build image %s: %v", image, err)
}

log.Info("Operator build complete.")
Expand Down
7 changes: 3 additions & 4 deletions cmd/operator-sdk/internal/genutil/k8s.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"github.com/operator-framework/operator-sdk/internal/util/k8sutil"
"github.com/operator-framework/operator-sdk/internal/util/projutil"

"github.com/pkg/errors"
log "github.com/sirupsen/logrus"
generatorargs "k8s.io/code-generator/cmd/deepcopy-gen/args"
"k8s.io/gengo/examples/deepcopy-gen/generators"
Expand All @@ -40,7 +39,7 @@ func K8sCodegen() error {

gvMap, err := k8sutil.ParseGroupSubpackages(scaffold.ApisDir)
if err != nil {
return fmt.Errorf("failed to parse group versions: (%v)", err)
return fmt.Errorf("failed to parse group versions: %v", err)
}
gvb := &strings.Builder{}
for g, vs := range gvMap {
Expand Down Expand Up @@ -91,7 +90,7 @@ func deepcopyGen(hf string, fqApis []string) error {
}

if err := generatorargs.Validate(args); err != nil {
return errors.Wrap(err, "deepcopy-gen argument validation error")
return fmt.Errorf("deepcopy-gen argument validation error: %v", err)
}

err = args.Execute(
Expand All @@ -100,7 +99,7 @@ func deepcopyGen(hf string, fqApis []string) error {
generators.Packages,
)
if err != nil {
return errors.Wrap(err, "deepcopy-gen generator error")
return fmt.Errorf("deepcopy-gen generator error: %v", err)
}
}
return nil
Expand Down
7 changes: 3 additions & 4 deletions cmd/operator-sdk/internal/genutil/openapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"github.com/operator-framework/operator-sdk/internal/util/k8sutil"
"github.com/operator-framework/operator-sdk/internal/util/projutil"

"github.com/pkg/errors"
log "github.com/sirupsen/logrus"
generatorargs "k8s.io/kube-openapi/cmd/openapi-gen/args"
"k8s.io/kube-openapi/pkg/generators"
Expand All @@ -39,7 +38,7 @@ func OpenAPIGen() error {

gvMap, err := k8sutil.ParseGroupSubpackages(scaffold.ApisDir)
if err != nil {
return fmt.Errorf("failed to parse group versions: (%v)", err)
return fmt.Errorf("failed to parse group versions: %v", err)
}
gvb := &strings.Builder{}
for g, vs := range gvMap {
Expand Down Expand Up @@ -83,7 +82,7 @@ func openAPIGen(hf string, fqApis []string) error {
// Print API rule violations to stdout
cargs.ReportFilename = "-"
if err := generatorargs.Validate(args); err != nil {
return errors.Wrap(err, "openapi-gen argument validation error")
return fmt.Errorf("openapi-gen argument validation error: %v", err)
}

err := args.Execute(
Expand All @@ -92,7 +91,7 @@ func openAPIGen(hf string, fqApis []string) error {
generators.Packages,
)
if err != nil {
return errors.Wrap(err, "openapi-gen generator error")
return fmt.Errorf("openapi-gen generator error: %v", err)
}
}
return nil
Expand Down
12 changes: 6 additions & 6 deletions cmd/operator-sdk/migrate/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ func migrateAnsible() error {
case os.IsNotExist(err):
log.Info("No playbook was found, so not including it in the new Dockerfile")
default:
return fmt.Errorf("error trying to stat %s: (%v)", ansible.PlaybookYamlFile, err)
return fmt.Errorf("error trying to stat %s: %v", ansible.PlaybookYamlFile, err)
}
if err := renameDockerfile(); err != nil {
return err
Expand All @@ -111,7 +111,7 @@ func migrateAnsible() error {
if headerFile != "" {
err = s.Execute(cfg, &scaffold.Boilerplate{BoilerplateSrcPath: headerFile})
if err != nil {
return fmt.Errorf("boilerplate scaffold failed: (%v)", err)
return fmt.Errorf("boilerplate scaffold failed: %v", err)
}
s.BoilerplatePath = headerFile
}
Expand All @@ -126,7 +126,7 @@ func migrateAnsible() error {
&ansible.AoLogs{},
)
if err != nil {
return fmt.Errorf("migrate ansible scaffold failed: (%v)", err)
return fmt.Errorf("migrate ansible scaffold failed: %v", err)
}
return nil
}
Expand All @@ -149,7 +149,7 @@ func migrateHelm() error {
if headerFile != "" {
err := s.Execute(cfg, &scaffold.Boilerplate{BoilerplateSrcPath: headerFile})
if err != nil {
return fmt.Errorf("boilerplate scaffold failed: (%v)", err)
return fmt.Errorf("boilerplate scaffold failed: %v", err)
}
s.BoilerplatePath = headerFile
}
Expand All @@ -166,7 +166,7 @@ func migrateHelm() error {
&helm.UserSetup{},
)
if err != nil {
return fmt.Errorf("migrate helm scaffold failed: (%v)", err)
return fmt.Errorf("migrate helm scaffold failed: %v", err)
}
return nil
}
Expand All @@ -176,7 +176,7 @@ func renameDockerfile() error {
newDockerfilePath := dockerfilePath + ".sdkold"
err := os.Rename(dockerfilePath, newDockerfilePath)
if err != nil {
return fmt.Errorf("failed to rename Dockerfile: (%v)", err)
return fmt.Errorf("failed to rename Dockerfile: %v", err)
}
log.Infof("Renamed Dockerfile to %s and replaced with newer version. Compare the new Dockerfile to your old one and manually migrate any customizations", newDockerfilePath)
return nil
Expand Down
29 changes: 14 additions & 15 deletions cmd/operator-sdk/new/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import (
"github.com/operator-framework/operator-sdk/internal/util/projutil"

"github.com/ghodss/yaml"
"github.com/pkg/errors"
log "github.com/sirupsen/logrus"
"github.com/spf13/cobra"
"k8s.io/client-go/discovery"
Expand Down Expand Up @@ -183,7 +182,7 @@ func doGoScaffold() error {
if headerFile != "" {
err := s.Execute(cfg, &scaffold.Boilerplate{BoilerplateSrcPath: headerFile})
if err != nil {
return fmt.Errorf("boilerplate scaffold failed: (%v)", err)
return fmt.Errorf("boilerplate scaffold failed: %v", err)
}
s.BoilerplatePath = headerFile
}
Expand All @@ -209,7 +208,7 @@ func doGoScaffold() error {
&scaffold.Gitignore{},
)
if err != nil {
return fmt.Errorf("new Go scaffold failed: (%v)", err)
return fmt.Errorf("new Go scaffold failed: %v", err)
}
return nil
}
Expand All @@ -222,7 +221,7 @@ func doAnsibleScaffold() error {

resource, err := scaffold.NewResource(apiVersion, kind)
if err != nil {
return fmt.Errorf("invalid apiVersion and kind: (%v)", err)
return fmt.Errorf("invalid apiVersion and kind: %v", err)
}

roleFiles := ansible.RolesFiles{Resource: *resource}
Expand Down Expand Up @@ -265,7 +264,7 @@ func doAnsibleScaffold() error {
&ansible.MoleculeTestLocalPrepare{Resource: *resource},
)
if err != nil {
return fmt.Errorf("new ansible scaffold failed: (%v)", err)
return fmt.Errorf("new ansible scaffold failed: %v", err)
}

if err = generateCRDNonGo(projectName, *resource); err != nil {
Expand All @@ -275,11 +274,11 @@ func doAnsibleScaffold() error {
// Remove placeholders from empty directories
err = os.Remove(filepath.Join(s.AbsProjectPath, roleFiles.Path))
if err != nil {
return fmt.Errorf("new ansible scaffold failed: (%v)", err)
return fmt.Errorf("new ansible scaffold failed: %v", err)
}
err = os.Remove(filepath.Join(s.AbsProjectPath, roleTemplates.Path))
if err != nil {
return fmt.Errorf("new ansible scaffold failed: (%v)", err)
return fmt.Errorf("new ansible scaffold failed: %v", err)
}

// Decide on playbook.
Expand All @@ -290,13 +289,13 @@ func doAnsibleScaffold() error {
&ansible.Playbook{Resource: *resource},
)
if err != nil {
return fmt.Errorf("new ansible playbook scaffold failed: (%v)", err)
return fmt.Errorf("new ansible playbook scaffold failed: %v", err)
}
}

// update deploy/role.yaml for the given resource r.
if err := scaffold.UpdateRoleForResource(resource, cfg.AbsProjectPath); err != nil {
return fmt.Errorf("failed to update the RBAC manifest for the resource (%v, %v): (%v)", resource.APIVersion, resource.Kind, err)
return fmt.Errorf("failed to update the RBAC manifest for the resource (%v, %v): %v", resource.APIVersion, resource.Kind, err)
}
return nil
}
Expand All @@ -317,14 +316,14 @@ func doHelmScaffold() error {

resource, chart, err := helm.CreateChart(cfg.AbsProjectPath, createOpts)
if err != nil {
return fmt.Errorf("failed to create helm chart: %w", err)
return fmt.Errorf("failed to create helm chart: %v", err)
}

valuesPath := filepath.Join("<project_dir>", helm.HelmChartsDir, chart.Name(), "values.yaml")

rawValues, err := yaml.Marshal(chart.Values)
if err != nil {
return fmt.Errorf("failed to get raw chart values: %w", err)
return fmt.Errorf("failed to get raw chart values: %v", err)
}
crSpec := fmt.Sprintf("# Default values copied from %s\n\n%s", valuesPath, rawValues)

Expand Down Expand Up @@ -354,15 +353,15 @@ func doHelmScaffold() error {
},
)
if err != nil {
return fmt.Errorf("new helm scaffold failed: %w", err)
return fmt.Errorf("new helm scaffold failed: %v", err)
}

if err = generateCRDNonGo(projectName, *resource); err != nil {
return err
}

if err := scaffold.UpdateRoleForResource(resource, cfg.AbsProjectPath); err != nil {
return fmt.Errorf("failed to update the RBAC manifest for resource (%v, %v): %w", resource.APIVersion, resource.Kind, err)
return fmt.Errorf("failed to update the RBAC manifest for resource (%v, %v): %v", resource.APIVersion, resource.Kind, err)
}
return nil
}
Expand All @@ -383,7 +382,7 @@ func generateCRDNonGo(projectName string, resource scaffold.Resource) error {

func verifyFlags() error {
if operatorType != projutil.OperatorTypeGo && operatorType != projutil.OperatorTypeAnsible && operatorType != projutil.OperatorTypeHelm {
return errors.Wrap(projutil.ErrUnknownOperatorType{Type: operatorType}, "value of --type can only be `go`, `ansible`, or `helm`")
return fmt.Errorf("value of --type can only be `go`, `ansible`, or `helm`: %v", projutil.ErrUnknownOperatorType{Type: operatorType})
}
if operatorType != projutil.OperatorTypeAnsible && generatePlaybook {
return fmt.Errorf("value of --generate-playbook can only be used with --type `ansible`")
Expand Down Expand Up @@ -461,7 +460,7 @@ func getDeps() error {
func initGit() error {
log.Info("Running git init")
if err := execProjCmd("git", "init"); err != nil {
return errors.Wrapf(err, "failed to run git init")
return fmt.Errorf("failed to run git init: %v", err)
}
log.Info("Run git init done")
return nil
Expand Down
4 changes: 2 additions & 2 deletions cmd/operator-sdk/olmcatalog/gen-csv.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func genCSVFunc(cmd *cobra.Command, args []string) error {
}
err = s.Execute(cfg, csv)
if err != nil {
return fmt.Errorf("catalog scaffold failed: (%v)", err)
return fmt.Errorf("catalog scaffold failed: %v", err)
}

gcfg := gen.Config{
Expand Down Expand Up @@ -168,7 +168,7 @@ func verifyGenCSVFlags() error {
func verifyCSVVersion(version string) error {
v, err := semver.NewVersion(version)
if err != nil {
return fmt.Errorf("%s is not a valid semantic version: (%v)", version, err)
return fmt.Errorf("%s is not a valid semantic version: %v", version, err)
}
// Ensures numerical values composing csvVersion don't contain leading 0's,
// ex. 01.01.01
Expand Down
Loading