fixes #15590 - ipv6 tftp orchestration

app/models/concerns/orchestration/dhcp.rb

@@ -71,10 +71,10 @@ def del_dhcp_conflicts
 
   # where are we booting from
   def boot_server
-    # if we don't manage tftp at all, we dont create a next-server entry.
+    # if we don't manage tftp for IPv4 at all, we dont create a next-server entry.
     return unless tftp?
 
-    # first try to ask our TFTP server for its boot server
+    # first try to ask our IPv4 TFTP server for its boot server
     bs = tftp.bootServer
     # if that failed, trying to guess out tftp next server based on the smart proxy hostname
     bs ||= URI.parse(subnet.tftp.url).host

app/models/concerns/orchestration/tftp.rb

@@ -11,25 +11,38 @@ module Orchestration::TFTP
     register_rebuild(:rebuild_tftp, N_('TFTP'))
   end
 
+  def tftp_ready?
+    # host.managed? and managed? should always come first so that orchestration doesn't
+    # even get tested for such objects
+    (host.nil? || host.managed?) && managed && provision? && (host && host.operatingsystem && host.pxe_loader.present?) && pxe_build? && SETTINGS[:unattended]
+  end
+
   def tftp?
-    # host.managed? and managed? should always come first so that orchestration doesn't even get tested for such objects
-    (host.nil? || host.managed?) && managed && provision? && !!(subnet && subnet.tftp?) && (host && host.operatingsystem && host.pxe_loader.present?) && pxe_build? && SETTINGS[:unattended]
+    tftp_ready? && !!(subnet && subnet.tftp?)
+  end
+
+  def tftp6?
+    tftp_ready? && !!(subnet6 && subnet6.tftp?)
   end
 
   def tftp
     subnet.tftp_proxy if tftp?
   end
 
+  def tftp6
+    subnet6.tftp_proxy if tftp6?
+  end
+
   def rebuild_tftp
-    if tftp?
+    if tftp? || tftp6?
       begin
         setTFTP
       rescue => e
-        Foreman::Logging.exception "Failed to rebuild TFTP record for #{name}, #{ip}", e, :level => :error
+        Foreman::Logging.exception "Failed to rebuild TFTP record for #{name} (#{ip}/#{ip6})", e, :level => :error
         false
       end
     else
-      logger.info "TFTP not supported for #{name}, #{ip}, skipping orchestration rebuild"
+      logger.info "TFTP not supported for #{name} (#{ip}/#{ip6}), skipping orchestration rebuild"
       true
     end
   end
@@ -80,7 +93,9 @@ def setTFTP(kind)
     content = generate_pxe_template(kind)
     if content
       logger.info "Deploying TFTP #{kind} configuration for #{host.name}"
-      tftp.set kind, mac, :pxeconfig => content
+      each_unique_feasible_tftp_proxy do |proxy|
+        proxy.set(kind, mac, :pxeconfig => content)
+      end
     else
       logger.info "Skipping TFTP #{kind} configuration for #{host.name}"
       true
@@ -91,19 +106,23 @@ def setTFTP(kind)
   # +returns+ : Boolean true on success
   def delTFTP(kind)
     logger.info "Delete the TFTP configuration for #{host.name}"
-    tftp.delete kind, mac
+    each_unique_feasible_tftp_proxy do |proxy|
+      proxy.delete(kind, mac)
+    end
   end
 
   def setTFTPBootFiles
     logger.info "Fetching required TFTP boot files for #{host.name}"
-    valid = true
+    valid = []
     host.operatingsystem.pxe_files(host.medium, host.architecture, host).each do |bootfile_info|
       for prefix, path in bootfile_info do
-        valid = false unless tftp.fetch_boot_file(:prefix => prefix.to_s, :path => path)
+        valid << each_unique_feasible_tftp_proxy do |proxy|
+          proxy.fetch_boot_file(:prefix => prefix.to_s, :path => path)
+        end
       end
     end
-    failure _("Failed to fetch boot files") unless valid
-    valid
+    failure _("Failed to fetch boot files") unless valid.all?
+    valid.all?
   end
 
   #empty method for rollbacks
@@ -113,7 +132,7 @@ def delTFTPBootFiles
   private
 
   def validate_tftp
