nautobot / nautobot-app-design-builder

https://docs.nautobot.com/projects/design-builder/en/latest/
Other
8 stars 2 forks source link

!next-prefix action tag using location and prefix type fails with prefix_length is null #154

Closed jeffkala closed 3 weeks ago

jeffkala commented 6 months ago

Environment

Expected Behavior

!next-prefix action tag will get-next properly when a filter criteria is specified to determine the "parent" prefixes to search in.

Observed Behavior

Prefix None failed validation

prefix_length: This field cannot be null.

Steps to Reproduce

  1. Create design
    ---
    prefixes:
    - "!next_prefix":
      locations: "{{ site_name.parent.parent.parent.id }}"
      type: "container"
      length: "{{ ipam_templates[site_size | lower]['template_prefix_size'] | int }}"
    status__name: "Active"
    description: "{{ site_name }} Parent Prefix"
    type: "container"
    "!ref": "parent_prefix"
  2. Have available prefixes in the parent location
  3. Execute the job.
  4. Gives error
    Traceback (most recent call last):
    File "/opt/nautobot/lib/python3.8/site-packages/celery/app/trace.py", line 477, in trace_task
    R = retval = fun(*args, **kwargs)
    File "/opt/nautobot/lib/python3.8/site-packages/celery/app/trace.py", line 760, in __protected_call__
    return self.run(*args, **kwargs)
    File "/opt/nautobot/lib/python3.8/site-packages/nautobot/extras/jobs.py", line 1136, in run_job
    result = job(*args, **kwargs)
    File "/opt/nautobot/lib/python3.8/site-packages/nautobot/extras/jobs.py", line 149, in __call__
    return self.run(*args, **deserialized_kwargs)
    File "/opt/nautobot/lib/python3.8/site-packages/nautobot_design_builder/design_job.py", line 143, in run
    return self._run_in_transaction(dryrun, **kwargs)
    File "/usr/lib/python3.8/contextlib.py", line 75, in inner
    return func(*args, **kwds)
    File "/opt/nautobot/lib/python3.8/site-packages/nautobot_design_builder/design_job.py", line 209, in _run_in_transaction
    raise ex
    File "/opt/nautobot/lib/python3.8/site-packages/nautobot_design_builder/design_job.py", line 190, in _run_in_transaction
    self.implement_design(context, design_file, not dryrun)
    File "/opt/nautobot/lib/python3.8/site-packages/nautobot_design_builder/design_job.py", line 138, in implement_design
    self.environment.implement_design(design, commit)
    File "/opt/nautobot/lib/python3.8/site-packages/nautobot_design_builder/design.py", line 761, in implement_design
    raise ex
    File "/opt/nautobot/lib/python3.8/site-packages/nautobot_design_builder/design.py", line 746, in implement_design
    self._create_objects(self.model_map[key], value)
    File "/opt/nautobot/lib/python3.8/site-packages/nautobot_design_builder/design.py", line 822, in _create_objects
    model.save()
    File "/opt/nautobot/lib/python3.8/site-packages/nautobot_design_builder/design.py", line 613, in save
    raise errors.DesignValidationError(self) from validation_error
    nautobot_design_builder.errors.DesignValidationError: <unprintable DesignValidationError object>

For additonal testing did validate the same _get_next functionality from ORM returns a value.

>>> stuff = {"locations": l.parent.parent.parent.id, "type": "container"}
>>> query = Q(**stuff)
>>> query
<Q: (AND: ('locations', UUID('7e3cff0a-629a-42fe-b578-d02ab4cff4ae')), ('type', 'container'))>
>>> prefixes = Prefix.objects.filter(query)
>>> prefixes
<PrefixQuerySet [<Prefix: 10.34.0.0/16>, <Prefix: 1.34.0.0/16>, <Prefix: 1.41.0.0/16>]>
>>> length = 25
>>> _get_next(prefixes, length)
'10.34.0.0/25'
abates commented 6 months ago

I think the issue here is that the length field is being passed in as a string, but it needs to be an integer. Try this:

---
prefixes:
  - "!next_prefix":
      locations: "{{ site_name.parent.parent.parent.id }}"
      type: "container"
      length: {{ ipam_templates[site_size | lower]['template_prefix_size'] | int }}
    status__name: "Active"
    description: "{{ site_name }} Parent Prefix"
    type: "container"
    "!ref": "parent_prefix"
jeffkala commented 6 months ago

Did try this and still got the same error. Even just hardcoded the length and it fails.

---
prefixes:
  - "!next_prefix":
      locations: "{{ site_name.parent.parent.parent.id }}"
      type: "container"
      length: 25
    status__name: "Active"
    description: "{{ site_name }} Parent Prefix"
    type: "container"
    "!ref": "parent_prefix"
jeffkala commented 6 months ago

Does seem like its something about no prefixes available in certain circumstances. If I play around with it and make the length much smaller it results in the expected error around "no prefixes available for ..." still debugging further to see what is happening but if I ensure the prefix is available it works as expected.

jeffkala commented 6 months ago

Still troubleshooting, but the culprit was a relationship with the attributes below. Once the relationship was deleted and nautobot restarted, the design job started working. image

itdependsnetworks commented 5 months ago

In a similar example, I just changed the source and destination to not have potential namepsace issues with the method names, such as prefix -> prefix1 and VLAN -> VLAN1 and that fixed the issue.

abates commented 5 months ago

I was able to reproduce this in a test and it is exactly as @itdependsnetworks mentioned. Design Builder translates the source/destination labels of custom relationships to actual fields on a proxy model. If there is a clash then this type of issue is caused. If you look at the Prefix model it already has a vlan field:

In [4]: Prefix._meta.get_field("vlan")
Out[4]: <django.db.models.fields.related.ForeignKey: vlan>
abates commented 1 month ago

@jeffkala do you consider this a bug that we should try and mitigate?

jeffkala commented 1 month ago

At least maybe we can just document it and provide best practices to work around it? I'm not fully onboard that we "must" mitigate it as it seems rare its hit.

abates commented 3 weeks ago

PR #200 introduces an error message and protection from overwriting built-in fields. This image is an example error message in the job result: Screenshot 2024-10-24 at 10 24 25 AM

jeffkala commented 3 weeks ago

PR #200 introduces an error message and protection from overwriting built-in fields. This image is an example error message in the job result: Screenshot 2024-10-24 at 10 24 25 AM

looks great @abates