Skip to content
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

[M] Added grace period for product deletion during refresh/import (ENT-4881) #3341

Merged
merged 1 commit into from Jun 2, 2022

Conversation

Ceiu
Copy link
Contributor

@Ceiu Ceiu commented May 18, 2022

  • Products orphaned during refresh or manifest import can now
    have a grace period before they will be deleted
  • Added the config "candlepin.refresh.orphaned_entity_grace_period"
    to control the duration of the grace period in days, or disable
    it entirely. By default this option is set to 30 days.

@nikosmoum nikosmoum requested review from a team and nikosmoum and removed request for a team May 18, 2022 19:29
@Ceiu
Copy link
Contributor Author

Ceiu commented May 18, 2022

For this implementation, I opted to keep the configuration aspect as the CandlepinPoolManager level, but I could see an argument to moving the config parsing lower. There's no real concrete reasoning behind the decision to how it landed, so if this doesn't jive with others or someone feels strongly about doing it a different way, I'm open to suggestions.

@Ceiu
Copy link
Contributor Author

Ceiu commented May 19, 2022

I'm realizing there's an optimization/cleanup that could be done with the product date fetching that could also be upgraded to use a per-product lookup as a fallback in the current state mismatch case that I might do next week.

Edit: nm, did it today

@Ceiu Ceiu force-pushed the crog/m/product_deletion_grace_period branch from 1f65e39 to 0e6aac8 Compare May 19, 2022 19:57
@nikosmoum nikosmoum self-assigned this May 31, 2022
Copy link
Member

@nikosmoum nikosmoum left a comment

Choose a reason for hiding this comment

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

The only question I have is if it is possible to write a spec test for this? Otherwise it looks good

@Ceiu
Copy link
Contributor Author

Ceiu commented Jun 1, 2022

@nikosmoum
I'm not sure what a spec test would look like here. At that level, we don't have control over the time for the various objects to setup exactly what we're looking to test, and at a time resolution of days, we don't have the liberty of waiting a second or two, either. I suppose we could do DB manip, but that's kinda hack. My intent is that the multiple layers of unit tests around both the visitor which does the cleanup and the curator's fetching/setting would cover us.

I've updated one unit test I saw that was lacking a check, but was there anything specific you wanted to see tested or an idea you had for the spec tests?

@nikosmoum
Copy link
Member

I just think it's good practice to have spec test for any feature/behavior, but you are right that there isn't a way to do this now except DB manipulation, so nevermind

@Ceiu Ceiu force-pushed the crog/m/product_deletion_grace_period branch from 0e6aac8 to 7906d94 Compare June 1, 2022 14:48
- Products orphaned during refresh or manifest import can now
  have a grace period before they will be deleted
- Added the config "candlepin.refresh.orphaned_entity_grace_period"
  to control the duration of the grace period in days, or disable
  it entirely. By default this option is set to 30 days.
@Ceiu Ceiu force-pushed the crog/m/product_deletion_grace_period branch from 7906d94 to fd88cab Compare June 1, 2022 16:50
@Ceiu
Copy link
Contributor Author

Ceiu commented Jun 1, 2022

Empty test filled in, a missing test was added, other tests updated accordingly.

@nikosmoum nikosmoum merged commit 0156358 into master Jun 2, 2022
1 check passed
@nikosmoum nikosmoum deleted the crog/m/product_deletion_grace_period branch June 2, 2022 10:12
wbclark added a commit to wbclark/katello that referenced this pull request Jul 11, 2022
…ociation

Candlepin may delete products if entitlements are removed from the
manifest (c.f. candlepin/candlepin#3341 ). This
leads to an issue in Katello once the entitlements are restored to the
manifest; contents get recreated in Candlepin but Katello needs to tell it
once again which environments should have access to which contents.
wbclark added a commit to wbclark/katello that referenced this pull request Jul 11, 2022
…ociation

Candlepin may delete products if entitlements are removed from the
manifest (c.f. candlepin/candlepin#3341 ). This
leads to an issue in Katello once the entitlements are restored to the
manifest; contents get recreated in Candlepin but Katello needs to tell it
once again which environments should have access to which contents.
wbclark added a commit to wbclark/katello that referenced this pull request Jul 13, 2022
…ociation

Candlepin may delete products if entitlements are removed from the
manifest (c.f. candlepin/candlepin#3341 ). This
leads to an issue in Katello once the entitlements are restored to the
manifest; contents get recreated in Candlepin but Katello needs to tell it
once again which environments should have access to which contents.
wbclark added a commit to wbclark/katello that referenced this pull request Jul 13, 2022
…ociation

Candlepin may delete products if entitlements are removed from the
manifest (c.f. candlepin/candlepin#3341 ). This
leads to an issue in Katello once the entitlements are restored to the
manifest; contents get recreated in Candlepin but Katello needs to tell it
once again which environments should have access to which contents.
wbclark added a commit to wbclark/katello that referenced this pull request Jul 14, 2022
…ociation

Candlepin may delete products if entitlements are removed from the
manifest (c.f. candlepin/candlepin#3341 ). This
leads to an issue in Katello once the entitlements are restored to the
manifest; contents get recreated in Candlepin but Katello needs to tell it
once again which environments should have access to which contents.
parthaa pushed a commit to Katello/katello that referenced this pull request Jul 16, 2022
…ociation (#10192)

Candlepin may delete products if entitlements are removed from the
manifest (c.f. candlepin/candlepin#3341 ). This
leads to an issue in Katello once the entitlements are restored to the
manifest; contents get recreated in Candlepin but Katello needs to tell it
once again which environments should have access to which contents.
ianballou pushed a commit to ianballou/katello that referenced this pull request Sep 27, 2022
…ociation (Katello#10192)

Candlepin may delete products if entitlements are removed from the
manifest (c.f. candlepin/candlepin#3341 ). This
leads to an issue in Katello once the entitlements are restored to the
manifest; contents get recreated in Candlepin but Katello needs to tell it
once again which environments should have access to which contents.

(cherry picked from commit e6a94f4)
ianballou pushed a commit to Katello/katello that referenced this pull request Oct 3, 2022
…ociation (#10192)

Candlepin may delete products if entitlements are removed from the
manifest (c.f. candlepin/candlepin#3341 ). This
leads to an issue in Katello once the entitlements are restored to the
manifest; contents get recreated in Candlepin but Katello needs to tell it
once again which environments should have access to which contents.

(cherry picked from commit e6a94f4)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants