Delete old deps after chart deps are updated

This change changes the order of operations in
pkg/downloader.Manager.downloadAll

Old charts are moved to tmp directory which is restored in case any
dependency update fails. Otherwise tmp dir is deleted.

cmd/helm/dependency_update_test.go

@@ -172,6 +172,75 @@ func TestDependencyUpdateCmd_SkipRefresh(t *testing.T) {
 	}
 }
 
+func TestDependencyUpdateCmd_DontDeleteOldChartsOnError(t *testing.T) {
+	hh, err := tempHelmHome(t)
+	if err != nil {
+		t.Fatal(err)
+	}
+	cleanup := resetEnv()
+	defer func() {
+		os.RemoveAll(hh.String())
+		cleanup()
+	}()
+
+	settings.Home = hh
+
+	srv := repotest.NewServer(hh.String())
+	defer srv.Stop()
+	copied, err := srv.CopyCharts("testdata/testcharts/*.tgz")
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Logf("Copied charts:\n%s", strings.Join(copied, "\n"))
+	t.Logf("Listening on directory %s", srv.Root())
+
+	chartname := "depupdelete"
+	if err := createTestingChart(hh.String(), chartname, srv.URL()); err != nil {
+		t.Fatal(err)
+	}
+
+	out := bytes.NewBuffer(nil)
+	duc := &dependencyUpdateCmd{out: out}
+	duc.helmhome = helmpath.Home(hh)
+	duc.chartpath = filepath.Join(hh.String(), chartname)
+
+	if err := duc.run(); err != nil {
+		output := out.String()
+		t.Logf("Output: %s", output)
+		t.Fatal(err)
+	}
+
+	// Chart repo is down
+	srv.Stop()
+
+	if err := duc.run(); err == nil {
+		output := out.String()
+		t.Logf("Output: %s", output)
+		t.Fatal("Expected error, got nil")
+	}
+
+	// Make sure charts dir still has dependencies
+	files, err := ioutil.ReadDir(filepath.Join(duc.chartpath, "charts"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	dependencies := []string{"compressedchart-0.1.0.tgz", "reqtest-0.1.0.tgz"}
+
+	if len(dependencies) != len(files) {
+		t.Fatalf("Expected %d chart dependencies, got %d", len(dependencies), len(files))
+	}
+	for index, file := range files {
+		if dependencies[index] != file.Name() {
+			t.Fatalf("Chart dependency %s not matching %s", dependencies[index], file.Name())
+		}
+	}
+
+	// Make sure tmpcharts is deleted
+	if _, err := os.Stat(filepath.Join(duc.chartpath, "tmpcharts")); !os.IsNotExist(err) {
+		t.Fatalf("tmpcharts dir still exists")
+	}
+}
+
 // createTestingChart creates a basic chart that depends on reqtest-0.1.0
 //
 // The baseURL can be used to point to a particular repository server.

pkg/downloader/manager.go

@@ -206,6 +206,7 @@ func (m *Manager) downloadAll(deps []*chartutil.Dependency) error {
 	}
 
 	destPath := filepath.Join(m.ChartPath, "charts")
+	tmpPath := filepath.Join(m.ChartPath, "tmpcharts")
 
 	// Create 'charts' directory if it doesn't already exist.
 	if fi, err := os.Stat(destPath); err != nil {
@@ -216,14 +217,16 @@ func (m *Manager) downloadAll(deps []*chartutil.Dependency) error {
 		return fmt.Errorf("%q is not a directory", destPath)
 	}
 
-	fmt.Fprintln(m.Out, "Deleting outdated charts")
-	for _, dep := range deps {
-		if err := m.safeDeleteDep(dep.Name, destPath); err != nil {
-			return err
-		}
+	if err := os.Rename(destPath, tmpPath); err != nil {
+		return fmt.Errorf("Unable to move current charts to tmp dir: %v", err)
+	}
+
+	if err := os.MkdirAll(destPath, 0755); err != nil {
+		return err
 	}
 
 	fmt.Fprintf(m.Out, "Saving %d charts\n", len(deps))
+	var saveError error
 	for _, dep := range deps {
 		if strings.HasPrefix(dep.Repository, "file://") {
 			if m.Debug {
@@ -231,7 +234,8 @@ func (m *Manager) downloadAll(deps []*chartutil.Dependency) error {
 			}
 			ver, err := tarFromLocalDir(m.ChartPath, dep.Name, dep.Repository, dep.Version)
 			if err != nil {
-				return err
+				saveError = err
+				break
 			}
 			dep.Version = ver
 			continue
@@ -243,12 +247,41 @@ func (m *Manager) downloadAll(deps []*chartutil.Dependency) error {
 		// https://github.com/kubernetes/helm/issues/1439
 		churl, err := findChartURL(dep.Name, dep.Version, dep.Repository, repos)
 		if err != nil {
-			return fmt.Errorf("could not find %s: %s", churl, err)
+			saveError = fmt.Errorf("could not find %s: %s", churl, err)
+			break
 		}
 
 		if _, _, err := dl.DownloadTo(churl, "", destPath); err != nil {
-			return fmt.Errorf("could not download %s: %s", churl, err)
+			saveError = fmt.Errorf("could not download %s: %s", churl, err)
+			break
+		}
+	}
+
+	if saveError == nil {
+		fmt.Fprintln(m.Out, "Deleting outdated charts")
+		for _, dep := range deps {
+			if err := m.safeDeleteDep(dep.Name, tmpPath); err != nil {
+				return err
+			}
+		}
+		if err := os.RemoveAll(tmpPath); err != nil {
+			return fmt.Errorf("Failed to remove %v: %v", tmpPath, err)
+		}
+	} else {
+		fmt.Fprintln(m.Out, "Save error occurred: ", saveError)
+		fmt.Fprintln(m.Out, "Deleting newly downloaded charts, restoring pre-update state")
+		for _, dep := range deps {
+			if err := m.safeDeleteDep(dep.Name, destPath); err != nil {
+				return err
+			}
+		}
+		if err := os.RemoveAll(destPath); err != nil {
+			return fmt.Errorf("Failed to remove %v: %v", destPath, err)
+		}
+		if err := os.Rename(tmpPath, destPath); err != nil {
+			return fmt.Errorf("Unable to move current charts to tmp dir: %v", err)
 		}
+		return saveError
 	}
 	return nil
 }