Merge pull request #1913 from technosophos/fix/1864-delete-old-deps

fix(helm): delete outdated deps

cmd/helm/dependency_update.go

@@ -28,10 +28,15 @@ const dependencyUpDesc = `
 Update the on-disk dependencies to mirror the requirements.yaml file.
 
 This command verifies that the required charts, as expressed in 'requirements.yaml',
-are present in 'charts/' and are at an acceptable version.
+are present in 'charts/' and are at an acceptable version. It will pull down
+the latest charts that satisfy the dependencies, and clean up old dependencies.
 
 On successful update, this will generate a lock file that can be used to
 rebuild the requirements to an exact version.
+
+Dependencies are not required to be represented in 'requirements.yaml'. For that
+reason, an update command will not remove charts unless they are (a) present
+in the requirements.yaml file, but (b) at the wrong version.
 `
 
 // dependencyUpdateCmd describes a 'helm dependency update'

cmd/helm/dependency_update_test.go

@@ -98,7 +98,34 @@ func TestDependencyUpdateCmd(t *testing.T) {
 		t.Errorf("Failed hash match: expected %s, got %s", hash, h)
 	}
 
-	t.Logf("Results: %s", out.String())
+	// Now change the dependencies and update. This verifies that on update,
+	// old dependencies are cleansed and new dependencies are added.
+	reqfile := &chartutil.Requirements{
+		Dependencies: []*chartutil.Dependency{
+			{Name: "reqtest", Version: "0.1.0", Repository: srv.URL()},
+			{Name: "compressedchart", Version: "0.3.0", Repository: srv.URL()},
+		},
+	}
+	dir := filepath.Join(hh, chartname)
+	if err := writeRequirements(dir, reqfile); err != nil {
+		t.Fatal(err)
+	}
+	if err := duc.run(); err != nil {
+		output := out.String()
+		t.Logf("Output: %s", output)
+		t.Fatal(err)
+	}
+
+	// In this second run, we should see compressedchart-0.3.0.tgz, and not
+	// the 0.1.0 version.
+	expect = filepath.Join(hh, chartname, "charts/compressedchart-0.3.0.tgz")
+	if _, err := os.Stat(expect); err != nil {
+		t.Fatalf("Expected %q: %s", expect, err)
+	}
+	dontExpect := filepath.Join(hh, chartname, "charts/compressedchart-0.1.0.tgz")
+	if _, err := os.Stat(dontExpect); err == nil {
+		t.Fatalf("Unexpected %q", dontExpect)
+	}
 }
 
 // createTestingChart creates a basic chart that depends on reqtest-0.1.0
@@ -117,8 +144,13 @@ func createTestingChart(dest, name, baseURL string) error {
 	req := &chartutil.Requirements{
 		Dependencies: []*chartutil.Dependency{
 			{Name: "reqtest", Version: "0.1.0", Repository: baseURL},
+			{Name: "compressedchart", Version: "0.1.0", Repository: baseURL},
 		},
 	}
+	return writeRequirements(dir, req)
+}
+
+func writeRequirements(dir string, req *chartutil.Requirements) error {
 	data, err := yaml.Marshal(req)
 	if err != nil {
 		return err

cmd/helm/downloader/manager.go

@@ -169,6 +169,9 @@ func (m *Manager) resolve(req *chartutil.Requirements, repoNames map[string]stri
 }
 
 // downloadAll takes a list of dependencies and downloads them into charts/
+//
+// It will delete versions of the chart that exist on disk and might cause
+// a conflict.
 func (m *Manager) downloadAll(deps []*chartutil.Dependency) error {
 	repos, err := m.loadChartRepositories()
 	if err != nil {
@@ -195,6 +198,9 @@ func (m *Manager) downloadAll(deps []*chartutil.Dependency) error {
 
 	fmt.Fprintf(m.Out, "Saving %d charts\n", len(deps))
 	for _, dep := range deps {
+		if err := m.safeDeleteDep(dep.Name, destPath); err != nil {
+			return err
+		}
 		fmt.Fprintf(m.Out, "Downloading %s from repo %s\n", dep.Name, dep.Repository)
 
 		// Any failure to resolve/download a chart should fail:
@@ -211,6 +217,39 @@ func (m *Manager) downloadAll(deps []*chartutil.Dependency) error {
 	return nil
 }
 
+// safeDeleteDep deletes any versions of the given dependency in the given directory.
+//
+// It does this by first matching the file name to an expected pattern, then loading
+// the file to verify that it is a chart with the same name as the given name.
+//
+// Because it requires tar file introspection, it is more intensive than a basic delete.
+//
+// This will only return errors that should stop processing entirely. Other errors
+// will emit log messages or be ignored.
+func (m *Manager) safeDeleteDep(name, dir string) error {
+	files, err := filepath.Glob(filepath.Join(dir, name+"-*.tgz"))
+	if err != nil {
+		// Only for ErrBadPattern
+		return err
+	}
+	for _, fname := range files {
+		ch, err := chartutil.LoadFile(fname)
+		if err != nil {
+			fmt.Fprintf(m.Out, "Could not verify %s for deletion: %s (Skipping)", fname, err)
+			continue
+		}
+		if ch.Metadata.Name != name {
+			// This is not the file you are looking for.
+			continue
+		}
+		if err := os.Remove(fname); err != nil {
+			fmt.Fprintf(m.Out, "Could not delete %s: %s (Skipping)", fname, err)
+			continue
+		}
+	}
+	return nil
+}
+
 // hasAllRepos ensures that all of the referenced deps are in the local repo cache.
 func (m *Manager) hasAllRepos(deps []*chartutil.Dependency) error {
 	rf, err := repo.LoadRepositoriesFile(m.HelmHome.RepositoryFile())