Merge pull request #10024 from hickeyma/feat/fail-on-repo-update

feat(repo update): Add flag for failing command if any update of a repo fails

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)
+	update               func([]*repo.ChartRepository, io.Writer, bool) error
-	repoFile  string
+	repoFile             string
-	repoCache string
+	repoCache            string
-	names     []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 o.update(repos, out, o.failOnRepoUpdateFail)
-	return nil
 }
 
-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 {

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")
+	}
+}