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

[healthd] Use unix_socket_path instead of loopback ip #14843

Merged
merged 5 commits into from
May 26, 2023
Merged

[healthd] Use unix_socket_path instead of loopback ip #14843

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
341108e
[health] Start system-health after interfaces-config
vivekrnv Apr 21, 2023
e5b0456
Merge branch 'sonic-net:master' into intf_cfg_dep_healthd
vivekrnv Apr 25, 2023
4a8a3e4
Revert "[health] Start system-health after interfaces-config"
vivekrnv May 10, 2023
a7f6bf9
Use unix_socket_path in system-health
vivekrnv May 10, 2023
551544b
Merge branch 'intf_cfg_dep_healthd' of github.com:vivekrnv/sonic-buil…
vivekrnv May 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/system-health/health_checker/hardware_checker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class HardwareChecker(HealthChecker):

def __init__(self):
HealthChecker.__init__(self)
self._db = SonicV2Connector(host="127.0.0.1")
self._db = SonicV2Connector(use_unix_socket_path=True)
self._db.connect(self._db.STATE_DB)

def get_category(self):
Expand Down
2 changes: 1 addition & 1 deletion src/system-health/health_checker/service_checker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ def check_services(self, config):
config (config.Config): Health checker configuration.
"""
if not self.config_db:
self.config_db = swsscommon.ConfigDBConnector()
self.config_db = swsscommon.ConfigDBConnector(use_unix_socket_path=True)
self.config_db.connect()
feature_table = self.config_db.get_table("FEATURE")
expected_running_containers, self.container_feature_dict = self.get_expected_running_containers(feature_table)
Expand Down
16 changes: 8 additions & 8 deletions src/system-health/health_checker/sysmonitor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def __init__(self,myQ):
self.task_queue = myQ

def subscribe_statedb(self):
state_db = swsscommon.DBConnector("STATE_DB", REDIS_TIMEOUT_MS, True)
state_db = swsscommon.DBConnector("STATE_DB", REDIS_TIMEOUT_MS, False)
sel = swsscommon.Select()
cst = swsscommon.SubscriberStateTable(state_db, "FEATURE")
sel.addSelectable(cst)
Expand Down Expand Up @@ -122,7 +122,7 @@ def __init__(self):
def post_system_status(self, state):
try:
if not self.state_db:
self.state_db = swsscommon.SonicV2Connector(host='127.0.0.1')
self.state_db = swsscommon.SonicV2Connector(use_unix_socket_path=True)
self.state_db.connect(self.state_db.STATE_DB)

self.state_db.set(self.state_db.STATE_DB, "SYSTEM_READY|SYSTEM_STATE", "Status", state)
Expand All @@ -135,7 +135,7 @@ def post_system_status(self, state):
def get_all_service_list(self):

if not self.config_db:
self.config_db = swsscommon.ConfigDBConnector()
self.config_db = swsscommon.ConfigDBConnector(use_unix_socket_path=True)
self.config_db.connect()

dir_list = []
Expand Down Expand Up @@ -197,10 +197,10 @@ def get_service_from_feature_table(self, dir_list):
#else, just return Up
def get_app_ready_status(self, service):
if not self.state_db:
self.state_db = swsscommon.SonicV2Connector(host='127.0.0.1')
self.state_db = swsscommon.SonicV2Connector(use_unix_socket_path=True)
self.state_db.connect(self.state_db.STATE_DB)
if not self.config_db:
self.config_db = swsscommon.ConfigDBConnector()
self.config_db = swsscommon.ConfigDBConnector(use_unix_socket_path=True)
self.config_db.connect()

fail_reason = ""
Expand Down Expand Up @@ -248,7 +248,7 @@ def run_systemctl_show(self, service):
#Sets the service status to state db
def post_unit_status(self, srv_name, srv_status, app_status, fail_reason, update_time):
if not self.state_db:
self.state_db = swsscommon.SonicV2Connector(host='127.0.0.1')
self.state_db = swsscommon.SonicV2Connector(use_unix_socket_path=True)
self.state_db.connect(self.state_db.STATE_DB)

key = 'ALL_SERVICE_STATUS|{}'.format(srv_name)
Expand Down Expand Up @@ -378,7 +378,7 @@ def update_system_status(self):
def check_unit_status(self, event):
#global dnsrvs_name
if not self.state_db:
self.state_db = swsscommon.SonicV2Connector(host='127.0.0.1')
self.state_db = swsscommon.SonicV2Connector(use_unix_socket_path=True)
self.state_db.connect(self.state_db.STATE_DB)
astate = "DOWN"

Expand Down Expand Up @@ -416,7 +416,7 @@ def check_unit_status(self, event):

def system_service(self):
if not self.state_db:
self.state_db = swsscommon.SonicV2Connector(host='127.0.0.1')
self.state_db = swsscommon.SonicV2Connector(use_unix_socket_path=True)
self.state_db.connect(self.state_db.STATE_DB)

mpmgr = multiprocessing.Manager()
Expand Down
2 changes: 1 addition & 1 deletion src/system-health/scripts/healthd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class HealthDaemon(DaemonBase):
Constructor of HealthDaemon.
"""
DaemonBase.__init__(self, SYSLOG_IDENTIFIER)
self._db = SonicV2Connector(host="127.0.0.1")
self._db = SonicV2Connector(use_unix_socket_path=True)
self._db.connect(self._db.STATE_DB)
self.stop_event = threading.Event()

Expand Down
2 changes: 1 addition & 1 deletion src/system-health/tests/mock_connector.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ class MockConnector(object):
STATE_DB = None
data = {}

def __init__(self, host):
def __init__(self, use_unix_socket_path):
Copy link
Collaborator

Choose a reason for hiding this comment

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

use_unix_socket_path

Does it need a default value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it's just a mock. the variable has no significance when running tests

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see the caller code is SonicV2Connector(use_unix_socket_path=True).

So ideally you need to mock for a optional parameter. Even the current works.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Still applicable, even this PR is merged.
@liat-grozovik @vivekrnv

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've ran the tests with the previous mock, and it failed. That's why i've updated the mock.

Previously, we used to call SonicV2Connector(host="127.0.0.1") with host argument, so the ctor for mock is also expected to have the host argument. Similarly for use_unix_socket_path arg

pass

def connect(self, db_id):
Expand Down