[generic-config-updater] interface's VRF patch change doesn't work

[wen587]

Description

When using apply-patch to change the VRF of an interface. The database change is seen but swss reports error and skip the change.

Steps to reproduce the issue

12/09/2021 Update:
IP is not requied to reproduce it. Just config apply-patch to change VRF for Loopback0 binding with Vrf_02 will reproduce the issue.
Patch Applier: Patch: [{"op": "replace", "path": "/LOOPBACK_INTERFACE/Loopback0/vrf_name", "value": "Vrf_01"}]
vlab-01 ERR swss#intfmgrd: :- doIntfGeneralTask: Loopback0 can not change to Vrf_01 directly, skipping

1.Start a clean KVM and set Loopback0 interface with any IP (10.1.0.32 here)
2.

sudo config vrf add Vrf_01
sudo config vrf add Vrf_02
sudo config interface vrf bind Loopback0 Vrf_02
sudo config int ip add Loopback0 10.1.0.32 (Set IP again due to VRF change remove previous IP)

3. sudo config apply-patch vrf.json
   [{"op": "replace", "path": "/LOOPBACK_INTERFACE/Loopback0/vrf_name", "value": "Vrf_01"}]

Describe the results you received

admin@vlab-01:~/lo$ sudo config apply-patch vrf_change.json
Patch Applier: Patch application starting.
Patch Applier: Patch: [{"op": "replace", "path": "/LOOPBACK_INTERFACE/Loopback0/vrf_name", "value": "Vrf_01"}]
Patch Applier: Validating patch is not making changes to tables without YANG models.
Patch Applier: Getting current config db.
Patch Applier: Simulating the target full config after applying the patch.
Patch Applier: Validating target config does not have empty tables, since they do not show up in ConfigDb.
Patch Applier: Validating target config according to YANG models.
Note: Below table(s) have no YANG models:
BGP_NEIGHBOR, BGP_PEER_RANGE, BUFFER_PG, BUFFER_POOL, BUFFER_PROFILE, BUFFER_QUEUE, CABLE_LENGTH, CONSOLE_SWITCH, DEV                                                                                                                        ICE_METADATA, DEVICE_NEIGHBOR_METADATA, DHCP_RELAY, DHCP_SERVER, FEATURE, KDUMP, QUEUE, RESTAPI, SCHEDULER, SNMP, SNM                                                                                                                        P_COMMUNITY, TELEMETRY, WRED_PROFILE,
Patch Applier: Sorting patch updates.
Note: Below table(s) have no YANG models:
BGP_NEIGHBOR, BGP_PEER_RANGE, BUFFER_PG, BUFFER_POOL, BUFFER_PROFILE, BUFFER_QUEUE, CABLE_LENGTH, CONSOLE_SWITCH, DEV                                                                                                                        ICE_METADATA, DEVICE_NEIGHBOR_METADATA, DHCP_RELAY, DHCP_SERVER, FEATURE, KDUMP, QUEUE, RESTAPI, SCHEDULER, SNMP, SNM                                                                                                                        P_COMMUNITY, TELEMETRY, WRED_PROFILE,
Note: Below table(s) have no YANG models:
BGP_NEIGHBOR, BGP_PEER_RANGE, BUFFER_PG, BUFFER_POOL, BUFFER_PROFILE, BUFFER_QUEUE, CABLE_LENGTH, CONSOLE_SWITCH, DEV                                                                                                                        ICE_METADATA, DEVICE_NEIGHBOR_METADATA, DHCP_RELAY, DHCP_SERVER, FEATURE, KDUMP, QUEUE, RESTAPI, SCHEDULER, SNMP, SNM                                                                                                                        P_COMMUNITY, TELEMETRY, WRED_PROFILE,
Patch Applier: The patch was sorted into 1 change:
Patch Applier:   * [{"op": "replace", "path": "/LOOPBACK_INTERFACE/Loopback0/vrf_name", "value": "Vrf_01"}]
Patch Applier: Applying changes in order.
Patch Applier: Verifying patch updates are reflected on ConfigDB.
Patch Applier: Patch application completed.

admin@vlab-01:~/lo$ show ip route vrf Vrf_01

