netbox-community / netbox

The premier source of truth powering network automation. Open source under Apache 2. Try NetBox Cloud free: https://netboxlabs.com/free-netbox-cloud/
http://netboxlabs.com/oss/netbox/
Apache License 2.0
15.93k stars 2.56k forks source link

Storage of contextual configuration data #1349

Closed bdlamprecht closed 6 years ago

bdlamprecht commented 7 years ago

Issue type:

Feature Request

Python version: 2.7.12 NetBox version: 2.0.8

With the goal of NetBox being THE "Source of Truth" for the desired state of a network, I noticed that it does not currently have a location to store values for IPv4/IPv6 addresses of DNS, NTP, TACACS, SYSLOG, SNMP, and other servers which support network devices (as well as servers).

While thinking through real-world scenarios, these values are usually per region and/or per site, so custom_fields could be used, but I think it would be applicable to a large portion of NetBox users to dedicate the time to implement this update to the ORM schema.

For my thinking, it I believe it would be best to possibly create a new section in IPAM to store these key:value pairs which could then be tied to either regions or sites to minimize querying the DB so often, but it is highly likely that some other place would be more beneficial.

Pinging @jeremystretch for his thoughts.

jeremystretch commented 7 years ago

I definitely see the value in adding something like this to NetBox. The tricky part is going to be implementing it. We want to provide something more valuable than a vanilla key/value store: as mentioned, we should be able to tie values to NetBox objects like regions, sites, etc. But we also want to ensure we maintain sufficient flexibility, not knowing what people might want to use it for. We also don't want to duplicate the functionality provided by custom fields.

At a higher level, this is going to tie in somewhat with configuration management (e.g. the ability to render device configurations using templates and NetBox data), but I haven't given that topic much thought yet.

bdlamprecht commented 7 years ago

I agree that great care needs to be taken to ensure we don't just implement another version of custom_fields. As you stated, you haven't given it much thought yet, but when you do, I'd be happy to help think through some of the overall structure and how that information should be tied together.

I'm not an expert by any means, but would just like to contribute back for all the help that has been provided.

jeremystretch commented 7 years ago

@bdlamprecht What prompted you to open this issue? Are you interested in generating device configs using this data as context, or did you have something different in mind?

bdlamprecht commented 7 years ago

Yes, I'm working on a POC using NetBox as a "Source of Truth" for Ansible which will then create / manage configuration templates for my large enterprise customer (1000+ templates) and push those changes onto the device using NAPALM (or possibly another Ansible "module").

Our "service delivery" team that installs the so-called "templates" which are created by hand rarely get them correct (which is understandable as they are, in my opinion, unnecessarily complex). This project I'm working on is to try and automate and orchestrate that process.

jeremystretch commented 7 years ago

Here's an idea: We create a ConfigContext model under the extras app to hold arbitrary key/value data in the user's choice of formats (YAML, JSON, etc.). Each ConfigContext instance can be tied to a set of regions and/or sites, with site context data superseding region context data. This allows users to define arbitrary data without incurring significant overhead.

For example, I create a ConfigContext assigned to a region called "North America" with the following:

dns_servers:
    - 8.8.8.8
    - 8.8.4.4
ntp_servers:
    - 99.224.25.39
    - 69.164.202.202
    - 132.163.4.102

Then I create a second ConfigContext for a specific site in that region with special DNS servers:

dns_servers:
    - 192.0.2.1
    - 192.0.2.2

The context available for a device in that site would render as:

dns_servers:
    - 192.0.2.1
    - 192.0.2.2
ntp_servers:
    - 99.224.25.39
    - 69.164.202.202
    - 132.163.4.102

We could make this available via the API, but the next stop logical step would be to render device configs from templates directly within NetBox.

bdlamprecht commented 7 years ago

Great, I love the idea of the "more-specific" information overrides the other "generic" information.
And the sooner it becomes available in the API the better, but I don't want to sound demanding. :smile:

I'm confused by your last statement however... Are you proposing that NetBox be used to store device configuration templates? If so, that is a huge endeavor, but not sure it is needed.

The POC I'm working on now would store the templates in GIT so you can generate the configs and push them to the devices based off of which "release" you have "checked-out" in GIT. I don't feel the need to duplicate efforts by other tools already in existence.

All this being said, it is great work and, once again, I really appreciate all of the work that you're doing with developing NetBox.

jeremystretch commented 7 years ago

I'm confused by your last statement however, are you proposing that NetBox be used to store device configuration templates? If so, that is a huge endeavor, but not sure it is needed.

Not store them natively, but perhaps read them from a repository.

The POC I'm working on now would store the templates in GIT so you can generate the configs and push them to the devices based off of which "release" you have "checked-out" in GIT.

What are you using to render the templates? Presumably you're pulling data from NetBox via the API?

bdlamprecht commented 7 years ago

Well, don't judge me as I'm new to the developer world and I'm fairly sure there's a better / more efficient way to accomplish the end goal I have.

Anyways, I'm using the Ansible uri module to query the NetBox API and then register the results into variables to be used by the Jinja2 templates I've created for both Cisco and Juniper devices.

jeremystretch commented 7 years ago

Well, don't judge me as I'm new to the developer world

Hey, me too! :smile:

Anyways, I'm using the Ansible uri module to query the NetBox API and then register the results into variables to be used by the Jinja2 templates I've created for both Cisco and Juniper devices.

That seems like a very reasonable approach. So it seems like in your case just making additional context data available via the API would be sufficient.

bdlamprecht commented 7 years ago

Yeah, I think that would work.

I'm not sure if I made this clear, but it would be important, at least for my scenario, to have the new ConfigContext data available (through the API) on a per-device view.

As an example, something like /api/extras/[NAME_HERE]/?device=[X] which would use the hierarchy you explained above to provide the more-specific information (region -> site -> device). However, if that is not possible due to the structure of the ORM, I'll take anything you can give me. :smile:

BTW, it doesn't seem like you're new to the developer world based off of how quickly you implement new features and update NetBox, but if so :1st_place_medal:

lampwins commented 7 years ago

So this is really exciting to see. I have been building this sort of thing in other places in my automation suite integrating with netbox. I have been really flustered because IMHO the best place for this is inside of netbox itself. That being said, I do have a few ideas (and would love to help see them through)...

Today I store this data either in group vars in ansible playbooks (for generic role based config) and a hacked together CMDB (for device specific config).

Not store them natively, but perhaps read them from a repository.

@jeremystretch I think it is best to store this in netbox. I would be fine storing this as actual yaml (as flat files) or as json in postgres. That is flexible though; if the solution is elegant, I don't really care.

I would want to see the ability to assign a ConfigContext to (but not necessarily limited to):

Device type and role are the interesting ones as they would allow me to derive use case for a device (as has been previously discussed in the Network to Code slack).

The hard part with allowing all of those is defining the hierarchy of precedence. To that end, I think the solution is allow it to be configurable rather than hard set. So perhaps have a default hierarchy but allow the user to modify it if the default presents conflicts for their use case (I assume it would, but maybe I am over thinking this part).

jeremystretch commented 7 years ago

Not store them natively, but perhaps read them from a repository.

I was referring to configuration templates. Those won't be managed by NetBox directly.

jeremystretch commented 7 years ago

I'm trying to decide how best to implement this in the API.

Option A: Always include config context with the DeviceSerializer (including when listing multiple devices). This seems terribly inefficient.

Option B: Leave DeviceSerializer alone and create a detail route to retrieve the config context for a particular device, e.g. GET /api/dcim/devices/<pk>/config-context/. This would require two API requests when rendering a config: one for the device attributes and one for the context.

Option C: Implement a request parameter like ?include_context=1, which when passed would add the config_context field to DeviceSerializer. (If omitted, the field would not be present.) I'm not sure if we want to set a precedence for adding/removing fields dynamically. I'd also have to give some more thought to structuring the queryset, since we have to add prefetch_related to pull in the relevant ConfigContexts, which should be avoided if we're not going to use it.

Open to other suggestions.

lampwins commented 7 years ago

I would not do option A. I only want the config context in specific situations, not all.

I would like to see a normal set of endpoints for ConfigContext, so /api/dcim/config-context/ and so forth. Then include serializers for the ConfigContext pk on all objects that accept contexts. I like this because it allows me to retrieve only certain contexts, if that is all I need, i.e. just site context.

