Skip to content
Merged
Show file tree
Hide file tree
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
35 changes: 35 additions & 0 deletions config/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import time
import itertools
import copy
import tempfile

from jsonpatch import JsonPatchConflict
from jsonpointer import JsonPointerException
Expand Down Expand Up @@ -142,6 +143,14 @@ def read_json_file(fileName):
raise Exception(str(e))
return result

# write given JSON file
def write_json_file(json_input, fileName):
try:
with open(fileName, 'w') as f:
json.dump(json_input, f, indent=4)
except Exception as e:
raise Exception(str(e))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

except and raise immediately does not add value. It is the same as no except at all.


def _get_breakout_options(ctx, args, incomplete):
""" Provides dynamic mode option as per user argument i.e. interface name """
all_mode_options = []
Expand Down Expand Up @@ -1525,6 +1534,12 @@ def reload(db, filename, yes, load_sysinfo, no_service_restart, force, file_form
# Get the file from user input, else take the default file /etc/sonic/config_db{NS_id}.json
if cfg_files:
file = cfg_files[inst+1]
# Save to tmpfile in case of stdin input which can only be read once
if file == "/dev/stdin":
file_input = read_json_file(file)
(_, tmpfname) = tempfile.mkstemp(dir="/tmp", suffix="_configReloadStdin")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Look around the login inside this function, it seems one time reading of file will be enough. Let's take further effort to reorg the existing code and optimize the logic.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sonic-cfggen -j read it first. Then sonic-cfggen read it second.

write_json_file(file_input, tmpfname)
file = tmpfname
else:
if file_format == 'config_db':
if namespace is None:
Expand All @@ -1540,6 +1555,19 @@ def reload(db, filename, yes, load_sysinfo, no_service_restart, force, file_form
click.echo("The config file {} doesn't exist".format(file))
continue

if file_format == 'config_db':
file_input = read_json_file(file)

platform = file_input.get("DEVICE_METADATA", {}).\
get("localhost", {}).get("platform")
mac = file_input.get("DEVICE_METADATA", {}).\
get("localhost", {}).get("mac")

if not platform or not mac:
log.log_warning("Input file does't have platform or mac. platform: {}, mac: {}"
.format(None if platform is None else platform, None if mac is None else mac))
load_sysinfo = True

if load_sysinfo:
try:
command = [SONIC_CFGGEN_PATH, "-j", file, '-v', "DEVICE_METADATA.localhost.hwsku"]
Expand Down Expand Up @@ -1598,6 +1626,13 @@ def reload(db, filename, yes, load_sysinfo, no_service_restart, force, file_form
clicommon.run_command(command, display_cmd=True)
client.set(config_db.INIT_INDICATOR, 1)

if os.path.exists(file) and file.endswith("_configReloadStdin"):
# Remove tmpfile
try:
os.remove(file)
except OSError as e:
click.echo("An error occurred while removing the temporary file: {}".format(str(e)), err=True)

# Migrate DB contents to latest version
db_migrator='/usr/local/bin/db_migrator.py'
if os.path.isfile(db_migrator) and os.access(db_migrator, os.X_OK):
Expand Down
102 changes: 101 additions & 1 deletion tests/config_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import sys
import unittest
import ipaddress
import shutil
from unittest import mock
from jsonpatch import JsonPatchConflict

Expand Down Expand Up @@ -261,6 +262,46 @@ def test_config_reload(self, get_cmd_module, setup_single_broadcom_asic):

assert "\n".join([l.rstrip() for l in result.output.split('\n')][:1]) == reload_config_with_sys_info_command_output

def test_config_reload_stdin(self, get_cmd_module, setup_single_broadcom_asic):
def mock_json_load(f):
device_metadata = {
"DEVICE_METADATA": {
"localhost": {
"docker_routing_config_mode": "split",
"hostname": "sonic",
"hwsku": "Seastone-DX010-25-50",
"mac": "00:e0:ec:89:6e:48",
"platform": "x86_64-cel_seastone-r0",
"type": "ToRRouter"
}
}
}
return device_metadata
with mock.patch("utilities_common.cli.run_command", mock.MagicMock(side_effect=mock_run_command_side_effect)) as mock_run_command,\
mock.patch("json.load", mock.MagicMock(side_effect=mock_json_load)):
(config, show) = get_cmd_module

dev_stdin = "/dev/stdin"
jsonfile_init_cfg = os.path.join(mock_db_path, "init_cfg.json")

# create object
config.INIT_CFG_FILE = jsonfile_init_cfg

db = Db()
runner = CliRunner()
obj = {'config_db': db.cfgdb}

# simulate 'config reload' to provoke load_sys_info option
result = runner.invoke(config.config.commands["reload"], [dev_stdin, "-l", "-n", "-y"], obj=obj)

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')][:1]) == reload_config_with_sys_info_command_output

@classmethod
def teardown_class(cls):
print("TEARDOWN")
Expand Down Expand Up @@ -464,9 +505,66 @@ def setup_class(cls):
print("SETUP")
import config.main
importlib.reload(config.main)
open(cls.dummy_cfg_file, 'w').close()

def add_sysinfo_to_cfg_file(self):
with open(self.dummy_cfg_file, 'w') as f:
device_metadata = {
"DEVICE_METADATA": {
"localhost": {
"platform": "some_platform",
"mac": "02:42:f0:7f:01:05"
}
}
}
f.write(json.dumps(device_metadata))

def test_reload_config_invalid_input(self, get_cmd_module, setup_single_broadcom_asic):
open(self.dummy_cfg_file, 'w').close()
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, '-y', '-f'])

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

def test_reload_config_no_sysinfo(self, get_cmd_module, setup_single_broadcom_asic):
with open(self.dummy_cfg_file, 'w') as f:
device_metadata = {
"DEVICE_METADATA": {
"localhost": {
"hwsku": "some_hwsku"
}
}
}
f.write(json.dumps(device_metadata))

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, '-y', '-f'])

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

def test_reload_config(self, get_cmd_module, setup_single_broadcom_asic):
self.add_sysinfo_to_cfg_file()
with mock.patch(
"utilities_common.cli.run_command",
mock.MagicMock(side_effect=mock_run_command_side_effect)
Expand All @@ -486,6 +584,7 @@ def test_reload_config(self, get_cmd_module, setup_single_broadcom_asic):
== RELOAD_CONFIG_DB_OUTPUT

def test_config_reload_disabled_service(self, get_cmd_module, setup_single_broadcom_asic):
self.add_sysinfo_to_cfg_file()
with mock.patch(
"utilities_common.cli.run_command",
mock.MagicMock(side_effect=mock_run_command_side_effect_disabled_timer)
Expand All @@ -505,6 +604,7 @@ def test_config_reload_disabled_service(self, get_cmd_module, setup_single_broad
assert "\n".join([l.rstrip() for l in result.output.split('\n')]) == reload_config_with_disabled_service_output

def test_reload_config_masic(self, get_cmd_module, setup_multi_broadcom_masic):
self.add_sysinfo_to_cfg_file()
with mock.patch(
"utilities_common.cli.run_command",
mock.MagicMock(side_effect=mock_run_command_side_effect)
Expand Down