Skip to content

Commit

Permalink
feat: add exception handling while ns check
Browse files Browse the repository at this point in the history
- check if ns is SC
- catches exception while SC check
- fix the existing test cases for new SC check
  • Loading branch information
a-spiker committed Jan 23, 2025
1 parent a711390 commit a163da5
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 29 deletions.
7 changes: 3 additions & 4 deletions lib/live_cluster/client/node.py
Original file line number Diff line number Diff line change
Expand Up @@ -2401,12 +2401,11 @@ async def info_cluster_stable(
resp = await self._info(req)

if "error" in resp.lower():
if "cluster not specified size" in resp or "unstable cluster" in resp:
raise ASInfoClusterStableError(resp)

raise ASInfoResponseError(
info_server_response_err = ASInfoResponseError(
ErrorsMsgs.INFO_SERVER_ERROR_RESPONSE, resp
)
logger.error(info_server_response_err)
raise info_server_response_err

return resp

Expand Down
56 changes: 44 additions & 12 deletions lib/live_cluster/manage_controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
from lib.base_controller import CommandHelp, ModifierHelp, ShellException
from lib.utils.lookup_dict import PrefixDict
from .client import (
ASInfoClusterStableError,
ASInfoResponseError,
ASInfoError,
ASProtocolError,
BoolConfigType,
Expand Down Expand Up @@ -2721,7 +2721,7 @@ def _check_and_log_cluster_stable(self, stable_data):
warning_str = "The cluster is unstable. It is advised that you do not manage the roster. Run 'info network' for more information."

for resp in stable_data.values():
if isinstance(resp, ASInfoClusterStableError):
if isinstance(resp, ASInfoResponseError):
logger.warning(warning_str)
return False

Expand Down Expand Up @@ -2753,16 +2753,21 @@ async def _check_ns_is_strong_consistency(self, ns):
"""
Check if a namespace is in strong consistency mode.
"""
namespace_stats = await self.cluster.info_namespace_statistics(ns, nodes='all')
namespace_stats = list(namespace_stats.values())[0]
if not namespace_stats or namespace_stats is None:
logger.error("namespace {} not does not exist".format(ns))
return False
try:
namespace_stats = await self.cluster.info_namespace_statistics(ns, nodes='all')
namespace_stats = list(namespace_stats.values())[0] if len(namespace_stats.values()) > 0 else None
if not namespace_stats or namespace_stats is None:
logger.error("namespace {} not does not exist".format(ns))
return False

strong_consistency = namespace_stats.get("strong-consistency", "false").lower() == 'true'
if strong_consistency is False:
logger.error("namespace {} is not in strong consistency mode".format(ns))
return strong_consistency

strong_consistency = namespace_stats.get("strong-consistency", "false").lower() == 'true'
if strong_consistency is False:
logger.error("namespace {} is not in strong consistency mode".format(ns))
return strong_consistency
except Exception as e:
logger.error("Error while checking namespace strong consistency mode: {}".format(e))
raise ASInfoError("Error while checking namespace strong consistency mode", e)

return strong_consistency

Expand Down Expand Up @@ -2806,6 +2811,13 @@ async def _do_default(self, line):
modifiers=self.modifiers,
mods=self.mods,
)

# to be run against a SC namespace only
ns_strong_consistency = await self._check_ns_is_strong_consistency(ns)
if isinstance(ns_strong_consistency, ASInfoError):
return
elif not ns_strong_consistency:
return

current_roster = asyncio.create_task(
self.cluster.info_roster(ns, nodes="principal")
Expand Down Expand Up @@ -2881,6 +2893,14 @@ async def _do_default(self, line):
modifiers=self.modifiers,
mods=self.mods,
)

# to be run against a SC namespace only
ns_strong_consistency = await self._check_ns_is_strong_consistency(ns)
if isinstance(ns_strong_consistency, ASInfoError):
return
elif not ns_strong_consistency:
return

current_roster = asyncio.create_task(
self.cluster.info_roster(ns, nodes="principal")
)
Expand Down Expand Up @@ -2974,6 +2994,13 @@ async def _do_default(self, line):
mods=self.mods,
)

# to be run against a SC namespace only
ns_strong_consistency = await self._check_ns_is_strong_consistency(ns)
if isinstance(ns_strong_consistency, ASInfoError):
return
elif not ns_strong_consistency:
return

if warn:
current_roster = asyncio.create_task(
self.cluster.info_roster(ns, nodes="principal")
Expand Down Expand Up @@ -3031,9 +3058,14 @@ def __init__(self):

async def _do_default(self, line):
ns = self.mods["ns"][0]

# to be run against a SC namespace only
ns_strong_consistency = await self._check_ns_is_strong_consistency(ns)
if not ns_strong_consistency:
if isinstance(ns_strong_consistency, ASInfoError):
return
elif not ns_strong_consistency:
return

current_roster = asyncio.create_task(
self.cluster.info_roster(ns, nodes="principal")
)
Expand Down
6 changes: 4 additions & 2 deletions test/unit/live_cluster/client/test_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
ASInfoConfigError,
ASInfoResponseError,
)
from lib.live_cluster.client.constants import ErrorsMsgs

with warnings.catch_warnings():
warnings.filterwarnings("ignore", category=DeprecationWarning)
Expand Down Expand Up @@ -2053,9 +2054,10 @@ async def test_info_cluster_stable(self):

async def test_info_cluster_stable_with_errors(self):
self.info_mock.return_value = "ERROR::cluster not specified size"
expected = ASInfoClusterStableError("ERROR::cluster not specified size")
expected = ASInfoResponseError(ErrorsMsgs.INFO_SERVER_ERROR_RESPONSE, "ERROR::cluster not specified size")

actual = await self.node.info_cluster_stable(cluster_size=3, namespace="bar")
print("LOLLL", actual)

self.info_mock.assert_called_with(
"cluster-stable:size=3;namespace=bar", self.ip
Expand All @@ -2067,7 +2069,7 @@ async def test_info_cluster_stable_with_errors(self):
)

self.info_mock.return_value = "ERROR::unstable cluster"
expected = ASInfoClusterStableError("ERROR::unstable cluster")
expected = ASInfoResponseError(ErrorsMsgs.INFO_SERVER_ERROR_RESPONSE, "ERROR::unstable cluster")

actual = await self.node.info_cluster_stable(cluster_size=3, namespace="bar")

Expand Down
72 changes: 61 additions & 11 deletions test/unit/live_cluster/test_manage_controller.py
Original file line number Diff line number Diff line change
Expand Up @@ -2693,7 +2693,7 @@ def __init__(self, input, output):
{
"1.1.1.1": "ABC",
"2.2.2.2": "ABC",
"3.3.3.3": ASInfoClusterStableError("foo"),
"3.3.3.3": ASInfoResponseError("", "foo"),
},
False,
),
Expand All @@ -2713,16 +2713,6 @@ def __init__(self, input, output):
"The cluster is unstable. It is advised that you do not manage the roster. Run 'info network' for more information."
)

