From 17833043c535b6c3a4c3c7bbf9f5483a1005fb74 Mon Sep 17 00:00:00 2001 From: Yash Mehrotra Date: Fri, 22 Sep 2023 08:09:43 +0530 Subject: [PATCH 1/2] fix: mark evicted pods as deleted --- db/config.go | 4 ++-- db/update.go | 11 +++++++++-- scrapers/kubernetes/kubernetes.go | 19 +++++++++++++++++++ 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/db/config.go b/db/config.go index 7b5c2044..0811f005 100644 --- a/db/config.go +++ b/db/config.go @@ -72,14 +72,14 @@ func CreateConfigItem(ci *models.ConfigItem) error { } // UpdateConfigItem updates all the fields of a given config item row -func UpdateConfigItem(ci *models.ConfigItem) error { +func UpdateConfigItem(ci *models.ConfigItem, updateDeletedAt bool) error { if err := db.Updates(ci).Error; err != nil { return err } // Since gorm ignores nil fields, we are setting deleted_at explicitly - if ci.DeletedAt != nil { + if updateDeletedAt { if err := db.Table("config_items").Where("id = ?", ci.ID).UpdateColumn("deleted_at", nil).Error; err != nil { return err } diff --git a/db/update.go b/db/update.go index 034f2668..6f2cb683 100644 --- a/db/update.go +++ b/db/update.go @@ -102,8 +102,15 @@ func updateCI(ctx *v1.ScrapeContext, ci models.ConfigItem) error { } ci.ID = existing.ID - ci.DeletedAt = existing.DeletedAt - if err := UpdateConfigItem(&ci); err != nil { + + // In case a resource was marked as deleted but is un-deleted now + // we set an update flag as gorm ignores nil pointers + updateDeletedAt := false + if ci.DeletedAt != existing.DeletedAt { + updateDeletedAt = true + } + + if err := UpdateConfigItem(&ci, updateDeletedAt); err != nil { if err := CreateConfigItem(&ci); err != nil { return fmt.Errorf("[%s] failed to update item %v", ci, err) } diff --git a/scrapers/kubernetes/kubernetes.go b/scrapers/kubernetes/kubernetes.go index 666de3e2..4daf55f0 100644 --- a/scrapers/kubernetes/kubernetes.go +++ b/scrapers/kubernetes/kubernetes.go @@ -4,6 +4,7 @@ import ( "fmt" "strconv" "strings" + "time" "github.com/Jeffail/gabs/v2" "github.com/flanksource/commons/collections" @@ -124,6 +125,23 @@ func (kubernetes KubernetesScraper) Scrape(ctx *v1.ScrapeContext) v1.ScrapeResul } createdAt := obj.GetCreationTimestamp().Time + var deletedAt *time.Time + if !obj.GetDeletionTimestamp().IsZero() { + deletedAt = &obj.GetDeletionTimestamp().Time + } + + // Evicted Pods must be considered deleted + if obj.GetKind() == "Pod" && status == string(health.HealthStatusDegraded) { + objStatus := obj.Object["status"].(map[string]any) + if val, ok := objStatus["reason"].(string); ok && val == "Evicted" { + // Use createdAt as default and try to parse the evict time + deletedAt = &createdAt + if evictTime, err := time.Parse(time.RFC3339, objStatus["startTime"].(string)); err != nil { + deletedAt = &evictTime + } + } + } + parentType, parentExternalID := getKubernetesParent(obj, resourceIDMap) results = append(results, v1.ScrapeResult{ BaseScraper: config.BaseScraper, @@ -134,6 +152,7 @@ func (kubernetes KubernetesScraper) Scrape(ctx *v1.ScrapeContext) v1.ScrapeResul Status: status, Description: description, CreatedAt: &createdAt, + DeletedAt: deletedAt, Config: cleanKubernetesObject(obj.Object), ID: string(obj.GetUID()), Tags: stripLabels(convertStringInterfaceMapToStringMap(tags), "-hash"), From efce409f72f02cb89deab30d400e3fb909fb99b6 Mon Sep 17 00:00:00 2001 From: Yash Mehrotra Date: Fri, 22 Sep 2023 12:54:35 +0530 Subject: [PATCH 2/2] chore: use faster query for config analysis --- db/config.go | 7 +++-- db/models/config_item.go | 45 ++++++++++++++++--------------- db/update.go | 17 +++++++----- scrapers/kubernetes/kubernetes.go | 5 ++-- 4 files changed, 40 insertions(+), 34 deletions(-) diff --git a/db/config.go b/db/config.go index 0811f005..29ad9e99 100644 --- a/db/config.go +++ b/db/config.go @@ -19,7 +19,7 @@ import ( // GetConfigItem returns a single config item result func GetConfigItem(extType, extID string) (*models.ConfigItem, error) { ci := models.ConfigItem{} - tx := db.Limit(1).Find(&ci, "type = ? and external_id @> ?", extType, pq.StringArray{extID}) + tx := db.Limit(1).Select("id", "config_class", "type", "config").Find(&ci, "type = ? and external_id @> ?", extType, pq.StringArray{extID}) if tx.RowsAffected == 0 { return nil, nil } @@ -72,14 +72,13 @@ func CreateConfigItem(ci *models.ConfigItem) error { } // UpdateConfigItem updates all the fields of a given config item row -func UpdateConfigItem(ci *models.ConfigItem, updateDeletedAt bool) error { - +func UpdateConfigItem(ci *models.ConfigItem) error { if err := db.Updates(ci).Error; err != nil { return err } // Since gorm ignores nil fields, we are setting deleted_at explicitly - if updateDeletedAt { + if ci.TouchDeletedAt { if err := db.Table("config_items").Where("id = ?", ci.ID).UpdateColumn("deleted_at", nil).Error; err != nil { return err } diff --git a/db/models/config_item.go b/db/models/config_item.go index 8d5a7dc6..d9108727 100644 --- a/db/models/config_item.go +++ b/db/models/config_item.go @@ -12,28 +12,29 @@ import ( // ConfigItem represents the config item database table type ConfigItem struct { - ID string `gorm:"primaryKey;unique_index;not null;column:id" json:"id" ` - ScraperID *uuid.UUID `gorm:"column:scraper_id;default:null" json:"scraper_id,omitempty"` - ConfigClass string `gorm:"column:config_class;default:''" json:"config_class" ` - ExternalID pq.StringArray `gorm:"column:external_id;type:[]text" json:"external_id,omitempty" ` - Type *string `gorm:"column:type;default:null" json:"type,omitempty" ` - Status *string `gorm:"column:status;default:null" json:"status,omitempty" ` - Name *string `gorm:"column:name;default:null" json:"name,omitempty" ` - Namespace *string `gorm:"column:namespace;default:null" json:"namespace,omitempty" ` - Description *string `gorm:"column:description;default:null" json:"description,omitempty" ` - Account *string `gorm:"column:account;default:null" json:"account,omitempty" ` - Config *string `gorm:"column:config;default:null" json:"config,omitempty" ` - Source *string `gorm:"column:source;default:null" json:"source,omitempty" ` - ParentID *string `gorm:"column:parent_id;default:null" json:"parent_id,omitempty"` - Path string `gorm:"column:path;default:null" json:"path,omitempty"` - CostPerMinute float64 `gorm:"column:cost_per_minute;default:null" json:"cost_per_minute,omitempty"` - CostTotal1d float64 `gorm:"column:cost_total_1d;default:null" json:"cost_total_1d,omitempty"` - CostTotal7d float64 `gorm:"column:cost_total_7d;default:null" json:"cost_total_7d,omitempty"` - CostTotal30d float64 `gorm:"column:cost_total_30d;default:null" json:"cost_total_30d,omitempty"` - Tags *v1.JSONStringMap `gorm:"column:tags;default:null" json:"tags,omitempty" ` - CreatedAt time.Time `gorm:"column:created_at" json:"created_at"` - UpdatedAt time.Time `gorm:"column:updated_at" json:"updated_at"` - DeletedAt *time.Time `gorm:"column:deleted_at" json:"deleted_at"` + ID string `gorm:"primaryKey;unique_index;not null;column:id" json:"id" ` + ScraperID *uuid.UUID `gorm:"column:scraper_id;default:null" json:"scraper_id,omitempty"` + ConfigClass string `gorm:"column:config_class;default:''" json:"config_class" ` + ExternalID pq.StringArray `gorm:"column:external_id;type:[]text" json:"external_id,omitempty" ` + Type *string `gorm:"column:type;default:null" json:"type,omitempty" ` + Status *string `gorm:"column:status;default:null" json:"status,omitempty" ` + Name *string `gorm:"column:name;default:null" json:"name,omitempty" ` + Namespace *string `gorm:"column:namespace;default:null" json:"namespace,omitempty" ` + Description *string `gorm:"column:description;default:null" json:"description,omitempty" ` + Account *string `gorm:"column:account;default:null" json:"account,omitempty" ` + Config *string `gorm:"column:config;default:null" json:"config,omitempty" ` + Source *string `gorm:"column:source;default:null" json:"source,omitempty" ` + ParentID *string `gorm:"column:parent_id;default:null" json:"parent_id,omitempty"` + Path string `gorm:"column:path;default:null" json:"path,omitempty"` + CostPerMinute float64 `gorm:"column:cost_per_minute;default:null" json:"cost_per_minute,omitempty"` + CostTotal1d float64 `gorm:"column:cost_total_1d;default:null" json:"cost_total_1d,omitempty"` + CostTotal7d float64 `gorm:"column:cost_total_7d;default:null" json:"cost_total_7d,omitempty"` + CostTotal30d float64 `gorm:"column:cost_total_30d;default:null" json:"cost_total_30d,omitempty"` + Tags *v1.JSONStringMap `gorm:"column:tags;default:null" json:"tags,omitempty" ` + CreatedAt time.Time `gorm:"column:created_at" json:"created_at"` + UpdatedAt time.Time `gorm:"column:updated_at" json:"updated_at"` + DeletedAt *time.Time `gorm:"column:deleted_at" json:"deleted_at"` + TouchDeletedAt bool `gorm:"-" json:"-"` } func (ci ConfigItem) String() string { diff --git a/db/update.go b/db/update.go index 6f2cb683..7474aca5 100644 --- a/db/update.go +++ b/db/update.go @@ -105,12 +105,11 @@ func updateCI(ctx *v1.ScrapeContext, ci models.ConfigItem) error { // In case a resource was marked as deleted but is un-deleted now // we set an update flag as gorm ignores nil pointers - updateDeletedAt := false if ci.DeletedAt != existing.DeletedAt { - updateDeletedAt = true + ci.TouchDeletedAt = true } - if err := UpdateConfigItem(&ci, updateDeletedAt); err != nil { + if err := UpdateConfigItem(&ci); err != nil { if err := CreateConfigItem(&ci); err != nil { return fmt.Errorf("[%s] failed to update item %v", ci, err) } @@ -184,8 +183,11 @@ func updateChange(ctx *v1.ScrapeContext, result *v1.ScrapeResult) error { func upsertAnalysis(ctx *v1.ScrapeContext, result *v1.ScrapeResult) error { analysis := result.AnalysisResult.ToConfigAnalysis() - ci, err := GetConfigItem(analysis.ConfigType, analysis.ExternalID) - if ci == nil { + ciID, err := FindConfigItemID(v1.ExternalID{ + ConfigType: analysis.ConfigType, + ExternalID: []string{analysis.ExternalID}, + }) + if ciID == nil { logger.Warnf("[Source=%s] [%s/%s] unable to find config item for analysis: %+v", analysis.Source, analysis.ConfigType, analysis.ExternalID, analysis) return nil } else if err != nil { @@ -193,7 +195,10 @@ func upsertAnalysis(ctx *v1.ScrapeContext, result *v1.ScrapeResult) error { } logger.Tracef("[%s/%s] ==> %s", analysis.ConfigType, analysis.ExternalID, analysis) - analysis.ConfigID = uuid.MustParse(ci.ID) + analysis.ConfigID, err = uuid.Parse(*ciID) + if err != nil { + return err + } analysis.ID = uuid.MustParse(ulid.MustNew().AsUUID()) analysis.ScraperID = ctx.ScrapeConfig.GetPersistedID() if analysis.Status == "" { diff --git a/scrapers/kubernetes/kubernetes.go b/scrapers/kubernetes/kubernetes.go index 4daf55f0..c44bda68 100644 --- a/scrapers/kubernetes/kubernetes.go +++ b/scrapers/kubernetes/kubernetes.go @@ -134,8 +134,9 @@ func (kubernetes KubernetesScraper) Scrape(ctx *v1.ScrapeContext) v1.ScrapeResul if obj.GetKind() == "Pod" && status == string(health.HealthStatusDegraded) { objStatus := obj.Object["status"].(map[string]any) if val, ok := objStatus["reason"].(string); ok && val == "Evicted" { - // Use createdAt as default and try to parse the evict time - deletedAt = &createdAt + // Use time.Now() as default and try to parse the evict time + timeNow := time.Now() + deletedAt = &timeNow if evictTime, err := time.Parse(time.RFC3339, objStatus["startTime"].(string)); err != nil { deletedAt = &evictTime }