Add Virtual Database Client + Unit Tests by randygaulmsft · Pull Request #18 · sonic-net/sonic-telemetry

randygaulmsft · 2019-03-25T21:51:49Z

This commit adds virtual database support for the SONiC target, built by an intern Chang Lui. I added some integration tests over the top of the virtual path + virtual database work by Chang. Here is a diagram describing the high level organization of most of Chang's contributions. Some important pieces are numbered, and described below.

Information about the Numbered Pieces of the Above Diagram

virtual_database_client/db_client.go
- NewDbClient - Instance created to deal with all RPC logic
- Get
- PollRun
virtual_database_client/handler_*.go
- Injects data into the search trie. The trie maps gNMI paths to Redis keys.
virtual_database_client/handler_<YOUR_DATA_TYPE>.go
- path2HdlrFuncTbl - Table to populate the search trie. Each entry represents a supported data type. Each handler references a function in handler_<YOUR_DATA_TYPE>.go file.
- TriePopulate - Called when module is initialized, and loops over path2HdlrFuncTbl.
virtual_database_client/path.go
- populateAlltablePaths - Translate gNMI path to Redis keys
- msi2TypedValue - Transform redis value to proto stream for RPC return value

Important Callstacks

Get

The below callstack reaches into the handlers to construct gNMI to Redis mappings.

Get - db_client.go line 188
populateAlltablePaths - path.go line 31
populateNewtablePath - path.go line 44
searchPathTrie - handler_func.go line 53
handler - handler_func.go line 60

The final line calls into the "virtual to real" conversion functions as defined in the handler_<YOUR_DATA_TYPE>.go files. Once a value is retrieved it is handed to tableData2TypedValue and converted to JSON form ready for gNMI proto stream return.

This callstack converts a Redis key value to an appropriate proto stream return value.

Get - db_client.go line 188
tableData2TypedValue - path.go line 99
tableData2Msi - path.go line 111
msi2TypedValue - path.go line 73

tableData2Msi calls into the redis API with redis.HGetAll, passing in redis keys for actual SONiC values. The redis return value is converted to gNMI proto stream form via msi2TypedValue.

Subscribe (StreamRun)

StreamRun - db_client.go line 126
dbPathSubscribe - path.go line 214 - Runs as a goroutine, performs actual susbscription logic. The calling function will wait until the goroutine syncs.
tableData2TypedValue - path.go line 99
tableData2Msi - path.go line 111
msi2TypedValue - path.go line 73
dbSingleTableKeySubscribe - path.go line 158

Subscribe (PollRun)

PollRun is not implemented!

NewDbClient