self.assertRaises(
ASInfoResponseError,
self.controller._check_and_log_cluster_stable,
{
"1.1.1.1": "ABC",
"2.2.2.2": "ABC",
"3.3.3.3": ASInfoResponseError("", "foo"),
},
)

def test_check_and_log_nodes_in_observed(self):
class test_case:
def __init__(self, observed, nodes, output, warning=None):
Expand Down Expand Up @@ -2795,6 +2785,9 @@ async def test_success(self):
"1.1.1.1": {"pending_roster": ["GHI"], "observed_nodes": []}
}
self.cluster_mock.info_roster_set.return_value = {"1.1.1.1": ASINFO_RESPONSE_OK}
self.cluster_mock.info_namespace_statistics.return_value = {
"1.1.1.1": { "strong-consistency": "true" }
}

await self.controller.execute(line.split())

Expand All @@ -2811,6 +2804,9 @@ async def test_success(self):
async def test_logs_error_from_roster(self):
line = "nodes ABC@rack1 DEF@rack2 ns test --no-warn"
error = ASInfoResponseError("blah", "error::foo")
self.cluster_mock.info_namespace_statistics.return_value = {
"1.1.1.1": { "strong-consistency": "true" }
}
self.cluster_mock.info_roster.return_value = {"1.1.1.1": error}

