This is an automated email from the ASF dual-hosted git repository.

adoroszlai pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ambari.git


The following commit(s) were added to refs/heads/trunk by this push:
     new 8613d58  AMBARI-24981. JournalNode may fail to start due to unreadable 
config file (#2679)
8613d58 is described below

commit 8613d58c347cc21961726811ac8d7bf43389ee6f
Author: Doroszlai, Attila <6454655+adorosz...@users.noreply.github.com>
AuthorDate: Tue Feb 5 15:48:25 2019 +0100

    AMBARI-24981. JournalNode may fail to start due to unreadable config file 
(#2679)
---
 .../resource_management/TestContentSources.py      |   5 +-
 .../python/resource_management/TestFileResource.py |  39 ++----
 .../TestPropertiesFileResource.py                  |  16 +--
 .../resource_management/TestRepositoryResource.py  |  12 +-
 .../resource_management/TestXmlConfigResource.py   |  17 ++-
 .../src/main/python/ambari_commons/os_utils.py     |   6 +
 .../resource_management/core/providers/system.py   |  68 +++++-----
 .../main/python/resource_management/core/sudo.py   | 140 +++++++++++----------
 .../libraries/providers/repository.py              |   6 +-
 9 files changed, 158 insertions(+), 151 deletions(-)

diff --git 
a/ambari-agent/src/test/python/resource_management/TestContentSources.py 
b/ambari-agent/src/test/python/resource_management/TestContentSources.py
index eb5eee2..37281bb 100644
--- a/ambari-agent/src/test/python/resource_management/TestContentSources.py
+++ b/ambari-agent/src/test/python/resource_management/TestContentSources.py
@@ -99,13 +99,14 @@ class TestContentSources(TestCase):
     self.assertEqual(opener_mock.call_count, 1)
     request_mock.assert_called_with('http://download/source')
     self.assertEqual(web_file_mock.read.call_count, 1)
-    
+
   @patch("__builtin__.open")
   @patch.object(urllib2, "Request")
   @patch.object(urllib2, "build_opener")
   @patch.object(os, "makedirs")
   @patch.object(os.path, "exists")
-  def test_download_source_get_content_cache_new(self, exists_mock, 
makedirs_mock, opener_mock, request_mock, open_mock):
+  @patch("resource_management.core.sudo.create_file")
+  def test_download_source_get_content_cache_new(self, create_mock, 
exists_mock, makedirs_mock, opener_mock, request_mock, open_mock):
     """
     Testing DownloadSource.get_content with cache on non-cached resource
     """
diff --git 
a/ambari-agent/src/test/python/resource_management/TestFileResource.py 
b/ambari-agent/src/test/python/resource_management/TestFileResource.py
index 8fc81e4..bcabdd29 100644
--- a/ambari-agent/src/test/python/resource_management/TestFileResource.py
+++ b/ambari-agent/src/test/python/resource_management/TestFileResource.py
@@ -18,7 +18,7 @@ limitations under the License.
 
 
 from unittest import TestCase
-from mock.mock import patch, MagicMock
+from mock.mock import patch, MagicMock, ANY
 from only_for_platform import get_platform, not_for_platform, os_distro_value, 
PLATFORM_WINDOWS
 
 from ambari_commons.os_check import OSCheck
@@ -103,10 +103,9 @@ class TestFileResource(TestCase):
            mode=0777,
            content='file-content'
       )
-    
 
-    create_file_mock.assert_called_with('/directory/file', 'file-content', 
encoding=None)
-    self.assertEqual(create_file_mock.call_count, 1)
+
+    create_file_mock.assert_called_once('/directory/file', 'file-content', 
encoding=None, on_file_created=ANY)
     ensure_mock.assert_called()
 
 
@@ -130,8 +129,8 @@ class TestFileResource(TestCase):
            content='new-content'
       )
 
-    read_file_mock.assert_called_with('/directory/file', encoding=None)    
-    create_file_mock.assert_called_with('/directory/file', 'new-content', 
encoding=None)
+    read_file_mock.assert_called_with('/directory/file', encoding=None)
+    create_file_mock.assert_called_with('/directory/file', 'new-content', 
encoding=None, on_file_created=ANY)
 
 
   @patch("resource_management.core.sudo.unlink")
@@ -276,16 +275,12 @@ class TestFileResource(TestCase):
   @patch("resource_management.core.sudo.chown")
   @patch("resource_management.core.sudo.chmod")
   @patch("resource_management.core.sudo.stat")
-  @patch("resource_management.core.sudo.create_file")
-  @patch("resource_management.core.sudo.path_exists")
-  @patch("resource_management.core.sudo.path_isdir")
-  def test_ensure_metadata(self, isdir_mock, exists_mock, create_file_mock, 
stat_mock, chmod_mock, chown_mock, getgrnam_mock,
+  def test_ensure_metadata(self, stat_mock, chmod_mock, chown_mock, 
getgrnam_mock,
                            getpwnam_mock):
     """
     Tests if _ensure_metadata changes owner, usergroup and permissions of file 
to proper values
     """
-    isdir_mock.side_effect = [False, True, False, True]
-    exists_mock.return_value = False
+    from resource_management.core.providers.system import _ensure_metadata
 
     class stat():
       def __init__(self):
@@ -300,17 +295,8 @@ class TestFileResource(TestCase):
     getgrnam_mock.return_value.gr_gid = 0
 
     with Environment('/') as env:
-      File('/directory/file',
-           action='create',
-           mode=0777,
-           content='file-content',
-           owner='root',
-           group='hdfs'
-      )
-    
+      _ensure_metadata('/directory/file', user='root', group='hdfs', mode=0777)
 
-    create_file_mock.assert_called_with('/directory/file', 'file-content', 
encoding=None)
-    self.assertEqual(create_file_mock.call_count, 1)
     stat_mock.assert_called_with('/directory/file')
     self.assertEqual(chmod_mock.call_count, 1)
     self.assertEqual(chown_mock.call_count, 1)
@@ -325,14 +311,7 @@ class TestFileResource(TestCase):
     getgrnam_mock.return_value.gr_gid = 1
 
     with Environment('/') as env:
-      File('/directory/file',
-           action='create',
-           mode=0777,
-           content='file-content',
-           owner='root',
-           group='hdfs'
-      )
-    
+      _ensure_metadata('/directory/file', user='root', group='hdfs', mode=0777)
 
     self.assertEqual(chmod_mock.call_count, 1)
     chown_mock.assert_called_with('/directory/file', None, None)
diff --git 
a/ambari-agent/src/test/python/resource_management/TestPropertiesFileResource.py
 
b/ambari-agent/src/test/python/resource_management/TestPropertiesFileResource.py
index 286ff37..6b7ae8f 100644
--- 
a/ambari-agent/src/test/python/resource_management/TestPropertiesFileResource.py
+++ 
b/ambari-agent/src/test/python/resource_management/TestPropertiesFileResource.py
@@ -23,7 +23,7 @@ Ambari Agent
 import os
 import time
 from unittest import TestCase
-from mock.mock import patch, MagicMock
+from mock.mock import patch, MagicMock, ANY
 from only_for_platform import get_platform, not_for_platform, os_distro_value, 
PLATFORM_WINDOWS
 
 from ambari_commons.os_check import OSCheck
@@ -69,7 +69,7 @@ class TestPropertiesFIleResource(TestCase):
                      properties={}
       )
 
-    
create_file_mock.assert_called_with('/somewhere_in_system/one_file.properties', 
u'# Generated by Apache Ambari. Today is Wednesday\n    \n    ', 
encoding="UTF-8")
+    
create_file_mock.assert_called_with('/somewhere_in_system/one_file.properties', 
u'# Generated by Apache Ambari. Today is Wednesday\n    \n    ', 
encoding="UTF-8", on_file_created=ANY)
     ensure_mock.assert_called()
 
 
@@ -102,7 +102,7 @@ class TestPropertiesFIleResource(TestCase):
                      properties={},
       )
 
-    create_file_mock.assert_called_with('/dir/and/dir/file.txt', u'# Generated 
by Apache Ambari. Some other day\n    \n    ', encoding="UTF-8")
+    create_file_mock.assert_called_with('/dir/and/dir/file.txt', u'# Generated 
by Apache Ambari. Some other day\n    \n    ', encoding="UTF-8", 
on_file_created=ANY)
     ensure_mock.assert_called()
 
 
@@ -127,15 +127,15 @@ class TestPropertiesFIleResource(TestCase):
     os_path_exists_mock.return_value = False
     time_asctime_mock.return_value = 777
 
-    
-    
+
+
 
     with Environment('/') as env:
       PropertiesFile('/dir/new_file',
                      properties={'property1': 'value1'},
       )
 
-    create_file_mock.assert_called_with('/dir/new_file', u'# Generated by 
Apache Ambari. 777\n    \nproperty1=value1\n    ', encoding="UTF-8")
+    create_file_mock.assert_called_with('/dir/new_file', u'# Generated by 
Apache Ambari. 777\n    \nproperty1=value1\n    ', encoding="UTF-8", 
on_file_created=ANY)
     ensure_mock.assert_called()
 
 
@@ -173,7 +173,7 @@ class TestPropertiesFIleResource(TestCase):
                      },
       )
 
-    create_file_mock.assert_called_with('/dir/new_file', u"# Generated by 
Apache Ambari. 777\n    \n=\nprop.1='.'yyyy-MM-dd-HH\nprop.2=INFO, 
openjpa\nprop.3=%d{ISO8601} %5p %c{1}:%L - 
%m%n\nprop.4=${oozie.log.dir}/oozie.log\nprop.empty=\n    ", encoding="UTF-8")
+    create_file_mock.assert_called_with('/dir/new_file', u"# Generated by 
Apache Ambari. 777\n    \n=\nprop.1='.'yyyy-MM-dd-HH\nprop.2=INFO, 
openjpa\nprop.3=%d{ISO8601} %5p %c{1}:%L - 
%m%n\nprop.4=${oozie.log.dir}/oozie.log\nprop.empty=\n    ", encoding="UTF-8", 
on_file_created=ANY)
     ensure_mock.assert_called()
 
 
@@ -210,5 +210,5 @@ class TestPropertiesFIleResource(TestCase):
       )
 
     read_file_mock.assert_called()
