From 3aa6b4d387e0134b33dba1bc83f47394b96cdcbd Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Wed, 18 Mar 2020 21:58:31 +0100 Subject: [PATCH] helm: reduce amount of error wrapping --- pkg/helm/v2/get.go | 4 +--- pkg/helm/v2/helm.go | 2 +- pkg/helm/v2/history.go | 4 +--- pkg/helm/v2/repository.go | 10 +++++----- pkg/helm/v2/upgrade.go | 8 ++++---- pkg/helm/v3/get.go | 3 +-- pkg/helm/v3/history.go | 4 +--- pkg/helm/v3/repository.go | 2 -- pkg/helm/v3/rollback.go | 3 +-- pkg/helm/v3/uninstall.go | 11 ++++------- pkg/helm/v3/upgrade.go | 8 +++----- pkg/release/release.go | 3 --- 12 files changed, 22 insertions(+), 40 deletions(-) diff --git a/pkg/helm/v2/get.go b/pkg/helm/v2/get.go index 4dcd9f518..657beb08c 100644 --- a/pkg/helm/v2/get.go +++ b/pkg/helm/v2/get.go @@ -3,8 +3,6 @@ package v2 import ( "strings" - "github.com/pkg/errors" - helmv2 "k8s.io/helm/pkg/helm" "github.com/fluxcd/helm-operator/pkg/helm" @@ -17,7 +15,7 @@ func (h *HelmV2) Get(releaseName string, opts helm.GetOptions) (*helm.Release, e if strings.Contains(err.Error(), "not found") { return nil, nil } - return nil, errors.Wrapf(err, "failed to retrieve release [%s]", releaseName) + return nil, err } return releaseToGenericRelease(res.Release), nil } diff --git a/pkg/helm/v2/helm.go b/pkg/helm/v2/helm.go index ee044e680..b32a0a260 100644 --- a/pkg/helm/v2/helm.go +++ b/pkg/helm/v2/helm.go @@ -57,7 +57,7 @@ func New(logger log.Logger, kubeClient *kubernetes.Clientset, opts TillerOptions for { client, host, err := newHelmClient(kubeClient, opts) if err != nil { - logger.Log("error", fmt.Sprintf("error creating Client (v2) client: %s", err.Error())) + logger.Log("error", fmt.Errorf("errored creating Helm 2 client: %w", err)) time.Sleep(20 * time.Second) continue } diff --git a/pkg/helm/v2/history.go b/pkg/helm/v2/history.go index 399a2930e..8a0c4ce11 100644 --- a/pkg/helm/v2/history.go +++ b/pkg/helm/v2/history.go @@ -1,8 +1,6 @@ package v2 import ( - "github.com/pkg/errors" - helmv2 "k8s.io/helm/pkg/helm" "github.com/fluxcd/helm-operator/pkg/helm" @@ -15,7 +13,7 @@ func (h *HelmV2) History(releaseName string, opts helm.HistoryOptions) ([]*helm. } res, err := h.client.ReleaseHistory(releaseName, max) if err != nil { - return nil, errors.Wrapf(statusMessageErr(err), "failed to retrieve history for [%s]", releaseName) + return nil, err } var rels []*helm.Release for _, r := range res.Releases { diff --git a/pkg/helm/v2/repository.go b/pkg/helm/v2/repository.go index c02d89157..a49dca117 100644 --- a/pkg/helm/v2/repository.go +++ b/pkg/helm/v2/repository.go @@ -1,17 +1,18 @@ package v2 import ( + "fmt" "os" "sync" "github.com/pkg/errors" + "k8s.io/helm/pkg/repo" ) var repositoryConfigLock sync.RWMutex func (h *HelmV2) RepositoryIndex() error { - repositoryConfigLock.RLock() f, err := loadRepositoryConfig() repositoryConfigLock.RUnlock() @@ -45,6 +46,9 @@ func (h *HelmV2) RepositoryAdd(name, url, username, password, certFile, keyFile, if err != nil { return err } + if f.Has(name) { + return fmt.Errorf("chart repository with name '%s' already exists", name) + } c := &repo.Entry{ Name: name, @@ -57,10 +61,6 @@ func (h *HelmV2) RepositoryAdd(name, url, username, password, certFile, keyFile, } f.Add(c) - if f.Has(name) { - return errors.New("chart repository with name %s already exists") - } - r, err := repo.NewChartRepository(c, getterProviders()) if err != nil { return err diff --git a/pkg/helm/v2/upgrade.go b/pkg/helm/v2/upgrade.go index 4766cef4e..bf11c65a0 100644 --- a/pkg/helm/v2/upgrade.go +++ b/pkg/helm/v2/upgrade.go @@ -1,7 +1,7 @@ package v2 import ( - "github.com/pkg/errors" + "fmt" "k8s.io/helm/pkg/chartutil" helmv2 "k8s.io/helm/pkg/helm" @@ -19,7 +19,7 @@ func (h *HelmV2) UpgradeFromPath(chartPath string, releaseName string, values [] // Load the chart from the given path chartRequested, err := chartutil.Load(chartPath) if err != nil { - return nil, errors.Wrapf(err, "failed to load chart from path [%s] for release [%s]", chartPath, releaseName) + return nil, err } var res releaseResponse @@ -39,7 +39,7 @@ func (h *HelmV2) UpgradeFromPath(chartPath string, releaseName string, values [] _, dErr := h.client.DeleteRelease(releaseName, helmv2.DeletePurge(true), helmv2.DeleteDisableHooks(opts.DisableHooks)) if dErr != nil { - return nil, errors.Wrapf(statusMessageErr(dErr), "failed to uninstall release, original installation error: %s", statusMessageErr(err)) + return nil, fmt.Errorf("%s, original installation error: %w", statusMessageErr(dErr), statusMessageErr(err)) } } } else { @@ -65,7 +65,7 @@ func (h *HelmV2) UpgradeFromPath(chartPath string, releaseName string, values [] helmv2.RollbackDryRun(opts.DryRun), helmv2.RollbackRecreate(opts.Recreate), helmv2.RollbackForce(opts.Force)) - return nil, errors.Wrapf(statusMessageErr(rErr), "failed to roll back release, original installation error: %s", statusMessageErr(err)) + return nil, fmt.Errorf("%s, original installation error: %w", statusMessageErr(rErr), statusMessageErr(err)) } } if err != nil { diff --git a/pkg/helm/v3/get.go b/pkg/helm/v3/get.go index 8068e7c76..3bdb7238e 100644 --- a/pkg/helm/v3/get.go +++ b/pkg/helm/v3/get.go @@ -1,7 +1,6 @@ package v3 import ( - "github.com/pkg/errors" "helm.sh/helm/v3/pkg/action" "helm.sh/helm/v3/pkg/storage/driver" @@ -24,7 +23,7 @@ func (h *HelmV3) Get(releaseName string, opts helm.GetOptions) (*helm.Release, e case driver.ErrReleaseNotFound: return nil, nil default: - return nil, errors.Wrapf(err, "failed to retrieve release '%s'", releaseName) + return nil, err } } diff --git a/pkg/helm/v3/history.go b/pkg/helm/v3/history.go index 5e9b36e3c..e8f386f9e 100644 --- a/pkg/helm/v3/history.go +++ b/pkg/helm/v3/history.go @@ -1,8 +1,6 @@ package v3 import ( - "github.com/pkg/errors" - "helm.sh/helm/v3/pkg/action" "helm.sh/helm/v3/pkg/releaseutil" @@ -20,7 +18,7 @@ func (h *HelmV3) History(releaseName string, opts helm.HistoryOptions) ([]*helm. hist, err := history.Run(releaseName) if err != nil { - return nil, errors.Wrapf(err, "failed to retrieve history for '%s'", releaseName) + return nil, err } releaseutil.Reverse(hist, releaseutil.SortByRevision) diff --git a/pkg/helm/v3/repository.go b/pkg/helm/v3/repository.go index e39280f5e..adb58e3a8 100644 --- a/pkg/helm/v3/repository.go +++ b/pkg/helm/v3/repository.go @@ -11,7 +11,6 @@ import ( var repositoryConfigLock sync.RWMutex func (h *HelmV3) RepositoryIndex() error { - repositoryConfigLock.RLock() f, err := loadRepositoryConfig() repositoryConfigLock.RUnlock() @@ -113,7 +112,6 @@ func (h *HelmV3) RepositoryImport(path string) error { h.logger.Log("error", err, "name", c.Name, "url", c.URL) continue } - t.Add(c) h.logger.Log("info", "successfully imported repository", "name", c.Name, "url", c.URL) } diff --git a/pkg/helm/v3/rollback.go b/pkg/helm/v3/rollback.go index 279c0bd22..24b9df661 100644 --- a/pkg/helm/v3/rollback.go +++ b/pkg/helm/v3/rollback.go @@ -1,7 +1,6 @@ package v3 import ( - "github.com/pkg/errors" "helm.sh/helm/v3/pkg/action" "github.com/fluxcd/helm-operator/pkg/helm" @@ -17,7 +16,7 @@ func (h *HelmV3) Rollback(releaseName string, opts helm.RollbackOptions) (*helm. rollbackOptions(opts).configure(rollback) if err := rollback.Run(releaseName); err != nil { - return nil, errors.Wrapf(err, "failed to perform rollback for release '%s'", releaseName) + return nil, err } // As rolling back does no longer return information about diff --git a/pkg/helm/v3/uninstall.go b/pkg/helm/v3/uninstall.go index 59d252a39..5664f7d9c 100644 --- a/pkg/helm/v3/uninstall.go +++ b/pkg/helm/v3/uninstall.go @@ -1,10 +1,9 @@ package v3 import ( - "github.com/fluxcd/helm-operator/pkg/helm" - "github.com/pkg/errors" - "helm.sh/helm/v3/pkg/action" + + "github.com/fluxcd/helm-operator/pkg/helm" ) func (h *HelmV3) Uninstall(releaseName string, opts helm.UninstallOptions) error { @@ -16,10 +15,8 @@ func (h *HelmV3) Uninstall(releaseName string, opts helm.UninstallOptions) error uninstall := action.NewUninstall(cfg) uninstallOptions(opts).configure(uninstall) - if _, err := uninstall.Run(releaseName); err != nil { - return errors.Wrapf(err, "failed to uninstall release '%s'", releaseName) - } - return nil + _, err = uninstall.Run(releaseName) + return err } type uninstallOptions helm.UninstallOptions diff --git a/pkg/helm/v3/upgrade.go b/pkg/helm/v3/upgrade.go index c1180f207..7fff8f80f 100644 --- a/pkg/helm/v3/upgrade.go +++ b/pkg/helm/v3/upgrade.go @@ -1,8 +1,6 @@ package v3 import ( - "github.com/pkg/errors" - "helm.sh/helm/v3/pkg/action" "helm.sh/helm/v3/pkg/chart/loader" "helm.sh/helm/v3/pkg/chartutil" @@ -23,13 +21,13 @@ func (h *HelmV3) UpgradeFromPath(chartPath string, releaseName string, values [] // all chart dependencies are present chartRequested, err := loader.Load(chartPath) if err != nil { - return nil, errors.Wrapf(err, "failed to load chart from path '%s' for release '%s'", chartPath, releaseName) + return nil, err } // Read and set values val, err := chartutil.ReadValues(values) if err != nil { - return nil, errors.Wrap(err, "failed to read values") + return nil, err } var res *release.Release @@ -44,7 +42,7 @@ func (h *HelmV3) UpgradeFromPath(chartPath string, releaseName string, values [] } if err != nil { - return nil, errors.Wrapf(err, "failed to upgrade chart for release [%s]", releaseName) + return nil, err } return releaseToGenericRelease(res), err } diff --git a/pkg/release/release.go b/pkg/release/release.go index 595e5ac61..eb0f866cc 100644 --- a/pkg/release/release.go +++ b/pkg/release/release.go @@ -65,9 +65,6 @@ func New(logger log.Logger, helmClients *helm.Clients, coreV1Client corev1client func (r *Release) Sync(hr *v1.HelmRelease) (err error) { defer func(start time.Time) { ObserveRelease(start, err == nil, hr.GetTargetNamespace(), hr.GetReleaseName()) - if err != nil { - println("ERROR: " + err.Error()) - } }(time.Now()) defer status.SetObservedGeneration(r.hrClient.HelmReleases(hr.Namespace), hr, hr.Generation)