From 5cf88bd832ea2186c1b4e60af1d367e61aa952cd Mon Sep 17 00:00:00 2001
From: Adnan Abdulhussein <adnan@prydoni.us>
Date: Wed, 21 Jun 2017 12:13:59 +0100
Subject: [PATCH] clean chart cache dir on error processing (#281)

* clean chart cache dir on error processing

* improve coverage

* address review

* use real function for clearChartDataDir

* fix defer approach
---
 .../cache/charthelper/chart_package_helper.go | 12 ++++++++-
 .../charthelper/chart_package_helper_test.go  | 25 +++++++++++++++++++
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/src/api/data/cache/charthelper/chart_package_helper.go b/src/api/data/cache/charthelper/chart_package_helper.go
index 16f039f4a..5245c41ef 100644
--- a/src/api/data/cache/charthelper/chart_package_helper.go
+++ b/src/api/data/cache/charthelper/chart_package_helper.go
@@ -22,11 +22,17 @@ const defaultTimeout time.Duration = 10 * time.Second
 
 // DownloadAndExtractChartTarball the chart tar file linked by metadata.Urls and store
 // the wanted files (i.e README.md) under chartDataDir
-var DownloadAndExtractChartTarball = func(chart *models.ChartPackage) error {
+var DownloadAndExtractChartTarball = func(chart *models.ChartPackage) (err error) {
 	if err := ensureChartDataDir(chart); err != nil {
 		return err
 	}
 
+	defer func() {
+		if err != nil {
+			cleanChartDataDir(chart)
+		}
+	}()
+
 	if !tarballExists(chart) {
 		if err := downloadTarball(chart); err != nil {
 			return err
@@ -134,6 +140,10 @@ var ensureChartDataDir = func(chart *models.ChartPackage) error {
 	return nil
 }
 
+func cleanChartDataDir(chart *models.ChartPackage) error {
+	return os.RemoveAll(chartDataDir(chart))
+}
+
 // TarballPath returns the location of the chart package in the local cache
 var TarballPath = func(chart *models.ChartPackage) string {
 	return filepath.Join(chartDataDir(chart), "chart.tgz")
diff --git a/src/api/data/cache/charthelper/chart_package_helper_test.go b/src/api/data/cache/charthelper/chart_package_helper_test.go
index eda325453..1752c47e4 100644
--- a/src/api/data/cache/charthelper/chart_package_helper_test.go
+++ b/src/api/data/cache/charthelper/chart_package_helper_test.go
@@ -44,6 +44,8 @@ func TestDownloadAndExtractChartTarballErrorDownload(t *testing.T) {
 	assert.NoErr(t, err)
 	err = DownloadAndExtractChartTarball(chart)
 	assert.ExistsErr(t, err, "Error downloading the tar file")
+	_, err = os.Stat(chartDataDir(chart))
+	assert.ExistsErr(t, err, "chart data dir has been removed")
 }
 
 func TestDownloadAndExtractChartTarballErrorExtract(t *testing.T) {
@@ -54,6 +56,8 @@ func TestDownloadAndExtractChartTarballErrorExtract(t *testing.T) {
 	assert.NoErr(t, err)
 	err = DownloadAndExtractChartTarball(chart)
 	assert.ExistsErr(t, err, "Error extracting tar file content")
+	_, err = os.Stat(chartDataDir(chart))
+	assert.ExistsErr(t, err, "chart data dir has been removed")
 }
 
 // It creates the tar file in local filesystem
@@ -200,6 +204,27 @@ func TestEnsureChartDataDir(t *testing.T) {
 	assert.NoErr(t, err)
 }
 
+func TestCleanChartDataDir(t *testing.T) {
+	chart, err := getTestChart()
+	assert.NoErr(t, err)
+	randomPath, _ := ioutil.TempDir(os.TempDir(), "chart")
+	DataDirBaseOrig := DataDirBase
+	defer func() { DataDirBase = DataDirBaseOrig }()
+	DataDirBase = func() string {
+		return randomPath
+	}
+	chartPath := chartDataDir(chart)
+	ensureChartDataDir(chart)
+	_, err = os.Stat(chartPath)
+	assert.NoErr(t, err)
+
+	err = cleanChartDataDir(chart)
+	assert.NoErr(t, err)
+
+	_, err = os.Stat(chartPath)
+	assert.ExistsErr(t, err, "chart dir removed")
+}
+
 // Required because DataDirBase has been overriden
 var origDataDirBase = DataDirBase()