From ccb0cf63e4778df03251d2866c7b485df3683f79 Mon Sep 17 00:00:00 2001 From: Martin Hickey Date: Fri, 13 Aug 2021 13:05:23 +0100 Subject: [PATCH] Add flag for failing if repo update fails Signed-off-by: Martin Hickey --- cmd/helm/repo_update.go | 30 ++++++++++--- cmd/helm/repo_update_test.go | 87 ++++++++++++++++++++++++++++++++++-- 2 files changed, 106 insertions(+), 11 deletions(-) diff --git a/cmd/helm/repo_update.go b/cmd/helm/repo_update.go index 43036847d..ae09f9195 100644 --- a/cmd/helm/repo_update.go +++ b/cmd/helm/repo_update.go @@ -41,10 +41,11 @@ To update all the repositories, use 'helm repo update'. var errNoRepositories = errors.New("no repositories found. You must add one before updating") type repoUpdateOptions struct { - update func([]*repo.ChartRepository, io.Writer) - repoFile string - repoCache string - names []string + update func([]*repo.ChartRepository, io.Writer, bool) error + repoFile string + repoCache string + names []string + failOnRepoUpdateFail bool } func newRepoUpdateCmd(out io.Writer) *cobra.Command { @@ -66,6 +67,13 @@ func newRepoUpdateCmd(out io.Writer) *cobra.Command { return o.run(out) }, } + + f := cmd.Flags() + + // Adding this flag for Helm 3 as stop gap functionality for https://github.com/helm/helm/issues/10016. + // This should be deprecated in Helm 4 by update to the behaviour of `helm repo update` command. + f.BoolVar(&o.failOnRepoUpdateFail, "fail-on-repo-update-fail", false, "update fails if any of the repository updates fail") + return cmd } @@ -103,26 +111,34 @@ func (o *repoUpdateOptions) run(out io.Writer) error { } } - o.update(repos, out) - return nil + return o.update(repos, out, o.failOnRepoUpdateFail) } -func updateCharts(repos []*repo.ChartRepository, out io.Writer) { +func updateCharts(repos []*repo.ChartRepository, out io.Writer, failOnRepoUpdateFail bool) error { fmt.Fprintln(out, "Hang tight while we grab the latest from your chart repositories...") var wg sync.WaitGroup + var repoFailList []string for _, re := range repos { wg.Add(1) go func(re *repo.ChartRepository) { defer wg.Done() if _, err := re.DownloadIndexFile(); err != nil { fmt.Fprintf(out, "...Unable to get an update from the %q chart repository (%s):\n\t%s\n", re.Config.Name, re.Config.URL, err) + repoFailList = append(repoFailList, re.Config.URL) } else { fmt.Fprintf(out, "...Successfully got an update from the %q chart repository\n", re.Config.Name) } }(re) } wg.Wait() + + if len(repoFailList) > 0 && failOnRepoUpdateFail { + return errors.New(fmt.Sprintf("Failed to update the following repositories: %s", + repoFailList)) + } + fmt.Fprintln(out, "Update Complete. ⎈Happy Helming!⎈") + return nil } func checkRequestedRepos(requestedRepos []string, validRepos []*repo.Entry) error { diff --git a/cmd/helm/repo_update_test.go b/cmd/helm/repo_update_test.go index d769c8aa7..cf2136ff0 100644 --- a/cmd/helm/repo_update_test.go +++ b/cmd/helm/repo_update_test.go @@ -35,10 +35,11 @@ func TestUpdateCmd(t *testing.T) { var out bytes.Buffer // Instead of using the HTTP updater, we provide our own for this test. // The TestUpdateCharts test verifies the HTTP behavior independently. - updater := func(repos []*repo.ChartRepository, out io.Writer) { + updater := func(repos []*repo.ChartRepository, out io.Writer, failOnRepoUpdateFail bool) error { for _, re := range repos { fmt.Fprintln(out, re.Config.Name) } + return nil } o := &repoUpdateOptions{ update: updater, @@ -59,10 +60,11 @@ func TestUpdateCmdMultiple(t *testing.T) { var out bytes.Buffer // Instead of using the HTTP updater, we provide our own for this test. // The TestUpdateCharts test verifies the HTTP behavior independently. - updater := func(repos []*repo.ChartRepository, out io.Writer) { + updater := func(repos []*repo.ChartRepository, out io.Writer, failOnRepoUpdateFail bool) error { for _, re := range repos { fmt.Fprintln(out, re.Config.Name) } + return nil } o := &repoUpdateOptions{ update: updater, @@ -84,10 +86,11 @@ func TestUpdateCmdInvalid(t *testing.T) { var out bytes.Buffer // Instead of using the HTTP updater, we provide our own for this test. // The TestUpdateCharts test verifies the HTTP behavior independently. - updater := func(repos []*repo.ChartRepository, out io.Writer) { + updater := func(repos []*repo.ChartRepository, out io.Writer, failOnRepoUpdateFail bool) error { for _, re := range repos { fmt.Fprintln(out, re.Config.Name) } + return nil } o := &repoUpdateOptions{ update: updater, @@ -144,7 +147,7 @@ func TestUpdateCharts(t *testing.T) { } b := bytes.NewBuffer(nil) - updateCharts([]*repo.ChartRepository{r}, b) + updateCharts([]*repo.ChartRepository{r}, b, false) got := b.String() if strings.Contains(got, "Unable to get an update") { @@ -159,3 +162,79 @@ func TestRepoUpdateFileCompletion(t *testing.T) { checkFileCompletion(t, "repo update", false) checkFileCompletion(t, "repo update repo1", false) } + +func TestUpdateChartsFail(t *testing.T) { + defer resetEnv()() + defer ensure.HelmHome(t)() + + ts, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*") + if err != nil { + t.Fatal(err) + } + defer ts.Stop() + + var invalidURL = ts.URL() + "55" + r, err := repo.NewChartRepository(&repo.Entry{ + Name: "charts", + URL: invalidURL, + }, getter.All(settings)) + if err != nil { + t.Error(err) + } + + b := bytes.NewBuffer(nil) + if err := updateCharts([]*repo.ChartRepository{r}, b, false); err != nil { + t.Error("Repo update should not return error if update of repository fails") + } + + got := b.String() + if !strings.Contains(got, "Unable to get an update") { + t.Errorf("Repo should have failed update but instead got: %q", got) + } + if !strings.Contains(got, "Update Complete.") { + t.Error("Update was not successful") + } +} + +func TestUpdateChartsFailWithError(t *testing.T) { + defer resetEnv()() + defer ensure.HelmHome(t)() + + ts, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*") + if err != nil { + t.Fatal(err) + } + defer ts.Stop() + + var invalidURL = ts.URL() + "55" + r, err := repo.NewChartRepository(&repo.Entry{ + Name: "charts", + URL: invalidURL, + }, getter.All(settings)) + if err != nil { + t.Error(err) + } + + b := bytes.NewBuffer(nil) + err = updateCharts([]*repo.ChartRepository{r}, b, true) + if err == nil { + t.Error("Repo update should return error because update of repository fails and 'fail-on-repo-update-fail' flag set") + return + } + var expectedErr = "Failed to update the following repositories" + var receivedErr = err.Error() + if !strings.Contains(receivedErr, expectedErr) { + t.Errorf("Expected error (%s) but got (%s) instead", expectedErr, receivedErr) + } + if !strings.Contains(receivedErr, invalidURL) { + t.Errorf("Expected invalid URL (%s) in error message but got (%s) instead", invalidURL, receivedErr) + } + + got := b.String() + if !strings.Contains(got, "Unable to get an update") { + t.Errorf("Repo should have failed update but instead got: %q", got) + } + if strings.Contains(got, "Update Complete.") { + t.Error("Update was not successful and should return error message because 'fail-on-repo-update-fail' flag set") + } +}