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 #36859 - Fix inconsistent repo publication #10776

Merged
merged 1 commit into from
Dec 20, 2023

Conversation

m-bucher
Copy link
Contributor

Fix repo not having correct publication due to e.g. killed pulp-publication task.
Or rather make sure next Optimized Sync will fix this, by identifying the inconsistency.

@quba42
Copy link
Contributor

quba42 commented Oct 25, 2023

Or rather make sure next Optimized Sync will fix this, by identifying the inconsistency.

This is not actually related to optimized vs. complete sync. Both types won't repair an affected repo without this change, unless there is new content in the upstream repo being synced. If there is no new content in the upstream, both types of sync will not create a new pulp repo version (full sync will simply take longer to not create a new repo version). And in both cases Katello will not republish if there is no new repo version.

Only a "republish repository metadata" action can force a republish, which can currently only be achieved via hammer.

@m-bucher m-bucher force-pushed the fix_inconsistent_publication branch from d9c75c3 to ee30afc Compare October 25, 2023 12:32
@@ -81,5 +81,24 @@ def test_save_new_version_from_lookup
assert_equal task.output[:contents_changed], true
assert_empty task.output[:updated_repositories] - [@repo1.id, @repo2.id]
end

def test_save_version_with_outdated_publication
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this the correct file for the test?

I kinda doubt it, but it seemed to be the best fir without creating a new test-class 😇

Copy link
Member

Choose a reason for hiding this comment

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

Works for me!

@m-bucher m-bucher force-pushed the fix_inconsistent_publication branch from ee30afc to 3250cc7 Compare October 25, 2023 12:40
@m-bucher m-bucher force-pushed the fix_inconsistent_publication branch 2 times, most recently from 463f2ed to e7de774 Compare November 2, 2023 14:50
@sbernhard
Copy link
Member

Can you please have a look at this @jeremylenz ? Any thoughts as we think, this should fix a pretty bad bug.

@jeremylenz
Copy link
Member

Added a review card to our sprint, thanks!

@m-bucher m-bucher force-pushed the fix_inconsistent_publication branch from e7de774 to 500413b Compare December 4, 2023 17:34
@quba42
Copy link
Contributor

quba42 commented Dec 11, 2023

For the benefit of any potential reviewers: Note that I put fairly detailed reproducer instructions on the accompanying issue: https://projects.theforeman.org/issues/36859

Copy link
Member

@ianballou ianballou left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! Just one bug found:

app/lib/actions/pulp3/repository/save_version.rb Outdated Show resolved Hide resolved
@m-bucher
Copy link
Contributor Author

[test katello]

@m-bucher m-bucher requested a review from ianballou December 19, 2023 13:51
Copy link
Member

@ianballou ianballou left a comment

Choose a reason for hiding this comment

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

Working for me! I tested it by editing the publication_href for a repository to a new one that I created. After syncing, the repository's publication_href was returned to normal.

@ianballou ianballou merged commit df80f7b into Katello:master Dec 20, 2023
6 checks passed
@m-bucher m-bucher deleted the fix_inconsistent_publication branch December 22, 2023 13:29
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.

5 participants