fix(azure): changed storage cleanup to delete files older than two days (#4012)

Signed-off-by: TrevorWalkerIBM <Trevorwalker@ibm.com>
Co-authored-by: Sean Holcomb <seanholcomb@gmail.com>

pkg/cloud/azure/storagebillingparser.go

@@ -117,7 +117,7 @@ func (asbp *AzureStorageBillingParser) ParseBillingData(start, end time.Time, re
 	if env.IsAzureDownloadBillingDataToDisk() {
 		// clean up old files that have been saved to disk before downloading new ones
 		localPath := env.GetAzureDownloadBillingDataPath()
-		if _, err := asbp.deleteFilesOlderThan7d(localPath); err != nil {
+		if _, err := asbp.deleteFilesOlderThanRetention(localPath); err != nil {
 			log.Warnf("CloudCost: Azure: ParseBillingData: failed to remove the following stale files: %v", err)
 		}
 		for _, blob := range blobInfos {

pkg/cloud/azure/storageconnection.go

@@ -14,6 +14,7 @@ import (
 	"github.com/Azure/azure-sdk-for-go/sdk/storage/azblob/container"
 	"github.com/opencost/opencost/core/pkg/log"
 	"github.com/opencost/opencost/pkg/cloud"
+	"github.com/opencost/opencost/pkg/env"
 )
 
 // StorageConnection provides access to Azure Storage
@@ -133,13 +134,14 @@ func (sc *StorageConnection) DownloadBlobToFile(localFilePath string, blob conta
 	return nil
 }
 
-// deleteFilesOlderThan7d recursively walks the directory specified and deletes
-// files which have not been modified in the last 7 days. Returns a list of
+// deleteFilesOlderThanRetention recursively walks the directory specified and deletes
+// files which have not been modified in the last N days. Returns a list of
 // files deleted.
-func (sc *StorageConnection) deleteFilesOlderThan7d(localPath string) ([]string, error) {
+// Retention period is determined by the CLOUD_COST_PV_RETENTION environment variable, which defaults to 2 days.
+func (sc *StorageConnection) deleteFilesOlderThanRetention(localPath string) ([]string, error) {
 	sc.lock.Lock()
 	defer sc.lock.Unlock()
-	duration := 7 * 24 * time.Hour
+	duration := time.Duration(env.GetCloudCostPvRetention()) * 24 * time.Hour
 	cleaned := []string{}
 	errs := []string{}
 
@@ -166,6 +168,6 @@ func (sc *StorageConnection) deleteFilesOlderThan7d(localPath string) ([]string,
 	if len(errs) == 0 {
 		return cleaned, nil
 	} else {
-		return cleaned, fmt.Errorf("deleteFilesOlderThan7d: %v", errs)
+		return cleaned, fmt.Errorf("deleteFilesOlderThanRetention: %v", errs)
 	}
 }

pkg/cloud/azure/storageconnection_test.go

@@ -0,0 +1,90 @@
+package azure
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/opencost/opencost/core/pkg/env"
+	pkgenv "github.com/opencost/opencost/pkg/env"
+)
+
+func TestDeleteFilesOlderThanRetention(t *testing.T) {
+	testCases := []struct {
+		name     string
+		pre      func()
+		files    map[string]time.Duration
+		deleted  int
+		expected string
+	}{
+		{
+			name: "Ensure the default value of '2' works",
+			files: map[string]time.Duration{
+				"today.gz":        1 * 24 * time.Hour,
+				"yesterday.gz":    1.5 * 24 * time.Hour,
+				"two_days_ago.gz": 3 * 24 * time.Hour,
+			},
+			deleted:  1,
+			expected: "today.gz,yesterday.gz",
+		},
+		{
+			name: "Ensure the a value of 7 works",
+			pre: func() {
+				env.Set(pkgenv.CloudCostPvRetentionEnvVar, "7")
+			},
+			files: map[string]time.Duration{
+				"today.gz":        1 * 24 * time.Hour,
+				"yesterday.gz":    1.5 * 24 * time.Hour,
+				"two_days_ago.gz": 3 * 24 * time.Hour,
+			},
+			deleted:  0,
+			expected: "today.gz,yesterday.gz,two_days_ago.gz",
+		},
+		{
+			name: "Ensure the a value of 7 works",
+			pre: func() {
+				env.Set(pkgenv.CloudCostPvRetentionEnvVar, "7")
+			},
+			files: map[string]time.Duration{
+				"today.gz":        1 * 24 * time.Hour,
+				"yesterday.gz":    1.5 * 24 * time.Hour,
+				"two_days_ago.gz": 3 * 24 * time.Hour,
+				"old_file.gz":     8 * 24 * time.Hour,
+			},
+			deleted:  1,
+			expected: "today.gz,yesterday.gz,two_days_ago.gz",
+		},
+	}
+	for _, tt := range testCases {
+		if tt.pre != nil {
+			tt.pre()
+		}
+		tmpDir, err := os.MkdirTemp("", "test-delete-files")
+		if err != nil {
+			t.Errorf("Failed to make temp directory: %v", err)
+		}
+		defer os.RemoveAll(tmpDir)
+		for name, days := range tt.files {
+			confPath := filepath.Join(tmpDir, name)
+			err = os.WriteFile(confPath, []byte(`{"status": "ok"}`), 0644)
+			if err != nil {
+				t.Errorf("Failed to write file inside temp directory: %v", err)
+			}
+			modTime := time.Now().Add(-days)
+			err = os.Chtimes(confPath, modTime, modTime)
+			if err != nil {
+				t.Errorf("Failed to set modification time for file: %v", err)
+			}
+		}
+
+		sc := &StorageConnection{}
+		cleaned, err := sc.deleteFilesOlderThanRetention(tmpDir)
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		if len(cleaned) != tt.deleted {
+			t.Errorf("deleteFilesOlderThanRetention() cleaned %d files, want %d", len(cleaned), tt.deleted)
+		}
+	}
+}

pkg/env/cloudcost.go

@@ -17,6 +17,7 @@ const (
 	CloudCostRefreshRateHoursEnvVar = "CLOUD_COST_REFRESH_RATE_HOURS"
 	CloudCostQueryWindowDaysEnvVar  = "CLOUD_COST_QUERY_WINDOW_DAYS"
 	CloudCostRunWindowDaysEnvVar    = "CLOUD_COST_RUN_WINDOW_DAYS"
+	CloudCostPvRetentionEnvVar      = "CLOUD_COST_PV_RETENTION"
 
 	CustomCostEnvVarPrefix          = "CUSTOM_COST_"
 	CustomCostEnabledEnvVar         = "CUSTOM_COST_ENABLED"
@@ -68,6 +69,10 @@ func GetCustomCostQueryWindowDays() int {
 	return env.GetInt(CustomCostQueryWindowDaysEnvVar, 7)
 }
 
+func GetCloudCostPvRetention() int {
+	return env.GetInt(CloudCostPvRetentionEnvVar, 2)
+}
+
 func GetCustomCost1dRetention() int {
 	return env.GetPrefixInt(CustomCostEnvVarPrefix, env.Resolution1dRetentionEnvVar, 30)
 }

pkg/env/cloudcost_test.go

@@ -36,3 +36,33 @@ func TestGetCloudCostConfigPath(t *testing.T) {
 	}
 
 }
+
+func TestGetCloudCostPvRetention(t *testing.T) {
+	tests := []struct {
+		name string
+		want int
+		pre  func()
+	}{
+		{
+			name: "Ensure the default value is '2'",
+			want: 2,
+		},
+		{
+			name: "Ensure the value is 7 when CLOUD_COST_PVC_RETENTION is set to '7'",
+			want: 7,
+			pre: func() {
+				env.Set(CloudCostPvRetentionEnvVar, "7")
+			},
+		},
+	}
+	for _, tt := range tests {
+		if tt.pre != nil {
+			tt.pre()
+		}
+		t.Run(tt.name, func(t *testing.T) {
+			if got := GetCloudCostPvRetention(); got != tt.want {
+				t.Errorf("GetCloudCostPvRetention() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}