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.

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

[portconfig]: Remove try block for db config initialization #10581

Merged
merged 6 commits into from
Apr 22, 2022

Conversation

SuvarnaMeenakshi
Copy link
Contributor

@SuvarnaMeenakshi SuvarnaMeenakshi commented Apr 14, 2022

Why I did it

  1. Provide fix for comment: https://github.com/Azure/sonic-buildimage/pull/10475/files#r847753187;
    Follow up issue to be done in a different PR:
    [sonic-cfggen]: Update UT to add port lanes #10362 - modified 2 test cases where the test case is required to get PORT table from config_db, the PR modified to use port_config.ini instead.

How I did it

  1. Try exception is not required in this scenario, so remove and modify to initial db config according to single or multi-asic platforms.

How to verify it

Verified on multi-asic device.

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

Fix unit-test case to read test from config db.

Signed-off-by: Suvarna Meenakshi <sumeenak@microsoft.com>
as it was before PR sonic-net#10362.

Signed-off-by: Suvarna Meenakshi <sumeenak@microsoft.com>
try:
if namespace is not None:
if not swsscommon.SonicDBConfig.isInit():
if multi_asic.is_multi_asic():
swsscommon.SonicDBConfig.load_sonic_global_db_config(namespace=namespace)
Copy link
Collaborator

@qiluo-msft qiluo-msft Apr 14, 2022

Choose a reason for hiding this comment

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

load_sonic_global_db_config

portconfig.py is just a library. It's the application's responsibility to load db config. Applications include:

  1. sonic-cfggen
  2. config
  3. etc #Closed

swsscommon.SonicDBConfig.load_sonic_global_db_config(namespace=namespace)
config_db = swsscommon.ConfigDBConnector(use_unix_socket_path=True, namespace=namespace)
except Exception as e:
Copy link
Collaborator

@qiluo-msft qiluo-msft Apr 15, 2022

Choose a reason for hiding this comment

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

Exception

Could you use this PR for first issue in your description? And separate the second issue into another PR. The 2nd one is still under discussion. #Closed

to avoid loading db config in library like portconfig.py.

Signed-off-by: Suvarna Meenakshi <sumeenak@microsoft.com>
@lgtm-com
Copy link

lgtm-com bot commented Apr 15, 2022

This pull request introduces 1 alert when merging 8f26eb9 into 04f810a - view on LGTM.com

new alerts:

  • 1 for Unused import

@@ -8,6 +8,7 @@
from swsscommon import swsscommon
from sonic_py_common import device_info
from sonic_py_common.multi_asic import get_asic_id_from_name
from sonic_py_common import multi_asic
Copy link
Collaborator

@qiluo-msft qiluo-msft Apr 15, 2022

Choose a reason for hiding this comment

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

multi_asic

from sonic_py_common import multi_asic


This line may trigger lgtm alert. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed lgtm alert

@lgtm-com
Copy link

lgtm-com bot commented Apr 15, 2022

This pull request introduces 1 alert when merging a0efc92833431007e4f928931683ceaaca5e1812 into 04f810a - view on LGTM.com

new alerts:

  • 1 for Unused import

Signed-off-by: Suvarna Meenakshi <sumeenak@microsoft.com>
Signed-off-by: Suvarna Meenakshi <sumeenak@microsoft.com>
@SuvarnaMeenakshi SuvarnaMeenakshi merged commit 5cd6bc4 into sonic-net:master Apr 22, 2022
@jerseyang
Copy link
Contributor

@SuvarnaMeenakshi, seems the new merge results my platform firsttime boot failed:

[ 10.368939] rc.local[477]: + touch /tmp/pending_config_initialization
[ 10.516144] rc.local[477]: + touch /tmp/notify_firstboot_to_platform
[ 10.604155] rc.local[477]: + [ ! -d /host/reboot-cause/platform ]
[ 10.680062] rc.local[477]: + [ -d /host/image-Add_Belgite_support.0-273d24a2a/platform/x86_64-cel_belgite-r0 ]
[ 10.804153] rc.local[477]: + dpkg -i /host/image-Add_Belgite_support.0-273d24a2a/platform/x86_64-cel_belgite-r0/platform-modules-belgite_0.9_amd64.deb
[ 10.975158] rc.local[615]: Selecting previously unselected package platform-modules-belgite.
[ 11.088101] rc.local[615]: (Reading database ... 33373 files and directories currently installed.)
[ 11.200160] rc.local[615]: Preparing to unpack .../platform-modules-belgite_0.9_amd64.deb ...
[ 11.312187] rc.local[615]: Unpacking platform-modules-belgite (0.9) ...
[ 11.404149] rc.local[615]: Setting up platform-modules-belgite (0.9) ...
[ 11.494446] rc.local[2174]: Traceback (most recent call last):
[ 11.572148] rc.local[2174]: File "/usr/local/bin/sonic-cfggen", line 466, in
[ 11.672139] rc.local[2174]: main()
[ 11.720152] rc.local[2174]: File "/usr/local/bin/sonic-cfggen", line 317, in main
[ 11.824149] rc.local[2174]: SonicDBConfig.load_sonic_db_config()
[ 11.912181] rc.local[2174]: File "/usr/lib/python3/dist-packages/swsscommon/swsscommon.py", line 1252, in load_sonic_db_config
[ 12.056118] rc.local[2174]: SonicDBConfig.initialize(sonic_db_file_path)
[ 12.148125] rc.local[2174]: File "/usr/lib/python3/dist-packages/swsscommon/swsscommon.py", line 1247, in initialize
[ 12.280165] rc.local[2174]: return _swsscommon.SonicDBConfig_initialize(*args, **kwargs)
[ 12.392147] rc.local[2174]: RuntimeError: Sonic database config file doesn't exist at /var/run/redis/sonic-db/database_config.json
[ 12.996604] rc.local[477]: + sync
[ 13.213605] rc.local[477]: + [ -n x86_64-cel_belgite-r0 ]
[ 13.284188] rc.local[477]: + [ -n ]
[ 13.336108] rc.local[477]: + mkdir -p /var/platform
[ 13.400201] rc.local[477]: + [ -f /etc/default/kdump-tools ]
[ 13.476168] rc.local[477]: + sed -i -e s/PLATFORM/x86_64-cel_belgite-r0/g /etc/default/kdump-tools
[ 13.596152] rc.local[477]: + firsttime_exit
[ 13.648166] rc.local[477]: + rm -rf /host/image-Add_Belgite_support.0-273d24a2a/platform/firsttime
[ 13.768173] rc.local[477]: + exit 0
[ 18.225351] DMAR: DRHD: handling fault status reg 3
[ 18.285384] DMAR: [DMA Write] Request device [00:1a.0] PASID ffffffff fault addr 0 [fault reason 05] PTE Write access is not set

Debian GNU/Linux 11 sonic ttyS0

can you help to have a look? Thanks!

@qiluo-msft
Copy link
Collaborator

This commit could not be cleanly cherry-picked to 202012. Please submit another PR.

@SuvarnaMeenakshi
Copy link
Contributor Author

@SuvarnaMeenakshi, seems the new merge results my platform firsttime boot failed:

