Device Redundancy Groups

[bryanculver]

Closes: #1892

What's Changed

Starting RedundancyGroup implementation.

So far what's implemented:

• DeviceRedundancyGroup model, supporting custom fields, tags, statuses, SecretGroups
  • failover_strategy is a choice, currently identified as Active/Active or Active/Passive
• Device has been updated:
  • A FK to DeviceRedundancyGroup as device_redundancy_group
  • A PositiveSmallInt for optional priority as device_redundancy_group_priority
• ConfigContext has been updated:
  • A ManyToManyField to dcim.DeviceRedundancyGroup as device_redundancy_groups for the purposes of applying a ConfigContext based upon a Devices DeviceRedundancyGroup membership
  • Necessary form, queryset, filter updates to make use of this change
  • Bug fixed on ConfigContextFilterSet: schema not a valid filter but exists on form, see https://demo.nautobot.com/extras/config-contexts/
  • schema filter form field on ConfigContextFilterForm wasn't compatible with proper filter field introduced above

Other changes due to changes in Factory fuzzing exposing test failures

• Devices form test (DeviceTestCase) was flaky

  • A DeviceType from factory may not be found that has a Manufacturer with an associated Platforms that is full depth and 1U height
    • We just needed a DeviceType of non-zero U height
    • Tests updated if needed to be offset by a derived U height

• BaseNetworkQuerySet and IPAddressQuerySet's parse_network_string function overhauled (used when performing incomplete searches for network addresses)

  • Searching for b2a (a valid abbreviated first hextet for an IPv6 address) would not return results. This was due to RE_HEXTET requiring a unabbreviated hextet for searching
    • RE_HEXTET updated to permit abbreviated forms of hextet
    • This change exacerbated the bug below
  • Searching for 10 would always result in only IPv4 searches, even though it also is a valid IPv6 beginning hextet. This is an ambiguous search string so should return results for both.
    • _is_ambiguous_network_string introduced to help determine if inputted string could be either a valid IPv4 beginning octet or a valid IPv6 beginning hextet
    • string_search updated to search for both IPv4 and IPv6 addresses if an ambiguous search string is given
  • To keep the output of parse_network_string consistent (both derived IP version and always a single netaddr.IPNetwork) and DRY the following changes were made:
    • _safe_parse_network_string introduced to forcibly try to parse inputted search string as either a IPv4 or IPv6 address or fragment
      • This calls internal parse_ipv4 and parse_ipv6 as parse_network_string did before
      • This keeps parse_network_string's original error handling for thrown netaddr.core.AddrFormatError and ensures always return a valid netaddr.IPNetwork object
    • _check_and_prep_ipv6 introduced to perform much of the "probably IPv6 address" logic originally done in parse_network_string
      • Will also prepare the string for parse_ipv6 by appending the necessary :s
    • parse_network_string simplified significantly by leveraging the discrete functions above
  • No tests were harmed in this rewrite however new ones were added to test the discrete functions added above

• RouteTargets View test case (RouteTargetTestCase) was exposed to have a flaky csv_data structure

  • Sometimes Tenants name can include a comma so structure was updated to always escape this field

• PrefixFactory may randomly decided to create a child of 2.2.2.2/32. This is because it checks the size of the prefix and attempts to create children if it's of a large enough size.

  • Weird behavior of prefix.size:
    • Prefix(2.2.2.2/31).size correctly returns 2 which can have two children in it, 2.2.2.2/32 and 2.2.2.3/32
    • Prefix(2.2.2.2/32).size correctly returns 1 but cannot have a child
  • Factory updated to not create children if prefix.size is 1

• VirtualChassis Filter test case (VirtualChassisTestCase) had too narrow of a region lookup

  • Region.objects.filter(sites__isnull=False, children__isnull=True, parent__isnull=True) may not have returned the required 3 Regions. The tests below were improved in earlier commits to not require no parents and no children so updated to cls.regions = Region.objects.filter(sites__isnull=False)[:3]

• Similar failure for DynamicFilterLookupExpressionTest Filter test as VirtualCassis filter test above.

• RackGroup Model test (test_change_rackgroup_site) may have randomly chosen the same site to update the rackgroups existing membership to

  • site_b updated to be the last object returned from an exclusion of the existing membership site

• Similar failure for Prefix Model test as RackGroup above.

• Location View test case (LocationTestCase) was exposed to have a flaky csv_data structure

  • Sometimes names can include a comma so structure was updated to always escape these fields

• RIR API View test case (RIRTest) may have had no deleteable objects to test

  • Aggregate has in inbound protected relationship to RIR which would prevent deletion. Instead of increasing random pool size, an ensured deletable object was created

• test_slug_not_modified was updated to ensure no collision on new slug source value as well as changing lookup expressions from __contains to __exact in the case where auto-generated names have nested details (ex: Device names: Device A and Device A - PSU)

TODO

• Explanation of Change(s)
• Added change log fragment(s) (for more information see the documentation)
• Attached Screenshots, Payload Example
• Unit, Integration Tests
• Documentation Updates (when adding/changing features)
• NA Example Plugin Updates (when adding/changing features)
• Outline Remaining Work, Constraints from Design

[lampwins]

Built-in validation for no two Devices with same priority?

I'd say no to this validation. There are valid real-world use cases where two devices would have the same priority. For instance, I might have a cluster of 4 firewalls with two 10 and two 20. End users could implement custom validators if they want this validation.

[gsnider2195]

Do we want a has_device_redundancy_group filter?

[bryanculver]

Does that hold up the merge?

[gsnider2195]

no

[jathanism]

Small requests here. Loving the changes to parse_network_string!

[jathanism]

[jathanism]

Not that it's a huge deal, but exact is the default. If we're switching to this, we might as well rip this out:

Suggested change

	filter_ = self.slug_source + "__exact"
	self.model.objects.filter(**{filter_: self.slug_test_object}).update(
	self.model.objects.filter(**{self.slug_source: self.slug_test_object}).update(

[bryanculver]

Did you test that suggestion?

[jathanism]

We're safe to use select_related, it just won't be "optimized" by Cacheops. See: Cacheops caveats, notably:

Update of "selected_related" object does not invalidate cache for queryset. Use .prefetch_related() instead.

[jathanism]

Another minor nit, but pretty sure it's redundant to specify field_name and to_field_name here. First because field_name is only required if the related field differs from the name of the filter defined on the filterset, and to_field_name defaults to slug. :)

Suggested change

	field_name="secrets_group",
	queryset=SecretsGroup.objects.all(),
	to_field_name="slug",
	queryset=SecretsGroup.objects.all(),

[jathanism]

Same here, field_name is likely redundant.

Suggested change

field_name="device_redundancy_group",

[jathanism]

Suggested change