Merge branch '11349-nodemanager-status-api'
authorTom Clegg <tom@curoverse.com>
Tue, 11 Apr 2017 15:33:17 +0000 (11:33 -0400)
committerTom Clegg <tom@curoverse.com>
Tue, 11 Apr 2017 15:33:17 +0000 (11:33 -0400)
refs #11349

build/build.list
doc/_includes/_navbar_top.liquid
sdk/cwl/arvados_cwl/arvcontainer.py
sdk/cwl/arvados_cwl/pathmapper.py
sdk/cwl/tests/arvados-tests.sh
sdk/python/arvados/__init__.py
sdk/python/arvados/api.py
sdk/python/setup.py
services/api/app/models/node.rb
services/api/test/unit/node_test.rb

index c9cdacabe1071513a1e6d2b1d97922f1ff93bdef..e163167a3a086fcdee675640bc2f45dca6c21501 100644 (file)
@@ -1,7 +1,7 @@
 #distribution(s)|name|version|iteration|type|architecture|extra fpm arguments
 debian8,ubuntu1204,centos7|python-gflags|2.0|2|python|all
-debian8,ubuntu1204,ubuntu1404,ubuntu1604,centos7|google-api-python-client|1.4.2|2|python|all
-debian8,ubuntu1204,ubuntu1404,ubuntu1604,centos7|oauth2client|1.5.2|2|python|all
+debian8,ubuntu1204,ubuntu1404,ubuntu1604,centos7|google-api-python-client|1.6.2|2|python|all
+debian8,ubuntu1204,ubuntu1404,centos7|oauth2client|1.5.2|2|python|all
 debian8,ubuntu1204,ubuntu1404,centos7|pyasn1|0.1.7|2|python|all
 debian8,ubuntu1204,ubuntu1404,centos7|pyasn1-modules|0.0.5|2|python|all
 debian8,ubuntu1204,ubuntu1404,ubuntu1604,centos7|rsa|3.4.2|2|python|all
index 6caf36a18882115027c288717a74146b8281dd49..4fd1edefe455155fcc11b2b094c7b7a6a4fcb3bd 100644 (file)
@@ -20,7 +20,7 @@
       </ul>
 
       <div class="pull-right" style="padding-top: 6px">
-        <form method="get" action="http://www.google.com/search">
+        <form method="get" action="https://www.google.com/search">
           <div class="input-group" style="width: 220px">
             <input type="text" class="form-control" name="q" placeholder="search">
             <div class="input-group-addon">
index 3090936121daf32d9ffa61744d1f5b932b73590a..28107b491ca45f760813c03cefa63bb9c693b331 100644 (file)
@@ -1,6 +1,7 @@
 import logging
 import json
 import os
+import urllib
 
 import ruamel.yaml as yaml
 
@@ -77,7 +78,7 @@ class ArvadosContainer(object):
                     "portable_data_hash": pdh
                 }
                 if len(sp) == 2:
-                    mounts[p]["path"] = sp[1]
+                    mounts[p]["path"] = urllib.unquote(sp[1])
 
         with Perf(metrics, "generatefiles %s" % self.name):
             if self.generatefiles["listing"]:
index 713162f66ac8db6593161990b979d170247f9283..a8619a8598a538d5ba7353390bc63e316a76a648 100644 (file)
@@ -2,6 +2,7 @@ import re
 import logging
 import uuid
 import os
+import urllib
 
 import arvados.commands.run
 import arvados.collection
@@ -34,7 +35,7 @@ class ArvPathMapper(PathMapper):
             if "#" in src:
                 src = src[:src.index("#")]
             if isinstance(src, basestring) and ArvPathMapper.pdh_path.match(src):
-                self._pathmap[src] = MapperEnt(src, self.collection_pattern % src[5:], "File")
+                self._pathmap[src] = MapperEnt(src, self.collection_pattern % urllib.unquote(src[5:]), "File")
             if src not in self._pathmap:
                 # Local FS ref, may need to be uploaded or may be on keep
                 # mount.
