Skip to content

Commit 0f8499b

Browse files
aayueshbaruiaayueshbarui
authored
Add OverrideLeafList support in ygot diff. (#1046)
* Add DeleteWithUpdateLeafList support in ygot diff. Implemented handling for DeleteWithUpdateLeafList operation in diff.go Added corresponding test cases in diff_test.go Tested locally to ensure correct functionality and pushed the generated Diff config in Arista FakeFabric (Google Internal Tool). Details: Explicitly delete and then update leaf-list, which is different and exists in both orig and modified. For example, if the original path is x/y/[a, b] and the modified leaf-list is x/y/[b, c], Then resulting notification will contain: Delete x/y/[a, b] Update x/y/[b, c] If this is false, then the leaf-list will be updated directly to the new value without the delete step. This is required because if the leaf-list is updated directly, the resulting notification will be: Update x/y/[b, c] This will cause the new value to be appended in the OC, but the original value will still be present in the OC. Resultant OC in the device will be x/y/[a, b, c]. * add newline * rename to OverrideLeafList * correct the delete op comment * correct the comment * ignore binary type * fix tests * ignore binary not uint8 * improve code
1 parent 07135be commit 0f8499b

File tree

2 files changed

+228
-4
lines changed

2 files changed

+228
-4
lines changed

‎ygot/diff.go‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,14 @@ type DiffPathOpt struct {
446446
// -ignore_shadow_schema_paths flag, and therefore have the
447447
// "shadow-path" tag.
448448
PreferShadowPath bool
449+
// Explicitly delete and then update leaf-list, which is different and exists in both orig and modified.
450+
// For example, if the original path is x/y/[a, b] and the modified leaf-list is x/y/[b, c],
451+
// Then resulting notification will contain:
452+
// - Delete { Path: x/y }
453+
// - Update { Path: x/y Value: [b,c] }
454+
// If this is false, then the leaf-list will be updated directly to the new value
455+
// without the delete step.
456+
OverrideLeafList bool
449457
}
450458

451459
// IsDiffOpt marks DiffPathOpt as a diff option.
@@ -646,6 +654,16 @@ func createAtomicNotif(atomicLeaves []*pathval, ts int64, subtreePfx *gnmiPath)
646654
return no, nil
647655
}
648656