-    create_file_mock.assert_called_with('/dir1/new_file', u'# Generated by 
Apache Ambari. 777\n    \nproperty_1=value1\n    ', encoding="UTF-8")
+    create_file_mock.assert_called_with('/dir1/new_file', u'# Generated by 
Apache Ambari. 777\n    \nproperty_1=value1\n    ', encoding="UTF-8", 
on_file_created=ANY)
     ensure_mock.assert_called()
diff --git 
a/ambari-agent/src/test/python/resource_management/TestRepositoryResource.py 
b/ambari-agent/src/test/python/resource_management/TestRepositoryResource.py
index 3bb91e8..e296e1d 100644
--- a/ambari-agent/src/test/python/resource_management/TestRepositoryResource.py
+++ b/ambari-agent/src/test/python/resource_management/TestRepositoryResource.py
@@ -194,8 +194,9 @@ class TestRepositoryResource(TestCase):
     @patch("resource_management.libraries.providers.repository.File")
     @patch("os.path.isfile", new=MagicMock(return_value=True))
     @patch("filecmp.cmp", new=MagicMock(return_value=False))
-    @patch.object(System, "os_release_name", new='precise')        
+    @patch.object(System, "os_release_name", new='precise')
     @patch.object(System, "os_family", new='ubuntu')
+    @patch("ambari_commons.os_utils.current_user", 
new=MagicMock(return_value='ambari-agent'))
     def test_create_repo_ubuntu_repo_exists(self, file_mock, execute_mock,
                                             tempfile_mock, call_mock, 
is_redhat_family, is_ubuntu_family, is_suse_family):
       is_redhat_family.return_value = False
@@ -218,13 +219,13 @@ class TestRepositoryResource(TestCase):
       call_content = file_mock.call_args_list[0]
       template_name = call_content[0][0]
       template_content = call_content[1]['content']
-      
+
       self.assertEquals(template_name, '/tmp/1.txt')
       self.assertEquals(template_content, 'deb 
http://download.base_url.org/rpm/ a b c')
-      
+
       copy_item0 = str(file_mock.call_args_list[1])
       copy_item1 = str(file_mock.call_args_list[2])
-      self.assertEqual(copy_item0, "call('/tmp/1.txt', 
content=StaticFile('/etc/apt/sources.list.d/HDP.list'))")
+      self.assertEqual(copy_item0, "call('/tmp/1.txt', owner='ambari-agent', 
content=StaticFile('/etc/apt/sources.list.d/HDP.list'))")
       self.assertEqual(copy_item1, "call('/etc/apt/sources.list.d/HDP.list', 
content=StaticFile('/tmp/1.txt'))")
       #'apt-get update -qq -o Dir::Etc::sourcelist="sources.list.d/HDP.list" 
-o APT::Get::List-Cleanup="0"')
       execute_command_item = execute_mock.call_args_list[0][0][0]
@@ -240,6 +241,7 @@ class TestRepositoryResource(TestCase):
     @patch("filecmp.cmp", new=MagicMock(return_value=False))
     @patch.object(System, "os_release_name", new='precise')
     @patch.object(System, "os_family", new='ubuntu')
+    @patch("ambari_commons.os_utils.current_user", 
new=MagicMock(return_value='ambari-agent'))
     def test_create_repo_ubuntu_gpg_key_wrong_output(self, file_mock, 
execute_mock,
                                             tempfile_mock, call_mock):
       """
@@ -268,7 +270,7 @@ class TestRepositoryResource(TestCase):
 
       copy_item0 = str(file_mock.call_args_list[1])
       copy_item1 = str(file_mock.call_args_list[2])
-      self.assertEqual(copy_item0, "call('/tmp/1.txt', 
content=StaticFile('/etc/apt/sources.list.d/HDP.list'))")
+      self.assertEqual(copy_item0, "call('/tmp/1.txt', owner='ambari-agent', 
content=StaticFile('/etc/apt/sources.list.d/HDP.list'))")
       self.assertEqual(copy_item1, "call('/etc/apt/sources.list.d/HDP.list', 
content=StaticFile('/tmp/1.txt'))")
       execute_command_item = execute_mock.call_args_list[0][0][0]
 
diff --git 
a/ambari-agent/src/test/python/resource_management/TestXmlConfigResource.py 
b/ambari-agent/src/test/python/resource_management/TestXmlConfigResource.py
index 9b4c1d5..a172da3 100644
--- a/ambari-agent/src/test/python/resource_management/TestXmlConfigResource.py
+++ b/ambari-agent/src/test/python/resource_management/TestXmlConfigResource.py
@@ -23,7 +23,7 @@ Ambari Agent
 import os
 import time
 from unittest import TestCase
-from mock.mock import patch, MagicMock
+from mock.mock import patch, MagicMock, ANY
 
 from only_for_platform import get_platform, not_for_platform, os_distro_value, 
PLATFORM_WINDOWS
 
@@ -68,7 +68,8 @@ class TestXmlConfigResource(TestCase):
                 configuration_attributes={}
                 )
 
-    create_file_mock.assert_called_with('/dir/conf/file.xml', u'  
<configuration  xmlns:xi="http://www.w3.org/2001/XInclude";>\n    \n  
</configuration>', encoding='UTF-8')
+    create_file_mock.assert_called_with('/dir/conf/file.xml', u'  
<configuration  xmlns:xi="http://www.w3.org/2001/XInclude";>\n    \n  
</configuration>',
+        encoding='UTF-8', on_file_created=ANY)
 
 
   @patch("resource_management.core.providers.system._ensure_metadata")
@@ -97,7 +98,8 @@ class TestXmlConfigResource(TestCase):
                 configuration_attributes={'attr': {'property1': 'attr_value'}}
                 )
 
-    create_file_mock.assert_called_with('/dir/conf/file.xml', u'  
<configuration  xmlns:xi="http://www.w3.org/2001/XInclude";>\n    \n    
<property>\n      <name>property1</name>\n      <value>value1</value>\n      
<attr>attr_value</attr>\n    </property>\n    \n  </configuration>', 
encoding='UTF-8')
+    create_file_mock.assert_called_with('/dir/conf/file.xml', u'  
<configuration  xmlns:xi="http://www.w3.org/2001/XInclude";>\n    \n    
<property>\n      <name>property1</name>\n      <value>value1</value>\n      
<attr>attr_value</attr>\n    </property>\n    \n  </configuration>',
+        encoding='UTF-8', on_file_created=ANY)
 
   @patch("resource_management.core.providers.system._ensure_metadata")
   @patch("resource_management.core.sudo.create_file")
@@ -126,7 +128,8 @@ class TestXmlConfigResource(TestCase):
                 xml_include_file="/dif/conf/include_file.xml"
                 )
 
-    create_file_mock.assert_called_with('/dir/conf/file.xml', u'  
<configuration  xmlns:xi="http://www.w3.org/2001/XInclude";>\n    \n    
<property>\n      <name>property1</name>\n      <value>value1</value>\n      
<attr>attr_value</attr>\n    </property>\n    \n    <xi:include 
href="/dif/conf/include_file.xml"/>\n    \n  </configuration>', 
encoding='UTF-8')
+    create_file_mock.assert_called_with('/dir/conf/file.xml', u'  
<configuration  xmlns:xi="http://www.w3.org/2001/XInclude";>\n    \n    
<property>\n      <name>property1</name>\n      <value>value1</value>\n      
<attr>attr_value</attr>\n    </property>\n    \n    <xi:include 
href="/dif/conf/include_file.xml"/>\n    \n  </configuration>',
+        encoding='UTF-8', on_file_created=ANY)
 
   @patch("resource_management.core.providers.system._ensure_metadata")
   @patch("resource_management.core.sudo.create_file")
@@ -178,7 +181,8 @@ class TestXmlConfigResource(TestCase):
                     }
                 })
 
-    create_file_mock.assert_called_with('/dir/conf/file.xml', u'  
<configuration  xmlns:xi="http://www.w3.org/2001/XInclude";>\n    \n    
<property>\n      <name></name>\n      <value></value>\n    </property>\n    \n 
   <property>\n      <name>prop.1</name>\n      
<value>&#39;.&#39;yyyy-MM-dd-HH</value>\n      <attr1>x</attr1>\n    
</property>\n    \n    <property>\n      <name>prop.2</name>\n      
<value>INFO, openjpa</value>\n    </property>\n    \n    <property>\n      
<name>prop.3</n [...]
+    create_file_mock.assert_called_with('/dir/conf/file.xml', u'  
<configuration  xmlns:xi="http://www.w3.org/2001/XInclude";>\n    \n    
<property>\n      <name></name>\n      <value></value>\n    </property>\n    \n 
   <property>\n      <name>prop.1</name>\n      
<value>&#39;.&#39;yyyy-MM-dd-HH</value>\n      <attr1>x</attr1>\n    
</property>\n    \n    <property>\n      <name>prop.2</name>\n      
<value>INFO, openjpa</value>\n    </property>\n    \n    <property>\n      
<name>prop.3</n [...]
+        encoding='UTF-8', on_file_created=ANY)
 
   @patch("resource_management.core.providers.system._ensure_metadata")
   @patch("resource_management.core.sudo.create_file")
@@ -211,7 +215,8 @@ class TestXmlConfigResource(TestCase):
                 configuration_attributes={}
                 )
 
-    create_file_mock.assert_called_with('/dir/conf/file.xml', u'  
<configuration  xmlns:xi="http://www.w3.org/2001/XInclude";>\n    \n    
<property>\n      <name></name>\n      <value></value>\n    </property>\n    \n 
   <property>\n      <name>first</name>\n      <value>should be first</value>\n 
   </property>\n    \n    <property>\n      <name>second</name>\n      
<value>should be second</value>\n    </property>\n    \n    <property>\n      
<name>third</name>\n      <value>should be thi [...]
+    create_file_mock.assert_called_with('/dir/conf/file.xml', u'  
<configuration  xmlns:xi="http://www.w3.org/2001/XInclude";>\n    \n    
<property>\n      <name></name>\n      <value></value>\n    </property>\n    \n 
   <property>\n      <name>first</name>\n      <value>should be first</value>\n 
   </property>\n    \n    <property>\n      <name>second</name>\n      
<value>should be second</value>\n    </property>\n    \n    <property>\n      
<name>third</name>\n      <value>should be thi [...]
+        encoding='UTF-8', on_file_created=ANY)
 
   @patch("resource_management.libraries.providers.xml_config.File")
   @patch("resource_management.core.sudo.path_exists")
diff --git a/ambari-common/src/main/python/ambari_commons/os_utils.py 
b/ambari-common/src/main/python/ambari_commons/os_utils.py
index 620bd0e..4224f00 100644
--- a/ambari-common/src/main/python/ambari_commons/os_utils.py
+++ b/ambari-common/src/main/python/ambari_commons/os_utils.py
@@ -43,6 +43,12 @@ else:
 from ambari_commons.exceptions import FatalException
 from ambari_commons.logging_utils import print_info_msg, print_warning_msg
 
+def current_user():
+  if OSCheck.is_windows_family():
+    return None
+  else:
+    return pwd.getpwuid(os.geteuid())[0]
+
 def get_used_ram():
   """
   Returns resident RAM used by current process in kilobytes
diff --git 
a/ambari-common/src/main/python/resource_management/core/providers/system.py 
b/ambari-common/src/main/python/resource_management/core/providers/system.py
index a286d03..e187c07 100644
--- a/ambari-common/src/main/python/resource_management/core/providers/system.py
+++ b/ambari-common/src/main/python/resource_management/core/providers/system.py
@@ -51,41 +51,41 @@ def _ensure_metadata(path, user, group, mode=None, 
cd_access=None, recursive_own
       _user_entity = pwd.getpwnam(user)
     except KeyError:
       raise Fail("User '{0}' doesn't exist".format(user))
-    
+
     if stat.st_uid != _user_entity.pw_uid:
       user_entity = _user_entity
       Logger.info(
         "Changing owner for %s from %d to %s" % (path, stat.st_uid, user))
-      
+
   if group:
     try:
       _group_entity = grp.getgrnam(group)
     except KeyError:
       raise Fail("Group '{0}' doesn't exist".format(group))
-    
+
     if stat.st_gid != _group_entity.gr_gid:
       group_entity = _group_entity
       Logger.info(
         "Changing group for %s from %d to %s" % (path, stat.st_gid, group))
-  
+
   if recursive_ownership:
     assert_not_safemode_folder(path, safemode_folders)
     sudo.chown_recursive(path, _user_entity, _group_entity, 
recursion_follow_links)
-  
+
   sudo.chown(path, user_entity, group_entity)
-  
+
   if recursive_mode_flags:
     if not isinstance(recursive_mode_flags, dict):
       raise Fail("'recursion_follow_links' value should be a dictionary with 
'f' and(or) 'd' key (for file and directory permission flags)")
-    
+
     regexp_to_match = "^({0},)*({0})$".format("[ugoa]+[+=-][rwx]+" )
     for key, flags in recursive_mode_flags.iteritems():
       if key != 'd' and key != 'f':
         raise Fail("'recursive_mode_flags' with value '%s' has unknown key 
'%s', only keys 'f' and 'd' are valid" % (str(recursive_mode_flags), str(key)))
-          
+
       if not re.match(regexp_to_match, flags):
         raise Fail("'recursive_mode_flags' found '%s', but should value format 
have the following format: [ugoa...][[+-=][perms...]...]." % (str(flags)))
-    
+
     assert_not_safemode_folder(path, safemode_folders)
     sudo.chmod_recursive(path, recursive_mode_flags, recursion_follow_links)
 
@@ -95,30 +95,30 @@ def _ensure_metadata(path, user, group, mode=None, 
cd_access=None, recursive_own
       Logger.info("Changing permission for %s from %o to %o" % (
       path, stat.st_mode, mode))
       sudo.chmod(path, mode)
-      
+
   if cd_access:
     if not re.match("^[ugoa]+$", cd_access):
       raise Fail("'cd_acess' value '%s' is not valid" % (cd_access))
-    
+
     dir_path = re.sub('/+', '/', path)
     while dir_path and dir_path != os.sep:
       if sudo.path_isdir(dir_path):
         sudo.chmod_extended(dir_path, cd_access+"+rx")
-        
+
       dir_path = os.path.split(dir_path)[0]
 
 
 class FileProvider(Provider):
   def action_create(self):
     path = self.resource.path
-    
+
     if sudo.path_isdir(path):
       raise Fail("Applying %s failed, directory with name %s exists" % 
(self.resource, path))
-    
+
     dirname = os.path.dirname(path)
     if not sudo.path_isdir(dirname):
       raise Fail("Applying %s failed, parent directory %s doesn't exist" % 
(self.resource, dirname))
-    
+
     write = False
     content = self._get_content()
     if not sudo.path_exists(path):
@@ -133,19 +133,25 @@ class FileProvider(Provider):
           if self.resource.backup:
             self.resource.env.backup_file(path)
 
+    owner = self.resource.owner or 'root'
+    group = self.resource.group or 'root'
+
     if write:
       Logger.info("Writing %s because %s" % (self.resource, reason))
-      sudo.create_file(path, content, encoding=self.resource.encoding)
+      def on_file_created(filename):
+        _ensure_metadata(filename, owner, group, mode=self.resource.mode, 
cd_access=self.resource.cd_access)
+        Logger.info("Moving %s to %s" % (filename, path))
 
-    _ensure_metadata(self.resource.path, self.resource.owner,
-                        self.resource.group, mode=self.resource.mode, 
cd_access=self.resource.cd_access)
+      sudo.create_file(path, content, encoding=self.resource.encoding, 
on_file_created=on_file_created)
+    else:
+      _ensure_metadata(path, owner, group, mode=self.resource.mode, 
cd_access=self.resource.cd_access)
 
   def action_delete(self):
     path = self.resource.path
-    
+
     if sudo.path_isdir(path):
       raise Fail("Applying %s failed, %s is directory not file!" % 
(self.resource, path))
-    
+
     if sudo.path_exists(path):
       Logger.info("Deleting %s" % self.resource)
       sudo.unlink(path)
@@ -167,7 +173,7 @@ class DirectoryProvider(Provider):
 
     if not sudo.path_exists(path):
       Logger.info("Creating directory %s since it doesn't exist." % 
self.resource)
-      
+
       # dead links should be followed, else we gonna have failures on trying 
to create directories on top of them.
       if self.resource.follow:
         # Follow symlink until the tail.
@@ -178,7 +184,7 @@ class DirectoryProvider(Provider):
           followed_links.add(path)
           prev_path = path
           path = sudo.readlink(path)
-          
+
           if not os.path.isabs(path):
             path = os.path.join(os.path.dirname(prev_path), path)
 
@@ -191,7 +197,7 @@ class DirectoryProvider(Provider):
         dirname = os.path.dirname(path)
         if not sudo.path_isdir(dirname):
           raise Fail("Applying %s failed, parent directory %s doesn't exist" % 
(self.resource, dirname))
-        
+
         try:
           sudo.makedir(path, self.resource.mode or 0755)
         except Exception as ex:
@@ -203,10 +209,10 @@ class DirectoryProvider(Provider):
 
     if not sudo.path_isdir(path):
       raise Fail("Applying %s failed, file %s already exists" % 
(self.resource, path))
-    
+
     _ensure_metadata(path, self.resource.owner, self.resource.group,
                         mode=self.resource.mode, 
cd_access=self.resource.cd_access,
-                        recursive_ownership=self.resource.recursive_ownership, 
recursive_mode_flags=self.resource.recursive_mode_flags, 
+                        recursive_ownership=self.resource.recursive_ownership, 
recursive_mode_flags=self.resource.recursive_mode_flags,
                         
recursion_follow_links=self.resource.recursion_follow_links, 
safemode_folders=self.resource.safemode_folders)
 
   def action_delete(self):
@@ -214,7 +220,7 @@ class DirectoryProvider(Provider):
     if sudo.path_exists(path):
       if not sudo.path_isdir(path):
         raise Fail("Applying %s failed, %s is not a directory" % 
(self.resource, path))
-      
+
       Logger.info("Removing directory %s and all its content" % self.resource)
       sudo.rmtree(path)
 
@@ -232,19 +238,19 @@ class LinkProvider(Provider):
           "%s trying to create a symlink with the same name as an existing 
file or directory" % self.resource)
       Logger.info("%s replacing old symlink to %s" % (self.resource, oldpath))
       sudo.unlink(path)
-      
+
     if self.resource.hard:
       if not sudo.path_exists(self.resource.to):
         raise Fail("Failed to apply %s, linking to nonexistent location %s" % 
(self.resource, self.resource.to))
       if sudo.path_isdir(self.resource.to):
         raise Fail("Failed to apply %s, cannot create hard link to a directory 
(%s)" % (self.resource, self.resource.to))
-      
+
       Logger.info("Creating hard %s" % self.resource)
       sudo.link(self.resource.to, path)
     else:
       if not sudo.path_exists(self.resource.to):
         Logger.info("Warning: linking to nonexistent location %s" % 
self.resource.to)
-        
+
       Logger.info("Creating symbolic %s to %s" % (self.resource, 
self.resource.to))
       sudo.symlink(self.resource.to, path)
 
@@ -260,7 +266,7 @@ class ExecuteProvider(Provider):
       if sudo.path_exists(self.resource.creates):
         Logger.info("Skipping %s due to creates" % self.resource)
         return
-      
+
     shell.checked_call(self.resource.command, 
logoutput=self.resource.logoutput,
                         cwd=self.resource.cwd, env=self.resource.environment,
                         user=self.resource.user, 
wait_for_finish=self.resource.wait_for_finish,
@@ -272,7 +278,7 @@ class ExecuteProvider(Provider):
                         
stdout=self.resource.stdout,stderr=self.resource.stderr,
                         tries=self.resource.tries, 
try_sleep=self.resource.try_sleep,
                         returns=self.resource.returns)
-       
+
 
 class ExecuteScriptProvider(Provider):
   def action_run(self):
diff --git a/ambari-common/src/main/python/resource_management/core/sudo.py 
b/ambari-common/src/main/python/resource_management/core/sudo.py
index 8a9481a..b35f67a 100644
--- a/ambari-common/src/main/python/resource_management/core/sudo.py
+++ b/ambari-common/src/main/python/resource_management/core/sudo.py
@@ -39,14 +39,14 @@ if os.geteuid() == 0:
     gid = group.gr_gid if group else -1
     if uid != -1 or gid != -1:
       return os.chown(path, uid, gid)
-      
+
   def chown_recursive(path, owner, group, follow_links=False):
     uid = owner.pw_uid if owner else -1
     gid = group.gr_gid if group else -1
-    
+
     if uid == -1 and gid == -1:
       return
-      
+
     for root, dirs, files in unicode_walk(path, followlinks=True):
       for name in files + dirs:
         try:
@@ -59,17 +59,17 @@ if os.geteuid() == 0:
           # ignoring OSError: [Errno 2] No such file or directory
           if ex.errno != errno.ENOENT:
             raise
-            
-  
+
+
   def chmod(path, mode):
     """
     Wrapper around python function
-    
+
     :type path str
     :type mode int
     """
     return os.chmod(path, mode)
-  
+
 
   def chmod_extended(path, mode):
     """
@@ -82,7 +82,7 @@ if os.geteuid() == 0:
   def chmod_recursive(path, recursive_mode_flags, 
recursion_follow_links=False):
     """
     Change recursively permissions on directories or files
-    
+
     :type path str
     :type recursive_mode_flags
     :type recursion_follow_links bool
@@ -101,9 +101,12 @@ if os.geteuid() == 0:
           full_file_path = os.path.join(root, file_name)
           chmod(full_file_path, attr_to_bitmask(files_attrib, 
initial_bitmask=os.stat(full_file_path).st_mode))
 
+  def move(src, dst):
+    shutil.move(src, dst)
+
   def copy(src, dst):
     shutil.copy(src, dst)
-    
+
   def makedirs(path, mode):
     try:
       os.makedirs(path, mode)
@@ -120,43 +123,37 @@ if os.geteuid() == 0:
         dirname = os.path.dirname(ex.filename)
         if os.path.islink(dirname) and not os.path.exists(dirname):
           raise Fail("Cannot create directory '{0}' as '{1}' is a looped 
symlink".format(path, dirname))
-        
+
       raise
-  
+
   def makedir(path, mode):
     os.mkdir(path)
-    
+
   def symlink(source, link_name):
     os.symlink(source, link_name)
-    
+
   def link(source, link_name):
     os.link(source, link_name)
-    
+
   def unlink(path):
     os.unlink(path)
 
   def rmtree(path):
     shutil.rmtree(path)
-    
-  def create_file(filename, content, encoding=None):
-    """
-    if content is None, create empty file
-    """
-    with open(filename, "wb") as fp:
-      if content:
-        content = content.encode(encoding) if encoding else content
-        fp.write(content)
-      
+
+  def create_file(filename, content, encoding=None, on_file_created=None):
+    _create_file(filename, content, encoding=encoding, sudo=False, 
on_file_created=on_file_created)
+
   def read_file(filename, encoding=None):
     with open(filename, "rb") as fp:
       content = fp.read()
-        
+
     content = content.decode(encoding) if encoding else content
     return content
-      
+
   def path_exists(path):
     return os.path.exists(path)
-  
+
   def path_isdir(path):
     return os.path.isdir(path)
 
@@ -165,10 +162,10 @@ if os.geteuid() == 0:
 
   def path_lexists(path):
     return os.path.lexists(path)
-  
+
   def readlink(path):
     return os.readlink(path)
-  
+
   def path_isfile(path):
     return os.path.isfile(path)
 
@@ -178,13 +175,13 @@ if os.geteuid() == 0:
         stat_val = os.stat(path)
         self.st_uid, self.st_gid, self.st_mode = stat_val.st_uid, 
stat_val.st_gid, stat_val.st_mode & 07777
     return Stat(path)
-  
+
   def kill(pid, signal):
     os.kill(pid, signal)
-    
+
   def listdir(path):
     return os.listdir(path)
-    
+
 else:
   # os.chown replacement
   def chown(path, owner, group):
@@ -192,7 +189,7 @@ else:
     group = group.gr_name if group else ""
     if owner or group:
       shell.checked_call(["chown", owner+":"+group, path], sudo=True)
-      
+
   def chown_recursive(path, owner, group, follow_links=False):
     owner = owner.pw_name if owner else ""
     group = group.gr_name if group else ""
@@ -201,72 +198,60 @@ else:
       if follow_links:
         flags.append("-L")
       shell.checked_call(["chown"] + flags + [owner+":"+group, path], 
sudo=True)
-      
+
   # os.chmod replacement
   def chmod(path, mode):
     shell.checked_call(["chmod", oct(mode), path], sudo=True)
-    
+
   def chmod_extended(path, mode):
     shell.checked_call(["chmod", mode, path], sudo=True)
-    
+
   # os.makedirs replacement
   def makedirs(path, mode):
     shell.checked_call(["mkdir", "-p", path], sudo=True)
     chmod(path, mode)
-    
+
   # os.makedir replacement
   def makedir(path, mode):
     shell.checked_call(["mkdir", path], sudo=True)
     chmod(path, mode)
-    
+
   # os.symlink replacement
   def symlink(source, link_name):
     shell.checked_call(["ln","-sf", source, link_name], sudo=True)
-    
+
   # os.link replacement
   def link(source, link_name):
     shell.checked_call(["ln", "-f", source, link_name], sudo=True)
-    
+
   # os unlink
   def unlink(path):
     shell.checked_call(["rm","-f", path], sudo=True)
-    
+
   # shutil.rmtree
   def rmtree(path):
     shell.checked_call(["rm","-rf", path], sudo=True)
-    
+
   # fp.write replacement
-  def create_file(filename, content, encoding=None):
-    """
-    if content is None, create empty file
-    """
-    content = content if content else ""
-    content = content.encode(encoding) if encoding else content
+  def create_file(filename, content, encoding=None, on_file_created=None):
+    _create_file(filename, content, encoding=encoding, sudo=True, 
on_file_created=on_file_created)
 
-    tmpf_name = tempfile.gettempdir() + os.sep + tempfile.template + 
str(time.time()) + "_" + str(random.randint(0, 1000))
-    try:
-        with open(tmpf_name, "wb") as fp:
-            fp.write(content)
-        shell.checked_call(["cp", "-f", tmpf_name, filename], sudo=True)
-    finally:
-        os.unlink(tmpf_name)
-      
   # fp.read replacement
   def read_file(filename, encoding=None):
     tmpf = tempfile.NamedTemporaryFile()
     shell.checked_call(["cp", "-f", filename, tmpf.name], sudo=True)
-    
+
     with tmpf:
       with open(tmpf.name, "rb") as fp:
         content = fp.read()
-        
+
     content = content.decode(encoding) if encoding else content
     return content
-      
+
   # os.path.exists
   def path_exists(path):
     return (shell.call(["test", "-e", path], sudo=True)[0] == 0)
-  
+
   # os.path.isdir
   def path_isdir(path):
     return (shell.call(["test", "-d", path], sudo=True)[0] == 0)
@@ -274,15 +259,15 @@ else:
   # os.path.islink
   def path_islink(path):
     return (shell.call(["test", "-L", path], sudo=True)[0] == 0)
-  
+
   # os.path.lexists
   def path_lexists(path):
     return (shell.call(["test", "-e", path], sudo=True)[0] == 0)
-  
+
   # os.readlink
   def readlink(path):
     return shell.checked_call(["readlink", path], sudo=True)[1].strip()
-  
+
   # os.path.isfile
   def path_isfile(path):
     return (shell.call(["test", "-f", path], sudo=True)[0] == 0)
@@ -298,16 +283,20 @@ else:
           raise Fail("Execution of '{0}' returned unexpected output. 
{2}\n{3}".format(cmd, code, err, out))
         uid_str, gid_str, mode_str = values
         self.st_uid, self.st_gid, self.st_mode = int(uid_str), int(gid_str), 
int(mode_str, 8)
-  
+
     return Stat(path)
-  
+
   # os.kill replacement
   def kill(pid, signal):
     try:
       shell.checked_call(["kill", "-"+str(signal), str(pid)], sudo=True)
     except Fail as ex:
       raise OSError(str(ex))
-    
+
+  # shutil.move replacement
+  def move(src, dst):
+    shell.checked_call(('mv', '-f', src, dst), sudo=True)
+
   # shutil.copy replacement
   def copy(src, dst):
     shell.checked_call(["cp", "-r", src, dst], sudo=True)
@@ -316,7 +305,7 @@ else:
   def listdir(path):
     if not path_isdir(path):
       raise Fail("{0} is not a directory. Cannot list files of 
it.".format(path))
-    
+
     code, out, err = shell.checked_call(["ls", path], sudo=True, 
stderr=subprocess32.PIPE)
     files = out.splitlines()
     return files
@@ -329,3 +318,20 @@ else:
 
     for key, flags in recursive_mode_flags.iteritems():
       shell.checked_call(["find"] + find_flags + [path, "-type", key, "-exec" 
, "chmod", flags ,"{}" ,";"])
+
+def _create_file(filename, content, encoding, sudo, on_file_created=None):
+    """
+    Creates file in a temporary location, then set permissions via 
on_file_created callback (if provided),
+    then move to final location.
+
+    Creates empty file if content is None.
+    """
+    content = content if content else ""
+    content = content.encode(encoding) if encoding else content
+
+    tmpf_name = tempfile.gettempdir() + os.sep + tempfile.template + 
str(time.time()) + "_" + str(random.randint(0, 1000))
+    with open(tmpf_name, "wb") as fp:
+      fp.write(content)
+    if on_file_created:
+      on_file_created(tmpf_name)
+    shell.checked_call(["mv", "-f", tmpf_name, filename], sudo=sudo)
diff --git 
a/ambari-common/src/main/python/resource_management/libraries/providers/repository.py
 
b/ambari-common/src/main/python/resource_management/libraries/providers/repository.py
index 16af6b9..6fd1155 100644
--- 
a/ambari-common/src/main/python/resource_management/libraries/providers/repository.py
+++ 
b/ambari-common/src/main/python/resource_management/libraries/providers/repository.py
@@ -23,7 +23,7 @@ Ambari Agent
 import os
 import filecmp
 import tempfile
-from ambari_commons import OSCheck
+from ambari_commons import OSCheck, os_utils
 from resource_management.core.resources import Execute
 from resource_management.core.resources import File
 from resource_management.core.providers import Provider
@@ -50,13 +50,15 @@ class RepositoryProvider(Provider):
           repo_file_content = repo_file_content.strip()
 
           File(tmpf.name,
-               content=repo_file_content
+               content=repo_file_content,
+               owner=os_utils.current_user(),
           )
 
           if os.path.isfile(repo_file_path):
             # a copy of old repo file, which will be readable by current user
             File(old_repo_tmpf.name,
                  content=StaticFile(repo_file_path),
+                 owner=os_utils.current_user(),
             )
 
           if not os.path.isfile(repo_file_path) or not filecmp.cmp(tmpf.name, 
old_repo_tmpf.name):

Reply via email to