Repository: ambari Updated Branches: refs/heads/trunk 45d67f60f -> 5ff857cd2
AMBARI-9341 Failing to register hosts on Centos5 sys.exit() should never be braced in a try/except. The behavior changed in Python 2.5. See the doc. Replaced sys.exit() calls with return statements. Normalized the return data structures. Fixed unit tests Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/5ff857cd Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/5ff857cd Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/5ff857cd Branch: refs/heads/trunk Commit: 5ff857cd2259c91286934ed522f54502d4f15fae Parents: 45d67f6 Author: Florian Barca <fba...@hortonworks.com> Authored: Tue Feb 3 14:56:02 2015 -0800 Committer: Florian Barca <fba...@hortonworks.com> Committed: Tue Feb 3 14:56:02 2015 -0800 ---------------------------------------------------------------------- ambari-server/src/main/python/setupAgent.py | 90 +++++---- ambari-server/src/test/python/TestSetupAgent.py | 190 +++++++++++-------- 2 files changed, 161 insertions(+), 119 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/5ff857cd/ambari-server/src/main/python/setupAgent.py ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/python/setupAgent.py b/ambari-server/src/main/python/setupAgent.py index b2c2d25..3595e2f 100755 --- a/ambari-server/src/main/python/setupAgent.py +++ b/ambari-server/src/main/python/setupAgent.py @@ -63,11 +63,13 @@ def configureAgent(server_hostname, user_run_as): """ Configure the agent so that it has all the configs knobs properly installed """ osCommand = ["sed", "-i.bak", "s/hostname=localhost/hostname=" + server_hostname + "/g", "/etc/ambari-agent/conf/ambari-agent.ini"] - execOsCommand(osCommand) + ret = execOsCommand(osCommand) + if ret['exitstatus'] != 0: + return ret osCommand = ["sed", "-i.bak", "s/run_as_user=.*$/run_as_user=" + user_run_as + "/g", "/etc/ambari-agent/conf/ambari-agent.ini"] - execOsCommand(osCommand) - return + ret = execOsCommand(osCommand) + return ret def runAgent(passPhrase, expected_hostname, user_run_as, verbose): os.environ[AMBARI_PASSPHRASE_VAR] = passPhrase @@ -75,18 +77,21 @@ def runAgent(passPhrase, expected_hostname, user_run_as, verbose): if verbose: vo = " -v" cmd = "su - %1s -c '/usr/sbin/ambari-agent restart --expected-hostname=%2s %3s'" % (user_run_as, expected_hostname, vo) - agent_retcode = subprocess.call(cmd, shell=True) + log = "" + p = subprocess.Popen(cmd, stdout=subprocess.PIPE, shell=True) + p.communicate() + agent_retcode = p.returncode for i in range(3): time.sleep(1) ret = execOsCommand(["tail", "-20", "/var/log/ambari-agent/ambari-agent.log"]) - if (not ret is None) and (0 == ret['exitstatus']): + if (0 == ret['exitstatus']): try: log = ret['log'] except Exception: log = "Log not found" print log - return agent_retcode - return agent_retcode + break + return {"exitstatus": agent_retcode, "log": log} def tryStopAgent(): verbose = False @@ -160,15 +165,14 @@ def checkServerReachability(host, port): ret = {} s = socket.socket() try: - s.connect((host, port)) - return + s.connect((host, port)) + ret = {"exitstatus": 0, "log": ""} except Exception: - ret["exitstatus"] = 1 - ret["log"] = "Host registration aborted. Ambari Agent host cannot reach Ambari Server '" +\ + ret["exitstatus"] = 1 + ret["log"] = "Host registration aborted. Ambari Agent host cannot reach Ambari Server '" +\ host+":"+str(port) + "'. " +\ "Please check the network connectivity between the Ambari Agent host and the Ambari Server" - sys.exit(ret) - pass + return ret # Command line syntax help @@ -183,10 +187,10 @@ def checkServerReachability(host, port): def parseArguments(argv=None): if argv is None: # make sure that arguments was passed - sys.exit(1) + return {"exitstatus": 2, "log": "No arguments were passed"} args = argv[1:] # shift path to script if len(args) < 3: - sys.exit({"exitstatus": 1, "log": "Was passed not all required arguments"}) + return {"exitstatus": 1, "log": "Not all required arguments were passed"} expected_hostname = args[0] passPhrase = args[1] @@ -204,48 +208,62 @@ def parseArguments(argv=None): except (Exception): server_port = 8080 - return expected_hostname, passPhrase, hostname, user_run_as, projectVersion, server_port + parsed_args = (expected_hostname, passPhrase, hostname, user_run_as, projectVersion, server_port) + return {"exitstatus": 0, "log": "", "parsed_args": parsed_args} def run_setup(argv=None): # Parse passed arguments - expected_hostname, passPhrase, hostname, user_run_as, projectVersion, server_port = parseArguments(argv) - checkServerReachability(hostname, server_port) - + retcode = parseArguments(argv) + if (retcode["exitstatus"] != 0): + return retcode + + (expected_hostname, passPhrase, hostname, user_run_as, projectVersion, server_port) = retcode["parsed_args"] + + retcode = checkServerReachability(hostname, server_port) + if (retcode["exitstatus"] != 0): + return retcode + if projectVersion == "null" or projectVersion == "{ambariVersion}" or projectVersion == "": retcode = getOptimalVersion("") else: retcode = getOptimalVersion(projectVersion) if retcode["exitstatus"] == 0 and retcode["log"] != None and retcode["log"] != "" and retcode["log"][0].strip() != "": - availiableProjectVersion = retcode["log"].strip() - if not isAgentPackageAlreadyInstalled(availiableProjectVersion): - ret = installAgent(availiableProjectVersion) - if (not ret["exitstatus"] == 0): - sys.exit(ret) + availiableProjectVersion = retcode["log"].strip() + if not isAgentPackageAlreadyInstalled(availiableProjectVersion): + retcode = installAgent(availiableProjectVersion) + if (not retcode["exitstatus"] == 0): + return retcode elif retcode["exitstatus"] == 1 and retcode["log"][0].strip() != "": - sys.exit({"exitstatus": 1, "log": "Desired version ("+projectVersion+") of ambari-agent package" + return {"exitstatus": 1, "log": "Desired version ("+projectVersion+") of ambari-agent package" " is not available." " Repository has following " - "versions of ambari-agent:"+retcode["log"][0].strip()}) + "versions of ambari-agent:"+retcode["log"][0].strip()} else: - sys.exit(retcode) - - configureAgent(hostname, user_run_as) - sys.exit(runAgent(passPhrase, expected_hostname, user_run_as, verbose)) + return retcode + + retcode = configureAgent(hostname, user_run_as) + if retcode['exitstatus'] != 0: + return retcode + return runAgent(passPhrase, expected_hostname, user_run_as, verbose) def main(argv=None): - #Try stop agent and check --verbose option if agent already run + #Try stop agent and check --verbose option if agent already run global verbose verbose = tryStopAgent() if verbose: - run_setup(argv) + exitcode = run_setup(argv) else: try: - run_setup(argv) + exitcode = run_setup(argv) except Exception, e: - print e - sys.exit(-1) + exitcode = {"exitstatus": -1, "log": str(e)} + return exitcode if __name__ == '__main__': logging.basicConfig(level=logging.DEBUG) - main(sys.argv) + ret = main(sys.argv) + retcode = ret["exitstatus"] + if 0 != retcode: + print ret["log"] + sys.exit(retcode) http://git-wip-us.apache.org/repos/asf/ambari/blob/5ff857cd/ambari-server/src/test/python/TestSetupAgent.py ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/python/TestSetupAgent.py b/ambari-server/src/test/python/TestSetupAgent.py index 249ce0b..031a642 100644 --- a/ambari-server/src/test/python/TestSetupAgent.py +++ b/ambari-server/src/test/python/TestSetupAgent.py @@ -27,20 +27,19 @@ class TestSetupAgent(TestCase): @patch("sys.exit") @patch("socket.socket") def test_checkServerReachability(self, socket_mock, exit_mock): - setup_agent.checkServerReachability("localhost", 8080) - self.assertTrue(socket_mock.called) - s = socket_mock.return_value - s.connect = MagicMock() - def side_effect(): - raise Exception(1, "socket is closed") - s.connect.side_effect = side_effect - try: - setup_agent.checkServerReachability("localhost", 8080) - self.fail("Should throw exception because port is closed") - except Exception: - # Expected - self.assertTrue(exit_mock.called) - pass + ret = setup_agent.checkServerReachability("localhost", 8080) + self.assertTrue(socket_mock.called) + + s = socket_mock.return_value + s.connect = MagicMock() + def side_effect(): + raise Exception(1, "socket is closed") + s.connect.side_effect = side_effect + ret = setup_agent.checkServerReachability("localhost", 8080) + self.assertTrue("exitstatus" in ret) + self.assertEqual(ret["exitstatus"], 1) + self.assertTrue("log" in ret) + pass @patch.object(setup_agent, 'execOsCommand') @@ -50,59 +49,66 @@ class TestSetupAgent(TestCase): setup_agent.configureAgent(hostname, "root") cmdStr = str(execOsCommand_mock.call_args_list[0][0]) self.assertTrue(hostname in cmdStr) + pass @patch.object(setup_agent, 'execOsCommand') @patch("os.environ") - @patch("subprocess.call") + @patch("subprocess.Popen") @patch("time.sleep") - def test_runAgent(self, sleep_mock, call_mock, environ_mock, execOsCommand_mock): + def test_runAgent(self, sleep_mock, popen_mock, environ_mock, execOsCommand_mock): expected_hostname = "test.hst" passphrase = "passphrase" - call_mock.return_value = 0 + agent_status = MagicMock() + agent_status.returncode = 0 + popen_mock.return_value = agent_status execOsCommand_mock.return_value = {'log': 'log', 'exitstatus': 0} # Test if expected_hostname is passed ret = setup_agent.runAgent(passphrase, expected_hostname, "root", False) - cmdStr = str(call_mock.call_args_list[0][0]) + cmdStr = str(popen_mock.call_args_list[0][0]) self.assertTrue(expected_hostname in cmdStr) self.assertFalse('-v' in cmdStr) - self.assertEqual(ret, 0) + self.assertEqual(ret["exitstatus"], 0) self.assertTrue(sleep_mock.called) self.assertEqual(execOsCommand_mock.call_count, 1) + execOsCommand_mock.reset_mock() - call_mock.reset_mock() + popen_mock.reset_mock() sleep_mock.reset_mock() # Test if verbose=True ret = setup_agent.runAgent(passphrase, expected_hostname, "root", True) self.assertTrue(expected_hostname in cmdStr) - cmdStr = str(call_mock.call_args_list[0][0]) + cmdStr = str(popen_mock.call_args_list[0][0]) self.assertTrue('-v' in cmdStr) - self.assertEqual(ret, 0) + self.assertEqual(ret["exitstatus"], 0) self.assertTrue(sleep_mock.called) self.assertEqual(execOsCommand_mock.call_count, 1) + execOsCommand_mock.reset_mock() - call_mock.reset_mock() + popen_mock.reset_mock() sleep_mock.reset_mock() # Key 'log' not found - execOsCommand_mock.return_value = None + execOsCommand_mock.return_value = {'log': 'log', 'exitstatus': 1} ret = setup_agent.runAgent(passphrase, expected_hostname, "root", False) - cmdStr = str(call_mock.call_args_list[0][0]) + cmdStr = str(popen_mock.call_args_list[0][0]) self.assertTrue(expected_hostname in cmdStr) - self.assertEqual(ret, 0) + self.assertEqual(ret["exitstatus"], 0) self.assertEqual(execOsCommand_mock.call_count, 3) + execOsCommand_mock.reset_mock() - call_mock.reset_mock() + popen_mock.reset_mock() # Retcode id not 0 - call_mock.return_value = 2 + agent_status.returncode = 2 execOsCommand_mock.return_value = {'log': 'log', 'exitstatus': 2} ret = setup_agent.runAgent(passphrase, expected_hostname, "root", False) - cmdStr = str(call_mock.call_args_list[0][0]) + cmdStr = str(popen_mock.call_args_list[0][0]) self.assertTrue(expected_hostname in cmdStr) - self.assertEqual(ret, 2) + self.assertEqual(ret["exitstatus"], 2) execOsCommand_mock.reset_mock() + pass @patch.object(setup_agent, 'getAvaliableAgentPackageVersions') @patch('ambari_commons.OSCheck.is_suse_family') @@ -215,6 +221,7 @@ class TestSetupAgent(TestCase): self.assertTrue(findNearestAgentPackageVersion_method.called) self.assertTrue(result_version["exitstatus"] == 1) + pass @patch.object(subprocess, 'Popen') def test_execOsCommand(self, Popen_mock): @@ -236,13 +243,18 @@ class TestSetupAgent(TestCase): getOptimalVersion_mock, is_ubuntu_family_mock, is_suse_family_mock, installAgent_mock, configureAgent_mock, runAgent_mock, isAgentPackageAlreadyInstalled_mock, tryStopAgent_mock): + checkServerReachability_mock.return_value = {'log': 'log', 'exitstatus': 0} installAgent_mock.return_value = {'log': 'log', 'exitstatus': 0} - runAgent_mock.return_value = 0 + configureAgent_mock.return_value = {'log': 'log', 'exitstatus': 0} + runAgent_mock.return_value = {'log': 'log', 'exitstatus': 0} getOptimalVersion_mock.return_value = {'log': '1.1.2, 1.1.3, ', 'exitstatus': 1} - setup_agent.main(("setupAgent.py","agents_host","password", "server_hostname","1.1.1","8080")) + ret = setup_agent.main(("setupAgent.py","agents_host","password", "server_hostname","1.1.1","8080")) self.assertTrue(tryStopAgent_mock.called) - self.assertTrue(exit_mock.called) + self.assertFalse(exit_mock.called) + self.assertTrue("exitstatus" in ret) + self.assertEqual(ret["exitstatus"], 1) self.assertTrue(getOptimalVersion_mock.called) + exit_mock.reset_mock() getOptimalVersion_mock.reset_mock() @@ -250,13 +262,16 @@ class TestSetupAgent(TestCase): isAgentPackageAlreadyInstalled_mock.return_value = False is_suse_family_mock.return_value = True is_ubuntu_family_mock.return_value = False - setup_agent.main(("setupAgent.py","agents_host","password", "server_hostname","1.1.1","8080")) - self.assertTrue(exit_mock.called) + ret = setup_agent.main(("setupAgent.py","agents_host","password", "server_hostname","1.1.1","8080")) + self.assertFalse(exit_mock.called) self.assertTrue(getOptimalVersion_mock.called) self.assertTrue(isAgentPackageAlreadyInstalled_mock.called) self.assertTrue(installAgent_mock.called) self.assertFalse(is_suse_family_mock.called) self.assertFalse(is_ubuntu_family_mock.called) + self.assertTrue("exitstatus" in ret) + self.assertEqual(ret["exitstatus"], 0) + exit_mock.reset_mock() getOptimalVersion_mock.reset_mock() isAgentPackageAlreadyInstalled_mock.reset_mock() @@ -265,12 +280,14 @@ class TestSetupAgent(TestCase): installAgent_mock.reset_mock() getOptimalVersion_mock.return_value = {'log': '', 'exitstatus': 0} - setup_agent.main(("setupAgent.py","agents_host","password", "server_hostname","1.1.1","8080")) - self.assertTrue(exit_mock.called) + ret = setup_agent.main(("setupAgent.py","agents_host","password", "server_hostname","1.1.1","8080")) + self.assertFalse(exit_mock.called) self.assertTrue(getOptimalVersion_mock.called) self.assertFalse(isAgentPackageAlreadyInstalled_mock.called) self.assertFalse(is_suse_family_mock.called) self.assertFalse(is_ubuntu_family_mock.called) + self.assertTrue("exitstatus" in ret) + self.assertEqual(ret["exitstatus"], 0) exit_mock.reset_mock() getOptimalVersion_mock.reset_mock() @@ -282,13 +299,16 @@ class TestSetupAgent(TestCase): is_suse_family_mock.return_value = False is_ubuntu_family_mock.return_value = False getOptimalVersion_mock.return_value = {'log': '1.1.1', 'exitstatus': 0} - setup_agent.main(("setupAgent.py","agents_host","password", "server_hostname","1.1.1","8080")) - self.assertTrue(exit_mock.called) + ret = setup_agent.main(("setupAgent.py","agents_host","password", "server_hostname","1.1.1","8080")) + self.assertFalse(exit_mock.called) self.assertTrue(getOptimalVersion_mock.called) self.assertTrue(isAgentPackageAlreadyInstalled_mock.called) self.assertTrue(installAgent_mock.called) self.assertFalse(is_suse_family_mock.called) self.assertFalse(is_ubuntu_family_mock.called) + self.assertTrue("exitstatus" in ret) + self.assertEqual(ret["exitstatus"], 0) + exit_mock.reset_mock() getOptimalVersion_mock.reset_mock() isAgentPackageAlreadyInstalled_mock.reset_mock() @@ -299,32 +319,36 @@ class TestSetupAgent(TestCase): is_ubuntu_family_mock.reset_mock() installAgent_mock.reset_mock() - setup_agent.main(("setupAgent.py","agents_host","password", "server_hostname","{ambariVersion}","8080")) + ret = setup_agent.main(("setupAgent.py","agents_host","password", "server_hostname","{ambariVersion}","8080")) self.assertTrue(getOptimalVersion_mock.called) - self.assertTrue(exit_mock.called) + self.assertFalse(exit_mock.called) + self.assertTrue("exitstatus" in ret) + self.assertEqual(ret["exitstatus"], 0) + exit_mock.reset_mock() getOptimalVersion_mock.reset_mock() - setup_agent.main(("setupAgent.py","agents_host","password", "server_hostname","null","8080")) - self.assertTrue(exit_mock.called) + ret = setup_agent.main(("setupAgent.py","agents_host","password", "server_hostname","null","8080")) + self.assertFalse(exit_mock.called) self.assertTrue(getOptimalVersion_mock.called) + self.assertTrue("exitstatus" in ret) + self.assertEqual(ret["exitstatus"], 0) + exit_mock.reset_mock() is_suse_family_mock.return_value = False is_ubuntu_family_mock.return_value = False - setup_agent.main(("setupAgent.py","agents_host","password", "server_hostname","null","null")) - self.assertTrue(exit_mock.called) + ret = setup_agent.main(("setupAgent.py","agents_host","password", "server_hostname","null","null")) + self.assertFalse(exit_mock.called) + self.assertTrue("exitstatus" in ret) + self.assertEqual(ret["exitstatus"], 0) + exit_mock.reset_mock() - def side_effect(retcode): - raise Exception(retcode, "sys.exit") - exit_mock.side_effect = side_effect #if "yum -y install --nogpgcheck ambari-agent" return not 0 result installAgent_mock.return_value = {'log': 'log', 'exitstatus': 1} - try: - setup_agent.main(("setupAgent.py","agents_host","password", "server_hostname","1.1.1","8080")) - self.fail("Should throw exception") - except Exception: - # Expected - pass - self.assertTrue(exit_mock.called) + ret = setup_agent.main(("setupAgent.py","agents_host","password", "server_hostname","1.1.1","8080")) + self.assertFalse(exit_mock.called) + self.assertTrue("exitstatus" in ret) + self.assertEqual(ret["exitstatus"], 1) + installAgent_mock.reset_mock() exit_mock.reset_mock() #if suse @@ -332,51 +356,51 @@ class TestSetupAgent(TestCase): is_ubuntu_family_mock.return_value = False #if "zypper install -y ambari-agent" return not 0 result installAgent_mock.return_value = {'log': 'log', 'exitstatus': 1} - try: - setup_agent.main(("setupAgent.py","agents_host","password", "server_hostname","1.1.1","8080")) - self.fail("Should throw exception") - except Exception: - # Expected - pass - self.assertTrue(exit_mock.called) + ret = setup_agent.main(("setupAgent.py","agents_host","password", "server_hostname","1.1.1","8080")) + self.assertFalse(exit_mock.called) + self.assertTrue("exitstatus" in ret) + self.assertEqual(ret["exitstatus"], 1) + exit_mock.reset_mock() #if ubuntu is_suse_family_mock.return_value = False is_ubuntu_family_mock.return_value = True installAgent_mock.return_value = {'log': 'log', 'exitstatus': 1} - try: - setup_agent.main(("setupAgent.py","agents_host","password", "server_hostname","1.1.1","8080")) - self.fail("Should throw exception") - except Exception: - # Expected - pass - self.assertTrue(exit_mock.called) + ret = setup_agent.main(("setupAgent.py","agents_host","password", "server_hostname","1.1.1","8080")) + self.assertFalse(exit_mock.called) + self.assertTrue("exitstatus" in ret) + self.assertEqual(ret["exitstatus"], 1) + pass @patch.object(setup_agent, 'execOsCommand') def test_findNearestAgentPackageVersion(self, execOsCommand_mock): - setup_agent.findNearestAgentPackageVersion("1.1.1") - self.assertTrue(execOsCommand_mock.called) - execOsCommand_mock.reset_mock() - setup_agent.findNearestAgentPackageVersion("") - self.assertTrue(execOsCommand_mock.called) + setup_agent.findNearestAgentPackageVersion("1.1.1") + self.assertTrue(execOsCommand_mock.called) + execOsCommand_mock.reset_mock() + setup_agent.findNearestAgentPackageVersion("") + self.assertTrue(execOsCommand_mock.called) + pass @patch.object(setup_agent, 'execOsCommand') def test_isAgentPackageAlreadyInstalled(self, execOsCommand_mock): - execOsCommand_mock.return_value = {"exitstatus": 0, "log": "1.1.1"} - self.assertTrue(setup_agent.isAgentPackageAlreadyInstalled("1.1.1")) - self.assertTrue(execOsCommand_mock.called) - execOsCommand_mock.reset_mock() - execOsCommand_mock.return_value = {"exitstatus": 1, "log": "1.1.1"} - self.assertFalse(setup_agent.isAgentPackageAlreadyInstalled("1.1.1")) - self.assertTrue(execOsCommand_mock.called) + execOsCommand_mock.return_value = {"exitstatus": 0, "log": "1.1.1"} + self.assertTrue(setup_agent.isAgentPackageAlreadyInstalled("1.1.1")) + self.assertTrue(execOsCommand_mock.called) + execOsCommand_mock.reset_mock() + execOsCommand_mock.return_value = {"exitstatus": 1, "log": "1.1.1"} + self.assertFalse(setup_agent.isAgentPackageAlreadyInstalled("1.1.1")) + self.assertTrue(execOsCommand_mock.called) + pass @patch.object(setup_agent, 'execOsCommand') def test_getAvaliableAgentPackageVersions(self, execOsCommand_mock): - setup_agent.getAvaliableAgentPackageVersions() - self.assertTrue(execOsCommand_mock.called) + setup_agent.getAvaliableAgentPackageVersions() + self.assertTrue(execOsCommand_mock.called) + pass @patch.object(setup_agent, 'execOsCommand') def test_installAgent(self, execOsCommand_mock): setup_agent.installAgent("1.1.1") self.assertTrue(execOsCommand_mock.called) + pass