NetBox 3.X #110

snaselj · 2023-11-24T17:26:39Z

Closes: NaN

What's Changed

Dropped NetBox 2.x support.
Dropped ChangeLog import.
Implemented NetBox 3.0 - 3.6 support.
Dropped Nautobot 1.x support.
Implemented Nautobot 2.0 - 2.1 support.
Added importer development documentation.

Considerations

Implement writing created and last_updated fields.
Test to re-run the import.
Parametrize importer to fail if any validation fails.
Resolve skipped fields and TBDs in nautobot_netbox_importer/diffsync/netbox.py.
Resolve validation error: django.core.exceptions.ValidationError: ['Rack R308 (Row 3) and power panel Panel 4 (MDF) are in different locations'].
Resolve validation error: django.core.exceptions.ValidationError: {'parent': ['A Location of type Location may only have a Location of the same type as its parent.']}.
Change InternalTypeStr to enum.
Stream processing of input JSON file, to avoid loading the entire file into memory.

Not a scope of this PR

Implement mysql tests?
Implement SSoT Job.
- Allow DiffSync deleting in Nautobot?
- Add-only ManyToMany field values?
- Add-only custom field values?

glennmatthews · 2023-12-05T14:27:04Z

Was it necessary to move so much code around as part of this feature? I don't really know where to begin reviewing this when diff-wise it basically looks like an entirely new implementation of the app.

glennmatthews · 2023-12-05T14:29:03Z

Also, does this just add 3.0 support or does it also drop support for all older NetBox versions? I can't really tell with the scope of the diffs here.

snaselj · 2023-12-05T15:46:40Z

Was it necessary to move so much code around as part of this feature? I don't really know where to begin reviewing this when diff-wise it basically looks like an entirely new implementation of the app.

Yes, the implementation is different, supports multiple NetBox 3.0, 3.1 and 3.2 now, going to add support up to 3.6. Will prepare better developer documentation before making it ready for review.

snaselj · 2023-12-05T15:47:34Z

Also, does this just add 3.0 support or does it also drop support for all older NetBox versions? I can't really tell with the scope of the diffs here.

Dropped support for Nautobot 1.x and NetBox 2.x, updated the description accordingly.

glennmatthews · 2023-12-11T19:26:27Z

Just a quick note that changelog imports was a user-requested feature in the past. How difficult would it be to re-add?

snaselj · 2023-12-11T19:49:34Z

Just a quick note that changelog imports was a user-requested feature in the past. How difficult would it be to re-add?

I don't have any testing data for it, so it's really difficult to estimate. Depends on the level of correctness, if it's just about importing JSON change object without change, then it should be quite easy to implement. When the importer should convert fields in JSON based on the content type, some issues can arise.

nautobot_netbox_importer/diffsync/base.py

glennmatthews · 2023-12-11T19:31:11Z

nautobot_netbox_importer/diffsync/base.py

+from typing import Any
+from typing import Iterable
+from typing import List
+from typing import Mapping
+from typing import MutableMapping
+from typing import Tuple
+from typing import Type
+from typing import Union
+from uuid import NAMESPACE_DNS
+from uuid import UUID
+from uuid import uuid5


nit: wouldn't the more usual black/isort/ruff convention be to group these, a la:

Suggested change

from typing import Any

from typing import Iterable

from typing import List

from typing import Mapping

from typing import MutableMapping

from typing import Tuple

from typing import Type

from typing import Union

from uuid import NAMESPACE_DNS

from uuid import UUID

from uuid import uuid5

from typing import (

Any,

Iterable,

List,

Mapping,

MutableMapping,

Tuple,

Type,

Union,

)

from uuid import NAMESPACE_DNS, UUID, uuid5

Same question/comment throughout.

This way passes through all CI checks and produces better git diffs.

nautobot_netbox_importer/diffsync/nautobot.py

glennmatthews · 2023-12-11T19:34:44Z

nautobot_netbox_importer/diffsync/nautobot.py

