[ARVADOS] updated: 2f46fb5769f41ac9cddf8c46bd0d2ab094375e82

[Git user]

Summary of changes:
 services/api/app/models/node.rb     | 10 ++++++++--
 services/api/test/unit/node_test.rb | 22 ++++++++++++++++------
 2 files changed, 24 insertions(+), 8 deletions(-)

       via  2f46fb5769f41ac9cddf8c46bd0d2ab094375e82 (commit)
      from  6da9f3666bc0ddee2d0be079cc30ba8b82804706 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 2f46fb5769f41ac9cddf8c46bd0d2ab094375e82
Author: Lucas Di Pentima <lucas at curoverse.com>
Date:   Mon Apr 10 13:51:28 2017 -0300

    6304: Clean up temp files that could exist in case of writing errors.
    Also added a test for this case, not working properly atm.

diff --git a/services/api/app/models/node.rb b/services/api/app/models/node.rb
index b57abfd..ca706d2 100644
--- a/services/api/app/models/node.rb
+++ b/services/api/app/models/node.rb
@@ -173,6 +173,7 @@ class Node < ArvadosModel
     }
 
     if Rails.configuration.dns_server_conf_dir and Rails.configuration.dns_server_conf_template
+      tmpfile = nil
       begin
         begin
           template = IO.read(Rails.configuration.dns_server_conf_template)
@@ -182,15 +183,20 @@ class Node < ArvadosModel
         end
 
         hostfile = File.join Rails.configuration.dns_server_conf_dir, "#{hostname}.conf"
-        tmpfile = Tempfile.open(["#{hostname}", ".conf.tmp"],
+        Tempfile.open(["#{hostname}", ".conf.tmp"],
                                  Rails.configuration.dns_server_conf_dir) do |f|
+          tmpfile = f.path
           f.puts template % template_vars
-          f.path
         end
         File.rename tmpfile, hostfile
       rescue IOError, SystemCallError => e
         logger.error "Writing #{hostfile}: #{e.message}"
         ok = false
+      ensure
+        if tmpfile and File.file? tmpfile
+          # Cleanup remaining temporary file.
+          File.unlink tmpfile
+        end
       end
     end
 
diff --git a/services/api/test/unit/node_test.rb b/services/api/test/unit/node_test.rb
index f9e0f4b..3895f04 100644
--- a/services/api/test/unit/node_test.rb
+++ b/services/api/test/unit/node_test.rb
@@ -1,4 +1,6 @@
 require 'test_helper'
+require 'tmpdir'
+require 'tempfile'
 
 class NodeTest < ActiveSupport::TestCase
   def ping_node(node_name, ping_data)
@@ -76,12 +78,20 @@ 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
+  test "don't leave temp files behind if there's an error writing them" do
+    Tempfile.any_instance.stubs(:puts).raises(IOError)
+    Dir.mktmpdir do |tmpdir|
+      Rails.configuration.dns_server_conf_dir = tmpdir
+      # This works
+      assert_raises IOError do
+        Tempfile.open(['testfile.txt']) do |f|
+          f.puts "This won't get written."
+        end
+      end
+      # This fails
+      refute Node.dns_server_update 'compute65535', '127.0.0.127'
+      assert Dir.entries(tmpdir).select{|f| File.file? f}.empty?
+    end
   end
 
   test "ping new node with no hostname and default config" do

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


hooks/post-receive
--