Fixes #36599 - Add hostname to hypervisor task with validation error

[chris1984]

What are the changes introduced in this pull request?

• In Actions::Katello::Host::Hypervisors returned the name of the hypervisor hostname reporting in instead of just hostname
• Returned an error like Foreman's validation citing the RFCs around hostnames
• Fixed a test that was failing because of my added validation

Considerations taken when implementing this change?

• Wrote the create_host_for_hypervisor method in a begin/rescue statement so that if the host does not have any issues we just move along like normal.

What are the testing steps for this pull request?

• Check out PR
• Install virt-who with dnf
• Create a virt-who config file in /etc/virt-who.d/ I called mine fake.conf Here is what mine looks like:

[fakevirt]
type=fake
file=/tmp/fake.json
is_hypervisor=True
owner=Default_Organization
hypervisor_id=$&

• Now in /tmp create a fake.json that looks like this

{
        "hypervisors": [{
                "uuid": "$&",
                "guests": [{
                                "guestId": "a8f73be7-c6a9-4c2a-8590-0ec2457c975d",
                                "state": 1
                        },
                        {
                                "guestId": "3e62be70-2365-4b53-8d2c-84c89cc1373a",
                                "state": 1
                        },
                        {
                                "guestId": "a746244b-125a-4f09-80bd-c043413a9153",
                                "state": 1
                        },
                        {       "guestId": "b5e07903-f5d8-44e6-b2dd-922743c402e2",
                                "state": 1
                        },
                        {       "guestId": "be263884-7c5c-4d02-a268-0cfadef3c6b4",
                                "state": 1
                        },
                        {       "guestId": "da172ae4-1b82-4900-975b-17ed24553f09",
                                "state": 1
                        },
                        {       "guestId": "ecf50029-a084-4c0e-813b-51f6b977f2f1",
                                "state": 1
                        }
                ]
        }]
}

• Register your system running virt-who to your devel box ( I just used my own devel box to register to itself)
• Start up virt-who
  • systemctl start virt-who
• Check in the production log to make sure you see the new error InvalidVirtWhoHost with the hostname virt-who-xxx like so
  ESC[32m2023-07-12T20:51:33ESC[0m [ESC[31mEESC[0m|ESC[36mbacESC[0m|7f084d2d] The hostname virt-who-$&-1 can contain only lowercase letters, numbers, dashes and dots according to RFC921, RFC952 and RFC1123 (Katello::Errors::InvalidVirtWhoHost)

[theforeman-bot]

Issues: #36599

[jeremylenz]

See comment below :)

[chris1984]

@jeremylenz let me know if this looks better?

I am still getting the desired result:

18:06:48 rails.1   | 2023-07-20T18:06:48 [E|bac|92627384] The hostname virt-who-$&-1 can contain only lowercase letters, numbers, dashes and dots
18:06:48 rails.1   |  92627384 |                                                         according to RFC921, RFC952 and RFC1123 (Katello::Errors::InvalidVirtWhoHost)
18:06:48 rails.1   |  92627384 | /home/vagrant/katello/app/models/katello/concerns/host_managed_extensions.rb:210:in `ensure_no_special_chars'

Based on the output though, I am more inclined to leave that as one who sentence since it has a lot of white space and putting a rubocop disable line length.

[jeremylenz]

let me know if this looks better?

A step in the right direction!

But should use errors.add, like in https://github.com/Katello/katello/blob/master/app/models/katello/root_repository.rb#L182C5-L187C8

This way you won't have to create a new error class (which isn't standard for validations), and it also may help you get around the long line length, since you can put a newline after each argument if you need to.

[chris1984]

@jeremylenz updated to using the errors.add method and shortened the error message so it's not a 1:1 dupe of the validation from Foreman.

[jeremylenz]

Validation code looks better now, thanks!

One more comment/question and then I'll leave it to another reviewer to do a functional test.

[chris1984]

@jeremylenz updated

[jeremylenz]

looks like some tests need updating ;) But LGTM from a code perspective. 👍

[chris1984]

@jeremylenz the tests are failing with the updated code, since this is not important (set to a target milestone) and I am not good with regex, just going to close this and put it on overflow for someone else to pickup. The hostnames in the test are correct.

[chris1984]

@jeremylenz how does this look? I still have a few places I need to update tests, but does it look fine for APJ? @sjha4 signed up to test it

[jeremylenz]

LGTM, APJ & Samir's test. 👍

[lfu]

Got error message with hostname displayed.

10:06:40 rails.1 | 2023-08-04T10:06:40 [E|bac|b157dd57] Validation failed: Name hostname can contain only lowercase letters, numbers, dashes and dots according to RFC921, RFC952 and RFC1123, Name hostname can contain only lowercase letters, numbers, dashes and dots according to RFC921, RFC952 and RFC1123, Name virt-who-$&-1 can contain only lowercase letters, numbers, dashes and dots. (ActiveRecord::RecordInvalid)

[lfu]

LGTM 👍🏻