+    def set_instance_defaults(self, **defaults: Any) -> None:
+        """Set default values for a Nautobot instance constructor."""
+        self.constructor_kwargs = defaults


Would it make sense for this to be set at __init__ time, or does it need to be a separate setter like this?

The structure of wrappers is created on the fly. Some parts can be set in the constructors, however it's possible, that the wrapper will already exists, before configuring it.

nautobot_netbox_importer/diffsync/nautobot.py

nautobot_netbox_importer/diffsync/netbox.py

nautobot_netbox_importer/diffsync/source.py

chadell · 2023-12-13T06:42:31Z

Overall feedback from notes:

Dropped NetBox 2.x support. -> Then, we should release a netbox-import version that supports as much as we did previous to this PR, and add a compatibility matrix
Dropped ChangeLog import. -> We should add this back, as @glennmatthews said, it was a customer ask and I see value for adopters.
Implemented NetBox 3.0 - 3.4 support.
Dropped Nautobot 1.x support. -> The same as point 1, we should have a release of NetBox 2.x -> Nautobot 1.x
Implemented Nautobot 2.0 support.

chadell · 2024-01-10T13:55:27Z

docs/user/summary.md

+- Content Types Back Mapping: --------------------------------------------------
+  Back mapping deviations from Nautobot content type to the source content type
+extras.role => Ambiguous
+users.user => auth.user


why is this in this list if not "Ambiguous"?

This list contain all deviations, mapping to the different content type is considered a deviation.

chadell · 2024-01-10T13:55:47Z

docs/user/summary.md

+
+## Content Types Back Mapping
+
+This section shows back mapping deviations from Nautobot content types to NetBox content types. Only those content types that differ between NetBox and Nautobot are shown.


what is the action task for the user with this info?

No action is necessary from the user, just summarizing deviations to be aware of that.

chadell · 2024-01-10T13:57:03Z

docs/user/summary.md

+_name (DATA) => _name (PrivateProperty)
+label (DATA) => label (CharField)
+description (DATA) => description (CharField)
+module (DATA) => module (NotFound)


what does it mean? is this information lost?

Yes, NotFound fields are skipped as described above.

https://github.com/nautobot/nautobot-app-netbox-importer/pull/110/files#diff-1a4b92f2e8bf12ae4a44e4d63c8a42caa969e6f492a4b61e1ae19348b2841921R245

chadell · 2024-01-10T13:58:25Z

docs/user/summary.md

+    - `(ReadOnlyProperty')` marks fields that are read-only properties in Nautobot and can't be imported.
+    - `(PrivateProperty)` marks fields that are prefixed with an underscore `_` and are considered private properties. Those fields are not imported.
+    - `(NotFound)` indicates the field is not found in Nautobot and can't be imported.
+    - `(DoNotImportLastUpdated)` are fields that are not imported as they are automatically updated by Nautobot.


the word seems only for "LastUpdated" not for any field updated by Nautobot

Updated the documentation to address this.

chadell · 2024-01-10T13:58:46Z

docs/user/summary.md

+- `=>` as a separator.
+- Nautobot field name.
+- Field type in parentheses with the following special cases:
+    - `SKIPPED` means the field is intentionally skipped by the importer.


why? could this be changed by the user?

Updated the importer to display disable reason, instead of SKIPPED for each disabled field.

chadell · 2024-01-10T14:00:46Z

docs/dev/generator.md

+
+The NetBox specific component is further segmented into sections, located in the `nautobot_netbox_importer/diffsync` directory:
+
+- `adapters/nautobot.py`: Inherits `NautobotAdapter` from the generic `NautobotAdapter` and implements pieces necessary for SSoT job.


why do we need a generic NautobotAdapter to create yet another NautobotAdapter? what is this abstraction providing?

The first one is DiffSync only related implementation. The other one is adding SSoT related things, but it's not probably necessary.

The reason to create the second one was to make the file structure the same way as is in other SSoT apps / integrations.

chadell · 2024-01-10T14:04:59Z

docs/dev/generator.md

