Skip to content

Commit

Permalink
Check if alt_hosts are reachable & online instead of reachable (#215)
Browse files Browse the repository at this point in the history
Definitions
Reachable: Successful socket connection
Online: Successful HTTP GET request to `/_nodes`

Fixes uncaught OpenSearchHttpError (status code 503): OpenSearch
Security not initialized. (Fixes transient integration test failure—test
1 from
#211 (comment))

---------

Co-authored-by: Mehdi Bendriss <[email protected]>
  • Loading branch information
carlcsaposs-canonical and Mehdi-Bendriss authored Apr 12, 2024
1 parent 755fe73 commit b699190
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 26 deletions.
20 changes: 1 addition & 19 deletions lib/charms/opensearch/v0/helper_http.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@
# See LICENSE file for licensing details.

"""File containing http related helpers."""
from typing import Any, Dict, List, Optional
from typing import Any, Dict, Optional

from charms.opensearch.v0.helper_networking import reachable_hosts
from tenacity import RetryCallState

# The unique Charmhub library identifier, never change it
Expand All @@ -31,20 +30,3 @@ def log_error(retry_state: RetryCallState):
)

return log_error


def full_urls(
primary_host: str, port: int, path: str, alt_hosts: List[str], check_hosts_reach: bool = False
) -> List[str]:
"""Returns a list of well formatted and potentially reachable hosts."""
target_hosts = [primary_host]
if alt_hosts:
target_hosts.extend([alt_host for alt_host in alt_hosts if alt_host != primary_host])

if not check_hosts_reach:
return target_hosts

return [
f"https://{host_candidate}:{port}/{path}"
for host_candidate in reachable_hosts(target_hosts)
]
24 changes: 17 additions & 7 deletions lib/charms/opensearch/v0/opensearch_distro.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
from charms.opensearch.v0.constants_secrets import ADMIN_PW
from charms.opensearch.v0.helper_cluster import Node
from charms.opensearch.v0.helper_conf_setter import YamlConfigSetter
from charms.opensearch.v0.helper_http import error_http_retry_log, full_urls
from charms.opensearch.v0.helper_http import error_http_retry_log
from charms.opensearch.v0.helper_networking import get_host_ip, is_reachable
from charms.opensearch.v0.opensearch_exceptions import (
OpenSearchCmdError,
Expand Down Expand Up @@ -153,13 +153,19 @@ def is_failed(self) -> bool:
"""Check if OpenSearch daemon has failed."""
pass

def is_node_up(self) -> bool:
"""Get status of current node. This assumes OpenSearch is Running."""
if not self.is_started():
def is_node_up(self, host: Optional[str] = None) -> bool:
"""Get status of node. This assumes OpenSearch is Running.
Defaults to this unit
"""
host = host or self.host
if not is_reachable(host, self.port):
return False

try:
resp_code = self.request("GET", "/_nodes", resp_status_code=True)
resp_code = self.request(
"GET", "/_nodes", host=host, check_hosts_reach=False, resp_status_code=True
)
return resp_code < 400
except (OpenSearchHttpError, Exception):
return False
Expand Down Expand Up @@ -228,7 +234,7 @@ def call(url: str) -> requests.Response:

request_kwargs = {
"method": method.upper(),
"url": urls[0],
"url": url,
"verify": f"{self.paths.certs}/chain.pem",
"headers": {
"Accept": "application/json",
Expand All @@ -251,7 +257,11 @@ def call(url: str) -> requests.Response:
if endpoint.startswith("/"):
endpoint = endpoint[1:]

urls = full_urls(host or self.host, self.port, endpoint, alt_hosts, check_hosts_reach)
urls = []
for host_candidate in (host or self.host, *(alt_hosts or [])):
if check_hosts_reach and not self.is_node_up(host_candidate):
continue
urls.append(f"https://{host_candidate}:{self.port}/{endpoint}")
if not urls:
raise OpenSearchHttpError(
f"Host {host or self.host}:{self.port} and alternative_hosts: {alt_hosts or []} not reachable."
Expand Down
1 change: 1 addition & 0 deletions tests/integration/relations/test_opensearch_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,7 @@ async def test_admin_relation(ops_test: OpsTest):
relation_id=admin_relation.id,
relation_name=ADMIN_RELATION_NAME,
)
logging.info(f"{run_bulk_read_index=}")
results = json.loads(run_bulk_read_index["results"])
logging.info(results)
artists = [
Expand Down
2 changes: 2 additions & 0 deletions tests/unit/lib/test_backups.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
from ops.testing import Harness

from charm import OpenSearchOperatorCharm
from tests.helpers import patch_network_get

TEST_BUCKET_NAME = "s3://bucket-test"
TEST_BASE_PATH = "/test"
Expand Down Expand Up @@ -444,6 +445,7 @@ def test_on_s3_broken_steps(
harness.charm.backup._execute_s3_broken_calls.assert_called_once()


@patch_network_get("1.1.1.1")
class TestBackups(unittest.TestCase):
maxDiff = None

Expand Down

0 comments on commit b699190

Please sign in to comment.