Fixes #6952 - Set hostgroup after provisioning from pxe #4112
Conversation
|
@amirfefer, thanks for your PR! By analyzing the history of the files in this pull request, we identified @ares, @GregSutcliffe and @isratrade to be potential reviewers. |
There was a problem hiding this comment.
It's nearly done. Just call set_hostgroup_defaults and please add something to theforeman.org documenting what this does, as foreman_hostgroup isn't provided by Ansible, Puppet etc...
Thanks @amirfefer
| @@ -120,6 +120,7 @@ def import_facts(facts) | |||
|
|
|||
| facts[:domain].try(:downcase!) | |||
|
|
|||
| hostgroup_title = facts.delete(:foreman_hostgroup) | |||
There was a problem hiding this comment.
This fact isn't standard, from what I read on the issue description it's a custom fact. I think the functionality is neat, but please add docs to this after it's merged
| @@ -209,6 +211,10 @@ def ==(comparison_object) | |||
| comparison_object.id == id | |||
| end | |||
|
|
|||
| def set_hostgroup(title) | |||
| self.hostgroup = Hostgroup.find_by_title(title) | |||
There was a problem hiding this comment.
Call set_hostgroup_defaults after, so that the attributes from the host group are applied
theforeman-bot
added
Needs re-review
Needs testing
and removed
Waiting on contributor
labels
| facts['foreman_hostgroup'] = hostgroup.title | ||
| post :facts, {:name => hostname, :facts => facts} | ||
| assert_response :success | ||
| assert_equal hostgroup.root_pass, Host.find_by_name(hostname).root_pass |
There was a problem hiding this comment.
Use find_by instead of dynamic find_by_name.
| hostname = fact_json['name'] | ||
| facts = fact_json['facts'] | ||
| facts['foreman_hostgroup'] = hostgroup.title | ||
| post :facts, {:name => hostname, :facts => facts} |
There was a problem hiding this comment.
Use keyword arguments instead of positional arguments for http call: post.
| facts['foreman_hostgroup'] = hostgroup.title | ||
| post :facts, {:name => hostname, :facts => facts} | ||
| assert_response :success | ||
| assert_equal hostgroup, Host.find_by_name(hostname).hostgroup |
There was a problem hiding this comment.
Use find_by instead of dynamic find_by_name.
| hostname = fact_json['name'] | ||
| facts = fact_json['facts'] | ||
| facts['foreman_hostgroup'] = hostgroup.title | ||
| post :facts, {:name => hostname, :facts => facts} |
There was a problem hiding this comment.
Use keyword arguments instead of positional arguments for http call: post.
| @@ -209,6 +211,11 @@ def ==(comparison_object) | |||
| comparison_object.id == id | |||
| end | |||
|
|
|||
| def set_hostgroup(title) | |||
| self.hostgroup = Hostgroup.find_by_title(title) | |||
There was a problem hiding this comment.
Use find_by instead of dynamic find_by_title.
After hostgroup provisioning, when the machine has registered with foreman, it should be in the same hostgroup. to achieve this behavier, the referenced snippet should be added to hostgroup provisioning template
|
Thanks for your review @dLobatog, I've added |
|
@amirfefer I'll fix the HoundCI qualms on merge, remember to write some docs for this on https://github.com/theforeman/theforeman.org/ please. Thanks! |
|
Merged as 9883149 , thanks @amirfefer ! |
After hostgroup provisioning, when the machine has registered
with foreman, it should be in the same hostgroup.
to achieve this behavier, the referenced snippet should be added
to hostgroup provisioning template.