Migration scripts use vyos.configtree which uses libvyosconfig so it's probably a bug there.

Attached are config.boot post-upgrade(migration) and config.boot pre-migration.
Notice:

• doubled hw-id lines
• missing opening and closing curly braces on lines 45, 75-77 (tag nodes)
• lines 8, 33, 35 are leaf nodes, those shouldn't have opening/closing curly braces after them
• some things are quoted, some are not

Not doing anything regarding the failed load and just rebooting has now hard-baked the eth0-eth2 names into config.boot without me doing anything. So something effectively decided to rename eth1-eth3 to eth0-eth2 and save it to config.boot.

https://github.com/vyos/vyos-smoketest/commit/f717f3f3ddb2afca78cf0ff3aab8457242d5f3d0

The problem was in to loop iterator when more then one address was added. This was introduced in T2635. The smoketest will be adjusted to cover this case so the issue won't reappear.

Here is the iptables-save output for Vyos-1.3-rolling-202006260117:

I checked the usage of netmap, but unfortunately I only found the equivalent configuration method of IPv4 on Wiki

Documentation commit here: GitHub fmertz/vyos-documentation/commits/system-display

This appears to be caused by the setting of service ssh listen-address; it appears the script generating the config is omitting the actual address. Removing a specific listening address is a temporary workaround.

Updated versions:

• libnetfilter-conntrack 1.0.8-1
• conntrack 1.4.6

nftables updated to 0.9.6 so the new nftables netmap feature can be used

Latest rolling has nftables 0.9.6 - closing this

I think this is a good idea. Maybe separate the protocols, http-client and ssh-client?

Or we can make protocol a multi node.

Since the user can specify both protocols inside, we'll probably have to duplicate the whole things if we detect that.

Maybe related to T2596

@jjakob that is in fact what we are doing: for stability, some of the changes are necessarily incremental --- these are not new abstractions, but rather, refinements of the existing ones, which allow the separation of new code from legacy. We are avoiding any patching of the old backend, simply replacing with the structures which will allow a switch to vyconf, while improving performance and extension of the new code.

Ok, let's switch to an empty banner.

I propose to provide alternatives is_ functions using the new XML code. I will provide a patch for review.

To be fair, I'm getting frustrated by the layers-upon-layers of new abstractions getting added on old code that doesn't work properly in the first place. I'd much prefer if we just started with a clean slate. I liked @thomas-mangin 's idea of replacing vyatta-cfg completely with our own code, either vyconf or his python daemon. I wouldn't waste time patching up the old backend, just make a decision in one place to replace it completely.

I think I ran into this today after upgrading from 1.3-rolling-202006110117 to 1.3-rolling-202006241940. My config had eth1-eth3 (as those were the default names created by a previous install of 1.3 somewhere around May) and those worked fine for numerous reboots before this upgrade. The first reboot after adding the new image, everything was fine. The 2nd reboot (actually a power outage) the interfaces were eth0-eth2 on the system, but eth1-eth3 in the config, so the config load failed.

I agree @jjakob that we are still converging on the best way to structure the salient information, namely the difference between effective and session configs, however this will be internalized: a simple example of use for testing and daemon (and other) is:

Keep in mind that the preferred way to implement scripts is, in my mind, the one used by interfaces-tunnel.py: it takes both session and effective configs and compares them, only applying the changes for the differences that are required. get_config_dict just takes the session config and so it requires a complete teardown and re-initialization of any system component that is configured from it (restarting instead of reloading services, deleting interfaces instead of modifying just 1 setting, ...)
I don't consider just get_config_dict as the preferred future way to implement features for that reason, rather it is the interfaces-tunnel that could be made the reference.
get_config_dict could be ran 2 times (once with effective=True) in each script, then the script could compare/make a diff of the 2 dicts, but that's what interfaces-tunnel ConfigurationState does.
Take for example a minor change in the current openvpn code - changing the description of the interface - currently results in a complete service outage (restart). AFAIK all scripts are like this. It didn't use to be that way in Vyatta, most perl scripts compared session and effective configs and just applied the necessary changes.

I'm not sure that we have to go that far, but let me take a closer look, and we can discuss; the model is to be able to call, in batch, the functions of an arbitrary conf_mode script, without variation in structure or behaviour.

I agree that this kind of changes are better done at the start of a development cycle.

There is really no difference between calling Config() and using it functions in the code - as is done by every module - vs having the class inherit from it as I do not use any private ( _ prefixed ) functions. I can modify the code to have self.config as an object of the class instead but IMO this is cosmetic and does not change anything with the API and coding guideline.

Only that it inherits from Config, hence the form of the Config instance can not be changed, say, to 'off-line' Config, or 'testing' Config, which would have different init functions; if it took Config as an argument instead, or a named argument with default the 'live' Config, then it would not be an issue for generalization. But we should discuss the details further, and collect results and suggestions.

not willing to take the lead on this task but happy to help.

@jestabro could you please clarify how interfaces-tunnel.py is not following the guideline. The class it uses to generate the dict is internal to the get_config() function and the dict API is respected.

Possibly related to T2205, it might have been fixed since this was reported.

Apparently the "still in use" check logic really leaves much to be desired. See T1292. I wonder if there's a general fix within the current approach.

That part is rewritten in current already. https://github.com/vyos/vyos-1x/blob/current/op-mode-definitions/show-log.xml#L212

Going to mention this in here:

@dmbaturin sure, PR https://github.com/vyos/vyatta-cfg-firewall/pull/17

Turns out it's because the conf mode "allowed: " is escaped and eval'd when it's passed to the shell: https://github.com/vyos/vyatta-cfg/blob/current/src/cstore/cstore.cpp#L756-L762

Could you make a pull request?

I've reviewed the code and it looks good to me.

The root cause was insufficient validation.

Whichever decision we make, let's not change this in 1.3—there are lots of changes already.

This rabbit hole goes deep. It's not just a display issue, but the whole reason we cannot have rollbacks without reboots—there's no way to generate an inverse changeset "thanks" to this.

This task will be resolved by removing Interface.pm altogether.

Glad to hear that!

Sorry it took so long! I've cherry-picked it into crux, will be in 1.2.6.

Ideally we may want to add an extended "if VRRP configured" check, or make keepalived produce an empty (or special) file when it's running but has no data. For now this fix should do though.

VTI is secretly IPIP, so it doesn't support IPv6. The real issue is that we don't support the IPv6 variant of VTI yet.

The user-data dir actually is preserved on upgrade, it's just the check that is faulty. Need to look into it.

Added a warning.