From 2875ce7163df4a5feb290bee29eb78af6a5a01dc Mon Sep 17 00:00:00 2001 From: Michael Mraka Date: Fri, 5 Jan 2024 11:35:32 +0100 Subject: [PATCH] RHINENG-6806: refactored package evaluation --- evaluator/evaluate_packages.go | 286 +++++++++++++++------------------ 1 file changed, 133 insertions(+), 153 deletions(-) diff --git a/evaluator/evaluate_packages.go b/evaluator/evaluate_packages.go index 9eb8480f2..770f301cf 100644 --- a/evaluator/evaluate_packages.go +++ b/evaluator/evaluate_packages.go @@ -26,13 +26,13 @@ func analyzePackages(tx *gorm.DB, system *models.SystemPlatform, vmaasData *vmaa return 0, 0, errors.Wrap(err, "lazy package save failed") } - pkgByName, err := loadPackages(tx, system, vmaasData) + pkgByName, installed, updatable, err := loadPackages(tx, system, vmaasData) if err != nil { evaluationCnt.WithLabelValues("error-pkg-data").Inc() return 0, 0, errors.Wrap(err, "Unable to load package data") } - installed, updatable, err = updateSystemPackages(tx, system, pkgByName, vmaasData) + err = updateSystemPackages(tx, system, pkgByName) if err != nil { evaluationCnt.WithLabelValues("error-system-pkgs").Inc() return 0, 0, errors.Wrap(err, "Unable to update system packages") @@ -137,233 +137,213 @@ func updatePackageNameDB(missing *models.PackageName) error { // Find relevant package data based on vmaas results func loadPackages(tx *gorm.DB, system *models.SystemPlatform, - vmaasData *vmaas.UpdatesV3Response) (*map[string]namedPackage, error) { + vmaasData *vmaas.UpdatesV3Response) (map[string]namedPackage, int, int, error) { defer utils.ObserveSecondsSince(time.Now(), evaluationPartDuration.WithLabelValues("packages-load")) - packages, err := loadSystemNEVRAsFromDB(tx, system, vmaasData) + packages, installed, updatable := packagesFromUpdateList(system, vmaasData) + err := loadSystemNEVRAsFromDB(tx, system, packages) if err != nil { - return nil, errors.Wrap(err, "loading packages") + return nil, 0, 0, errors.Wrap(err, "loading packages") } - pkgByNevra := packages2NevraMap(packages) - return &pkgByNevra, nil + return packages, installed, updatable, nil } -func packages2NevraMap(packages []namedPackage) map[string]namedPackage { - pkgByNevra := make(map[string]namedPackage, len(packages)) - for _, p := range packages { - // make sure nevra contains epoch even if epoch==0 - nevraString := utils.NEVRAStringE(p.Name, p.EVRA, true) - pkgByNevra[nevraString] = p - } - return pkgByNevra -} - -func loadSystemNEVRAsFromDB(tx *gorm.DB, system *models.SystemPlatform, - vmaasData *vmaas.UpdatesV3Response) ([]namedPackage, error) { +func packagesFromUpdateList(system *models.SystemPlatform, vmaasData *vmaas.UpdatesV3Response) ( + map[string]namedPackage, int, int) { + installed := 0 + updatable := 0 updates := vmaasData.GetUpdateList() numUpdates := len(updates) - packageIDs := make([]int64, 0, numUpdates) - packages := make([]namedPackage, 0, numUpdates) - id2index := make(map[int64]int, numUpdates) - i := 0 - for nevra := range updates { + // allocate also space for removed packages + packages := make(map[string]namedPackage, numUpdates*2) + for nevra, pkgUpdate := range updates { + if !isValidNevra(nevra) { + continue + } + installed++ + availableUpdates := pkgUpdate.GetAvailableUpdates() + if len(availableUpdates) > 0 { + updatable++ + } pkgMeta, ok := memoryPackageCache.GetByNevra(nevra) + // before we used nevra.EVRAString() function which shows only non zero epoch, keep it consistent + // maybe we need here something like: evra := strings.TrimPrefix(upData.GetEVRA(), "0:") if ok { - packageIDs = append(packageIDs, pkgMeta.ID) - p := namedPackage{ - NameID: pkgMeta.NameID, - Name: pkgMeta.Name, - PackageID: pkgMeta.ID, - EVRA: pkgMeta.Evra, - WasStored: false, + installableID, applicableID := latestPackagesFromUpdatesList(availableUpdates) + packages[nevra] = namedPackage{ + NameID: pkgMeta.NameID, + Name: pkgMeta.Name, + PackageID: pkgMeta.ID, + EVRA: pkgMeta.Evra, + Change: Add, + InstallableID: installableID, + ApplicableID: applicableID, } - packages = append(packages, p) - id2index[pkgMeta.ID] = i - i++ } } - rows, err := tx.Table("system_package2"). - Select("package_id, installable_id, applicable_id"). + utils.LogInfo("inventoryID", system.InventoryID, "packages", numUpdates) + return packages, installed, updatable +} + +func loadSystemNEVRAsFromDB(tx *gorm.DB, system *models.SystemPlatform, packages map[string]namedPackage) error { + rows, err := tx.Table("system_package2 sp2"). + Select("sp2.name_id, pn.name, sp2.package_id, p.evra, sp2.installable_id, sp2.applicable_id"). + Joins("JOIN package p ON p.id = sp2.package_id"). + Joins("JOIN package_name pn on pn.id = sp2.name_id"). Where("rh_account_id = ? AND system_id = ?", system.RhAccountID, system.ID). - Where("package_id in (?)", packageIDs). Rows() if err != nil { - return nil, err + return err } var columns namedPackage + numStored := 0 + defer rows.Close() for rows.Next() { err = tx.ScanRows(rows, &columns) if err != nil { - return nil, err + return err + } + numStored++ + nevra := utils.NEVRAStringE(columns.Name, columns.EVRA, true) + if p, ok := packages[nevra]; ok { + if latestPkgsChanged(p, columns) { + p.Change = Update + } else { + p.Change = Keep + } + } else { + packages[nevra] = namedPackage{ + NameID: columns.NameID, + PackageID: columns.PackageID, + EVRA: columns.EVRA, + Change: Remove, + ApplicableID: columns.ApplicableID, + InstallableID: columns.InstallableID, + } } - index := id2index[columns.PackageID] - packages[index].WasStored = true - packages[index].InstallableID = columns.InstallableID - packages[index].ApplicableID = columns.ApplicableID } - utils.LogInfo("inventoryID", system.InventoryID, "packages", numUpdates, "already stored", len(packages)) - return packages, err + + utils.LogInfo("inventoryID", system.InventoryID, "already stored", numStored) + return err } -func isValidNevra(nevra string, packagesByNEVRA *map[string]namedPackage) bool { +func isValidNevra(nevra string) bool { // skip "phantom" package - if strings.HasPrefix(nevra, "gpg-pubkey") { - return false - } - - // Check whether we have that NEVRA in DB - currentNamedPackage := (*packagesByNEVRA)[nevra] - if currentNamedPackage.PackageID == 0 { - utils.LogTrace("nevra", nevra, "Unknown package") - return false - } - return true + return !strings.HasPrefix(nevra, "gpg-pubkey") } -func latestPkgsChanged(currentNamedPackage *namedPackage, installableID, applicableID int64) bool { - currentInstallableID, currentApplicableID := int64(0), int64(0) - if currentNamedPackage.InstallableID != nil { - currentInstallableID = *currentNamedPackage.InstallableID - } - if currentNamedPackage.ApplicableID != nil { - currentApplicableID = *currentNamedPackage.ApplicableID - } - - if installableID == currentInstallableID && applicableID == currentApplicableID { - // If the update_data we want to store is null, we skip only if there was a row for this specific - // system_package already stored. - if installableID == 0 && applicableID == 0 && currentNamedPackage.WasStored { - return false - } - - // If its not null, then the previous check ensured that the old update data matches new one - if installableID != 0 || applicableID != 0 { - return false - } - } - return true +func latestPkgsChanged(current, stored namedPackage) bool { + installableEqual := (current.InstallableID == nil && stored.InstallableID == nil) || + (current.InstallableID != nil && stored.InstallableID != nil && + current.InstallableID == stored.InstallableID) + applicableEqual := (current.ApplicableID == nil && stored.ApplicableID == nil) || + (current.ApplicableID != nil && stored.ApplicableID != nil && + current.ApplicableID == stored.ApplicableID) + return !(installableEqual && applicableEqual) } -func createSystemPackage(nevra string, - updateData *vmaas.UpdatesV3ResponseUpdateList, - system *models.SystemPlatform, - packagesByNEVRA *map[string]namedPackage) (systemPackagePtr *models.SystemPackage, updatesChanged bool) { - installableID, applicableID := latestPackagesFromVmaasResponse(updateData) - - // Skip overwriting entries which have the same data as before - currentNamedPackage := (*packagesByNEVRA)[nevra] - if !latestPkgsChanged(¤tNamedPackage, installableID, applicableID) { - return nil, false - } - +func createSystemPackage(system *models.SystemPlatform, pkg namedPackage) models.SystemPackage { systemPackage := models.SystemPackage{ - RhAccountID: system.RhAccountID, - SystemID: system.ID, - PackageID: currentNamedPackage.PackageID, - NameID: currentNamedPackage.NameID, - } - if installableID != 0 { - systemPackage.InstallableID = &installableID - } - if applicableID != 0 { - systemPackage.ApplicableID = &applicableID - } - return &systemPackage, true + RhAccountID: system.RhAccountID, + SystemID: system.ID, + PackageID: pkg.PackageID, + NameID: pkg.NameID, + InstallableID: pkg.InstallableID, + ApplicableID: pkg.ApplicableID, + } + return systemPackage } func updateSystemPackages(tx *gorm.DB, system *models.SystemPlatform, - packagesByNEVRA *map[string]namedPackage, - vmaasData *vmaas.UpdatesV3Response) (installed, updatable int, err error) { + packagesByNEVRA map[string]namedPackage) error { defer utils.ObserveSecondsSince(time.Now(), evaluationPartDuration.WithLabelValues("packages-store")) - updates := vmaasData.GetUpdateList() - if err := deleteOldSystemPackages(tx, system, packagesByNEVRA); err != nil { - return 0, 0, err - } - - toStore := make([]models.SystemPackage, 0, len(updates)) - for nevra, updateData := range updates { - isValid := isValidNevra(nevra, packagesByNEVRA) - if !isValid { - continue - } - installed++ - if len(updateData.GetAvailableUpdates()) > 0 { - updatable++ + nPkgs := len(packagesByNEVRA) + removedPkgIDs := make([]int64, 0, nPkgs) + updatedPackages := make([]models.SystemPackage, 0, nPkgs) + + for _, pkg := range packagesByNEVRA { + switch pkg.Change { + case Remove: + removedPkgIDs = append(removedPkgIDs, pkg.NameID) + case Add: + fallthrough + case Update: + systemPackage := createSystemPackage(system, pkg) + updatedPackages = append(updatedPackages, systemPackage) } + } - systemPackagePtr, updatesChanged := createSystemPackage(nevra, updateData, system, packagesByNEVRA) - if updatesChanged { - toStore = append(toStore, *systemPackagePtr) - } + if err := deleteOldSystemPackages(tx, system, removedPkgIDs); err != nil { + return err } - return installed, updatable, errors.Wrap( - database.UnnestInsert(tx, - "INSERT INTO system_package2 (rh_account_id, system_id, package_id, name_id, installable_id, applicable_id)"+ - " (select * from unnest($1::int[], $2::bigint[], $3::bigint[], $4::bigint[], $5::bigint[], $6::bigint[]))"+ - " ON CONFLICT (rh_account_id, system_id, package_id)"+ - " DO UPDATE SET installable_id = EXCLUDED.installable_id, applicable_id = EXCLUDED.applicable_id", toStore), + + err := database.UnnestInsert(tx, + `INSERT INTO system_package2 (rh_account_id, system_id, package_id, name_id, installable_id, applicable_id) + (select * from unnest($1::int[], $2::bigint[], $3::bigint[], $4::bigint[], $5::bigint[], $6::bigint[])) + ON CONFLICT (rh_account_id, system_id, package_id) + DO UPDATE SET installable_id = EXCLUDED.installable_id, applicable_id = EXCLUDED.applicable_id`, updatedPackages) + return errors.Wrap(err, "Storing system packages") } -func latestPackagesFromVmaasResponse(updateData *vmaas.UpdatesV3ResponseUpdateList) (int64, int64) { +func latestPackagesFromUpdatesList(updatePkgData []vmaas.UpdatesV3ResponseAvailableUpdates) (*int64, *int64) { var ( latestInstallable, latestApplicable string - installableID, applicableID int64 + installableID, applicableID *int64 ) - uniqUpdates := make(map[string]bool) - for _, upData := range updateData.GetAvailableUpdates() { + for _, upData := range updatePkgData { nevra := upData.GetPackage() if len(nevra) == 0 { // no update continue } - // Keep only unique entries for each update in the list - if !uniqUpdates[nevra] { - uniqUpdates[nevra] = true - switch upData.StatusID { - case INSTALLABLE: - latestInstallable = nevra - case APPLICABLE: - latestApplicable = nevra - } + switch upData.StatusID { + case INSTALLABLE: + latestInstallable = nevra + case APPLICABLE: + latestApplicable = nevra } } if len(latestInstallable) > 0 { if installableFromCache, ok := memoryPackageCache.GetByNevra(latestInstallable); ok { - installableID = installableFromCache.ID + installableID = &installableFromCache.ID } } if len(latestApplicable) > 0 { if applicableFromCache, ok := memoryPackageCache.GetByNevra(latestApplicable); ok { - applicableID = applicableFromCache.ID + applicableID = &applicableFromCache.ID } } return installableID, applicableID } -func deleteOldSystemPackages(tx *gorm.DB, system *models.SystemPlatform, - packagesByNEVRA *map[string]namedPackage) error { - pkgIds := make([]int64, 0, len(*packagesByNEVRA)) - for _, pkg := range *packagesByNEVRA { - pkgIds = append(pkgIds, pkg.PackageID) - } - +func deleteOldSystemPackages(tx *gorm.DB, system *models.SystemPlatform, pkgIds []int64) error { err := tx.Where("rh_account_id = ? ", system.RhAccountID). Where("system_id = ?", system.ID). - Where("package_id not in (?)", pkgIds). + Where("package_id in (?)", pkgIds). Delete(&models.SystemPackage{}).Error return errors.Wrap(err, "Deleting outdated system packages") } +type ChangeType int8 + +const ( + None ChangeType = iota + Add + Keep + Update + Remove +) + type namedPackage struct { NameID int64 Name string PackageID int64 EVRA string - WasStored bool InstallableID *int64 ApplicableID *int64 + Change ChangeType }