Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Logging fixes, connectivity checker, sensitive data default, bogus command executions fix #280

Merged
merged 7 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changes/200.added
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Added basic connectivity checker using Netutils tcp_ping method.
1 change: 1 addition & 0 deletions changes/255.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Fixed Timeout and Authenticaiton failure detection to base netmiko_send_commands task.
1 change: 1 addition & 0 deletions changes/272.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Fixed incorrect default on SSoT based jobs for has_sensitive_data Meta field. Now defaults to false.
2 changes: 2 additions & 0 deletions changes/279.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Fix incorrect "no platform set" error which was catching all errors.
- Properly skip command extraction when command_getter fails.
12 changes: 12 additions & 0 deletions nautobot_device_onboarding/jobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -262,11 +262,16 @@ 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,
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,
Expand Down Expand Up @@ -550,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)

Expand All @@ -570,8 +576,13 @@ 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.")
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.")
Expand Down Expand Up @@ -731,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)
Expand Down
14 changes: 13 additions & 1 deletion nautobot_device_onboarding/nornir_plays/command_getter.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,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
Expand Down Expand Up @@ -108,6 +109,11 @@ 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 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],
Expand All @@ -123,7 +129,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 = {}
Expand Down Expand Up @@ -168,6 +174,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 = []
Expand Down
65 changes: 33 additions & 32 deletions nautobot_device_onboarding/nornir_plays/processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,43 +47,44 @@ 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},
)
self.logger.info(
f"Netmiko Send Commands failed on {host.name} with result: {result[0].result}",
extra={"object": host.name},
)
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.
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."""
Expand Down
2 changes: 2 additions & 0 deletions nautobot_device_onboarding/tests/test_jobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down