Use unix socket path in ConfigDBConnector for some components #10179

stepanblyschak · 2022-03-08T12:34:03Z

Signed-off-by: Stepan Blyschak [email protected]

Why I did it

Using TCP connection might be interrupted when restaring interfaces-config.service. Unix socket does not have this issue.

How I did it

Use unix socket.

How to verify it

Restart interfaces-config.service with hostcfgd.

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

Description for the changelog

Link to config_db schema for YANG module changes

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

Signed-off-by: Stepan Blyschak <[email protected]>

liat-grozovik · 2022-03-15T13:42:30Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2022-03-15T13:42:42Z

Azure Pipelines successfully started running 1 pipeline(s).

qiluo-msft · 2022-03-18T01:32:58Z

src/sonic-py-common/sonic_py_common/multi_asic.py

@@ -43,7 +43,7 @@ def connect_config_db_for_ns(namespace=DEFAULT_NAMESPACE):
    Returns:
      handle to the config_db for a namespace
    """
-    config_db = swsscommon.ConfigDBConnector(namespace=namespace)
+    config_db = swsscommon.ConfigDBConnector(use_unix_socket_path=True, namespace=namespace)


use_unix_socket_path

Since this is a library function, it will impact many applications. One impact that the new code requires sudo for the application.

To make migration easier, how about add a parameter to connect_config_db_for_ns() and by default do not use unix socket. And you can migrate the applications one by one. #Closed

@qiluo-msft In my test use_unix_socket_path does not require sudo:

admin@r-tigon-20:~$ python Python 3.9.2 (default, Feb 28 2021, 17:03:44) [GCC 10.2.1 20210110] on linux Type "help", "copyright", "credits" or "license" for more information. >>> from swsscommon.swsscommon import ConfigDBConnector >>> c=ConfigDBConnector(use_unix_socket_path=True) >>> c.connect() >>>

Agree. Still application may still want to customize the connection method. Could you add a parameter to connect_config_db_for_ns() and by default do not use unix socket. And you can migrate the applications one by one.

@qiluo-msft In some cases it is not possible to do. Considering an application want's to import from a library:

>>> from utilities_common.db import Db ^C Traceback (most recent call last): File "<stdin>", line 1, in <module> File "/usr/local/lib/python3.9/dist-packages/utilities_common/db.py", line 4, in <module> from utilities_common.multi_asic import multi_asic_ns_choices File "/usr/local/lib/python3.9/dist-packages/utilities_common/multi_asic.py", line 102, in <module> default=multi_asic_display_default_option(), File "/usr/local/lib/python3.9/dist-packages/utilities_common/multi_asic.py", line 93, in multi_asic_display_default_option if not multi_asic.is_multi_asic() and not device_info.is_chassis(): File "/usr/local/lib/python3.9/dist-packages/sonic_py_common/device_info.py", line 427, in is_chassis return is_voq_chassis() or is_packet_chassis() File "/usr/local/lib/python3.9/dist-packages/sonic_py_common/device_info.py", line 417, in is_voq_chassis switch_type = get_platform_info().get('switch_type') File "/usr/local/lib/python3.9/dist-packages/sonic_py_common/device_info.py", line 353, in get_platform_info hw_info_dict['hwsku'] = get_hwsku() File "/usr/local/lib/python3.9/dist-packages/sonic_py_common/device_info.py", line 126, in get_hwsku return get_localhost_info('hwsku') File "/usr/local/lib/python3.9/dist-packages/sonic_py_common/device_info.py", line 47, in get_localhost_info config_db.connect() File "/usr/local/lib/python3.9/dist-packages/swsssdk/configdb.py", line 84, in connect self.db_connect('CONFIG_DB', wait_for_init, retry_on) File "/usr/local/lib/python3.9/dist-packages/swsssdk/configdb.py", line 79, in db_connect SonicV2Connector.connect(self, self.db_name, retry_on) File "/usr/local/lib/python3.9/dist-packages/swsssdk/dbconnector.py", line 268, in connect self.dbintf.connect(db_id, db_name, retry_on) File "/usr/local/lib/python3.9/dist-packages/swsssdk/interface.py", line 175, in connect self._onetime_connect(db_id, db_name) File "/usr/local/lib/python3.9/dist-packages/swsssdk/interface.py", line 192, in _onetime_connect client.config_set('notify-keyspace-events', self.KEYSPACE_EVENTS) File "/usr/local/lib/python3.9/dist-packages/redis/client.py", line 1243, in config_set return self.execute_command('CONFIG SET', name, value) File "/usr/local/lib/python3.9/dist-packages/redis/client.py", line 898, in execute_command conn = self.connection or pool.get_connection(command_name, **options) File "/usr/local/lib/python3.9/dist-packages/redis/connection.py", line 1192, in get_connection connection.connect() File "/usr/local/lib/python3.9/dist-packages/redis/connection.py", line 559, in connect sock = self._connect() File "/usr/local/lib/python3.9/dist-packages/redis/connection.py", line 603, in _connect sock.connect(socket_address) KeyboardInterrupt

This will connect to DB using TCP. But the question is, how an application doing import needs to tell the library to use unix socket. One may add a boolean flag before doing import but this is a hack which might not work actually when the import is done from another place. I don't think executing much code like connecting to a database during import time is correct for general purpose sonic library. So I would need to refactor that part, which is actually a bit of work in an area which I do not have the right expertise (chassis, multi-asic, etc.). A similar story for the multi_asic.py.
Considering that I don't see a problem using unix socket now (the sudo is no more required) I would prefer to change the default in the library.

Just to clarify: I am asking adding a parameter to connect_config_db_for_ns(), not get_platform_info().
I agree with you on the case study of get_platform_info().

@qiluo-msft I see, only for connect_config_db_for_ns it is possible. I fixed that.

I double checked and there should be permission issue. Only checked swsssdk library. But swsscommon should be the same. Make sure you could not read the file /var/run/redis/redis.sock. So the suggestion is to keep the library behave the old way if there is no explicit option.

>>> c.connect() Traceback (most recent call last): File "<stdin>", line 1, in <module> File "/usr/local/lib/python2.7/dist-packages/swsssdk/configdb.py", line 74, in connect self.db_connect('CONFIG_DB', wait_for_init, retry_on) File "/usr/local/lib/python2.7/dist-packages/swsssdk/configdb.py", line 69, in db_connect SonicV2Connector.connect(self, self.db_name, retry_on) File "/usr/local/lib/python2.7/dist-packages/swsssdk/dbconnector.py", line 250, in connect self.dbintf.connect(db_id, retry_on) File "/usr/local/lib/python2.7/dist-packages/swsssdk/interface.py", line 171, in connect self._onetime_connect(db_id) File "/usr/local/lib/python2.7/dist-packages/swsssdk/interface.py", line 183, in _onetime_connect client.config_set('notify-keyspace-events', self.KEYSPACE_EVENTS) File "/usr/local/lib/python2.7/dist-packages/redis/client.py", line 719, in config_set return self.execute_command('CONFIG SET', name, value) File "/usr/local/lib/python2.7/dist-packages/redis/client.py", line 673, in execute_command connection.send_command(*args) File "/usr/local/lib/python2.7/dist-packages/redis/connection.py", line 610, in send_command self.send_packed_command(self.pack_command(*args)) File "/usr/local/lib/python2.7/dist-packages/redis/connection.py", line 585, in send_packed_command self.connect() File "/usr/local/lib/python2.7/dist-packages/redis/connection.py", line 489, in connect raise ConnectionError(self._error_message(e)) redis.exceptions.ConnectionError: Error 13 connecting to unix socket: /var/run/redis/redis.sock. Permission denied.

@qiluo-msft what is your SONiC version ?

Signed-off-by: Stepan Blyschak <[email protected]>

liat-grozovik · 2022-03-22T16:54:22Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2022-03-22T17:06:24Z

Azure Pipelines successfully started running 1 pipeline(s).

stepanblyschak · 2022-03-24T15:03:44Z

Investigating checkers failures

Signed-off-by: Stepan Blyschak <[email protected]>

stepanblyschak · 2022-04-01T10:48:17Z

202012 has another PR #10436.

qiluo-msft · 2022-04-02T00:05:21Z

src/sonic-py-common/sonic_py_common/device_info.py

@@ -43,7 +43,7 @@

 def get_localhost_info(field):
    try:
-        config_db = ConfigDBConnector()
+        config_db = ConfigDBConnector(use_unix_socket_path=True)


use_unix_socket_path

Changing default behavior will cause a permission issue to non privilege users.

liat-grozovik · 2022-04-13T10:16:51Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2022-04-13T10:17:04Z

Azure Pipelines successfully started running 1 pipeline(s).

qiluo-msft · 2022-04-25T05:32:41Z

src/sonic-py-common/sonic_py_common/device_info.py

@@ -43,7 +43,7 @@

 def get_localhost_info(field):


get_localhost_info

If you intention is to fix hostcfgd, you do not need to modify this file.

No, this is not the main intension. Please see the following back trace:

Traceback (most recent call last):', ' File "/usr/local/bin/sonic-installer", line 5, in <module>, from sonic_installer.main import sonic_installer', ' File "/usr/local/lib/python3.9/dist-packages/sonic_installer/main.py", line 7, in <module>', import utilities_common.cli as clicommon', ' File "/usr/local/lib/python3.9/dist-packages/utilities_common/cli.py", line 189, in <module>', iface_alias_converter = InterfaceAliasConverter()', ' File "/usr/local/lib/python3.9/dist-packages/utilities_common/cli.py", line 126, in __init__', self.port_dict = multi_asic.get_port_table()', ' File "/usr/local/lib/python3.9/dist-packages/sonic_py_common/multi_asic.py", line 301, in get_port_table', ports = get_port_table_for_asic(ns)', ' File "/usr/local/lib/python3.9/dist-packages/sonic_py_common/multi_asic.py", line 315, in get_port_table_for_asic', config_db = connect_config_db_for_ns(namespace)', ' File "/usr/local/lib/python3.9/dist-packages/sonic_py_common/multi_asic.py", line 47, in connect_config_db_for_ns', config_db.connect()', File "/usr/lib/python3/dist-packages/swsscommon/swsscommon.py", line 1829, in connect', return _swsscommon.ConfigDBConnector_Native_connect(self, wait_for_init, retry_on)', 'RuntimeError: Unable to connect to redis: Cannot assign requested address

arlakshm · 2022-04-25T19:48:27Z

adding @judyjoseph and @abdosi
I think using unix socket may result in using sudo for most of the multi asic show CLI.
Recently, this PR sonic-net/sonic-py-swsssdk#120 made the change to use Unix socket for only user in privilaged access.

davidpil2002 · 2023-05-28T14:22:24Z

adding @judyjoseph and @abdosi I think using unix socket may result in using sudo for most of the multi asic show CLI. Recently, this PR sonic-net/sonic-py-swsssdk#120 made the change to use Unix socket for only user in privilaged access.

Hi @arlakshm
Hope all is well,
due to some security modifications that we are planning to do,
we would like to insist on moving to UDS, please can you share some examples in the code related to your concern about moving to UDS?

Use unix socket path in ConfigDBConnector for some components

89fa106

Signed-off-by: Stepan Blyschak <[email protected]>

stepanblyschak requested a review from lguohan as a code owner March 8, 2022 12:34

stepanblyschak mentioned this pull request Mar 8, 2022

[hostcfgd.service] start after interfaces-config.service #9953

Closed

6 tasks

liat-grozovik added Included in 202111 Branch Request for 202012 Branch Bug 🐛 labels Mar 10, 2022

liat-grozovik previously approved these changes Mar 10, 2022

View reviewed changes

liat-grozovik requested a review from qiluo-msft March 10, 2022 11:44

qiluo-msft reviewed Mar 18, 2022

View reviewed changes

Another place added where unix sock usage is required

aefff93

Signed-off-by: Stepan Blyschak <[email protected]>

stepanblyschak dismissed liat-grozovik’s stale review via aefff93 March 21, 2022 17:31

stepanblyschak added 2 commits March 28, 2022 15:45

add another use_unix_socket_path

78eee06

Signed-off-by: Stepan Blyschak <[email protected]>

add param for connect_configdb_for_ns

8a79393

Signed-off-by: Stepan Blyschak <[email protected]>

stepanblyschak mentioned this pull request Apr 1, 2022

[sonic_installer] Use use_unix_socket_path=True in DB connector sonic-net/sonic-utilities#2096

Closed

stepanblyschak removed the Request for 202012 Branch label Apr 1, 2022

stepanblyschak added Request for 202111 Branch For PRs being requested for 202111 branch and removed Included in 202111 Branch labels Apr 1, 2022

qiluo-msft reviewed Apr 2, 2022

View reviewed changes

qiluo-msft mentioned this pull request Apr 3, 2022

[202012] Use unix socket path in ConfigDBConnector for some components #10436

Closed

6 tasks

prsunny mentioned this pull request Apr 13, 2022

sonic-installer does not work when there is no database running #10434

Closed

qiluo-msft reviewed Apr 25, 2022

View reviewed changes

stepanblyschak closed this May 4, 2022

judyjoseph removed the Request for 202111 Branch For PRs being requested for 202111 branch label May 8, 2022

stepanblyschak deleted the use_unix_socket_path branch September 23, 2022 13:33

vivekrnv mentioned this pull request May 9, 2023

[healthd] Use unix_socket_path instead of loopback ip #14843

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use unix socket path in ConfigDBConnector for some components #10179

Use unix socket path in ConfigDBConnector for some components #10179

stepanblyschak commented Mar 8, 2022 •

edited

Loading

liat-grozovik commented Mar 15, 2022

azure-pipelines bot commented Mar 15, 2022

qiluo-msft Mar 18, 2022 •

edited

Loading

stepanblyschak Mar 18, 2022

qiluo-msft Mar 21, 2022

stepanblyschak Mar 21, 2022

qiluo-msft Mar 29, 2022

stepanblyschak Mar 30, 2022

qiluo-msft Apr 2, 2022

stepanblyschak Apr 4, 2022

liat-grozovik commented Mar 22, 2022

azure-pipelines bot commented Mar 22, 2022

stepanblyschak commented Mar 24, 2022

stepanblyschak commented Apr 1, 2022

qiluo-msft Apr 2, 2022

liat-grozovik commented Apr 13, 2022

azure-pipelines bot commented Apr 13, 2022

qiluo-msft Apr 25, 2022

stepanblyschak Apr 26, 2022

arlakshm commented Apr 25, 2022

davidpil2002 commented May 28, 2023

Use unix socket path in ConfigDBConnector for some components #10179

Use unix socket path in ConfigDBConnector for some components #10179

Conversation

stepanblyschak commented Mar 8, 2022 • edited Loading

Why I did it

How I did it

How to verify it

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

Description for the changelog

Link to config_db schema for YANG module changes

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

liat-grozovik commented Mar 15, 2022

azure-pipelines bot commented Mar 15, 2022

qiluo-msft Mar 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liat-grozovik commented Mar 22, 2022

azure-pipelines bot commented Mar 22, 2022

stepanblyschak commented Mar 24, 2022

stepanblyschak commented Apr 1, 2022

Choose a reason for hiding this comment

liat-grozovik commented Apr 13, 2022

azure-pipelines bot commented Apr 13, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arlakshm commented Apr 25, 2022

davidpil2002 commented May 28, 2023

stepanblyschak commented Mar 8, 2022 •

edited

Loading

qiluo-msft Mar 18, 2022 •

edited

Loading