diff --git a/src/backups.py b/src/backups.py index 28902f8bd3..57fa33b872 100644 --- a/src/backups.py +++ b/src/backups.py @@ -21,10 +21,7 @@ from botocore.exceptions import ClientError, ConnectTimeoutError, ParamValidationError, SSLError from botocore.loaders import create_loader from botocore.regions import EndpointResolver -from charms.data_platform_libs.v0.s3 import ( - CredentialsChangedEvent, - S3Requirer, -) +from charms.data_platform_libs.v0.s3 import CredentialsChangedEvent, S3Requirer from charms.operator_libs_linux.v2 import snap from jinja2 import Template from ops.charm import ActionEvent, HookEvent @@ -108,11 +105,13 @@ def _tls_ca_chain_filename(self) -> str: return "" def _get_s3_session_resource(self, s3_parameters: dict): - session = Session( - aws_access_key_id=s3_parameters["access-key"], - aws_secret_access_key=s3_parameters["secret-key"], - region_name=s3_parameters["region"], - ) + kwargs = { + "aws_access_key_id": s3_parameters["access-key"], + "aws_secret_access_key": s3_parameters["secret-key"], + } + if "region" in s3_parameters: + kwargs["region_name"] = s3_parameters["region"] + session = Session(**kwargs) return session.resource( "s3", endpoint_url=self._construct_endpoint(s3_parameters), @@ -200,7 +199,7 @@ def can_use_s3_repository(self) -> tuple[bool, str]: else: if return_code != 0: - logger.error(stderr) + logger.error(f"Failed to run pgbackrest: {stderr}") return False, FAILED_TO_INITIALIZE_STANZA_ERROR_MESSAGE for stanza in json.loads(stdout): @@ -252,7 +251,7 @@ def _construct_endpoint(self, s3_parameters: dict) -> str: # Construct the endpoint using the region. resolver = EndpointResolver(data) - endpoint_data = resolver.construct_endpoint("s3", s3_parameters["region"]) + endpoint_data = resolver.construct_endpoint("s3", s3_parameters.get("region")) # Use the built endpoint if it is an AWS endpoint. if endpoint_data and endpoint.endswith(endpoint_data["dnsSuffix"]): @@ -266,7 +265,7 @@ def _create_bucket_if_not_exists(self) -> None: return bucket_name = s3_parameters["bucket"] - region = s3_parameters.get("region") + region = s3_parameters.get("region", "") try: s3 = self._get_s3_session_resource(s3_parameters) @@ -640,10 +639,10 @@ def _initialise_stanza(self, event: HookEvent) -> bool: raise Exception(stderr) except TimeoutError as e: raise e - except Exception as e: + except Exception: # If the check command doesn't succeed, remove the stanza name # and rollback the configuration. - logger.exception(e) + logger.exception("Failed to initialise stanza:") self._s3_initialization_set_failure(FAILED_TO_INITIALIZE_STANZA_ERROR_MESSAGE) return False @@ -689,10 +688,10 @@ def check_stanza(self) -> bool: if return_code != 0: raise Exception(stderr) self.charm._set_primary_status_message() - except Exception as e: + except Exception: # If the check command doesn't succeed, remove the stanza name # and rollback the configuration. - logger.exception(e) + logger.exception("Failed to check stanza:") self._s3_initialization_set_failure(FAILED_TO_INITIALIZE_STANZA_ERROR_MESSAGE) self.charm.update_config() return False @@ -975,9 +974,9 @@ def _run_backup( else: try: backup_id = list(self._list_backups(show_failed=True).keys())[-1] - except ListBackupsError as e: - logger.exception(e) + except ListBackupsError: error_message = "Failed to retrieve backup id" + logger.exception(error_message) logger.error(f"Backup failed: {error_message}") event.fail(error_message) return @@ -1330,8 +1329,6 @@ def _retrieve_s3_parameters(self) -> tuple[dict, list[str]]: # Add some sensible defaults (as expected by the code) for missing optional parameters s3_parameters.setdefault("endpoint", "https://s3.amazonaws.com") - # Existing behaviour is none not a str - s3_parameters.setdefault("region", None) # type: ignore s3_parameters.setdefault("path", "") s3_parameters.setdefault("s3-uri-style", "host") s3_parameters.setdefault("delete-older-than-days", "9999999") diff --git a/templates/pgbackrest.conf.j2 b/templates/pgbackrest.conf.j2 index ce4dbbe2ca..ff44a6d4a8 100644 --- a/templates/pgbackrest.conf.j2 +++ b/templates/pgbackrest.conf.j2 @@ -8,7 +8,11 @@ repo1-retention-full={{ retention_full }} repo1-retention-history=365 repo1-type=s3 repo1-path={{ path }} +{%- if region %} repo1-s3-region={{ region }} +{% else %} +repo1-s3-region="" +{%- endif %} repo1-s3-endpoint={{ endpoint }} repo1-s3-bucket={{ bucket }} repo1-s3-uri-style={{ s3_uri_style }} diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 668459db7c..2740fa317d 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -44,7 +44,7 @@ def get_cloud_config(cloud: str) -> tuple[dict[str, str], dict[str, str]] | None "endpoint": "https://storage.googleapis.com", "bucket": "data-charms-testing", "path": f"/postgresql-k8s/{uuid.uuid1()}", - "region": "", + "region": "us-east-1", }, { "access-key": os.environ["GCP_ACCESS_KEY"], "secret-key": os.environ["GCP_SECRET_KEY"], diff --git a/tests/integration/test_backups_ceph.py b/tests/integration/test_backups_ceph.py index 4a70004405..aedb7171e1 100644 --- a/tests/integration/test_backups_ceph.py +++ b/tests/integration/test_backups_ceph.py @@ -7,16 +7,11 @@ import os import socket import subprocess -import time -import boto3 -import botocore.exceptions import pytest from pytest_operator.plugin import OpsTest -from .helpers import ( - backup_operations, -) +from .helpers import backup_operations logger = logging.getLogger(__name__) @@ -133,24 +128,6 @@ def microceph(): key = json.loads(output)["keys"][0] key_id = key["access_key"] secret_key = key["secret_key"] - logger.info("Creating microceph bucket") - for attempt in range(3): - try: - boto3.client( - "s3", - endpoint_url=f"https://{host_ip}", - aws_access_key_id=key_id, - aws_secret_access_key=secret_key, - verify="./ca.crt", - ).create_bucket(Bucket=_BUCKET) - except botocore.exceptions.EndpointConnectionError: - if attempt == 2: - raise - # microceph is not ready yet - logger.info("Unable to connect to microceph via S3. Retrying") - time.sleep(1) - else: - break logger.info("Set up microceph") return ConnectionInformation(key_id, secret_key, _BUCKET) diff --git a/tests/unit/test_backups.py b/tests/unit/test_backups.py index b0a6f6daa2..4267541a2a 100644 --- a/tests/unit/test_backups.py +++ b/tests/unit/test_backups.py @@ -1803,7 +1803,6 @@ def test_retrieve_s3_parameters(harness): "delete-older-than-days": "9999999", "endpoint": "https://s3.amazonaws.com", "path": "/", - "region": None, "s3-uri-style": "host", "secret-key": "test-secret-key", },