Skip to content

[show] Add bgpraw to show run all #2537

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

Merged
qiluo-msft merged 9 commits into from
Jan 14, 2023
Merged

[show] Add bgpraw to show run all #2537

Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
be4aa05
[show] Add bgpraw to show run all
wen587 Dec 5, 2022
4bc7e7b
fix multiasic error
wen587 Dec 5, 2022
bcdfe21
change to rvtysh
wen587 Dec 5, 2022
80d63dc
add test
wen587 Dec 21, 2022
ed1cce3
resolve comment
wen587 Jan 4, 2023
a69380d
coverage
wen587 Jan 5, 2023
f5fc969
avoid shell=true
wen587 Jan 5, 2023
1070c59
fix comment
wen587 Jan 10, 2023
3c5b6f8
change to (stdout, rc); remove bgpraw check
wen587 Jan 10, 2023
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
7 changes: 6 additions & 1 deletion show/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1384,7 +1384,12 @@ def runningconfiguration():
def all(verbose):
"""Show full running configuration"""
cmd = "sonic-cfggen -d --print-data"
run_command(cmd, display_cmd=verbose)
output = json.loads(run_command(cmd, display_cmd=verbose, return_cmd=True))
Copy link
Contributor

@ganglyu ganglyu Dec 20, 2022

Choose a reason for hiding this comment

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

Should we check the output of run_command?

Copy link
Contributor Author

@wen587 wen587 Dec 20, 2022

Choose a reason for hiding this comment

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

You mean the format of output or bgpraw in output?

Copy link
Contributor

@ganglyu ganglyu Dec 21, 2022

Choose a reason for hiding this comment

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

geoffrey@vm-04:~$ python3
Python 3.6.9 (default, Nov 25 2022, 14:10:45)
[GCC 8.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.

import json
json.loads("")
Traceback (most recent call last):
File "", line 1, in
File "/usr/lib/python3.6/json/init.py", line 354, in loads
return _default_decoder.decode(s)
File "/usr/lib/python3.6/json/decoder.py", line 339, in decode
obj, end = self.raw_decode(s, idx=_w(s, 0).end())
File "/usr/lib/python3.6/json/decoder.py", line 357, in raw_decode
raise JSONDecodeError("Expecting value", s, err.value) from None
json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)

Copy link
Contributor Author

@wen587 wen587 Dec 21, 2022

Choose a reason for hiding this comment

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

Maybe that should be checked in sonic-cfggen?
sonic-cfggen guarantees that --print-data could be json.dump.

Copy link
Contributor

@qiluo-msft qiluo-msft Dec 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

display_cmd

run_command will internally call click.echo() or sys.exit(), so there will be side-effect on corner cases. #Closed

if not multi_asic.is_multi_asic():
bgpraw_cmd = "sudo {} -c 'show running-config'".format(constants.RVTYSH_COMMAND)
Copy link
Contributor

@qiluo-msft qiluo-msft Dec 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

sudo

Could you check if really need sudo? #Closed

Copy link
Contributor

@arlakshm arlakshm Dec 20, 2022

Choose a reason for hiding this comment

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

I think if we use rvtysh we dont need need sudo. I think if we use sudo then the user has have RW privilages

bgpraw = run_command(bgpraw_cmd, display_cmd=verbose, return_cmd=True)
Copy link
Contributor

@arlakshm arlakshm Dec 20, 2022

Choose a reason for hiding this comment

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

if the bgp container is down, bgpraw will contain error messge. Do we want to store that or make output['bgpraw'] empty?

Copy link
Contributor

@qiluo-msft qiluo-msft Dec 20, 2022

Choose a reason for hiding this comment

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

Empty running-config value for bgp down is a good suggestion.

Copy link
Contributor Author

@wen587 wen587 Dec 21, 2022

Choose a reason for hiding this comment

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

The run_command was calling subprocess. The output will be empty though subprocess calling is error.
So, the bgpraw will be empty if bgp container is down.

output['bgpraw'] = bgpraw
click.echo(json.dumps(output, indent=4))


# 'acl' subcommand ("show runningconfiguration acl")
Expand Down