Refs #36706 - Remove remaining katello-agent code including Agent Events & Dynflow actions #10710

jeremylenz · 2023-08-28T14:37:27Z

What are the changes introduced in this pull request?

Remove remaining katello-agent code, including

katello-agent events from Event Daemon
katello-agent Dynflow actions
katello-agent Dispatcher and Dispatch Histories
miscellaneous stragglers

Considerations taken when implementing this change?

Probably not enough

This removes Agent Events, but the other two event types handled by the Event Daemon (Katello Events and Candlepin Events) should be unaffected.

What are the testing steps for this pull request?

Run migrations
Verify that tests pass and the remaining tests make sense
Verify that basic functionality still works, particularly incremental updates
Verify that Katello Events and Candlepin Events still work properly

theforeman-bot · 2023-08-28T14:37:30Z

Issues: #36706

jeremylenz · 2023-08-28T14:43:38Z

This is currenly rebased on the PR that removes API endpoints; I'll rebase on master when that's in.

jeremylenz · 2023-08-28T14:51:13Z

app/models/katello/ping.rb

-      # get package information for katello and its components
-      def packages
-        names = PACKAGES.join("|")
-        packages = `rpm -qa | egrep "#{names}"`
-        packages.split("\n").sort
-      end


I couldn't see anywhere where this was used. 🤷

jeremylenz · 2023-08-28T14:51:52Z

katello.gemspec

@@ -37,7 +37,6 @@ Gem::Specification.new do |gem|
  gem.add_dependency "foreman_remote_execution", ">= 7.1.0"
  gem.add_dependency "dynflow", ">= 1.6.1"
  gem.add_dependency "activerecord-import"
-  gem.add_dependency "qpid_proton"


oh hey, @Katello/packagers 😇

jturel

You got all the things I thought someone might miss like removing the DispatchHistory table and qpid gem. Nice!

jeremylenz · 2023-08-29T18:42:13Z

rebased

parthaa · 2023-08-29T19:02:01Z

have you thought about removing this https://github.com/Katello/katello/blob/master/app/controllers/katello/api/rhsm/candlepin_proxies_controller.rb#L189

parthaa · 2023-08-29T19:23:41Z

app/lib/katello/concerns/base_template_scope_extensions.rb

@@ -358,11 +353,9 @@ def get_task_input(task)

      def parse_errata(task)
        task_input = get_task_input(task)
-        agent_input = task_input['errata'] || task_input['content']


I am wondering if this should be agent_input = task_input['errata'] or may be below
@_tasks_errata_cache[task.id] ||= task_input['errata'].presence? || errata_ids_from_template_invocation(task, task_input)

On second thought, I'm remembering now that this is used in the Applied Errata report so that both katello-agent-applied and REX-applied errata are included in the report. So I restored this to the way it was (with improved comments.)

jeremylenz · 2023-08-30T14:45:52Z

have you thought about removing this https://github.com/Katello/katello/blob/master/app/controllers/katello/api/rhsm/candlepin_proxies_controller.rb#L189

I'm a bit nervous about that one. Are we sure it's not also used by the subscription-manager plugin somewhere? It's in the rhsm_proxies_controller so it probably felt safer not to touch it. I could update the description..

I did do some spot checks just now, and I can't get sub-man to hit that endpoint (it doesn't with subscription-manager repos or subscription-manager repo-override) so maybe it is safe to remove.. thoughts?

parthaa · 2023-08-30T19:06:03Z

I did do some spot checks just now, and I can't get sub-man to hit that endpoint (it doesn't with subscription-manager repos or subscription-manager repo-override) so maybe it is safe to remove.. thoughts?

On 2nd thoughts i think it is used by katello-host-tools.
https://github.com/Katello/katello-host-tools/blob/master/src/katello/repos.py#L31

So unless we are removing dnf upload, I say we leave this endpoint alone.

jeremylenz · 2023-08-30T19:08:04Z

So unless we are removing dnf upload, I say we leave this endpoint alone.

Sounds good to me. In which case, is this good to go now? 😄

parthaa · 2023-08-30T20:26:45Z

Need to remove this line https://github.com/Katello/katello/blob/master/engines/bastion_katello/lib/bastion_katello/engine.rb#L43

jeremylenz · 2023-08-30T20:39:46Z

removed

engines/bastion_katello/lib/bastion_katello/engine.rb

ehelms · 2023-08-31T00:50:41Z

I know you are already pretty far along, something to think about in the future for these kind of changes is to split them into two parts:

Do the bare minimum to stop exposing whatever the feature is to users anymore
Clean-up the code at some future date

This both can allow you to move faster by getting in the actual drop faster (as the code changes are much smaller, and less risky) and it can provide a fall-back mechanism for some amount of time if the change or timeline is risky.

I mentioned a similar thing here for the installer part as well -- theforeman/puppet-foreman_proxy_content#455 (comment)

jeremylenz · 2023-08-31T13:51:49Z

split them into two parts:

Do the bare minimum to stop exposing whatever the feature is to users anymore

Clean-up the code at some future date

Makes sense! I would actually consider this PR the "clean-up at some future date," since all our API and UI changes in Katello are already merged. From most to least essential, I think about it like

