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

attribute "last_updated" not updated when primary IP gets removed from device/virtual machine #5419

Closed
bb-Ricardo opened this issue Dec 4, 2020 · 5 comments
Assignees
Labels
status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application

Comments

@bb-Ricardo
Copy link

bb-Ricardo commented Dec 4, 2020

Environment

  • Python version: 3.6.8
  • NetBox version: 3.9.10

Steps to Reproduce (assuming we have a VM with a assigned primary ip)

  1. query API data of said virtual machine
  2. delete assigned primary IP
  3. query API data of said virtual machine again

Expected Behavior

The last_updated attribute is set to a current time/date value

Observed Behavior

The last_updated attribute is still the same as from before the deletion of the primary IP

Note

Adding an Primary IP updates the last_updated attribute correctly

Example

curl -X GET https://example.com/api/virtualization/virtual-machines/128/?exclude=config_context
{
    "id": 128,
    "url": "https://example.com/api/virtualization/virtual-machines/128/",
    "name": "git",
    "status": {
        "value": "active",
        "label": "Active"
    },
    "site": {
        "id": 1,
        "url": "https://example.com/api/dcim/sites/1/",
        "name": "New York",
        "slug": "new-york"
    },
    "cluster": {
        "id": 4,
        "url": "https://example.com/api/virtualization/clusters/4/",
        "name": "East"
    },
    "role": {
        "id": 8,
        "url": "https://example.com/api/dcim/device-roles/8/",
        "name": "Server",
        "slug": "server"
    },
    "tenant": null,
    "platform": {
        "id": 13,
        "url": "https://example.com/api/dcim/platforms/13/",
        "name": "CentOS 8 (64-bit)",
        "slug": "centos-8-64-bit"
    },
    "primary_ip": {
        "id": 871,
        "url": "https://example.com/api/ipam/ip-addresses/871/",
        "family": 4,
        "address": "10.1.2.3/24"
    },
    "primary_ip4": {
        "id": 871,
        "url": "https://example.com/api/ipam/ip-addresses/871/",
        "family": 4,
        "address": "10.1.2.3/24"
    },
    "primary_ip6": null,
    "vcpus": 4,
    "memory": 8192,
    "disk": 74,
    "comments": "",
    "local_context_data": null,
    "tags": [],
    "custom_fields": {
        "tos_uuid": null
    },
    "created": "2020-11-30",
    "last_updated": "2020-11-30T15:06:11.441024Z"
}

curl -X DELETE "https://example.com/api/ipam/ip-addresses/871/"

curl -X GET https://example.com/api/virtualization/virtual-machines/128/?exclude=config_context
{
    "id": 128,
    "url": "https://example.com/api/virtualization/virtual-machines/128/",
    "name": "git",
    "status": {
        "value": "active",
        "label": "Active"
    },
    "site": {
        "id": 1,
        "url": "https://example.com/api/dcim/sites/1/",
        "name": "New York",
        "slug": "new-york"
    },
    "cluster": {
        "id": 4,
        "url": "https://example.com/api/virtualization/clusters/4/",
        "name": "East"
    },
    "role": {
        "id": 8,
        "url": "https://example.com/api/dcim/device-roles/8/",
        "name": "Server",
        "slug": "server"
    },
    "tenant": null,
    "platform": {
        "id": 13,
        "url": "https://example.com/api/dcim/platforms/13/",
        "name": "CentOS 8 (64-bit)",
        "slug": "centos-8-64-bit"
    },
    "primary_ip": null,
    "primary_ip4": null,
    "primary_ip6": null,
    "vcpus": 4,
    "memory": 8192,
    "disk": 74,
    "comments": "",
    "local_context_data": null,
    "tags": [],
    "custom_fields": {
        "tos_uuid": null
    },
    "created": "2020-11-30",
    "last_updated": "2020-11-30T15:06:11.441024Z"
}
@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application labels Dec 4, 2020
@jeremystretch
Copy link
Member

The problem here is that the disassociation of the deleted IP address is being handled at the database level: save() on the device/VM never gets called to update the primary_ip4 or primary_ip6 field.

@DanSheps
Copy link
Member

DanSheps commented Dec 9, 2020

We could update the device model from the IPAddress model, but... This would trigger a webhook or any other defined signals. Would we want this behaviour?

@bb-Ricardo
Copy link
Author

Would a database trigger be enough?

@jeremystretch
Copy link
Member

This would trigger a webhook or any other defined signals. Would we want this behaviour?

Strictly speaking, this behavior would be required. The parent device/VM is being modified via the deletion of an assigned primary IP, so that change needs to be communicated just as if the object was being modified directly.

Would a database trigger be enough?

It would be sufficient to update the field, but not to invoke the complete change logging process (creating a change record and triggering any webhooks).

I see two options:

  • Option A: Hook into the pre_delete signal for IPAddress and manually nullify the primary_ip* field of a parent object as needed.
  • Option B: Change the on_delete behavior for these fields from SET_NULL to PROTECT. This would force the user to remove the primary IP designation before allowing the IP address to be deleted.

IMO A is the preferred approach because it preserves the existing behavior, but I'm open to arguments in support of option B.

@sdktr
Copy link
Contributor

sdktr commented Jan 11, 2021

I opt for option A as well. Option B would be prefered if this method supported a confirmation flag. If a user action is confronted with a PROTECT such as this one, it would imo be user friendly to warn, but allow for an additional confirmation that allows for the delete anyway. If this would happen option A (signaling a change on the parent device/VM) would still need to be implemented?

@jeremystretch jeremystretch added status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation and removed status: accepted This issue has been accepted for implementation labels Mar 29, 2021
@jeremystretch jeremystretch self-assigned this Apr 13, 2021
@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation and removed status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation labels Apr 13, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application
Projects
None yet
Development

No branches or pull requests

4 participants