openwisp-controller icon indicating copy to clipboard operation
openwisp-controller copied to clipboard

[controller] Device name changed by a template is not updated in the device model instance

Open okraits opened this issue 5 years ago • 7 comments

We have a template to change the device hostname based on the location where it's in use:

{
    "system": [
        {
            "config_name": "system",
            "config_value": "system",
            "hostname": "{{ LOCATION_NAME }}"
        }
    ]
}

There's one issue with this, though:

The device name in the device model instance is not updated because:

  • it is not sent to the controller by openwisp-config, for example by update_info: https://github.com/openwisp/openwisp-config/blob/master/openwisp-config/files/openwisp.agent#L353
  • it's not included in UPDATABLE_FIELDS: https://github.com/openwisp/openwisp-controller/blob/64598a775d641d706da4ac22cf5ffbd7a3889792/openwisp_controller/config/controller/views.py#L129

I think it's a valid usecase to update the device hostname with a template and thus have this change reflected in the device model instance.

@nemesisdesign What do you think?

okraits avatar Sep 24 '20 13:09 okraits

@okraits the flow is unusual, you set the name of the device on openwisp which is then modified by the device itself after it downloads the configuration specified in a template on OpenWISP. It's a bit strange.

Where is LOCATION_NAME coming from? Is surely coming from somewhere isn't? Can't you simply script the name changing in another way that is internal to your application?

nemesifier avatar Oct 01 '20 03:10 nemesifier

@okraits the flow is unusual, you set the name of the device on openwisp which is then modified by the device itself after it downloads the configuration specified in a template on OpenWISP. It's a bit strange.

The flow only uses core functionality of OpenWISP, I wouldn't call that unusual:

  • The device registers itself at the controller
  • the uci configuration on the device (here the hostname) is changed by a template which uses a context variable

IMO the device info on the controller should be updated if any device information present on the controller is outdated, be it the hostname, the firmware version or one of the IPs. Device info on the controller is of no use if it is outdated. The hostname can change if a device is migrated from one location to another one.

Where is LOCATION_NAME coming from? Is surely coming from somewhere isn't?

It's a context variable in the device config on the controller.

Can't you simply script the name changing in another way that is internal to your application?

Why would I want to do that? AFAIK configuration management should be the job of OpenWISP.

okraits avatar Oct 01 '20 06:10 okraits

@nemesisdesign How can we solve this? I think the implementation wouldn't take much effort - the client needs to send the hostname together with the other infos and the controller needs to take that data and update the device object.

okraits avatar Oct 05 '20 18:10 okraits

@nemesisdesign I can't wait because I need this to be fixed, so I implemented a fix.

okraits avatar Oct 13 '20 09:10 okraits

@okraits at the moment I don't feel confident in proceeding with this until I have analyzed it well, the reason is the same as for the feature that allows openwisp-config to collect the full configuration of the controller: many flows change and the impact of these kind of changes can be assessed only with usage, so I suggest you to use your own modifications for now, we can keep this open and collect more feedback.

Maybe the change will not impact the system at all, but I have to dedicate time to testing it and at the moment I am very busy on other fronts which I announced before so I want to focus on those now.

Thank you for your patience.

nemesifier avatar Oct 13 '20 14:10 nemesifier

@nemesisdesign Did you have a chance to test or think about this?

okraits avatar Oct 15 '21 07:10 okraits

@nemesisdesign Did you have a chance to test or think about this?

I am still focused on delivering the next major release of OpenWISP.

nemesifier avatar Oct 15 '21 16:10 nemesifier