[ARVADOS] created: ecd8381224f7883db0504eb338d7f1c55fc4349f

[Git user]

        at  ecd8381224f7883db0504eb338d7f1c55fc4349f (commit)


commit ecd8381224f7883db0504eb338d7f1c55fc4349f
Author: Tom Clegg <tom at curoverse.com>
Date:   Mon Jan 30 15:17:52 2017 -0500

    10868: Remove old DNS entry immediately when a new node re-uses an old node's IP address.

diff --git a/services/api/app/models/node.rb b/services/api/app/models/node.rb
index 1855020..e0cdda1 100644
--- a/services/api/app/models/node.rb
+++ b/services/api/app/models/node.rb
@@ -139,23 +139,21 @@ class Node < ArvadosModel
   end
 
   def dns_server_update
-    if hostname_changed? && hostname_was
+    if ip_address_changed? && ip_address
+      Node.where('id != ? and ip_address = ?',
+                 id, ip_address).each do |stale_node|
+        # One or more(!) stale node records have the same IP address
+        # as the new node. Clear the ip_address field on the stale
+        # nodes. Otherwise, we (via SLURM) might inadvertently connect
+        # to the new node using the old node's hostname.
+        stale_node.update_attributes!(ip_address: nil)
+      end
+    end
+    if hostname_was && hostname_changed?
       self.class.dns_server_update(hostname_was, UNUSED_NODE_IP)
     end
-    if hostname_changed? or ip_address_changed?
-      if ip_address
-        Node.where('id != ? and ip_address = ? and last_ping_at < ?',
-                   id, ip_address, 10.minutes.ago).each do |stale_node|
-          # One or more stale compute node records have the same IP
-          # address as the new node.  Clear the ip_address field on
-          # the stale nodes.
-          stale_node.ip_address = nil
-          stale_node.save!
-        end
-      end
-      if hostname
-        self.class.dns_server_update(hostname, ip_address || UNUSED_NODE_IP)
-      end
+    if hostname && (hostname_changed? || ip_address_changed?)
+      self.class.dns_server_update(hostname, ip_address || UNUSED_NODE_IP)
     end
   end
 
diff --git a/services/api/test/unit/node_test.rb b/services/api/test/unit/node_test.rb
index df8c22b..c1e77f6 100644
--- a/services/api/test/unit/node_test.rb
+++ b/services/api/test/unit/node_test.rb
@@ -152,4 +152,31 @@ class NodeTest < ActiveSupport::TestCase
       node.update_attributes!(hostname: 'foo0', ip_address: '10.11.12.14')
     end
   end
+
+  test 'newest ping wins IP address conflict' do
+    act_as_system_user do
+      n1, n2 = Node.create!, Node.create!
+
+      n1.ping(ip: '10.5.5.5', ping_secret: n1.info['ping_secret'])
+      n1.reload
+
+      Node.expects(:dns_server_update).with(n1.hostname, Node::UNUSED_NODE_IP)
+      Node.expects(:dns_server_update).with(Not(equals(n1.hostname)), '10.5.5.5')
+      n2.ping(ip: '10.5.5.5', ping_secret: n2.info['ping_secret'])
+
+      n1.reload
+      n2.reload
+      assert_nil n1.ip_address
+      assert_equal '10.5.5.5', n2.ip_address
+
+      Node.expects(:dns_server_update).with(n2.hostname, Node::UNUSED_NODE_IP)
+      Node.expects(:dns_server_update).with(n1.hostname, '10.5.5.5')
+      n1.ping(ip: '10.5.5.5', ping_secret: n1.info['ping_secret'])
+
+      n1.reload
+      n2.reload
+      assert_nil n2.ip_address
+      assert_equal '10.5.5.5', n1.ip_address
+    end
+  end
 end

-----------------------------------------------------------------------


hooks/post-receive
--