await self.controller.execute(line.split())
Expand All @@ -2822,6 +2818,9 @@ async def test_logs_error_from_roster(self):
async def test_logs_error_from_roster_set(self):
error = ASInfoResponseError("blah", "error::foo")
line = "nodes ABC@rack1 DEF@rack2 ns test --no-warn"
self.cluster_mock.info_namespace_statistics.return_value = {
"1.1.1.1": { "strong-consistency": "true" }
}
self.cluster_mock.info_roster.return_value = {
"1.1.1.1": {"pending_roster": ["GHI"], "observed_nodes": []}
}
Expand All @@ -2837,6 +2836,9 @@ async def test_raises_error_from_roster(self):
error = Exception("test exception")
line = "nodes ABC@rack1 DEF@rack2 ns test --no-warn"
self.cluster_mock.info_roster.return_value = {"1.1.1.1": error}
self.cluster_mock.info_namespace_statistics.return_value = {
"1.1.1.1": { "strong-consistency": "true" }
}

await test_util.assert_exception_async(
self, Exception, "test exception", self.controller.execute, line.split()
Expand All @@ -2851,6 +2853,9 @@ async def test_raises_error_from_roster_set(self):
self.cluster_mock.info_roster.return_value = {
"1.1.1.1": {"pending_roster": ["GHI"], "observed_nodes": []}
}
self.cluster_mock.info_namespace_statistics.return_value = {
"1.1.1.1": { "strong-consistency": "true" }
}
self.cluster_mock.info_roster_set.return_value = {"1.1.1.1": error}

await test_util.assert_exception_async(
Expand All @@ -2865,6 +2870,9 @@ async def test_warn_returns_false(self):
self.controller.warn = True
self.prompt_mock.return_value = False
self.cluster_mock.info_cluster_stable.return_value = {"1.1.1.1": "ABCDEF"}
self.cluster_mock.info_namespace_statistics.return_value = {
"1.1.1.1": { "strong-consistency": "true" }
}
self.cluster_mock.info_roster.return_value = {
"1.1.1.1": {"pending_roster": ["GHI"], "observed_nodes": ["foo"]}
}
Expand All @@ -2890,6 +2898,9 @@ async def test_warn_returns_true(self):
self.cluster_mock.info_roster.return_value = {
"1.1.1.1": {"pending_roster": ["GHI"], "observed_nodes": ["bar"]}
}
self.cluster_mock.info_namespace_statistics.return_value = {
"1.1.1.1": { "strong-consistency": "true" }
}
self.cluster_mock.info_roster_set.return_value = {"1.1.1.1": ASINFO_RESPONSE_OK}

await self.controller.execute(line.split())
Expand Down Expand Up @@ -2931,6 +2942,9 @@ async def test_success(self):
"1.1.1.1": {"pending_roster": ["ABC", "DEF"], "observed_nodes": []}
}
self.cluster_mock.info_roster_set.return_value = {"1.1.1.1": ASINFO_RESPONSE_OK}
self.cluster_mock.info_namespace_statistics.return_value = {
"1.1.1.1": { "strong-consistency": "true" }
}

await self.controller.execute(line.split())

Expand All @@ -2948,6 +2962,9 @@ async def test_logs_error_from_roster(self):
line = "nodes ABC@rack1 DEF@rack2 ns test --no-warn"
error = ASInfoResponseError("blah", "error::foo")
self.cluster_mock.info_roster.return_value = {"1.1.1.1": error}
self.cluster_mock.info_namespace_statistics.return_value = {
"1.1.1.1": { "strong-consistency": "true" }
}

await self.controller.execute(line.split())

Expand All @@ -2964,6 +2981,9 @@ async def test_logs_error_from_roster_set(self):
"observed_nodes": [],
}
}
self.cluster_mock.info_namespace_statistics.return_value = {
"1.1.1.1": { "strong-consistency": "true" }
}
self.cluster_mock.info_roster_set.return_value = {"1.1.1.1": error}

await self.controller.execute(line.split())
Expand All @@ -2978,6 +2998,9 @@ async def test_logs_error_when_node_not_in_pending(self):
"1.1.1.1": {"pending_roster": ["ABC", "DEF"], "observed_nodes": []}
}
self.cluster_mock.info_cluster_stable.return_value = {"1.1.1.1": "CDEF"}
self.cluster_mock.info_namespace_statistics.return_value = {
"1.1.1.1": { "strong-consistency": "true" }
}
self.prompt_mock.return_value = False
self.cluster_mock.info_roster_set.return_value = {"1.1.1.1": ASINFO_RESPONSE_OK}

Expand All @@ -2996,6 +3019,9 @@ async def test_raises_error_from_roster(self):
error = Exception("test exception")
line = "nodes ABC@rack1 DEF@rack2 ns test --no-warn"
self.cluster_mock.info_roster.return_value = {"1.1.1.1": error}
self.cluster_mock.info_namespace_statistics.return_value = {
"1.1.1.1": { "strong-consistency": "true" }
}

