sonic-telemetry
sonic-telemetry copied to clipboard
Add Virtual Database Client + Unit Tests
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.
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?
Add Wenda to verify PFC WD logic
@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.
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?
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
@qiluo-msft: @randygaulmsft is working on adding the PR check build.
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.
@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.
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
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.
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.
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