@@ -44,7 +45,7 @@ class ArvPathMapper(PathMapper):
                     if isinstance(st, arvados.commands.run.UploadFile):
                         uploadfiles.add((src, ab, st))
                     elif isinstance(st, arvados.commands.run.ArvFile):
-                        self._pathmap[src] = MapperEnt(st.fn, self.collection_pattern % st.fn[5:], "File")
+                        self._pathmap[src] = MapperEnt(st.fn, self.collection_pattern % urllib.unquote(st.fn[5:]), "File")
                     elif src.startswith("_:"):
                         if "contents" in srcobj:
                             pass
@@ -59,7 +60,7 @@ class ArvPathMapper(PathMapper):
                     self.visit(l, uploadfiles)
         elif srcobj["class"] == "Directory":
             if isinstance(src, basestring) and ArvPathMapper.pdh_dirpath.match(src):
-                self._pathmap[src] = MapperEnt(src, self.collection_pattern % src[5:], "Directory")
+                self._pathmap[src] = MapperEnt(src, self.collection_pattern % urllib.unquote(src[5:]), "Directory")
             for l in srcobj.get("listing", []):
                 self.visit(l, uploadfiles)
 
@@ -90,7 +91,7 @@ class ArvPathMapper(PathMapper):
             loc = k["location"]
             if loc in already_uploaded:
                 v = already_uploaded[loc]
-                self._pathmap[loc] = MapperEnt(v.resolved, self.collection_pattern % v.resolved[5:], "File")
+                self._pathmap[loc] = MapperEnt(v.resolved, self.collection_pattern % urllib.unquote(v.resolved[5:]), "File")
 
         for srcobj in referenced_files:
             self.visit(srcobj, uploadfiles)
@@ -105,7 +106,7 @@ class ArvPathMapper(PathMapper):
                                              project=self.arvrunner.project_uuid)
 
         for src, ab, st in uploadfiles:
-            self._pathmap[src] = MapperEnt(st.fn, self.collection_pattern % st.fn[5:], "File")
+            self._pathmap[src] = MapperEnt(urllib.quote(st.fn, "/:+@"), self.collection_pattern % st.fn[5:], "File")
             self.arvrunner.add_uploaded(src, self._pathmap[src])
 
         for srcobj in referenced_files:
index 86467040a5c7440ae6aed42b506fbae935ab45eb..2c03812ed3e38e339f1323abd25e2bc4ec61c8dc 100755 (executable)
@@ -2,4 +2,4 @@
 if ! arv-get d7514270f356df848477718d58308cc4+94 > /dev/null ; then
     arv-put --portable-data-hash testdir
 fi
-exec cwltest --test arvados-tests.yml --tool $PWD/runner.sh
+exec cwltest --test arvados-tests.yml --tool $PWD/runner.sh $@
index b74f828f4bd04a2a6321aa50e5f823cb3a2496ab..b96a4c8bd30882bbcec27d453fe9732f24dd6d1c 100644 (file)
@@ -1,4 +1,3 @@
-import gflags
 import httplib
 import httplib2
 import logging
index 1af50b311020f4ed87391cd2a1e76463f39394f5..543725b516beada820f9b3e001d1267024436b02 100644 (file)
@@ -44,7 +44,7 @@ class OrderedJsonModel(apiclient.model.JsonModel):
         return body
 
 
-def _intercept_http_request(self, uri, **kwargs):
+def _intercept_http_request(self, uri, method="GET", **kwargs):
     if (self.max_request_size and
         kwargs.get('body') and
         self.max_request_size < len(kwargs['body'])):
@@ -58,7 +58,7 @@ def _intercept_http_request(self, uri, **kwargs):
 
     kwargs['headers']['Authorization'] = 'OAuth2 %s' % self.arvados_api_token
 