[ 10.368939] rc.local[477]: + touch /tmp/pending_config_initialization [ 10.516144] rc.local[477]: + touch /tmp/notify_firstboot_to_platform [ 10.604155] rc.local[477]: + [ ! -d /host/reboot-cause/platform ] [ 10.680062] rc.local[477]: + [ -d /host/image-Add_Belgite_support.0-273d24a2a/platform/x86_64-cel_belgite-r0 ] [ 10.804153] rc.local[477]: + dpkg -i /host/image-Add_Belgite_support.0-273d24a2a/platform/x86_64-cel_belgite-r0/platform-modules-belgite_0.9_amd64.deb [ 10.975158] rc.local[615]: Selecting previously unselected package platform-modules-belgite. [ 11.088101] rc.local[615]: (Reading database ... 33373 files and directories currently installed.) [ 11.200160] rc.local[615]: Preparing to unpack .../platform-modules-belgite_0.9_amd64.deb ... [ 11.312187] rc.local[615]: Unpacking platform-modules-belgite (0.9) ... [ 11.404149] rc.local[615]: Setting up platform-modules-belgite (0.9) ... [ 11.494446] rc.local[2174]: Traceback (most recent call last): [ 11.572148] rc.local[2174]: File "/usr/local/bin/sonic-cfggen", line 466, in [ 11.672139] rc.local[2174]: main() [ 11.720152] rc.local[2174]: File "/usr/local/bin/sonic-cfggen", line 317, in main [ 11.824149] rc.local[2174]: SonicDBConfig.load_sonic_db_config() [ 11.912181] rc.local[2174]: File "/usr/lib/python3/dist-packages/swsscommon/swsscommon.py", line 1252, in load_sonic_db_config [ 12.056118] rc.local[2174]: SonicDBConfig.initialize(sonic_db_file_path) [ 12.148125] rc.local[2174]: File "/usr/lib/python3/dist-packages/swsscommon/swsscommon.py", line 1247, in initialize [ 12.280165] rc.local[2174]: return _swsscommon.SonicDBConfig_initialize(*args, **kwargs) [ 12.392147] rc.local[2174]: RuntimeError: Sonic database config file doesn't exist at /var/run/redis/sonic-db/database_config.json [ 12.996604] rc.local[477]: + sync [ 13.213605] rc.local[477]: + [ -n x86_64-cel_belgite-r0 ] [ 13.284188] rc.local[477]: + [ -n ] [ 13.336108] rc.local[477]: + mkdir -p /var/platform [ 13.400201] rc.local[477]: + [ -f /etc/default/kdump-tools ] [ 13.476168] rc.local[477]: + sed -i -e s/PLATFORM/x86_64-cel_belgite-r0/g /etc/default/kdump-tools [ 13.596152] rc.local[477]: + firsttime_exit [ 13.648166] rc.local[477]: + rm -rf /host/image-Add_Belgite_support.0-273d24a2a/platform/firsttime [ 13.768173] rc.local[477]: + exit 0 [ 18.225351] DMAR: DRHD: handling fault status reg 3 [ 18.285384] DMAR: [DMA Write] Request device [00:1a.0] PASID ffffffff fault addr 0 [fault reason 05] PTE Write access is not set

Debian GNU/Linux 11 sonic ttyS0

can you help to have a look? Thanks!

Thanks for reporting, I will check this error , will provide a clean fix.

@SuvarnaMeenakshi
Copy link
Contributor Author

@SuvarnaMeenakshi, seems the new merge results my platform firsttime boot failed:
[ 10.368939] rc.local[477]: + touch /tmp/pending_config_initialization [ 10.516144] rc.local[477]: + touch /tmp/notify_firstboot_to_platform [ 10.604155] rc.local[477]: + [ ! -d /host/reboot-cause/platform ] [ 10.680062] rc.local[477]: + [ -d /host/image-Add_Belgite_support.0-273d24a2a/platform/x86_64-cel_belgite-r0 ] [ 10.804153] rc.local[477]: + dpkg -i /host/image-Add_Belgite_support.0-273d24a2a/platform/x86_64-cel_belgite-r0/platform-modules-belgite_0.9_amd64.deb [ 10.975158] rc.local[615]: Selecting previously unselected package platform-modules-belgite. [ 11.088101] rc.local[615]: (Reading database ... 33373 files and directories currently installed.) [ 11.200160] rc.local[615]: Preparing to unpack .../platform-modules-belgite_0.9_amd64.deb ... [ 11.312187] rc.local[615]: Unpacking platform-modules-belgite (0.9) ... [ 11.404149] rc.local[615]: Setting up platform-modules-belgite (0.9) ... [ 11.494446] rc.local[2174]: Traceback (most recent call last): [ 11.572148] rc.local[2174]: File "/usr/local/bin/sonic-cfggen", line 466, in [ 11.672139] rc.local[2174]: main() [ 11.720152] rc.local[2174]: File "/usr/local/bin/sonic-cfggen", line 317, in main [ 11.824149] rc.local[2174]: SonicDBConfig.load_sonic_db_config() [ 11.912181] rc.local[2174]: File "/usr/lib/python3/dist-packages/swsscommon/swsscommon.py", line 1252, in load_sonic_db_config [ 12.056118] rc.local[2174]: SonicDBConfig.initialize(sonic_db_file_path) [ 12.148125] rc.local[2174]: File "/usr/lib/python3/dist-packages/swsscommon/swsscommon.py", line 1247, in initialize [ 12.280165] rc.local[2174]: return _swsscommon.SonicDBConfig_initialize(*args, **kwargs) [ 12.392147] rc.local[2174]: RuntimeError: Sonic database config file doesn't exist at /var/run/redis/sonic-db/database_config.json [ 12.996604] rc.local[477]: + sync [ 13.213605] rc.local[477]: + [ -n x86_64-cel_belgite-r0 ] [ 13.284188] rc.local[477]: + [ -n ] [ 13.336108] rc.local[477]: + mkdir -p /var/platform [ 13.400201] rc.local[477]: + [ -f /etc/default/kdump-tools ] [ 13.476168] rc.local[477]: + sed -i -e s/PLATFORM/x86_64-cel_belgite-r0/g /etc/default/kdump-tools [ 13.596152] rc.local[477]: + firsttime_exit [ 13.648166] rc.local[477]: + rm -rf /host/image-Add_Belgite_support.0-273d24a2a/platform/firsttime [ 13.768173] rc.local[477]: + exit 0 [ 18.225351] DMAR: DRHD: handling fault status reg 3 [ 18.285384] DMAR: [DMA Write] Request device [00:1a.0] PASID ffffffff fault addr 0 [fault reason 05] PTE Write access is not set
Debian GNU/Linux 11 sonic ttyS0
can you help to have a look? Thanks!

Thanks for reporting, I will check this error , will provide a clean fix.

Raised #10756 to revert this PR as it causes issue during boot up.

@qiluo-msft
Copy link
Collaborator

qiluo-msft commented May 10, 2022

Since this commit is later reverted on master. I will not cherry-pick to 202012.

SuvarnaMeenakshi added a commit that referenced this pull request Jun 9, 2022
…10960)

Why I did it
Provide fix for comment: https://github.com/Azure/sonic-buildimage/pull/10475/files#r847753187;
Move laoding database config to application code instead of portconfig as portconfig is used as a library.
#10581 was raised for this fix, but had to be reverted due to issue with multi-asic platform.

How I did it
Remove try exception handing from portconfig.py during config_db intialization.
Move loading of database config to application that uses portconfig.py.

How to verify it
unit-test passes.
Verified that it does not cause issue during boot up of multi-asic VS image.
Verified that config_db generation was successful in multi-asic VS.
liushilongbuaa pushed a commit to liushilongbuaa/sonic-buildimage that referenced this pull request Jun 20, 2022
Related work items: #49, #58, #107, sonic-net#247, sonic-net#249, sonic-net#277, sonic-net#593, sonic-net#597, sonic-net#1035, sonic-net#2130, sonic-net#2150, sonic-net#2165, sonic-net#2169, sonic-net#2178, sonic-net#2179, sonic-net#2187, sonic-net#2188, sonic-net#2191, sonic-net#2195, sonic-net#2197, sonic-net#2198, sonic-net#2200, sonic-net#2202, sonic-net#2206, sonic-net#2209, sonic-net#2211, sonic-net#2216, sonic-net#7909, sonic-net#8927, sonic-net#9681, sonic-net#9733, sonic-net#9746, sonic-net#9850, sonic-net#9967, sonic-net#10104, sonic-net#10152, sonic-net#10168, sonic-net#10228, sonic-net#10266, sonic-net#10288, sonic-net#10294, sonic-net#10313, sonic-net#10394, sonic-net#10403, sonic-net#10404, sonic-net#10421, sonic-net#10431, sonic-net#10437, sonic-net#10445, sonic-net#10457, sonic-net#10458, sonic-net#10465, sonic-net#10467, sonic-net#10469, sonic-net#10470, sonic-net#10474, sonic-net#10477, sonic-net#10478, sonic-net#10482, sonic-net#10485, sonic-net#10488, sonic-net#10489, sonic-net#10492, sonic-net#10494, sonic-net#10498, sonic-net#10501, sonic-net#10509, sonic-net#10512, sonic-net#10514, sonic-net#10516, sonic-net#10517, sonic-net#10523, sonic-net#10525, sonic-net#10531, sonic-net#10532, sonic-net#10538, sonic-net#10555, sonic-net#10557, sonic-net#10559, sonic-net#10561, sonic-net#10565, sonic-net#10572, sonic-net#10574, sonic-net#10576, sonic-net#10578, sonic-net#10581, sonic-net#10585, sonic-net#10587, sonic-net#10599, sonic-net#10607, sonic-net#10611, sonic-net#10616, sonic-net#10618, sonic-net#10619, sonic-net#10623, sonic-net#10624, sonic-net#10633, sonic-net#10646, sonic-net#10655, sonic-net#10660, sonic-net#10664, sonic-net#10680, sonic-net#10683
liushilongbuaa pushed a commit to liushilongbuaa/sonic-buildimage that referenced this pull request Jun 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants