vpp: T8080: Fix handling of configuration system lock after vpp commit failure

[natali-rs1985]

Change summary

• Bail out early if the VPP service is not running, to prevent running configuration steps when VPP is unavailable.
• Call wait_for_commit_lock() in the reset section to make sure reset actions are properly synchronized with other commit operations.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Code style update (formatting, renaming)
• Refactoring (no functional changes)
• Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
• Other (please describe):

Related Task(s)

• https://vyos.dev/T8080

Related PR(s)

vyos/vyatta-cfg#120

How to test / Smoketest result

set vpp settings interface eth1 driver 'dpdk'
commit

set vpp settings buffers page-size 1G

vyos@vyos# commit
[ vpp ]

WARNING: NOTE: Current dataplane capacity (estimated): 2.1 M IPv4
routes. Exceeding these values will lead to a dataplane out-of-memory
condition and a crash. Extensive use of features like ACLs, NAT and
others may reduce the numbers above. Please read the documentation for
details: https://docs.vyos.io/

An error occurred: VPP service failed to start. VPP service will be
restarted with the previous configuration
[[vpp]] failed
Commit failed
[edit]
vyos@vyos# show vpp
+settings {
+    buffers {
+        page-size 1G
+    }
+}

VPP service restarts correctly after error in commit:

vyos@vyos# run show interfaces
Codes: S - State, L - Link, u - Up, D - Down, A - Admin Down
Interface    IP Address        MAC                VRF        MTU  S/L    Description
-----------  ----------------  -----------------  -------  -----  -----  -------------
eth0         -                 0c:58:8e:4c:00:00  default   1500  u/D
eth1         -                 0c:58:8e:4c:00:01  default   1500  u/D
eth2         10.20.40.141/24   0c:58:8e:4c:00:02  default   1500  u/u
eth3         10.217.32.131/24  0c:58:8e:4c:00:03  default   1500  u/u
lo           127.0.0.1/8       00:00:00:00:00:00  default  65536  u/u
             ::1/128
[edit]
vyos@vyos# run show vpp interfaces
Kernel    Dataplane    Type    IP Address    MAC                  MTU  State
--------  -----------  ------  ------------  -----------------  -----  -------
          eth1         dpdk                  0c:58:8e:4c:00:01   1500  down
          local0       local                 00:00:00:00:00:00      0  down
eth1      tap4096      virtio                02:fe:9f:5c:65:75   9000  up
[edit]

Checklist:

• I have read the CONTRIBUTING document
• I have linked this PR to one or more Phabricator Task(s)
• I have run the components SMOKETESTS if applicable
• My commit headlines contain a valid Task id
• My change requires a change to the documentation
• I have updated the documentation accordingly

[github-actions]

👍
No issues in PR Title / Commit Title

[alexk37]

VPP restarts properly after commit failure, no 'commit in progress' exception occurs

vyos@VyOS-for-Smoke-Tests# comp
[vpp settings buffers]
- page-size "2M"
+ page-size "1G"

[edit]
vyos@VyOS-for-Smoke-Tests# commit
[ vpp ]

WARNING: NOTE: Current dataplane capacity (estimated): 2.1 M IPv4
routes. Exceeding these values will lead to a dataplane out-of-memory
condition and a crash. Extensive use of features like ACLs, NAT and
others may reduce the numbers above. Please read the documentation for
details: https://docs.vyos.io/

An error occurred: VPP service is not running or failed to start. VPP
service will be restarted with the previous configuration
[[vpp]] failed
Commit failed
[edit]
vyos@VyOS-for-Smoke-Tests# sudo vppctl sh int
              Name               Idx    State  MTU (L3/IP4/IP6/MPLS)     Counter          Count     
eth1                              1      up          9000/0/0/0     rx packets                     3
                                                                    rx bytes                    1770
                                                                    tx packets                    15
                                                                    tx bytes                    2262
                                                                    drops                          3
                                                                    ip4                            3
local0                            0     down          0/0/0/0       
tap4096                           2      up          9000/0/0/0     rx packets                    14
                                                                    rx bytes                    2132
                                                                    ip4                            3
                                                                    ip6                           11

[zdc]

For me, this change looks correct, except I would probably not use bare Exception here, to avoid catching all possible exceptions in the code after.

[natali-rs1985]

@zdc Changed Exception to RuntimeError. Is it ok?

[zdc]

@zdc Changed Exception to RuntimeError. Is it ok?

Better, but I would prefer a custom exception for this case, if you wish to use an exception. Check an example: https://github.com/natali-rs1985/vyos-1x/blob/e2513d5f88b632fef16f9188acd7e6dc860a3512/python/vyos/base.py#L65

[github-actions]

CI integration 👍 passed!

Details

CI logs

• CLI Smoketests (no interfaces) 👍 passed
• CLI Smoketests VPP 👍 passed
• CLI Smoketests (interfaces only) 👍 passed
• Config tests 👍 passed
• Config tests VPP 👍 passed
• RAID1 tests 👍 passed
• TPM tests 👍 passed

[dmbaturin]

The logic seems correct, and the concern about using a dedicated exceptions is addressed.

[dmbaturin]

Should there be a hierarchy of VPP exceptions now? I'll need to check what else we can add there.