Skip to content

Commit c492746

Browse files
lmicciniclaude
andcommitted
Lazy-import novaclient and keystoneauth1 SDK modules
Move novaclient and keystoneauth1 imports from module level into the functions that use them (nova_login, _server_evacuate, _handle_nova_exception, main). Eliminates ~135 lines of mock exception boilerplate across 4 test files. Add shared patch_pipeline() fixture in conftest.py replacing 7-deep nested patch blocks in process_service tests. Bind conftest sys.modules submodule mocks to parent mock attributes so lazy imports and patch() targets resolve to the same objects. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 797736a commit c492746

6 files changed

Lines changed: 165 additions & 300 deletions

File tree

templates/instanceha/bin/instanceha.py

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,6 @@
2828
from collections import defaultdict
2929
from abc import ABC, abstractmethod
3030

31-
from novaclient import client
32-
from novaclient.exceptions import Conflict, NotFound, Forbidden, Unauthorized
33-
34-
from keystoneauth1 import loading
35-
from keystoneauth1 import session as ksc_session
36-
from keystoneauth1.exceptions.discovery import DiscoveryFailure
37-
3831
from prometheus_client import Counter, Gauge, Histogram, generate_latest, CONTENT_TYPE_LATEST
3932

4033

@@ -1761,6 +1754,7 @@ def _host_evacuate(connection, failed_service, service, target_host=None) -> boo
17611754

17621755
def _server_evacuate(connection, server, target_host=None, server_obj=None) -> EvacuationResult:
17631756
"""Evacuate a single server instance, returning EvacuationResult."""
1757+
from novaclient.exceptions import Conflict, NotFound, Forbidden, Unauthorized
17641758
success = False
17651759
error_message = ""
17661760
resp_status_code = None
@@ -2017,6 +2011,11 @@ def _server_evacuate_future(connection, server, target_host=None,
20172011
def _nova_login(plugin_name: str, auth_kwargs: dict, region_name: str,
20182012
ca_bundle: Optional[str] = None) -> OpenStackClient:
20192013
"""Create and return Nova client connection. Raises NovaConnectionError on failure."""
2014+
from keystoneauth1 import loading
2015+
from keystoneauth1 import session as ksc_session
2016+
from keystoneauth1.exceptions.discovery import DiscoveryFailure
2017+
from novaclient import client
2018+
from novaclient.exceptions import Unauthorized
20202019
try:
20212020
loader = loading.get_plugin_loader(plugin_name)
20222021
auth = loader.load_from_options(**auth_kwargs)
@@ -2055,15 +2054,14 @@ def nova_login_ac(credentials: ACLoginCredentials, ca_bundle: Optional[str] = No
20552054
}, credentials.region_name, ca_bundle)
20562055

20572056

2058-
NOVA_EXCEPTION_MESSAGES = {
2059-
NotFound: "Resource not found",
2060-
Conflict: "Conflicting operation",
2061-
}
2062-
2063-
20642057
def _handle_nova_exception(operation: str, service_info: str, e: Exception, is_critical: bool = True) -> bool:
20652058
"""Handle Nova API exceptions with consistent logging."""
2066-
error_msg = NOVA_EXCEPTION_MESSAGES.get(type(e))
2059+
from novaclient.exceptions import NotFound, Conflict
2060+
nova_exception_messages = {
2061+
NotFound: "Resource not found",
2062+
Conflict: "Conflicting operation",
2063+
}
2064+
error_msg = nova_exception_messages.get(type(e))
20672065

20682066
if error_msg:
20692067
logging.error("Failed to %s for %s. %s: %s", operation, service_info, error_msg, type(e).__name__)
@@ -3558,6 +3556,9 @@ def _sigterm_handler(signum, frame):
35583556

35593557
consecutive_failures = 0
35603558

3559+
from novaclient.exceptions import Unauthorized
3560+
from keystoneauth1.exceptions.discovery import DiscoveryFailure
3561+
35613562
while not service.shutdown_event.is_set():
35623563
service.update_health_hash()
35633564
poll_interval = service.config.get_config_value('POLL')

test/instanceha/conftest.py

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,18 @@
66

77
import os
88
import sys
9-
from unittest.mock import MagicMock
9+
from contextlib import contextmanager
10+
from unittest.mock import MagicMock, patch
1011

1112

1213
# --- Mock OpenStack dependencies before any test file imports instanceha ---
1314

1415
if 'novaclient' not in sys.modules:
15-
sys.modules['novaclient'] = MagicMock()
16-
sys.modules['novaclient.client'] = MagicMock()
16+
_novaclient_mock = MagicMock()
17+
_novaclient_client_mock = MagicMock()
18+
_novaclient_mock.client = _novaclient_client_mock
19+
sys.modules['novaclient'] = _novaclient_mock
20+
sys.modules['novaclient.client'] = _novaclient_client_mock
1721

1822
class NotFound(Exception):
1923
pass
@@ -32,12 +36,18 @@ class Unauthorized(Exception):
3236
novaclient_exceptions.Conflict = Conflict
3337
novaclient_exceptions.Forbidden = Forbidden
3438
novaclient_exceptions.Unauthorized = Unauthorized
39+
_novaclient_mock.exceptions = novaclient_exceptions
3540
sys.modules['novaclient.exceptions'] = novaclient_exceptions
3641

3742
if 'keystoneauth1' not in sys.modules:
38-
sys.modules['keystoneauth1'] = MagicMock()
39-
sys.modules['keystoneauth1.loading'] = MagicMock()
40-
sys.modules['keystoneauth1.session'] = MagicMock()
43+
_keystoneauth_mock = MagicMock()
44+
_loading_mock = MagicMock()
45+
_session_mock = MagicMock()
46+
_keystoneauth_mock.loading = _loading_mock
47+
_keystoneauth_mock.session = _session_mock
48+
sys.modules['keystoneauth1'] = _keystoneauth_mock
49+
sys.modules['keystoneauth1.loading'] = _loading_mock
50+
sys.modules['keystoneauth1.session'] = _session_mock
4151

4252
class DiscoveryFailure(Exception):
4353
pass
@@ -48,6 +58,7 @@ class DiscoveryFailure(Exception):
4858
exceptions_module = MagicMock()
4959
exceptions_module.discovery = discovery_module
5060

61+
_keystoneauth_mock.exceptions = exceptions_module
5162
sys.modules['keystoneauth1.exceptions'] = exceptions_module
5263
sys.modules['keystoneauth1.exceptions.discovery'] = discovery_module
5364

@@ -59,3 +70,32 @@ class DiscoveryFailure(Exception):
5970
_abs_path = os.path.abspath(_instanceha_path)
6071
if _abs_path not in sys.path:
6172
sys.path.insert(0, _abs_path)
73+
74+
75+
# --- Shared test fixtures ---
76+
77+
import instanceha # noqa: E402
78+
79+
80+
@contextmanager
81+
def patch_pipeline(conn=None, fence=True, disable=True,
82+
reserved=None, evacuate=True, recovery=True):
83+
"""Patch all process_service pipeline steps with configurable return values.
84+
85+
Yields a dict of mock objects keyed by step name, so tests can assert
86+
on individual steps (e.g. mocks['disable'].assert_not_called()).
87+
"""
88+
if reserved is None:
89+
reserved = instanceha.ReservedHostResult(success=True, hostname=None)
90+
with patch('instanceha._get_nova_connection', return_value=conn) as m_conn, \
91+
patch('instanceha._host_fence', return_value=fence) as m_fence, \
92+
patch('instanceha._host_disable', return_value=disable) as m_disable, \
93+
patch('instanceha._manage_reserved_hosts', return_value=reserved) as m_reserved, \
94+
patch('instanceha._host_evacuate', return_value=evacuate) as m_evacuate, \
95+
patch('instanceha._post_evacuation_recovery', return_value=recovery) as m_recovery, \
96+
patch('instanceha.track_host_processing') as m_track:
97+
yield {
98+
'conn': m_conn, 'fence': m_fence, 'disable': m_disable,
99+
'reserved': m_reserved, 'evacuate': m_evacuate,
100+
'recovery': m_recovery, 'track': m_track,
101+
}

test/instanceha/test_coverage_gaps.py

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -761,10 +761,6 @@ def test_timeout(self):
761761
# Main loop backoff tests
762762
# ============================================================================
763763

764-
class _TestDiscoveryFailure(Exception):
765-
pass
766-
767-
768764
class TestMainLoopBackoff(unittest.TestCase):
769765
"""Tests for main() exponential backoff on Nova failures."""
770766

