Fixes #13248 - rely on foreman fact importer to import interfaces #5713
Conversation
| end | ||
|
|
||
| # rubocop:disable Style/AccessorMethodName: | ||
| def get_interfaces |
There was a problem hiding this comment.
overrides the base model
| @@ -245,7 +243,7 @@ def server_status | |||
| def facts | |||
| User.as_anonymous_admin do | |||
| sync_task(::Actions::Katello::Host::Update, @host, rhsm_params) | |||
| @host.subscription_facet.update_facts(rhsm_params[:facts]) unless rhsm_params[:facts].blank? | |||
| Katello::Host::SubscriptionFacet.update_facts(@host, rhsm_params[:facts]) | |||
There was a problem hiding this comment.
Are we not assured the host has a subscription facet here?
There was a problem hiding this comment.
we are in the find_host method
There was a problem hiding this comment.
I don't see why we'd call a class method to then call methods on the object instead of having an instance method on subscription_facet like the original line had?
There was a problem hiding this comment.
The reason is because facts have to be imported before the host is created. Maybe is hould move this to the RhsmFactImporter class ? Probably makes more sense there if its not on an instance
There was a problem hiding this comment.
I am confused -- if you have @host here, how is it not "created" ?
There was a problem hiding this comment.
Here it is created, here it is not: https://github.com/Katello/katello/pull/5713/files#diff-50713e26c68a3af05393d936d0f19328R10
There was a problem hiding this comment.
In the code you linked, where is the host created? At the save (https://github.com/Katello/katello/pull/5713/files#diff-50713e26c68a3af05393d936d0f19328R25) ?
There was a problem hiding this comment.
Actually is it saved to the db as part of the fact import, if any error occurs within the plan() phase, it rolls it all back.
|
To test this, I need a VM without an IP address? Or do you have another verification scenario? |
|
@ehelms to test one of the possibly failure conditions yes, you need a box that has at least one interface without an ipadress. So a VM with an extra nic that isn't configured, or a desktop with a wireless interface that is disabled, anything like that. |
theforeman-bot
added
Needs testing
Needs re-review
and removed
Waiting on contributor
labels
This fixes a variety of issues around registration, including: * Registering fedora 20 (and possibly newer) machines * proper selection of a primary interface * proper detection of bringed and virt interfaces * No longer creates an empty primary interface on host creation
|
@ehelms any more thoughts on this? |
|
ACK |
Fixes #13248 - rely on foreman fact importer to import interfaces
This fixes a variety of issues around registration, including: