From 6784d9e946935f4754d164eed9cb870208a48768 Mon Sep 17 00:00:00 2001 From: maipbui Date: Tue, 28 Feb 2023 16:07:55 +0000 Subject: [PATCH 1/4] replace shell=True in barefoot Signed-off-by: maipbui --- show/plugins/barefoot.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/show/plugins/barefoot.py b/show/plugins/barefoot.py index bc80c47ba3..b6673e444b 100644 --- a/show/plugins/barefoot.py +++ b/show/plugins/barefoot.py @@ -4,6 +4,7 @@ import json import subprocess from sonic_py_common import device_info +from sonic_py_common.general import getstatusoutput_noshell_pipe, check_output_pipe @click.group() def barefoot(): @@ -25,8 +26,9 @@ def profile(): # Print current profile click.echo('Current profile: ', nl=False) - subprocess.run('docker exec -it syncd readlink /opt/bfn/install | sed ' - r's/install_\\\(.\*\\\)_profile/\\1/', check=True, shell=True) + cmd0 = ['docker', 'exec', '-it', 'syncd', 'readlink', '/opt/bfn/install'] + cmd1 = ['sed', r's/install_\\\(.\*\\\)_profile/\\1/'] + check_output_pipe(cmd0, cmd1) # Exclude current and unsupported profiles opts = '' @@ -37,9 +39,10 @@ def profile(): # Print profile list click.echo('Available profile(s):') - subprocess.run('docker exec -it syncd find /opt/bfn -mindepth 1 ' - r'-maxdepth 1 -type d -name install_\*_profile ' + opts + '| sed ' - r's%/opt/bfn/install_\\\(.\*\\\)_profile%\\1%', shell=True) + cmd0 = ['docker', 'exec', '-it', 'syncd', 'find', '/opt/bfn', '-mindepth', '1',\ + r'-maxdepth', '1', r'-type', 'd', r'-name', r'install_\*_profile', opts] + cmd1 = ["sed", r's%/opt/bfn/install_\\\(.\*\\\)_profile%\\1%'] + getstatusoutput_noshell_pipe(cmd0, cmd1) def register(cli): version_info = device_info.get_sonic_version_info() From c19958a90a44150c5ced1aebb349b6c609664318 Mon Sep 17 00:00:00 2001 From: maipbui Date: Wed, 1 Mar 2023 17:19:57 +0000 Subject: [PATCH 2/4] address PR comments Signed-off-by: maipbui --- show/plugins/barefoot.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/show/plugins/barefoot.py b/show/plugins/barefoot.py index b6673e444b..51d8ea159a 100644 --- a/show/plugins/barefoot.py +++ b/show/plugins/barefoot.py @@ -40,7 +40,7 @@ def profile(): # Print profile list click.echo('Available profile(s):') cmd0 = ['docker', 'exec', '-it', 'syncd', 'find', '/opt/bfn', '-mindepth', '1',\ - r'-maxdepth', '1', r'-type', 'd', r'-name', r'install_\*_profile', opts] + '-maxdepth', '1', '-type', 'd', '-name', r'install_\*_profile', opts] cmd1 = ["sed", r's%/opt/bfn/install_\\\(.\*\\\)_profile%\\1%'] getstatusoutput_noshell_pipe(cmd0, cmd1) From 1443f47027e65922e3ceaa5b5e253b1ec4932f79 Mon Sep 17 00:00:00 2001 From: maipbui Date: Wed, 8 Mar 2023 16:48:58 +0000 Subject: [PATCH 3/4] use getstatusoutput for both Signed-off-by: maipbui --- show/plugins/barefoot.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/show/plugins/barefoot.py b/show/plugins/barefoot.py index 51d8ea159a..07255c1868 100644 --- a/show/plugins/barefoot.py +++ b/show/plugins/barefoot.py @@ -4,7 +4,7 @@ import json import subprocess from sonic_py_common import device_info -from sonic_py_common.general import getstatusoutput_noshell_pipe, check_output_pipe +from sonic_py_common.general import getstatusoutput_noshell_pipe @click.group() def barefoot(): @@ -28,7 +28,7 @@ def profile(): click.echo('Current profile: ', nl=False) cmd0 = ['docker', 'exec', '-it', 'syncd', 'readlink', '/opt/bfn/install'] cmd1 = ['sed', r's/install_\\\(.\*\\\)_profile/\\1/'] - check_output_pipe(cmd0, cmd1) + getstatusoutput_noshell_pipe(cmd0, cmd1) # Exclude current and unsupported profiles opts = '' From 021956b3bf6fe4332264d42966b3b3c0f3dbcd3c Mon Sep 17 00:00:00 2001 From: Mai Bui Date: Thu, 20 Apr 2023 00:31:12 +0000 Subject: [PATCH 4/4] add UT Signed-off-by: Mai Bui --- show/plugins/barefoot.py | 4 +-- tests/show_barefoot_test.py | 53 +++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 tests/show_barefoot_test.py diff --git a/show/plugins/barefoot.py b/show/plugins/barefoot.py index 07255c1868..2afdb566ed 100644 --- a/show/plugins/barefoot.py +++ b/show/plugins/barefoot.py @@ -33,9 +33,9 @@ def profile(): # Exclude current and unsupported profiles opts = '' if chip_family == 'tofino': - opts = r'\! -name install_y\*_profile ' + opts = r'\! -name install_y\*_profile' elif chip_family == 'tofino2': - opts = r'\! -name install_x\*_profile ' + opts = r'\! -name install_x\*_profile' # Print profile list click.echo('Available profile(s):') diff --git a/tests/show_barefoot_test.py b/tests/show_barefoot_test.py new file mode 100644 index 0000000000..1ec6f17616 --- /dev/null +++ b/tests/show_barefoot_test.py @@ -0,0 +1,53 @@ +import json +import click +import pytest +from click.testing import CliRunner +import show.plugins.barefoot as show +from unittest.mock import call, patch, mock_open, MagicMock + + +class TestShowBarefoot(object): + def setup(self): + print('SETUP') + + @patch('subprocess.run') + def test_default_profile(self, mock_run): + mock_run.return_value.returncode = 1 + runner = CliRunner() + result = runner.invoke(show.profile, []) + assert result.exit_code == 0 + assert result.output == 'Current profile: default\n' + mock_run.assert_called_once_with(['docker', 'exec', '-it', 'syncd', 'test', '-h', '/opt/bfn/install']) + + @patch('show.plugins.barefoot.getstatusoutput_noshell_pipe') + @patch('show.plugins.barefoot.device_info.get_path_to_hwsku_dir', MagicMock(return_value='/usr/share/sonic/hwsku_dir')) + @patch('subprocess.run') + def test_nondefault_profile(self, mock_run, mock_cmd): + mock_run.return_value.returncode = 0 + chip_list = [{'chip_family': 'TOFINO'}] + mock_open_args = mock_open(read_data=json.dumps({'chip_list': chip_list})) + expected_calls = [ + call( + ['docker', 'exec', '-it', 'syncd', 'readlink', '/opt/bfn/install'], + ['sed', 's/install_\\\\\\(.\\*\\\\\\)_profile/\\\\1/'] + ), + + call( + ['docker', 'exec', '-it', 'syncd', 'find', '/opt/bfn', '-mindepth', '1',\ + '-maxdepth', '1', '-type', 'd', '-name', r'install_\*_profile', r'\! -name install_y\*_profile'], + ["sed", r's%/opt/bfn/install_\\\(.\*\\\)_profile%\\1%'] + ) + ] + + with patch("builtins.open", mock_open_args) as mock_open_file: + runner = CliRunner() + result = runner.invoke(show.profile) + assert result.exit_code == 0 + + mock_run.assert_called_once_with(['docker', 'exec', '-it', 'syncd', 'test', '-h', '/opt/bfn/install']) + mock_open_file.assert_called_once_with('/usr/share/sonic/hwsku_dir/switch-tna-sai.conf') + assert mock_cmd.call_args_list == expected_calls + + def teardown(self): + print('TEARDOWN') +