From dc506a93ce30cdcd33303aa85be5a5777330795f Mon Sep 17 00:00:00 2001 From: Jeff Kala Date: Mon, 9 Dec 2024 08:54:38 -0600 Subject: [PATCH 1/7] initial logging updates --- .../nornir_plays/command_getter.py | 9 ++- .../nornir_plays/inventory_creator.py | 2 +- .../nornir_plays/processor.py | 79 +++++++++++-------- 3 files changed, 52 insertions(+), 38 deletions(-) diff --git a/nautobot_device_onboarding/nornir_plays/command_getter.py b/nautobot_device_onboarding/nornir_plays/command_getter.py index 89f941eb..6c9ddd9b 100755 --- a/nautobot_device_onboarding/nornir_plays/command_getter.py +++ b/nautobot_device_onboarding/nornir_plays/command_getter.py @@ -1,8 +1,8 @@ """CommandGetter.""" - +import traceback import json from typing import Dict - +from netutils.ping import tcp_ping from django.conf import settings from nautobot.dcim.models import Platform from nautobot.dcim.utils import get_all_network_driver_mappings @@ -108,6 +108,10 @@ def netmiko_send_commands( return Result( host=task.host, result=f"{task.host.name} has missing definitions in command_mapper YAML file.", failed=True ) + if not tcp_ping(task.host.hostname, task.host.port): + return Result( + host=task.host, result=f"{task.host.name} failed connectivity check via tcp_ping.", failed=True + ) task.host.data["platform_parsing_info"] = command_getter_yaml_data[task.host.platform] commands = _get_commands_to_run( command_getter_yaml_data[task.host.platform][command_getter_job], @@ -276,6 +280,7 @@ def sync_devices_command_getter(job_result, log_level, kwargs): **kwargs, ) except Exception as err: # pylint: disable=broad-exception-caught + traceback.print_exc() logger.info(f"Error During Sync Devices Command Getter: {err}") return compiled_results diff --git a/nautobot_device_onboarding/nornir_plays/inventory_creator.py b/nautobot_device_onboarding/nornir_plays/inventory_creator.py index 7254cf49..23e9b1f4 100755 --- a/nautobot_device_onboarding/nornir_plays/inventory_creator.py +++ b/nautobot_device_onboarding/nornir_plays/inventory_creator.py @@ -10,7 +10,7 @@ def guess_netmiko_device_type(hostname, username, password, port): guessed_device_type = None remote_device = { - "device_type": "autodetect", + "device_type": "autodetect", "host": hostname, "username": username, "password": password, diff --git a/nautobot_device_onboarding/nornir_plays/processor.py b/nautobot_device_onboarding/nornir_plays/processor.py index dc36b3ac..6be0b81f 100755 --- a/nautobot_device_onboarding/nornir_plays/processor.py +++ b/nautobot_device_onboarding/nornir_plays/processor.py @@ -47,43 +47,52 @@ def task_instance_completed(self, task: Task, host: Host, result: MultiResult) - ) # If any main task resulted in a failed:True then add that key so ssot side can ignore that entry. if result[0].failed: - if task.params["command_getter_job"] == "sync_devices": - self.logger.info( - f"{host.name} has no platform set. Removing it from the sync process.", - extra={"object": host.name}, - ) - del self.data[host.name] - else: - self.data[host.name].update({"failed": True}) + self.logger.info( + f"{host.name} {result[0].failed} {result[0].result}", + extra={"object": host.name}, + ) + self.data[host.name].update({"failed": True}) + # del self.data[host.name] + # self.data[host.name].update({"failed": True}) + # del self.data[host.name] + # if task.params["command_getter_job"] == "sync_devices": + # self.logger.info( + # f"{host.name} has no platform set. Removing it from the sync process.", + # extra={"object": host.name}, + # ) + # del self.data[host.name] + # else: + # self.data[host.name].update({"failed": True}) # [1:] because result 1 is the (network_send_commands ) task which runs all the subtask, it has no result. - for res in result[1:]: - parsed_command_outputs[res.name] = res.result + if not self.data[host.name].get("failed"): + for res in result[1:]: + parsed_command_outputs[res.name] = res.result - ready_for_ssot_data = extract_show_data( - host, parsed_command_outputs, task.params["command_getter_job"], self.kwargs["debug"] - ) - if task.params["command_getter_job"] == "sync_devices": - try: - validate(ready_for_ssot_data, NETWORK_DEVICES_SCHEMA) - except ValidationError as e: - if self.kwargs["debug"]: - self.logger.debug(f"Schema validation failed for {host.name}. Error: {e}.") - self.data[host.name] = {"failed": True, "failed_reason": "Schema validation failed."} - else: - if self.kwargs["debug"]: - self.logger.debug(f"Ready for ssot data: {host.name} {ready_for_ssot_data}") - self.data[host.name].update(ready_for_ssot_data) - elif task.params["command_getter_job"] == "sync_network_data": - try: - validate(ready_for_ssot_data, NETWORK_DATA_SCHEMA) - except ValidationError as err: - if self.kwargs["debug"]: - self.logger.debug(f"Schema validation failed for {host.name} Error: {err}") - self.data[host.name] = {"failed": True, "failed_reason": "Schema validation failed."} - else: - if self.kwargs["debug"]: - self.logger.debug(f"Ready for ssot data: {host.name} {ready_for_ssot_data}") - self.data[host.name].update(ready_for_ssot_data) + ready_for_ssot_data = extract_show_data( + host, parsed_command_outputs, task.params["command_getter_job"], self.kwargs["debug"] + ) + if task.params["command_getter_job"] == "sync_devices": + try: + validate(ready_for_ssot_data, NETWORK_DEVICES_SCHEMA) + except ValidationError as e: + if self.kwargs["debug"]: + self.logger.debug(f"Schema validation failed for {host.name}. Error: {e}.") + self.data[host.name] = {"failed": True, "failed_reason": "Schema validation failed."} + else: + if self.kwargs["debug"]: + self.logger.debug(f"Ready for ssot data: {host.name} {ready_for_ssot_data}") + self.data[host.name].update(ready_for_ssot_data) + elif task.params["command_getter_job"] == "sync_network_data": + try: + validate(ready_for_ssot_data, NETWORK_DATA_SCHEMA) + except ValidationError as err: + if self.kwargs["debug"]: + self.logger.debug(f"Schema validation failed for {host.name} Error: {err}") + self.data[host.name] = {"failed": True, "failed_reason": "Schema validation failed."} + else: + if self.kwargs["debug"]: + self.logger.debug(f"Ready for ssot data: {host.name} {ready_for_ssot_data}") + self.data[host.name].update(ready_for_ssot_data) def subtask_instance_completed(self, task: Task, host: Host, result: MultiResult) -> None: """Processor for logging and data processing on subtask completed.""" From d1fb4b18aae2d1abd9f6e113f0fc5aaaf33dee9b Mon Sep 17 00:00:00 2001 From: Jeff Kala Date: Mon, 9 Dec 2024 16:04:11 -0600 Subject: [PATCH 2/7] add change fragments, clean up data --- changes/200.added | 1 + changes/255.fixed | 1 + changes/272.fixed | 1 + changes/279.fixed | 2 ++ nautobot_device_onboarding/jobs.py | 2 ++ .../nornir_plays/command_getter.py | 10 +++++++--- .../nornir_plays/processor.py | 20 ++++++------------- 7 files changed, 20 insertions(+), 17 deletions(-) create mode 100755 changes/200.added create mode 100755 changes/255.fixed create mode 100755 changes/272.fixed create mode 100755 changes/279.fixed diff --git a/changes/200.added b/changes/200.added new file mode 100755 index 00000000..b4898048 --- /dev/null +++ b/changes/200.added @@ -0,0 +1 @@ +- Added basic connectivity checker using Netutils tcp_ping method. diff --git a/changes/255.fixed b/changes/255.fixed new file mode 100755 index 00000000..e320953a --- /dev/null +++ b/changes/255.fixed @@ -0,0 +1 @@ +- Fixed Timeout and Authenticaiton failure detection to base netmiko_send_commands task. diff --git a/changes/272.fixed b/changes/272.fixed new file mode 100755 index 00000000..bb62ad4e --- /dev/null +++ b/changes/272.fixed @@ -0,0 +1 @@ +- Fixed incorrect default on SSoT based jobs for has_sensitive_data Meta field. Now defaults to false. diff --git a/changes/279.fixed b/changes/279.fixed new file mode 100755 index 00000000..23eb03a0 --- /dev/null +++ b/changes/279.fixed @@ -0,0 +1,2 @@ +- Fix incorrect "no platform set" error which was catching all errors. +- Properly skip command extraction when command_getter fails. diff --git a/nautobot_device_onboarding/jobs.py b/nautobot_device_onboarding/jobs.py index 62b5cc51..11e369c7 100755 --- a/nautobot_device_onboarding/jobs.py +++ b/nautobot_device_onboarding/jobs.py @@ -262,6 +262,7 @@ class Meta: name = "Sync Devices From Network" description = "Synchronize basic device information into Nautobot from one or more network devices. Information includes Device Name, Serial Number, Management IP/Interface." + has_sensitive_variables = False debug = BooleanVar( default=False, @@ -570,6 +571,7 @@ class Meta: name = "Sync Network Data From Network" description = "Synchronize extended device attribute information into Nautobot from one or more network devices. Information includes Interfaces, IP Addresses, Prefixes, VLANs and VRFs." + has_sensitive_variables = False debug = BooleanVar(description="Enable for more verbose logging.") sync_vlans = BooleanVar(default=False, description="Sync VLANs and interface VLAN assignments.") diff --git a/nautobot_device_onboarding/nornir_plays/command_getter.py b/nautobot_device_onboarding/nornir_plays/command_getter.py index 6c9ddd9b..960f5ee2 100755 --- a/nautobot_device_onboarding/nornir_plays/command_getter.py +++ b/nautobot_device_onboarding/nornir_plays/command_getter.py @@ -1,5 +1,4 @@ """CommandGetter.""" -import traceback import json from typing import Dict from netutils.ping import tcp_ping @@ -127,7 +126,7 @@ def netmiko_send_commands( f"{task.host.platform} has missing definitions for cables in command_mapper YAML file. Cables will not be loaded." ) - logger.debug(f"Commands to run: {commands}") + logger.debug(f"Commands to run: {[cmd['command'] for cmd in commands]}") # All commands in this for loop are running within 1 device connection. for result_idx, command in enumerate(commands): send_command_kwargs = {} @@ -172,6 +171,12 @@ def netmiko_send_commands( except Exception: task.result.failed = False except NornirSubTaskError: + # These exceptions indicate that the device is unreachable or the credentials are incorrect. + # We should fail the task early to avoid trying all commands on a device that is unreachable. + if type(task.results[result_idx].exception).__name__ == "NetmikoAuthenticationException": + return Result(host=task.host, result=f"{task.host.name} failed authentication.", failed=True) + if type(task.results[result_idx].exception).__name__ == "NetmikoTimeoutException": + return Result(host=task.host, result=f"{task.host.name} SSH Timeout Occured.", failed=True) # We don't want to fail the entire subtask if SubTaskError is hit, set result to empty list and failed to False # Handle this type or result latter in the ETL process. task.results[result_idx].result = [] @@ -280,7 +285,6 @@ def sync_devices_command_getter(job_result, log_level, kwargs): **kwargs, ) except Exception as err: # pylint: disable=broad-exception-caught - traceback.print_exc() logger.info(f"Error During Sync Devices Command Getter: {err}") return compiled_results diff --git a/nautobot_device_onboarding/nornir_plays/processor.py b/nautobot_device_onboarding/nornir_plays/processor.py index 6be0b81f..85746bfc 100755 --- a/nautobot_device_onboarding/nornir_plays/processor.py +++ b/nautobot_device_onboarding/nornir_plays/processor.py @@ -48,26 +48,18 @@ def task_instance_completed(self, task: Task, host: Host, result: MultiResult) - # If any main task resulted in a failed:True then add that key so ssot side can ignore that entry. if result[0].failed: self.logger.info( - f"{host.name} {result[0].failed} {result[0].result}", + f"Netmiko Send Commands failed on {host.name} with result: {result[0].result}", extra={"object": host.name}, ) - self.data[host.name].update({"failed": True}) - # del self.data[host.name] - # self.data[host.name].update({"failed": True}) - # del self.data[host.name] - # if task.params["command_getter_job"] == "sync_devices": - # self.logger.info( - # f"{host.name} has no platform set. Removing it from the sync process.", - # extra={"object": host.name}, - # ) - # del self.data[host.name] - # else: - # self.data[host.name].update({"failed": True}) + if "has no platform set" in result[0].result: + del self.data[host.name] + else: + self.data[host.name].update({"failed": True}) # [1:] because result 1 is the (network_send_commands ) task which runs all the subtask, it has no result. if not self.data[host.name].get("failed"): for res in result[1:]: parsed_command_outputs[res.name] = res.result - + ready_for_ssot_data = extract_show_data( host, parsed_command_outputs, task.params["command_getter_job"], self.kwargs["debug"] ) From 2e008d31bbbbe68f72a2b493c5d1732520fdae9d Mon Sep 17 00:00:00 2001 From: Jeff Kala Date: Mon, 9 Dec 2024 16:06:34 -0600 Subject: [PATCH 3/7] ruff formatting --- nautobot_device_onboarding/nornir_plays/command_getter.py | 3 ++- nautobot_device_onboarding/nornir_plays/inventory_creator.py | 2 +- nautobot_device_onboarding/nornir_plays/processor.py | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/nautobot_device_onboarding/nornir_plays/command_getter.py b/nautobot_device_onboarding/nornir_plays/command_getter.py index 960f5ee2..d9b65dc2 100755 --- a/nautobot_device_onboarding/nornir_plays/command_getter.py +++ b/nautobot_device_onboarding/nornir_plays/command_getter.py @@ -1,7 +1,7 @@ """CommandGetter.""" import json from typing import Dict -from netutils.ping import tcp_ping + from django.conf import settings from nautobot.dcim.models import Platform from nautobot.dcim.utils import get_all_network_driver_mappings @@ -9,6 +9,7 @@ from nautobot.extras.models import SecretsGroup from nautobot_plugin_nornir.constants import NORNIR_SETTINGS from nautobot_plugin_nornir.plugins.inventory.nautobot_orm import NautobotORMInventory +from netutils.ping import tcp_ping from nornir import InitNornir from nornir.core.exceptions import NornirSubTaskError from nornir.core.plugins.inventory import InventoryPluginRegister diff --git a/nautobot_device_onboarding/nornir_plays/inventory_creator.py b/nautobot_device_onboarding/nornir_plays/inventory_creator.py index 23e9b1f4..7254cf49 100755 --- a/nautobot_device_onboarding/nornir_plays/inventory_creator.py +++ b/nautobot_device_onboarding/nornir_plays/inventory_creator.py @@ -10,7 +10,7 @@ def guess_netmiko_device_type(hostname, username, password, port): guessed_device_type = None remote_device = { - "device_type": "autodetect", + "device_type": "autodetect", "host": hostname, "username": username, "password": password, diff --git a/nautobot_device_onboarding/nornir_plays/processor.py b/nautobot_device_onboarding/nornir_plays/processor.py index 85746bfc..4abe13c3 100755 --- a/nautobot_device_onboarding/nornir_plays/processor.py +++ b/nautobot_device_onboarding/nornir_plays/processor.py @@ -59,7 +59,7 @@ def task_instance_completed(self, task: Task, host: Host, result: MultiResult) - if not self.data[host.name].get("failed"): for res in result[1:]: parsed_command_outputs[res.name] = res.result - + ready_for_ssot_data = extract_show_data( host, parsed_command_outputs, task.params["command_getter_job"], self.kwargs["debug"] ) From 49020ebdd0f7fe3ee510f196a20468f4ca480562 Mon Sep 17 00:00:00 2001 From: Jeff Kala Date: Mon, 9 Dec 2024 16:13:13 -0600 Subject: [PATCH 4/7] ruff formatting --- nautobot_device_onboarding/nornir_plays/command_getter.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/nautobot_device_onboarding/nornir_plays/command_getter.py b/nautobot_device_onboarding/nornir_plays/command_getter.py index d9b65dc2..f9852d4a 100755 --- a/nautobot_device_onboarding/nornir_plays/command_getter.py +++ b/nautobot_device_onboarding/nornir_plays/command_getter.py @@ -1,4 +1,5 @@ """CommandGetter.""" + import json from typing import Dict @@ -109,9 +110,7 @@ def netmiko_send_commands( host=task.host, result=f"{task.host.name} has missing definitions in command_mapper YAML file.", failed=True ) if not tcp_ping(task.host.hostname, task.host.port): - return Result( - host=task.host, result=f"{task.host.name} failed connectivity check via tcp_ping.", failed=True - ) + return Result(host=task.host, result=f"{task.host.name} failed connectivity check via tcp_ping.", failed=True) task.host.data["platform_parsing_info"] = command_getter_yaml_data[task.host.platform] commands = _get_commands_to_run( command_getter_yaml_data[task.host.platform][command_getter_job], From 5bc943d23f0ff40338724e3879e9966de0a76512 Mon Sep 17 00:00:00 2001 From: Jeff Kala Date: Tue, 10 Dec 2024 10:22:29 -0600 Subject: [PATCH 5/7] make connectivity check optional and default to false to aovid jumphost issues --- nautobot_device_onboarding/jobs.py | 10 ++++++++++ .../nornir_plays/command_getter.py | 5 +++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/nautobot_device_onboarding/jobs.py b/nautobot_device_onboarding/jobs.py index 11e369c7..6ed3b70f 100755 --- a/nautobot_device_onboarding/jobs.py +++ b/nautobot_device_onboarding/jobs.py @@ -268,6 +268,10 @@ class Meta: default=False, description="Enable for more verbose logging.", ) + connectivity_test = BooleanVar( + default=False, + description="Enable to test connectivity to the device(s) prior to attempting onboarding.", + ) csv_file = FileVar( label="CSV File", required=False, @@ -551,6 +555,7 @@ def run( "secrets_group": secrets_group, "platform": platform, "csv_file": "", + "connectivity_test": kwargs["connectivity_test"], } super().run(dryrun, memory_profiling, *args, **kwargs) @@ -574,6 +579,10 @@ class Meta: has_sensitive_variables = False debug = BooleanVar(description="Enable for more verbose logging.") + connectivity_test = BooleanVar( + default=False, + description="Enable to test connectivity to the device(s) prior to attempting onboarding.", + ) sync_vlans = BooleanVar(default=False, description="Sync VLANs and interface VLAN assignments.") sync_vrfs = BooleanVar(default=False, description="Sync VRFs and interface VRF assignments.") sync_cables = BooleanVar(default=False, description="Sync cables between interfaces via a LLDP or CDP.") @@ -733,6 +742,7 @@ def run( "sync_vlans": sync_vlans, "sync_vrfs": sync_vrfs, "sync_cables": sync_cables, + "connectivity_test": kwargs["connectivity_test"], } super().run(dryrun, memory_profiling, *args, **kwargs) diff --git a/nautobot_device_onboarding/nornir_plays/command_getter.py b/nautobot_device_onboarding/nornir_plays/command_getter.py index f9852d4a..4f70dbe7 100755 --- a/nautobot_device_onboarding/nornir_plays/command_getter.py +++ b/nautobot_device_onboarding/nornir_plays/command_getter.py @@ -109,8 +109,9 @@ def netmiko_send_commands( return Result( host=task.host, result=f"{task.host.name} has missing definitions in command_mapper YAML file.", failed=True ) - if not tcp_ping(task.host.hostname, task.host.port): - return Result(host=task.host, result=f"{task.host.name} failed connectivity check via tcp_ping.", failed=True) + if orig_job_kwargs["connectivity_test"]: + if not tcp_ping(task.host.hostname, task.host.port): + return Result(host=task.host, result=f"{task.host.name} failed connectivity check via tcp_ping.", failed=True) task.host.data["platform_parsing_info"] = command_getter_yaml_data[task.host.platform] commands = _get_commands_to_run( command_getter_yaml_data[task.host.platform][command_getter_job], From 196e1839ffbfd2c7700c6b77681627fc4b9f4645 Mon Sep 17 00:00:00 2001 From: Jeff Kala Date: Tue, 10 Dec 2024 10:59:21 -0600 Subject: [PATCH 6/7] fix formatting and linting --- nautobot_device_onboarding/nornir_plays/command_getter.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/nautobot_device_onboarding/nornir_plays/command_getter.py b/nautobot_device_onboarding/nornir_plays/command_getter.py index 4f70dbe7..f017fbcf 100755 --- a/nautobot_device_onboarding/nornir_plays/command_getter.py +++ b/nautobot_device_onboarding/nornir_plays/command_getter.py @@ -111,7 +111,9 @@ def netmiko_send_commands( ) if orig_job_kwargs["connectivity_test"]: if not tcp_ping(task.host.hostname, task.host.port): - return Result(host=task.host, result=f"{task.host.name} failed connectivity check via tcp_ping.", failed=True) + return Result( + host=task.host, result=f"{task.host.name} failed connectivity check via tcp_ping.", failed=True + ) task.host.data["platform_parsing_info"] = command_getter_yaml_data[task.host.platform] commands = _get_commands_to_run( command_getter_yaml_data[task.host.platform][command_getter_job], From 205a381b06f20123c13b65c75ff9ad6ca2bd9c79 Mon Sep 17 00:00:00 2001 From: Jeff Kala Date: Tue, 10 Dec 2024 12:33:10 -0600 Subject: [PATCH 7/7] fix unittest --- nautobot_device_onboarding/tests/test_jobs.py | 2 ++ nautobot_device_onboarding/tests/test_sync_devices_models.py | 3 +++ 2 files changed, 5 insertions(+) diff --git a/nautobot_device_onboarding/tests/test_jobs.py b/nautobot_device_onboarding/tests/test_jobs.py index ea55a7b4..bb81d950 100644 --- a/nautobot_device_onboarding/tests/test_jobs.py +++ b/nautobot_device_onboarding/tests/test_jobs.py @@ -29,6 +29,7 @@ def test_sync_devices__success(self, device_data): job_form_inputs = { "debug": True, + "connectivity_test": False, "dryrun": False, "csv_file": None, "location": self.testing_objects["location_1"].pk, @@ -139,6 +140,7 @@ def test_sync_network_data__success(self, device_data): job_form_inputs = { "debug": True, + "connectivity_test": False, "dryrun": False, "sync_vlans": True, "sync_vrfs": True, diff --git a/nautobot_device_onboarding/tests/test_sync_devices_models.py b/nautobot_device_onboarding/tests/test_sync_devices_models.py index 6d8aa1d5..429493df 100644 --- a/nautobot_device_onboarding/tests/test_sync_devices_models.py +++ b/nautobot_device_onboarding/tests/test_sync_devices_models.py @@ -28,6 +28,7 @@ def test_device_update__missing_primary_ip__success(self, device_data): job_form_inputs = { "debug": True, + "connectivity_test": False, "dryrun": False, "csv_file": None, "location": self.testing_objects["location"].pk, @@ -75,6 +76,7 @@ def test_device_update__primary_ip_and_interface__success(self, device_data): job_form_inputs = { "debug": True, + "connectivity_test": False, "dryrun": False, "csv_file": None, "location": self.testing_objects["location"].pk, @@ -117,6 +119,7 @@ def test_device_update__interface_only__success(self, device_data): job_form_inputs = { "debug": True, + "connectivity_test": False, "dryrun": False, "csv_file": None, "location": self.testing_objects["location"].pk,