This plays into my earlier comments on dynamic hierarchy. In this model the dev/api user gets full control in how they interpret the hierarchy. Obviously this means if they want to build a full config, they may need a series of api calls all at once (is that really a bad thing?) to get all the contexts for a config. To that end, I also like options B which would implement the "default hierarchy" I proposed.

Just my thoughts.

bdlamprecht commented 7 years ago

I agree that Option A is definitely a no-go for me.
However, I'm torn between Option B and Option C. Let me explain...

Not necessarily in my scenario I detailed above, but I can foresee some circumstances where not all ConfigContext information that is defined for either a region, site, or device would be needed for each API call (i.e. NTP servers, DNS servers, SNMP servers, TACACS servers, and any other key-value defined by the user). That would be an overload.

Now that I think about it, perhaps a mixture between both Option B and Option C would be ideal if the API could be structured in a way where you could do something along the lines of /api/dcim/devices/<pk>/include_context=dns,ntp,snmp which would return only the values stored in the keys dns and ntp and snmp but not tacacs (or vlan as mentioned in #1367). That would allow the user ultimate control over what they want it to do.

As @jeremystretch mentioned, that WOULD set a "precedence for adding/removing fields dynamically". However, I believe that it would add value and it would be up to the end-user to implement it correctly and understand the pros and cons to structure their data correctly. Some documentation on the "whys" would probably be required :wink: and I'd be willing to help write it once I understand the structure that is implemented.

Again, these thoughts are coming from a newbie to the developer world so this might be fairly complex to do or not even possible at all, so take my comments with a "grain-of-salt".

cimnine commented 7 years ago

Now that I think about it, perhaps a mixture between both Option B and Option C would be ideal if the API could be structured in a way where you could do something along the lines of /api/dcim/devices//include_context=dns,ntp,snmp which would return only the values stored in the keys dns and ntp and snmp but not tacacs (or vlan as mentioned in #1367). That would allow the user ultimate control over what they want it to do.

I would vote against this, as it makes the api a lot harder to query. It seems rather simple to me to only use the values you need on the client side. A system such as the proposed would only make sense to me when one would add a lot (like thousands) of such context fields, because then there would be significant overhead in fetching the values from the database and also in transferring that data to the client.

Option C is my favourite anyway. It is simple enough to implement in an API client. It could even be added to the query for many devices, i.e. /api/dcim/devices?include_context=1. This could be useful when trying to render a file consisting of the information of many devices.

bdlamprecht commented 7 years ago

While I'm not opposed to Option C, I don't understand the following statement:

I would vote against this, as it makes the api a lot harder to query.

In the example I used above you could always still query the API via /api/dcim/<pk> to just get the basic details already provided, only by adding ?include_context=<item> would you get the additional requested data (you could also have a special keyword all for ?include_context= that returns everything).

However, if that is too complex to implement on the server-side, I'd be okay having Option C as it follows the core guidelines of NetBox:

When given a choice between a relatively simple 80% solution and a much more complex complete solution, the former will typically be favored. This ensures a lean codebase with a low learning curve.

Again, another disclaimer, this is coming from a recent transfer from the networking world to the developer world (DevOps) so I apologize in advance for any ignorance that I'm showing :smiley: .

jeremystretch commented 7 years ago

I think I prefer option B. It might help to better explain the way I see it working. Let's say you've defined three ConfigContext objects:

Global

dns1: 8.8.8.8
dns2: 8.8.4.4
syslog1: 10.0.3.21
syslog2: 10.0.3.22

Site A

dns1: 192.168.0.10
dns2: 192.168.0.11
snmp: OMGnetbox#1

Edge Router

allow_ssh:
  - 192.0.2.1
  - 192.0.2.17

You would retrieve these ConfigContexts via an API endpoint at /api/configs/config-contexts/. The same endpoint would also be used to create, update, and delete ConfigContexts.

Now, rendering the config context for a given device is different. If we want to get the rendered context for a device (e.g. an edge router in site A), we'd request /api/dcim/devices/<pk>/config-context/, which would return:

{
    "allow_ssh": ["192.0.2.1", "192.0.2.17"],
    "dns1": "192.168.0.10",
    "dns2": "192.168.0.11",
    "snmp": "OMGnetbox#1",
    "syslog1": 10.0.3.21",
    "syslog2": 10.0.3.22"
}

Notice that the DNS values from the global ConfigContext have been replaced by the ConfigContext for site A. It's important to point out that this request does not return a real ConfigContext object, but rather a dictionary compiled from all ConfigContexts applicable to this device.

bdlamprecht commented 7 years ago

Yeah, I see where you are coming from. This statement is going to show my inexperience, but in all reality, the chance of having too many ConfigContext objects to make it have any noticeable affect on performance is really quite low, so I'd like to retract my previous statements above.

The way you explained the rendering above makes sense to me as well. As you mentioned previously, this would allow the current API to continue to function as it has while also allowing an addtional API call to /config-context/ to gather additional data if and when it is needed (i.e. not all the time).

So although my vote doesn't mean much since I'm not the one developing it, I'm onboard for Option B.

lampwins commented 7 years ago

@jeremystretch I agree with this implementation. It is important to me to always maintain ConfigContext as its own entity in the API. This allows me to derive my own hierarchy for rendering the config if for whatever reason I do not agree with the hierarchy implemented at /api/dcim/devices/<pk>/config-context/ or only need a partial rendering.

cimnine commented 7 years ago

I don't follow the argument for option B completely when it comes to /api/dcim/device/<PK>?include_context=1 vs /api/dcim/device/<PK>/config-context.

We also define e.g. sites in /api/dcim/sites/, but they're included in /api/dcim/device/<PK> and I don't have to query /api/dcim/device/<PK>/site to get the device's site information.

I understand, that including the config-context by default is not desirable, because it causes overhead to calculate it's values, but that's why Option C suggests the '?include_context=1' flag.

Myself, I'd rather do one call to the API, than two. At times it's hard to correlate two calls. Also, the second call could fail due to some error condition (e.g. network) that I will have to take care of.

Maybe I could share here one use-case we're investigating and where this feature would become very useful to us: Creating our 'static assignments' dhcp configuration file. We want to replace our dhcp (and bootp) config file generator to fetch all information from Netbox. It would therefore be very convenient to just query /api/dcim/devices?include_context=1&some=filters, rather than to make two calls or even two calls per device. Besides the obvious performance penalty for having to make n requests, there'd be also the overhead on the consuming side of the api to correlate the (at least) two calls.

For our solution, we'd also leverage the hierarchical values, e.g.:

Global:

bootp_image: default.img

Region 1:

dns:
  - 192.168.1.1
  - 192.168.1.2

Region 2:

dns:
  - 192.168.2.1
  - 192.168.2.2

Device 2 (overwrite default value):

bootp_image: special.img
cimnine commented 7 years ago

Notice that the DNS values from the global ConfigContext have been replaced by the ConfigContext for site A. It's important to point out that this request does not return a real ConfigContext object, but rather a dictionary compiled from all ConfigContexts applicable to this device.

I think I now get what you would like to express here: /api/dcim/devices/<pk>/config-context are aggregated values over multiple levels and therefore deserve their own endpoint, isn't it?

But I still think, that from an API-user's perspective, it'd be favourable to get all required information about an entity in one call to the api, rather than two. (Especially, if I would have to request information on more than one entity, as I have made a case for above.)

jeremystretch commented 7 years ago

I think I now get what you would like to express here: /api/dcim/devices//config-context are aggregated values over multiple levels and therefore deserve their own endpoint, isn't it?

Yep, exactly.

But I still think, that from an API-user's perspective, it'd be favourable to get all required information about an entity in one call to the api, rather than two.

I agree that it would be ideal, but consider that multiple requests are needed anyway to retrieve interfaces, IP addresses, VLANs, etc. I'm not sure whether it would be practical (or efficient) to try and return all of that in a single response.

cimnine commented 7 years ago

that multiple requests are needed anyway

That would only be true for that exact case I made above. One might only need the information belonging to a device, e.g. to populate a cloud-init user-data template.

Another argument I'd like to make in favour of ?include_context=1 is that this can be added to any api level, e.g. to /api/dcim/devices/<pk> and to /api/dcim/devices.

Whereas /api/dcim/devices/config-context could not work, because a device's PK is expected instead of config-context.

jeremystretch commented 7 years ago

Another argument I'd like to make in favour of ?include_context=1 is that this can be added to any api level

The queryset gets pretty unwieldy when you start pulling devices in bulk. We'd need to prefetch related ConfigContexts for all devices in the list. Since a ConfigContext can be related by region, site, device_role, tenant, and/or tenant_group (plus global contexts), this becomes very inefficient. Additionally, needing to render contexts for multiple devices in a single request seems like an uncommon use case.

cimnine commented 7 years ago

The queryset gets pretty unwieldy when you start pulling devices in bulk.

Sure, I agree. That's why it's never enabled by default.

this becomes very inefficient.

It's all the same inefficient when I would query each device one-by-one.

Nevertheless, I was just saying that I would prefer the ?include_context=1 variant and I think it is more 'future-proof'. Don't let that stop you from choosing the other option though.

sdktr commented 7 years ago

Does anyone see it as a nice-to-have to be able to concatenate the content of different inheritence levels instead of getting the most specific value (to whatever preference agreed upon)?

For example: Global:

allowed_management_acl:
- 10.0.0.0/24

Site1:

allowed_management_acl:
- *global <-- syntax taken from [1]
- 10.1.1.0/24

When querying the device could render:

allowed_management_acl:
- 10.0.0.0/24
- 10.1.1.0/24

Since there doesn't seem to be a standard way of doing this with YAML, a syntax would have to be come up with including the 'fixup' code to flatten the aggregated content?

Example taken from [1] https://stackoverflow.com/questions/9254178/is-there-yaml-syntax-for-sharing-part-of-a-list-or-map

jeremystretch commented 7 years ago

@sdktr The Stack Overflow post you linked mentions the YAML merge key. Seems reasonable, but I'd have to see how involved supporting its logic would be.

puck commented 7 years ago

Interesting feature. I can see this being very useful.

With regard to allowing people to store the data how they want in https://github.com/digitalocean/netbox/issues/1349#issuecomment-316194935 if you used the PosgreSQL JSON field type, then you get free form JSON, and could convert to YAML (or whatever) based on what is requested from the API. This would also make it much easier to display, edit & validate this content in the UI in a nice manner, rather than just a giant text field.

PostgreSQL also has nice features now for querying for data inside the JSONB data type fields, so you'd be able to support queries for things in this data in the future (or for people using SQL as their API).

More info on how PG handles JSON is here: https://www.postgresql.org/docs/9.6/static/datatype-json.html

bdlamprecht commented 7 years ago

Any updates on this request?

I seem to recall that there was a alpha version working in one of your branches. Just looking to see if I could get a timeline for when this could be implemented.

tobymccann commented 7 years ago

I think this would be an extremely valuable feature that would further Netbox as the SoT. As more organizations automate their environments, data becomes essential and defining a data model for managing device configurations is required.

In our case, we are wanting to manage thousands of devices with Ansible and Ansible Tower/AWX. We have chosen to align with OpenConfig to build our own data model. We would like to store and associate the data in a hierarchy so we can align it with Ansible's group and host variable structure and minimize data duplication (DRY).

Option B that Jeremy mentions would work well for this use case. It would be very interesting to see if YANG or JSON data models (like OpenConfig) could be broken apart and represented within Netbox directly to accomplish the task of loading your organizations data. It would be nice to see a default hierarchy of precedence that could be customized based on what the customer requires:

Tenant Region Site Room RackGroup Rack Pod or Cluster DeviceType DeviceRole Device

Making this data accessible via the API would we critical. With it, customers can create their own dynamic inventories and complete device data models to use with their config management tool of choice, e.g. Ansible, Salt, etc.

Configuration templates should not live in Netbox as these would be managed by the customer in git repositories. Another feature possible with Netbox however would be a front end for managing these templates and integrating with git to do so.

bdlamprecht commented 7 years ago

@jeremystretch I know you are very busy, but is there anything I can do to help expedite this request?

As I stated previously, I don't want to be "annoying annoying or in any way demanding", however, the "proof-of-concept" that I've been working on for a few months is highly dependent on this functionality. To that statement, you mentioned you wanted to "evaluate some other approaches before a commit to the solution" is released.

What do you think would be of the most help to you?

  1. Testing the POC branch that you had this working with.
  2. Coding the "other approaches" (would need to know what your current thoughts are).
  3. Financial incentives (would not able to offer much as it would come from personal funds).
  4. Something else entirely that I didn't enumerate above.

Really interested in getting this working within the next few months. Again, thanks for your work on this excellent project and especially for releasing it as open-source.

jeremystretch commented 6 years ago

I've been thinking about this more lately. I'm hung up on the hierarchical approach because it doesn't allow the assignment of values along two dimensions.

Simple Hierarchy

For example, say we implemented a hierarchy like this, wherein data defined at each layer supercedes any data with the same key defined at the layer above it:

Contextual data could be defined directly on each object as a JSONB field.

But what happens if you need to assign, say, radius_server based on both site and role? Four different sites and three different roles would result in 12 unique values for radius_server (A through L).

       | Role 1 | Role 2 | Role 3 |
-----------------------------------
Site 1 |   A    |   B    |   C    |
Site 2 |   D    |   E    |   F    |
Site 3 |   G    |   H    |   I    |
Site 4 |   J    |   K    |   L    |

We can define a unique value for each role and site, but one must take precedence over the other. In this example, role would win because it is below site in the hierarchy.

We can work around this by defining the value for the most common of the three roles at the site level, and then defining values for each of the other two roles directly on the individual devices, but this is redundant and error-prone. And it becomes even less efficient if another dimension like platform or device type is introduced.

Alternative Approach

Instead of defining context data directly on each object as a field, we can introduce a ConfigContext model which has ManyToManyFields typing it to sites, roles, etc., as well as a JSONB field to store the data. In our example above with four sites and three roles, we would create 12 ConfigContexts:

  1. ConfigContext(site='Site 1', role='Role 1')
  2. ConfigContext(site='Site 1', role='Role 2')
  3. ConfigContext(site='Site 1', role='Role 3')
  4. ConfigContext(site='Site 2', role='Role 1')
  5. ConfigContext(site='Site 2', role='Role 2')
  6. ConfigContext(site='Site 2', role='Role 3')
  7. And so on...

To retrieve all ConfigContexts matching a particular device, we would query like this:

ConfigContext.objects.filter(
    site__in=[device.site, None],
    role__in=[device.role, None]
)

This will return all ConfigContexts assigned to the device's site and/or role (as well as any which are not assigned to any particular site or role). (Of course, we can match on other attributes as well, not just site and role.) This approach should be fine for individual devices, but doesn't scale well when retrieving multiple devices. This is especially true if we decide to filter across multiple tables, like region=device.site.region.

One interesting but somewhat scary approach would be to render the config context for every device each time a ConfigContext is modified, and store the rendered context locally on each device. I'm not sure yet how involved this process would be, but it's worth considering that context is likely to change far less often that it will be read. And pre-computing the rendered context for each device would do wonders for performance, particularly in retrieving many devices via the API.

I'm curious to hear any thoughts others have before I go any further with this.

jeremystretch commented 6 years ago

Following up on my comment above after some discussion on NetworkToCode. I envision the ConfigContext model looking something like this:

class ConfigContext(models.Model):
    name = models.CharField(max_length=100, unique=True)
    weight = models.PositiveSmallIntegerField(default=1000)
    site = models.ManyToManyField(to='dcim.Site', blank=True)
    role = models.ManyToManyField(to='dcim.DeviceRole', blank=True)
    platform = models.ManyToManyField(to='dcim.Platform', blank=True)
    tenant = models.ManyToManyField(to='tenancy.Tenant', blank=True)
    data = JSONField()

    class Meta:
        ordering = ['weight', 'name']

We may decide to add other fields like region; this is just an example for the sake of discussion.

Each ConfigContext is assigned its relevant sites, roles, etc. The data field stores configuration context in JSON format. A weight can be set to prefer one ConfigContext or the other where overlapping data exists (with creation time serving as a tie breaker).

Additionally, we'd add a rendered_context JSON field on the Device model:

class Device(CreatedUpdatedModel, CustomFieldModel):
    ...
    rendered_context = JSONField(blank=True)

Adding, modifying, or deleting a ConfigContext triggers rendering of configuration data for every device. (If we adopt a message queue as has been suggested for #81, this operation can be handled in the background.) This would involve:

1. Retrieve all unique sets of devices

Every device which has the same site, role, tenant, and platform (in this example) will receive the same rendered configuration data. Thus, we only need to render the context for each unique set of devices, rather than for each device independently, using the following query:

device_sets = Device.objects.order_by(
    'site_id', 'device_role_id', 'tenant_id', 'platform_id'
).distinct(
    'site', 'device_role', 'tenant', 'platform'
).values(
    'site', 'device_role', 'tenant', 'platform'
)

2. Render the context data for each set

For each set of devices, we query all relevant ConfigContexts, ordering them by weight and time of creation. A dictionary is created and updated with the contents of each ConfigContext in sequence.

contexts = ConfigContext.objects.filter(
    Q(site=device_set['site']) | Q(site=None),
    Q(role=device_set['device_role']) | Q(role=None),
    Q(tenant=device_set['tenant']) | Q(tenant=None),
    Q(platform=device_set['platform']) | Q(platform=None),
).order_by('weight', 'name')

data = {}
for context in contexts:
    data.update(context.data)

3. Update all devices in the set at once

Using bulk_update() allows us to hit every device in a set with one query, while avoiding running save() on each object (and updating its modification time).

Device.objects.filter(**device_set).update(rendered_context=data)

Obviously, lots of testing is needed, but it seems plausible. We do potentially end up with quite a bit of redundant data, but this denormalization buys us the ability to return all config context for a device directly from the device table. This means we can pull down the config context for many devices at once with a negligible performance penalty.

sdktr commented 6 years ago

Just a thought: should this 'configcontext' and 'custom fields' functionality be merged? So instead of having a custom field per model, we could use the model as proposed above, but instead of having one context (the configcontext) we could manage them as custom fields with config being just one of them.

To mimic the 'custom fields' functionality as it is today it would just be a one-to-one mapping to the relevant object that stores it (like the simple configcontext proposed before jeremy pitched the configcontext as a seperate model)

An additional benefit of upgrading the 'custom field per table' to the 'context is a seperate model' is that migrations of the database are simplified. There would be just a seperate table/model with all the custom stuff in there instead of each installation having their own tables with custom columns.

Quoting @jeremystretch model for this:

class Context(models.Model): name = models.CharField(max_length=100, unique=True) contexttype <--- reference to user-maintainable list of contexttypes, 'config' being an example weight = models.PositiveSmallIntegerField(default=1000) site = models.ManyToManyField(to='dcim.Site', blank=True) role = models.ManyToManyField(to='dcim.DeviceRole', blank=True) platform = models.ManyToManyField(to='dcim.Platform', blank=True) tenant = models.ManyToManyField(to='tenancy.Tenant', blank=True) data = JSONField() <--- json could hold all the datatypes that 'custom fields' support today

jeremystretch commented 6 years ago

Just a thought: should this 'configcontext' and 'custom fields' functionality be merged?

Each serves a different purpose and complement one another. A custom field allows a user to specify (even require) a particular value per object. They can be assigned to most primary objects in NetBox. A custom field works well if you need to track values which are generally unique to each object.

A config context is intended to provide additional, optional, free-form data to a set of devices. This would better accommodate data that is the same for each device that meets an arbitrary classification (e.g. all access switches at site D). Also note that contextual data by its nature cannot be declared as mandatory.

sdktr commented 6 years ago

I understand that both might have their own primary usage, especially the way they are positioned today.

The reason I came up with this is because of how you proposed to make 'configcontext' it's own seperate model. If I understood your intentions correctly, you did this to have a more flexible combination of data end up at the device level (instead of just inheritance in whatever order).

The way you set up your model is that it could have many-to-many relations to one or more objects. That same datamodel seems to provide a good (imho: better) way of storing the simpler 'custom fields' that have a one-to-one relation to an object today.

The 'data contract' for these 'custom fields 2.0' can be stored in a seperate table. This way mandatory data can be checked for at the object level (or whatever the data construct dictates).


Besides 'configuration data' I have other data that won't fit in the regular fields of Netbox objects today. My initial thought would be to store json in 'custom fields' (if possible?) and use basic inheritence to end up with a complete json object at the requested object. Then you proposed 'configcontext as a seperate model' and I thought that would be a great addition to 'custom fields' as well to fascilitate for the more complex composition of 'custom fields' data from multiple Netbox objects. Adding the two together made me think: we should store the 'custom fields' and 'configcontext' as a seperate model that can fasciltiate both use cases and more.

jeremystretch commented 6 years ago

Work on this feature has been merged into develop-2.4.