admin@vlab-01:~/lo$ show ip route vrf Vrf_02
VRF Vrf_02:
C>* 10.1.0.32/32 is directly connected, Loopback0, 00:05:39
admin@vlab-01:~/lo$ sudo cat /var/log/syslog | less
Dec  6 08:54:35.160668 vlab-01 NOTICE /config: Patch Applier: Applying changes in order.
Dec  6 08:54:35.668188 vlab-01 DEBUG bgp#bgpcfgd: Received message : '('Loopback0', 'SET', (('vrf_name', 'Vrf_01'),))
Dec  6 08:54:35.669081 vlab-01 ERR swss#intfmgrd: :- doIntfGeneralTask: Loopback0 can not change to Vrf_01 directly, skipping

Describe the results you expected

If the patch is applied successfully, Loopback0 should be connected to Vrf_01. But above command show empty ip route in Vrf_01

Additional information you deem important (e.g. issue happens only occasionally)

Output of show version

SONiC Software Version: SONiC.renuka.0-0c68585ec
Distribution: Debian 10.11
Kernel: 4.19.0-12-2-amd64
Build commit: 0c68585
Build date: Thu Oct 28 09:37:47 UTC 2021
Built by: jxie@jingwen-vm-01

Platform: x86_64-kvm_x86_64-r0
HwSKU: Force10-S6000
ASIC: vs
ASIC Count: 1
Serial Number: N/A
Model Number: N/A
Hardware Revision: N/A
Uptime: 05:07:49 up 26 days, 21:51, 1 user, load average: 1.24, 1.63, 1.40

(paste your output here)

[ghooo]

[Yesterday 18:06] Jingwen Xie
When interface is bind to a VRF, its IP is expected to be removed.

[9:45 AM] Jingwen XieHi Prince, any update on that?
I also checked that after we add the interface to VRF, its IP is removed. Is that expected?

[9:46 AM] Prince Sunnythat is expected.. coz we have to add the interface to VRF first before associating IP

[ghooo]

Removing all loopback0 related configs and adding it back with the new vrf_name solved the problem:

admin@vlab-01:~/lo$ sudo config vrf add Vrf_01
admin@vlab-01:~/lo$ sudo config vrf add Vrf_02
admin@vlab-01:~/lo$ sudo config interface vrf bind Loopback0 Vrf_02
admin@vlab-01:~/lo$ sudo config int ip add Loopback0 10.1.0.32
admin@vlab-01:~/lo$ show ip route vrf Vrf_01

admin@vlab-01:~/lo$ show ip route vrf Vrf_02
Codes: K - kernel route, C - connected, S - static, R - RIP,
       O - OSPF, I - IS-IS, B - BGP, E - EIGRP, N - NHRP,
       T - Table, v - VNC, V - VNC-Direct, A - Babel, D - SHARP,
       F - PBR, f - OpenFabric,
       > - selected route, * - FIB route, q - queued, r - rejected, b - backup

VRF Vrf_02:
C>* 10.1.0.32/32 is directly connected, Loopback0, 00:00:20

admin@vlab-01:~/lo$ vim remov_lo0.json-patch
admin@vlab-01:~/lo$ vim add_lo0.json-patch
admin@vlab-01:~/lo$ sudo config apply-patch remov_lo0.json-patch -i ''
Patch Applier: Patch application starting.
Patch Applier: Patch: [{"op": "remove", "path": "/LOOPBACK_INTERFACE"}]
Patch Applier: Getting current config db.
Patch Applier: Simulating the target full config after applying the patch.
Patch Applier: Validating target config does not have empty tables, since they do not show up in ConfigDb.
Patch Applier: Sorting patch updates.
Patch Applier: The patch was sorted into 1 change:
Patch Applier:   * [{"op": "remove", "path": "/LOOPBACK_INTERFACE"}]
Patch Applier: Applying 1 change in order:
Patch Applier:   * [{"op": "remove", "path": "/LOOPBACK_INTERFACE"}]
Patch Applier: Verifying patch updates are reflected on ConfigDB.
Patch Applier: Patch application completed.
Patch applied successfully.
admin@vlab-01:~/lo$ show ip route vrf Vrf_01

admin@vlab-01:~/lo$ show ip route vrf Vrf_02

