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.

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

fix logging (5/edge) - via copying solution over from VM charm #228

Merged
merged 1 commit into from
Nov 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
24 changes: 14 additions & 10 deletions lib/charms/mongodb/v0/mongodb_backups.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import re
import subprocess
import time
from typing import Dict, List
from typing import Dict, List, Optional

from charms.data_platform_libs.v0.s3 import CredentialsChangedEvent, S3Requirer
from charms.mongodb.v0.helpers import (
Expand All @@ -35,14 +35,14 @@
)

# The unique Charmhub library identifier, never change it
LIBID = "9f2b91c6128d48d6ba22724bf365da3b"
LIBID = "18c461132b824ace91af0d7abe85f40e"

# Increment this major API version when introducing breaking changes
LIBAPI = 0

# Increment this PATCH version before using `charmcraft publish-lib` or reset
# to 0 if you are raising the major API version
LIBPATCH = 2
LIBPATCH = 7

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -420,11 +420,15 @@ def _wait_pbm_status(self) -> None:
except ExecError as e:
self.charm.unit.status = BlockedStatus(process_pbm_error(e.stdout))

def _get_pbm_status(self) -> StatusBase:
def _get_pbm_status(self) -> Optional[StatusBase]:
"""Retrieve pbm status."""
if not self.charm.has_backup_service():
return WaitingStatus("waiting for pbm to start")

if not self.model.get_relation(S3_RELATION):
logger.info("No configurations for backups, not relation to s3-charm.")
return None

try:
previous_pbm_status = self.charm.unit.status
pbm_status = self.charm.run_pbm_command(PBM_STATUS_CMD)
Expand Down Expand Up @@ -505,7 +509,7 @@ def _try_to_restore(self, backup_id: str) -> None:

If PBM is resyncing, the function will retry to create backup
(up to BACKUP_RESTORE_MAX_ATTEMPTS times) with BACKUP_RESTORE_ATTEMPT_COOLDOWN
time between attepts.
time between attempts.

