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

[broadcom] Add broadcom ASIC info to "show version" click command #3621

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
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
39 changes: 39 additions & 0 deletions show/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -1480,6 +1480,18 @@ def version(verbose):
platform_info = device_info.get_platform_info()
chassis_info = platform.get_chassis_info()

if platform_info['asic_type'] == 'broadcom':
asic_info = _get_broadcom_info()
asic_info_str = []
asic_info_str.extend(
# NOTE: No ':' because that is in the output as well.
["ASIC API {}".format(line) for line in asic_info['versions']])
asic_info_str.extend(
["ASIC Model: {}".format(line) for line in asic_info['units']])
asic_info_str = '\n'.join(asic_info_str)
else:
asic_info = None

sys_uptime_cmd = ["uptime"]
sys_uptime = subprocess.Popen(sys_uptime_cmd, text=True, stdout=subprocess.PIPE)

Expand All @@ -1496,6 +1508,8 @@ def version(verbose):
click.echo("HwSKU: {}".format(platform_info['hwsku']))
click.echo("ASIC: {}".format(platform_info['asic_type']))
click.echo("ASIC Count: {}".format(platform_info['asic_count']))
if asic_info:
Copy link
Contributor

Choose a reason for hiding this comment

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

would this be disruptive for some test cases if adding output in the middle of show version cli?

Copy link
Author

Choose a reason for hiding this comment

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

Legit question.

If I look at tests/show_platform_test.py, it should not change (for now) because it has a mellanox ASIC_TYPE and not a broadcom one.

My reason for placing it in the middle was this: the top info can then be copy-pasted into bug reports without including machine-specific serial numbers.

If you want, I can add a test there for the broadcom model with this output included.

Copy link
Author

Choose a reason for hiding this comment

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

B.t.w. I'd rather fetch the info once at initialization from the ASIC and get it from a state(?) db during show version, but that's probably more invasive.

Copy link
Contributor

Choose a reason for hiding this comment

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

my thought is better to keep output backward compatibility, since we are not sure if there's other customize scripts would be broken.

Copy link
Author

Choose a reason for hiding this comment

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

Ok ok.

So, after the second paragraph and before the docker image list then?

click.echo(asic_info_str)
click.echo("Serial Number: {}".format(chassis_info['serial']))
click.echo("Model Number: {}".format(chassis_info['model']))
click.echo("Hardware Revision: {}".format(chassis_info['revision']))
Expand All @@ -1506,6 +1520,31 @@ def version(verbose):
p = subprocess.Popen(cmd, text=True, stdout=subprocess.PIPE)
click.echo(p.stdout.read())


def _get_broadcom_info():
def _bcmcmd(command):
try:
res = subprocess.check_output(
# NOTE: There's also -n $UNIT in the bcmcmd shell script.
['bcmcmd', '-t', '1', command],
stderr=subprocess.DEVNULL, text=True)
except (FileNotFoundError, PermissionError, subprocess.CalledProcessError) as e:
res = str(e)
lines = [
line.strip() for line in res.split('\n')
if line.strip() not in (
'', command, 'drivshell>')]
res = '\n'.join(lines)
return res

ret = {
# Slow: these calls easily take 2x300ms.
'versions': _bcmcmd('bsv').replace('\n', ' ').split(', '),
'units': _bcmcmd('show unit').split('\n'),
# 'monitor_version': _bcmcmd('ver'),
}
return ret

#
# 'environment' command ("show environment")
#
Expand Down
Loading