657+
// isLeafList returns true if the value is a leaf-list.
658+
func isLeafList(val any) bool {
659+
if val == nil {
660+
return false
661+
}
662+
v := reflect.TypeOf(val)
663+
// Binary is represented as a []byte (slice), but is not a leaf-list.
664+
return v.Kind() == reflect.Slice && v.Name() != BinaryTypeName
665+
}
666+
649667
// diff produces a slice of notifications given two GoStructs.
650668
//
651669
// See documentation for Diff and DiffWithAtomic for more information.
@@ -678,15 +696,21 @@ func diff(original, modified GoStruct, withAtomic bool, opts ...DiffOpt) ([]*gnm
678696

679697
var atomicNotifs []*gnmipb.Notification
680698
n := &gnmipb.Notification{}
681-
processUpdate := func(path string, modVal *pathInfo) error {
699+
processUpdate := func(path string, modVal *pathInfo, origVal *pathInfo) error {
700+
diffopts := hasDiffPathOpt(opts)
682701
if orderedMap, isOrderedMap := modVal.val.(GoOrderedMap); isOrderedMap {
683-
diffopts := hasDiffPathOpt(opts)
684702
preferShadowPath := diffopts != nil && diffopts.PreferShadowPath
685703
notif, err := orderedMapNotif(orderedMap, newPathElemGNMIPath(modVal.path.GetElem()), 0, preferShadowPath)
686704
if err != nil {
687705
return err
688706
}
689707
atomicNotifs = append(atomicNotifs, notif)
708+
} else if diffopts != nil && diffopts.OverrideLeafList && isLeafList(modVal.val) && origVal != nil {
709+
// Handle the leaf-list replace. Delete the original path before updating it.
710+
n.Delete = append(n.Delete, origVal.path)
711+
if err := appendUpdate(n, path, modVal); err != nil {
712+
return err
713+
}
690714
} else {
691715
// The contents of the value should indicate that value a has changed
692716
// to value b.
@@ -700,7 +724,7 @@ func diff(original, modified GoStruct, withAtomic bool, opts ...DiffOpt) ([]*gnm
700724
for origPath, origVal := range origLeavesStr {
701725
if modVal, ok := modLeavesStr[origPath]; ok {
702726
if !reflect.DeepEqual(origVal.val, modVal.val) {
703-
if err := processUpdate(origPath, modVal); err != nil {
727+
if err := processUpdate(origPath, modVal, origVal); err != nil {
704728
return nil, err
705729
}
706730
}
@@ -723,7 +747,7 @@ func diff(original, modified GoStruct, withAtomic bool, opts ...DiffOpt) ([]*gnm
723747
// not they are updates.
724748
for modPath, modVal := range modLeavesStr {
725749
if _, ok := origLeavesStr[modPath]; !ok {
726-
if err := processUpdate(modPath, modVal); err != nil {
750+
if err := processUpdate(modPath, modVal, nil); err != nil {
727751
return nil, err
728752
}
729753
}

‎ygot/diff_test.go‎

Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1680,3 +1680,203 @@ func TestLeastSpecificPath(t *testing.T) {
16801680
}
16811681
}
16821682
}
1683+
1684+
type TestDiffDeleteStruct struct {
1685+
Leaf *string `path:"leaf"`
1686+
LeafList []string `path:"leaf-list"`
1687+
OrderedMap map[string]*TestDiffDeleteStructChild `path:"ordered-map" ordered-by:"user"`
1688+
Binary Binary `path:"binary"`
1689+
UInt8LeafList []uint8 `path:"u-int8-leaf-list"`
1690+
}
1691+
1692+
type TestDiffDeleteStructChild struct {
1693+
ChildLeaf *string `path:"child-leaf"`
1694+
}
1695+
1696+
func (*TestDiffDeleteStruct) IsYANGGoStruct() {}
1697+
1698+
func (*TestDiffDeleteStructChild) IsYANGGoStruct() {}
1699+
1700+
func (t *TestDiffDeleteStruct) ΛListKeyMap() (map[string]any, error) {
1701+
return nil, nil
1702+
}
1703+
1704+
func (t *TestDiffDeleteStructChild) ΛListKeyMap() (map[string]any, error) {
1705+
return nil, nil
1706+
}
1707+
1708+
func TestDiffOverrideLeafList(t *testing.T) {
1709+
tests := []struct {
1710+
name string
1711+
original *TestDiffDeleteStruct
1712+
modified *TestDiffDeleteStruct
1713+
opts []DiffOpt
1714+
want *gnmipb.Notification
1715+
wantErr bool
1716+
}{{
1717+
name: "delete leaf list",
1718+
original: &TestDiffDeleteStruct{
1719+
LeafList: []string{"a", "b"},
1720+
Binary: Binary("binary-old"),
1721+
UInt8LeafList: []uint8{1, 2, 3},
1722+
},
1723+
modified: &TestDiffDeleteStruct{
1724+
LeafList: []string{"b", "c"},
1725+
Binary: Binary("binary-new"),
1726+
UInt8LeafList: []uint8{4, 5, 6},
1727+
},
1728+
opts: []DiffOpt{&DiffPathOpt{OverrideLeafList: true}},
1729+
want: &gnmipb.Notification{
1730+
Delete: []*gnmipb.Path{
1731+
{Elem: []*gnmipb.PathElem{{Name: "leaf-list"}}},
1732+
{Elem: []*gnmipb.PathElem{{Name: "u-int8-leaf-list"}}},
1733+
// Binary is not deleted because it is not a leaf-list.
1734+
},
1735+
Update: []*gnmipb.Update{{
1736+
Path: &gnmipb.Path{Elem: []*gnmipb.PathElem{{Name: "leaf-list"}}},
1737+
Val: &gnmipb.TypedValue{
1738+
Value: &gnmipb.TypedValue_LeaflistVal{
1739+
LeaflistVal: &gnmipb.ScalarArray{
1740+
Element: []*gnmipb.TypedValue{
1741+
{Value: &gnmipb.TypedValue_StringVal{StringVal: "b"}},
1742+
{Value: &gnmipb.TypedValue_StringVal{StringVal: "c"}},
1743+
},
1744+
},
1745+
},
1746+
},
1747+
}, {
1748+
Path: &gnmipb.Path{Elem: []*gnmipb.PathElem{{Name: "binary"}}},
1749+
Val: &gnmipb.TypedValue{Value: &gnmipb.TypedValue_BytesVal{BytesVal: []byte("binary-new")}},
1750+
}, {
1751+
Path: &gnmipb.Path{Elem: []*gnmipb.PathElem{{Name: "u-int8-leaf-list"}}},
1752+
Val: &gnmipb.TypedValue{Value: &gnmipb.TypedValue_LeaflistVal{
1753+
LeaflistVal: &gnmipb.ScalarArray{
1754+
Element: []*gnmipb.TypedValue{
1755+
{Value: &gnmipb.TypedValue_UintVal{4}},
1756+
{Value: &gnmipb.TypedValue_UintVal{5}},
1757+
{Value: &gnmipb.TypedValue_UintVal{6}},
1758+
},
1759+
},
1760+
}},
1761+
}},
1762+
},
1763+
}, {
1764+
name: "no delete leaf list",
1765+
original: &TestDiffDeleteStruct{
1766+
LeafList: []string{"a", "b"},
1767+
Binary: Binary("binary-old"),
1768+
UInt8LeafList: []uint8{1, 2, 3},
1769+
},
1770+
modified: &TestDiffDeleteStruct{
1771+
LeafList: []string{"b", "c"},
1772+
UInt8LeafList: []uint8{4, 5, 6},
1773+
Binary: Binary("binary-new"),
1774+
},
1775+
want: &gnmipb.Notification{
1776+
Update: []*gnmipb.Update{{
1777+
Path: &gnmipb.Path{Elem: []*gnmipb.PathElem{{Name: "leaf-list"}}},
1778+
Val: &gnmipb.TypedValue{
1779+
Value: &gnmipb.TypedValue_LeaflistVal{
1780+
LeaflistVal: &gnmipb.ScalarArray{
1781+
Element: []*gnmipb.TypedValue{
1782+
{Value: &gnmipb.TypedValue_StringVal{StringVal: "b"}},
1783+
{Value: &gnmipb.TypedValue_StringVal{StringVal: "c"}},
1784+
},
1785+
},
1786+
},
1787+
},
1788+
}, {
1789+
Path: &gnmipb.Path{Elem: []*gnmipb.PathElem{{Name: "binary"}}},
1790+
Val: &gnmipb.TypedValue{Value: &gnmipb.TypedValue_BytesVal{BytesVal: []byte("binary-new")}},
1791+
}, {
1792+
Path: &gnmipb.Path{Elem: []*gnmipb.PathElem{{Name: "u-int8-leaf-list"}}},
1793+
Val: &gnmipb.TypedValue{Value: &gnmipb.TypedValue_LeaflistVal{
1794+
LeaflistVal: &gnmipb.ScalarArray{
1795+
Element: []*gnmipb.TypedValue{
1796+
{Value: &gnmipb.TypedValue_UintVal{4}},
1797+
{Value: &gnmipb.TypedValue_UintVal{5}},
1798+
{Value: &gnmipb.TypedValue_UintVal{6}},
1799+
},
1800+
},
1801+
}},
1802+
}},
1803+
},
1804+
}, {
1805+
name: "no change leaf list",
1806+
original: &TestDiffDeleteStruct{
1807+
LeafList: []string{"a", "b"},
1808+
},
1809+
modified: &TestDiffDeleteStruct{
1810+
LeafList: []string{"a", "b"},
1811+
},
1812+
opts: []DiffOpt{&DiffPathOpt{OverrideLeafList: true}},
1813+
want: &gnmipb.Notification{},
1814+
}, {
1815+
name: "add leaf list",
1816+
original: &TestDiffDeleteStruct{},
1817+
modified: &TestDiffDeleteStruct{
1818+
LeafList: []string{"b", "c"},
1819+
Binary: Binary("binary-new"),
1820+
UInt8LeafList: []uint8{4, 5, 6},
1821+
},
1822+
opts: []DiffOpt{&DiffPathOpt{OverrideLeafList: true}},
1823+
want: &gnmipb.Notification{
1824+
Update: []*gnmipb.Update{{
1825+
Path: &gnmipb.Path{Elem: []*gnmipb.PathElem{{Name: "leaf-list"}}},
1826+
Val: &gnmipb.TypedValue{
1827+
Value: &gnmipb.TypedValue_LeaflistVal{
1828+
LeaflistVal: &gnmipb.ScalarArray{
1829+
Element: []*gnmipb.TypedValue{
1830+
{Value: &gnmipb.TypedValue_StringVal{StringVal: "b"}},
1831+
{Value: &gnmipb.TypedValue_StringVal{StringVal: "c"}},
1832+
},
1833+
},
1834+
},
1835+
},
1836+
}, {
1837+
Path: &gnmipb.Path{Elem: []*gnmipb.PathElem{{Name: "binary"}}},
1838+
Val: &gnmipb.TypedValue{Value: &gnmipb.TypedValue_BytesVal{BytesVal: []byte("binary-new")}},
1839+
}, {
1840+
Path: &gnmipb.Path{Elem: []*gnmipb.PathElem{{Name: "u-int8-leaf-list"}}},
1841+
Val: &gnmipb.TypedValue{Value: &gnmipb.TypedValue_LeaflistVal{
1842+
LeaflistVal: &gnmipb.ScalarArray{
1843+
Element: []*gnmipb.TypedValue{
1844+
{Value: &gnmipb.TypedValue_UintVal{4}},
1845+
{Value: &gnmipb.TypedValue_UintVal{5}},
1846+
{Value: &gnmipb.TypedValue_UintVal{6}},
1847+
},
1848+
},
1849+
}},
1850+
}},
1851+
},
1852+
}, {
1853+
name: "delete leaf list",
1854+
original: &TestDiffDeleteStruct{
1855+
LeafList: []string{"a", "b"},
1856+
Binary: Binary("binary"),
1857+
UInt8LeafList: []uint8{1, 2, 3},
1858+
},
1859+
modified: &TestDiffDeleteStruct{},
1860+
opts: []DiffOpt{&DiffPathOpt{OverrideLeafList: true}},
1861+
want: &gnmipb.Notification{
1862+
Delete: []*gnmipb.Path{
1863+
{Elem: []*gnmipb.PathElem{{Name: "leaf-list"}}},
1864+
{Elem: []*gnmipb.PathElem{{Name: "binary"}}},
1865+
{Elem: []*gnmipb.PathElem{{Name: "u-int8-leaf-list"}}},
1866+
},
1867+
},
1868+
}}
1869+
1870+
for _, tt := range tests {
1871+
t.Run(tt.name, func(t *testing.T) {
1872+
got, err := Diff(tt.original, tt.modified, tt.opts...)
1873+
if (err != nil) != tt.wantErr {
1874+
t.Errorf("Diff() error = %v, wantErr %v", err, tt.wantErr)
1875+
return
1876+
}
1877+
if diff := cmp.Diff(tt.want, got, protocmp.Transform(), protocmp.SortRepeatedFields((*gnmipb.Notification)(nil), "update", "delete")); diff != "" {
1878+
t.Errorf("Diff() returned unexpected diff (-want +got):\n%s", diff)
1879+
}
1880+
})
1881+
}
1882+
}

0 commit comments

Comments
 (0)