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 #23928 - Migrate Pulp Sync Plans #7479

Closed
wants to merge 3 commits into from

Conversation

sjha4
Copy link
Member

@sjha4 sjha4 commented Jun 27, 2018

To do:

  • Fix timezone issues when creating recurring logic
  • Support Update (Using Enable/Disable?)
  • Modify Product Add/Remove to remove pulp sync plan calls
  • Optimizations
  • Create and start recurring logics for existing sync plans
  • Delete repo schedules in Pulp
  • Tests

@theforeman-bot
Copy link

Issues: #23928

@sjha4 sjha4 changed the title Fixes #23928 - Migrate Pulp Sync Plans [WIP]Fixes #23928 - Migrate Pulp Sync Plans Jun 27, 2018
Copy link
Member Author

@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.

@jlsherrill :
How do I add the has_one association on the foreman_tasks_recurring_logic without changing the model in Foreman task repo..If I add an activesupport concern, will it be included in the recurring logic model like other foreman related concerns in katello?

@jlsherrill
Copy link
Member

@sjha4 yep, exactly, just like all of these: https://github.com/Katello/katello/blob/master/lib/katello/engine.rb#L144-L161

I imagine recurring_logic would 'has_one' sync plan, and a sync plan would 'belong_to' a recurring_logic

@sjha4
Copy link
Member Author

sjha4 commented Jun 28, 2018

I imagine recurring_logic would 'has_one' sync plan, and a sync plan would 'belong_to' a recurring_logic

Right..Also, do you see any reason to add/remove products via dynflow task if we are not updating the pulp schedule anymore..I can think of bulk processing as one..If that's not the case we can move all of it to the sync_plan controller to simplify the code.

@jlsherrill
Copy link
Member

@sjha4 exactly, we can simplify it quite a bit!

@sjha4 sjha4 force-pushed the pulpSyncPlan branch 2 times, most recently from 2fdc289 to afe9153 Compare July 2, 2018 15:29
@sjha4 sjha4 force-pushed the pulpSyncPlan branch 2 times, most recently from 1b4fc51 to 159251e Compare July 10, 2018 15:39
@sjha4 sjha4 force-pushed the pulpSyncPlan branch 2 times, most recently from 20aba45 to 7019427 Compare July 20, 2018 16:48
@sjha4 sjha4 changed the title [WIP]Fixes #23928 - Migrate Pulp Sync Plans Fixes #23928 - Migrate Pulp Sync Plans Jul 20, 2018
@sjha4 sjha4 force-pushed the pulpSyncPlan branch 2 times, most recently from 8707fb9 to c0eb7f7 Compare July 20, 2018 23:06
@sjha4
Copy link
Member Author

sjha4 commented Jul 23, 2018

[test katello]

@sjha4 sjha4 force-pushed the pulpSyncPlan branch 4 times, most recently from 7ba5e68 to e8b62d7 Compare July 23, 2018 18:10
@sjha4
Copy link
Member Author

sjha4 commented Jul 23, 2018

[test katello]

1 similar comment
@sjha4
Copy link
Member Author

sjha4 commented Jul 24, 2018

[test katello]

@jlsherrill
Copy link
Member

looks like the tests are failing because of:

/var/lib/workspace/workspace/katello-pr-test/test/actions/katello/sync_plan/add_products_test.rb:3:in `<top (required)>': uninitialized constant Actions::Katello::SyncPlan::AddProducts (NameError)

@sjha4
Copy link
Member Author

sjha4 commented Jul 24, 2018

Aah..I looked at the webpack issue and assumed that was it..Thanks @jlsherrill 👍

@jlsherrill
Copy link
Member

Also some rubocop errors. Not sure why you're seeing webpack issues.... Maybe repushing will cause those to go away.

@theforeman-bot
Copy link

There were the following issues with the commit message:

  • 29a5fd1 must be in the format fixes #redmine_number - brief description

If you don't have a ticket number, please create an issue in Redmine.

More guidelines are available in Coding Standards or on the Foreman wiki.


This message was auto-generated by Foreman's prprocessor

sync_plan.products.each do |product|
product.repos(product.library).each do |repo_k|
repo = ::Katello::Repository.find(repo_k.id)
repo.sync_schedule(nil)
Copy link
Member

Choose a reason for hiding this comment

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

We can't rely on this method to exist (In fact you should probably delete it). Instead i would call into runcible directly with something like:

Katello.pulp_server.extensions.repository.remove_schedules(repo.pulp_id)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok..I'll go ahead and test with this:
Katello.pulp_server.extensions.repository.remove_schedules(repo.pulp_id, repo.importer_type)

@theforeman-bot
Copy link

There were the following issues with the commit message:

  • 29a5fd1 must be in the format fixes #redmine_number - brief description

If you don't have a ticket number, please create an issue in Redmine.

More guidelines are available in Coding Standards or on the Foreman wiki.


This message was auto-generated by Foreman's prprocessor

@theforeman-bot
Copy link

There were the following issues with the commit message:

  • 29a5fd1 must be in the format fixes #redmine_number - brief description

If you don't have a ticket number, please create an issue in Redmine.

More guidelines are available in Coding Standards or on the Foreman wiki.


This message was auto-generated by Foreman's prprocessor

@theforeman-bot
Copy link

There were the following issues with the commit message:

  • 29a5fd1 must be in the format fixes #redmine_number - brief description

If you don't have a ticket number, please create an issue in Redmine.

More guidelines are available in Coding Standards or on the Foreman wiki.


This message was auto-generated by Foreman's prprocessor

@theforeman-bot
Copy link

There were the following issues with the commit message:

  • 29a5fd1 must be in the format fixes #redmine_number - brief description

If you don't have a ticket number, please create an issue in Redmine.

More guidelines are available in Coding Standards or on the Foreman wiki.


This message was auto-generated by Foreman's prprocessor

@sjha4
Copy link
Member Author

sjha4 commented Aug 14, 2018

Closing this and moving commits to #7625.

@sjha4 sjha4 closed this Aug 14, 2018
@omkarkhatavkar
Copy link
Contributor

Whenever we update the recurring logic the new recurring logic will be created (new resource) and older one cancelled (not sure it is an intended behaviour, why not update the same resource ?) If not we can delete the cancelled recurring logic on the update itself.

Any thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants