-
Notifications
You must be signed in to change notification settings - Fork 740
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
Pdu and Console connection info in connection graph #2257
Conversation
ansible/files/sonic_lab_devices.csv
Outdated
@@ -1,5 +1,8 @@ | |||
Hostname,ManagementIp,HwSku,Type | |||
Hostname,ManagementIp,HwSku,Type,Protocol | |||
str-msn2700-01,10.251.0.188/23,Mellanox-2700,DevSonic | |||
str-7260-10,10.251.0.13/23,Arista-7260QX-64,FanoutLeaf | |||
str-7260-11,10.251.0.234/23,Arista-7260QX-64,FanoutRoot | |||
str-acs-serv-01,10.251.0.245/23,TestServ,Server |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For devices do not have protocol information, maybe an extra comma at the end of line is required to make these lines conform with the CSV headers? For example:
str-msn2700-01,10.251.0.188/23,Mellanox-2700,DevSonic,
str-7260-10,10.251.0.13/23,Arista-7260QX-64,FanoutLeaf,
str-7260-11,10.251.0.234/23,Arista-7260QX-64,FanoutRoot,
str-acs-serv-01,10.251.0.245/23,TestServ,Server,
This pull request introduces 6 alerts and fixes 1 when merging 6f0bbb49531c9ab57c98ce4bdd7fe050eb630a64 into 9b47bae - view on LGTM.com new alerts:
fixed alerts:
|
This pull request fixes 2 alerts when merging 44dedc1523f42f0aaea26d6950e5ec2cb8086ed1 into 9b47bae - view on LGTM.com fixed alerts:
|
This pull request fixes 2 alerts when merging 7435ec0e5e24d88c7b37a03c9baafbad4f74f104 into e8bfd88 - view on LGTM.com fixed alerts:
|
retest vsimage please |
@yxieca can you please review and approve this change if it's ok to merge? |
retest vsimage please |
retest this please |
retest vsimage please |
/Azurepipelines run |
Azure Pipelines successfully started running 1 pipeline(s). |
- add -i option to generate device/link/console/pdu file names. - allow graph file missing console/pdu information.
This pull request introduces 1 alert and fixes 3 when merging 5730c96 into e1d9265 - view on LGTM.com new alerts:
fixed alerts:
|
This pull request fixes 3 alerts when merging 395e903 into e1d9265 - view on LGTM.com fixed alerts:
|
This pull request fixes 3 alerts when merging 126a5e9 into 70c7a95 - view on LGTM.com fixed alerts:
|
This pull request fixes 3 alerts when merging fa92e99 into 70c7a95 - view on LGTM.com fixed alerts:
|
/Azurepipelines run |
Azure Pipelines successfully started running 1 pipeline(s). |
Description of PR
Summary:
Fixes # (issue)
Type of change
Approach
What is the motivation for this PR?
Add console/PDU link to the device connection graph.
How did you do it?
How did you verify/test it?
Tested with graph creation without specifying the console and pdu information.
Tested with instrumentation code (on graph generated before this change, and graph generated with this change but no console/pdu information, and graph with incremental console/pdu information):