Page MenuHomeVyOS Platform

CLI node priority is not inversed on node deletion
Closed, WontfixPublicBUG

Description

VyOS uses hard-coded priorities for CLI nodes to ensure the system is configured in a specific order.
As most of you know priorities can be listed using /opt/vyatta/sbin/priority.pl

During debugging of T5428 I noticed that the VRF configuration script is executed prior to the interface script when removing things.

To test:

set interfaces ethernet eth2 vrf purple4
set interfaces ethernet eth2 address dhcp
set vrf name purple4 table 65000

Now remove the interface based VRF configuration and the VRF

[email protected]# delete vrf
[email protected]# delete interfaces ethernet eth2 vrf
[email protected]# commit
[ vrf ] <- Priority 299
Hello from vrf.py
[ interfaces ethernet eth2 ] <- Priority 318
Hello from interfaces-ethernet.py

I've first seen this in syslog as it looked weird to me seeing the vrf script getting executred first:

Aug 18 13:49:16 sudo[4427]:      cpo : TTY=pts/0 ; PWD=/home/cpo ; USER=root ; COMMAND=/usr/bin/sh -c ' /usr/libexec/vyos/conf_mode/vrf.py'
... noise ...
Aug 18 13:49:17 sudo[4458]:      cpo : TTY=pts/0 ; PWD=/home/cpo ; USER=root ; COMMAND=/usr/bin/sh -c ' VYOS_TAGNODE_VALUE=\'eth2\' /usr/libexec/vyos/conf_mode/interfaces-ethernet.py'

In my humble opinion, priorities should be executed ascending during set commands and descending (high to low) when deleting CLI nodes.

Details

Difficulty level
Unknown (require assessment)
Version
1.4-rolling, 1.3.3
Why the issue appeared?
Will be filled on close
Is it a breaking change?
Unspecified (possibly destroys the router)
Issue type
Unspecified (please specify)

Event Timeline

The short story:

Under the legacy commit algorithm, nodes are deleted in order of reverse priority if they are nodes with a defined owner/priority; sub-nodes of the node that have no explicit owner/priority are not, in fact added to the delete priority queue, but are processed in the set priority queue as a destructive set.

It becomes clear, after the fact, why this was an approach, given the legacy structure in terms of unionfs nodes and exec files.

However, the implementation also fails to recursively set priorities (assuming that could be well-defined), except on inversion error, hence the PrioNode instances are only created at the owner/priority level, which excludes their consideration for a priority queue.

It is not surprising then that the legacy commit algorithm was undergoing revision in code that unfortunately never made it to production.

Firstly, a simple example, in which the tag nodes have owner attributes:

set:

vyos@vyos# set low low-tag low0 node single-value val0

vyos@vyos# set high high-tag high0 node multiple-value val1

vyos@vyos# commit
[ low low-tag low0 ]
Calling low.py

[ high high-tag high0 ]
Calling high.py

delete:

vyos@vyos# delete low low-tag low0

vyos@vyos# delete high high-tag high0 

vyos@vyos# commit
[ high high-tag high0 ]
Calling high.py

[ low low-tag low0 ]
Calling low.py

set as above, then delete sub-nodes of nodes with attribute owner:

vyos@vyos# delete low low-tag low0 node single-value 

vyos@vyos# delete high high-tag high0 node multiple-value val1

vyos@vyos# commit
[ low low-tag low0 ]
Calling low.py

[ high high-tag high0 ]
Calling high.py

In the last case, no paths are added to the delete priority queue --- it is only a set operation, at the level of the owner attribute, processing the sub nodes defined in the 'working config'.
The same behavior follows from deleting the intermediary nodes 'node' rather than the leaf nodes.
On the other hand, consider a path with multiple owner attributes:

vyos@vyos# set service https api

vyos@vyos# commit
[ service https ]
Calling https.py

[ service https api ]
Calling http-api.py
Calling https.py

(The final https.py call is explicit by a config mode dependency.)

vyos@vyos# delete service https 

vyos@vyos# commit
[ service https api ]
Calling http-api.py

[ service https ]
Calling https.py

The fact is that this behavior is implicit in the legacy design; there nonetheless may be ways to address the matter without waiting for the full replacement of the commit algorithm.

jestabro closed this task as Wontfix.EditedSep 5 2023, 8:47 PM

A workaround was provided in T5428 for the specific issue.

As mentioned above, the problems with the legacy commit algorithm and handling of priorities are deeply rooted, and changes to the legacy code are not recommended. Instead the focus will be on its retirement sooner rather than later. This will be closed as 'Wontfix', meaning wontfix in the context of the legacy code.