address review comment from stacy

githubpkim · Oct 7, 2016 · 332ea14 · 332ea14
1 parent 5a4e796
commit 332ea14
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 7 deletions.
diff --git a/lib/jnpr/junos/utils/sw.py b/lib/jnpr/junos/utils/sw.py
@@ -208,8 +208,9 @@ def pkgaddISSU(self, remote_package, **kvargs):
     def _parse_pkgadd_response(self, rsp):
         got = rsp.getparent()
         rc = int(got.findtext('package-result').strip())
-        if rc != 0:
-            self.log("software pkgadd message: %s" % got.findtext('output'))
+        output_msg = '\n'.join([i.text for i in got.findall('output')])
+        self.log("software pkgadd package-result: %s\nOutput: %s" % (
+            rc, output_msg))
         return rc == 0
 
     # -------------------------------------------------------------------------
@@ -232,8 +233,9 @@ def validate(self, remote_package, issu=False, **kwargs):
             rsp = self.rpc.request_package_validate(
                 package_name=remote_package, **kwargs).getparent()
         rc = int(rsp.findtext('package-result'))
-        if rc != 0:
-            self.log("software validation message: %s" % rsp.findtext('output'))
+        output_msg = '\n'.join([i.text for i in rsp.findall('output')])
+        self.log("software validate package-result: %s\nOutput: %s" % (
+            rc, output_msg))
         return 0 == rc
 
     def remote_checksum(self, remote_package, timeout=300):
@@ -446,6 +448,9 @@ def myprogress(dev, report):
         if issu is True and nssu is True:
             raise TypeError(
                 'install function can either take issu or nssu not both')
+        elif (issu is True or nssu is True) and self._multi_RE is not True:
+            raise TypeError(
+                'ISSU/NSSU requires Multi RE setup')
 
         def _progress(report):
             if progress is True:

diff --git a/tests/unit/utils/test_sw.py b/tests/unit/utils/test_sw.py
@@ -35,6 +35,8 @@
          '2RE': False, 'serialnumber': 'aaf5fe5f9b88', 'fqdn': 'firefly',
          'virtual': True, 'switch_style': 'NONE', 'version': '12.1X46-D15.3',
          'HOME': '/cf/var/home/rick', 'srx_cluster': False,
+         'version_RE0': '16.1-20160925.0',
+         'version_RE1': '16.1-20160925.0',
          'model': 'FIREFLY-PERIMETER',
          'RE0': {'status': 'Testing',
                  'last_reboot_reason': 'Router rebooted after a '
@@ -77,7 +79,7 @@ def test_sw_hashfile(self):
     def test_sw_constructor_multi_re(self, mock_execute):
         mock_execute.side_effect = self._mock_manager
         self.sw = SW(self.dev)
-        self.assertFalse(self.sw._multi_RE)
+        self.assertTrue(self.sw._multi_RE)
 
     @patch('jnpr.junos.Device.execute')
     def test_sw_constructor_multi_vc(self, mock_execute):
@@ -181,6 +183,14 @@ def test_sw_install_issu_nssu_both_error(self, mock_execute):
         self.assertRaises(TypeError, self.sw.install, package,
                           nssu=True, issu=True)
 
+    @patch('jnpr.junos.Device.execute')
+    def test_sw_install_issu_single_re_error(self, mock_execute):
+        mock_execute.side_effect = self._mock_manager
+        package = 'test.tgz'
+        self.sw._multi_RE = False
+        self.assertRaises(TypeError, self.sw.install, package,
+                          nssu=True, issu=True)
+
     @patch('jnpr.junos.Device.execute')
     def test_sw_pkgaddISSU(self, mock_execute):
         mock_execute.side_effect = self._mock_manager
@@ -330,11 +340,11 @@ def test_sw_install_mixed_vc(self, mock_pkgadd):
     @patch('jnpr.junos.utils.sw.SW.pkgadd')
     def test_sw_install_multi_vc_mode_disabled(self, mock_pkgadd):
         mock_pkgadd.return_value = True
-        self.dev._facts = {
+        self.dev._facts = {'2RE': True,
             'domain': None, 'RE1': {
                 'status': 'OK', 'model': 'RE-EX8208',
                 'mastership_state': 'backup'}, 'ifd_style': 'SWITCH',
-            'version_RE1': '12.3R7.7', 'version_RE0': '12.3', '2RE': True,
+            'version_RE1': '12.3R7.7', 'version_RE0': '12.3',
             'serialnumber': 'XXXXXX', 'fqdn': 'XXXXXX',
             'RE0': {'status': 'OK', 'model': 'RE-EX8208',
                     'mastership_state': 'master'}, 'switch_style': 'VLAN',
@@ -405,6 +415,7 @@ def test_sw_install_mixed_vc_TypeError(self, mock_pkgadd):
     def test_sw_install_kwargs_force_host(self, mock_execute):
         self.sw.install('file', no_copy=True, force_host=True)
         rpc = [
+            '<request-package-add><force-host/><no-validate/><re1/><package-name>/var/tmp/file</package-name></request-package-add>',
             '<request-package-add><force-host/><no-validate/><package-name>/var/tmp/file</package-name></request-package-add>',
             '<request-package-add><force-host/><package-name>/var/tmp/file</package-name><no-validate/></request-package-add>',
             '<request-package-add><package-name>/var/tmp/file</package-name><no-validate/><force-host/></request-package-add>',
@@ -470,6 +481,7 @@ def test_sw_reboot_multi_re_vc(self, mock_execute):
     def test_sw_reboot_mixed_vc(self, mock_execute):
         mock_execute.side_effect = self._mock_manager
         self.sw._mixed_VC = True
+        self.sw._multi_VC = True
         self.sw.reboot()
         self.assertTrue('all-members' in
                         (etree.tostring(mock_execute.call_args[0][0]).decode('utf-8')))