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.

Sign up for GitHub

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

[minigraph.py] Generate items needed by streaming telemetry service #3759

Merged
merged 3 commits into from
Nov 21, 2019
Merged

[minigraph.py] Generate items needed by streaming telemetry service #3759

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8fa5ef5
adding telemetry keys to minigraph
pra-moh Nov 15, 2019
ea42c67
adding indent to line
pra-moh Nov 21, 2019
effb808
adding space between colon and opening brace
pra-moh Nov 21, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion src/sonic-config-engine/minigraph.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -627,7 +627,13 @@ def parse_xml(filename, platform=None, port_config_file=None):
'hostname': hostname,
'hwsku': hwsku,
'type': current_device['type']
}}
},
'x509': {
'server_crt': '/etc/sonic/telemetry/streamingtelemetryserver.cer',
'server_key': '/etc/sonic/telemetry/streamingtelemetryclient.key',
'ca_crt': '/etc/sonic/telemetry/dsmsroot.cer'
Copy link
Contributor

@jleveque jleveque Nov 15, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

What if we need other certificates on the SONiC box? Maybe the keys are too generic: "server_crt", "server_key", "ca_crt". Maybe we need to prefix them with the component they're used for, e.g., "telemetry_server_crt", "telemetry_server_key", "telemetry_ca_crt", etc.

Or maybe this should be moved out of the DEVICE_METADATA table and into the TELEMETRY table below?

@lguohan: What is your opinion here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi Joe, such keys are used in telemetry contrainers and already defined:
https://github.com/Azure/sonic-buildimage/blob/master/dockers/docker-sonic-telemetry/telemetry.sh

If we need change, we need change this place as well. It will break other partner using this feature now, if we device to use other key names.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah. Good to know.

@lguohan: Should we merge this as-is and deal with making changes to support other certificates if and when we need them?

Copy link
Contributor

Choose a reason for hiding this comment

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

@hui-ma: I think we can go ahead and merge this as-is for now, but I think we need to go back and think about a way to restructure this in order for SONiC to manage multiple keys/certs.

Copy link
Contributor

Choose a reason for hiding this comment

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

sounds good to me

}
}
results['BGP_NEIGHBOR'] = bgp_sessions
results['BGP_MONITORS'] = bgp_monitors
results['BGP_PEER_RANGE'] = bgp_peers_with_range
Expand Down Expand Up @@ -806,6 +812,13 @@ def parse_xml(filename, platform=None, port_config_file=None):
'status': 'enabled'
}
}
results['TELEMETRY'] = {
'gnmi':{
'client_auth': 'true',
'port': '50051',
'log_level': '2'
}
}

# Do not configure the minigraph's mirror session, which is currently unused
# mirror_sessions = {}
Expand Down