await test_util.assert_exception_async(
self, Exception, "test exception", self.controller.execute, line.split()
Expand All @@ -3007,6 +3033,9 @@ async def test_raises_error_from_roster(self):
async def test_raises_error_from_roster_set(self):
error = Exception("test exception")
line = "nodes ABC@rack1 DEF@rack2 ns test --no-warn"
self.cluster_mock.info_namespace_statistics.return_value = {
"1.1.1.1": { "strong-consistency": "true" }
}
self.cluster_mock.info_roster.return_value = {
"1.1.1.1": {
"pending_roster": ["GHI", "ABC@rack1", "DEF@rack2"],
Expand All @@ -3027,6 +3056,9 @@ async def test_warn_returns_false(self):
self.controller.warn = True
self.prompt_mock.return_value = False
self.cluster_mock.info_cluster_stable.return_value = {"1.1.1.1": "ABCDEF"}
self.cluster_mock.info_namespace_statistics.return_value = {
"1.1.1.1": { "strong-consistency": "true" }
}
self.cluster_mock.info_roster.return_value = {
"1.1.1.1": {
"pending_roster": ["ABC@rack1", "DEF@rack2", "GHI"],
Expand All @@ -3049,6 +3081,9 @@ async def test_warn_returns_true(self):
self.controller.warn = True
self.prompt_mock.return_value = True
self.cluster_mock.info_cluster_stable.return_value = {"1.1.1.1": "ABCDEF"}
self.cluster_mock.info_namespace_statistics.return_value = {
"1.1.1.1": { "strong-consistency": "true" }
}
self.cluster_mock.info_roster.return_value = {
"1.1.1.1": {
"pending_roster": ["ABC@rack1", "DEF@rack2", "GHI"],
Expand Down Expand Up @@ -3096,6 +3131,9 @@ def setUp(self) -> None:
async def test_success(self):
line = "ABC@rack1 DEF@rack2 ns test --no-warn"
self.cluster_mock.info_roster_set.return_value = {"1.1.1.1": ASINFO_RESPONSE_OK}
self.cluster_mock.info_namespace_statistics.return_value = {
"1.1.1.1": { "strong-consistency": "true" }
}

await self.controller.execute(line.split())

Expand All @@ -3110,6 +3148,9 @@ async def test_success(self):
async def test_logs_error_from_roster_set(self):
line = "ABC@rack1 DEF@rack2 ns test --no-warn"
error = ASInfoResponseError("blah", "error::foo")
self.cluster_mock.info_namespace_statistics.return_value = {
"1.1.1.1": { "strong-consistency": "true" }
}
self.cluster_mock.info_roster_set.return_value = {"1.1.1.1": error}

await self.controller.execute(line.split())
Expand All @@ -3124,6 +3165,9 @@ async def test_logs_error_from_roster_set(self):
async def test_raises_error_from_roster_set(self):
error = Exception("test exception")
line = "ABC@rack1 DEF@rack2 ns test --no-warn"
self.cluster_mock.info_namespace_statistics.return_value = {
"1.1.1.1": { "strong-consistency": "true" }
}
self.cluster_mock.info_cluster_stable.return_value = {"1.1.1.1": "ABCDF"}
self.cluster_mock.info_roster.return_value = {
"1.1.1.1": {
Expand All @@ -3147,6 +3191,9 @@ async def test_warn_returns_false(self):
self.controller.warn = True
self.prompt_mock.return_value = False
self.cluster_mock.info_cluster_stable.return_value = {"1.1.1.1": "ABCDEF"}
self.cluster_mock.info_namespace_statistics.return_value = {
"1.1.1.1": { "strong-consistency": "true" }
}
self.cluster_mock.info_roster.return_value = {
"1.1.1.1": {
"observed_nodes": [],
Expand All @@ -3171,6 +3218,9 @@ async def test_warn_returns_true(self):
self.controller.warn = True
self.prompt_mock.return_value = True
self.cluster_mock.info_cluster_stable.return_value = {"1.1.1.1": "ABCDEF"}
self.cluster_mock.info_namespace_statistics.return_value = {
"1.1.1.1": { "strong-consistency": "true" }
}
self.cluster_mock.info_roster.return_value = {
"1.1.1.1": {
"observed_nodes": ["jar"],
Expand Down

0 comments on commit a163da5

Please sign in to comment.