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

Handling Ignored Tenants in IP Address Synchronization to Prevent Tenant.DoesNotExist Exceptions #564

Open
TheHack42 opened this issue Oct 3, 2024 · 0 comments
Labels
integration: ciscoaci Issues/PRs for Cisco ACI integration. type: bug Issues/PRs addressing a bug.

Comments

@TheHack42
Copy link

TheHack42 commented Oct 3, 2024

Environment

  • Python version: 3.11.9
  • Nautobot version: 2.3.2
  • nautobot-ssot version: develop

Expected Behavior

The IP addresses and subnet_as_prefix should be synchronized without exceptions, even if they are linked to a tenant that is listed in ignore_tenants. The system should either skip synchronizing those IP addresses/subnet_as_prefix or assign a None value to the tenant but still proceed with the synchronization.

Observed Behavior

When attempting to synchronize IP addresses or subnet_as_prefix associated with a tenant listed in ignore_tenants, an exception of type Tenant.DoesNotExist is raised. This happens because the tenant does not exist in the database, as it has been ignored according to the configuration.

Steps to Reproduce

  1. Configure the ignore_tenants setting to ignore certain tenants in your ACI environment.
  2. Attempt to load and synchronize IP addresses or subnet_as_prefix related to one of these tenants.
  3. An exception Tenant.DoesNotExist will be raised when trying to synchronize an IP address or subnet_as_prefix for the ignored tenant.

Suggested Fix

To prevent this exception, the ignore_tenants setting should be checked before attempting to synchronize IP addresses or subnet_as_prefix in the load_ipaddresses function.

  • One option is to skip the synchronization of IP addresses/subnet_as_prefix that are associated with an ignored tenant.
  • Another option is to set the tenant to None for such cases but still proceed with synchronizing the IP address/subnet_as_prefix.

Which of these approaches would be preferable?

Code Section

Here is the code section causing the issue:

https://github.com/nautobot/nautobot-app-ssot/blob/develop/nautobot_ssot/integrations/aci/diffsync/adapters/aci.py#L132

def load_ipaddresses(self):
    """Load IPAddresses from ACI. Retrieves controller IPs, OOB Mgmt IP of leaf/spine, and Bridge Domain subnet IPs."""
    node_dict = self.conn.get_nodes()
    # Leaf/Spine management IP addresses
    mgmt_tenant = f"{self.tenant_prefix}:mgmt"
    for node in node_dict.values():
        if node.get("oob_ip"):
            if node.get("subnet"):
                subnet = node["subnet"]
            else:
                subnet = ""
            if subnet:
                self.load_subnet_as_prefix(
                    prefix=subnet,
                    namespace="Global",
                    site=self.site,
                    vrf="oob",
                    vrf_tenant=mgmt_tenant,
                    tenant=mgmt_tenant,
                )
            new_ipaddress = self.ip_address(
                address=node["oob_ip"],
                prefix=subnet,
                device=node["name"],
                status="Active",
                description=f"ACI {node['role']}: {node['name']}",
                interface="mgmt0",
                tenant=mgmt_tenant,
                namespace="Global",
                site=self.site,
                site_tag=self.site,
            )
            try:
                self.get(obj=new_ipaddress, identifier=new_ipaddress.get_unique_id())
            except ObjectNotFound:
                self.add(new_ipaddress)
            else:
                self.job.logger.warning("Duplicate DiffSync IPAddress Object found and has not been loaded.")

    controller_dict = self.conn.get_controllers()
    # Controller IP addresses
    for controller in controller_dict.values():
        if controller.get("oob_ip"):
            if controller.get("subnet"):
                subnet = controller["subnet"]
            else:
                subnet = ""
            if subnet:
                self.load_subnet_as_prefix(
                    prefix=subnet,
                    namespace="Global",
                    site=self.site,
                    vrf="oob",
                    vrf_tenant=mgmt_tenant,
                    tenant=mgmt_tenant,
                )
            new_ipaddress = self.ip_address(
                address=f"{controller['oob_ip']}",
                prefix=subnet,
                device=controller["name"],
                status="Active",
                description=f"ACI {controller['role']}: {controller['name']}",
                interface="mgmt0",
                tenant=mgmt_tenant,
                namespace="Global",
                site=self.site,
                site_tag=self.site,
            )
            self.add(new_ipaddress)
    # Bridge domain subnets
    bd_dict = self.conn.get_bds(tenant="all")
    for bd_key, bd_value in bd_dict.items():
        if bd_value.get("subnets"):
            tenant_name = f"{self.tenant_prefix}:{bd_value.get('tenant')}"
            if bd_value.get("vrf_tenant"):
                vrf_tenant = f"{self.tenant_prefix}:{bd_value['vrf_tenant']}"
            else:
                vrf_tenant = None
            if bd_value.get("tenant") == "mgmt":
                _namespace = "Global"
            else:
                _namespace = vrf_tenant or tenant_name
            for subnet in bd_value["subnets"]:
                prefix = ip_network(subnet[0], strict=False).with_prefixlen
                self.load_subnet_as_prefix(
                    prefix=prefix,
                    namespace=_namespace,
                    site=self.site,
                    vrf=bd_value["vrf"],
                    vrf_tenant=vrf_tenant,
                    tenant=vrf_tenant or tenant_name,
                )
                new_ipaddress = self.ip_address(
                    address=subnet[0],
                    prefix=prefix,
                    status="Active",
                    description=f"ACI Bridge Domain: {bd_key}",
                    device=None,
                    interface=None,
                    tenant=vrf_tenant or tenant_name,
                    namespace=_namespace,
                    site=self.site,
                    site_tag=self.site,
                )
                try:
                    self.get(obj=new_ipaddress, identifier=new_ipaddress.get_unique_id())
                except ObjectNotFound:
                    self.add(new_ipaddress)
                else:
                    self.job.logger.warning(
                        f"Duplicate DiffSync IPAddress Object found: {new_ipaddress.address} in Tenant {new_ipaddress.tenant} and has not been loaded.",
                    )

