Skip to content

HMS-5913: add package dates from RHEL lifecycle #1106

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rverdile
Copy link
Contributor

@rverdile rverdile commented May 7, 2025

Summary

Adds package streams info to package_sources. Pulls package stream info from the roadmap API and adds it to package sources if the RPM name matches.

Also adds RHEL EoL as the end date for any package source that is not in the roadmap API. Uses the end date for the latest released version of the matching RHEL major version.

Testing steps

To test package streams:

  1. Search for "nodejs" in a rhel9 repo
  2. Under package sources, you should see both the node 16 module and the 16.14.0 package stream

To test the rhel lifecycle info:

  1. When a RHEL package doesn't exist in the roadmap, the RHEL end date is used instead
  2. Search with a codeready repo. For example, "help2man" in https://cdn.redhat.com/content/dist/rhel9/9/aarch64/codeready-builder/os/

@jlsherrill
Copy link
Member

@rverdile rverdile force-pushed the pkg-lifecycle-info branch 2 times, most recently from 5ffcf35 to afc5730 Compare May 8, 2025 20:02
@rverdile rverdile marked this pull request as ready for review May 8, 2025 20:05
@rverdile rverdile force-pushed the pkg-lifecycle-info branch from afc5730 to 7133cb6 Compare May 9, 2025 20:42
Adds package streams info to package_sources. Pulls package stream info
from the roadmap API and adds it to package sources if the RPM name
matches.

Also adds RHEL EoL as the end date for any package source that is not in
the roadmap API. Uses the end date for the latest released version of
the matching RHEL major version.
@rverdile rverdile force-pushed the pkg-lifecycle-info branch from 7133cb6 to c49cc13 Compare May 9, 2025 20:47
@jlsherrill jlsherrill requested a review from Copilot May 12, 2025 17:31
@jlsherrill jlsherrill self-assigned this May 12, 2025
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds package stream and RHEL lifecycle information to package sources by integrating with the roadmap API. Key changes include:

  • Retrieving and appending package stream details for RPMs via new roadmap API calls.
  • Adding fallback logic to include RHEL lifecycle end dates if package stream data is not available.
  • Updating tests, mocks, and cache interfaces to accommodate the new lifecycle functionality.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
pkg/dao/rpms_test.go Added test cases for RHEL repo configurations and lifecycle info.
pkg/dao/rpms.go Updated search logic to add package sources and lifecycle info.
pkg/clients/roadmap_client/roadmap_client_mock.go Added mocks for lifecycle API calls.
pkg/clients/roadmap_client/roadmap.go Introduced lifecycle response types and functions.
pkg/clients/roadmap_client/client.go Updated client interface for new lifecycle methods.
pkg/cache/redis.go Added cache functions for RHEL lifecycle data.
pkg/cache/noop.go Added no-op implementations for RHEL lifecycle caching.
pkg/cache/cache_mock.go Added mocks for the new RHEL lifecycle cache functions.
pkg/cache/cache.go Extended cache interface to include lifecycle methods.

}

packageMap := make(map[string]int)
for _, pkg := range res {
Copy link
Preview

Copilot AI May 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accessing pkg.Versions[0] without checking if the Versions slice is non-empty might lead to a runtime panic. Please add a check to ensure the slice is not empty before attempting conversion.

Suggested change
for _, pkg := range res {
for _, pkg := range res {
if len(pkg.Versions) == 0 {
continue // Skip if Versions slice is empty
}

Copilot uses AI. Check for mistakes.

@@ -64,3 +64,12 @@ func (c *noOpCache) GetRoadmapAppstreams(ctx context.Context) ([]byte, error) {
// SetRoadmapAppstreams a NoOp version to store cached roadmap appstreams check
func (c *noOpCache) SetRoadmapAppstreams(ctx context.Context, response []byte) {
}

// GetRoadmapAppstreams a NoOp version to fetch a cached roadmap rhel lifecycle check
Copy link
Preview

Copilot AI May 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The comment for GetRoadmapRhelLifecycle incorrectly refers to 'Appstreams' instead of 'RhelLifecycle'. Please update the comment to accurately describe its purpose.

Suggested change
// GetRoadmapAppstreams a NoOp version to fetch a cached roadmap rhel lifecycle check
// GetRoadmapRhelLifecycle a NoOp version to fetch a cached roadmap rhel lifecycle check

Copilot uses AI. Check for mistakes.

return nil, NotFound
}

// SetRoadmapAppstreams a NoOp version to store cached roadmap rhel lifecycle check
Copy link
Preview

Copilot AI May 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The comment for SetRoadmapRhelLifecycle seems to be a copy of the Appstreams comment; please update it to correctly refer to storing the RHEL lifecycle information.

Suggested change
// SetRoadmapAppstreams a NoOp version to store cached roadmap rhel lifecycle check
// SetRoadmapRhelLifecycle a NoOp version to store cached roadmap RHEL lifecycle information

Copilot uses AI. Check for mistakes.

Comment on lines +199 to +200
db := r.db.Debug().WithContext(ctx).
Select("DISTINCT ON(rpms.name) rpms.name as package_name", "rpms.summary", "rpms.uuid").
Copy link
Preview

Copilot AI May 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The use of db.Debug() may have been left unintentionally and can clutter production logs. Consider removing it or gating the debug logging for production use.

Suggested change
db := r.db.Debug().WithContext(ctx).
Select("DISTINCT ON(rpms.name) rpms.name as package_name", "rpms.summary", "rpms.uuid").
db := r.db.WithContext(ctx)
if config.Get().Debug { // Enable debug logging if the debug flag is set
db = db.Debug()
}
db = db.Select("DISTINCT ON(rpms.name) rpms.name as package_name", "rpms.summary", "rpms.uuid").

Copilot uses AI. Check for mistakes.

if startDate.Before(currentDate) || startDate.Equal(currentDate) {
if existing, found := rhelEolMap[item.Major]; !found || (item.Minor > existing.Minor) {
rhelEolMap[item.Major] = item
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

chatted privately, but we actually want to pick the earliest start date and latest end date for a given RHEL major release (9)


func (c *redisCache) SetRoadmapRhelLifecycle(ctx context.Context, response []byte) {
key := roadmapRhelLifecycleKey(ctx)
c.client.Set(ctx, key, string(response), config.Get().Clients.Redis.Expiration.SubscriptionCheck)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it probably makes sense to set this per service (for example subscription check should probably once an hour, but the roadmap expiration could be once per day). thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought subscription check is once a day, but I agree it should probably be separate. I'll fix that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants