6304: Avoid possible dns update race condition by writing the temp file with unique...

Catch IO specific exceptions when trying to write the config files.
Added test to assert that no config file is written when the update command is not set up but the config dir is.

diff --git a/services/api/app/models/node.rb b/services/api/app/models/node.rb
index c7ac090adc3b9d7bc6b265707ac0c3b4973521e7..b57abfde91f84c20ad84bca562b93fa922e84e28 100644 (file)
--- a/services/api/app/models/node.rb
+++ b/services/api/app/models/node.rb
@@ -1,3 +1,5 @@
+require 'tempfile'
+
 class Node < ArvadosModel
   include HasUuid
   include KindAndEtag
@@ -174,17 +176,19 @@ class Node < ArvadosModel
       begin
         begin
           template = IO.read(Rails.configuration.dns_server_conf_template)
-        rescue => e
+        rescue IOError, SystemCallError => e
           logger.error "Reading #{Rails.configuration.dns_server_conf_template}: #{e.message}"
           raise
         end
 
         hostfile = File.join Rails.configuration.dns_server_conf_dir, "#{hostname}.conf"
-        File.open hostfile+'.tmp', 'w' do |f|
+        tmpfile = Tempfile.open(["#{hostname}", ".conf.tmp"],
+                                 Rails.configuration.dns_server_conf_dir) do |f|
           f.puts template % template_vars
+          f.path
         end
-        File.rename hostfile+'.tmp', hostfile
-      rescue => e
+        File.rename tmpfile, hostfile
+      rescue IOError, SystemCallError => e
         logger.error "Writing #{hostfile}: #{e.message}"
         ok = false
       end
@@ -205,7 +209,7 @@ class Node < ArvadosModel
           # Typically, this is used to trigger a dns server restart
           f.puts Rails.configuration.dns_server_reload_command
         end
-      rescue => e
+      rescue IOError, SystemCallError => e
         logger.error "Unable to write #{restartfile}: #{e.message}"
         ok = false
       end

diff --git a/services/api/test/unit/node_test.rb b/services/api/test/unit/node_test.rb
index c1e77f6a4d4cf78e6e3ac7ce77b4ffe5a2fd4c9e..f9e0f4bc08fe35bc2bd14afddeb6cd7699eb9e26 100644 (file)
--- a/services/api/test/unit/node_test.rb
+++ b/services/api/test/unit/node_test.rb
@@ -76,6 +76,14 @@ class NodeTest < ActiveSupport::TestCase
     assert Node.dns_server_update 'compute65535', '127.0.0.127'
   end
 
+  test "dns update with dir configured but no command configured" do
+    Rails.configuration.dns_server_update_command = false
+    Rails.configuration.dns_server_conf_dir = Rails.root.join 'tmp'
+    conffile = Rails.root.join 'tmp', 'compute65535.conf'
+    assert Node.dns_server_update 'compute65535', '127.0.0.127'
+    refute File.exist? conffile
+  end
+
   test "ping new node with no hostname and default config" do
     node = ping_node(:new_with_no_hostname, {})
     slot_number = node.slot_number