Added snap install + upgrade libs and deps (#48)

### Issue: This PR implements [DPE-1261](https://warthogs.atlassian.net/browse/DPE-1261), namely this PR implements: - completion of the implementation of the snap based distro for the charm - minor refactoring of service related methods (start / stop) - upgrade of: - python dependencies - charm libs - removal of `_create_directories` patches on unit tests. ### Notes: **Only** review the following (the rest is charm libs upgrades): - `lib/charms/opensearch/v0/opensearch_distro.py` - `src/opensearch.py` - `src/charm.py` - `metadata.yaml` - `requirements.txt` - unit tests (removal of `_create_directories` patches from unit tests) [DPE-1261]: https://warthogs.atlassian.net/browse/DPE-1261?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
canonical · Mar 7, 2023 · 28af8f7 · 28af8f7
1 parent a83dab0
commit 28af8f7
Show file tree

Hide file tree

Showing 19 changed files with 196 additions and 106 deletions.
diff --git a/lib/charms/data_platform_libs/v0/data_interfaces.py b/lib/charms/data_platform_libs/v0/data_interfaces.py
@@ -1,4 +1,4 @@
-# Copyright 2022 Canonical Ltd.
+# Copyright 2023 Canonical Ltd.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -280,7 +280,6 @@ def _on_topic_requested(self, event: TopicRequestedEvent):
 
 import json
 import logging
-import os
 from abc import ABC, abstractmethod
 from collections import namedtuple
 from datetime import datetime
@@ -304,7 +303,9 @@ def _on_topic_requested(self, event: TopicRequestedEvent):
 
 # Increment this PATCH version before using `charmcraft publish-lib` or reset
 # to 0 if you are raising the major API version
-LIBPATCH = 4
+LIBPATCH = 7
+
+PYDEPS = ["ops>=2.0.0"]
 
 logger = logging.getLogger(__name__)
 
@@ -501,12 +502,6 @@ def fetch_relation_data(self) -> dict:
             a dict of the values stored in the relation data bag
                 for all relation instances (indexed by the relation ID).
         """
-        if "-relation-broken" in os.environ.get("JUJU_HOOK_NAME", ""):
-            # read more in https://bugs.launchpad.net/juju/+bug/1960934
-            raise RuntimeError(
-                "`fetch_relation_data` cannot be used in `*-relation-broken` events"
-            )
-
         data = {}
         for relation in self.relations:
             data[relation.id] = {
@@ -544,7 +539,19 @@ def _diff(self, event: RelationChangedEvent) -> Diff:
     @property
     def relations(self) -> List[Relation]:
         """The list of Relation instances associated with this relation_name."""
-        return list(self.charm.model.relations[self.relation_name])
+        return [
+            relation
+            for relation in self.charm.model.relations[self.relation_name]
+            if self._is_relation_active(relation)
+        ]
+
+    @staticmethod
+    def _is_relation_active(relation: Relation):
+        try:
+            _ = repr(relation.data)
+            return True
+        except RuntimeError:
+            return False
 
     @staticmethod
     def _is_resource_created_for_relation(relation: Relation):
@@ -564,12 +571,28 @@ def is_resource_created(self, relation_id: Optional[int] = None) -> bool:
 
         Returns:
             True or False
+
+        Raises:
+            IndexError: If relation_id is provided but that relation does not exist
         """
-        if relation_id:
-            return self._is_resource_created_for_relation(self.relations[relation_id])
+        if relation_id is not None:
+            try:
+                relation = [relation for relation in self.relations if relation.id == relation_id][
+                    0
+                ]
+                return self._is_resource_created_for_relation(relation)
+            except IndexError:
+                raise IndexError(f"relation id {relation_id} cannot be accessed")
         else:
-            return all(
-                [self._is_resource_created_for_relation(relation) for relation in self.relations]
+            return (
+                all(
+                    [
+                        self._is_resource_created_for_relation(relation)
+                        for relation in self.relations
+                    ]
+                )
+                if self.relations
+                else False
             )
 
 

diff --git a/lib/charms/opensearch/v0/opensearch_distro.py b/lib/charms/opensearch/v0/opensearch_distro.py
@@ -2,7 +2,6 @@
 # See LICENSE file for licensing details.
 
 """Base class for Opensearch distributions."""
-
 import json
 import logging
 import os
@@ -11,9 +10,9 @@
 import subprocess
 import time
 from abc import ABC, abstractmethod
+from datetime import datetime
 from functools import cached_property
 from os.path import exists
-from pathlib import Path
 from typing import Dict, List, Optional, Set, Union
 
 import requests
@@ -29,6 +28,7 @@
     OpenSearchCmdError,
     OpenSearchError,
     OpenSearchHttpError,
+    OpenSearchStartTimeoutError,
 )
 from requests import HTTPError
 
@@ -78,22 +78,29 @@ class OpenSearchDistribution(ABC):
 
     def __init__(self, charm, peer_relation_name: str):
         self.paths = self._build_paths()
-        self._create_directories()
         self._set_env_variables()
 
         self.config = YamlConfigSetter(base_path=self.paths.conf)
         self._charm = charm
         self._peer_relation_name = peer_relation_name
 
-    @abstractmethod
     def install(self):
         """Install the package."""
         pass
 
-    @abstractmethod
     def start(self):
         """Start the opensearch service."""
-        pass
+        if self.is_started():
+            return
+
+        # start the opensearch service
+        self._start_service()
+
+        start = datetime.now()
+        while not self.is_node_up() and (datetime.now() - start).seconds < 75:
+            time.sleep(3)
+        else:
+            raise OpenSearchStartTimeoutError()
 
     def restart(self):
         """Restart the opensearch service."""
@@ -103,10 +110,19 @@ def restart(self):
         self.start()
 
     def stop(self):
-        """Stop OpenSearch.."""
+        """Stop OpenSearch."""
         # stop the opensearch service
         self._stop_service()
 
+        start = datetime.now()
+        while self.is_started() and (datetime.now() - start).seconds < 60:
+            time.sleep(3)
+
+    @abstractmethod
+    def _start_service(self):
+        """Start the opensearch service."""
+        pass
+
     @abstractmethod
     def _stop_service(self):
         """Stop the opensearch service."""
@@ -131,10 +147,9 @@ def is_node_up(self) -> bool:
             return False
 
         try:
-            self.request("GET", "/_nodes")
-            return True
-        except (OpenSearchHttpError, Exception) as e:
-            logger.exception(e)
+            resp_code = self.request("GET", "/_nodes", resp_status_code=True)
+            return resp_code < 400
+        except (OpenSearchHttpError, Exception):
             return False
 
     def run_bin(self, bin_script_name: str, args: str = None):
@@ -264,8 +279,7 @@ def call(
 
         return resp.json()
 
-    @staticmethod
-    def write_file(path: str, data: str, override: bool = True):
+    def write_file(self, path: str, data: str, override: bool = True):
         """Persists data into file. Useful for files generated on the fly, such as certs etc."""
         if not override and exists(path):
             return
@@ -315,11 +329,6 @@ def _build_paths(self) -> Paths:
         """Build the Paths object."""
         pass
 
-    def _create_directories(self) -> None:
-        """Create the directories defined in self.paths."""
-        for dir_path in self.paths.__dict__.values():
-            Path(dir_path).mkdir(parents=True, exist_ok=True)
-
     def _set_env_variables(self):
         """Set the necessary environment variables."""
         os.environ["OPENSEARCH_HOME"] = self.paths.home

diff --git a/lib/charms/operator_libs_linux/v0/passwd.py b/lib/charms/operator_libs_linux/v0/passwd.py
@@ -45,7 +45,7 @@
 
 # Increment this PATCH version before using `charmcraft publish-lib` or reset
 # to 0 if you are raising the major API version
-LIBPATCH = 3
+LIBPATCH = 4
 
 
 def user_exists(user: Union[str, int]) -> Optional[pwd.struct_passwd]:
@@ -99,6 +99,7 @@ def add_user(
     secondary_groups: List[str] = None,
     uid: int = None,
     home_dir: str = None,
+    create_home: bool = True,
 ) -> str:
     """Add a user to the system.
 
@@ -113,6 +114,7 @@ def add_user(
         secondary_groups: Optional list of additional groups
         uid: UID for user being created
         home_dir: Home directory for user
+        create_home: Force home directory creation
 
     Returns:
         The password database entry struct, as returned by `pwd.getpwnam`
@@ -135,7 +137,9 @@ def add_user(
     if home_dir:
         cmd.extend(["--home", str(home_dir)])
     if password:
-        cmd.extend(["--password", password, "--create-home"])
+        cmd.extend(["--password", password])
+    if create_home:
+        cmd.append("--create-home")
     if system_user or password is None:
         cmd.append("--system")
 

diff --git a/lib/charms/operator_libs_linux/v1/snap.py b/lib/charms/operator_libs_linux/v1/snap.py
@@ -60,6 +60,7 @@
 import json
 import logging
 import os
+import re
 import socket
 import subprocess
 import sys
@@ -82,7 +83,11 @@
 
 # Increment this PATCH version before using `charmcraft publish-lib` or reset
 # to 0 if you are raising the major API version
-LIBPATCH = 5
+LIBPATCH = 7
+
+
+# Regex to locate 7-bit C1 ANSI sequences
+ansi_filter = re.compile(r"\x1B(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])")
 
 
 def _cache_init(func):
@@ -284,7 +289,15 @@ def _snap_daemons(
         command: List[str],
         services: Optional[List[str]] = None,
     ) -> CompletedProcess:
+        """Perform snap app commands.
+
+        Args:
+          command: the snap command to execute
+          services: the snap service to execute command on
 
+        Raises:
+          SnapError if there is a problem encountered
+        """
         if services:
             # an attempt to keep the command constrained to the snap instance's services
             services = ["{}.{}".format(self._name, service) for service in services]
@@ -355,6 +368,32 @@ def logs(self, services: Optional[List[str]] = None, num_lines: Optional[int] =
         args = ["logs", "-n={}".format(num_lines)] if num_lines else ["logs"]
         return self._snap_daemons(args, services).stdout
 
+    def connect(
+        self, plug: str, service: Optional[str] = None, slot: Optional[str] = None
+    ) -> None:
+        """Connects a plug to a slot.
+
+        Args:
+            plug (str): the plug to connect
+            service (str): (optional) the snap service name to plug into
+            slot (str): (optional) the snap service slot to plug in to
+
+        Raises:
+            SnapError if there is a problem encountered
+        """
+        command = ["connect", "{}:{}".format(self._name, plug)]
+
+        if service and slot:
+            command = command + ["{}:{}".format(service, slot)]
+        elif slot:
+            command = command + [slot]
+
+        _cmd = ["snap", *command]
+        try:
+            subprocess.run(_cmd, universal_newlines=True, check=True, capture_output=True)
+        except CalledProcessError as e:
+            raise SnapError("Could not {} for snap [{}]: {}".format(_cmd, self._name, e.stderr))
+
     def restart(
         self, services: Optional[List[str]] = None, reload: Optional[bool] = False
     ) -> None:
@@ -907,12 +946,19 @@ def install_local(
     if dangerous:
         _cmd.append("--dangerous")
     try:
-        result = subprocess.check_output(_cmd, universal_newlines=True).splitlines()[0]
+        result = subprocess.check_output(_cmd, universal_newlines=True).splitlines()[-1]
         snap_name, _ = result.split(" ", 1)
+        snap_name = ansi_filter.sub("", snap_name)
 
         c = SnapCache()
 
-        return c[snap_name]
+        try:
+            return c[snap_name]
+        except SnapAPIError as e:
+            logger.error(
+                "Could not find snap {} when querying Snapd socket: {}".format(snap_name, e.body)
+            )
+            raise SnapError("Failed to find snap {} in Snap cache".format(snap_name))
     except CalledProcessError as e:
         raise SnapError("Could not install snap {}: {}".format(filename, e.output))
 

diff --git a/lib/charms/operator_libs_linux/v1/systemd.py b/lib/charms/operator_libs_linux/v1/systemd.py
@@ -60,7 +60,7 @@
 
 # Increment this PATCH version before using `charmcraft publish-lib` or reset
 # to 0 if you are raising the major API version
-LIBPATCH = 0
+LIBPATCH = 1
 
 
 class SystemdError(Exception):
@@ -131,7 +131,7 @@ def service_running(service_name: str) -> bool:
     """Determine whether a system service is running.
 
     Args:
-        service_name: the name of the service
+        service_name: the name of the service to check
     """
     return _systemctl("is-active", service_name, quiet=True)
 
@@ -140,7 +140,7 @@ def service_start(service_name: str) -> bool:
     """Start a system service.
 
     Args:
-        service_name: the name of the service to stop
+        service_name: the name of the service to start
     """
     return _systemctl("start", service_name)
 

diff --git a/lib/charms/tls_certificates_interface/v1/tls_certificates.py b/lib/charms/tls_certificates_interface/v1/tls_certificates.py
@@ -1,6 +1,7 @@
 # Copyright 2021 Canonical Ltd.
 # See LICENSE file for licensing details.
 
+
 """Library for the tls-certificates relation.
 
 This library contains the Requires and Provides classes for handling the tls-certificates
@@ -271,7 +272,8 @@ def _on_certificate_revoked(self, event: CertificateRevokedEvent) -> None:
 
 # Increment this PATCH version before using `charmcraft publish-lib` or reset
 # to 0 if you are raising the major API version
-LIBPATCH = 11
+LIBPATCH = 12
+
 
 REQUIRER_JSON_SCHEMA = {
     "$schema": "http://json-schema.org/draft-04/schema#",
@@ -1267,7 +1269,7 @@ def _relation_data_is_valid(certificates_data: dict) -> bool:
             return False
 
     def _on_relation_changed(self, event: RelationChangedEvent) -> None:
-        """Handler triggerred on relation changed events.
+        """Handler triggered on relation changed events.
 
         Args:
             event: Juju event

diff --git a/metadata.yaml b/metadata.yaml
@@ -27,4 +27,4 @@ requires:
 storage:
   opensearch-data:
     type: filesystem
-    location: /mnt/opensearch/data  # for snaps: /var/snap/opensearch/common/data
+    location: /var/snap/opensearch/common  # /mnt/opensearch/data