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

Fixes #36914 - skip missing hosts during applicability generate #10796

Merged

Conversation

ianballou
Copy link
Member

What are the changes introduced in this pull request?

Adds error handling to the block in BulkGenerate that calculates applicability for all hosts.

The issue here is that we cannot use resource locking because it's a list of hosts. So, we can't stop hosts from being deleted during applicability calculation.

We thought we handled this before with am existence check, but turns out some folks still hit the issue because the host is deleted part way through calculate_and_import_applicability.

NoMethodError and all PG errors are handled here to cover our bases but not be too broad. Plus the errors are raised back to the logs so debugging shouldn't be too much harder.

Also makes bulk generate skip itself on an error so the task doesn't get paused.

Considerations taken when implementing this change?

I don't see any reason why applicability generation needs to result in a paused task. I don't see it causing a data integrity issue.

What are the testing steps for this pull request?

  1. Edit the code to error out somewhere in calculate_and_import_applicability
  2. Trigger applicability generation for a host, ideally the "natural" way via subscription-manager
  3. See that the error is handled and that it pops up in the logs.

@ianballou ianballou force-pushed the 36914-applicability-missing-host branch from 4df9ccf to 4bc64af Compare November 14, 2023 19:20
@ianballou ianballou requested a review from sjha4 November 14, 2023 19:25
@ianballou ianballou force-pushed the 36914-applicability-missing-host branch from 4bc64af to a07ff62 Compare November 14, 2023 19:27
@ianballou ianballou force-pushed the 36914-applicability-missing-host branch from a07ff62 to 2100081 Compare November 14, 2023 20:12
Copy link
Member

@sjha4 sjha4 left a comment

Choose a reason for hiding this comment

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

Code looks good. Task now skips and ends upon failure and is able to log errors when error introduced in calculate_and_import_applicability method instead of pausing.

@ianballou ianballou merged commit 17c731a into Katello:master Nov 15, 2023
@ianballou ianballou deleted the 36914-applicability-missing-host branch November 15, 2023 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants