Rename ResourceProfile.check_if_sa_running()

* Update log message and rename method to make debugging easier
* Fix pep8 violations in touched files

JIRA: YARDSTICK-859

Change-Id: Id138d72cd8557ffacd5dcc81d06feeb484e13625
Signed-off-by: Emma Foley <emma.l.foley@intel.com>

7 files changed, 53 insertions, 55 deletions

diff --git a/tests/unit/network_services/nfvi/test_resource.py b/tests/unit/network_services/nfvi/test_resource.py
index 5c2f890e8..4584b093d 100644
--- a/tests/unit/network_services/nfvi/test_resource.py
+++ b/tests/unit/network_services/nfvi/test_resource.py
@@ -12,11 +12,10 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from __future__ import absolute_import
-import unittest
-
 import errno
+
 import mock
+import unittest
 
 from yardstick.network_services.nfvi.resource import ResourceProfile
 from yardstick.network_services.nfvi import resource, collectd
@@ -105,14 +104,16 @@ class TestResourceProfile(unittest.TestCase):
     def test___init__(self):
         self.assertEqual(True, self.resource_profile.enable)
 
-    def test_check_if_sa_running(self):
-        self.assertEqual(self.resource_profile.check_if_sa_running("collectd"),
+    def test_check_if_system_agent_running(self):
+        self.assertEqual(self.resource_profile.check_if_system_agent_running("collectd"),
                          (0, ""))
 
-    def test_check_if_sa_running_excetion(self):
+    def test_check_if_system_agent_running_excetion(self):
         with mock.patch.object(self.resource_profile.connection, "execute") as mock_execute:
             mock_execute.side_effect = OSError(errno.ECONNRESET, "error")
-            self.assertEqual(self.resource_profile.check_if_sa_running("collectd"), (1, None))
+            self.assertEqual(
+                self.resource_profile.check_if_system_agent_running("collectd"),
+                (1, None))
 
     def test_get_cpu_data(self):
         reskey = ["", "cpufreq", "cpufreq-0"]
@@ -139,8 +140,7 @@ class TestResourceProfile(unittest.TestCase):
             self.resource_profile._setup_ovs_stats(self.ssh_mock))
 
     @mock.patch("yardstick.network_services.nfvi.resource.open")
-    @mock.patch("yardstick.network_services.nfvi.resource.os")
-    def test__provide_config_file(self, mock_open, mock_os):
+    def test__provide_config_file(self, *args):
         loadplugin = range(5)
         port_names = range(5)
         kwargs = {
@@ -152,13 +152,13 @@ class TestResourceProfile(unittest.TestCase):
         self.ssh_mock.execute.assert_called_once()
 
     @mock.patch("yardstick.network_services.nfvi.resource.open")
-    def test_initiate_systemagent(self, mock_open):
+    def test_initiate_systemagent(self, *args):
         self.resource_profile._start_collectd = mock.Mock()
         self.assertIsNone(
             self.resource_profile.initiate_systemagent("/opt/nsb_bin"))
 
     @mock.patch("yardstick.network_services.nfvi.resource.open")
-    def test_initiate_systemagent_raise(self, mock_open):
+    def test_initiate_systemagent_raise(self, *args):
         self.resource_profile._start_collectd = mock.Mock(side_effect=RuntimeError)
         with self.assertRaises(RuntimeError):
             self.resource_profile.initiate_systemagent("/opt/nsb_bin")
@@ -267,8 +267,10 @@ class TestResourceProfile(unittest.TestCase):
     def test_stop(self):
         self.assertIsNone(self.resource_profile.stop())
 
-    def test_stop(self):
+    def test_stop_amqp_not_running(self):
         self.resource_profile.amqp_client = mock.MagicMock()
+        # TODO(efoley): Fix this incorrect test.
+        # Should check that we don't try to stop amqp when it's not running
         self.assertIsNone(self.resource_profile.stop())
 
 if __name__ == '__main__':
diff --git a/tests/unit/network_services/vnf_generic/vnf/test_prox_helpers.py b/tests/unit/network_services/vnf_generic/vnf/test_prox_helpers.py
index ed49c7000..0ac46c632 100644
--- a/tests/unit/network_services/vnf_generic/vnf/test_prox_helpers.py
+++ b/tests/unit/network_services/vnf_generic/vnf/test_prox_helpers.py
@@ -16,15 +16,17 @@
 #
 
 from itertools import repeat, chain
-import mock
 import os
 import socket
 import time
+
+import mock
 import unittest
 
 from tests.unit import STL_MOCKS
 from yardstick.network_services.vnf_generic.vnf.base import VnfdHelper
 
+
 STLClient = mock.MagicMock()
 stl_patch = mock.patch.dict("sys.modules", STL_MOCKS)
 stl_patch.start()
@@ -1433,9 +1435,9 @@ class TestProxResourceHelper(unittest.TestCase):
         helper = ProxResourceHelper(mock.MagicMock())
         helper.resource = resource = mock.MagicMock()
 
-        resource.check_if_sa_running.return_value = 0, '1234'
+        resource.check_if_system_agent_running.return_value = 0, '1234'
         resource.amqp_collect_nfvi_kpi.return_value = 543
-        resource.check_if_sa_running.return_value = (0, None)
+        resource.check_if_system_agent_running.return_value = (0, None)
 
         expected = {'core': 543}
         result = helper.collect_collectd_kpi()
@@ -1447,9 +1449,9 @@ class TestProxResourceHelper(unittest.TestCase):
         helper._result = {'z': 123}
         helper.resource = resource = mock.MagicMock()
 
-        resource.check_if_sa_running.return_value = 0, '1234'
+        resource.check_if_system_agent_running.return_value = 0, '1234'
         resource.amqp_collect_nfvi_kpi.return_value = 543
-        resource.check_if_sa_running.return_value = (0, None)
+        resource.check_if_system_agent_running.return_value = (0, None)
 
         queue.empty.return_value = False
         queue.get.return_value = {'a': 789}
diff --git a/tests/unit/network_services/vnf_generic/vnf/test_tg_prox.py b/tests/unit/network_services/vnf_generic/vnf/test_tg_prox.py
index 7b4d79e02..0104e7f63 100644
--- a/tests/unit/network_services/vnf_generic/vnf/test_tg_prox.py
+++ b/tests/unit/network_services/vnf_generic/vnf/test_tg_prox.py
@@ -330,7 +330,7 @@ class TestProxTrafficGen(unittest.TestCase):
 
         prox_traffic_gen = ProxTrafficGen(NAME, self.VNFD0)
         prox_traffic_gen._vnf_wrapper.resource_helper.resource = mock.MagicMock(
-            **{"check_if_sa_running.return_value": [False]})
+            **{"self.check_if_system_agent_running.return_value": [False]})
         prox_traffic_gen._vnf_wrapper.vnf_execute = mock.Mock(return_value="")
         self.assertEqual({}, prox_traffic_gen.collect_kpi())
 
