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 #28189 - Refresh repo when needed #8415

Merged
merged 2 commits into from Nov 12, 2019

Conversation

parthaa
Copy link
Contributor

@parthaa parthaa commented Nov 4, 2019

When a manifest refresh is triggered all the RH repos update their
distributors and importers, even if those repos did not change. This
commit makes a repo update its importers and distributors only if
needed.

@theforeman-bot
Copy link

Issues: #28189

@parthaa
Copy link
Contributor Author

parthaa commented Nov 4, 2019

To test this
In master

  • Import a manifest
  • Enable a couple of RH repos
  • Refresh manifest
  • Goto the dynflow task for the RefreshManifest and check for Actions::Pulp::Repository::Refresh (success)
  • Make sure you see a bunch of pulp tasks in the output for the Pulp::Repository::Refresh

Now check out the PR

  • Refresh manifest
  • Goto the dynflow task for the RefreshManifest and check for Actions::Pulp::Repository::Refresh (success)
  • Make sure you see pulp_tasks:[] in the output for the Pulp::Repository::Refresh

@parthaa
Copy link
Contributor Author

parthaa commented Nov 5, 2019

[test katello]

@jjeffers
Copy link
Member

jjeffers commented Nov 5, 2019

@parthaa is there a test case where the refresh_if_needed is triggered? I think I see that the test change shows the condition where it is not triggered. Correct me if I'm reading that incorrectly.

@parthaa
Copy link
Contributor Author

parthaa commented Nov 5, 2019

@parthaa is there a test case where the refresh_if_needed is triggered? I think I see that the test change shows the condition where it is not triggered. Correct me if I'm reading that incorrectly.

https://github.com/Katello/katello/pull/8415/files#diff-9a8be58a012da76042ea5e21ad94e40aR160

I test if refresh_if_needed does not call refresh

@@ -199,6 +201,10 @@ def proxy_host_importer_value
root.ignore_global_proxy ? "" : nil
end

def refresh_if_needed
refresh if needs_importer_updates? || needs_distributor_updates?
end
Copy link
Member

Choose a reason for hiding this comment

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

this is the same logic as mirror_needs_updates? maybe we could combine those into one method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

When a manifest refresh is triggered all the RH repos update their
distributors and importers, even if those repos did not change. This
commit makes a repo update its importers and distributors only if
needed.
@parthaa
Copy link
Contributor Author

parthaa commented Nov 7, 2019

[test katello]

2 similar comments
@parthaa
Copy link
Contributor Author

parthaa commented Nov 7, 2019

[test katello]

@parthaa
Copy link
Contributor Author

parthaa commented Nov 8, 2019

[test katello]

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.

ACK based on the below testing:

  1. On master, manifest refresh causes pulp repo refresh on all enabled repos.
  2. On branch, manifest refresh does not invoke pulp tasks for enabled repos when not needed
    Refresh_not_needed
  3. On branch, manifest refresh does invoke pulp tasks for enabled repos when needed
    Refresh_needed

@parthaa parthaa merged commit 1237135 into Katello:master Nov 12, 2019
@parthaa parthaa deleted the refreshIfNeeded branch November 12, 2019 16:14
jturel pushed a commit to jturel/katello that referenced this pull request Nov 20, 2019
* Fixes #28189 - Refresh repo when needed

When a manifest refresh is triggered all the RH repos update their
distributors and importers, even if those repos did not change. This
commit makes a repo update its importers and distributors only if
needed.

(cherry picked from commit 1237135)
jturel pushed a commit to jturel/katello that referenced this pull request Nov 20, 2019
* Fixes #28189 - Refresh repo when needed

When a manifest refresh is triggered all the RH repos update their
distributors and importers, even if those repos did not change. This
commit makes a repo update its importers and distributors only if
needed.

(cherry picked from commit 1237135)
jturel pushed a commit that referenced this pull request Nov 25, 2019
* Fixes #28189 - Refresh repo when needed

When a manifest refresh is triggered all the RH repos update their
distributors and importers, even if those repos did not change. This
commit makes a repo update its importers and distributors only if
needed.

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