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

[vdahiya12]

This PR addresses fixes in sonic-py-common to imitate the behavior inside
sonic-cfggen. Essentially this is a fix for accessing the port-config file.
First check if there is a platform.json file for config generation
and then for legacy port_config.ini.
Also updating the sub-module sonic-utilities.

sonic-utilities update submodule includes all these PR's

Fix pfcwd stats crash with invalid queue name (#1077)
[show][bgp]Display the Total number of neighbors in the show ip bgp(v6) summary. (#1079)
[config] Update SONiC Environment Vars When Loading Minigraph (#1073)
Multi asic platform changes for interface, portchannel commands (#878)
Update Command-Reference.md (#1075)
[filter-fdb] Fix Filter FDB With IPv6 Present in Config DB (#1059)
[config] Remove _get_breakout_cfg_file_name helper function (#1069)
[SHOW][BGP] support show ip(v6) bgp summary for multi asic platform (#1064)
[fanshow] Display other fan status, such as Updating (#1014)
Add ip_prefix len based on proxy_arp status (#1046)
Enable the platform specific ssd firmware upgrade during reboot (#954)
[show][cli[show interface portchannel support for Multi ASIC (#1005)
support show interface commands for multi ASIC platforms (#1006)

Signed-off-by: vaibhav-dahiya vdahiya@microsoft.com

[tahmed-dev]

line 199 and 202 are appending the same file. The list from the sonic-config-engine is:

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

[jleveque]

Yes. the second is no longer needed.

[jleveque]

Question is, why were we checking for port_config.ini in this order?

1. /usr/share/sonic/hwsku/
2. /usr/share/sonic/device/<platform>/<hwsku>/<ASIC>/
3. /usr/share/sonic/device/<platform>/<hwsku>/
4. /usr/share/sonic/<platform>/

@samaity, @SuvarnaMeenakshi: Can you help answer this? Determining whether we are running inside a container versus in the host OS should now be handled by get_paths_to_platform_and_hwsku_dirs(). What is the new proper order with which to look for the port_config.ini file?

Before the addition of multi-ASIC functionality, the port_config.ini file was always located under the HwSKU directory, which, in the host OS was at /usr/share/sonic/device/<platform>/<hwsku>/ and in the containers was mounted at /usr/share/sonic/hwsku/. Why did we start checking the <platform> directory? Is there now a case where port_config.ini will reside directly under the platform directory? That has never been the case as far as I'm aware.

I believe the proper method for finding the port_config.ini file now that we have multi-ASIC functionality should be as simple as this:

# This function will return `/usr/share/sonic/hwsku/` as hwsku_path if run inside a container
# or `/usr/share/sonic/device/<platform>/<hwsku>/` if run in the host OS
(platform_path, hwsku_path) = get_paths_to_platform_and_hwsku_dirs()

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

Please correct me if I am wrong.

[jleveque]

As discussed, please also remove the get_port_config_file_name() function from portconfig.py as you had done in #5253, and close that PR as it will be redundant to this one.

[vdahiya12]

As discussed, please also remove the get_port_config_file_name() function from portconfig.py as you had done in #5253, and close that PR as it will be redundant to this one.

fixed

[jleveque]

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

[SuvarnaMeenakshi]

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?

[jleveque]

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?

[samaity]

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

[jleveque]

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?

[vdahiya12]

retest this please

[jleveque]

@vdahiya12: The PR builds are now failing because the sonic-config-engine unit tests do not run on any platform, therefore sonic-py-common.get_platform() returns None. Any unit test which calls get_platform() or another sonic-py-common function which calls get_platform() (e.g., get_paths_to_platform_and_hwsku_dirs() or get_port_config_file_name()) should mock those function(s), as they are part of a separate package, it is not testing them directly. Can you please add the appropriate mocks to the sonic-config-engine tests in the PR to allow the unit tests to pass?

[vdahiya12]

Retest vsimage please

[vdahiya12]

Retest vsimage please

[vdahiya12]

Retest vsimage please

[vdahiya12]

Retest vsimage please

[vdahiya12]

Retest vsimage please