From 931963119e200bc2d24d2412f2707dbafc9c1fc4 Mon Sep 17 00:00:00 2001 From: Yash Mehrotra Date: Mon, 4 Dec 2023 13:06:03 +0530 Subject: [PATCH] fix: config retention tests and flow --- scrapers/retention.go | 47 ++++++++++++++++------ scrapers/runscrapers_test.go | 78 +++++++++++++++++++++++++----------- 2 files changed, 88 insertions(+), 37 deletions(-) diff --git a/scrapers/retention.go b/scrapers/retention.go index 9823f903..6aa57d07 100644 --- 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 index 230978ad..af28abf6 100644 --- 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(¤tCount). 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()) }) }) })