Called when an RPC is recieved by the gNMI service running on the SONiC device (gnmi_server/*.go). These functions are called to initialize gNMI to Redis mappings:

initCountersPortNameMap
initCountersQueueNameMap
initAliasMap
initCountersPfcwdNameMap

These functions load the various Redis targets into suitable run-time formats, used in gNMI <-> SONiC key transformations via the database client's trie data structure. These functions are using redis.HGetAll, from the Go redis API to load out key-value pairs.

Running Tests

Use go test -v ./package_folder. Make sure the folder points to a particular package you want to test.

…eparator

…lient to virtual_database_client

dialout/dialout_client/dialout_client.go

dialout/dialout_client/dialout_client_test.go

gnmi_server/client_subscribe.go

README.md

hui-ma · 2019-04-10T16:34:16Z

The flowchart and callstack is very helpful to new developers and reviewers. Thanks for adding them.
Could you please also add some for PollRun callstack?

hui-ma · 2019-04-10T16:36:46Z

Add Wenda to verify PFC WD logic

randygaulmsft · 2019-04-14T22:52:03Z

@hui-ma PollRun is not implemented. StreamRun is implemented though.

Many of Jipan's tests from gnmi_server are not possible with the virtual database client, because it does not support some of the query features such as lookup up a single field under PfcCounter. However, it is possible to write some tests for StreamRun via Subscribe.

Not implemented in virtual database:

Path to a specific field under PfcCounter.
PollRun.
Set.
Probably more, but would require more time to all missing pieces.

Can have additional testing to closer match Jipan's tests:

StreamRun.

I will work on StreamRun tests.

jipanyang · 2019-04-15T03:36:17Z

Could you use gofmt to format your changes?

There are some existing virtual path implementations integrated with the StreamRun() and PollRun() as of today. It looks you duplicated part of the code, what are the major differences?

qiluo-msft · 2019-04-19T19:39:15Z

Could you share the command line to unit test? We plan to add it into PR checker and automate it, even for this PR. #Closed

jleveque · 2019-04-19T19:42:47Z

@qiluo-msft: @randygaulmsft is working on adding the PR check build.

hui-ma

Please push your latest change. Thanks!

gnmi_server/client_subscribe.go

sonic_data_client/db_client.go

hui-ma · 2019-04-25T18:47:27Z

sonic_data_client/db_client.go

+		// Debug log
+		//log.V(5).Infof("msi: %v\n", msi)
+		//log.V(5).Infof("key: %v\n", key)
+		//log.V(5).Infof("op: %v\n", op)


Remove these

They don't appear to have been removed.

Ah these were in multiple files. I removed them from virtual_database_client but this one was under sonic_data_client.

Please remove if you have not. Thanks!

virtual_database_client/db_client.go

…ting

…lemetry into new-pfcwd

hui-ma

Add a few comments

virtual_database_client/handler_pfcwd.go

sonic_data_client/db_client.go

randygaulmsft · 2019-05-15T19:24:14Z

Could you use gofmt to format your changes?

There are some existing virtual path implementations integrated with the StreamRun() and PollRun() as of today. It looks you duplicated part of the code, what are the major differences?

@jipanyang Ran go format (push pending). Also I noticed a lot of code duplication in virtual_client_database as well. This was written by Chang Lui the previous intern. The idea here was to augment the telemetry container with the new virtual database to add new query features.

Your code was duplicated to avoid potentially breaking the existing implementation. As far as I know there are no significant changes, other than string translations from vendor-specific form through the new generic query form.

randygaulmsft · 2019-05-15T19:53:27Z

@qiluo-msft

I've already setup the PR builder. Here's the current version of the script as-is in Jenkins.

#!/bin/bash -xe

# Install golang.
go_version=go1.12.4.linux-amd64

if [ ! -e $go_version.tar.gz ]
then
    wget -q -o /dev/null https://storage.googleapis.com/golang/$go_version.tar.gz
    tar xf $go_version.tar.gz
fi

export GOROOT=$WORKSPACE/go
export PATH=$PATH:$GOROOT/bin
export GOPATH=$HOME/go

# Setup and install redis, which requires some minor conf settings for the unit tests.
sudo apt -qq -y install redis-server

sudo sed -i 's/^supervised no/supervised systemd/' /etc/redis/redis.conf
sudo sed -i '$ a unixsocket /var/run/redis/redis.sock' /etc/redis/redis.conf
sudo sed -i '$ a unixsocketperm 766' /etc/redis/redis.conf

sudo service redis stop
sudo service redis start

# Install and test specific packages.
go get -v -t github.com/Azure/sonic-telemetry/gnmi_server/...
go test -v $GOPATH/src/github.com/Azure/sonic-telemetry/gnmi_server

go get -v -t github.com/Azure/sonic-telemetry/virtual_database_client/...
go test -v $GOPATH/src/github.com/Azure/sonic-telemetry/virtual_database_client

NOTE: I just now added Azure/sonic-telemetry/virtual_database_client (the last two lines of the script), so the PR builder will fail until this PR is actually merged in, since the paths do not quite yet match.

randygaulmsft · 2019-05-16T23:31:12Z

Get and run code:

go get -u github.com/Azure/sonic-telemetry/gnmi_server
cd ~/go/src/github.com/Azure
delete all files in here
git clone -b new-pfcwd --single-branch https://github.com/randygaulmsft/sonic-telemetry.git
cd sonic-telemetry
go test -v ./virtual_database_client

sonic_data_client/db_client.go

virtual_database_client/handler_pfcwd.go

virtual_database_client/handler_queue_counter.go

virtual_database_client/map_init.go

jleveque

As comments

virtual_database_client/path.go

randygaulmsft · 2019-05-17T05:41:16Z

Please note the Jenkins script is failing now since I added the new package path. This is expected. The tests are running properly as expected, and besides any more feedback the pull request is ready for merging.

hui-ma · 2019-05-17T17:08:04Z

The new unittest depends on unixsocket while the legacy on doesn't need. Ideally, the new one should not either. Please check what the new dependence is introduced in the new test case. unixsocket is for better performance. Therefore, unit test doesn't necessary depends on it.

hui-ma · 2019-05-17T17:09:59Z

Even with unixsocket, I saw failure case. e.g, FAIL: TestVirtualDatabaseGNMISubscribe/Stream_query_for_Interfaces/Port[name=Ethernet68]/Queue[name=Queue3]/Pfcwd,_updating_PFC_WD_QUEUE_STATS_DEADLOCK_DETECTED_from_0_to_1. (2.53s)

Please verify and fix if there is any

hui-ma · 2019-05-17T17:21:33Z

sonic_data_client/db_client.go

+		// Debug log
+		//log.V(5).Infof("msi: %v\n", msi)
+		//log.V(5).Infof("key: %v\n", key)
+		//log.V(5).Infof("op: %v\n", op)


Please remove if you have not. Thanks!

hui-ma · 2019-05-17T17:24:04Z

virtual_database_client/handler_pfcwd.go

+func v2rPortQueuePfcwdStats(path *gnmipb.Path, pathG2S *map[*gnmipb.Path][]tablePath) error {
+	var tmpl = gnmipb.Path{}
+	GetTmpl_PortQueuePfcwdStats(&tmpl)
+	//fmt.Printf("tmpl: %v\n", &tmpl)


remove please

hui-ma · 2019-05-17T17:28:03Z

virtual_database_client/handler_pfcwd.go

+				out_tblPaths = append(out_tblPaths, tblPath_port)
+			}
+
+			//fmt.Printf("tablePath: %v\n", &tblPath_que)


please do a search and remove such commented "printf" lines.

chaliu and others added 7 commits September 5, 2018 13:24

Add feature to support query for PFC watchdog

43a8fcd

fix typos;rewrite log messeages; replace hard-coded : with database s…

3908994

…eparator

replace hardcoded table separator |

cca4c30

Implement new virtual path for pfcwd

750133e

Unit tests for virtual database target SONiC, rename new_sonic_data_c…

b2c6c5e

…lient to virtual_database_client

Merge branch 'master' into new-pfcwd

9ea0465

remove testing directory imports

0fcf9a5

randygaulmsft requested review from hui-ma, jipanyang, jleveque and lguohan March 26, 2019 19:11

randygaulmsft requested a review from bhoomika11 April 2, 2019 19:22

lguohan reviewed Apr 4, 2019

View reviewed changes

dialout/dialout_client/dialout_client.go Outdated Show resolved Hide resolved

lguohan reviewed Apr 4, 2019

View reviewed changes

dialout/dialout_client/dialout_client_test.go Outdated Show resolved Hide resolved

lguohan reviewed Apr 5, 2019

View reviewed changes

gnmi_server/client_subscribe.go Outdated Show resolved Hide resolved

lguohan reviewed Apr 5, 2019

View reviewed changes

gnmi_server/client_subscribe.go Show resolved Hide resolved

hui-ma reviewed Apr 10, 2019

View reviewed changes

README.md Show resolved Hide resolved

hui-ma requested review from hui-ma and wendani April 10, 2019 16:34

randygaulmsft added 2 commits April 10, 2019 21:07

silence unnecessary printfs

f0e0fdb

remove more debug printf's

bd4af90

hui-ma reviewed Apr 25, 2019

View reviewed changes

randygaulmsft added 2 commits April 25, 2019 19:59

Attempt to fixup import paths and remove old unnecessary debug format…

da43c69

…ting

Merge branch 'new-pfcwd' of https://github.com/randygaulmsft/sonic-te…

6dbe27c

…lemetry into new-pfcwd

hui-ma reviewed Apr 25, 2019

View reviewed changes

virtual_database_client/handler_pfcwd.go Outdated Show resolved Hide resolved

sonic_data_client/db_client.go Outdated Show resolved Hide resolved

jleveque reviewed Apr 30, 2019

View reviewed changes

sonic_data_client/db_client.go Outdated Show resolved Hide resolved

jleveque reviewed Apr 30, 2019

View reviewed changes

sonic_data_client/db_client.go Outdated Show resolved Hide resolved

randygaulmsft added 5 commits May 5, 2019 20:33

get tests in a good state

d16da4c

Got a basic subscribe test working.

b414595

Figured out way to subscribe to target SONiC_DB

e29cfa5

Wrapped up stream queries.

e597358

gofmt on all code under folder.

90d4437

randygaulmsft added 2 commits May 15, 2019 19:36

Remove dead code (comments).

ede7a94

SONiC_DB to SONIC_DB

f01f39c

jleveque reviewed May 17, 2019

View reviewed changes

sonic_data_client/db_client.go Outdated Show resolved Hide resolved

jleveque reviewed May 17, 2019

View reviewed changes

virtual_database_client/handler_pfcwd.go Outdated Show resolved Hide resolved

jleveque reviewed May 17, 2019

View reviewed changes

virtual_database_client/handler_queue_counter.go Outdated Show resolved Hide resolved

jleveque reviewed May 17, 2019

View reviewed changes

virtual_database_client/map_init.go Outdated Show resolved Hide resolved

jleveque suggested changes May 17, 2019

View reviewed changes

virtual_database_client/path.go Outdated Show resolved Hide resolved

virtual_database_client/path.go Outdated Show resolved Hide resolved

randygaulmsft added 2 commits May 17, 2019 05:33

Removed dead code.

d33fca6

Remove more dead code.

9f94324

hui-ma reviewed May 17, 2019

View reviewed changes

randygaulmsft added 2 commits May 17, 2019 21:23

sdc to vdc (use correct path for setting TCP redis settings)

6f2b220

Remove dead code from sonic_data_client.

53c0e70

Conversation

randygaulmsft commented Mar 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Information about the Numbered Pieces of the Above Diagram

Important Callstacks

Get

Subscribe (StreamRun)

Subscribe (PollRun)

NewDbClient

Running Tests

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hui-ma commented Apr 10, 2019

Uh oh!

hui-ma commented Apr 10, 2019

Uh oh!

randygaulmsft commented Apr 14, 2019

Uh oh!

jipanyang commented Apr 15, 2019

Uh oh!

qiluo-msft commented Apr 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jleveque commented Apr 19, 2019

Uh oh!

hui-ma left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hui-ma left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

randygaulmsft commented May 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

randygaulmsft commented May 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

randygaulmsft commented May 16, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jleveque left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

randygaulmsft commented May 17, 2019

Uh oh!

hui-ma commented May 17, 2019

Uh oh!

hui-ma commented May 17, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

randygaulmsft commented Mar 25, 2019 •

edited

Loading

qiluo-msft commented Apr 19, 2019 •

edited

Loading

randygaulmsft commented May 15, 2019 •

edited

Loading

randygaulmsft commented May 15, 2019 •

edited

Loading