-    retryable = kwargs.get('method', 'GET') in [
+    retryable = method in [
         'DELETE', 'GET', 'HEAD', 'OPTIONS', 'PUT']
     retry_count = self._retry_count if retryable else 0
 
@@ -75,7 +75,7 @@ def _intercept_http_request(self, uri, **kwargs):
     for _ in range(retry_count):
         self._last_request_time = time.time()
         try:
-            return self.orig_http_request(uri, **kwargs)
+            return self.orig_http_request(uri, method, **kwargs)
         except httplib.HTTPException:
             _logger.debug("Retrying API request in %d s after HTTP error",
                           delay, exc_info=True)
@@ -93,7 +93,7 @@ def _intercept_http_request(self, uri, **kwargs):
         delay = delay * self._retry_delay_backoff
 
     self._last_request_time = time.time()
-    return self.orig_http_request(uri, **kwargs)
+    return self.orig_http_request(uri, method, **kwargs)
 
 def _patch_http_request(http, api_token):
     http.arvados_api_token = api_token
index be10632091d382f2dbfdcc3afd398ac97695f81e..5387b0232ab47ae74f5e9c125915b796ecfe1d96 100644 (file)
@@ -45,15 +45,13 @@ setup(name='arvados-python-client',
           ('share/doc/arvados-python-client', ['LICENSE-2.0.txt', 'README.rst']),
       ],
       install_requires=[
-          'google-api-python-client==1.4.2',
-          'oauth2client >=1.4.6, <2',
+          'google-api-python-client==1.6.2, <1.7',
           'ciso8601',
-          'httplib2',
+          'httplib2 >= 0.9.2',
           'pycurl >=7.19.5.1',
-          'python-gflags<3.0',
           'setuptools',
           'ws4py<0.4',
-          'ruamel.yaml==0.13.7'
+          'ruamel.yaml>=0.13.7'
       ],
       test_suite='tests',
       tests_require=['pbr<1.7.0', 'mock>=1.0', 'PyYAML'],
index c7ac090adc3b9d7bc6b265707ac0c3b4973521e7..82ea0acbd63d9b12e2b31cd4d35fec783b869b59 100644 (file)
@@ -1,3 +1,5 @@
+require 'tempfile'
+
 class Node < ArvadosModel
   include HasUuid
   include KindAndEtag
@@ -171,22 +173,30 @@ 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)
-        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|
+        Tempfile.open(["#{hostname}-", ".conf.tmp"],
+                                 Rails.configuration.dns_server_conf_dir) do |f|
+          tmpfile = f.path
           f.puts template % template_vars
         end
-        File.rename hostfile+'.tmp', hostfile
-      rescue => e
+        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
 
@@ -205,7 +215,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
index c1e77f6a4d4cf78e6e3ac7ce77b4ffe5a2fd4c9e..2330e7c528f6a304b05cf318fcc2482e91e62b8d 100644 (file)
@@ -1,4 +1,6 @@
 require 'test_helper'
+require 'tmpdir'
+require 'tempfile'
 
 class NodeTest < ActiveSupport::TestCase
   def ping_node(node_name, ping_data)
@@ -76,6 +78,16 @@ class NodeTest < ActiveSupport::TestCase
     assert Node.dns_server_update 'compute65535', '127.0.0.127'
   end
 
+  test "don't leave temp files behind if there's an error writing them" do
+    Rails.configuration.dns_server_conf_template = Rails.root.join 'config', 'unbound.template'
+    Tempfile.any_instance.stubs(:puts).raises(IOError)
+    Dir.mktmpdir do |tmpdir|
+      Rails.configuration.dns_server_conf_dir = tmpdir
+      refute Node.dns_server_update 'compute65535', '127.0.0.127'
+      assert_empty Dir.entries(tmpdir).select{|f| File.file? f}
+    end
+  end
+
   test "ping new node with no hostname and default config" do
     node = ping_node(:new_with_no_hostname, {})
     slot_number = node.slot_number