@@ -782,12 +778,13 @@ def _make_service_mock(self):
782778
svc.shutdown_event = threading.Event()
783779
return svc
784780

785-
@patch('instanceha.DiscoveryFailure', _TestDiscoveryFailure)
786781
@patch('instanceha.signal.signal')
787782
@patch('instanceha._establish_nova_connection')
788783
@patch('instanceha._initialize_service')
789784
@patch('instanceha.ConfigManager')
790785
def test_unauthorized_triggers_reconnection(self, mock_cm, mock_init, mock_conn, mock_signal):
786+
from novaclient.exceptions import Unauthorized
787+
791788
mock_cm.return_value.get_config_value = Mock(return_value='INFO')
792789
service = self._make_service_mock()
793790
mock_init.return_value = service
@@ -799,14 +796,13 @@ def unauthorized_then_stop(*args, **kwargs):
799796
call_count[0] += 1
800797
if call_count[0] >= 2:
801798
service.shutdown_event.set()
802-
raise instanceha.Unauthorized('expired')
799+
raise Unauthorized('expired')
803800
conn.services.list.side_effect = unauthorized_then_stop
804801

805802
instanceha.main()
806803

807804
mock_conn.assert_called()
808805

809-
@patch('instanceha.DiscoveryFailure', _TestDiscoveryFailure)
810806
@patch('instanceha.signal.signal')
811807
@patch('instanceha._establish_nova_connection')
812808
@patch('instanceha._initialize_service')
@@ -830,7 +826,6 @@ def error_then_stop(*args, **kwargs):
830826

831827
self.assertGreaterEqual(call_count[0], 2)
832828

833-
@patch('instanceha.DiscoveryFailure', _TestDiscoveryFailure)
834829
@patch('instanceha.signal.signal')
835830
@patch('instanceha._process_reenabling')
836831
@patch('instanceha._process_stale_services')
@@ -860,7 +855,6 @@ def services_then_stop(*args, **kwargs):
860855

861856
mock_cat.assert_called()
862857

863-
@patch('instanceha.DiscoveryFailure', _TestDiscoveryFailure)
864858
@patch('instanceha.signal.signal')
865859
@patch('instanceha._establish_nova_connection')
866860
@patch('instanceha._initialize_service')

test/instanceha/test_critical_error_paths.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -178,12 +178,12 @@ def test_nova_login_discovery_failure(self):
178178
"""Test Nova login with discovery failure."""
179179
from keystoneauth1.exceptions.discovery import DiscoveryFailure
180180

181-
with patch('instanceha.loading.get_plugin_loader') as mock_loader:
181+
with patch('keystoneauth1.loading.get_plugin_loader') as mock_loader:
182182
mock_auth = Mock()
183183
mock_loader.return_value.load_from_options.return_value = mock_auth
184184

185-
with patch('instanceha.ksc_session.Session') as mock_session:
186-
with patch('instanceha.client.Client') as mock_client:
185+
with patch('keystoneauth1.session.Session') as mock_session:
186+
with patch('novaclient.client.Client') as mock_client:
187187
mock_nova = Mock()
188188
mock_nova.versions.get_current.side_effect = DiscoveryFailure('Discovery failed')
189189
mock_client.return_value = mock_nova
@@ -200,12 +200,12 @@ def test_nova_login_unauthorized(self):
200200
"""Test Nova login with unauthorized exception."""
201201
from novaclient.exceptions import Unauthorized
202202

203-
with patch('instanceha.loading.get_plugin_loader') as mock_loader:
203+
with patch('keystoneauth1.loading.get_plugin_loader') as mock_loader:
204204
mock_auth = Mock()
205205
mock_loader.return_value.load_from_options.return_value = mock_auth
206206

207-
with patch('instanceha.ksc_session.Session') as mock_session:
208-
with patch('instanceha.client.Client') as mock_client:
207+
with patch('keystoneauth1.session.Session') as mock_session:
208+
with patch('novaclient.client.Client') as mock_client:
209209
mock_nova = Mock()
210210
mock_nova.versions.get_current.side_effect = Unauthorized('Bad credentials')
211211
mock_client.return_value = mock_nova

0 commit comments

Comments
 (0)