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

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

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()