Fix Code (Skipping)

    def load_ipaddresses(self):
        """Load IPAddresses from ACI. Retrieves controller IPs, OOB Mgmt IP of leaf/spine, and Bridge Domain subnet IPs."""
        node_dict = self.conn.get_nodes()
        # Leaf/Spine management IP addresses
        mgmt_tenant = f"{self.tenant_prefix}:mgmt"
        for node in node_dict.values():
            if node.get("oob_ip") and "mgmt" not in PLUGIN_CFG.get("ignore_tenants"):
                if node.get("subnet"):
                    subnet = node["subnet"]
                else:
                    subnet = ""
                if subnet:
                    self.load_subnet_as_prefix(
                        prefix=subnet,
                        namespace="Global",
                        site=self.site,
                        vrf="oob",
                        vrf_tenant=mgmt_tenant,
                        tenant=mgmt_tenant,
                    )
                new_ipaddress = self.ip_address(
                    address=node["oob_ip"],
                    prefix=subnet,
                    device=node["name"],
                    status="Active",
                    description=f"ACI {node['role']}: {node['name']}",
                    interface="mgmt0",
                    tenant=mgmt_tenant,
                    namespace="Global",
                    site=self.site,
                    site_tag=self.site,
                )
                # Using Try/Except to check for an existing loaded object
                # If the object doesn't exist we can create it
                # Otherwise we log a message warning the user of the duplicate.
                try:
                    self.get(obj=new_ipaddress, identifier=new_ipaddress.get_unique_id())
                except ObjectNotFound:
                    self.add(new_ipaddress)
                else:
                    self.job.logger.warning("Duplicate DiffSync IPAddress Object found and has not been loaded.")

        controller_dict = self.conn.get_controllers()
        # Controller IP addresses
        for controller in controller_dict.values():
            if controller.get("oob_ip") and "mgmt" not in PLUGIN_CFG.get("ignore_tenants"):
                if controller.get("subnet"):
                    subnet = controller["subnet"]
                else:
                    subnet = ""
                if subnet:
                    self.load_subnet_as_prefix(
                        prefix=subnet,
                        namespace="Global",
                        site=self.site,
                        vrf="oob",
                        vrf_tenant=mgmt_tenant,
                        tenant=mgmt_tenant,
                    )
                new_ipaddress = self.ip_address(
                    address=f"{controller['oob_ip']}",
                    prefix=subnet,
                    device=controller["name"],
                    status="Active",
                    description=f"ACI {controller['role']}: {controller['name']}",
                    interface="mgmt0",
                    tenant=mgmt_tenant,
                    namespace="Global",
                    site=self.site,
                    site_tag=self.site,
                )
                self.add(new_ipaddress)
        # Bridge domain subnets
        bd_dict = self.conn.get_bds(tenant="all")
        for bd_key, bd_value in bd_dict.items():
            if bd_value.get("subnets"):
                tenant_name = f"{self.tenant_prefix}:{bd_value.get('tenant')}"
                if bd_value.get("vrf_tenant"):
                    vrf_tenant = f"{self.tenant_prefix}:{bd_value['vrf_tenant']}"
                else:
                    vrf_tenant = None
                if bd_value.get("tenant") == "mgmt":
                    _namespace = "Global"
                else:
                    _namespace = vrf_tenant or tenant_name
                for subnet in bd_value["subnets"]:
                    if all(tenant not in PLUGIN_CFG.get("ignore_tenants") for tenant in [bd_value.get('tenant'), bd_value.get('vrf_tenant')]):
                        prefix = ip_network(subnet[0], strict=False).with_prefixlen
                        self.load_subnet_as_prefix(
                            prefix=prefix,
                            namespace=_namespace,
                            site=self.site,
                            vrf=bd_value["vrf"],
                            vrf_tenant=vrf_tenant,
                            tenant=vrf_tenant or tenant_name,
                        )
                        new_ipaddress = self.ip_address(
                            address=subnet[0],
                            prefix=prefix,
                            status="Active",
                            description=f"ACI Bridge Domain: {bd_key}",
                            device=None,
                            interface=None,
                            tenant=vrf_tenant or tenant_name,
                            namespace=_namespace,
                            site=self.site,
                            site_tag=self.site,
                        )
                        # Using Try/Except to check for an existing loaded object
                        # If the object doesn't exist we can create it
                        # Otherwise we log a message warning the user of the duplicate.
                        try:
                            self.get(obj=new_ipaddress, identifier=new_ipaddress.get_unique_id())
                        except ObjectNotFound:
                            self.add(new_ipaddress)
                        else:
                            self.job.logger.warning(
                                f"Duplicate DiffSync IPAddress Object found: {new_ipaddress.address} in Tenant {new_ipaddress.tenant} and has not been loaded.",
                            )

Additional Context

The issue stems from the fact that the tenants listed in ignore_tenants are not present in the database, which causes an exception to be raised when the code attempts to associate an IP address or subnet_as_prefix with these tenants.

In the current implementation, this is not checked before creating IP addresses or subnet_as_prefix, leading to a Tenant.DoesNotExist exception when one of these ignored tenants is referenced. To resolve this, we need to implement a mechanism that either skips these tenants entirely or assigns a None value for the tenant but still proceeds with the IP address or subnet_as_prefix creation.

Please advise on which approach would be most appropriate:

  • Skipping tenants: Don't create the IP addresses or subnet_as_prefix associated with tenants that are ignored.
  • Setting tenant to None: Proceed with creating the IP addresses or subnet_as_prefix, but set the tenant to None when an ignored tenant is encountered.

Thank you

@jdrew82 jdrew82 added type: bug Issues/PRs addressing a bug. integration: ciscoaci Issues/PRs for Cisco ACI integration. labels Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration: ciscoaci Issues/PRs for Cisco ACI integration. type: bug Issues/PRs addressing a bug.
Projects
None yet
Development

No branches or pull requests

2 participants