Repository: saltstack/salt
Pull request: juniper modules/states docstring, parameter name fixes
### What does this PR do?
Fix https://github.com/Juniper/salt/issues/141 & https://github.com/Juniper/salt/issues/142

### What issues does this PR fix or reference?
Fixes: Minor doc strings and parameter names


### Merge requirements satisfied?
**[NOTICE] Bug fixes or features added to Salt require tests.**
<!-- Please review the [test documentation](https://docs.saltstack.com/en/master/topics/tutorials/writing_tests.html) for details on how to implement tests into Salt's test suite. -->
- [x] Docs
- [x] Changelog - https://docs.saltstack.com/en/master/topics/development/changelog.html
- [x] Tests written/updated

### Commits signed with GPG?
Yes/No

Please review [Salt's Contributing Guide](https://docs.saltstack.com/en/master/topics/development/contributing.html) for best practices.

See GitHub's [page on GPG signing](https://help.github.com/articles/signing-commits-using-gpg/) for more information about signing commits with GPG.


Problem statement:
handle localhost session
### Description of Issue
```
[DEBUG   ] Subprocess Thread-4-Schedule-__proxy_keepalive added
[DEBUG   ] schedule.handle_func: adding this job to the jobcache with data {'id': 'fcr5a_native', 'fun': 'status.proxy_reconnect', 'fun_args': [], 'schedule': '__proxy_keepalive', 'jid': '20200414090540489456', 'pid': 74052}
[DEBUG   ] LazyLoaded status.proxy_reconnect
[ERROR   ] Unhandled exception running status.proxy_reconnect
Traceback (most recent call last):
  File "salt/utils/schedule.py", line 769, in handle_func
  File "/var/db/scripts/jet/salt/modules/status.py", line 1728, in proxy_reconnect
    is_alive = __proxy__[proxy_keepalive_fn](opts)
  File "/var/db/scripts/jet/salt/proxy/junos.py", line 154, in alive
    thisproxy['conn'].connected = ping()
  File "/var/db/scripts/jet/salt/proxy/junos.py", line 175, in ping
    if dev._conn._session._transport.is_active():
AttributeError: 'NoneType' object has no attribute 'is_active'
[DEBUG   ] schedule.handle_func: Removing /var/local/salt/cache/proxy/fcr5a_native/proc/20200414090540489456
[DEBUG   ] LazyLoaded mine.update
 ```

Hints from issue discussion:
@vnitinv I am working on the salt binary and from an email thread on the matter:
```
We figured out issue in supporting ioproc connectivity in salt binary and raised ticket in github. This issue is fixed and will send pull request soon.
```
Wondering if you could identify the PR or the fix, as fixing issues with the salt binary at the moment and would like to utilize the fix.
Thanks
@dmurphy18 

For localhost, we dont prefer connection via ssh, but a local mechanism (name ioproc)
https://github.com/ncclient/ncclient/blob/master/ncclient/transport/third_party/junos/ioproc.py

This works all perfectly to make a connection even with the new local minion which we are building. There is a minor issue with keep-alive (ping function). 

I was not sure if we should do the changes in an existing pull request to saltstack (as this change won't be used by any user) and also wanted to get that code checked in first before we make any new changes.

https://github.com/saltstack/salt/blob/master/salt/proxy/junos.py#L193

ping function need to change to 
```
def ping():
    '''
    Ping?  Pong!
    '''
    dev = conn()
    # Check that the underlying netconf connection still exists.
    if dev._conn is None:
        return False
    
    # call rpc only if ncclient queue is empty. If not empty that means other
    # rpc call is going on.
    if hasattr(dev._conn, '_session'):
        if (
            dev._conn._session._transport is not None and
            dev._conn._session._transport.is_active()) or \
            (dev._conn._session._transport is None and
             isinstance(dev._conn._session, IOProc)):
            # there is no on going rpc call. buffer tell can be 1 as it stores
            # remaining char after "]]>]]>" which can be a new line char
            if dev._conn._session._buffer.tell() <= 1 and \
                    dev._conn._session._q.empty():
                return _rpc_file_list(dev)
            else:
                log.debug('skipped ping() call as proxy already getting data')
                return True
        else:
            # ssh connection is lost
            return False
    else:
        # other connection modes, like telnet
        return _rpc_file_list(dev)
```

@vnitinv Thanks for that information. Actually I was about to make those changes to ping this morning myself, but I shall incorporate your code in the changes since I assume you have tested it etc. Converting the local native minion to using localhost, among a few other changes for Juniper  switches.
> @vnitinv Thanks for that information. Actually I was about to make those changes to ping this morning myself, but I shall incorporate your code in the changes since I assume you have tested it etc. Converting the local native minion to using localhost, among a few other changes for Juniper switches.

Getting salt on Junos box will be a great feature addition. Looking forward to it.
@dmurphy18 Did you fix this or want me to send a pull request for this? I don't see this code in salt repo as of today
Go ahead and submit a PR, I made the change in the native minion and am still working on it (core dump on MX,not QFX) so it will be a while before it is ready for merge.  Don't forget to include a test, and mark me as a Reviewer when you submit

Commit message: fix pre-commit failures

Patch to review:
diff --git a/salt/modules/junos.py b/salt/modules/junos.py
index f61fbbca1439..c8a380a1b14a 100644
--- a/salt/modules/junos.py
+++ b/salt/modules/junos.py
@@ -17,6 +17,7 @@
 # Import Python libraries
 from __future__ import absolute_import, print_function, unicode_literals
 
+import copy
 import glob
 import json
 import logging
@@ -51,6 +52,7 @@
     import jnpr.junos.cfg
     import jxmlease
     from jnpr.junos.factory.optable import OpTable
+    from jnpr.junos.factory.cfgtable import CfgTable
     import jnpr.junos.op as tables_dir
     from jnpr.junos.factory.factory_loader import FactoryLoader
     import yamlordereddictloader
@@ -200,11 +202,6 @@ def rpc(cmd=None, dest=None, **kwargs):
     ret = {}
     ret["out"] = True
 
-    if cmd is None:
-        ret["message"] = "Please provide the rpc to execute."
-        ret["out"] = False
-        return ret
-
     op = dict()
     if "__pub_arg" in kwargs:
         if kwargs["__pub_arg"]:
@@ -216,6 +213,14 @@ def rpc(cmd=None, dest=None, **kwargs):
                 op[key] = value
     else:
         op.update(kwargs)
+
+    if cmd is None:
+        cmd = kwargs.pop("rpc", None)
+    if cmd is None:
+        ret["message"] = "Please provide the rpc to execute."
+        ret["out"] = False
+        return ret
+
     format_ = op.pop("format", "xml")
     # when called from state, dest becomes part of op via __pub_arg
     dest = dest or op.pop("dest", None)
@@ -250,13 +255,10 @@ def rpc(cmd=None, dest=None, **kwargs):
             return ret
 
     if format_ == "text":
-        # Earlier it was ret['message']
         ret["rpc_reply"] = reply.text
     elif format_ == "json":
-        # Earlier it was ret['message']
         ret["rpc_reply"] = reply
     else:
-        # Earlier it was ret['message']
         ret["rpc_reply"] = jxmlease.parse(etree.tostring(reply))
 
     if dest:
@@ -786,7 +788,7 @@ def install_config(path=None, **kwargs):
 
     mode : exclusive
         The mode in which the configuration is locked. Can be one of
-        ``private``, ``dynamic``, ``batch``, ``exclusive``.
+        ``private``, ``dynamic``, ``batch``, ``exclusive``, ``ephemeral``
 
     dev_timeout : 30
         Set NETCONF RPC timeout. Can be used for commands which take a while to
@@ -794,12 +796,13 @@ def install_config(path=None, **kwargs):
 
     overwrite : False
         Set to ``True`` if you want this file is to completely replace the
-        configuration file.
+        configuration file. Sets action to override
+
+        .. note:: This option cannot be used if **format** is "set".
 
-    replace : False
-        Specify whether the configuration file uses ``replace:`` statements. If
-        ``True``, only those statements under the ``replace`` tag will be
-        changed.
+    merge : False
+        If set to ``True`` will set the load-config action to merge.
+        the default load-config action is 'replace' for xml/json/text config
 
     format
         Determines the format of the contents
@@ -906,6 +909,8 @@ def install_config(path=None, **kwargs):
             template_format = "set"
         elif path.endswith("xml"):
             template_format = "xml"
+        elif path.endswith("json"):
+            template_format = "json"
         else:
             template_format = "text"
 
@@ -921,13 +926,18 @@ def install_config(path=None, **kwargs):
         del op["overwrite"]
 
     db_mode = op.pop("mode", "exclusive")
-    if write_diff and db_mode == "dynamic":
-        ret["message"] = "Write diff is not supported with dynamic configuration mode"
+    if write_diff and db_mode in ["dynamic", "ephemeral"]:
+        ret[
+            "message"
+        ] = "Write diff is not supported with dynamic/ephemeral configuration mode"
         ret["out"] = False
         return ret
 
+    config_params = {}
+    if "ephemeral_instance" in op:
+        config_params["ephemeral_instance"] = op.pop("ephemeral_instance")
     try:
-        with Config(conn, mode=db_mode) as cu:
+        with Config(conn, mode=db_mode, **config_params) as cu:
             try:
                 cu.load(**op)
             except Exception as exception:  # pylint: disable=broad-except
@@ -940,7 +950,10 @@ def install_config(path=None, **kwargs):
             finally:
                 salt.utils.files.safe_rm(template_cached_path)
 
-            if db_mode != "dynamic":
+            config_diff = None
+            if db_mode in ["dynamic", "ephemeral"]:
+                log.debug("diff is not supported for dynamic and ephemeral")
+            else:
                 config_diff = cu.diff()
                 if config_diff is None:
                     ret["message"] = "Configuration already applied!"
@@ -955,7 +968,9 @@ def install_config(path=None, **kwargs):
 
             # Assume commit_check succeeds and initialize variable check
             check = True
-            if db_mode != "dynamic":
+            if db_mode in ["dynamic", "ephemeral"]:
+                log.debug("commit check not supported for dynamic and ephemeral")
+            else:
                 try:
                     check = cu.commit_check()
                 except Exception as exception:  # pylint: disable=broad-except
@@ -1000,10 +1015,10 @@ def install_config(path=None, **kwargs):
                     exception
                 )
                 ret["out"] = False
-    except ValueError:
-        ret[
-            "message"
-        ] = "Invalid mode. Modes supported: private, dynamic, batch, exclusive"
+    except ValueError as ex:
+        message = "install_config failed due to: {0}".format(str(ex))
+        log.error(message)
+        ret["message"] = message
         ret["out"] = False
     except LockError as ex:
         log.error("Configuration database is locked")
@@ -1059,12 +1074,11 @@ def install_os(path=None, **kwargs):
         device. The default is ``/var/tmp``. If the value of :path: or
         is a URL, then the value of :remote_path: is unused.
 
-    dev_timeout : 30
+    dev_timeout : 1800
         The NETCONF RPC timeout (in seconds). This argument was added since most of
-        the time the "package add" RPC takes a significant amount of time.  The default
-        RPC timeout is 30 seconds.  So this :timeout: value will be
-        used in the context of the SW installation process.  Defaults to
-        30 minutes (30*60=1800)
+        the time the "package add" RPC takes a significant amount of time.
+        So this :timeout: value will be used in the context of the SW installation
+        process.  Defaults to 30 minutes (30*60=1800 seconds)
 
     reboot : False
         Whether to reboot after installation
@@ -1122,8 +1136,8 @@ def install_os(path=None, **kwargs):
     # timeout value is not honoured by sw.install if not passed as argument
     # currently, timeout is set to be maximum of default 1800 and user passed timeout value
     # For info: https://github.com/Juniper/salt/issues/116
-    op.pop("dev_timeout", None)
-    timeout = max(1800, conn.timeout)
+    dev_timeout = op.pop("dev_timeout", 0)
+    timeout = max(1800, conn.timeout, dev_timeout)
     # Reboot should not be passed as a keyword argument to install(),
     # Please refer to https://github.com/Juniper/salt/issues/115 for more details
     reboot = op.pop("reboot", False)
@@ -1171,8 +1185,13 @@ def install_os(path=None, **kwargs):
 
     # Handle reboot, after the install has finished
     if reboot is True:
+        reboot_kwargs = {}
+        if "vmhost" in op and op.get("vmhost") is True:
+            reboot_kwargs["vmhost"] = True
+        if "all_re" in op:
+            reboot_kwargs["all_re"] = op.get("all_re")
         try:
-            conn.sw.reboot()
+            conn.sw.reboot(**reboot_kwargs)
         except Exception as exception:  # pylint: disable=broad-except
             ret[
                 "message"
@@ -1299,18 +1318,13 @@ def load(path=None, **kwargs):
 
     overwrite : False
         Set to ``True`` if you want this file is to completely replace the
-        configuration file.
+        configuration file. Sets action to override
 
-    replace : False
-        Specify whether the configuration file uses ``replace:`` statements. If
-        ``True``, only those statements under the ``replace`` tag will be
-        changed.
+        .. note:: This option cannot be used if **format** is "set".
 
     merge : False
         If set to ``True`` will set the load-config action to merge.
-
-    format
-        Determines the format of the contents
+        the default load-config action is 'replace' for xml/json/text config
 
     update : False
         Compare a complete loaded configuration against the candidate
@@ -1321,6 +1335,9 @@ def load(path=None, **kwargs):
         affected by the changed configuration elements parse the new
         configuration. This action is supported from PyEZ 2.1.
 
+    format
+        Determines the format of the contents
+
     template_vars
       Variables to be passed into the template processing engine in addition to
       those present in pillar, the minion configuration, grains, etc.  You may
@@ -1393,6 +1410,8 @@ def load(path=None, **kwargs):
             template_format = "set"
         elif path.endswith("xml"):
             template_format = "xml"
+        elif path.endswith("json"):
+            template_format = "json"
         else:
             template_format = "text"
 
@@ -1468,7 +1487,7 @@ def get_table(
     key=None,
     key_items=None,
     filters=None,
-    template_args=None,
+    table_args=None,
 ):
     """
     .. versionadded:: Sodium
@@ -1497,15 +1516,17 @@ def get_table(
     filters:
         To select only filter for the dictionary from columns
 
-    template_args:
+    table_args:
         key/value pair which should render Jinja template command
+        or are passed as args to rpc call in op table
 
     CLI Example:
 
     .. code-block:: bash
 
-        salt 'device_name' junos.get_table
+        salt 'device_name' junos.get_table RouteTable routes.yml
     """
+
     conn = __proxy__["junos.conn"]()
     ret = {}
     ret["out"] = True
@@ -1520,20 +1541,28 @@ def get_table(
         get_kvargs["key_items"] = key_items
     if filters is not None:
         get_kvargs["filters"] = filters
-    if template_args is not None and isinstance(template_args, dict):
-        get_kvargs["args"] = template_args
+    if table_args is not None and isinstance(table_args, dict):
+        get_kvargs["args"] = table_args
     pyez_tables_path = os.path.dirname(os.path.abspath(tables_dir.__file__))
     try:
         if path is not None:
-            file_loc = glob.glob(os.path.join(path, "{}".format(table_file)))
+            file_loc = glob.glob(os.path.join(path, "{0}".format(table_file)))
         else:
             file_loc = glob.glob(
-                os.path.join(pyez_tables_path, "{}".format(table_file))
+                os.path.join(pyez_tables_path, "{0}".format(table_file))
             )
         if len(file_loc) == 1:
             file_name = file_loc[0]
-        else:
-            ret["message"] = "Given table file {} cannot be located".format(table_file)
+        elif len(file_loc) > 1:
+            ret[
+                "message"
+            ] = "Given table file {0} is located at multiple location".format(
+                table_file
+            )
+            ret["out"] = False
+            return ret
+        elif len(file_loc) == 0:
+            ret["message"] = "Given table file {0} cannot be located".format(table_file)
             ret["out"] = False
             return ret
         try:
@@ -1566,13 +1595,17 @@ def get_table(
             ret["out"] = False
             return ret
         ret["reply"] = json.loads(data.to_json())
-        if data.__class__.__bases__[0] == OpTable:
+        if data.__class__.__bases__[0] in [OpTable, CfgTable]:
             # Sets key value if not present in YAML. To be used by returner
             if ret["table"][table].get("key") is None:
                 ret["table"][table]["key"] = data.ITEM_NAME_XPATH
             # If key is provided from salt state file.
             if key is not None:
                 ret["table"][table]["key"] = data.KEY
+            if table_args is not None:
+                args = copy.copy(data.GET_ARGS)
+                args.update(table_args)
+                ret["table"][table]["args"] = args
         else:
             if target is not None:
                 ret["table"][table]["target"] = data.TARGET
@@ -1580,9 +1613,18 @@ def get_table(
                 ret["table"][table]["key"] = data.KEY
             if key_items is not None:
                 ret["table"][table]["key_items"] = data.KEY_ITEMS
-            if template_args is not None:
-                ret["table"][table]["args"] = data.CMD_ARGS
+            if table_args is not None:
+                args = copy.copy(data.CMD_ARGS)
+                args.update(table_args)
+                ret["table"][table]["args"] = args
                 ret["table"][table]["command"] = data.GET_CMD
+    except ConnectClosedError:
+        ret["message"] = (
+            "Got ConnectClosedError exception. Connection lost "
+            "with {0}".format(str(conn))
+        )
+        ret["out"] = False
+        return ret
     except Exception as err:  # pylint: disable=broad-except
         ret["message"] = "Uncaught exception - please report: {0}".format(str(err))
         traceback.print_exc()
diff --git a/salt/proxy/junos.py b/salt/proxy/junos.py
index c0c61e9564f4..88b94c58b223 100644
--- a/salt/proxy/junos.py
+++ b/salt/proxy/junos.py
@@ -57,6 +57,7 @@
         ConnectTimeoutError,
     )
     from ncclient.operations.errors import TimeoutExpiredError
+    from ncclient.transport.third_party.junos.ioproc import IOProc
 except ImportError:
     HAS_JUNOS = False
 
@@ -181,7 +182,6 @@ def ping():
     """
     Ping?  Pong!
     """
-
     dev = conn()
     # Check that the underlying netconf connection still exists.
     if dev._conn is None:
@@ -190,7 +190,13 @@ def ping():
     # call rpc only if ncclient queue is empty. If not empty that means other
     # rpc call is going on.
     if hasattr(dev._conn, "_session"):
-        if dev._conn._session._transport.is_active():
+        if (
+            dev._conn._session._transport is not None
+            and dev._conn._session._transport.is_active()
+        ) or (
+            dev._conn._session._transport is None
+            and isinstance(dev._conn._session, IOProc)
+        ):
             # there is no on going rpc call. buffer tell can be 1 as it stores
             # remaining char after "]]>]]>" which can be a new line char
             if dev._conn._session._buffer.tell() <= 1 and dev._conn._session._q.empty():
diff --git a/salt/states/junos.py b/salt/states/junos.py
index fdeda7bee833..700b1f0a15f9 100644
--- a/salt/states/junos.py
+++ b/salt/states/junos.py
@@ -33,7 +33,7 @@ def wrapper(*args, **kwargs):
 
 
 @resultdecorator
-def rpc(name, dest=None, format="xml", args=None, **kwargs):
+def rpc(name, rpc=None, dest=None, format="xml", args=None, **kwargs):
     """
     Executes the given rpc. The returned data can be stored in a file
     by specifying the destination path with dest as an argument
@@ -42,14 +42,22 @@ def rpc(name, dest=None, format="xml", args=None, **kwargs):
 
         get-interface-information:
             junos.rpc:
-              - dest: /home/user/rpc.log
-              - interface_name: lo0
+                - dest: /home/user/rpc.log
+                - interface_name: lo0
 
+        fetch interface information with terse:
+            junos.rpc:
+                - rpc: get-interface-information
+                - terse: True
 
     Parameters:
       Required
-        * cmd:
-          The rpc to be executed. (default = None)
+        * rpc:
+          The rpc to be executed (default = None) If not provided name to be
+          used as rpc.
+
+          .. versionadded:: Sodium
+
       Optional
         * dest:
           Destination file where the rpc output is stored. (default = None)
@@ -71,12 +79,19 @@ def rpc(name, dest=None, format="xml", args=None, **kwargs):
               Name of the interface whose information you want.
     """
     ret = {"name": name, "changes": {}, "result": True, "comment": ""}
+    if rpc is None:
+        # for backward, will use name as rpc if rpc not provided explicitly
+        # for example
+        # get-interface-information:
+        #     junos.rpc:
+        #         - dest: /home/user/rpc.log
+        rpc = name
     if args is not None:
         ret["changes"] = __salt__["junos.rpc"](
-            name, dest=dest, format=format, args=args, **kwargs
+            rpc, dest=dest, format=format, args=args, **kwargs
         )
     else:
-        ret["changes"] = __salt__["junos.rpc"](name, dest=dest, format=format, **kwargs)
+        ret["changes"] = __salt__["junos.rpc"](rpc, dest=dest, format=format, **kwargs)
     return ret
 
 
@@ -214,7 +229,7 @@ def diff(name, d_id, **kwargs):
 
 
 @resultdecorator
-def cli(name, **kwargs):
+def cli(name, command=None, **kwargs):
     """
     Executes the CLI commands and reuturns the text output.
 
@@ -224,10 +239,18 @@ def cli(name, **kwargs):
               junos.cli:
                 - format: xml
 
+            get software version of device:
+              junos.cli:
+                - command: show version
+                - format: text
+                - dest: /home/user/show_version.log
+
     Parameters:
       Required
         * command:
           The command that need to be executed on Junos CLI. (default = None)
+          If not provided name to be used as command.
+
       Optional
         * kwargs: Keyworded arguments which can be provided like-
             * format:
@@ -241,7 +264,14 @@ def cli(name, **kwargs):
                (default = None)
     """
     ret = {"name": name, "changes": {}, "result": True, "comment": ""}
-    ret["changes"] = __salt__["junos.cli"](name, **kwargs)
+    if command is None:
+        # for backward, will use name as command if command not provided explicitly
+        # for example
+        # show version:
+        #     junos.cli:
+        #         - /home/user/show_version.log
+        command = name
+    ret["changes"] = __salt__["junos.cli"](command, **kwargs)
     return ret
 
 
@@ -272,7 +302,7 @@ def shutdown(name, **kwargs):
 
 
 @resultdecorator
-def install_config(name, **kwargs):
+def install_config(name, path, **kwargs):
     """
     Loads and commits the configuration provided.
 
@@ -280,7 +310,7 @@ def install_config(name, **kwargs):
 
             Install the mentioned config:
               junos.install_config:
-                - path: salt//configs/interface.set
+                - path: salt://configs/interface.set
                 - timeout: 100
                 - diffs_file: 'var/log/diff'
 
@@ -289,20 +319,22 @@ def install_config(name, **kwargs):
 
             Install the mentioned config:
               junos.install_config:
-                - template_path: salt//configs/interface.set
+                - path: salt://configs/interface.set
                 - timeout: 100
                 - template_vars:
                     interface_name: lo0
                     description: Creating interface via SaltStack.
 
 
-    name
+    path
         Path where the configuration/template file is present. If the file has
         a ``*.conf`` extension, the content is treated as text format. If the
         file has a ``*.xml`` extension, the content is treated as XML format. If
         the file has a ``*.set`` extension, the content is treated as Junos OS
         ``set`` commands
 
+        .. versionadded:: Sodium
+
     template_vars
       The dictionary of data for the jinja variables present in the jinja
       template
@@ -312,12 +344,14 @@ def install_config(name, **kwargs):
       execute.
 
     overwrite : False
-      Set to ``True`` if you want this file is to completely replace the
-       configuration file.
+        Set to ``True`` if you want this file is to completely replace the
+        configuration file. Sets action to override
 
-    replace : False
-      Specify whether the configuration file uses "replace:" statements.  Only
-      those statements under the 'replace' tag will be changed.
+        .. note:: This option cannot be used if **format** is "set".
+
+    merge : False
+        If set to ``True`` will set the load-config action to merge.
+        the default load-config action is 'replace' for xml/json/text config
 
     comment
       Provide a comment to the commit. (default = None)
@@ -337,7 +371,7 @@ def install_config(name, **kwargs):
 
     """
     ret = {"name": name, "changes": {}, "result": True, "comment": ""}
-    ret["changes"] = __salt__["junos.install_config"](name, **kwargs)
+    ret["changes"] = __salt__["junos.install_config"](path, **kwargs)
     return ret
 
 
@@ -455,7 +489,7 @@ def unlock(name):
 
 
 @resultdecorator
-def load(name, **kwargs):
+def load(name, path, **kwargs):
     """
     Loads the configuration provided onto the junos device.
 
@@ -463,13 +497,13 @@ def load(name, **kwargs):
 
         Install the mentioned config:
           junos.load:
-            - path: salt//configs/interface.set
+            - path: salt://configs/interface.set
 
     .. code-block:: yaml
 
         Install the mentioned config:
           junos.load:
-            - template_path: salt//configs/interface.set
+            - path: salt://configs/interface.set
             - template_vars:
                 interface_name: lo0
                 description: Creating interface via SaltStack.
@@ -481,23 +515,24 @@ def load(name, **kwargs):
         set interfaces {{ interface_name }} unit 0
 
 
-    name
+    path:
         Path where the configuration/template file is present. If the file has
         a ``*.conf`` extension, the content is treated as text format. If the
         file has a ``*.xml`` extension, the content is treated as XML format. If
         the file has a ``*.set`` extension, the content is treated as Junos OS
         ``set`` commands.
 
+        .. versionadded:: Sodium
+
     overwrite : False
         Set to ``True`` if you want this file is to completely replace the
-        configuration file.
+        configuration file. Sets action to override
 
-    replace : False
-        Specify whether the configuration file uses "replace:" statements.
-        Only those statements under the 'replace' tag will be changed.
+        .. note:: This option cannot be used if **format** is "set".
 
-    format:
-      Determines the format of the contents.
+    merge : False
+        If set to ``True`` will set the load-config action to merge.
+        the default load-config action is 'replace' for xml/json/text config
 
     update : False
         Compare a complete loaded configuration against the candidate
@@ -508,6 +543,9 @@ def load(name, **kwargs):
         affected by the changed configuration elements parse the new
         configuration. This action is supported from PyEZ 2.1 (default = False)
 
+    format
+      Determines the format of the contents.
+
     template_vars
       Variables to be passed into the template processing engine in addition
       to those present in __pillar__, __opts__, __grains__, etc.
@@ -516,7 +554,7 @@ def load(name, **kwargs):
 
     """
     ret = {"name": name, "changes": {}, "result": True, "comment": ""}
-    ret["changes"] = __salt__["junos.load"](name, **kwargs)
+    ret["changes"] = __salt__["junos.load"](path, **kwargs)
     return ret
 
 
@@ -549,7 +587,14 @@ def get_table(name, table, table_file, **kwargs):
         get route details:
           junos.get_table:
             - table: RouteTable
-            - file: routes.yml
+            - table_file: routes.yml
+
+        get interface details:
+          junos.get_table:
+            - table: EthPortTable
+            - table_file: ethport.yml
+            - table_args:
+                interface_name: ge-0/0/0
 
     name (required)
         task definition
diff --git a/tests/unit/modules/test_junos.py b/tests/unit/modules/test_junos.py
index 0103bb1c795b..42d0e1c541e9 100644
--- a/tests/unit/modules/test_junos.py
+++ b/tests/unit/modules/test_junos.py
@@ -1620,7 +1620,7 @@ def test_install_config_write_diff_dynamic_mode(self):
             ret = dict()
             ret[
                 "message"
-            ] = "Write diff is not supported with dynamic configuration mode"
+            ] = "Write diff is not supported with dynamic/ephemeral configuration mode"
             ret["out"] = False
             self.assertEqual(
                 junos.install_config(
@@ -1652,9 +1652,7 @@ def test_install_config_unknown_mode(self):
             mock_diff.return_value = "diff"
             mock_commit_check.return_value = True
             ret = dict()
-            ret[
-                "message"
-            ] = "Invalid mode. Modes supported: private, dynamic, batch, exclusive"
+            ret["message"] = "install_config failed due to: unsupported action: abcdef"
             ret["out"] = False
             self.assertEqual(
                 junos.install_config("actual/path/config", mode="abcdef"), ret


Code review comments:
diff_hunk: @@ -272,15 +302,15 @@ def shutdown(name, **kwargs):
 
 
 @resultdecorator
-def install_config(name, **kwargs):
+def install_config(name, path, **kwargs):
path: salt/states/junos.py
text: @user1:
Why adding path when name provided the 
286                 - name: salt://configs/interface.set

do not see reason to change label of field, yes it is a path but 'name' is used elsewhere in Salt Doc. too for a provided path.


@author:
reverted via https://github.com/saltstack/salt/pull/57281/commits/682cb0a08baa14e1fa4684163c5588cbda672d25#diff-442286d51776ddefc791bc7f15e42fc2R285

diff_hunk: @@ -455,21 +489,21 @@ def unlock(name):
 
 
 @resultdecorator
-def load(name, **kwargs):
+def load(name, path, **kwargs):
path: salt/states/junos.py
text: @user1:
Same issue as above with name and path

@author:
reverted via https://github.com/saltstack/salt/pull/57281/commits/682cb0a08baa14e1fa4684163c5588cbda672d25#diff-442286d51776ddefc791bc7f15e42fc2R470

diff_hunk: @@ -337,7 +371,7 @@ def install_config(name, **kwargs):
 
     """
     ret = {"name": name, "changes": {}, "result": True, "comment": ""}
-    ret["changes"] = __salt__["junos.install_config"](name, **kwargs)
+    ret["changes"] = __salt__["junos.install_config"](path, **kwargs)
path: salt/states/junos.py
text: @user1:
Above you had name, path, kwargs, and now it is just a swap of name for path, see comment above

@author:
reverted via https://github.com/saltstack/salt/pull/57281/commits/682cb0a08baa14e1fa4684163c5588cbda672d25#diff-442286d51776ddefc791bc7f15e42fc2R352

diff_hunk: @@ -33,7 +33,7 @@ def wrapper(*args, **kwargs):
 
 
 @resultdecorator
-def rpc(name, dest=None, format="xml", args=None, **kwargs):
+def rpc(name, rpc=None, dest=None, format="xml", args=None, **kwargs):
path: salt/states/junos.py
text: @user1:
Best to add parameters to the end of an existing definition, that way it stays backwards compatible

@author:
reverted via https://github.com/saltstack/salt/pull/57281/commits/682cb0a08baa14e1fa4684163c5588cbda672d25#diff-442286d51776ddefc791bc7f15e42fc2R36

diff_hunk: @@ -71,12 +79,19 @@ def rpc(name, dest=None, format="xml", args=None, **kwargs):
               Name of the interface whose information you want.
     """
     ret = {"name": name, "changes": {}, "result": True, "comment": ""}
+    if rpc is None:
+        # for backward, will use name as rpc if rpc not provided explicitly
+        # for example
+        # get-interface-information:
+        #     junos.rpc:
+        #         - dest: /home/user/rpc.log
+        rpc = name
     if args is not None:
         ret["changes"] = __salt__["junos.rpc"](
-            name, dest=dest, format=format, args=args, **kwargs
+            rpc, dest=dest, format=format, args=args, **kwargs
path: salt/states/junos.py
text: @user1:
see above comment about new parameters best placed at the end

@author:
reverted via https://github.com/saltstack/salt/pull/57281/commits/682cb0a08baa14e1fa4684163c5588cbda672d25#diff-442286d51776ddefc791bc7f15e42fc2R80

diff_hunk: @@ -71,12 +79,19 @@ def rpc(name, dest=None, format="xml", args=None, **kwargs):
               Name of the interface whose information you want.
     """
     ret = {"name": name, "changes": {}, "result": True, "comment": ""}
+    if rpc is None:
+        # for backward, will use name as rpc if rpc not provided explicitly
+        # for example
+        # get-interface-information:
+        #     junos.rpc:
+        #         - dest: /home/user/rpc.log
+        rpc = name
     if args is not None:
         ret["changes"] = __salt__["junos.rpc"](
-            name, dest=dest, format=format, args=args, **kwargs
+            rpc, dest=dest, format=format, args=args, **kwargs
         )
     else:
-        ret["changes"] = __salt__["junos.rpc"](name, dest=dest, format=format, **kwargs)
+        ret["changes"] = __salt__["junos.rpc"](rpc, dest=dest, format=format, **kwargs)
path: salt/states/junos.py
text: @user1:
as above, new\ params at the end

@author:
reverted via https://github.com/saltstack/salt/pull/57281/commits/682cb0a08baa14e1fa4684163c5588cbda672d25#diff-442286d51776ddefc791bc7f15e42fc2R83