+
+The initial step involves creating a `SourceAdapter()`. It accepts an argument, `get_source_data`, which is `Callable` that returns `Iterable` of the source data items. Each source data item is encapsulated in `SourceRecord(content_type: ContentTypeStr, data: Dict)` instances. `SourceAdapter` constructor also passes any additional arguments to its ancestor, the `DiffSync` class.
+
+The data undergoes two cycles: the first to establish the structure and the second to import the actual data.


could you provide a structure with these two cycles and the sub steps involved?

Steps are described in this document, updated this doc to mention that.

chadell · 2024-01-10T14:08:04Z

General Questions:

What has to be done (as a developer) to support a Nautobot minor version?
What has to be done (as a developer) to support a Nautobot major version?
What has to be done (as a developer) to support a Nautobot App?
What has to be done (as a developer) to support a NetBox new version?
What has to be done (as a user) to provide some customization in the import method?

bryanculver · 2024-01-10T17:22:23Z

General Questions:

What has to be done (as a developer) to support a Nautobot minor version?

What has to be done (as a developer) to support a Nautobot major version?

What has to be done (as a developer) to support a Nautobot App?

What has to be done (as a developer) to support a NetBox new version?

What has to be done (as a user) to provide some customization in the import method?

I'd suggest more targeted questions & answers, because if nothing changed between versions it can be a noop (AFAIK the way this refactor works is that it's data-type check instead of version check alleviates many of those concerns). So:

What has to be done (as a developer) if NetBox changes a field's data type?
(same as above but Nautobot side)
What has to be done (as a developer) if NetBox adds a new model?
What has to be done (as a developer) if NetBox changes a relation from 1-to-many to many-to-many?

Link to places where we are already handling these changes instead of speaking in hypotheticals.

I'd also like to see two other pieces of information:

A logical (not technical) diagram for the data from in from NetBox and out to Nautobot
- [NetBox] -> SourceAdapter -> (what does data look like here and where it is "held") -> TargetAdapter -> [Nautobot]
- And what types of decision trees does a data type transformation look like?
A few examples of roadblocks we ran into with the existing architecture when adding support for new versions (the below are hypotheticals so they are likely incorrect, but for demonstrations for justification)
- "When adding support for Netbox 3.X+1 we were copying and pasting much of the same code for 3.X"
- "When adding support for Nautobot 2.0, copy and paste code for Netbox 3.2-3.6 had to have repetitive updates and we found bugs where it was fixed in one place and not others"
- "Removing support for Nautobot 1.X was proving to be very difficult and was effectively a rewrite anyway"

- Refactored input reading

snaselj · 2024-01-12T15:51:42Z

General Questions:

What has to be done (as a developer) to support a Nautobot minor version?

What has to be done (as a developer) to support a Nautobot major version?

What has to be done (as a developer) to support a Nautobot App?

What has to be done (as a developer) to support a NetBox new version?

What has to be done (as a user) to provide some customization in the import method?

I'd suggest more targeted questions & answers, because if nothing changed between versions it can be a noop (AFAIK the way this refactor works is that it's data-type check instead of version check alleviates many of those concerns). So:

What has to be done (as a developer) if NetBox changes a field's data type?

(same as above but Nautobot side)

What has to be done (as a developer) if NetBox adds a new model?

What has to be done (as a developer) if NetBox changes a relation from 1-to-many to many-to-many?

Link to places where we are already handling these changes instead of speaking in hypotheticals.

I'd also like to see two other pieces of information:

A logical (not technical) diagram for the data from in from NetBox and out to Nautobot

Still WIP

[NetBox] -> SourceAdapter -> (what does data look like here and where it is "held") -> TargetAdapter -> [Nautobot]

And what types of decision trees does a data type transformation look like?

A few examples of roadblocks we ran into with the existing architecture when adding support for new versions (the below are hypotheticals so they are likely incorrect, but for demonstrations for justification)

"When adding support for Netbox 3.X+1 we were copying and pasting much of the same code for 3.X"

