Replace delete pool events with a recurring job

[jturel]

What are the changes introduced in this pull request?

This PR proposes no longer handling delete pool events from Candlepin by favoring a recurring daily job instead

Considerations taken when implementing this change?

• Deliberately take a simplistic approach to deletion by only removing expired pools as recorded in the Katello DB vs a full reconciliation by comparing Katello pools to Candlepin pools. I don't think the latter is necessary..

• This PR doesn't account for manifest refresh, removal, etc. Manifest actions appear to result in a full reconciliation of Katello Pool objects against Candlepin, so any pools removed from CP would be removed from Katello at this time
  

  katello/app/models/katello/glue/provider.rb

  Lines 151 to 154 in 0ffbbef

  	def index_subscriptions
  	Katello::Subscription.import_all(organization)
  	Katello::Pool.import_all(organization)
  	end

What are the testing steps for this pull request?

• Create an expired pool ex Katello::Pool.create!(organization_id: 1, subscription_id: 1, cp_id: '12345', end_date: 1.year.ago)
• Run the job (rails console is fine) and check that the pool is deleted

Summary by Sourcery

Replace Candlepin pool deletion event handling with a scheduled job that removes expired pools from the Katello database.

New Features:

• Introduce a recurring DestroyExpired action that deletes expired Katello pools and records removed pool IDs.

Enhancements:

• Add an expired scope on Katello::Pool to select pools whose end date is in the past.
• Schedule the DestroyExpired pool cleanup action to run daily via the Katello engine scheduled tasks.

Tests:

• Add a Dynflow action test ensuring DestroyExpired deletes expired pools and exposes their IDs in the action output.

Chores:

• Remove DeletePool event type, associated event handling, and tests, including Candlepin pool.deleted message handling and queue registration.

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• The Katello::Pool.expired scope uses Date.new...DateTime.now, which mixes Date and DateTime and hardcodes a start bound; consider simplifying to a one-sided comparison with a time-zone-aware value (e.g. where('end_date < ?', Time.zone.now) or where(end_date: ..Time.zone.now)), which also avoids the odd Date.new default.
• DestroyExpired calls ::Katello::Pool.expired.destroy_all in one shot, which could become expensive if many pools are expired; consider deleting in batches (e.g. find_each or limited scopes) or adding some safety bounds to avoid long-running transactions.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The `Katello::Pool.expired` scope uses `Date.new...DateTime.now`, which mixes `Date` and `DateTime` and hardcodes a start bound; consider simplifying to a one-sided comparison with a time-zone-aware value (e.g. `where('end_date < ?', Time.zone.now)` or `where(end_date: ..Time.zone.now)`), which also avoids the odd `Date.new` default.
- `DestroyExpired` calls `::Katello::Pool.expired.destroy_all` in one shot, which could become expensive if many pools are expired; consider deleting in batches (e.g. `find_each` or limited scopes) or adding some safety bounds to avoid long-running transactions.

## Individual Comments

### Comment 1
<location> `app/models/katello/pool.rb:17-21` </location>
<code_context>
     end
     scope :upstream, -> { where.not(upstream_pool_id: nil) }
     scope :redhat, -> { joins(:products).merge(Katello::Product.redhat).distinct }
+    scope :expired, -> { where(end_date: Date.new...DateTime.now) }

     include Glue::Candlepin::CandlepinObject
</code_context>

<issue_to_address>
**suggestion (bug_risk):** Clarify the date range for `expired` scope and consider time zone / range semantics

`Date.new` as the lower bound is a bit surprising and mixes `Date` with `DateTime`. If the intent is just `end_date < now`, using `where('end_date < ?', Time.zone.now)` (or a range with an explicit lower bound) avoids implicit type casting. Also, the exclusive `...` range means records with `end_date == DateTime.now` are treated as non-expired; if that’s not intended, consider `..` or a simple `<` comparison instead.

```suggestion
    scope :upstream, -> { where.not(upstream_pool_id: nil) }
    scope :redhat, -> { joins(:products).merge(Katello::Product.redhat).distinct }
    scope :expired, -> { where('end_date < ?', Time.zone.now) }

    include Glue::Candlepin::CandlepinObject
```
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

suggestion (bug_risk): Clarify the date range for expired scope and consider time zone / range semantics

Date.new as the lower bound is a bit surprising and mixes Date with DateTime. If the intent is just end_date < now, using where('end_date < ?', Time.zone.now) (or a range with an explicit lower bound) avoids implicit type casting. Also, the exclusive ... range means records with end_date == DateTime.now are treated as non-expired; if that’s not intended, consider .. or a simple < comparison instead.

Suggested change

	scope :upstream, -> { where.not(upstream_pool_id: nil) }
	scope :redhat, -> { joins(:products).merge(Katello::Product.redhat).distinct }
	scope :expired, -> { where(end_date: Date.new...DateTime.now) }
	include Glue::Candlepin::CandlepinObject
	scope :upstream, -> { where.not(upstream_pool_id: nil) }
	scope :redhat, -> { joins(:products).merge(Katello::Product.redhat).distinct }
	scope :expired, -> { where('end_date < ?', Time.zone.now) }
	include Glue::Candlepin::CandlepinObject

[jturel]

This is something I added way back when fixing a bug around subscriptions being assigned to incorrect organizations. It wasn't related to that change and no one asked for it, so I thought it was a good time to remove it in favor of the daily cleanup job.