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.

Sign up for GitHub

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

Jump to bottom

Fixes #38192 - Separate the Satellite Inventory Plugin from "host_registration_insights" parameter #10433

Closed
wants to merge 1 commit into from
Closed

Fixes #38192 - Separate the Satellite Inventory Plugin from "host_registration_insights" parameter #10433

wants to merge 1 commit into from

Conversation

chris1984
Copy link
Member

  • Adding a new parameter called host_registration_inventory_plugin to allow the host to be included/excluded from inventory upload
  • Added some unit tests

What are the changes introduced in this pull request?

What are the testing steps for this pull request?

  • Check out PR
  • Register a host with the parameter set to false or true
  • Check the host parameter to see if the value is reflected during the global registration

@chris1984
Copy link
Member Author

@stejskalleos here is the pr

@chris1984 chris1984 mentioned this pull request Feb 4, 2025
Open
@stejskalleos stejskalleos self-assigned this Feb 7, 2025
stejskalleos
stejskalleos requested changes Feb 11, 2025
View reviewed changes
Copy link
Contributor

@stejskalleos stejskalleos left a comment

Choose a reason for hiding this comment

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

@chris1984 I don't see any changes in the host_init_config template. Currently, the parameter can be set in UI, but the registered host won't be affected.

app/controllers/api/v2/registration_commands_controller.rb
@@ -13,6 +13,7 @@ class RegistrationCommandsController < V2::BaseController
param :operatingsystem_id, :number, desc: N_("ID of the Operating System to register the host in. Operating system must have a `host_init_config` template assigned")
param :smart_proxy_id, :number, desc: N_("ID of the Smart Proxy. This Proxy must have enabled both the 'Templates' and 'Registration' features")
param :setup_insights, :bool, desc: N_("Set 'host_registration_insights' parameter for the host. If it is set to true, insights client will be installed and registered on Red Hat family operating systems")
param :setup_insights_inventory, :bool, desc: N_("Set 'host_registration_insights_inventory' parameter for the host. If it is set to true, insights data about the host will be uploaded to console.redhat.com during report generation")
Copy link
Contributor

Choose a reason for hiding this comment

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

@Lennonka text review please

app/controllers/api/v2/registration_controller.rb
@@ -24,6 +24,7 @@ class RegistrationController < V2::BaseController
param :operatingsystem_id, :number, desc: N_("ID of the Operating System to register the host in. Takes precedence over the `operatingsystem` parameter")
param :operatingsystem, String, desc: N_("Title of the Operating System to register the host in")
param :setup_insights, :bool, desc: N_("Set 'host_registration_insights' parameter for the host. If it is set to true, insights client will be installed and registered on Red Hat family operating systems")
param :setup_insights_inventory, :bool, desc: N_("Set 'host_registration_insights_inventory' parameter for the host. If it is set to true, insights data about the host will be uploaded to console.redhat.com during report generation")
Copy link
Contributor

Choose a reason for hiding this comment

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

@Lennonka text review please

@chris1984
Copy link
Member Author

@chris1984 I don't see any changes in the host_init_config template. Currently, the parameter can be set in UI, but the registered host won't be affected.

@stejskalleos do I need any logic in the host_init_config_default.erb ? it is just so we can search on that parameter here in rh_cloud https://github.com/theforeman/foreman_rh_cloud/pull/947/files#diff-6a672e3eadb9fcfaf14ec4f967b040dc86066d1305b748a5bb770683fc188297R47

@stejskalleos
Copy link
Contributor

stejskalleos commented Feb 11, 2025 •
edited
Loading

do I need any logic in the host_init_config_default.erb ? it is just so we can search on that parameter here in rh_cloud

So we are adding the whole logic just to be able to search the host? Can it be done in an easier way?

@chris1984
Copy link
Member Author

chris1984 commented Feb 11, 2025 •
edited
Loading

Added some logic to create a fact in the host_init_config_default.erb checked on my host and it looks good

[root@ip-10-0-168-159 ~]# ll /etc/rhsm/facts/
total 8
-rw-r--r--. 1 root root 278 Feb 11 15:07 insights-client.facts
-rw-r--r--. 1 root root  39 Feb 11 15:25 insights-inventory.facts

@chris1984
Fixes #38192 - Separate the Satellite Inventory Plugin from "host_reg…
1374f4d 
…istration_insights" parameter

* Adding a new parameter called host_registration_inventory_plugin to
  allow the host to be included/excluded from inventory upload
* Added some unit tests
@chris1984
Copy link
Member Author

After talking with Leos, this should live in the rh_cloud plugin and is also a feature. Closing this out

@chris1984 chris1984 closed this Feb 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stejskalleos stejskalleos stejskalleos requested changes

Assignees

@stejskalleos stejskalleos

Labels
Needs re-review Templates UI
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@chris1984 @stejskalleos