-    return unless tftp?
+    return unless tftp? || tftp6?
     return unless host.operatingsystem
     pxe_kind = host.operatingsystem.pxe_loader_kind(host)
     if pxe_kind && host.provisioning_template({:kind => pxe_kind}).nil?
@@ -123,7 +142,7 @@ def validate_tftp
   end
 
   def queue_tftp
-    return unless tftp? && no_errors
+    return unless (tftp? || tftp6?) && no_errors
     # Jumpstart builds require only minimal tftp services. They do require a tftp object to query for the boot_server.
     return true if host.jumpstart?
     new_record? ? queue_tftp_create : queue_tftp_update
@@ -156,7 +175,7 @@ def queue_tftp_update
 
   def queue_tftp_destroy(validate = true, priority = 20, host = self)
     if validate
-      return unless tftp? && no_errors
+      return unless (tftp? || tftp6?) && no_errors
       return true if host.jumpstart?
     end
     host.operatingsystem.template_kinds.each do |kind|
@@ -167,4 +186,18 @@ def queue_tftp_destroy(validate = true, priority = 20, host = self)
   def no_errors
     errors.empty? && host.errors.empty?
   end
+
+  def unique_feasible_tftp_proxies
+    proxies = []
+    proxies << tftp if tftp?
+    proxies << tftp6 if tftp6?
+    proxies.uniq { |p| p.url }
+  end
+
+  def each_unique_feasible_tftp_proxy
+    results = unique_feasible_tftp_proxies.map do |proxy|
+      yield(proxy)
+    end
+    results.all?
+  end
 end

app/models/host/managed.rb

@@ -176,7 +176,7 @@ class Jail < ::Safemode::Jail
     include Orchestration::Compute
     include Rails.application.routes.url_helpers
     # TFTP orchestration delegation
-    delegate :tftp?, :tftp, :generate_pxe_template, :to => :provision_interface
+    delegate :tftp?, :tftp6?, :tftp, :tftp6, :generate_pxe_template, :to => :provision_interface
     include Orchestration::Puppetca
     include Orchestration::SSHProvision
     include Orchestration::Realm

test/factories/host_related.rb

@@ -363,6 +363,10 @@ def set_nic_attributes(host, attributes, evaluator)
       subnet { FactoryGirl.build(:subnet_ipv4, :tftp, locations: [location], organizations: [organization]) }
     end
 
+    trait :with_tftp_v6_subnet do
+      subnet6 { FactoryGirl.build(:subnet_ipv6, :tftp, locations: [location], organizations: [organization]) }
+    end
+
     trait :with_tftp_orchestration do
       managed
       with_tftp_subnet
@@ -375,6 +379,33 @@ def set_nic_attributes(host, attributes, evaluator)
       end
     end
 
+    trait :with_tftp_v6_orchestration do
+      managed
+      with_tftp_v6_subnet
+      interfaces do
+        [FactoryGirl.build(:nic_managed, :primary => true,
+                                         :provision => true,
+                                         :domain => FactoryGirl.build(:domain),
+                                         :subnet6 => subnet6,
+                                         :ip6 => IPAddr.new(subnet6.ipaddr.to_i + 1, subnet6.family).to_s)]
+      end
+    end
+
+    trait :with_tftp_dual_stack_orchestration do
+      managed
+      with_tftp_subnet
+      with_tftp_v6_subnet
+      interfaces do
+        [FactoryGirl.build(:nic_managed, :primary => true,
+                                         :provision => true,
+                                         :domain => FactoryGirl.build(:domain),
+                                         :subnet => subnet,
+                                         :subnet6 => subnet6,
+                                         :ip => subnet.network.sub(/0\Z/, '2'),
+                                         :ip6 => IPAddr.new(subnet6.ipaddr.to_i + 1, subnet6.family).to_s)]
+      end
+    end
+
     trait :with_puppet_orchestration do
       managed
       environment

test/unit/orchestration/dhcp_test.rb

@@ -75,18 +75,18 @@ def teardown
     assert_equal '<Sun-Fire-V210>', d.vendor
   end
 
-  test "provision interface DHCP records should contain default filename/next-server attributes" do
+  test "provision interface DHCP records should contain default filename/next-server attributes for IPv4 tftp proxy" do
     ProxyAPI::TFTP.any_instance.expects(:bootServer).returns('192.168.1.1')
     subnet = FactoryGirl.build(:subnet_ipv4, :dhcp, :tftp)
-    h = FactoryGirl.create(:host, :with_dhcp_orchestration, :with_tftp_orchestration, :subnet => subnet)
+    h = FactoryGirl.create(:host, :with_dhcp_orchestration, :with_tftp_dual_stack_orchestration, :subnet => subnet)
     assert_equal 'grub2/grubx64.efi', h.provision_interface.dhcp_record.filename
     assert_equal '192.168.1.1', h.provision_interface.dhcp_record.nextServer
   end
 
-  test "provision interface DHCP records should contain explicit filename/next-server attributes" do
+  test "provision interface DHCP records should contain explicit filename/next-server attributes for IPv4 tftp proxy" do
     ProxyAPI::TFTP.any_instance.expects(:bootServer).returns('192.168.1.1')
     subnet = FactoryGirl.build(:subnet_ipv4, :dhcp, :tftp)
-    h = FactoryGirl.create(:host, :with_dhcp_orchestration, :with_tftp_orchestration, :subnet => subnet, :pxe_loader => 'PXELinux BIOS')
+    h = FactoryGirl.create(:host, :with_dhcp_orchestration, :with_tftp_dual_stack_orchestration, :subnet => subnet, :pxe_loader => 'PXELinux BIOS')
     assert_equal 'pxelinux.0', h.provision_interface.dhcp_record.filename
     assert_equal '192.168.1.1', h.provision_interface.dhcp_record.nextServer
   end

test/unit/orchestration/tftp_test.rb

@@ -3,28 +3,127 @@
 class TFTPOrchestrationTest < ActiveSupport::TestCase
   setup :disable_orchestration
 
-  test "host_should_have_tftp" do
-    if unattended?
-      h = FactoryGirl.build(:host, :managed, :with_tftp_orchestration)
-      assert h.tftp?
-      assert_not_nil h.tftp
+  context 'host without tftp orchestration' do
+    setup do
+      @host = FactoryGirl.create(:host)
+    end
+
+    test 'should not have any tftp' do
+      skip_without_unattended
+      assert_equal false, @host.tftp?
+      assert_equal false, @host.tftp6?
+      assert_equal nil, @host.tftp
+      assert_equal nil, @host.tftp6
+    end
+
+    test '#setTFTP should not call any tftp proxy' do
+      ProxyAPI::TFTP.any_instance.expects(:set).never
+      @host.provision_interface.stubs(:generate_pxe_template).returns('Template')
+      @host.provision_interface.send(:setTFTP, 'PXEGrub2')
+    end
+
+    test 'should not queue tftp' do
+      @host.provision_interface.send(:queue_tftp)
+      tasks = @host.queue.all.map { |t| t.name }
+      assert_empty tasks
     end
   end
 
-  test "host_should_not_have_tftp" do
-    if unattended?
-      h = FactoryGirl.create(:host)
-      assert_equal false, h.tftp?
-      assert_equal nil, h.tftp
+  context 'host with ipv4 tftp' do
+    setup do
+      @host = FactoryGirl.build(:host, :managed, :with_tftp_orchestration, :build => true)
+    end
+
+    test 'should have tftp' do
+      skip_without_unattended
+      assert @host.tftp?
+      refute @host.tftp6?
+      assert_not_nil @host.tftp
+      assert_nil @host.tftp6
+    end
+
+    test '#setTFTP should call one tftp proxy' do
+      ProxyAPI::TFTP.any_instance.expects(:set).once
+      @host.provision_interface.stubs(:generate_pxe_template).returns('Template')
+      @host.provision_interface.send(:setTFTP, 'PXEGrub2')
+    end
+
+    test 'should queue tftp' do
+      @host.provision_interface.send(:queue_tftp)
+      tasks = @host.queue.all.map { |t| t.name }
+      assert_includes tasks, "Deploy TFTP PXEGrub config for #{@host.provision_interface}"
+      assert_includes tasks, "Fetch TFTP boot files for #{@host.provision_interface}"
+    end
+
+    test "without pxe loader should not have tftp" do
+      skip_without_unattended
+      @host.expects(:pxe_loader).returns('').at_least(1)
+      assert_equal false, @host.tftp?
+      assert_equal nil, @host.tftp
     end
   end
 
