Skip to content

Commit

Permalink
[config] config reload should generate sysinfo if missing (sonic-net#…
Browse files Browse the repository at this point in the history
…3031)

What I did
Missing platform and mac in CONFIG_DB will result in container failure. We should make the config reload generate those info if missing.

How I did it
Add missing sys info if config_db.json doesn't contain it.

How to verify it
Unit test
  • Loading branch information
wen587 authored Nov 2, 2023
1 parent e01fc89 commit a4eeb69
Show file tree
Hide file tree
Showing 2 changed files with 136 additions and 1 deletion.
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))

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")
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

0 comments on commit a4eeb69

Please sign in to comment.