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 #36638 - Allow metadata regenerate with --force flag on API/Hammer #10681

Merged
merged 1 commit into from
Aug 4, 2023

Conversation

sjha4
Copy link
Member

@sjha4 sjha4 commented Aug 4, 2023

What are the changes introduced in this pull request?

The plan for Regenerating metadata on "complete_mirroring" is as follows:
a) Disallowed on the UI : Republishing on UI on repos and cv versions with complete_mirroring repos will not be allowed.
b) Allowed in hammer : only with --force true

Considerations taken when implementing this change?

We want to discourage users from republishing metadata locally on Pulp for repositories with complete mirroring. Disallowing on UI and only allowing via hammer/api with --force flag makes it so.

What are the testing steps for this pull request?

  1. Create a yum repo with mirroring policy = Complete Mirroring and another with not mirror complete.
  2. Sync the repos, add them to a CV and publish.
  3. Check to see that Republishing Metadata action is disabled on repo with complete mirroring and enabled for the other on the UI.
  4. In hammer, you should be able to regenerate metadata on both, however only with --force 1 on complete mirroring repo.
    Check hammer -r repository republish --id $REPO_ID --force false vs hammer -r repository republish --id $REPO_ID --force true
  5. For the CV version, republishing metadata from Version Details > Actions will be disabled. (Since a mirror complete repo exists in the version)
  6. Via hammer, you'll get an error message when republishing metadata without force flag.
  7. Hammer should allow republishing metadata with --force flag. Check number of sub-actions in the bulk task to make sure all repos get republished with
    hammer -r content-view version republish-repositories --id $VERSION_ID --force 1 and hammer -r content-view version republish-repositories --id $VERSION_ID --force 0

@theforeman-bot
Copy link

Issues: #36638

Copy link
Member

@jeremylenz jeremylenz left a comment

Choose a reason for hiding this comment

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

Thanks @sjha4

Everything works as advertised, except this:

Seems the hammer error message here includes all mirror-complete repos:

Could not republish the Content View:
  Metadata republishing is dangerous on content view versions with repositories with the 'Complete Mirroring' mirroring policy.
            Update mirroring policies on these repositories ["Red Hat Enterprise Linux 8 for x86_64 - BaseOS RPMs 8", "Red Hat Enterprise Linux 8 for x86_64 - AppStream RPMs 8", "JustinComplete"] or Use 'force' parameter to stop mirroring upstream metadata and regenerate metadata locally.

But I don't have the RHEL ones included in the content view:

image

mirror_complete_repos = @content_view_version.repositories.joins(:root).where(root: { mirroring_policy: ::Katello::RootRepository::MIRRORING_POLICY_COMPLETE })
if mirror_complete_repos.size > 0 && !::Foreman::Cast.to_bool(params[:force])
fail HttpErrors::BadRequest, _("Metadata republishing is dangerous on content view versions with repositories with the 'Complete Mirroring' mirroring policy.
Update mirroring policies on these repositories #{mirror_complete_repos.pluck(:name)} or Use 'force' parameter to stop mirroring upstream metadata and regenerate metadata locally.")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Update mirroring policies on these repositories #{mirror_complete_repos.pluck(:name)} or Use 'force' parameter to stop mirroring upstream metadata and regenerate metadata locally.")
Update mirroring policies on these repositories #{mirror_complete_repos.pluck(:name)}, or use the 'force' parameter to stop mirroring upstream metadata and regenerate metadata locally.")

fail HttpErrors::BadRequest, _("Metadata republishing is not allowed on repositories with the 'Complete Mirroring' mirroring policy.")
if @repository.mirroring_policy == Katello::RootRepository::MIRRORING_POLICY_COMPLETE && !::Foreman::Cast.to_bool(params[:force])
fail HttpErrors::BadRequest, _("Metadata republishing is dangerous on repositories with the 'Complete Mirroring' mirroring policy.
Change mirroring policy on the repository and try again or use 'force' parameter via hammer/API to stop mirroring upstream metadata and regenerate metadata locally.")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Change mirroring policy on the repository and try again or use 'force' parameter via hammer/API to stop mirroring upstream metadata and regenerate metadata locally.")
Change the mirroring policy on the repository and try again, or use the 'force' parameter to stop mirroring upstream metadata and regenerate metadata locally.")

app/controllers/katello/api/v2/repositories_controller.rb Outdated Show resolved Hide resolved
Copy link
Member

@jeremylenz jeremylenz left a comment

Choose a reason for hiding this comment

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

ACK 👍

@sjha4 sjha4 merged commit b32de37 into Katello:master Aug 4, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants