From 1624381dd228d82881072053bd27ba066774af16 Mon Sep 17 00:00:00 2001 From: rsmekala Date: Wed, 11 Sep 2019 20:10:41 +0530 Subject: [PATCH 1/4] Use cache_file instead of get_file --- salt/modules/junos.py | 58 ++++++++++++++++++++++++++++--------------- 1 file changed, 38 insertions(+), 20 deletions(-) diff --git a/salt/modules/junos.py b/salt/modules/junos.py index 92fda7cf3631..54773c35bef2 100644 --- a/salt/modules/junos.py +++ b/salt/modules/junos.py @@ -5,11 +5,6 @@ :maturity: new :dependencies: junos-eznc, jxmlease -.. note:: - - Those who wish to use junos-eznc (PyEZ) version >= 2.1.0, must - use the latest salt code from github until the next release. - Refer to :mod:`junos ` for information on connecting to junos proxy. ''' @@ -190,9 +185,9 @@ def rpc(cmd=None, **kwargs): .. code-block:: bash - salt 'device' junos.rpc get_config dest=/var/log/config.txt format=text filter='' - salt 'device' junos.rpc get-interface-information dest=/home/user/interface.xml interface_name='lo0' terse=True - salt 'device' junos.rpc get-chassis-inventory + salt 'device_name' junos.rpc get_config dest=/var/log/config.txt format=text filter='' + salt 'device_name' junos.rpc get-interface-information dest=/home/user/interface.xml interface_name='lo0' terse=True + salt 'device_name' junos.rpc get-chassis-inventory ''' conn = __proxy__['junos.conn']() @@ -269,6 +264,8 @@ def rpc(cmd=None, **kwargs): # Earlier it was ret['message'] ret['rpc_reply'] = jxmlease.parse(etree.tostring(reply)) + # The file is written only on the minion. use cp.push + # to copy the file onto master if dest: if format_ == 'text': write_response = reply.text @@ -505,6 +502,8 @@ def rollback(**kwargs): if 'diffs_file' in op and op['diffs_file'] is not None: diff = conn.cu.diff() + # The file is written only on the minion. use cp.push + # to copy the file onto master if diff is not None: with salt.utils.files.fopen(op['diffs_file'], 'w') as fp: fp.write(salt.utils.stringutils.to_str(diff)) @@ -691,6 +690,8 @@ def cli(command=None, **kwargs): result = etree.tostring(result) ret['message'] = jxmlease.parse(result) + # The file is written only on the minion. use cp.push + # to copy the file onto master if 'dest' in op and op['dest'] is not None: try: with salt.utils.files.fopen(op['dest'], 'w') as fp: @@ -786,7 +787,7 @@ def install_config(path=None, **kwargs): Commits the changes if the commit checks or throws an error. path (required) - Path where the configuration/template file is present. If the file has + Path where the configuration/template file is present on master. 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 @@ -877,6 +878,9 @@ def install_config(path=None, **kwargs): template_vars = op["template_vars"] try: + # We cannot use cache_file here as this needs to be rendered + # on every run unlike cache_file. Also there is no module as + # cache_template template_cached_path = salt.utils.files.mkstemp() __salt__['cp.get_template']( path, @@ -898,6 +902,8 @@ def install_config(path=None, **kwargs): ret['out'] = False return ret + # The file is written only on the minion. use cp.push + # to copy the file onto master write_diff = '' if 'diffs_file' in op and op['diffs_file'] is not None: write_diff = op['diffs_file'] @@ -915,6 +921,7 @@ def install_config(path=None, **kwargs): op['format'] = template_format + # TODO: document the default behaviour of the commit operation if 'replace' in op and op['replace']: op['merge'] = False del op['replace'] @@ -943,6 +950,7 @@ def install_config(path=None, **kwargs): return ret finally: + # Remove the temp file. salt.utils.files.safe_rm(template_cached_path) if db_mode != 'dynamic': @@ -991,6 +999,8 @@ def install_config(path=None, **kwargs): ret['out'] = True try: + # The file is written only on the minion. use cp.push + # to copy the file onto master if write_diff and config_diff is not None: with salt.utils.files.fopen(write_diff, 'w') as fp: fp.write(salt.utils.stringutils.to_str(config_diff)) @@ -1047,7 +1057,7 @@ def install_os(path=None, **kwargs): the device is rebooted, if reboot=True is given as a keyworded argument. path (required) - Path where the image file is present on the proxy minion + Path where the image file is present on the master remote_path : /var/tmp If the value of path is a file path on the local @@ -1133,10 +1143,12 @@ def install_os(path=None, **kwargs): if not no_copy_: # To handle invalid image path scenario try: - image_cached_path = salt.utils.files.mkstemp() - __salt__['cp.get_file'](path, image_cached_path) + # cache_file is better suited here than get_file + image_cached_path = __salt__['cp.cache_file'](path) - if not os.path.isfile(image_cached_path): + # If it wasn't able find the file on master, it will return false + # else the cache path + if not image_cached_path and not os.path.isfile(image_cached_path): ret['message'] = 'Invalid image path.' ret['out'] = False return ret @@ -1159,9 +1171,7 @@ def install_os(path=None, **kwargs): ret['message'] = 'Installation failed due to: "{0}"'.format(exception) ret['out'] = False return ret - finally: - if not no_copy_: - salt.utils.files.safe_rm(image_cached_path) + # No need to remove the cache file # Handle reboot, after the install has finished if reboot is True: @@ -1180,7 +1190,7 @@ def install_os(path=None, **kwargs): def file_copy(src=None, dest=None): ''' - Copies the file from the local device to the junos device + Copies the file from the master to the junos device src The source path where the file is kept. @@ -1198,12 +1208,19 @@ def file_copy(src=None, dest=None): ret = {} ret['out'] = True + # First, copy the file from master to local + if src is None: ret['message'] = \ - 'Please provide the absolute path of the file to be copied.' + 'Please provide the path of the file to be copied.' ret['out'] = False return ret - if not os.path.isfile(src): + + # cache_file is better suited here than get_file + cached_path = __salt__['cp.cache_file'](src) + # If it wasn't able find the file on master, it will return false + # else the cache path + if not cached_path and not os.path.isfile(src): ret['message'] = 'Invalid source file path' ret['out'] = False return ret @@ -1216,7 +1233,7 @@ def file_copy(src=None, dest=None): try: with SCP(conn, progress=True) as scp: - scp.put(src, dest) + scp.put(cached_path, dest) ret['message'] = 'Successfully copied file from {0} to {1}'.format( src, dest) except Exception as exception: @@ -1456,6 +1473,7 @@ def commit_check(): return ret +# TODO: this is the last module that is pending def get_table(table, table_file, path=None, target=None, key=None, key_items=None, filters=None, template_args=None): ''' From 6eea9fe05e615c01128eb98e55a86b3a78bb27f2 Mon Sep 17 00:00:00 2001 From: rsmekala Date: Thu, 12 Sep 2019 12:32:52 +0530 Subject: [PATCH 2/4] Updated get_table to look for table_file on master --- salt/modules/junos.py | 94 ++++++++++++++++++++++++++++--------------- 1 file changed, 62 insertions(+), 32 deletions(-) diff --git a/salt/modules/junos.py b/salt/modules/junos.py index 54773c35bef2..fd23e0d42778 100644 --- a/salt/modules/junos.py +++ b/salt/modules/junos.py @@ -1220,7 +1220,7 @@ def file_copy(src=None, dest=None): cached_path = __salt__['cp.cache_file'](src) # If it wasn't able find the file on master, it will return false # else the cache path - if not cached_path and not os.path.isfile(src): + if not cached_path and not os.path.isfile(cached_path): ret['message'] = 'Invalid source file path' ret['out'] = False return ret @@ -1474,8 +1474,7 @@ def commit_check(): # TODO: this is the last module that is pending -def get_table(table, table_file, path=None, target=None, key=None, key_items=None, - filters=None, template_args=None): +def get_table(table, table_file, **kwargs): ''' Retrieve data from a Junos device using Tables/Views @@ -1485,10 +1484,6 @@ def get_table(table, table_file, path=None, target=None, key=None, key_items=Non table_file (required) YAML file that has the table specified in table parameter - path: - Path of location of the YAML file. - defaults to op directory in jnpr.junos.op - target: if command need to run on FPC, can specify fpc target @@ -1515,33 +1510,68 @@ def get_table(table, table_file, path=None, target=None, key=None, key_items=Non ret['out'] = True ret['hostname'] = conn._hostname ret['tablename'] = table - get_kvargs = {} - if target is not None: - get_kvargs['target'] = target - if key is not None: - get_kvargs['key'] = key - if key_items is not None: - 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 + + # path = None, target = None, key = None, key_items = None, + # filters = None, template_args = None + + # TODO: Move this logic to one place instead of placing + # it in each and every module + + # Update the kwargs to create a op dictionary, useful when + # executed with state module + op = {} + if '__pub_arg' in kwargs: + if kwargs['__pub_arg']: + if isinstance(kwargs['__pub_arg'][-1], dict): + op.update(kwargs['__pub_arg'][-1]) + else: + op.update(kwargs) + + # get_kvargs = {} + # if 'target' in op and op['target'] is not None: + # get_kvargs['target'] = target + # if key is not None: + # get_kvargs['key'] = key + # if key_items is not None: + # 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 pyez_tables_path = os.path.dirname(os.path.abspath(tables_dir.__file__)) - # First copy the tablefile here + # The key name has to be changed template_args -> args, if it exists + if 'template_args' in op and isinstance(op['template_args'], dict): + op['args'] = op['template_args'] + del op['template_args'] + + # Find out where is the table_file on master or minion or non-existent + if os.path.isabs(table_file) or table_file.startswith('salt://'): + # In this case the table_file is present on + # First copy the tablefile here + table_file_loc = __salt__['cp.cache_file'](table_file) + # If it wasn't able find the file on master, it will return false + # else the cache path + if not table_file_loc and not os.path.isfile(table_file_loc): + ret['message'] = 'Invalid table file path' + ret['out'] = False + return ret + else: + table_file_loc = os.path.join(pyez_tables_path, '{}'.format(table_file)) + try: - if path is not None: - file_loc = glob.glob(os.path.join(path, '{}'.format(table_file))) - else: - file_loc = glob.glob(os.path.join(pyez_tables_path, '{}'.format(table_file))) - if len(file_loc) == 1: - file_name = file_loc[0] + # Find if the file really exists, + # or the user is playing with the module :) + table_file_loc = glob.glob(table_file_loc) + if len(table_file_loc) == 1: + table_file_name = table_file_loc[0] else: + # Caught you probably ret['message'] = 'Given table file {} cannot be located'.format(table_file) ret['out'] = False return ret try: - with salt.utils.files.fopen(file_name) as fp: + with salt.utils.files.fopen(table_file_name) as fp: ret['table'] = yaml.load(fp.read(), Loader=yamlordereddictloader.Loader) globals().update(FactoryLoader().load(ret['table'])) @@ -1552,7 +1582,7 @@ def get_table(table, table_file, path=None, target=None, key=None, key_items=Non return ret try: data = globals()[table](conn) - data.get(**get_kvargs) + data.get(**op) except KeyError as err: ret['message'] = 'Uncaught exception during get API call - please ' \ 'report: {0}'.format(six.text_type(err)) @@ -1569,16 +1599,16 @@ def get_table(table, table_file, path=None, target=None, key=None, key_items=Non 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: + if 'key' in op and op['key'] is not None: ret['table'][table]['key'] = data.KEY else: - if target is not None: + if 'target' and op['target'] is not None: ret['table'][table]['target'] = data.TARGET - if key is not None: + if 'key' in op and op['key'] is not None: ret['table'][table]['key'] = data.KEY - if key_items is not None: + if 'key_items' and op['key_items'] is not None: ret['table'][table]['key_items'] = data.KEY_ITEMS - if template_args is not None: + if 'args' in op and op['args'] is not None: ret['table'][table]['args'] = data.CMD_ARGS ret['table'][table]['command'] = data.GET_CMD except Exception as err: From 5a5b3475e9e8696f4972e22c2c5790727889799a Mon Sep 17 00:00:00 2001 From: rsmekala Date: Fri, 13 Sep 2019 11:38:46 +0530 Subject: [PATCH 3/4] Fixed if statement for key_items --- salt/modules/junos.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/modules/junos.py b/salt/modules/junos.py index fd23e0d42778..16bc5b16aaf6 100644 --- a/salt/modules/junos.py +++ b/salt/modules/junos.py @@ -1606,7 +1606,7 @@ def get_table(table, table_file, **kwargs): ret['table'][table]['target'] = data.TARGET if 'key' in op and op['key'] is not None: ret['table'][table]['key'] = data.KEY - if 'key_items' and op['key_items'] is not None: + if 'key_items' in op and op['key_items'] is not None: ret['table'][table]['key_items'] = data.KEY_ITEMS if 'args' in op and op['args'] is not None: ret['table'][table]['args'] = data.CMD_ARGS From 4e8ae8420adb0698a6764f08850011ae8f77140f Mon Sep 17 00:00:00 2001 From: rsmekala Date: Fri, 13 Sep 2019 12:10:40 +0530 Subject: [PATCH 4/4] Fixed if statement for target --- salt/modules/junos.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/modules/junos.py b/salt/modules/junos.py index 16bc5b16aaf6..dd174fc56d22 100644 --- a/salt/modules/junos.py +++ b/salt/modules/junos.py @@ -1602,7 +1602,7 @@ def get_table(table, table_file, **kwargs): if 'key' in op and op['key'] is not None: ret['table'][table]['key'] = data.KEY else: - if 'target' and op['target'] is not None: + if 'target' in op and op['target'] is not None: ret['table'][table]['target'] = data.TARGET if 'key' in op and op['key'] is not None: ret['table'][table]['key'] = data.KEY