diff --git a/tests/unit/network_services/vnf_generic/vnf/test_vpe_vnf.py b/tests/unit/network_services/vnf_generic/vnf/test_vpe_vnf.py
index 55cd4d2e8..1abc53688 100644
--- a/tests/unit/network_services/vnf_generic/vnf/test_vpe_vnf.py
+++ b/tests/unit/network_services/vnf_generic/vnf/test_vpe_vnf.py
@@ -15,11 +15,12 @@
 # limitations under the License.
 #
 
-import mock
 from multiprocessing import Process, Queue
 import os
-import six.moves.configparser as configparser
 import time
+
+import mock
+import six.moves.configparser as configparser
 import unittest
 
 from tests.unit import STL_MOCKS
@@ -543,9 +544,9 @@ class TestVpeApproxVnf(unittest.TestCase):
         mock_ssh(ssh)
 
         resource = mock.Mock(autospec=ResourceProfile)
-        resource.check_if_sa_running.return_value = 1, ''
+        resource.check_if_system_agent_running.return_value = 1, ''
         resource.amqp_collect_nfvi_kpi.return_value = {'foo': 234}
-        resource.check_if_sa_running.return_value = (1, None)
+        resource.check_if_system_agent_running.return_value = (1, None)
 
         vpe_approx_vnf = VpeApproxVnf(NAME, self.VNFD_0)
         vpe_approx_vnf.q_in = mock.MagicMock()
@@ -567,7 +568,7 @@ class TestVpeApproxVnf(unittest.TestCase):
         mock_ssh(ssh)
 
         resource = mock.Mock(autospec=ResourceProfile)
-        resource.check_if_sa_running.return_value = 0, '1234'
+        resource.check_if_system_agent_running.return_value = 0, '1234'
         resource.amqp_collect_nfvi_kpi.return_value = {'foo': 234}
 
         vpe_approx_vnf = VpeApproxVnf(NAME, self.VNFD_0)
diff --git a/yardstick/network_services/collector/subscriber.py b/yardstick/network_services/collector/subscriber.py
index 4dc5a796e..7e18302eb 100644
--- a/yardstick/network_services/collector/subscriber.py
+++ b/yardstick/network_services/collector/subscriber.py
@@ -62,12 +62,13 @@ class Collector(object):
             # Result example:
             # {"VNF1: { "tput" : [1000, 999] }, "VNF2": { "latency": 100 }}
             LOG.debug("collect KPI for %s", node_name)
-            if resource.check_if_sa_running("collectd")[0] != 0:
+            if resource.check_if_system_agent_running("collectd")[0] != 0:
                 continue
 
             try:
                 results[node_name] = {"core": resource.amqp_collect_nfvi_kpi()}
                 LOG.debug("%s collect KPIs %s", node_name, results[node_name]['core'])
