Skip to content

Commit

Permalink
[config] Fix 'config reload -l' command to get filename by default (s…
Browse files Browse the repository at this point in the history
…onic-net#1611)

Fix sonic-net#7433

Right now config reload -l is getting failed due to an error.
I guess the problem is here in sonic-utilities repo. If user does not provide filename with config reload -l, command = "{} -j {} -v DEVICE_METADATA.localhost.hwsku".format(SONIC_CFGGEN_PATH, filename) will not provide cfg_hwsku i.e. hwsku parameter as it should which will later cause problem here command = "{} -H -k {} --write-to-db".format(SONIC_CFGGEN_PATH, cfg_hwsku) as hwsku is not available around that time.

that's why we notice errors like No such file or directory: 'None' as pasted in this issue.

- How I did it
To Fix the issue, moved the part where the code gets cfg_hwsku command = "{} -j {} -v DEVICE_METADATA.localhost.hwsku".format(SONIC_CFGGEN_PATH, file) to the same location it needed as we get filename by default.

- How to verify it
'sudo config reload -l'
Added test cases.

Signed-off-by: Sangita Maity <samaity@linkedin.com>
  • Loading branch information
Sangita Maity authored Dec 23, 2021
1 parent ed2fa69 commit 1a2a9a3
Show file tree
Hide file tree
Showing 4 changed files with 221 additions and 10 deletions.
29 changes: 19 additions & 10 deletions config/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -1376,16 +1376,6 @@ def reload(db, filename, yes, load_sysinfo, no_service_restart, disable_arp_cach
click.echo("Input {} config file(s) separated by comma for multiple files ".format(num_cfg_file))
return

if load_sysinfo:
command = "{} -j {} -v DEVICE_METADATA.localhost.hwsku".format(SONIC_CFGGEN_PATH, filename)
proc = subprocess.Popen(command, shell=True, text=True, stdout=subprocess.PIPE)
cfg_hwsku, err = proc.communicate()
if err:
click.echo("Could not get the HWSKU from config file, exiting")
sys.exit(1)
else:
cfg_hwsku = cfg_hwsku.strip()

# For dual ToR devices, cache ARP and FDB info
localhost_metadata = db.cfgdb.get_table('DEVICE_METADATA')['localhost']
cache_arp_table = not disable_arp_cache and 'subtype' in localhost_metadata and localhost_metadata['subtype'].lower() == 'dualtor'
Expand Down Expand Up @@ -1427,6 +1417,25 @@ def reload(db, filename, yes, load_sysinfo, no_service_restart, disable_arp_cach
click.echo("The config file {} doesn't exist".format(file))
continue

if load_sysinfo:
try:
command = "{} -j {} -v DEVICE_METADATA.localhost.hwsku".format(SONIC_CFGGEN_PATH, file)
proc = subprocess.Popen(command, shell=True, text=True, stdout=subprocess.PIPE)
output, err = proc.communicate()

except FileNotFoundError as e:
click.echo("{}".format(str(e)), err=True)
raise click.Abort()
except Exception as e:
click.echo("{}\n{}".format(type(e), str(e)), err=True)
raise click.Abort()

if not output:
click.secho("Could not get the HWSKU from config file, Exiting!!!", fg='magenta')
sys.exit(1)

cfg_hwsku = output.strip()

if namespace is None:
config_db = ConfigDBConnector()
else:
Expand Down
62 changes: 62 additions & 0 deletions tests/config_reload_input/config_db.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
{
"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"
}
},
"VLAN_MEMBER": {
"Vlan1000|Ethernet0": {
"tagging_mode": "untagged"
},
"Vlan1000|Ethernet4": {
"tagging_mode": "untagged"
},
"Vlan1000|Ethernet8": {
"tagging_mode": "untagged"
}
},
"VLAN": {
"Vlan1000": {
"vlanid": "1000",
"dhcp_servers": [
"192.0.0.1",
"192.0.0.2",
"192.0.0.3",
"192.0.0.4"
]
}
},
"PORT": {
"Ethernet0": {
"alias": "Eth1",
"lanes": "65, 66, 67, 68",
"description": "Ethernet0 100G link",
"speed": "100000"
},
"Ethernet4": {
"admin_status": "up",
"alias": "fortyGigE0/4",
"description": "Servers0:eth0",
"index": "1",
"lanes": "29,30,31,32",
"mtu": "9100",
"pfc_asym": "off",
"speed": "40000"
},
"Ethernet8": {
"admin_status": "up",
"alias": "fortyGigE0/8",
"description": "Servers1:eth0",
"index": "2",
"lanes": "33,34,35,36",
"mtu": "9100",
"pfc_asym": "off",
"speed": "40000"
}
}
}
68 changes: 68 additions & 0 deletions tests/config_reload_input/init_cfg.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
{
"DEVICE_METADATA": {
"localhost": {
"buffer_model": "traditional",
"default_bgp_status": "up",
"default_pfcwd_status": "disable"
}
},
"CRM": {
"Config": {
"polling_interval": "300",
"ipv4_route_threshold_type": "percentage",
"ipv4_route_low_threshold": "70",
"ipv4_route_high_threshold": "85",
"ipv6_route_threshold_type": "percentage",
"ipv6_route_low_threshold": "70",
"ipv6_route_high_threshold": "85",
"ipv4_nexthop_threshold_type": "percentage",
"ipv4_nexthop_low_threshold": "70",
"ipv4_nexthop_high_threshold": "85",
"ipv6_nexthop_threshold_type": "percentage",
"ipv6_nexthop_low_threshold": "70",
"ipv6_nexthop_high_threshold": "85",
"ipv4_neighbor_threshold_type": "percentage",
"ipv4_neighbor_low_threshold": "70",
"ipv4_neighbor_high_threshold": "85",
"ipv6_neighbor_threshold_type": "percentage",
"ipv6_neighbor_low_threshold": "70",
"ipv6_neighbor_high_threshold": "85",
"nexthop_group_member_threshold_type": "percentage",
"nexthop_group_member_low_threshold": "70",
"nexthop_group_member_high_threshold": "85",
"nexthop_group_threshold_type": "percentage",
"nexthop_group_low_threshold": "70",
"nexthop_group_high_threshold": "85",
"acl_table_threshold_type": "percentage",
"acl_table_low_threshold": "70",
"acl_table_high_threshold": "85",
"acl_group_threshold_type": "percentage",
"acl_group_low_threshold": "70",
"acl_group_high_threshold": "85",
"acl_entry_threshold_type": "percentage",
"acl_entry_low_threshold": "70",
"acl_entry_high_threshold": "85",
"acl_counter_threshold_type": "percentage",
"acl_counter_low_threshold": "70",
"acl_counter_high_threshold": "85",
"fdb_entry_threshold_type": "percentage",
"fdb_entry_low_threshold": "70",
"fdb_entry_high_threshold": "85",
"snat_entry_threshold_type": "percentage",
"snat_entry_low_threshold": "70",
"snat_entry_high_threshold": "85",
"dnat_entry_threshold_type": "percentage",
"dnat_entry_low_threshold": "70",
"dnat_entry_high_threshold": "85",
"ipmc_entry_threshold_type": "percentage",
"ipmc_entry_low_threshold": "70",
"ipmc_entry_high_threshold": "85",
"mpls_inseg_threshold_type": "percentage",
"mpls_inseg_low_threshold": "70",
"mpls_inseg_high_threshold": "85",
"mpls_nexthop_threshold_type": "percentage",
"mpls_nexthop_low_threshold": "70",
"mpls_nexthop_high_threshold": "85"
}
}
}
72 changes: 72 additions & 0 deletions tests/config_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,25 @@

from sonic_py_common import device_info
from utilities_common.db import Db
from utilities_common.general import load_module_from_source

from generic_config_updater.generic_updater import ConfigFormat

import config.main as config

# Add Test, module and script path.
test_path = os.path.dirname(os.path.abspath(__file__))
modules_path = os.path.dirname(test_path)
scripts_path = os.path.join(modules_path, "scripts")
sys.path.insert(0, test_path)
sys.path.insert(0, modules_path)
sys.path.insert(0, scripts_path)
os.environ["PATH"] += os.pathsep + scripts_path

# Config Reload input Path
mock_db_path = os.path.join(test_path, "config_reload_input")


load_minigraph_command_output="""\
Stopping SONiC target ...
Running command: /usr/local/bin/sonic-cfggen -H -m --write-to-db
Expand Down Expand Up @@ -55,6 +69,10 @@
Reloading Monit configuration ...
"""

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

def mock_run_command_side_effect(*args, **kwargs):
command = args[0]

Expand All @@ -70,6 +88,60 @@ def mock_run_command_side_effect(*args, **kwargs):
return ''


# Load sonic-cfggen from source since /usr/local/bin/sonic-cfggen does not have .py extension.
sonic_cfggen = load_module_from_source('sonic_cfggen', '/usr/local/bin/sonic-cfggen')


class TestConfigReload(object):
@classmethod
def setup_class(cls):
os.environ['UTILITIES_UNIT_TESTING'] = "1"
print("SETUP")

from .mock_tables import mock_single_asic
importlib.reload(mock_single_asic)

import config.main
importlib.reload(config.main)

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:
(config, show) = get_cmd_module

jsonfile_config = os.path.join(mock_db_path, "config_db.json")
jsonfile_init_cfg = os.path.join(mock_db_path, "init_cfg.json")

# create object
config.INIT_CFG_FILE = jsonfile_init_cfg
config.DEFAULT_CONFIG_DB_FILE = jsonfile_config

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"], ["-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')][:2]) == reload_config_with_sys_info_command_output

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

# change back to single asic config
from .mock_tables import dbconnector
from .mock_tables import mock_single_asic
importlib.reload(mock_single_asic)
dbconnector.load_namespace_config()


class TestLoadMinigraph(object):
@classmethod
def setup_class(cls):
Expand Down

0 comments on commit 1a2a9a3

Please sign in to comment.