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

Fix sonic-installer and 'show version' command crash when database docker not running issue. #2183

Conversation

liuh-80
Copy link
Contributor

@liuh-80 liuh-80 commented May 24, 2022

Fix sonic-installer and 'show version' command crash when database docker not running issue.

Description

Global variable utilies_common.cli.iface_alias_converter will connect to config DB when initialize itself. If database docker not running, the initialize code will crash. 
To fix this issue, change the variable to lazy initialize variable with lazy_object_proxy.

Motivation and Context

Fix this issue: sonic-net/sonic-buildimage#10434

How Has This Been Tested?

Pass all UT and sonic-buildimage E2E test.

Additional Information (Optional)

@liuh-80
Copy link
Contributor Author

liuh-80 commented May 24, 2022

Code change in this PR will validate and pass E2E test with this: sonic-net/sonic-buildimage#10909

utilities_common/cli.py Outdated Show resolved Hide resolved
@liuh-80
Copy link
Contributor Author

liuh-80 commented May 27, 2022

Test with build image failed, not caused by change in this PR, seems fixed in master branch, will merge latest code and test again.

@qiluo-msft
Copy link
Contributor

Could you update PR description?

  1. "Remove global variable iface_alias_converter" this is not true
  2. Please share more insight what is wrong with old code, since this is a fix.

@liuh-80 liuh-80 changed the title Fix sonic-installer and 'show version' command crash when database docker not running issue by remove global variable iface_alias_converter. Fix sonic-installer and 'show version' command crash when database docker not running issue. May 30, 2022
@qiluo-msft qiluo-msft merged commit 9e310e5 into sonic-net:master May 30, 2022
@stepanblyschak
Copy link
Contributor

@qiluo-msft @liuh-80 Is there going to be a UT covering this fix ?

1 similar comment
@stepanblyschak
Copy link
Contributor

@qiluo-msft @liuh-80 Is there going to be a UT covering this fix ?

@qiluo-msft
Copy link
Contributor

@liuh-80 Please add unit test for this fix.

@liuh-80
Copy link
Contributor Author

liuh-80 commented Jun 1, 2022

@liuh-80 Please add unit test for this fix.

Here is UT PR: sonic-net/sonic-mgmt#5722

qiluo-msft pushed a commit that referenced this pull request Jul 4, 2022
#### What I did
Fix pfcwd connect DB with exception issue: sonic-net/sonic-buildimage#11269

pfcwd implicit depends on InterfaceAliasConverter() to initialize DB config, however following PR change InterfaceAliasConverter() behavior to lazy initialize, then pfcwd failed when try connect to DB without initialize DB config:

#2183

#### How I did it
Load  DB config in pfcwd.

#### How to verify it
Pass all UT.
@stepanblyschak
Copy link
Contributor

@liuh-80 @qiluo-msft Could you please cherry pick this PR to 2205?

yxieca pushed a commit that referenced this pull request Sep 8, 2022
…cker not running issue. (#2183)

Fix sonic-installer and 'show version' command crash when database docker not running issue.

#### Description
    Global variable utilies_common.cli.iface_alias_converter will connect to config DB when initialize itself. If database docker not running, the initialize code will crash. 
    To fix this issue, change the variable to lazy initialize variable with lazy_object_proxy.

#### Motivation and Context

Fix this issue: sonic-net/sonic-buildimage#10434

#### How Has This Been Tested?
    Pass all UT and sonic-buildimage E2E test.

#### Additional Information (Optional)
gechiang pushed a commit to gechiang/sonic-utilities that referenced this pull request Sep 14, 2022
#### What I did
Fix pfcwd connect DB with exception issue: sonic-net/sonic-buildimage#11269

pfcwd implicit depends on InterfaceAliasConverter() to initialize DB config, however following PR change InterfaceAliasConverter() behavior to lazy initialize, then pfcwd failed when try connect to DB without initialize DB config:

sonic-net#2183

#### How I did it
Load  DB config in pfcwd.

#### How to verify it
Pass all UT.
gechiang added a commit that referenced this pull request Sep 15, 2022
#### What I did
Fix pfcwd connect DB with exception issue: sonic-net/sonic-buildimage#11269

pfcwd implicit depends on InterfaceAliasConverter() to initialize DB config, however following PR change InterfaceAliasConverter() behavior to lazy initialize, then pfcwd failed when try connect to DB without initialize DB config:

#2183

#### How I did it
Load  DB config in pfcwd.

#### How to verify it
Pass all UT.

Co-authored-by: Hua Liu <58683130+liuh-80@users.noreply.github.com>
malletvapid23 added a commit to malletvapid23/Sonic-Utility that referenced this pull request Aug 3, 2023
#### What I did
Fix pfcwd connect DB with exception issue: sonic-net/sonic-buildimage#11269

pfcwd implicit depends on InterfaceAliasConverter() to initialize DB config, however following PR change InterfaceAliasConverter() behavior to lazy initialize, then pfcwd failed when try connect to DB without initialize DB config:

sonic-net/sonic-utilities#2183

#### How I did it
Load  DB config in pfcwd.

#### How to verify it
Pass all UT.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants