Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial commit for sonic-gnmi. #3

Closed
wants to merge 25 commits into from
Closed

Initial commit for sonic-gnmi. #3

wants to merge 25 commits into from

Conversation

ganglyu
Copy link
Contributor

@ganglyu ganglyu commented Jun 18, 2022

Signed-off-by: Gang Lv ganglv@microsoft.com

Why I did it

This is the initial commit for sonic-gnmi.
HLD is under review, sonic-net/SONiC#996

How I did it

Modify sonic-telemetry to support incremental configuration and full configuration.

How to verify it

Run unit test for sonic-gnmi

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

Signed-off-by: Gang Lv ganglv@microsoft.com
Makefile Outdated Show resolved Hide resolved
common_utils/context.go Outdated Show resolved Hide resolved
Makefile Show resolved Hide resolved
@ganglyu ganglyu requested a review from qiluo-msft June 23, 2022 13:21
gnmi_dump/gnmi_dump.go Outdated Show resolved Hide resolved
gnmi_server/gnoi.go Outdated Show resolved Hide resolved
gnmi_server/server.go Outdated Show resolved Hide resolved
patches/apply.sh Outdated Show resolved Hide resolved
patches/glog.patch Outdated Show resolved Hide resolved
@@ -52,7 +52,16 @@
pbPath, err := xpath.ToGNMIPath(pathValuePair[0])
if err != nil {
log.Exitf("error in parsing xpath %q to gnmi path", pathValuePair[0])
@@ -144,8 +166,10 @@
}
var pbVal *pb.TypedValue
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TypedValue

Why we need this patch?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This patch introduced jwt token, and also modified path schema.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we use # in path?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we use # in path?

For example, update path is /a/b/c:xyz, and then /a/b/c is path, and xyz is path value.
If the first char of path value is '@', value is a file name.
If the first char is '#', value is nil.
It's possible to use special character in path, but I don't think we use '#' as the first char in path.

var (
memKey = 7749
memSize = 1024
memMode = 01600
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

memMode

Add comment

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You did not mention "memMode" in above comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have updated the comment for memMode.

)

var (
memKey = 7749
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

memKey

Add comment? what is the key?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it a shared credential between server and gnmi_dump?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

@ganglyu ganglyu requested a review from qiluo-msft June 24, 2022 03:35

package common_utils

const GNMI_WORK_PATH = "/etc/sonic/gnmi"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/etc/sonic/gnmi

/etc is for config files. Working path may fit more under /var/run.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both host and gNMI container can access /etc/sonic. So gNMI server can create a patch file in this path, and notify host to run “config apply-patch" with this patch file.
Do you think we should create a working path under /var/run, and share this path between host and gNMI container?

renukamanavalan pushed a commit that referenced this pull request Sep 1, 2022
@zhangyanzhao
Copy link

@qiluo-msft would you please review and approve this PR?
@ganglyu the build conflict need be fixed.

@ganglyu ganglyu closed this Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants