Ping utility can optionally survive DNS failure

- Previously, DNS was queried within the `Ping` utility constructor, so a DNS failure would always raised an exception.
- Now the DNS query is in the standard `run()` method, so a failure from DNS will raise optionally, depending on the `validateAfter` parameter.
- `Command` declared as a new style class so that `super(Ping, self).run()` can be called.
Co-authored-by: NLarry Hamel <lhamel@pivotal.io>
Co-authored-by: NJemish Patel <jpatel@pivotal.io>

gpMgmt/bin/gppylib/commands/base.py

@@ -675,7 +675,7 @@ class RMIExecutionContext(ExecutionContext):
         pass
 
 
-class Command:
+class Command(object):
     """ TODO:
     """
     name = None

gpMgmt/bin/gppylib/commands/unix.py

@@ -388,17 +388,29 @@ class Ping(Command):
     def __init__(self, name, hostToPing, ctxt=LOCAL, remoteHost=None, obj=None):
         self.hostToPing = hostToPing
         self.obj = obj
-        pingToUse = findCmdInPath('ping')
+        self.pingToUse = findCmdInPath('ping')
+        cmdStr = "%s -c 1 %s" % (self.pingToUse, self.hostToPing)
+        Command.__init__(self, name, cmdStr, ctxt, remoteHost)
 
+    def run(self, validateAfter=False):
         if curr_platform == LINUX or curr_platform == DARWIN or curr_platform == OPENBSD:
             # Get the family of the address we need to ping.  If it's AF_INET6
             # we must use ping6 to ping it.
-            addrinfo = socket.getaddrinfo(hostToPing, None)
-            if addrinfo and addrinfo[0] and addrinfo[0][0] == socket.AF_INET6:
-                pingToUse = SYSTEM.getPing6()
 
-        cmdStr = "%s -c 1 %s" % (pingToUse, hostToPing)
-        Command.__init__(self, name, cmdStr, ctxt, remoteHost)
+            try:
+                addrinfo = socket.getaddrinfo(self.hostToPing, None)
+                if addrinfo and addrinfo[0] and addrinfo[0][0] == socket.AF_INET6:
+                    self.pingToUse = SYSTEM.getPing6()
+                    self.cmdStr = "%s -c 1 %s" % (self.pingToUse, self.hostToPing)
+            except Exception as e:
+                self.results = CommandResult(1, '', 'Failed to get ip address: ' + str(e), False, True)
+                if validateAfter:
+                    self.validate()
+                else:
+                    # we know the next step of running ping is useless
+                    return
+
+        super(Ping, self).run(validateAfter)
 
     @staticmethod
     def ping_list(host_list):

gpMgmt/bin/gppylib/operations/test/unit/test_unit_ping.py

+from mock import patch
+from test.unit.gp_unittest import GpTestCase, run_tests
+from gppylib.commands.base import CommandResult
+from gppylib.commands.unix import Ping
+
+
+class PingTestCase(GpTestCase):
+    ping_command = None
+
+    def set_successful_command_results(self, _):
+        self.ping_command.set_results(CommandResult(0, '', '', True, True))
+
+    def setUp(self):
+        self.apply_patches([
+            # used for failure tests, re-patched for success in success tests
+            patch('gppylib.commands.unix.socket.getaddrinfo', side_effect=Exception('Test Exception')),
+            # always succeed if we call the superclass run() method
+            patch('gppylib.operations.unix.Command.run', side_effect=self.set_successful_command_results),
+        ])
+
+    def test_ping_cmdStr_is_correct(self):
+        ping_command = Ping("testing", "doesNotExist.foo.com")
+        self.assertIn("-c 1 doesNotExist.foo.com", ping_command.cmdStr)
+
+    # patch the patch to provide success
+    @patch('gppylib.commands.unix.socket.getaddrinfo', return_value=[(2, 2, 17, '', ('172.217.6.46', 0)), (2, 1, 6, '', ('172.217.6.46', 0)), (30, 2, 17, '', ('2607:f8b0:4005:809::200e', 0, 0, 0)), (30, 1, 6, '', ('2607:f8b0:4005:809::200e', 0, 0, 0))])
+    def test_happy_path_without_validation_pings(self, socket):
+        self.ping_command = Ping("testing", "google.com")
+        self.ping_command.run(validateAfter=False)
+        self.assertEquals(self.ping_command.get_results().rc, 0)
+
+    # patch the patch to provide success
+    @patch('gppylib.commands.unix.socket.getaddrinfo', return_value=[(2, 2, 17, '', ('172.217.6.46', 0)), (2, 1, 6, '', ('172.217.6.46', 0)), (30, 2, 17, '', ('2607:f8b0:4005:809::200e', 0, 0, 0)), (30, 1, 6, '', ('2607:f8b0:4005:809::200e', 0, 0, 0))])
+    def test_happy_path_with_validation_pings(self, socket):
+        self.ping_command = Ping("testing", "google.com")
+        self.ping_command.run(validateAfter=True)
+        self.assertEquals(self.ping_command.get_results().rc, 0)
+
+    def test_ping_survives_dns_failure(self):
+        ping_command = Ping("testing", "doesNotExist.foo.com")
+        ping_command.run(validateAfter=False)
+        self.assertEquals(ping_command.get_results().rc, 1)
+        self.assertIn("Failed to get ip address", ping_command.get_results().stderr)
+
+    def test_ping_when_validating_fails_on_dns_failure(self):
+        ping_command = Ping("testing", "doesNotExist.foo.com")
+        with self.assertRaisesRegexp(Exception, 'Test Exception'):
+            ping_command.run(validateAfter=True)
+
+
+if __name__ == '__main__':
+    run_tests()