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

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

Merged
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 4 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
38 changes: 23 additions & 15 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 @@ -167,41 +167,49 @@ def get_paths_to_platform_and_hwsku_dirs():
# Get platform and hwsku
(platform, hwsku) = get_platform_and_hwsku()

platform_path = None
hwsku_path = None
# Determine whether we're running in a container or on the host
platform_path_host = os.path.join(HOST_DEVICE_PATH, platform)
if platform:
platform_path_host = os.path.join(HOST_DEVICE_PATH, platform)

if os.path.isdir(CONTAINER_PLATFORM_PATH):
if os.path.isdir(CONTAINER_PLATFORM_PATH):
platform_path = CONTAINER_PLATFORM_PATH
elif os.path.isdir(platform_path_host):
elif platform_path_host and os.path.isdir(platform_path_host):
platform_path = platform_path_host
else:
raise OSError("Failed to locate platform directory")

hwsku_path = os.path.join(platform_path, hwsku)
if hwsku:
hwsku_path = os.path.join(platform_path, hwsku)
jleveque marked this conversation as resolved.
Show resolved Hide resolved

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
jleveque marked this conversation as resolved.
Show resolved Hide resolved

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_json.append(os.path.join(platform_path, PLATFORM_JSON))
tahmed-dev marked this conversation as resolved.
Show resolved Hide resolved

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
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
Contributor

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
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
Collaborator

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