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.

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

Fixes #16105 - Force DNS rebuild when provisioning discovered host #297

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions app/models/nic/managed_extensions.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
module Nic::ManagedExtensions
extend ActiveSupport::Concern

included do
after_validation :discovery_queue_rebuild_dns
end

def discovery_queue_rebuild_dns
return unless (dns? || dns6? || reverse_dns? || reverse_dns6?) && errors.empty? && Setting[:discovery_always_rebuild_dns]
return if self.host.new_record? # Discovered Hosts already exist, and new_records will break `find`
return unless self.host.type_changed? and ::Host::Base.find(self.host.id).type == "Host::Discovered"
return if self.pending_dns_record_changes?
logger.debug "Queuing DNS rebuild for #{self}"
queue.create(:name => _("Rebuild DNS for %s") % self, :priority => 10, :action => [self, :set_discovery_rebuild_dns])
end

def set_discovery_rebuild_dns
logger.debug "Executing DNS rebuild for #{self}"
rebuild_dns
end

def del_discovery_rebuild_dns
# Just a place holder, so if rollback is needed, we won't get an exception about not knowing how to rollback.
# This is a no-op, since we cannot really rollback rebuild_dns
end
end
1 change: 1 addition & 0 deletions app/models/setting/discovered.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ def self.load_defaults
self.set('discovery_facts_ipmi', N_("Regex to organize facts for ipmi section"), "", N_("IPMI facts")),
self.set('discovery_lock', N_("Automatically generate PXE configuration to pin a newly discovered host to discovery"), false, N_("Lock PXE")),
self.set('discovery_lock_template', N_("PXE template to be used when pinning a host to discovery"), 'pxelinux_discovery', N_("Locked template name"),nil,{ :collection => Proc.new {Hash[ProvisioningTemplate.all.map{|template| [template[:name], template[:name]]}]} }),
self.set('discovery_always_rebuild_dns', N_("Force DNS entries creation when provisioning discovered host"), true, N_("Force DNS")),
Copy link
Member

Choose a reason for hiding this comment

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

Looks great. Why this is enabled by default? Normally, discovered hosts should not be added to DNS, thus I'd set this to false.

Copy link
Member Author

Choose a reason for hiding this comment

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

This only affects when you provision the host, not when it enters discovery, and in that case you always expect DNS to be created.

Copy link
Member

Choose a reason for hiding this comment

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

This only affects when you provision the host, not when it enters discovery, and in that case you always expect DNS to be created.

Disregard my comment, this should be turned on by default.

Later,
Lukas #lzap Zapletal

].compact.each { |s| self.create s.update(:category => "Setting::Discovered")}
end

Expand Down
1 change: 1 addition & 0 deletions lib/foreman_discovery/engine.rb
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ class Engine < ::Rails::Engine
::Host::Managed.send :include, Host::ManagedExtensions
::Hostgroup.send :include, HostgroupExtensions
::FactValue.send :include, FactValueExtensions
::Nic::Managed.send :include, Nic::ManagedExtensions

# Controller extensions
::Api::V2::FactValuesController.send :include, Api::V2::FactValuesControllerExtensions
Expand Down
52 changes: 52 additions & 0 deletions test/functional/discovered_hosts_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ class DiscoveredHostsControllerTest < ActionController::TestCase
"physicalprocessorcount" => "42",
"discovery_version" => "3.0.0",
}
FactoryGirl.create(:setting,
:name => 'discovery_always_rebuild_dns',
:value => true,
:category => 'Setting::Discovered')
FactoryGirl.create(:setting,
:name => 'discovery_reboot',
:value => true,
Expand Down Expand Up @@ -268,9 +272,57 @@ def test_reboot_all_error
assert_nil flash[:notice]
end

def test_no_dns_rebuild_if_dns_pending
host = Host::Discovered.import_host(@facts)
hostgroup = prepare_hostgroup_for_dns_rebuild(host)
Nic::Managed.any_instance.expects(:rebuild_dns).never
Host::Managed.any_instance.stubs(:skip_orchestration?).returns(false)
put :update, {:commit => "Update", :id => host.id,
:host => {
:name => 'mytest',
:hostgroup_id => hostgroup.id,
}
}, set_session_user_default_manager
end

def test_dns_rebuild
host = prepare_host_for_dns_rebuild
hostgroup = prepare_hostgroup_for_dns_rebuild(host)
Nic::Managed.any_instance.expects(:rebuild_dns)
Host::Managed.any_instance.stubs(:skip_orchestration?).returns(false)
put :update, {:commit => "Update", :id => host.id,
:host => {
:name => 'mytest',
:hostgroup_id => hostgroup.id,
}
}, set_session_user_default_manager
end

def test_dns_rebuild_with_auto_provision
host = prepare_host_for_dns_rebuild
hostgroup = prepare_hostgroup_for_dns_rebuild(host)
Nic::Managed.any_instance.expects(:rebuild_dns)
Host::Managed.any_instance.stubs(:skip_orchestration?).returns(false)
FactoryGirl.create(:discovery_rule, :priority => 1, :search => "name = mytest.myorchdomain.net", :hostgroup_id => hostgroup.id, :organizations => [host.organization], :locations => [host.location])
post :auto_provision, { :id => host.id }, set_session_user_default_manager
end

private

def initialize_host
User.current = users(:admin)
end

def prepare_host_for_dns_rebuild
host = Host::Discovered.import_host(@facts)
host.name = 'mytest.myorchdomain.net'
host.save
host
end

def prepare_hostgroup_for_dns_rebuild(host)
hostgroup = setup_hostgroup(host)
hostgroup.domain = FactoryGirl.create(:domain, :name => 'myorchdomain.net', :dns => FactoryGirl.create(:smart_proxy, :features => [FactoryGirl.create(:feature, :dns)]))
hostgroup
end
end