Skip to content

[sonic-utilities][sonic-py-common] Move logic to get port config file path to sonic-py-common and update sonic-utilities to comply #5264

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

Merged
vdahiya12 merged 18 commits into from
Sep 4, 2020
Merged

[sonic-utilities][sonic-py-common] Move logic to get port config file path to sonic-py-common and update sonic-utilities to comply #5264

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
c7a9f2f
[sonic-utilities]update submodule with fix
vdahiya12 Aug 27, 2020
9bf9aed
[sonic-utilities] Update submodule
vdahiya12 Aug 27, 2020
9b0ae79
[sonic-py-common] address comments
vdahiya12 Aug 27, 2020
ab2f8a3
[sonic-py-common] address comments
vdahiya12 Aug 28, 2020
d9c7acc
[sonic-py-common] address comments
vdahiya12 Aug 28, 2020
bd27d97
[sonic-config-engine] address comments
vdahiya12 Aug 30, 2020
9841702
[sonic-py-common] fixing the build
vdahiya12 Sep 2, 2020
1a1c126
[sonic-py-common] fix build errors
vdahiya12 Sep 2, 2020
1b67ff8
[sonic-py-common] changes to make code simpler
vdahiya12 Sep 2, 2020
e131ead
[sonic-py-common] fix the coments
vdahiya12 Sep 2, 2020
e18a0c7
[sonic-py-common] fix the build/add more logic
vdahiya12 Sep 4, 2020
e1d34d8
[sonic-py-common] fix the args passing
vdahiya12 Sep 4, 2020
38a0bb8
[sonic-py-common] fixing the logic
vdahiya12 Sep 4, 2020
2607b9f
[sonic-py-common] fixing the logic
vdahiya12 Sep 4, 2020
db4149e
[sonic-py-common] fixed the logic
vdahiya12 Sep 4, 2020
de5fda7
[sonic-py-common] adding the function back
vdahiya12 Sep 4, 2020
8988996
[sonic_py_common] refactoring the code
vdahiya12 Sep 4, 2020
8047e8a
[sonic-py-common] fixing the comment format
vdahiya12 Sep 4, 2020
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
32 changes: 3 additions & 29 deletions src/sonic-config-engine/portconfig.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import re
from collections import OrderedDict
from swsssdk import ConfigDBConnector
from sonic_py_common import device_info
except ImportError as e:
raise ImportError("%s - required module not found" % str(e))

Expand Down Expand Up @@ -64,33 +65,6 @@ def db_connect_configdb():
config_db = None
return config_db

def get_port_config_file_name(hwsku=None, platform=None, asic=None):

# check 'platform.json' file presence
port_config_candidates_Json = []
port_config_candidates_Json.append(os.path.join(PLATFORM_ROOT_PATH_DOCKER, PLATFORM_JSON))
if platform:
port_config_candidates_Json.append(os.path.join(PLATFORM_ROOT_PATH, platform, PLATFORM_JSON))

# check 'portconfig.ini' file presence
port_config_candidates = []
port_config_candidates.append(os.path.join(HWSKU_ROOT_PATH, PORT_CONFIG_INI))
if hwsku:
if platform:
if asic:
port_config_candidates.append(os.path.join(PLATFORM_ROOT_PATH, platform, hwsku, asic, PORT_CONFIG_INI))
port_config_candidates.append(os.path.join(PLATFORM_ROOT_PATH, platform, hwsku, PORT_CONFIG_INI))
port_config_candidates.append(os.path.join(PLATFORM_ROOT_PATH_DOCKER, hwsku, PORT_CONFIG_INI))
port_config_candidates.append(os.path.join(SONIC_ROOT_PATH, hwsku, PORT_CONFIG_INI))

elif platform and not hwsku:
port_config_candidates.append(os.path.join(PLATFORM_ROOT_PATH, platform, PORT_CONFIG_INI))

for candidate in port_config_candidates_Json + port_config_candidates:
if os.path.isfile(candidate):
return candidate
return None

def get_hwsku_file_name(hwsku=None, platform=None):
hwsku_candidates_Json = []
hwsku_candidates_Json.append(os.path.join(HWSKU_ROOT_PATH, HWSKU_JSON))
Expand Down Expand Up @@ -119,7 +93,7 @@ def get_port_config(hwsku=None, platform=None, port_config_file=None, hwsku_conf
return (ports, port_alias_map, port_alias_asic_map)

if not port_config_file:
port_config_file = get_port_config_file_name(hwsku, platform, asic)
port_config_file = device_info.get_path_to_port_config_file(asic)
if not port_config_file:
return ({}, {}, {})

Expand Down Expand Up @@ -289,7 +263,7 @@ def parse_platform_json_file(hwsku_json_file, platform_json_file):

def get_breakout_mode(hwsku=None, platform=None, port_config_file=None):
if not port_config_file:
port_config_file = get_port_config_file_name(hwsku, platform)
port_config_file = device_info.get_path_to_port_config_file()
if not port_config_file:
return None
if port_config_file.endswith('.json'):
Expand Down
5 changes: 3 additions & 2 deletions src/sonic-config-engine/sonic-cfggen
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,10 @@ from minigraph import minigraph_encoder
from minigraph import parse_xml
from minigraph import parse_device_desc_xml
from minigraph import parse_asic_sub_role
from portconfig import get_port_config, get_port_config_file_name, get_breakout_mode
from portconfig import get_port_config, get_breakout_mode
from sonic_py_common.device_info import get_platform, get_system_mac
from sonic_py_common.multi_asic import get_asic_id_from_name, is_multi_asic
from sonic_py_common import device_info
from config_samples import generate_sample_config
from config_samples import get_available_config
from swsssdk import SonicV2Connector, ConfigDBConnector, SonicDBConfig, ConfigDBPipeConnector
Expand Down Expand Up @@ -301,7 +302,7 @@ def main():
}}}
deep_update(data, hardware_data)
if args.port_config is None:
args.port_config = get_port_config_file_name(hwsku, platform)
args.port_config = device_info.get_path_to_port_config_file()
(ports, _, _) = get_port_config(hwsku, platform, args.port_config, asic_id)
if not ports:
print('Failed to get port config', file=sys.stderr)
Expand Down
26 changes: 15 additions & 11 deletions src/sonic-py-common/sonic_py_common/device_info.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,27 +181,31 @@ def get_paths_to_platform_and_hwsku_dirs():

return (platform_path, hwsku_path)


def get_path_to_port_config_file():
def get_path_to_port_config_file(asic=None):
"""
Retrieves the path to the device's port configuration file

Returns:
A string containing the path the the device's port configuration file
"""
# Get platform and hwsku path
(platform_path, hwsku_path) = get_paths_to_platform_and_hwsku_dirs()

# First check for the presence of the new 'platform.json' file
port_config_file_path = os.path.join(platform_path, PLATFORM_JSON_FILE)
if not os.path.isfile(port_config_file_path):
# platform.json doesn't exist. Try loading the legacy 'port_config.ini' file
port_config_file_path = os.path.join(hwsku_path, PORT_CONFIG_FILE)
if not os.path.isfile(port_config_file_path):
raise OSError("Failed to detect port config file: {}".format(port_config_file_path))
port_config_candidates = []

# Check for 'platform.json' file presence first
port_config_candidates.append(os.path.join(platform_path, PLATFORM_JSON))

return port_config_file_path
# Check for 'port_config.ini' file presence in a few locations
port_config_candidates.append(os.path.join(hwsku_path, PORT_CONFIG_INI))
if asic:
port_config_candidates.append(os.path.join(hwsku_path, asic, PORT_CONFIG_INI))
port_config_candidates.append(os.path.join(platform_path, PORT_CONFIG_INI))
Copy link
Copy Markdown
Contributor

@jleveque jleveque Aug 28, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I believe we should be able to simplify this as follows, but will wait for input from @samaity and @SuvarnaMeenakshi in case recent changes have altered this:

    if asic:
        port_config_candidates.append(os.path.join(hwsku_path, asic, PORT_CONFIG_INI))
    else:
        port_config_candidates.append(os.path.join(hwsku_path, PORT_CONFIG_INI))

Copy link
Copy Markdown
Contributor

@SuvarnaMeenakshi SuvarnaMeenakshi Aug 29, 2020

Choose a reason for hiding this comment

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

Yes, the above suggested simplification looks good. If asic is defined, then platform/hwsku/port_config.ini will not be present as per current design.
In the previous code, I do not see port_config_candidates.append(os.path.join(platform_path, PORT_CONFIG_INI)). being directly used, and is used only if hwsku is not defined. Any reason for removing that check?

Copy link
Copy Markdown
Contributor

@jleveque jleveque Aug 29, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, the above suggested simplification looks good. If asic is defined, then platform/hwsku/port_config.ini will not be present as per current design.

Great. Thanks for the confirmation!

In the previous code, I do not see port_config_candidates.append(os.path.join(platform_path, PORT_CONFIG_INI)). being directly used, and is used only if hwsku is not defined. Any reason for removing that check?

Because port_config.ini has never resided directly under the <platform> directory (unless something changed recently), as it has always been hardware SKU-specific. That's one of the reasons I'm asking these questions. I don't believe that is a valid check, and I want to confirm that it can be removed. @samaity: It appears you added this directory as a candidate. What was your reasoning?

Copy link
Copy Markdown
Collaborator

@samaity samaity Sep 2, 2020

Choose a reason for hiding this comment

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

@jleveque , As I have understood till now and also during platform.json file porting, the order was like below.

/usr/share/sonic/hwsku/
/usr/share/sonic/device/<platform>/<hwsku>/
/usr/share/sonic/platform/<hwsku>/
/usr/share/sonic/<hwsku>/

So, I followed the same, but yeah, I guess, I mistakenly added the directory. I don't think it's needed for port_config.ini. however, platform.json will be under the platform directory. So we are safe with the changes.

Copy link
Copy Markdown
Contributor

@jleveque jleveque Sep 2, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

So to finalize/summarize for posterity, platform.json will always be found under:

  • /usr/share/sonic/device/<platform>/ in the host OS
  • /usr/share/sonic/platform/ inside a container

And port_config.ini will always be found under:

  • /usr/share/sonic/device/<platform>/<hwsku>/<ASIC index>/ in the host OS of a multi-ASIC switch
  • /usr/share/sonic/device/<platform>/<hwsku>/ in the host OS of a single-ASIC switch
  • /usr/share/sonic/hwsku/<ASIC index>/ (also /usr/share/sonic/platform/<hwsku>/<ASIC index>/) in a container on a multi-ASIC switch
  • /usr/share/sonic/hwsku/ (also /usr/share/sonic/platform/<hwsku>/) in a container on a single-ASIC switch

@SuvarnaMeenakshi, @samaity: Correct?


for candidate in port_config_candidates:
if os.path.isfile(candidate):
return candidate

return None

def get_sonic_version_info():
if not os.path.isfile(SONIC_VERSION_YAML_PATH):
Expand Down
2 changes: 1 addition & 1 deletion src/sonic-utilities
Open in desktop
Submodule sonic-utilities updated from 17fb37 to 58c296