diff --git a/cmd/helm/dependency_update_test.go b/cmd/helm/dependency_update_test.go index 227959c53..e29cb35de 100644 --- a/cmd/helm/dependency_update_test.go +++ b/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. diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index 388010fbc..2d4b73d8a 100644 --- a/pkg/downloader/manager.go +++ b/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 }