-  test "host_without_pxe_loader_should_not_have_tftp" do
-    skip_without_unattended
-    h = FactoryGirl.build(:host, :managed, :with_tftp_orchestration)
-    h.expects(:pxe_loader).returns('').at_least(1)
-    assert_equal false, h.tftp?
-    assert_equal nil, h.tftp
+  context 'host with ipv6 tftp' do
+    setup do
+      @host = FactoryGirl.build(:host, :managed, :with_tftp_v6_orchestration, :build => true)
+    end
+
+    test "should have ipv6 tftp" do
+      skip_without_unattended
+      refute @host.tftp?
+      assert @host.tftp6?
+      assert_nil @host.tftp
+      assert_not_nil @host.tftp6
+      assert_nil @host.subnet
+    end
+
+    test '#setTFTP should call one tftp proxy' do
+      ProxyAPI::TFTP.any_instance.expects(:set).once
+      @host.provision_interface.stubs(:generate_pxe_template).returns('Template')
+      @host.provision_interface.send(:setTFTP, 'PXEGrub2')
+    end
+
+    test 'should queue tftp' do
+      @host.provision_interface.send(:queue_tftp)
+      tasks = @host.queue.all.map { |t| t.name }
+      assert_includes tasks, "Deploy TFTP PXEGrub config for #{@host.provision_interface}"
+      assert_includes tasks, "Fetch TFTP boot files for #{@host.provision_interface}"
+    end
+  end
+
+  context 'host with ipv4 and ipv6 tftp' do
+    setup do
+      @host = FactoryGirl.build(:host, :managed, :with_tftp_dual_stack_orchestration, :build => true)
+    end
+
+    test "host should have ipv4 and ipv6 tftp" do
+      skip_without_unattended
+      assert @host.tftp?
+      assert @host.tftp6?
+      assert_not_nil @host.tftp
+      assert_not_nil @host.tftp6
+    end
+
+    test '#setTFTP should call both tftp proxies' do
+      ProxyAPI::TFTP.any_instance.expects(:set).twice
+      @host.provision_interface.stubs(:generate_pxe_template).returns('Template')
+      @host.provision_interface.send(:setTFTP, 'PXEGrub2')
+    end
+
+    test '#setTFTP should call just one proxy if the proxies are unique' do
+      ProxyAPI::TFTP.any_instance.expects(:set).once
+      @host.provision_interface.stubs(:generate_pxe_template).returns('Template')
+      @host.provision_interface.subnet6.tftp = @host.provision_interface.subnet.tftp
+      assert @host.provision_interface.subnet6.save!
+      @host.provision_interface.send(:setTFTP, 'PXEGrub2')
+    end
+
+    test 'should queue tftp' do
+      @host.provision_interface.send(:queue_tftp)
+      tasks = @host.queue.all.map { |t| t.name }
+      assert_includes tasks, "Deploy TFTP PXEGrub config for #{@host.provision_interface}"
+      assert_includes tasks, "Fetch TFTP boot files for #{@host.provision_interface}"
+    end
   end
 
   test 'unmanaged should not call methods after managed?' do
@@ -79,10 +178,16 @@ class TFTPOrchestrationTest < ActiveSupport::TestCase
     end
   end
 
-  test "should_rebuild_tftp" do
-    h = FactoryGirl.create(:host, :with_tftp_orchestration)
+  test 'should rebuild tftp IPv4' do
+    host = FactoryGirl.create(:host, :with_tftp_orchestration)
+    Nic::Managed.any_instance.expects(:setTFTP).returns(true)
+    assert host.interfaces.first.rebuild_tftp
+  end
+
+  test 'should rebuild tftp IPv6' do
+    host = FactoryGirl.create(:host, :with_tftp_v6_orchestration)
     Nic::Managed.any_instance.expects(:setTFTP).returns(true)
-    assert h.interfaces.first.rebuild_tftp
+    assert host.interfaces.first.rebuild_tftp
   end
 
   describe "validation" do