Skip to content

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

Merged
SuvarnaMeenakshi merged 6 commits intosonic-net:masterfrom
SuvarnaMeenakshi:portconfig_master
Apr 22, 2022
Merged

[portconfig]: Remove try block for db config initialization#10581
SuvarnaMeenakshi merged 6 commits intosonic-net:masterfrom
SuvarnaMeenakshi:portconfig_master

Conversation

@SuvarnaMeenakshi
Copy link
Copy Markdown
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 <[email protected]>
as it was before PR sonic-net#10362.

Signed-off-by: Suvarna Meenakshi <[email protected]>
Comment thread src/sonic-config-engine/portconfig.py Outdated
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
Copy Markdown
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

if multi_asic.is_multi_asic():
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
Copy Markdown
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 <[email protected]>
@lgtm-com
Copy link
Copy Markdown

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

Comment thread src/sonic-config-engine/portconfig.py Outdated
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown

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 <[email protected]>
@SuvarnaMeenakshi SuvarnaMeenakshi merged commit 5cd6bc4 into sonic-net:master Apr 22, 2022
@jerseyang
Copy link
Copy Markdown
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
Copy Markdown
Collaborator

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

@SuvarnaMeenakshi
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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