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

Jump to bottom

Revert "sonic-utilities: Update config reload() to verify formatting of an input file" #2586

Merged
merged 1 commit into from
Jan 7, 2023
Merged

Revert "sonic-utilities: Update config reload() to verify formatting of an input file" #2586

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 6 additions & 33 deletions config/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1189,17 +1189,6 @@ def load_backend_acl(cfg_db, device_type):
if os.path.isfile(BACKEND_ACL_FILE):
clicommon.run_command("acl-loader update incremental {}".format(BACKEND_ACL_FILE), display_cmd=True)

def validate_config_file(file):
"""
A validator to check config files for syntax errors
"""
try:
# Load golden config json
read_json_file(file)
except Exception as e:
click.secho("Bad format: json file '{}' broken.\n{}".format(file, str(e)),
fg='magenta')
sys.exit(1)

# This is our main entrypoint - the main 'config' command
@click.group(cls=clicommon.AbbreviationGroup, context_settings=CONTEXT_SETTINGS)
Expand Down Expand Up @@ -1578,8 +1567,10 @@ def reload(db, filename, yes, load_sysinfo, no_service_restart, force, file_form
click.echo("Input {} config file(s) separated by comma for multiple files ".format(num_cfg_file))
return

# Create a dictionary to store each cfg_file, namespace, and a bool representing if a the file exists
cfg_file_dict = {}
#Stop services before config push
if not no_service_restart:
log.log_info("'reload' stopping services...")
_stop_services()

# In Single ASIC platforms we have single DB service. In multi-ASIC platforms we have a global DB
# service running in the host + DB services running in each ASIC namespace created per ASIC.
Expand All @@ -1604,27 +1595,9 @@ def reload(db, filename, yes, load_sysinfo, no_service_restart, force, file_form
else:
file = DEFAULT_CONFIG_YANG_FILE

# Check if the file exists before proceeding
# Instead of exiting, skip the current namespace and check the next one
if not os.path.exists(file):
cfg_file_dict[inst] = [file, namespace, False]
continue
cfg_file_dict[inst] = [file, namespace, True]

# Check the file is properly formatted before proceeding.
validate_config_file(file)

#Validate INIT_CFG_FILE if it exits
if os.path.isfile(INIT_CFG_FILE):
validate_config_file(INIT_CFG_FILE)

#Stop services before config push
if not no_service_restart:
log.log_info("'reload' stopping services...")
_stop_services()

for file, namespace, file_exists in cfg_file_dict.values():
if not file_exists:
# Check the file exists before proceeding.
if not os.path.exists(file):
click.echo("The config file {} doesn't exist".format(file))
continue

Expand Down
62 changes: 0 additions & 62 deletions tests/config_reload_input/config_db_invalid.json
View file
Open in desktop

This file was deleted.

115 changes: 2 additions & 113 deletions tests/config_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import traceback
import json
import jsonpatch
import shutil
import sys
import unittest
import ipaddress
Expand Down Expand Up @@ -89,12 +88,6 @@
Reloading Monit configuration ...
"""

RELOAD_CONFIG_DB_OUTPUT_INVALID_MSG = """\
Bad format: json file"""

RELOAD_CONFIG_DB_OUTPUT_INVALID_ERROR = """\
Expecting ',' delimiter: line 12 column 5 (char 321)"""

RELOAD_YANG_CFG_OUTPUT = """\
Stopping SONiC target ...
Running command: /usr/local/bin/sonic-cfggen -Y /tmp/config.json --write-to-db
Expand All @@ -111,15 +104,6 @@
Reloading Monit configuration ...
"""

RELOAD_MASIC_CONFIG_DB_OUTPUT_FILE_NOT_EXIST = """\
Stopping SONiC target ...
Running command: /usr/local/bin/sonic-cfggen -j /tmp/config.json --write-to-db
The config file non_exist.json doesn't exist
Running command: /usr/local/bin/sonic-cfggen -j /tmp/config.json -n asic1 --write-to-db
Restarting SONiC target ...
Reloading Monit configuration ...
"""

reload_config_with_sys_info_command_output="""\
Running command: /usr/local/bin/sonic-cfggen -H -k Seastone-DX010-25-50 --write-to-db"""

Expand Down Expand Up @@ -211,7 +195,6 @@ def mock_run_command_side_effect_gnmi(*args, **kwargs):

class TestConfigReload(object):
dummy_cfg_file = os.path.join(os.sep, "tmp", "config.json")
dummy_cfg_file_contents = os.path.join(mock_db_path, "config_db.json")

@classmethod
def setup_class(cls):
Expand All @@ -223,8 +206,7 @@ def setup_class(cls):

import config.main
importlib.reload(config.main)
shutil.copyfile(cls.dummy_cfg_file_contents, cls.dummy_cfg_file)
open(cls.dummy_cfg_file, 'r').close()
open(cls.dummy_cfg_file, 'w').close()

def test_config_reload(self, get_cmd_module, setup_single_broadcom_asic):
with mock.patch("utilities_common.cli.run_command", mock.MagicMock(side_effect=mock_run_command_side_effect)) as mock_run_command:
Expand Down Expand Up @@ -497,17 +479,14 @@ def teardown_class(cls):

class TestReloadConfig(object):
dummy_cfg_file = os.path.join(os.sep, "tmp", "config.json")
dummy_cfg_file_contents = os.path.join(mock_db_path, "config_db.json")
dummy_cfg_file_invalid = os.path.join(mock_db_path, "config_db_invalid.json")

@classmethod
def setup_class(cls):
os.environ['UTILITIES_UNIT_TESTING'] = "1"
print("SETUP")
import config.main
importlib.reload(config.main)
shutil.copyfile(cls.dummy_cfg_file_contents, cls.dummy_cfg_file)
open(cls.dummy_cfg_file, 'r').close()
open(cls.dummy_cfg_file, 'w').close()

def test_reload_config(self, get_cmd_module, setup_single_broadcom_asic):
with mock.patch(
Expand All @@ -528,27 +507,6 @@ def test_reload_config(self, get_cmd_module, setup_single_broadcom_asic):
assert "\n".join([l.rstrip() for l in result.output.split('\n')]) \
== RELOAD_CONFIG_DB_OUTPUT

def test_reload_config_invalid_config_file(self, get_cmd_module, setup_single_broadcom_asic):
with mock.patch(
"utilities_common.cli.run_command",
mock.MagicMock(side_effect=mock_run_command_side_effect)
) as mock_run_command:
(config, show) = get_cmd_module
runner = CliRunner()

result = runner.invoke(
config.config.commands["reload"],
[self.dummy_cfg_file_invalid, '-y', '-f'])

print(result.exit_code)
print(result.output)
traceback.print_tb(result.exc_info[2])
assert result.exit_code == 1

output = "\n".join([l.rstrip() for l in result.output.split('\n')])
assert RELOAD_CONFIG_DB_OUTPUT_INVALID_MSG in output
assert RELOAD_CONFIG_DB_OUTPUT_INVALID_ERROR in output

def test_config_reload_disabled_service(self, get_cmd_module, setup_single_broadcom_asic):
with mock.patch(
"utilities_common.cli.run_command",
Expand Down Expand Up @@ -591,54 +549,6 @@ def test_reload_config_masic(self, get_cmd_module, setup_multi_broadcom_masic):
assert "\n".join([l.rstrip() for l in result.output.split('\n')]) \
== RELOAD_MASIC_CONFIG_DB_OUTPUT

def test_reload_config_masic_invalid(self, get_cmd_module, setup_multi_broadcom_masic):
with mock.patch(
"utilities_common.cli.run_command",
mock.MagicMock(side_effect=mock_run_command_side_effect)
) as mock_run_command:
(config, show) = get_cmd_module
runner = CliRunner()
# 3 config files: 1 for host and 2 for asic
cfg_files = "{},{},{}".format(
self.dummy_cfg_file,
self.dummy_cfg_file_invalid,
self.dummy_cfg_file)
result = runner.invoke(
config.config.commands["reload"],
[cfg_files, '-y', '-f'])

print(result.exit_code)
print(result.output)
traceback.print_tb(result.exc_info[2])
assert result.exit_code == 1

output = "\n".join([l.rstrip() for l in result.output.split('\n')])
assert RELOAD_CONFIG_DB_OUTPUT_INVALID_MSG in output
assert RELOAD_CONFIG_DB_OUTPUT_INVALID_ERROR in output

def test_reload_config_masic_non_exist_file(self, get_cmd_module, setup_multi_broadcom_masic):
with mock.patch(
"utilities_common.cli.run_command",
mock.MagicMock(side_effect=mock_run_command_side_effect)
) as mock_run_command:
(config, show) = get_cmd_module
runner = CliRunner()
# 3 config files: 1 for host and 2 for asic
cfg_files = "{},{},{}".format(
self.dummy_cfg_file,
"non_exist.json",
self.dummy_cfg_file)
result = runner.invoke(
config.config.commands["reload"],
[cfg_files, '-y', '-f'])

print(result.exit_code)
print(result.output)
traceback.print_tb(result.exc_info[2])
assert result.exit_code == 0
assert "\n".join([l.rstrip() for l in result.output.split('\n')]) \
== RELOAD_MASIC_CONFIG_DB_OUTPUT_FILE_NOT_EXIST

def test_reload_yang_config(self, get_cmd_module,
setup_single_broadcom_asic):
with mock.patch(
Expand All @@ -658,27 +568,6 @@ def test_reload_yang_config(self, get_cmd_module,
assert "\n".join([l.rstrip() for l in result.output.split('\n')]) \
== RELOAD_YANG_CFG_OUTPUT

def test_reload_yang_config_invalid(self, get_cmd_module,
setup_single_broadcom_asic):
with mock.patch(
"utilities_common.cli.run_command",
mock.MagicMock(side_effect=mock_run_command_side_effect)
) as mock_run_command:
(config, show) = get_cmd_module
runner = CliRunner()

result = runner.invoke(config.config.commands["reload"],
[self.dummy_cfg_file_invalid, '-y', '-f', '-t', 'config_yang'])

print(result.exit_code)
print(result.output)
traceback.print_tb(result.exc_info[2])
assert result.exit_code == 1

output = "\n".join([l.rstrip() for l in result.output.split('\n')])
assert RELOAD_CONFIG_DB_OUTPUT_INVALID_MSG in output
assert RELOAD_CONFIG_DB_OUTPUT_INVALID_ERROR in output

@classmethod
def teardown_class(cls):
os.environ['UTILITIES_UNIT_TESTING'] = "0"
Expand Down