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

[backward-conversion] Recognize templates #100

Open
nemesifier opened this issue Sep 8, 2017 · 7 comments · May be fixed by #152
Open

[backward-conversion] Recognize templates #100

nemesifier opened this issue Sep 8, 2017 · 7 comments · May be fixed by #152

Comments

@nemesifier
Copy link
Member

nemesifier commented Sep 8, 2017

There's an issue with the backward conversion which I did not consider in the beginning.

Imagine the situation in which the configuration of a device is converted to NetJSON and sent to an openwisp2 instance.

Case 1) First registration

The system gets a NetJSON containing the full configuration, which will be inserted in the config field, BUT will override any configuration present in templates that are either set as default or are set in a second step by a user.

Case 2) Updates

Imagine an already registered OpenWRT device. A local user logs in via SSH or luci and updates some configurations. Some magic trick then detects the configuration changes and sends the updated NetJSON full configuration to the server.

Without an appropriate mechanism, the configuration present in templates will be duplicated in the main config field

In short

The library needs to understand how to handle both cases, in the first, if same configuration keys are present both in the local conf and in templates, the templates win and the local conf values are discarded.

In the second case, local conf values are discarded only if they are equal to the same values present in the values of templates.

Challenges

I'm noting down challenges regarding this task.

Recognize variables

When a configuration is received, if the content of a configuration option matches the content of a variable in a template, it should be recognized as the template, not as a custom local configuration that needs to be stored.

@NoumbissiValere
Copy link
Contributor

NoumbissiValere commented Mar 5, 2019

@nemesisdesign please can you give me directives on how to reproduce this issue on my local repo? so that i can better visualize the issue in other to easily get a fix for it. thanks

@atb00ker
Copy link
Member

atb00ker commented Apr 4, 2020

Okay, so just to confirm if I have understood it correctly.
I need to add a flag say --new-registered that will be passed when a new device is registered. This will make sure that if the key exist in template, then it's used instead of the one received from device.

However, if --new-registered is not passed the behavior does not change (because currently config is already given precedence over templates.)

Am I correct in my understanding of the issue? 😄

@nemesifier
Copy link
Member Author

Kind of a flag, I'd think it more as a function argument.

I am not sure you understood the aim of this change.

We need to make able OpenWISP to capture the configuration from the devices, so we'll send the full config from the devices to OpenWISP Controller which in turn will call netjsonconfig to understand what to do.

Let's start with the more common case, normal updates.
In normal updates, we just have to be sure that the part of the config coming from the device which is equal what is present in templates doesn't get duplicated in the config of the device.
We build a netjsonconfig backend instance passing config and templates, then we call a method, this method will update the instance so that the config sent by the device is included in the object, this config being separated in config and templates so that the part of the config that is the same to the templates is not duplicated in config. Then we will save this in OpenWISP Controller.

The other case is when a device registers. I'm not 100% sure about the solution proposed in this old ticket. I would discuss that further before proceeding.

BTW, until this point, have you understood something?

@atb00ker
Copy link
Member

atb00ker commented Apr 12, 2020

BTW, until this point, have you understood something?

I think I am able to understand the problem.
Now, About how I have implemented the part sending device config to server is called after openwisp-controller configurations are applied to the device.
Hence, the case 1 mentioned in the issue will not happen, instead, only case 2 happens.

As of case 2, please correct me if I have misunderstood anything about what we want:

  1. Receive full config from device
  2. Compare the config and template saved in the controller with received config
  3. Find keys with different values and store these values in config of the device.

** Note: I am assuming that we convert the configs we received from device to NetJson DeviceConfiguration json and we compare these keys to the keys of NetJson DeviceConfiguration stored in the controller.

So, bottom line, we need a function to compare two different DeviceConfigurations and return keys that have different value and store those values.

Is my understanding correct?

@nemesifier
Copy link
Member Author

@atb00ker: I'll try to explain with some pseudocode.

We need to be able to do the following:

backend_instance = OpenWrt(
    native=tar_gz_fileobj,
    config=device.config,
    templates=[management_vpn, snmpd]
)

management_vpn and snmpd represent default templates that are stored in OpenWISP.

The result should be a backend_instance which will get:

  • all the configuration that is passed to native (the config received from the device), and is also available in templates, will not be present in backend_instance.config
  • the rest will be put in backend_instance.config

At that point in openwisp-controller we'll just have to do:

device.config.config = backend_instance.config
device.config.full_clean()
device.config.save()

Is it clearer now?

atb00ker added a commit to atb00ker/netjsonconfig that referenced this issue Apr 19, 2020
atb00ker added a commit to atb00ker/netjsonconfig that referenced this issue Apr 20, 2020
@atb00ker atb00ker linked a pull request Apr 20, 2020 that will close this issue
3 tasks
atb00ker added a commit to atb00ker/netjsonconfig that referenced this issue Apr 20, 2020
atb00ker added a commit to atb00ker/netjsonconfig that referenced this issue Apr 20, 2020
atb00ker added a commit to atb00ker/netjsonconfig that referenced this issue Jul 16, 2020
@feckert
Copy link

feckert commented Apr 9, 2021

@atb00ker
So far I have been dealing with the openwisp agent in the openwrt router.
Now I wanted to try the new approach of having the router push the configuration to the server.
I have also adapted the server with this changes in this pullrequest.
So far so good.

But I don't think that's all, is it?

I need more input on what I need to do now
I don't know much about the server.
I need more input from your side.

@atb00ker
Copy link
Member

atb00ker commented Apr 9, 2021

When you say you have taken changes from pull request, I assume you are talking about openwisp/openwisp-config#107

And yes, that's not enough, after this we would need to complete this work in netjsonconfig #152

When that is complete, the next step is taking that release of netjsonconfig to openwisp-controller (basically update the dependency) and create endpoints to access it from API.
Then we only need to release it in ansible-openwisp2 to complete it.

The blocker from my end & Federico's end is bandwidth, we are not able to get to it, so if you have experience with Django, please test it and see if there are any bugs / problems, if yes, please help us solve them.

That's a high level idea of what remains in this. (I think it is a lot of work considering you don't have knowledge about the server.)
However, if you are willing to take this task, then I'll try to help you as much as I can! 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: To do (Device management)
Development

Successfully merging a pull request may close this issue.

4 participants