"When adding support for Nautobot 2.0, copy and paste code for Netbox 3.2-3.6 had to have repetitive updates and we found bugs where it was fixed in one place and not others"

"Removing support for Nautobot 1.X was proving to be very difficult and was effectively a rewrite anyway"

Questions are addressed in doc/dev/faq.md and doc/user/faq.md.

snaselj · 2024-01-12T15:52:49Z

General Questions:

What has to be done (as a developer) to support a Nautobot minor version?

What has to be done (as a developer) to support a Nautobot major version?

What has to be done (as a developer) to support a Nautobot App?

What has to be done (as a developer) to support a NetBox new version?

What has to be done (as a user) to provide some customization in the import method?

Questions are addressed in doc/dev/faq.md and doc/user/faq.md.

chadell · 2024-01-16T06:26:08Z

docs/dev/faq.md

+## What has to be done to support a major NetBox version?
+
+A new major version of the `nautobot-netbox-importer` package should be released to address updated deviations between NetBox and Nautobot models, independent of the previous major version.
+
+## What has to be done to support a major Nautobot version?
+
+Similar to the previous question, a new independent release of the `nautobot-netbox-importer` package should be released.


ok, so every time a new major version for Nautobot or NetBox is released, a new major version of this app has to be released to match it. I guess this will be documented in a versions matrix or similar

Seems to be the best option for me. Optionally it would be possible to support multiple major NetBox releases at once. Depends on the release cadence, and the decision which major release matrix should be supported. This can be reconsidered in the future.

Updated compatibility matrix to cover the current status.

chadell · 2024-01-16T06:28:25Z

docs/dev/faq.md

+
+Similar to the previous question, a new independent release of the `nautobot-netbox-importer` package should be released.
+
+## What has to be done to support a Nautobot App?


are we planning to add out-of-the-box support to some Nautobot Apps? or it's only for customization?
if so, what happens when there is a new major release on a Nautobot App?

I understand that the NetBox plugin ecosystem could be also covered by this plugin with similar type of customization, isn't it? if so, maybe document it

Yes, official Nautobot Apps support is a planned feature.

Added Q&A covering Nautobot / NetBox apps support.

snaselj · 2024-01-16T09:19:41Z

A logical (not technical) diagram for the data from in from NetBox and out to Nautobot

Data flow diagram is here: https://github.com/nautobot/nautobot-app-netbox-importer/pull/110/files#diff-a854f6366a0abe3dea5e2e1ce1802a7123a271bb6670d891c805b923b7cfc77c

snaselj · 2024-01-16T09:20:12Z

All comments should be addressed now.

bryanculver · 2024-01-16T18:51:44Z

docs/user/summary.md

+wireless.wirelesslan => wireless.wirelesslan | Disabled with reason: Nautobot content type: `wireless.wirelesslan` not found
+wireless.wirelesslink => wireless.wirelesslink | Disabled with reason: Nautobot content type: `wireless.wirelesslink` not found
+django_rq.queue => django_rq.queue | Disabled with reason: Nautobot content type: `django_rq.queue` not found
+secrets.secret => secrets.secret | Disabled with reason: Nautobot content type: `secrets.secret` not found


Is this the same as extras.secret?

Added to the follow-up ticket.

bryanculver · 2024-01-16T18:53:51Z

I don't pretend to be the one who should be a gatekeeper to this PR, and I'm sure it might take some time to grok the documentation @snaselj has added but I do feel it should be re-reviewed.

chadell

LGTM

Let's get this in, and it could be a great opportunity to create a follow-up PR to address the new NetBox 3.7 release to demonstrate how a new version should be added

chore: NetBox 3.x importer

003426e

snaselj force-pushed the u/snaselj-netbox3 branch from 9cfa893 to 003426e Compare December 4, 2023 14:26

snaselj added 2 commits December 5, 2023 14:21

chore: Nautobot 2.x

b13d43a

chore: Tests

6962b15

chore: Tests

e23fe55

snaselj added 12 commits December 7, 2023 14:25

chore: 3.3, 3.4

8fc8470