-            except Exception:
-                LOG.exception("")
+            # NOTE(elfoley): catch a more specific error
+            except Exception as exc:  # pylint: disable=broad-except
+                LOG.exception(exc)
         return results
diff --git a/yardstick/network_services/nfvi/resource.py b/yardstick/network_services/nfvi/resource.py
index adf4d8ae6..dc5c46a86 100644
--- a/yardstick/network_services/nfvi/resource.py
+++ b/yardstick/network_services/nfvi/resource.py
@@ -13,20 +13,16 @@
 # limitations under the License.
 """ Resource collection definitions """
 
-from __future__ import absolute_import
-from __future__ import print_function
-
-import logging
-from itertools import chain
-
 import errno
-import jinja2
+from itertools import chain
+import logging
+import multiprocessing
 import os
 import os.path
 import re
-import multiprocessing
-import pkg_resources
 
+import jinja2
+import pkg_resources
 from oslo_config import cfg
 from oslo_utils.encodeutils import safe_decode
 
@@ -92,7 +88,7 @@ class ResourceProfile(object):
 
         return cls(node, plugins=plugins, interval=interval, timeout=timeout)
 
-    def check_if_sa_running(self, process):
+    def check_if_system_agent_running(self, process):
         """ verify if system agent is running """
         try:
             err, pid, _ = self.connection.execute("pgrep -f %s" % process)
@@ -101,7 +97,7 @@ class ResourceProfile(object):
         except OSError as e:
             if e.errno in {errno.ECONNRESET}:
                 # if we can't connect to check, then we won't be able to connect to stop it
-                LOG.exception("can't connect to host to check collectd status")
+                LOG.exception("Can't connect to host to check %s status", process)
                 return 1, None
             raise
 
@@ -327,7 +323,7 @@ class ResourceProfile(object):
             self.amqp_client.terminate()
 
         LOG.debug("Check if %s is running", agent)
-        status, pid = self.check_if_sa_running(agent)
+        status, pid = self.check_if_system_agent_running(agent)
         LOG.debug("status %s  pid %s", status, pid)
         if status != 0:
             return
diff --git a/yardstick/network_services/vnf_generic/vnf/sample_vnf.py b/yardstick/network_services/vnf_generic/vnf/sample_vnf.py
index 5eeb6c889..fbaaa0ca8 100644
--- a/yardstick/network_services/vnf_generic/vnf/sample_vnf.py
+++ b/yardstick/network_services/vnf_generic/vnf/sample_vnf.py
@@ -13,38 +13,34 @@
 # limitations under the License.
 """ Base class implementation for generic vnf implementation """
 
-from __future__ import absolute_import
-
-import posixpath
-import time
+from collections import Mapping
 import logging
+from multiprocessing import Queue, Value, Process
 import os
+import posixpath
 import re
-import subprocess
-from collections import Mapping
-from multiprocessing import Queue, Value, Process
-
 from six.moves import cStringIO
+import subprocess
+import time
 
+from trex_stl_lib.trex_stl_client import LoggerApi
+from trex_stl_lib.trex_stl_client import STLClient
+from trex_stl_lib.trex_stl_exceptions import STLError
 from yardstick.benchmark.contexts.base import Context
 from yardstick.benchmark.scenarios.networking.vnf_generic import find_relative_file
 from yardstick.common import exceptions as y_exceptions
 from yardstick.common.process import check_if_process_failed
+from yardstick.network_services.helpers.dpdkbindnic_helper import DpdkBindHelper
 from yardstick.network_services.helpers.samplevnf_helper import PortPairs
 from yardstick.network_services.helpers.samplevnf_helper import MultiPortConfig
-from yardstick.network_services.helpers.dpdkbindnic_helper import DpdkBindHelper
 from yardstick.network_services.nfvi.resource import ResourceProfile
+from yardstick.network_services.utils import get_nsb_option
 from yardstick.network_services.vnf_generic.vnf.base import GenericVNF
-from yardstick.network_services.vnf_generic.vnf.base import QueueFileWrapper
 from yardstick.network_services.vnf_generic.vnf.base import GenericTrafficGen
-from yardstick.network_services.utils import get_nsb_option
-
-from trex_stl_lib.trex_stl_client import STLClient
-from trex_stl_lib.trex_stl_client import LoggerApi
-from trex_stl_lib.trex_stl_exceptions import STLError
-
+from yardstick.network_services.vnf_generic.vnf.base import QueueFileWrapper
 from yardstick.ssh import AutoConnectSSH
 
+
 DPDK_VERSION = "dpdk-16.07"
 
 LOG = logging.getLogger(__name__)
@@ -346,7 +342,7 @@ class ResourceHelper(object):
 
     def _collect_resource_kpi(self):
         result = {}
-        status = self.resource.check_if_sa_running("collectd")[0]
+        status = self.resource.check_if_system_agent_running("collectd")[0]
         if status == 0:
             result = self.resource.amqp_collect_nfvi_kpi()