fix: config retention tests and flow

flanksource · Dec 4, 2023 · 9319631 · 9319631
1 parent 36a4300
commit 9319631
Show file tree

Hide file tree

Showing 2 changed files with 88 additions and 37 deletions.
diff --git a/scrapers/retention.go b/scrapers/retention.go
@@ -1,7 +1,9 @@
 package scrapers
 
 import (
+	"database/sql"
 	"fmt"
+	"strings"
 
 	"github.com/flanksource/commons/duration"
 	"github.com/flanksource/commons/logger"
@@ -11,35 +13,54 @@ import (
 )
 
 func ProcessChangeRetention(ctx context.Context, scraperID uuid.UUID, spec v1.ChangeRetentionSpec) error {
-	age, err := duration.ParseDuration(spec.Age)
-	if err != nil {
-		return fmt.Errorf("error parsing age %s as duration: %w", spec.Age, err)
+	var whereClauses []string
+
+	var ageMinutes int
+	if spec.Age != "" {
+		age, err := duration.ParseDuration(spec.Age)
+		if err != nil {
+			return fmt.Errorf("error parsing age %s as duration: %w", spec.Age, err)
+		}
+		ageMinutes = int(age.Minutes())
+
+		whereClauses = append(whereClauses, `((now()- created_at) > interval '1 minute' * @ageMinutes)`)
+	}
+
+	if spec.Count > 0 {
+		whereClauses = append(whereClauses, `seq > @count`)
 	}
-	ageMinutes := int(age.Minutes())
 
-	query := `
+	if len(whereClauses) == 0 {
+		return fmt.Errorf("both age and count cannot be empty")
+	}
+
+	query := fmt.Sprintf(`
         WITH latest_config_changes AS (
             SELECT id, change_type, created_at, ROW_NUMBER() OVER(ORDER BY created_at DESC) AS seq
             FROM config_changes
             WHERE
-                change_type = ? AND
-                config_id IN (SELECT id FROM config_items WHERE scraper_id = ?) AND
-                ((now()- created_at) < interval '1 minute' * ?)
+                change_type = @changeType AND
+                config_id IN (SELECT id FROM config_items WHERE scraper_id = @scraperID)
         )
         DELETE FROM config_changes
         WHERE id IN (
-            SELECT id from latest_config_changes WHERE seq > ?
+            SELECT id from latest_config_changes
+            WHERE %s
         )
-    `
+    `, strings.Join(whereClauses, " OR "))
 
-	result := ctx.DB().Exec(query, spec.Name, scraperID, ageMinutes, spec.Count)
+	result := ctx.DB().Exec(query,
+		sql.Named("changeType", spec.Name),
+		sql.Named("scraperID", scraperID),
+		sql.Named("ageMinutes", ageMinutes),
+		sql.Named("count", spec.Count),
+	)
 	if err := result.Error; err != nil {
 		return fmt.Errorf("error retaining config changes: %w", err)
 	}
 
 	if result.RowsAffected > 0 {
-		logger.Infof("Marked %d config_changes as deleted", result.RowsAffected)
+		logger.Infof("Deleted %d config_changes as per ChangeRetentionSpec[%s]", result.RowsAffected, spec.Name)
 	}
-
 	return nil
 }
diff --git a/scrapers/runscrapers_test.go b/scrapers/runscrapers_test.go
@@ -224,7 +224,6 @@ var _ = Describe("Scrapers test", Ordered, func() {
 		})
 
 		It("should retain config changes as per the spec", func() {
-
 			dummyScraper := dutymodels.ConfigScraper{
 				Name:   "Test",
 				Spec:   `{"foo":"bar"}`,
@@ -233,35 +232,39 @@ var _ = Describe("Scrapers test", Ordered, func() {
 			err := db.DefaultDB().Create(&dummyScraper).Error
 			Expect(err).To(BeNil())
 
-			ciID := uuid.New()
+			configItemID := uuid.New().String()
 			dummyCI := models.ConfigItem{
-				ID:          ciID.String(),
+				ID:          configItemID,
 				ConfigClass: "Test",
 				ScraperID:   &dummyScraper.ID,
 			}
 			err = db.DefaultDB().Create(&dummyCI).Error
 			Expect(err).To(BeNil())
 
-			configItemID := dummyCI.ID
-
 			twoDaysAgo := time.Now().Add(-2 * 24 * time.Hour)
 			fiveDaysAgo := time.Now().Add(-5 * 24 * time.Hour)
+			tenDaysAgo := time.Now().Add(-10 * 24 * time.Hour)
 			configChanges := []models.ConfigChange{
-				{ConfigID: configItemID, ChangeType: "TestDiff"},
-				{ConfigID: configItemID, ChangeType: "TestDiff"},
-				{ConfigID: configItemID, ChangeType: "TestDiff"},
-				{ConfigID: configItemID, ChangeType: "TestDiff"},
-				{ConfigID: configItemID, ChangeType: "TestDiff"},
-				{ConfigID: configItemID, ChangeType: "TestDiff"},
-				{ConfigID: configItemID, ChangeType: "TestDiff", CreatedAt: &twoDaysAgo},
-				{ConfigID: configItemID, ChangeType: "TestDiff", CreatedAt: &twoDaysAgo},
-				{ConfigID: configItemID, ChangeType: "TestDiff", CreatedAt: &twoDaysAgo},
-				{ConfigID: configItemID, ChangeType: "TestDiff", CreatedAt: &fiveDaysAgo},
-				{ConfigID: configItemID, ChangeType: "TestDiff", CreatedAt: &fiveDaysAgo},
-				{ConfigID: configItemID, ChangeType: "TestDiff", CreatedAt: &fiveDaysAgo},
-				{ConfigID: configItemID, ChangeType: "TestDiff", CreatedAt: &fiveDaysAgo},
+				{ConfigID: configItemID, ChangeType: "TestDiff", ExternalChangeId: uuid.New().String()},
+				{ConfigID: configItemID, ChangeType: "TestDiff", ExternalChangeId: uuid.New().String()},
+				{ConfigID: configItemID, ChangeType: "TestDiff", ExternalChangeId: uuid.New().String()},
+				{ConfigID: configItemID, ChangeType: "TestDiff", ExternalChangeId: uuid.New().String()},
+				{ConfigID: configItemID, ChangeType: "TestDiff", ExternalChangeId: uuid.New().String()},
+				{ConfigID: configItemID, ChangeType: "TestDiff", CreatedAt: &twoDaysAgo, ExternalChangeId: uuid.New().String()},
+				{ConfigID: configItemID, ChangeType: "TestDiff", CreatedAt: &twoDaysAgo, ExternalChangeId: uuid.New().String()},
+				{ConfigID: configItemID, ChangeType: "TestDiff", CreatedAt: &twoDaysAgo, ExternalChangeId: uuid.New().String()},
+				{ConfigID: configItemID, ChangeType: "TestDiff", CreatedAt: &twoDaysAgo, ExternalChangeId: uuid.New().String()},
+				{ConfigID: configItemID, ChangeType: "TestDiff", CreatedAt: &fiveDaysAgo, ExternalChangeId: uuid.New().String()},
+				{ConfigID: configItemID, ChangeType: "TestDiff", CreatedAt: &fiveDaysAgo, ExternalChangeId: uuid.New().String()},
+				{ConfigID: configItemID, ChangeType: "TestDiff", CreatedAt: &fiveDaysAgo, ExternalChangeId: uuid.New().String()},
+				{ConfigID: configItemID, ChangeType: "TestDiff", CreatedAt: &fiveDaysAgo, ExternalChangeId: uuid.New().String()},
+				{ConfigID: configItemID, ChangeType: "TestDiff", CreatedAt: &fiveDaysAgo, ExternalChangeId: uuid.New().String()},
+				{ConfigID: configItemID, ChangeType: "TestDiff", CreatedAt: &fiveDaysAgo, ExternalChangeId: uuid.New().String()},
+				{ConfigID: configItemID, ChangeType: "TestDiff", CreatedAt: &tenDaysAgo, ExternalChangeId: uuid.New().String()},
+				{ConfigID: configItemID, ChangeType: "TestDiff", CreatedAt: &tenDaysAgo, ExternalChangeId: uuid.New().String()},
 			}
-			err = db.DefaultDB().Create(&configChanges).Error
+
+			err = db.DefaultDB().Table("config_changes").Create(&configChanges).Error
 			Expect(err).To(BeNil())
 
 			var currentCount int
@@ -270,19 +273,46 @@ var _ = Describe("Scrapers test", Ordered, func() {
 				Scan(&currentCount).
 				Error
 			Expect(err).To(BeNil())
-			Expect(currentCount, len(configChanges))
+			Expect(currentCount).To(Equal(len(configChanges)))
 
 			ctx := context.NewContext(gocontext.Background()).WithDB(db.DefaultDB(), db.Pool)
-			err = ProcessChangeRetention(ctx, dummyScraper.ID, v1.ChangeRetentionSpec{Name: "TestDiff", Age: "3d", Count: 10})
+
+			// Everything older than 8 days should be removed
+			err = ProcessChangeRetention(ctx, dummyScraper.ID, v1.ChangeRetentionSpec{Name: "TestDiff", Age: "8d"})
+			Expect(err).To(BeNil())
+			var count1 int
+			err = db.DefaultDB().
+				Raw(`SELECT COUNT(*) FROM config_changes WHERE change_type = ? AND config_id = ?`, "TestDiff", configItemID).
+				Scan(&count1).
+				Error
 			Expect(err).To(BeNil())
+			Expect(count1).To(Equal(15))
 
-			var newCount int
+			// Only keep latest 12 config changes
+			err = ProcessChangeRetention(ctx, dummyScraper.ID, v1.ChangeRetentionSpec{Name: "TestDiff", Count: 12})
+			Expect(err).To(BeNil())
+			var count2 int
 			err = db.DefaultDB().
 				Raw(`SELECT COUNT(*) FROM config_changes WHERE change_type = ? AND config_id = ?`, "TestDiff", configItemID).
-				Scan(&newCount).
+				Scan(&count2).
 				Error
 			Expect(err).To(BeNil())
-			Expect(newCount, 8)
+			Expect(count2).To(Equal(12))
+
+			// Keep config changes which are newer than 3 days and max count can be 10
+			err = ProcessChangeRetention(ctx, dummyScraper.ID, v1.ChangeRetentionSpec{Name: "TestDiff", Age: "3d", Count: 10})
+			Expect(err).To(BeNil())
+			var count3 int
+			err = db.DefaultDB().
+				Raw(`SELECT COUNT(*) FROM config_changes WHERE change_type = ? AND config_id = ?`, "TestDiff", configItemID).
+				Scan(&count3).
+				Error
+			Expect(err).To(BeNil())
+			Expect(count3).To(Equal(9))
+
+			// No params in ChangeRetentionSpec should fail
+			err = ProcessChangeRetention(ctx, dummyScraper.ID, v1.ChangeRetentionSpec{Name: "TestDiff"})
+			Expect(err).ToNot(BeNil())
 		})
 	})
 })