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 #36528 - handle integer hash keys in katello-agent actions #10627

Merged

Conversation

jeremylenz
Copy link
Member

@jeremylenz jeremylenz commented Jun 22, 2023

What are the changes introduced in this pull request?

For some reason, args[:dispatch_histories] is ending up with integer keys, but the code was expecting string keys. This caused no sub-plans to be created for any Katello-agent bulk action.

I changed the code to handle both string and integer hash keys. After doing this, I noticed that the action would succeed on the host, but the task would still fail:

RuntimeError

Host did not respond within 20 seconds. The task has been cancelled. Is katello-agent installed and goferd running on the Host?

This was because Katello::AgentAction#plan was still expecting options[:dispatch_histories] to have string keys. I changed that to handle both string and integer as well.

Considerations taken when implementing this change?

Originally I had just changed .to_s to .to_i, but realized I don't necessarily know for sure all the code paths that create Katello::Agent::DispatchHistorys. Figured it was safer to handle both data types.

What are the testing steps for this pull request?

Set up katello-agent:

sudo foreman-installer \
--scenario katello-devel \
--katello-devel-modulestream-nodejs=14\
--foreman-proxy-content-enable-katello-agent=true

Administer > Settings > Content > Use remote execution by default > No

Enable the RH Client repo
Install katello-agent on a host

Test basic setup: Hosts > All Hosts > your host > Packages > kebab > Install packages > Install "via katello-agent"
Task should succeed.

Test bulk actions:
Hosts > Content Hosts > Select your host (you don't need more than one) > Select action > Manage Packages > type a package name > Install > via katello-agent

Before: Task fails and Dynflow console for the task shows no subplans.
After: Task succeeds with no errors. Bulk action shows sub-plans for each host.

@theforeman-bot
Copy link

Issues: #36528

@jeremylenz jeremylenz force-pushed the 36528-bulk-agent-actions-broken branch from 5b1c2cc to bc9accc Compare June 22, 2023 18:31
@jeremylenz jeremylenz force-pushed the 36528-bulk-agent-actions-broken branch from bc9accc to 891de3d Compare June 30, 2023 20:40
@jeremylenz
Copy link
Member Author

@chris1984 added comment 👍

@lfu
Copy link
Member

lfu commented Jun 30, 2023

Before
subplan
pending task

After
success subplan
success task

Copy link
Member

@lfu lfu left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏻

@jeremylenz jeremylenz merged commit f9bf7d0 into Katello:master Jun 30, 2023
5 checks passed
lfu pushed a commit to lfu/katello that referenced this pull request Jul 12, 2023
lfu pushed a commit that referenced this pull request Jul 12, 2023
wbclark pushed a commit to wbclark/katello that referenced this pull request Sep 7, 2023
wbclark pushed a commit that referenced this pull request Sep 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants