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

Jump to bottom

[multi-asic]: remove load_sonic_global_db_config calls #8173

Merged
merged 4 commits into from
Aug 6, 2021
Merged

[multi-asic]: remove load_sonic_global_db_config calls #8173

merged 4 commits into from
Aug 6, 2021

Conversation

arlakshm
Copy link
Contributor

@arlakshm arlakshm commented Jul 13, 2021 •
edited
Loading

Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan arlakshm@microsoft.com

Why I did it

Remove the call to SonicDBConfig.load_sonic_global_db_config() in the multi asic functions.
The expection is the client calling this function will call SonicDBConfig.load_sonic_global_db_config()

This PR is dependent on the PR sonic-net/sonic-utilities#1712

How to verify it

compile sonic-utilities

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

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106

Description for the changelog

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

@arlakshm
remove load_sonic_global_db_config calls
bfa16f9 
Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>
@arlakshm arlakshm requested a review from lguohan as a code owner July 13, 2021 19:44
@arlakshm arlakshm requested a review from qiluo-msft July 13, 2021 19:44
@arlakshm arlakshm mentioned this pull request Jul 13, 2021
Merged
@qiluo-msft
Copy link
Collaborator 

        swsscommon.SonicDBConfig.initializeGlobalConfig()

Is it same as load_db_config()? If yes, we can call that function, and remove device_info from this file.

Refers to: src/sonic-host-services/scripts/caclmgrd:617 in bfa16f9. [](commit_id = bfa16f9, deletion_comment = False)

@qiluo-msft
Copy link
Collaborator

The vstest failure is relating this PR

teamd/test_portchannel.py:17: AssertionError
----------------------------- Captured stdout call -----------------------------
-----rc=1 for cmd config portchannel add PortChannel0001-----
Traceback (most recent call last):
  File "/usr/local/bin/config", line 8, in <module>
    sys.exit(config())
  File "/usr/local/lib/python3.7/dist-packages/click/core.py", line 764, in __call__
    return self.main(*args, **kwargs)
  File "/usr/local/lib/python3.7/dist-packages/click/core.py", line 717, in main
    rv = self.invoke(ctx)
  File "/usr/local/lib/python3.7/dist-packages/click/core.py", line 1134, in invoke
    Command.invoke(self, ctx)
  File "/usr/local/lib/python3.7/dist-packages/click/core.py", line 956, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/usr/local/lib/python3.7/dist-packages/click/core.py", line 555, in invoke
    return callback(*args, **kwargs)
  File "/usr/local/lib/python3.7/dist-packages/click/decorators.py", line 17, in new_func
    return f(get_current_context(), *args, **kwargs)
  File "/usr/local/lib/python3.7/dist-packages/config/main.py", line 954, in config
    SonicDBConfig.initialize()
RuntimeError: SonicDBConfig already initialized

qiluo-msft
qiluo-msft reviewed Jul 14, 2021
View reviewed changes
src/sonic-py-common/sonic_py_common/multi_asic.py Outdated
if is_multi_asic():
swsscommon.SonicDBConfig.load_sonic_global_db_config()
else:
swsscommon.SonicDBConfig.initialize()
Copy link
Collaborator

@qiluo-msft qiluo-msft Jul 14, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Not a blocking issue. You may use load_sonic_db_config which is available in both swsscommon and swsssdk. #Closed

@arlakshm
move load_db_config from sonic_py_common to sonic_utilities
8665ecd 
Signed-off-by: Arvindsrinivasan Lakshmi Narasimhan <arlakshm@microsoft.com>
@arlakshm
Copy link
Contributor Author

The vstest failure is relating this PR

teamd/test_portchannel.py:17: AssertionError
----------------------------- Captured stdout call -----------------------------
-----rc=1 for cmd config portchannel add PortChannel0001-----
Traceback (most recent call last):
  File "/usr/local/bin/config", line 8, in <module>
    sys.exit(config())
  File "/usr/local/lib/python3.7/dist-packages/click/core.py", line 764, in __call__
    return self.main(*args, **kwargs)
  File "/usr/local/lib/python3.7/dist-packages/click/core.py", line 717, in main
    rv = self.invoke(ctx)
  File "/usr/local/lib/python3.7/dist-packages/click/core.py", line 1134, in invoke
    Command.invoke(self, ctx)
  File "/usr/local/lib/python3.7/dist-packages/click/core.py", line 956, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/usr/local/lib/python3.7/dist-packages/click/core.py", line 555, in invoke
    return callback(*args, **kwargs)
  File "/usr/local/lib/python3.7/dist-packages/click/decorators.py", line 17, in new_func
    return f(get_current_context(), *args, **kwargs)
  File "/usr/local/lib/python3.7/dist-packages/config/main.py", line 954, in config
    SonicDBConfig.initialize()
RuntimeError: SonicDBConfig already initialized

Hi Qi,
As discussed this error is caused because of the following sequence:

  • For every CLI the file utilities_common.cli is imported. In this file we initialize the global variable
    iface_alias_converter = InterfaceAliasConverter()
  • In the init of the class InterfaceAliasConverter we call theconnect_config_db_for_ns()with namespace option
    For single asic the namespace is default_namespace(“ “)
  • In the init of the sonicV2connector class we call the function get_db_list().
  • In this function if the db is not initialized we initialize the db.
    Thereby loading the database file
  • So in main.py when we try to initialize the db again it errors out

To prevent this error, added a new function load_db_config in sonic-utilities to safely load the database config files.
the changes are in PR sonic-net/sonic-utilities#1712.

@arlakshm
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@arlakshm
Copy link
Contributor Author

/Azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yozhao101
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yozhao101
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@arlakshm
Copy link
Contributor Author

arlakshm commented Aug 5, 2021

/Azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@qiluo-msft qiluo-msft merged commit 302f889 into sonic-net:master Aug 6, 2021
carl-nokia pushed a commit to carl-nokia/sonic-buildimage that referenced this pull request Aug 7, 2021
@arlakshm
[multi-asic]: remove load_sonic_global_db_config calls (sonic-net#8173)
1f8e725 
#### Why I did it
Remove the call to `SonicDBConfig.load_sonic_global_db_config()` in the multi asic functions.
The expection is the client calling this function will call `SonicDBConfig.load_sonic_global_db_config()`

This PR is dependent on the PR sonic-net/sonic-utilities#1712 
#### How to verify it
compile sonic-utilities
@SuvarnaMeenakshi SuvarnaMeenakshi mentioned this pull request Aug 10, 2021
Merged
SuvarnaMeenakshi added a commit to sonic-net/sonic-utilities that referenced this pull request Aug 17, 2021
@SuvarnaMeenakshi
Load the database global_db. (#1752)
5002745 
What I did
Recent change was done to remove call to load database global_db which expects the clients to load global_db.
[sonic-net/sonic-buildimage#8173]
The load global_db was missing in multi_asic.py which was causing "show interfaces cli" to fail.
While testing on multi-asic VS image:
How I did it
load global_db config in multi_asic.py.

How to verify it
Load multi-asic VS image with the fix.
Signed-off-by: Suvarna Meenakshi <sumeenak@microsoft.com>
@SuvarnaMeenakshi SuvarnaMeenakshi mentioned this pull request Aug 24, 2021
Merged
4 tasks
judyjoseph pushed a commit to sonic-net/sonic-utilities that referenced this pull request Aug 26, 2021
@SuvarnaMeenakshi @judyjoseph
Load the database global_db. (#1752)
e567a60 
What I did
Recent change was done to remove call to load database global_db which expects the clients to load global_db.
[sonic-net/sonic-buildimage#8173]
The load global_db was missing in multi_asic.py which was causing "show interfaces cli" to fail.
While testing on multi-asic VS image:
How I did it
load global_db config in multi_asic.py.

How to verify it
Load multi-asic VS image with the fix.
Signed-off-by: Suvarna Meenakshi <sumeenak@microsoft.com>
SuvarnaMeenakshi added a commit to sonic-net/sonic-mgmt that referenced this pull request Sep 1, 2021
@SuvarnaMeenakshi
[multi-asic]: Add function to load database config (#4112)
f6586d6 
What is the motivation for this PR?
With the changes in sonic-net/sonic-buildimage#8173, the applications that connect to namespace dbs have to load global database configuration. Due to this change, the sonic-py-common.multi_asic functions that connect to database will fail if database configuration is not loaded in ansible library functions.

How did you do it?
Add a new module_utility library function to load the right db configuration based on single or multi-asic platform.
Invoke this function in port_util and lag_facts where mutli_asic library function is invoked which connects to db.

How did you verify/test it?
Bring up a multi-asic VS testbed.
./testbed-cli.sh -t vtestbed.csv -m veos_vtb -k ceos add-topo vms-kvm-four-asic-t1-lag password.txt
Before adding this change, failure seen while running test_bgp_fact.py
judyjoseph pushed a commit that referenced this pull request Sep 27, 2021
@arlakshm @judyjoseph
[multi-asic]: remove load_sonic_global_db_config calls (#8173)
78c3172 
#### Why I did it
Remove the call to `SonicDBConfig.load_sonic_global_db_config()` in the multi asic functions.
The expection is the client calling this function will call `SonicDBConfig.load_sonic_global_db_config()`

This PR is dependent on the PR sonic-net/sonic-utilities#1712 
#### How to verify it
compile sonic-utilities
vmittal-msft pushed a commit to vmittal-msft/sonic-mgmt that referenced this pull request Sep 28, 2021
@SuvarnaMeenakshi @vmittal-msft
[multi-asic]: Add function to load database config (sonic-net#4112)
21a9d1e 
What is the motivation for this PR?
With the changes in sonic-net/sonic-buildimage#8173, the applications that connect to namespace dbs have to load global database configuration. Due to this change, the sonic-py-common.multi_asic functions that connect to database will fail if database configuration is not loaded in ansible library functions.

How did you do it?
Add a new module_utility library function to load the right db configuration based on single or multi-asic platform.
Invoke this function in port_util and lag_facts where mutli_asic library function is invoked which connects to db.

How did you verify/test it?
Bring up a multi-asic VS testbed.
./testbed-cli.sh -t vtestbed.csv -m veos_vtb -k ceos add-topo vms-kvm-four-asic-t1-lag password.txt
Before adding this change, failure seen while running test_bgp_fact.py
malletvapid23 added a commit to malletvapid23/Sonic-Utility that referenced this pull request Aug 3, 2023
@malletvapid23
Load the database global_db. (#1752)
c8e4774 
What I did
Recent change was done to remove call to load database global_db which expects the clients to load global_db.
[sonic-net/sonic-buildimage#8173]
The load global_db was missing in multi_asic.py which was causing "show interfaces cli" to fail.
While testing on multi-asic VS image:
How I did it
load global_db config in multi_asic.py.

How to verify it
Load multi-asic VS image with the fix.
Signed-off-by: Suvarna Meenakshi <sumeenak@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qiluo-msft qiluo-msft qiluo-msft approved these changes

@lguohan lguohan Awaiting requested review from lguohan lguohan is a code owner

Assignees
No one assigned
Labels
Included in 202106 Branch Request for 202106 Branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@arlakshm @qiluo-msft @yozhao101 @judyjoseph