admin@vlab-01:~/lo$ sudo config apply-patch add_lo0.json-patch -i ''
Patch Applier: Patch application starting.
Patch Applier: Patch: [{"op": "add", "path": "/LOOPBACK_INTERFACE", "value": {"Loopback0": {"vrf_name": "Vrf_01"}, "Loopback0|10.1.0.32/32": {}}}]
Patch Applier: Getting current config db.
Patch Applier: Simulating the target full config after applying the patch.
Patch Applier: Validating target config does not have empty tables, since they do not show up in ConfigDb.
Patch Applier: Sorting patch updates.
Patch Applier: The patch was sorted into 1 change:
Patch Applier:   * [{"op": "add", "path": "/LOOPBACK_INTERFACE", "value": {"Loopback0": {"vrf_name": "Vrf_01"}, "Loopback0|10.1.0.32/32": {}}}]
Patch Applier: Applying 1 change in order:
Patch Applier:   * [{"op": "add", "path": "/LOOPBACK_INTERFACE", "value": {"Loopback0": {"vrf_name": "Vrf_01"}, "Loopback0|10.1.0.32/32": {}}}]
Patch Applier: Verifying patch updates are reflected on ConfigDB.
Patch Applier: Patch application completed.
Patch applied successfully.
admin@vlab-01:~/lo$ show ip route vrf Vrf_01
Codes: K - kernel route, C - connected, S - static, R - RIP,
       O - OSPF, I - IS-IS, B - BGP, E - EIGRP, N - NHRP,
       T - Table, v - VNC, V - VNC-Direct, A - Babel, D - SHARP,
       F - PBR, f - OpenFabric,
       > - selected route, * - FIB route, q - queued, r - rejected, b - backup

VRF Vrf_01:
C>* 10.1.0.32/32 is directly connected, Loopback0, 00:00:18

admin@vlab-01:~/lo$ show ip route vrf Vrf_02

admin@vlab-01:~/lo$ 

[ghooo]

vrf_name cannot be deleted

Dec 10 22:43:17.178553 vlab-01 NOTICE /config: Patch Applier: * [{"op": "remove", "path": "/LOOPBACK_INTERFACE/Loopback0/vrf_name"}]
Dec 10 22:43:17.466954 vlab-01 DEBUG bgp#bgpcfgd: Received message : '('Loopback0', 'SET', (('NULL', 'NULL'),))'
Dec 10 22:43:17.474403 vlab-01 ERR swss#intfmgrd: :- doIntfGeneralTask: Loopback0 can not change to directly, skipping

[ghooo]

vrf_name cannot be replaced

Dec 10 22:34:04.454059 vlab-01 NOTICE /config: Patch Applier: * [{"op": "replace", "path": "/LOOPBACK_INTERFACE/Loopback0/vrf_name", "value": "Vrf_01"}]
Dec 10 22:34:04.770602 vlab-01 ERR swss#intfmgrd: :- doIntfGeneralTask: Loopback0 can not change to Vrf_01 directly, skipping

[ghooo]

We have 3 basic operations in the patch-sorter:

• replace
• delete
• add

Since vrf_name cannot be replaced nor deleted. In order to update it we have to delete its parent instead i.e. delete Loopback0.

To implement this we will:

• Mark vrf_name as CreateOnly i.e. (not replacable)-- hardcoded for now in the patch sorter
• Mark vrf_name as NotDeletable -- will add a new validator and hardcode vrf_name as not deletable
• Make sure DeleteRefsMoveExtender will delete Loopback0 related refs i.e Loopback0|<ip>

The parent Loopback0 will be deleted then added back, while deleting Loopback0, we will have to delete all associated ips i.e. delete also Loopback0|10.1.0.31 and Loopback0|::F003:0. Once they are This will simulate the steps in the issue description:

• Delete Loopback0 IPs
• Delete Loopback0 itself
• Add Loopback0 with with new vrf_name
• Add Loopback0 IPs

[ghooo]

Discussed offline with @qiluo-msft

Going to update create-only flag meaning.

From, Field is not replaceable but can be added or deleted. In other words:

• Field can be added
• Field can be deleted
• Field cannot be replaced

To, Field is only created, but never modified/updated. In other words:

• Field cannot be added, only if the parent is added
• Field cannot be deleted, only if the parent is deleted
• Field cannot be replaced