fix: Tests

54ae4ae

fixes

e4ebd34

chore: PR preparations

ec49203

Merge remote-tracking branch 'origin/develop' into u/snaselj-netbox3

b11b23f

fix: Missing summary

7a6220f

fix: CI to Nautobot 2

aab1258

fix: pylint

e82ca30

chore: Docs updates

f592da4

fix: Python 3.8 compatibility

a9fcbdb

fix: pylint

0acf4e8

chore: Disable mysql tests

420e1e3

snaselj marked this pull request as ready for review December 11, 2023 18:59

snaselj requested review from glennmatthews, lampwins and chadell as code owners December 11, 2023 18:59

glennmatthews reviewed Dec 11, 2023

View reviewed changes

chore: PR fixes

e0d2b50

chadell reviewed Jan 10, 2024

View reviewed changes

snaselj changed the title ~~NetBox 3.0~~ NetBox 3.X Jan 10, 2024

snaselj added 3 commits January 12, 2024 08:26

fix: Add disable reason to fields

0f2e91b

- Refactored input reading

feat: NetBox Adapater customization

cbee828

docs: FAQs and updates

4bc3efb

doc: Data flow

b4e6f23

chadell reviewed Jan 16, 2024

View reviewed changes

snaselj added 2 commits January 16, 2024 08:58

docs: Nautobot / NetBox apps support

427b992

doc: compatibility matrix

7def426

bryanculver reviewed Jan 16, 2024

View reviewed changes

chadell approved these changes Jan 18, 2024

View reviewed changes

whitej6 merged commit bca6058 into develop Jan 18, 2024
14 checks passed

snaselj deleted the u/snaselj-netbox3 branch January 22, 2024 08:00

This was referenced Apr 8, 2024

Add support for importing NetBox 3.5.x data #104

Closed

Add support for importing NetBox 3.6.x data #105

Closed

gsnider2195 mentioned this pull request Sep 6, 2024

Cookie updated by NetworkToCode Cookie Drift Manager Tool #179

Merged


		## Content Types Back Mapping

		This section shows back mapping deviations from Nautobot content types to NetBox content types. Only those content types that differ between NetBox and Nautobot are shown.


		The NetBox specific component is further segmented into sections, located in the `nautobot_netbox_importer/diffsync` directory:

		- `adapters/nautobot.py`: Inherits `NautobotAdapter` from the generic `NautobotAdapter` and implements pieces necessary for SSoT job.


		The initial step involves creating a `SourceAdapter()`. It accepts an argument, `get_source_data`, which is `Callable` that returns `Iterable` of the source data items. Each source data item is encapsulated in `SourceRecord(content_type: ContentTypeStr, data: Dict)` instances. `SourceAdapter` constructor also passes any additional arguments to its ancestor, the `DiffSync` class.

		The data undergoes two cycles: the first to establish the structure and the second to import the actual data.


		Similar to the previous question, a new independent release of the `nautobot-netbox-importer` package should be released.

		## What has to be done to support a Nautobot App?

NetBox 3.X #110

NetBox 3.X #110

Conversation

snaselj commented Nov 24, 2023 • edited Loading

Closes: NaN

What's Changed

Considerations

Not a scope of this PR

glennmatthews commented Dec 5, 2023

glennmatthews commented Dec 5, 2023

snaselj commented Dec 5, 2023 • edited Loading

snaselj commented Dec 5, 2023

glennmatthews commented Dec 11, 2023

snaselj commented Dec 11, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chadell commented Dec 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chadell commented Jan 10, 2024 • edited Loading

bryanculver commented Jan 10, 2024

snaselj commented Jan 12, 2024

snaselj commented Jan 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

snaselj commented Jan 16, 2024

snaselj commented Jan 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bryanculver commented Jan 16, 2024

chadell left a comment

Choose a reason for hiding this comment

snaselj commented Nov 24, 2023 •

edited

Loading

snaselj commented Dec 5, 2023 •

edited

Loading

chadell commented Jan 10, 2024 •

edited

Loading