Add additional fields for convert2RHEL

[jeremylenz]

This adds several convert2RHEL fields to the generated report:

"conversions": {
        "source_os": {
          "name": "AlmaLinux",
          "version": "8.10"
        },
        "target_os": {
          "name": "Red Hat Enterprise Linux",
          "version": "8.10"
        },
        "convert2rhel_through_foreman": 1,
        "activity": "conversion",
        "packages_0_nevra": "convert2rhel-0:2.0.0-1.el8.noarch",
        "packages_0_signature": "RSA/SHA256, Thu May 30 13:31:33 2024, Key ID 199e2f91fd431d51",
        "activity_started": "2024-07-11T17:28:54.281664Z",
        "activity_ended": "2024-07-11T17:48:47.026664Z",
        "success": "true"
      },

I had to learn several things the hard way here:

1. For some reason, RH Cloud seems to have its own JSON generator in lib/foreman_inventory_upload/generators/json_stream.rb. !!??
2. In order to use the simple_field and object_field methods of this JSON generator properly, you must pass a second argument, last. If you don't pass last, and your field happens to be the last field in a object, the stream will include an extra comma and thus stop being valid JSON.
3. In order to be able to use the fact_values method, you have to add the fact names to self.fact_names in lib/foreman_inventory_upload/generators/queries.rb. If a fact name returns nil, simple_field will just return and skip that line. This causes problems with no. 2 above, because you never know for sure if a field will be the last field. (I'm guessing this is the reason for the comma method.

Testing

see the instructions on https://github.com/theforeman/foreman_rh_cloud/pull/896 Make sure to test with different types of hosts (insights-enabled, insights-disabled)

Pipe your report output through jq (that's also a handy way to know if it's valid JSON) and make sure all the new values show up.

[jeremylenz]

Did you want to remove the top level one and just use yours?

I left both in because I couldn't decide.

@bocekm thoughts on the above data shape? We could remove the top-level convert2rhel_through_foreman if nothing relies on it yet.

[ShimShtein]

@jeremylenz The reason we have our own implementation for JSON is because the standard one does not have a notion of streaming. If we use the regular JSON, it will create a ton of objects in Ruby's heap, which is not desired, especially if you are working with large JSONs. This is also the reason that we have the comma and last methods - we need to know if we should write the comma into the string, or is it the last object, and hence the comma is not needed.

If you have a better idea of last object detection and emission, I would really appreciate it.

My way to deal with the last parameter was to put the fields that we're not sure that they will appear before a field that is always written. This way we can be sure that the always written field is the last one. It's not bulletproof, but it worked till now.

[jeremylenz]

The reason we have our own implementation for JSON is because the standard one does not have a notion of streaming. If we use the regular JSON, it will create a ton of objects in Ruby's heap, which is not desired, especially if you are working with large JSONs. This is also the reason that we have the comma and last methods - we need to know if we should write the comma into the string, or is it the last object, and hence the comma is not needed.

If you have a better idea of last object detection and emission, I would really appreciate it.

Thanks for explaining :) My first thoughts were to use rabl, or to build a Ruby hash first and then convert it to json. But to be honest I haven't done much of a deep dive into how to minimize allocations etc., or if either of those would help in that context. We could add that to our list for RHCloud.

[jeremylenz]

Added default values to a few simple_field calls to ensure that we don't generate invalid JSON. Verified that generating the report works both with and without those facts present.

[jeremylenz]

Test failures unrelated, I think. That test passes locally; can we try running again?

[bocekm]

@bocekm thoughts on the above data shape? We could remove the top-level convert2rhel_through_foreman if nothing relies on it yet.

You can remove the top level one. Nothing relies on it so far, reading it in Tableau/Grokket is the next step (https://issues.redhat.com/browse/PNTBIZIN-12423) after all these conversion related changes to rh_cloud are shipped and available in Satellite.

[jeremylenz]

Removed the top-level convert2rhel_through_foreman node.

[chris1984]

@ColeHiggins2 can you test this one please :)

[ColeHiggins2]

@jeremylenz @chris1984 Verified and tested with Packit

[bocekm]

It seems to me that the "success" field should be returing true/false without quotes. With quotes it's a string and not boolean, isn't it?