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 #38117 - Refresh smart proxy sync history when proxy is updated #11270

Merged
merged 1 commit into from
Dec 23, 2024

Conversation

sjha4
Copy link
Member

@sjha4 sjha4 commented Dec 19, 2024

What are the changes introduced in this pull request?

Refresh smart proxy sync history when sp is updated and only keep sync histories of repos available to capsule.

Considerations taken when implementing this change?

When LCE is removed from sp, we will now assume the sync history is not needed anymore.
What this does is that when env is added back, an optimized sync will not skip over this repo during sync.

What are the testing steps for this pull request?

  1. Sync a LCE to the Smart proxy. This shows the package counts for cv correctly.
  2. Now remove the LCE from Smart proxy.
  3. Now add the previous LCE to the Smart proxy and sync it.
  4. Package count now shows up as N/A
  5. Refresh count doesnt fix this issue. Only complete sync fixes the counts again.

The issue here is that optimized sync simply skips the repos from the environment cause their old sync histories have not been cleared.

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 well! Just one comment about calculating the sync histories to delete.

Comment on lines 488 to 493
repos = smart_proxy_helper.repositories_available_to_capsule
repo_ids = repos.map(&:id)
if repo_ids.empty?
self.smart_proxy_sync_histories.delete_all
else
self.smart_proxy_sync_histories.where("repository_id NOT IN (?)", repo_ids).delete_all
end
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
repos = smart_proxy_helper.repositories_available_to_capsule
repo_ids = repos.map(&:id)
if repo_ids.empty?
self.smart_proxy_sync_histories.delete_all
else
self.smart_proxy_sync_histories.where("repository_id NOT IN (?)", repo_ids).delete_all
end
repos = smart_proxy_helper.repositories_available_to_capsule.select(:id)
# Use size to ensure repos don't get loaded unnecessarily
if repos.size == 0
self.smart_proxy_sync_histories.delete_all
else
# Uses a single SQL query without needing to load IDs
self.smart_proxy_sync_histories.where(id: repos).delete_all
end

Copy link
Member Author

Choose a reason for hiding this comment

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

Ack..Updated to this with slight change to maintain previous logic:
self.smart_proxy_sync_histories.where.not(repository_id: repos).delete_all

@sjha4 sjha4 force-pushed the clear_sp_sync_history_when_LE_removed branch from f764ef2 to 0792cf6 Compare December 20, 2024 20:29
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.

Looks like tests broke, but the change on a running machine works well for me.

@sjha4
Copy link
Member Author

sjha4 commented Dec 23, 2024

All relevant failures..Will update tests..

@sjha4 sjha4 force-pushed the clear_sp_sync_history_when_LE_removed branch from 0792cf6 to 0292963 Compare December 23, 2024 14:56
@sjha4 sjha4 force-pushed the clear_sp_sync_history_when_LE_removed branch from 0292963 to d55fe8e Compare December 23, 2024 15:12
@sjha4 sjha4 merged commit fb6cca7 into Katello:master Dec 23, 2024
26 checks passed
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