drakkar-lig / walt-python-packages

Home of walt-node, walt-server, walt-client and walt-common python packages.
https://walt-project.liglab.fr
BSD 3-Clause "New" or "Revised" License
5 stars 3 forks source link

Users should be able to configure virtual nodes more precisely #19

Closed eduble closed 4 years ago

eduble commented 5 years ago

Some images require more RAM than default amount of 512M to boot. We could have a command line such as

$ walt node config my-vnode ram 1G

Of course, this would return an error if the node is not virtual.

For more coherency, we could remove walt node netsetup [LAN/NAT] command and use this instead:

$ walt node config my-vnode netsetup NAT

And we could specify several settings in one call:

$ walt node config my-vnode netsetup NAT ram 1G

As we see in these examples, some settings would require the node to be virtual, others not.

When we have other models of virtual nodes implemented (let's say a model arm64-virt), we could also change this using:

$ walt node config my-vnode model arm64-virt

And revert to our current model of virtual node :

$ walt node config my-vnode model pc-x86-64

Typing just

$ walt node config my-vnode

would display the current state of parameters.

For more coherency again, we would rename walt device admin <my-device> (which allows to change switches configuration, such as PoE and SNMP) to walt device config <my-device>, and change command line handling for a similar user interface:

Typing walt device config <a-node> would act the same as walt node config <a-node> (currently walt device admin <my-device> requires the specified device to be a switch or an "unknown" device).

The verbose explanations we currently have when typing walt device admin <my-device> would be turned to a doc page available using walt help show device-config and a tip about this would be printed in commands output.

audeoudh commented 5 years ago

Which part will be responsible of the interpretation of the argument part (e.g. netsetup NAT ram 1G) ?

Make the client responsible of that is not a good idea: the API should be updated whenever we add a new configuration, some configuration would be absurd for some node/device (e.g. walt node config rpi-3b ram 16G won't work), etc.

Make the server responsible of that seems better, as the server knows many things than the client. However, this implies creating a new API handler.

eduble commented 5 years ago

I agree, as usual we try to keep the client simple. It could just:

For walt device config <device-set> variant, I realise that for now we always manage devices one at a time. Thus we have no function to verify the <device-set> yet. That would be useful though, because repeating the existing walt device admin command with the same settings for many devices is quite anoying. We could have a server API function parse_set_of_devices, similar to the one we have for nodes (parse_set_of_nodes), except that it would accept any type of device. That could be a preliminary commit, whatever we do next.

Back to your comment: the rest of the verifications should be done by the server API function, as you suggested. For now we had apply_switch_conf (see server/walt/server/threads/main/api/cs.py), called by walt device admin (see client/walt/client/device/admin.py). This should be removed and replaced by our new generic API function that we could call set_device_config for example.

audeoudh commented 5 years ago

verify the syntax: walt node config <node-set> <setting-name> <setting-value> [...], thus have an even number of arguments after the <node-set>

As the client doesn't know what a <setting-name> and a <setting value> is, it can only verify that the command has this form (regex-style): walt node config <node-set> [<word> <word>]+.

This also implies that the settings are always given with a name and a value as two separated words. We cannot imagine complex settings, where there is many values: e.g. radio-range <min> <max>. Should we consider such situations?

Indeed, I would prefer do all verification on the server side. The server itself have to do the “even number of arguments” check. So we double-check that, for no special improvement (the server can still write on the client's stderr if an error occurs). Thus, the new functions set_node/device_config will be called with two arguments : the affected node/device set, and all the settings unparsed as a long string.

audeoudh commented 5 years ago

Partially fixed by 8738b60 and commits around it.