fixes #16951 - ipv6 compute orchestration

app/helpers/hosts_helper.rb

@@ -257,6 +257,7 @@ def overview_fields(host)
     fields += [[_("Domain"), (link_to(host.domain, hosts_path(:search => "domain = #{host.domain}")))]] if host.domain.present?
     fields += [[_("Realm"), (link_to(host.realm, hosts_path(:search => "realm = #{host.realm}")))]] if host.realm.present?
     fields += [[_("IP Address"), host.ip]] if host.ip.present?
+    fields += [[_("IPv6 Address"), host.ip6]] if host.ip6.present?
     fields += [[_("MAC Address"), host.mac]] if host.mac.present?
     fields += [[_("Puppet Environment"), (link_to(host.environment, hosts_path(:search => "environment = #{host.environment}")))]] if host.environment.present?
     fields += [[_("Host Architecture"), (link_to(host.arch, hosts_path(:search => "architecture = #{host.arch}")))]] if host.arch.present?

app/models/concerns/orchestration/compute.rb

@@ -33,6 +33,10 @@ def ip_available?
     ip.present? || compute_provides?(:ip)
   end
 
+  def ip6_available?
+    ip6.present? || compute_provides?(:ip6)
+  end
+
   def mac_available?
     mac.present? || compute_provides?(:mac)
   end
@@ -53,8 +57,8 @@ def queue_compute_create
                  :action => [self, :setUserData]) if find_image.try(:user_data)
     queue.create(:name   => _("Set up compute instance %s") % self, :priority => 2,
                  :action => [self, :setCompute])
-    queue.create(:name   => _("Acquire IP address for %s") % self, :priority => 3,
-                 :action => [self, :setComputeIP]) if compute_provides?(:ip)
+    queue.create(:name   => _("Acquire IP addresses for %s") % self, :priority => 3,
+                 :action => [self, :setComputeIP]) if compute_provides?(:ip) || compute_provides?(:ip6)
     queue.create(:name   => _("Query instance details for %s") % self, :priority => 4,
                  :action => [self, :setComputeDetails])
     queue.create(:name   => _("Power up compute instance %s") % self, :priority => 1000,
@@ -124,16 +128,21 @@ def setComputeDetails
       attrs.each do |foreman_attr, fog_attr|
         if foreman_attr == :mac
           return false unless match_macs_to_nics(fog_attr)
-        elsif foreman_attr == :ip
-          value = vm.send(fog_attr) || find_address
+        elsif [:ip, :ip6].include?(foreman_attr)
+          value = vm.send(fog_attr) || find_address(foreman_attr)
           self.send("#{foreman_attr}=", value)
-          return false unless validate_foreman_attr(value, ::Nic::Base, foreman_attr)
+          return false if self.send(foreman_attr).present? && !validate_foreman_attr(value, ::Nic::Base, foreman_attr)
         else
           value = vm.send(fog_attr)
           self.send("#{foreman_attr}=", value)
-          return false unless validate_foreman_attr(value, Host, foreman_attr)
+          return false unless validate_required_foreman_attr(value, Host, foreman_attr)
         end
       end
+
+      if ip.blank? && ip6.blank? && (compute_provides?(:ip) || compute_provides?(:ip6))
+        return failure(_("Failed to acquire IP addresses from compute resource for %s") % name)
+      end
+
       true
     else
       failure _("failed to save %s") % name
@@ -144,12 +153,14 @@ def delComputeDetails; end
 
   def setComputeIP
     attrs = compute_resource.provided_attributes
-    if attrs.keys.include?(:ip)
+    if attrs.keys.include?(:ip) || attrs.keys.include?(:ip6)
       logger.info "Waiting for #{name} to become ready"
       vm.wait_for { self.ready? }
       logger.info "waiting for instance to acquire ip address"
       vm.wait_for do
-        self.send(attrs[:ip]).present? || self.ip_addresses.present?
+        (attrs.keys.include?(:ip) && self.send(attrs[:ip]).present?) ||
+          (attrs.keys.include?(:ip6) && self.send(attrs[:ip6]).present?) ||
+          self.ip_addresses.present?
       end
     end
   rescue => e
@@ -225,19 +236,25 @@ def validate_compute_provisioning
     end
   end
 
-  def find_address
+  def find_address(type)
+    vm_addresses = filter_ip_addresses(vm.ip_addresses, type)
+
+    # We can exit early if the host already has any kind of ip and the vm does not
+    # provide one for this kind to speed up things
+    return if (ip.present? || ip6.present?) && vm_addresses.empty?
+
     # We need to return fast for user-data, so that we save the host before
     # cloud-init finishes, even if the IP is not reachable by Foreman. We do have
     # to return a real IP though, or Foreman will fail to save the host.
-    return vm.ip_addresses.first if (vm.ip_addresses.present? && self.compute_attributes[:user_data].present?)
+    return vm_addresses.first if (vm_addresses.present? && self.compute_attributes[:user_data].present?)
 
     # Loop over the addresses waiting for one to come up
     ip = nil
     begin
       Timeout.timeout(120) do
         until ip
-          ip = vm.ip_addresses.find { |addr| ssh_open?(addr) }
-          sleep 2
+          ip = filter_ip_addresses(vm.ip_addresses, type).detect { |addr| ssh_open?(addr) }
+          sleep 2 unless ip
         end
       end
     rescue Timeout::Error
@@ -246,12 +263,17 @@ def find_address
       # images do require an IP, but it's more accurate to return something here
       # if we have it, and let the SSH orchestration fail (and notify) for an
       # unreachable IP
-      ip = vm.ip_addresses.first if ip.blank?
-      logger.info "acquisition of ip address timed out, using #{ip}"
+      ip = filter_ip_addresses(vm.ip_addresses, type).first if ip.blank?
+      logger.info "acquisition of #{type} address timed out, using #{ip}"
     end
     ip
   end
 
+  def filter_ip_addresses(addresses, type)
+    check_method = type == :ip6 ? :ipv6? : :ipv4?
+    addresses.map {|ip| IPAddr.new(ip) rescue nil}.compact.select(&check_method).map(&:to_s)
+  end
+
   def ssh_open?(ip)
     begin
       Timeout.timeout(1) do
@@ -269,20 +291,23 @@ def ssh_open?(ip)
     false
   end
 
-  def validate_foreman_attr(value,object,attr)
+  def validate_foreman_attr(value, object, attr)
     # we can't ensure uniqueness of #foreman_attr using normal rails
     # validations as that gets in a later step in the process
     # therefore we must validate its not used already in our db.
-    if value.blank?
-      delCompute
-      return failure("#{attr} value is blank!")
-    elsif (other_object = object.send("find_by_#{attr}", value))
-      delCompute
+    if (other_object = object.send("find_by_#{attr}", value))
       return failure("#{attr} #{value} is already used by #{other_object}")
     end
     true
   end
 
+  def validate_required_foreman_attr(value, object, attr)
+    if value.blank?
+      return failure("#{attr} value is blank!")
+    end
+    validate_foreman_attr(value, object, attr)
+  end
+
   def match_macs_to_nics(fog_attr)
     # mac/ip are properties of the NIC, and there may be more than one,
     # so we need to loop. First store the nics returned from Fog in a local
@@ -309,7 +334,7 @@ def match_macs_to_nics(fog_attr)
 
       # validate_foreman_attr handles the failure msg, so we just bubble
       # the false state up the stack
-      return false unless validate_foreman_attr(mac,Nic::Base.physical,:mac)
+      return false unless validate_required_foreman_attr(mac,Nic::Base.physical,:mac)
     end
     true
   end

app/models/nic/base.rb

@@ -168,10 +168,23 @@ def require_ip6_validation?
       require_ip_validation?(:ip6, :ip, subnet6)
     end
 
+    # Overwrite setter for ip to force normalization
+    # even when address is set during a callback
+    def ip=(addr)
+      super(Net::Validations.normalize_ip(addr))
+    end
+
+    # Overwrite setter for ip6 to force normalization
+    # even when address is set during a callback
+    def ip6=(addr)
+      super(Net::Validations.normalize_ip6(addr))
+    end
+
     protected
 
     def normalize_mac
       self.mac = Net::Validations.normalize_mac(mac)
+      true
     rescue Net::Validations::Error => e
       self.errors.add(:mac, e.message)
     end

test/models/nics/base_test.rb

@@ -156,4 +156,25 @@ class Nic::BaseTest < ActiveSupport::TestCase
       end
     end
   end
+
+  describe 'normalization' do
+    let(:host) { FactoryGirl.build(:host) }
+    let(:nic) { FactoryGirl.build(:nic_base, :host => host) }
+
+    test 'it normalizes ipv4 address' do
+      nic.ip = '001.001.001.001'
+      assert_equal '1.1.1.1', nic.ip
+    end
+
+    test 'it normalizes ipv6 address' do
+      nic.ip6 = '2001:0db8:0000:0000:0000::0001'
+      assert_equal '2001:db8::1', nic.ip6
+    end
+
+    test 'it normalizes mac' do
+      nic.mac = 'aa-bb-cc-dd-ee-ff'
+      assert_valid nic # normalization is done before validation
+      assert_equal 'aa:bb:cc:dd:ee:ff', nic.mac
+    end
+  end
 end

test/models/orchestration/compute_test.rb

@@ -18,9 +18,41 @@ class ComputeOrchestrationTest < ActiveSupport::TestCase
       an_ip = '1.2.3.4'
       @cr.stubs(:provided_attributes).returns({:ip => :ip})
       @host.vm.expects(:ip).returns(an_ip)
-      @host.expects(:ip=).returns(an_ip)
-      @host.expects(:validate_foreman_attr).returns(true)
-      assert(@host.send :setComputeDetails)
+      assert @host.send(:setComputeDetails), "Failed to setComputeDetails, errors: #{@host.errors.full_messages}"
+      assert_equal an_ip, @host.ip
+    end
+
+    test "are set for CR providing IP via find_addresses" do
+      an_ip = '1.2.3.4'
+      @cr.stubs(:provided_attributes).returns({:ip => :ip})
+      @host.vm.stubs(:ip_addresses).returns([an_ip])
+      @host.vm.expects(:ip).returns(nil)
+      @host.expects(:ssh_open?).at_least_once.with(an_ip).returns(true)
+      @host.stubs(:compute_attributes).returns({})
+      assert @host.send(:setComputeDetails), "Failed to setComputeDetails, errors: #{@host.errors.full_messages}"
+      assert_equal an_ip, @host.ip
+    end
+
+    test "are set for CR providing IPv6" do
+      an_ip6 = '2001:db8::1'
+      @cr.stubs(:provided_attributes).returns({:ip6 => :ip6})
+      @host.vm.expects(:ip6).returns(an_ip6)
+      assert @host.send(:setComputeDetails), "Failed to setComputeDetails, errors: #{@host.errors.full_messages}"
+      assert_equal an_ip6, @host.ip6
+    end
+
+    test "are set for CR returning a blank value for ip6 while claiming to provide ip4 + ip6" do
+      # Create a host with ip6 = nil to test that validate_foreman_attr does not detect a duplicate ip
+      FactoryGirl.create(:host)
+
+      an_ip = '1.2.3.4'
+      @cr.stubs(:provided_attributes).returns({:ip => :ip, :ip6 => :ip6})
+      @host.vm.stubs(:ip_addresses).returns([an_ip])
+      @host.vm.expects(:ip).returns(an_ip)
+      @host.vm.expects(:ip6).returns(nil)
+      assert @host.send(:setComputeDetails), "Failed to setComputeDetails, errors: #{@host.errors.full_messages}"
+      assert_equal an_ip, @host.ip
+      assert_nil @host.ip6
     end
 
     test "are set for CR providing an unknown attribute" do

test/unit/net/validations_test.rb

@@ -206,4 +206,12 @@ class ValidationsTest < ActiveSupport::TestCase
   test "should ignore invalid data when normalizing IPv4 address" do
     assert_equal "xyz.1.2.3", Net::Validations.normalize_ip("xyz.1.2.3")
   end
+
+  test "should normalize IPv6 address" do
+    assert_equal '2001:db8::1', Net::Validations.normalize_ip6('2001:0db8:0000:0000::0001')
+  end
+
+  test "should ignore invalid data when normalizing IPv6 address" do
+    assert_equal 'invalid', Net::Validations.normalize_ip6('invalid')
+  end
 end