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 #36500 - Optimize DockerMetaTag query and CV version deletion to run a single invocation of the method #10599

Merged
merged 1 commit into from
Jul 20, 2023

Conversation

sjha4
Copy link
Member

@sjha4 sjha4 commented Jun 12, 2023

What are the changes introduced in this pull request?

  1. Avoid using pluck in Metatag cleanup.
  2. Adds RepoMetaTag deletion before cleanup of schema_id = nil records to prevent foreign key violation failures.
  3. Don't call metatag cleanup on repo destroy of every docker repo during CV version deletion or remove from environment.
  4. Add metatag cleanup to finalize of those 2 actions directly so it runs once on each invocation.
  5. Deleting CV version in environments calls both and hence metatag cleanup runs twice for that.
    Once for removing CV from environment and once for deleting CV version itself

Considerations taken when implementing this change?

What are the testing steps for this pull request?

  1. Add several docker repos to a CV and publish/promote few versions.
  2. Delete a version, make sure docker_cleanup param is set to false in repo_destroy finalize action spawned by CV version delete.
  3. Test other actions like CV remove from env, Cv version delete, CV delete, repo delete etc. to make sure metatag cleanup doesn't happen every repo, rather only in finalize of parent actions like CVE delete or CVV delete.

@theforeman-bot
Copy link

Issues: #36500

@sjha4
Copy link
Member Author

sjha4 commented Jun 13, 2023

[test katello]

@sjha4
Copy link
Member Author

sjha4 commented Jun 13, 2023

Test failure related. Will update.

@parthaa
Copy link
Contributor

parthaa commented Jul 18, 2023

Other than cleanup the code looks ok. I am going to quickly test once the suggested changes have been addressed.

Copy link
Contributor

@parthaa parthaa left a comment

Choose a reason for hiding this comment

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

Works as advertised :)

@sjha4 sjha4 merged commit 6077a1e into Katello:master Jul 20, 2023
5 checks passed
wbclark pushed a commit to wbclark/katello that referenced this pull request Sep 7, 2023
…o run a single invocation of the method (Katello#10599)

(cherry picked from commit 6077a1e)
wbclark pushed a commit that referenced this pull request Sep 27, 2023
…o run a single invocation of the method (#10599)

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