If PMB returen any other error, the function will raise RestoreError.
"""
Expand All @@ -523,7 +527,7 @@ def _try_to_restore(self, backup_id: str) -> None:
restore_cmd = restore_cmd + remapping_args.split(" ")
self.charm.run_pbm_command(restore_cmd)
except (subprocess.CalledProcessError, ExecError) as e:
if type(e) is subprocess.CalledProcessError:
if isinstance(e, subprocess.CalledProcessError):
error_message = e.output.decode("utf-8")
else:
error_message = str(e.stderr)
Expand All @@ -541,7 +545,7 @@ def _try_to_backup(self):

If PBM is resyncing, the function will retry to create backup
(up to BACKUP_RESTORE_MAX_ATTEMPTS times)
with BACKUP_RESTORE_ATTEMPT_COOLDOWN time between attepts.
with BACKUP_RESTORE_ATTEMPT_COOLDOWN time between attempts.

If PMB returen any other error, the function will raise BackupError.
"""
Expand All @@ -560,7 +564,7 @@ def _try_to_backup(self):
)
return backup_id_match.group("backup_id") if backup_id_match else "N/A"
except (subprocess.CalledProcessError, ExecError) as e:
if type(e) is subprocess.CalledProcessError:
if isinstance(e, subprocess.CalledProcessError):
error_message = e.output.decode("utf-8")
else:
error_message = str(e.stderr)
Expand Down Expand Up @@ -636,13 +640,13 @@ def _get_backup_restore_operation_result(self, current_pbm_status, previous_pbm_
to contain the operation type (backup/restore) and the backup id.
"""
if (
type(current_pbm_status) is type(previous_pbm_status)
isinstance(current_pbm_status, type(previous_pbm_status))
and current_pbm_status.message == previous_pbm_status.message
):
return f"Operation is still in progress: '{current_pbm_status.message}'"

if (
type(previous_pbm_status) is MaintenanceStatus
isinstance(previous_pbm_status, MaintenanceStatus)
and "backup id:" in previous_pbm_status.message
):
backup_id = previous_pbm_status.message.split("backup id:")[-1].strip()
Expand Down
18 changes: 18 additions & 0 deletions tests/unit/test_mongodb_backups.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,9 @@ def setUp(self):
@patch("charm.MongoDBCharm.run_pbm_command")
def test_get_pbm_status_snap_not_present(self, pbm_command, service):
"""Tests that when the snap is not present pbm is in blocked state."""
relation_id = self.harness.add_relation(RELATION_NAME, "s3-integrator")
self.harness.add_relation_unit(relation_id, "s3-integrator/0")

container = self.harness.model.unit.get_container("mongod")
self.harness.set_can_connect(container, True)
pbm_command.side_effect = ModelError("service pbm-agent not found")
Expand All @@ -53,6 +56,9 @@ def test_get_pbm_status_snap_not_present(self, pbm_command, service):
@patch("charm.MongoDBCharm.run_pbm_command")
def test_get_pbm_status_resync(self, pbm_command, service):
"""Tests that when pbm is resyncing that pbm is in waiting state."""
relation_id = self.harness.add_relation(RELATION_NAME, "s3-integrator")
self.harness.add_relation_unit(relation_id, "s3-integrator/0")

container = self.harness.model.unit.get_container("mongod")
self.harness.set_can_connect(container, True)
service.return_value = "pbm"
Expand All @@ -65,6 +71,9 @@ def test_get_pbm_status_resync(self, pbm_command, service):
@patch("charm.MongoDBCharm.run_pbm_command")
def test_get_pbm_status_running(self, pbm_command, service):
"""Tests that when pbm not running an op that pbm is in active state."""
relation_id = self.harness.add_relation(RELATION_NAME, "s3-integrator")
self.harness.add_relation_unit(relation_id, "s3-integrator/0")

container = self.harness.model.unit.get_container("mongod")
self.harness.set_can_connect(container, True)
service.return_value = "pbm"
Expand All @@ -75,6 +84,9 @@ def test_get_pbm_status_running(self, pbm_command, service):
@patch("charm.MongoDBCharm.run_pbm_command")
def test_get_pbm_status_incorrect_cred(self, pbm_command, service):
"""Tests that when pbm has incorrect credentials that pbm is in blocked state."""
relation_id = self.harness.add_relation(RELATION_NAME, "s3-integrator")
self.harness.add_relation_unit(relation_id, "s3-integrator/0")

container = self.harness.model.unit.get_container("mongod")
self.harness.set_can_connect(container, True)
service.return_value = "pbm"
Expand All @@ -87,6 +99,9 @@ def test_get_pbm_status_incorrect_cred(self, pbm_command, service):
@patch("charm.MongoDBCharm.run_pbm_command")
def test_get_pbm_status_incorrect_conf(self, pbm_command, service):
"""Tests that when pbm has incorrect configs that pbm is in blocked state."""
relation_id = self.harness.add_relation(RELATION_NAME, "s3-integrator")
self.harness.add_relation_unit(relation_id, "s3-integrator/0")

container = self.harness.model.unit.get_container("mongod")
self.harness.set_can_connect(container, True)
service.return_value = "pbm"
Expand Down Expand Up @@ -669,6 +684,9 @@ def test_remap_replicaset_remap_necessary(self, run_pbm_command):
@patch("charm.MongoDBCharm.run_pbm_command")
def test_get_pbm_status_backup(self, run_pbm_command, service):
"""Tests that when pbm running a backup that pbm is in maintenance state."""
relation_id = self.harness.add_relation(RELATION_NAME, "s3-integrator")
self.harness.add_relation_unit(relation_id, "s3-integrator/0")

service.return_value = "pbm"
run_pbm_command.return_value = '{"running":{"type":"backup","name":"2023-09-04T12:15:58Z","startTS":1693829759,"status":"oplog backup","opID":"64f5ca7e777e294530289465"}}'
self.assertTrue(
Expand Down