Installer / foreman-maintain - prevent users from having katello-agent enabled. And if we can't do that,
remove from UI / API / CLI - (if this is all we can get in before release, it's not ideal but still workable)
and then finally, remove all the rest of the unused code in Katello (this PR)
remove unused miscellaneous stuff elsewhere (client tools, sosreport, etc.)

parthaa

ACK
Verified that incremental updates does the right thing and always tries remote execution.

…nts & Dynflow actions (#10710) * Refs #36706 - Remove agent events and actions * Refs #36706 - Remove dispatch histories via migration * Refs #36706 - fix tests * Refs #36706 - remove katello-agent-notice * Refs #36706 - use errata presence * Refs #36706 - add back agent_input * Refs #36706 - remove bastion reference and more stragglers (cherry picked from commit ea06f3d)

theforeman-bot added the Not yet reviewed label Aug 28, 2023

github-actions bot added the Packaging Change label Aug 28, 2023

jeremylenz force-pushed the 36706-face-your-daemons branch from 1008d3c to e7dc71f Compare August 28, 2023 14:41

jeremylenz commented Aug 28, 2023

View reviewed changes

jeremylenz changed the title ~~36706 face your daemons~~ Refs #36706 - Remove remaining katello-agent code including Agent Events & Dynflow actions Aug 28, 2023

jeremylenz marked this pull request as ready for review August 28, 2023 17:37

jeremylenz requested review from jturel, parthaa and ianballou August 28, 2023 17:37

jturel approved these changes Aug 28, 2023

View reviewed changes

jeremylenz added 4 commits August 29, 2023 14:40

Refs #36706 - Remove agent events and actions

171dfa3

Refs #36706 - Remove dispatch histories via migration

793386d

Refs #36706 - fix tests

269e744

Refs #36706 - remove katello-agent-notice

4d90155

jeremylenz force-pushed the 36706-face-your-daemons branch from 0e1b1b1 to 4d90155 Compare August 29, 2023 18:41

parthaa reviewed Aug 29, 2023

View reviewed changes

jeremylenz added 2 commits August 30, 2023 12:49

Refs #36706 - use errata presence

fe6b3d4

Refs #36706 - add back agent_input

0f789c7

parthaa reviewed Aug 30, 2023

View reviewed changes

engines/bastion_katello/lib/bastion_katello/engine.rb Outdated Show resolved Hide resolved

Refs #36706 - remove bastion reference and more stragglers

997ad6a

jeremylenz force-pushed the 36706-face-your-daemons branch from cf2c033 to 997ad6a Compare August 30, 2023 20:47

parthaa approved these changes Aug 31, 2023

View reviewed changes

jeremylenz merged commit ea06f3d into Katello:master Aug 31, 2023
4 of 5 checks passed

ofedoren mentioned this pull request Sep 4, 2023

Fixes #36717 - Remove katello-agent leftovers from redhat-register.erb theforeman/foreman#9820

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refs #36706 - Remove remaining katello-agent code including Agent Events & Dynflow actions #10710

Refs #36706 - Remove remaining katello-agent code including Agent Events & Dynflow actions #10710

jeremylenz commented Aug 28, 2023 •

edited

Loading

theforeman-bot commented Aug 28, 2023

jeremylenz commented Aug 28, 2023 •

edited

Loading

jeremylenz Aug 28, 2023

jeremylenz Aug 28, 2023

jturel left a comment

jeremylenz commented Aug 29, 2023

parthaa commented Aug 29, 2023

parthaa Aug 29, 2023

jeremylenz Aug 30, 2023

jeremylenz commented Aug 30, 2023

parthaa commented Aug 30, 2023 •

edited

Loading

jeremylenz commented Aug 30, 2023 •

edited

Loading

parthaa commented Aug 30, 2023

jeremylenz commented Aug 30, 2023

ehelms commented Aug 31, 2023

jeremylenz commented Aug 31, 2023 •

edited

Loading

parthaa left a comment •

edited

Loading

Refs #36706 - Remove remaining katello-agent code including Agent Events & Dynflow actions #10710

Refs #36706 - Remove remaining katello-agent code including Agent Events & Dynflow actions #10710

Conversation

jeremylenz commented Aug 28, 2023 • edited Loading

What are the changes introduced in this pull request?

Considerations taken when implementing this change?

What are the testing steps for this pull request?

theforeman-bot commented Aug 28, 2023

jeremylenz commented Aug 28, 2023 • edited Loading

jeremylenz Aug 28, 2023

Choose a reason for hiding this comment

jeremylenz Aug 28, 2023

Choose a reason for hiding this comment

jturel left a comment

Choose a reason for hiding this comment

jeremylenz commented Aug 29, 2023

parthaa commented Aug 29, 2023

parthaa Aug 29, 2023

Choose a reason for hiding this comment

jeremylenz Aug 30, 2023

Choose a reason for hiding this comment

jeremylenz commented Aug 30, 2023

parthaa commented Aug 30, 2023 • edited Loading

jeremylenz commented Aug 30, 2023 • edited Loading

parthaa commented Aug 30, 2023

jeremylenz commented Aug 30, 2023

ehelms commented Aug 31, 2023

jeremylenz commented Aug 31, 2023 • edited Loading

parthaa left a comment • edited Loading

Choose a reason for hiding this comment

jeremylenz commented Aug 28, 2023 •

edited

Loading

jeremylenz commented Aug 28, 2023 •

edited

Loading

parthaa commented Aug 30, 2023 •

edited

Loading

jeremylenz commented Aug 30, 2023 •

edited

Loading

jeremylenz commented Aug 31, 2023 •

edited

Loading

parthaa left a comment •

edited

Loading