Juniper private config

networking_generic_switch/devices/netmiko_devices/__init__.py

@@ -24,6 +24,7 @@
 from tooz import coordination
 
 from networking_generic_switch import devices
+from networking_generic_switch.devices import utils as device_utils
 from networking_generic_switch import exceptions as exc
 from networking_generic_switch import locking as ngs_lock
 
@@ -117,11 +118,11 @@ def _create_connection():
         except tenacity.RetryError as e:
             LOG.error("Reached maximum SSH connection attempts, not retrying")
             raise exc.GenericSwitchNetmikoConnectError(
-                config=self.config, error=e)
+                config=device_utils.sanitise_config(self.config), error=e)
         except Exception as e:
             LOG.error("Unexpected exception during SSH connection")
             raise exc.GenericSwitchNetmikoConnectError(
-                config=self.config, error=e)
+                config=device_utils.sanitise_config(self.config), error=e)
 
         # Now yield the connection to the caller.
         with net_connect:
@@ -135,15 +136,17 @@ def send_commands_to_device(self, cmd_set):
         try:
             with ngs_lock.PoolLock(self.locker, **self.lock_kwargs):
                 with self._get_connection() as net_connect:
-                    net_connect.enable()
-                    output = net_connect.send_config_set(
-                        config_commands=cmd_set)
+                    output = self.send_config_set(net_connect, cmd_set)
                     # NOTE (vsaienko) always save configuration
                     # when configuration is applied successfully.
                     self.save_configuration(net_connect)
+        except exc.GenericSwitchException as e:
+            # Reraise without modification exceptions originating from this
+            # module.
+            raise
         except Exception as e:
-            raise exc.GenericSwitchNetmikoConnectError(config=self.config,
-                                                       error=e)
+            raise exc.GenericSwitchNetmikoConnectError(
+                config=device_utils.sanitise_config(self.config), error=e)
 
         LOG.debug(output)
         return output
@@ -187,6 +190,16 @@ def delete_port(self, port, segmentation_id):
                                   port=port,
                                   segmentation_id=segmentation_id))
 
+    def send_config_set(self, net_connect, cmd_set):
+        """Send a set of configuration lines to the device.
+
+        :param net_connect: a netmiko connection object.
+        :param cmd_set: a list of configuration lines to send.
+        :returns: The output of the configuration commands.
+        """
+        net_connect.enable()
+        return net_connect.send_config_set(config_commands=cmd_set)
+
     def save_configuration(self, net_connect):
         """Save the device's configuration.
 

networking_generic_switch/devices/netmiko_devices/juniper.py

@@ -12,7 +12,23 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import logging
+
+import tenacity
+
 from networking_generic_switch.devices import netmiko_devices
+from networking_generic_switch.devices import utils as device_utils
+from networking_generic_switch import exceptions as exc
+
+LOG = logging.getLogger(__name__)
+
+# Internal ngs options will not be passed to driver.
+JUNIPER_INTERNAL_OPTS = [
+    # Timeout (seconds) for committing configuration changes.
+    {'name': 'ngs_commit_timeout', 'default': 60},
+    # Interval (seconds) between attempts to commit configuration changes.
+    {'name': 'ngs_commit_interval', 'default': 5},
+]
 
 
 class Juniper(netmiko_devices.NetmikoSwitch):
@@ -49,11 +65,89 @@ class Juniper(netmiko_devices.NetmikoSwitch):
         'vlan members {segmentation_id}',
     )
 
+    def __init__(self, device_cfg):
+        super(Juniper, self).__init__(device_cfg)
+
+        # Do not expose Juniper internal options to device config.
+        for opt in JUNIPER_INTERNAL_OPTS:
+            opt_name = opt['name']
+            if opt_name in self.config:
+                self.ngs_config[opt_name] = self.config.pop(opt_name)
+            elif 'default' in opt:
+                self.ngs_config[opt_name] = opt['default']
+
+    def send_config_set(self, net_connect, cmd_set):
+        """Send a set of configuration lines to the device.
+
+        :param net_connect: a netmiko connection object.
+        :param cmd_set: a list of configuration lines to send.
+        :returns: The output of the configuration commands.
+        """
+        # We use the private configuration mode, which hides the configuration
+        # changes of concurrent sessions from us, and discards uncommitted
+        # changes on termination of the session.
+        net_connect.config_mode(config_command='configure private')
+
+        # Don't exit configuration mode, as we still need to commit the changes
+        # in save_configuration().
+        return net_connect.send_config_set(config_commands=cmd_set,
+                                           exit_config_mode=False)
+
     def save_configuration(self, net_connect):
         """Save the device's configuration.
 
         :param net_connect: a netmiko connection object.
+        :raises GenericSwitchNetmikoConfigError if saving the configuration
+            fails.
         """
         # Junos configuration is transactional, and requires an explicit commit
-        # of changes in order for them to be applied.
-        net_connect.commit()
+        # of changes in order for them to be applied. Since committing requires
+        # an exclusive lock on the configuration database, it can fail if
+        # another session has a lock. We use a retry mechanism to work around
+        # this.
+
+        class DBLocked(Exception):
+            """Switch configuration DB is locked by another user."""
+
+            def __init__(self, err):
+                self.err = err
+
+        @tenacity.retry(
+            # Log a message after each failed attempt.
+            after=tenacity.after_log(LOG, logging.DEBUG),
+            # Reraise exceptions if our final attempt fails.
+            reraise=True,
+            # Retry on failure to commit the configuration due to the DB
+            # being locked by another session.
+            retry=(tenacity.retry_if_exception_type(DBLocked)),
+            # Stop after the configured timeout.
+            stop=tenacity.stop_after_delay(
+                int(self.ngs_config['ngs_commit_timeout'])),
+            # Wait for the configured interval between attempts.
+            wait=tenacity.wait_fixed(
+                int(self.ngs_config['ngs_commit_interval'])),
+        )
+        def commit():
+            try:
+                net_connect.commit()
+            except ValueError as e:
+                # Netmiko raises ValueError on commit failure, and appends the
+                # CLI output to the exception message. Raise a more specific
+                # exception for a locked DB, on which tenacity will retry.
+                if "error: configuration database locked" in str(e):
+                    raise DBLocked(e)
+                raise
+
+        try:
+            commit()
+        except DBLocked as e:
+            msg = ("Reached timeout waiting for switch configuration DB lock: "
+                   "%s" % e.err)
+            LOG.error(msg)
+            raise exc.GenericSwitchNetmikoConfigError(
+                config=device_utils.sanitise_config(self.config), error=msg)
+        except ValueError as e:
+            msg = "Failed to commit configuration: %s" % e
+            LOG.error(msg)
+            raise exc.GenericSwitchNetmikoConfigError(
+                config=device_utils.sanitise_config(self.config), error=msg)

networking_generic_switch/devices/utils.py

@@ -33,3 +33,16 @@ def get_switch_device(switches, switch_info=None,
                 return switch
     if switch_info:
         return switches.get(switch_info)
+
+
+def sanitise_config(config):
+    """Return a sanitised the configuration of a switch device.
+
+    :param config: a configuration dict to sanitise.
+    :returns: a copy of the configuration, with sensitive fields removed.
+    """
+    sanitised_fields = {"password"}
+    return {
+        key: "******" if key in sanitised_fields else value
+        for key, value in config.items()
+    }

networking_generic_switch/exceptions.py

@@ -35,3 +35,7 @@ class GenericSwitchNetmikoNotSupported(GenericSwitchException):
 
 class GenericSwitchNetmikoConnectError(GenericSwitchException):
     message = _("Netmiko connection error: %(config)s, error: %(error)s")
+
+
+class GenericSwitchNetmikoConfigError(GenericSwitchException):
+    message = _("Netmiko configuration error: %(config)s, error: %(error)s")

networking_generic_switch/tests/unit/devices/test_utils.py

@@ -52,3 +52,9 @@ def test_get_switch_device_fallback_to_switch_info(self):
         self.assertEqual(self.devices['A'], device_utils.get_switch_device(
             self.devices, switch_info='A',
             ngs_mac_address='11:22:33:44:55:77'))
+
+    def test_sanitise_config(self):
+        config = {'username': 'fake-user', 'password': 'fake-password'}
+        result = device_utils.sanitise_config(config)
+        expected = {'username': 'fake-user', 'password': '******'}
+        self.assertEqual(expected, result)

networking_generic_switch/tests/unit/netmiko/test_juniper.py

@@ -13,8 +13,12 @@
 #    under the License.
 
 import mock
+import netmiko
+import tenacity
 
+from networking_generic_switch.devices import netmiko_devices
 from networking_generic_switch.devices.netmiko_devices import juniper
+from networking_generic_switch import exceptions as exc
 from networking_generic_switch.tests.unit.netmiko import test_netmiko_base
 
 
@@ -51,7 +55,8 @@ def test_add_network_with_trunk_ports(self, mock_exec):
                 'NetmikoSwitch.send_commands_to_device')
     def test_del_network(self, mock_exec):
         self.switch.del_network(33, '0ae071f55be943e480eae41fefe85b21')
-        mock_exec.assert_called_with(['delete vlans 0ae071f55be943e480eae41fefe85b21'])
+        mock_exec.assert_called_with(
+            ['delete vlans 0ae071f55be943e480eae41fefe85b21'])
 
     @mock.patch('networking_generic_switch.devices.netmiko_devices.'
                 'NetmikoSwitch.send_commands_to_device')
@@ -83,11 +88,74 @@ def test_delete_port(self, mock_exec):
             ['delete interface 3333 unit 0 family ethernet-switching '
              'vlan members'])
 
+    def test_send_config_set(self):
+        connect_mock = mock.MagicMock(netmiko.base_connection.BaseConnection)
+        connect_mock.send_config_set.return_value = 'fake output'
+        result = self.switch.send_config_set(connect_mock, ['spam ham aaaa'])
+        self.assertFalse(connect_mock.enable.called)
+        connect_mock.send_config_set.assert_called_once_with(
+            config_commands=['spam ham aaaa'], exit_config_mode=False)
+        self.assertEqual('fake output', result)
+
     def test_save_configuration(self):
         mock_connection = mock.Mock()
         self.switch.save_configuration(mock_connection)
         mock_connection.commit.assert_called_once_with()
 
+    @mock.patch.object(netmiko_devices.tenacity, 'wait_fixed',
+                       return_value=tenacity.wait_fixed(0.01))
+    @mock.patch.object(netmiko_devices.tenacity, 'stop_after_delay',
+                       return_value=tenacity.stop_after_delay(0.1))
+    def test_save_configuration_timeout(self, m_stop, m_wait):
+        mock_connection = mock.Mock()
+        output = """
+error: configuration database locked by:
+  user terminal p0 (pid 1234) on since 2017-1-1 00:00:00 UTC
+      exclusive private [edit]
+
+{master:0}[edit]"""
+        mock_connection.commit.side_effect = ValueError(
+            "Commit failed with the following errors:\n\n{0}".format(output))
+
+        self.assertRaisesRegexp(exc.GenericSwitchNetmikoConfigError,
+                                "Reached timeout waiting for",
+                                self.switch.save_configuration,
+                                mock_connection)
+        self.assertGreater(mock_connection.commit.call_count, 1)
+        m_stop.assert_called_once_with(60)
+        m_wait.assert_called_once_with(5)
+
+    def test_save_configuration_error(self):
+        mock_connection = mock.Mock()
+        output = """
+[edit vlans]
+  'duplicate-vlan'
+    l2ald: Duplicate vlan-id exists for vlan duplicate-vlan
+[edit vlans]
+  Failed to parse vlan hierarchy completely
+error: configuration check-out failed
+
+{master:0}[edit]"""
+        mock_connection.commit.side_effect = ValueError(
+            "Commit failed with the following errors:\n\n{0}".format(output))
+
+        self.assertRaisesRegexp(exc.GenericSwitchNetmikoConfigError,
+                                "Failed to commit configuration",
+                                self.switch.save_configuration,
+                                mock_connection)
+        mock_connection.commit.assert_called_once_with()
+
+    @mock.patch.object(netmiko_devices.tenacity, 'wait_fixed')
+    @mock.patch.object(netmiko_devices.tenacity, 'stop_after_delay')
+    def test_save_configuration_non_default_timing(self, m_stop, m_wait):
+        self.switch = self._make_switch_device({'ngs_commit_timeout': 42,
+                                                'ngs_commit_interval': 43})
+        mock_connection = mock.Mock()
+        self.switch.save_configuration(mock_connection)
+        mock_connection.commit.assert_called_once_with()
+        m_stop.assert_called_once_with(42)
+        m_wait.assert_called_once_with(43)
+
     def test__format_commands(self):
         cmd_set = self.switch._format_commands(
             juniper.Juniper.ADD_NETWORK,