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

Add convert2rhel fact to host report #896

Merged
merged 4 commits into from
Sep 6, 2024
Merged

Add convert2rhel fact to host report #896

merged 4 commits into from
Sep 6, 2024

Conversation

jeremylenz
Copy link
Collaborator

@jeremylenz jeremylenz commented Aug 27, 2024 •
edited
Loading

To test:

Setup

On a registered host, create a custom fact file that includes conversions.env.CONVERT2RHEL_THROUGH_FOREMAN:

[root@toledo8 ~]# cat /etc/rhsm/facts/convert2rhel.facts {
"conversions.version": "1",
"conversions.activity": "conversion",
"conversions.packages.0.nevra": "convert2rhel-0:2.0.0-1.el8.noarch",
"conversions.packages.0.signature": "RSA/SHA256, Thu May 30 13:31:33 2024, Key ID 199e2f91fd431d51",
"conversions.executed": "/usr/bin/convert2rhel",
"conversions.success": true,
"conversions.activity_started": "2024-07-11T17:28:54.281664Z",
"conversions.activity_ended": "2024-07-11T17:48:47.026664Z",
"conversions.source_os.id": "Cerulean Leopard",
"conversions.source_os.name": "AlmaLinux",
"conversions.source_os.version": "8.10",
"conversions.target_os.id": "Ootpa",
"conversions.target_os.name": "Red Hat Enterprise Linux",
"conversions.target_os.version": "8.10",
"conversions.env.CONVERT2RHEL_THROUGH_FOREMAN": "1",
"conversions.run_id": "null"
}

Update facts:

subscription-manager facts --update

Verify that the host's subscription_facet_attributes have convert2rhel_through_foreman: 1

Test

Ensure that the host's host_registration_insights parameter is set to true
On the Inventory Upload page, click 'Sync inventory status' and wait for the sync to complete

On the host, run

organization_id=1 target=/var/lib/foreman/red_hat_inventory/generated_reports/ bundle exec rake rh_cloud_inventory:report:generate

You should see, at the end of the output, something like

Successfully generated /tmp/d20240827-17482-t377is/report_for_1.tar.xz for organization id 1

CD into that directory, then run

$ tar -vxf report_for_1.tar.xz
./
./metadata.json
./d5f5c08b-7903-4c58-8c08-53d1f37c753c.json

All files except metadata.json will be about individual hosts. Find your host and verify its json contains convert2rhel_through_foreman:

$ cat d5f5c08b-7903-4c58-8c08-53d1f37c753c.json | jq
{
  "report_slice_id": "d5f5c08b-7903-4c58-8c08-53d1f37c753c",
  "hosts": [
    {
      "fqdn": "rhel9b.fedora.example.com",
      "account": "1212729",
      "subscription_manager_id": "8cea8fa3-c2c1-463a-953c-ae29386ca7b5",
      "satellite_id": "8cea8fa3-c2c1-463a-953c-ae29386ca7b5",
      "convert2rhel_through_foreman": 1,
...

@jeremylenz jeremylenz marked this pull request as ready for review August 27, 2024 17:07
@jeremylenz
Copy link
Collaborator Author

The test failures are

NoMethodError: undefined method `convert2rhel_through_foreman' for #<Katello::Host::SubscriptionFacet:0x000055fcfcf4dfe0>

looks like the tests may be using old Katello code?

@jeremylenz
Copy link
Collaborator Author

@ShimShtein does the katello pin look good?

jeremylenz
jeremylenz commented Aug 29, 2024
View reviewed changes
test/controllers/insights_cloud/api/machine_telemetries_controller_test.rb Outdated
@@ -108,6 +108,7 @@ class MachineTelemetriesControllerTest < ActionController::TestCase
context '#branch_info' do
setup do
User.current = User.find_by(login: 'secret_admin')
Setting[:allow_multiple_content_views] = true
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is going to break downstream, unless we expose the setting via foreman_theme_satellite.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I am missing something. Is this setting introduced by the theme? I think I have seen it in Katello.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The multi-CV feature isn't fully complete yet, so it's blocked downstream. See https://github.com/RedHatSatellite/foreman_theme_satellite/blob/develop/app/models/concerns/upstream_only_settings.rb

This means that downstream, the setting is not exposed and Setting[:allow_multiple_content_views] = true will error.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

additional context: This had been working (unintentionally) until Katello implemented https://projects.theforeman.org/issues/37657

Copy link
Member

Choose a reason for hiding this comment

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

@jeremylenz Can you add a if condition or dry catch to skip the test if the setting is not there?

ShimShtein
ShimShtein reviewed Sep 3, 2024
View reviewed changes
foreman_rh_cloud.gemspec Outdated Show resolved Hide resolved
test/controllers/insights_cloud/api/machine_telemetries_controller_test.rb Outdated
@@ -108,6 +108,7 @@ class MachineTelemetriesControllerTest < ActionController::TestCase
context '#branch_info' do
setup do
User.current = User.find_by(login: 'secret_admin')
Setting[:allow_multiple_content_views] = true
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I am missing something. Is this setting introduced by the theme? I think I have seen it in Katello.

chris1984
chris1984 approved these changes Sep 4, 2024
View reviewed changes
Copy link
Member

@chris1984 chris1984 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Works great, thanks @jeremylenz. Looks fine from a code and function test. @ShimShtein do you have any more comments on the theme setting or the gem pin?

Tested on 6.16 latest snap

[root@ip-10-0-167-3 done]# cat 4dcdd615-bb4e-4500-84d5-b06a32969c80.json | jq
{
  "report_slice_id": "4dcdd615-bb4e-4500-84d5-b06a32969c80",
  "hosts": [
    {
      "fqdn": "ip-10-0-167-160.rhos-01.prod.psi.rdu2.redhat.com",
      "account": "540155",
      "subscription_manager_id": "4fa3fdf6-cb4d-4d0f-b186-6f67aaa21932",
      "satellite_id": "4fa3fdf6-cb4d-4d0f-b186-6f67aaa21932",
      "bios_uuid": "D903DB6C-4672-461F-A477-069556E70F05",
      "convert2rhel_through_foreman": 1,

@jeremylenz jeremylenz force-pushed the add-convert2rhel-fact branch 2 times, most recently from fe8452b to 061703f Compare September 5, 2024 15:49
@jeremylenz
Copy link
Collaborator Author

@ShimShtein @chris1984 updated to skip tests if multiCV isn't available.

@chris1984 chris1984 self-assigned this Sep 6, 2024
chris1984
chris1984 approved these changes Sep 6, 2024
View reviewed changes
Copy link
Member

@chris1984 chris1984 left a comment

Choose a reason for hiding this comment

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

Looks good, thanks @jeremylenz

@chris1984 chris1984 merged commit 017a2eb into theforeman:v10 Sep 6, 2024
17 checks passed
@jeremylenz jeremylenz deleted the add-convert2rhel-fact branch September 6, 2024 19:55
@chris1984 chris1984 mentioned this pull request Sep 20, 2024
Merged
@jeremylenz jeremylenz mentioned this pull request Oct 22, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ShimShtein ShimShtein ShimShtein left review comments

@chris1984 chris1984 chris1984 approved these changes

Assignees

@chris1984 chris1984

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jeremylenz @chris1984 @ShimShtein