From 80f247f0749d880f20ba1406a5abc413908c41d8 Mon Sep 17 00:00:00 2001 From: Mykola Marzhan Date: Wed, 15 May 2024 12:16:16 +0200 Subject: [PATCH 1/8] add ability to remove backups after a number of days --- README.md | 1 + config.yaml | 11 +++++++ lib/charms/data_platform_libs/v0/s3.py | 29 +++++++++++++++++-- src/charm.py | 9 ++++-- src/constants.py | 1 + .../lib/charms/data_platform_libs/v0/s3.py | 29 +++++++++++++++++-- tests/integration/test_s3_charm.py | 4 +++ 7 files changed, 75 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 7563742..6ec93fc 100644 --- a/README.md +++ b/README.md @@ -50,6 +50,7 @@ To configure the S3 integrator charm, you may provide the following configuratio - storage-class:the storage class for objects uploaded to the object storage. - tls-ca-chain: the complete CA chain, which can be used for HTTPS validation. - s3-api-version: the S3 protocol specific API signature. +- experimental-delete-older-than-days: the amount of day after which backups going to be deleted. EXPERIMENTAL option. The only mandatory fields for the integrator are access-key secret-key and bucket. diff --git a/config.yaml b/config.yaml index 0e59887..15ee4cc 100644 --- a/config.yaml +++ b/config.yaml @@ -36,3 +36,14 @@ options: type: string description: S3 protocol specific API signature. default: '' + experimental-delete-older-than-days: + type: string + description: | + Full backups can be retained for a number of days. The removal of expired + backups happens imediatelly after finishing the first successful backup after + retention period. + When full backup expires, the all differential and incremental backups which + depends on this full backup also expires. + This option is EXPRERIMENTAL. + Allowed values are: from 1 to 9999999. + default: '' diff --git a/lib/charms/data_platform_libs/v0/s3.py b/lib/charms/data_platform_libs/v0/s3.py index 7beb113..f5614aa 100644 --- a/lib/charms/data_platform_libs/v0/s3.py +++ b/lib/charms/data_platform_libs/v0/s3.py @@ -137,7 +137,7 @@ def _on_credential_gone(self, event: CredentialsGoneEvent): # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 4 +LIBPATCH = 5 logger = logging.getLogger(__name__) @@ -212,7 +212,7 @@ class S3CredentialEvents(CharmEvents): class S3Provider(Object): """A provider handler for communicating S3 credentials to consumers.""" - on = S3CredentialEvents() # pyright: ignore [reportGeneralTypeIssues] + on = S3CredentialEvents() # pyright: ignore [reportAssignmentType] def __init__( self, @@ -481,6 +481,18 @@ def set_s3_api_version(self, relation_id: int, s3_api_version: str) -> None: """ self.update_connection_info(relation_id, {"s3-api-version": s3_api_version}) + def set_delete_older_than_days(self, relation_id: int, days: int) -> None: + """Sets the retention days for full backups in application databag. + + This function writes in the application data bag, therefore, + only the leader unit can call it. + + Args: + relation_id: the identifier for a particular relation. + days: the value. + """ + self.update_connection_info(relation_id, {"delete-older-than-days": str(days)}) + def set_attributes(self, relation_id: int, attributes: List[str]) -> None: """Sets the connection attributes in application databag. @@ -580,6 +592,17 @@ def s3_api_version(self) -> Optional[str]: return self.relation.data[self.relation.app].get("s3-api-version") + @property + def delete_older_than_days(self) -> Optional[int]: + """Returns the retention days for full backups.""" + if not self.relation.app: + return None + + days = self.relation.data[self.relation.app].get("delete-older-than-days") + if days is None: + return None + return int(days) + @property def attributes(self) -> Optional[List[str]]: """Returns the attributes.""" @@ -613,7 +636,7 @@ class S3CredentialRequiresEvents(ObjectEvents): class S3Requirer(Object): """Requires-side of the s3 relation.""" - on = S3CredentialRequiresEvents() # pyright: ignore[reportGeneralTypeIssues] + on = S3CredentialRequiresEvents() # pyright: ignore[reportAssignmentType] def __init__( self, charm: ops.charm.CharmBase, relation_name: str, bucket_name: Optional[str] = None diff --git a/src/charm.py b/src/charm.py index eba0930..dd248fe 100755 --- a/src/charm.py +++ b/src/charm.py @@ -68,7 +68,7 @@ def _on_start(self, _: StartEvent) -> None: if missing_options: self.unit.status = ops.model.BlockedStatus(f"Missing parameters: {missing_options}") - def _on_config_changed(self, _: ConfigChangedEvent) -> None: + def _on_config_changed(self, _: ConfigChangedEvent) -> None: # noqa: C901 """Event handler for configuration changed events.""" # Only execute in the unit leader if not self.unit.is_leader(): @@ -102,9 +102,12 @@ def _on_config_changed(self, _: ConfigChangedEvent) -> None: ) update_config.update({option: ca_chain}) self.set_secret("app", option, json.dumps(ca_chain)) + elif option == "experimental-delete-older-than-days" and self.config[option] != "": + update_config.update({"delete-older-than-days": str(self.config[option])}) + self.set_secret("app", "delete-older-than-days", str(self.config[option])) else: - update_config.update({option: self.config[option]}) - self.set_secret("app", option, self.config[option]) + update_config.update({option: str(self.config[option])}) + self.set_secret("app", option, str(self.config[option])) if len(self.s3_provider.relations) > 0: for relation in self.s3_provider.relations: diff --git a/src/constants.py b/src/constants.py index 307e5ef..f33a552 100644 --- a/src/constants.py +++ b/src/constants.py @@ -16,6 +16,7 @@ "s3-api-version", "s3-uri-style", "tls-ca-chain", + "delete-older-than-days", ] S3_MANDATORY_OPTIONS = [ "access-key", diff --git a/tests/integration/application-charm/lib/charms/data_platform_libs/v0/s3.py b/tests/integration/application-charm/lib/charms/data_platform_libs/v0/s3.py index 7beb113..f5614aa 100644 --- a/tests/integration/application-charm/lib/charms/data_platform_libs/v0/s3.py +++ b/tests/integration/application-charm/lib/charms/data_platform_libs/v0/s3.py @@ -137,7 +137,7 @@ def _on_credential_gone(self, event: CredentialsGoneEvent): # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 4 +LIBPATCH = 5 logger = logging.getLogger(__name__) @@ -212,7 +212,7 @@ class S3CredentialEvents(CharmEvents): class S3Provider(Object): """A provider handler for communicating S3 credentials to consumers.""" - on = S3CredentialEvents() # pyright: ignore [reportGeneralTypeIssues] + on = S3CredentialEvents() # pyright: ignore [reportAssignmentType] def __init__( self, @@ -481,6 +481,18 @@ def set_s3_api_version(self, relation_id: int, s3_api_version: str) -> None: """ self.update_connection_info(relation_id, {"s3-api-version": s3_api_version}) + def set_delete_older_than_days(self, relation_id: int, days: int) -> None: + """Sets the retention days for full backups in application databag. + + This function writes in the application data bag, therefore, + only the leader unit can call it. + + Args: + relation_id: the identifier for a particular relation. + days: the value. + """ + self.update_connection_info(relation_id, {"delete-older-than-days": str(days)}) + def set_attributes(self, relation_id: int, attributes: List[str]) -> None: """Sets the connection attributes in application databag. @@ -580,6 +592,17 @@ def s3_api_version(self) -> Optional[str]: return self.relation.data[self.relation.app].get("s3-api-version") + @property + def delete_older_than_days(self) -> Optional[int]: + """Returns the retention days for full backups.""" + if not self.relation.app: + return None + + days = self.relation.data[self.relation.app].get("delete-older-than-days") + if days is None: + return None + return int(days) + @property def attributes(self) -> Optional[List[str]]: """Returns the attributes.""" @@ -613,7 +636,7 @@ class S3CredentialRequiresEvents(ObjectEvents): class S3Requirer(Object): """Requires-side of the s3 relation.""" - on = S3CredentialRequiresEvents() # pyright: ignore[reportGeneralTypeIssues] + on = S3CredentialRequiresEvents() # pyright: ignore[reportAssignmentType] def __init__( self, charm: ops.charm.CharmBase, relation_name: str, bucket_name: Optional[str] = None diff --git a/tests/integration/test_s3_charm.py b/tests/integration/test_s3_charm.py index e68d274..bd63448 100644 --- a/tests/integration/test_s3_charm.py +++ b/tests/integration/test_s3_charm.py @@ -138,6 +138,7 @@ async def test_config_options(ops_test: OpsTest): "path": "/test/path_1/", "region": "us-east-2", "endpoint": "s3.amazonaws.com", + "delete-older-than-days": "30", } # apply new configuration options await ops_test.model.applications[S3_APP_NAME].set_config(configuration_parameters) @@ -151,6 +152,7 @@ async def test_config_options(ops_test: OpsTest): # test the correctness of the configuration fields assert configured_options["storage-class"] == "cinder" assert configured_options["s3-api-version"] == "1.0" + assert configured_options["delete-older-than-days"] == "7" assert len(json.loads(configured_options["attributes"])) == 3 assert len(json.loads(configured_options["tls-ca-chain"])) == 2 assert configured_options["region"] == "us-east-2" @@ -187,6 +189,7 @@ async def test_relation_creation(ops_test: OpsTest): assert application_data["secret-key"] == "new-test-secret-key" assert application_data["storage-class"] == "cinder" assert application_data["s3-api-version"] == "1.0" + assert application_data["delete-older-than-days"] == "7" assert len(json.loads(application_data["attributes"])) == 3 assert len(json.loads(application_data["tls-ca-chain"])) == 2 assert application_data["region"] == "us-east-2" @@ -223,6 +226,7 @@ async def test_relation_creation(ops_test: OpsTest): assert application_data["secret-key"] == "new-test-secret-key" assert application_data["storage-class"] == "cinder" assert application_data["s3-api-version"] == "1.0" + assert application_data["delete-older-than-days"] == "7" assert len(json.loads(application_data["attributes"])) == 3 assert len(json.loads(application_data["tls-ca-chain"])) == 2 assert application_data["region"] == "us-east-2" From f61cabb48c1d990bc68c686d8a904ea61e0e75b8 Mon Sep 17 00:00:00 2001 From: Lucas Gameiro Borges Date: Thu, 16 May 2024 12:07:02 +0000 Subject: [PATCH 2/8] handle retention config --- src/charm.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/charm.py b/src/charm.py index dd248fe..64fcbd8 100755 --- a/src/charm.py +++ b/src/charm.py @@ -79,6 +79,18 @@ def _on_config_changed(self, _: ConfigChangedEvent) -> None: # noqa: C901 # iterate over the option and check for updates for option in S3_OPTIONS: + # experimental options should be handled from the config with the "experimental-" prefix + if option == "delete-older-than-days" and f"experimental-{option}" in self.config: + config_value = str(self.config[f"experimental-{option}"]) + # reset value if empty config_value + if config_value == "" and self.get_secret("app", option) is not None: + self.set_secret("app", option, None) + update_config.update({option: ""}) + elif config_value != "": + update_config.update({option: config_value}) + self.set_secret("app", option, config_value) + continue + if option not in self.config: logger.warning(f"Option {option} is not valid option!") continue @@ -102,9 +114,6 @@ def _on_config_changed(self, _: ConfigChangedEvent) -> None: # noqa: C901 ) update_config.update({option: ca_chain}) self.set_secret("app", option, json.dumps(ca_chain)) - elif option == "experimental-delete-older-than-days" and self.config[option] != "": - update_config.update({"delete-older-than-days": str(self.config[option])}) - self.set_secret("app", "delete-older-than-days", str(self.config[option])) else: update_config.update({option: str(self.config[option])}) self.set_secret("app", option, str(self.config[option])) From 113c041bbc79836e26ed1f9a4d9254e2d93d33f2 Mon Sep 17 00:00:00 2001 From: Lucas Gameiro Borges Date: Thu, 16 May 2024 19:49:49 +0000 Subject: [PATCH 3/8] fix linting --- src/charm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/charm.py b/src/charm.py index 64fcbd8..5fc1b75 100755 --- a/src/charm.py +++ b/src/charm.py @@ -79,7 +79,7 @@ def _on_config_changed(self, _: ConfigChangedEvent) -> None: # noqa: C901 # iterate over the option and check for updates for option in S3_OPTIONS: - # experimental options should be handled from the config with the "experimental-" prefix + # experimental config options should be handled with the "experimental-" prefix if option == "delete-older-than-days" and f"experimental-{option}" in self.config: config_value = str(self.config[f"experimental-{option}"]) # reset value if empty config_value From ae5db6a7a658be420e1c70ab2631745febf1e142 Mon Sep 17 00:00:00 2001 From: Lucas Gameiro Borges Date: Thu, 16 May 2024 20:04:28 +0000 Subject: [PATCH 4/8] fix integration test --- tests/integration/test_s3_charm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/test_s3_charm.py b/tests/integration/test_s3_charm.py index bd63448..138538d 100644 --- a/tests/integration/test_s3_charm.py +++ b/tests/integration/test_s3_charm.py @@ -138,7 +138,7 @@ async def test_config_options(ops_test: OpsTest): "path": "/test/path_1/", "region": "us-east-2", "endpoint": "s3.amazonaws.com", - "delete-older-than-days": "30", + "experimental-delete-older-than-days": "30", } # apply new configuration options await ops_test.model.applications[S3_APP_NAME].set_config(configuration_parameters) From 24e2fb4c27afb4c1b4a441d8976b6fc258acfeb8 Mon Sep 17 00:00:00 2001 From: Lucas Gameiro Borges Date: Thu, 16 May 2024 20:15:52 +0000 Subject: [PATCH 5/8] fix retention in test --- tests/integration/test_s3_charm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/test_s3_charm.py b/tests/integration/test_s3_charm.py index 138538d..ab1a64f 100644 --- a/tests/integration/test_s3_charm.py +++ b/tests/integration/test_s3_charm.py @@ -138,7 +138,7 @@ async def test_config_options(ops_test: OpsTest): "path": "/test/path_1/", "region": "us-east-2", "endpoint": "s3.amazonaws.com", - "experimental-delete-older-than-days": "30", + "experimental-delete-older-than-days": "7", } # apply new configuration options await ops_test.model.applications[S3_APP_NAME].set_config(configuration_parameters) From c26f70102aeaf2aa5006ae86e0298914fd7f7123 Mon Sep 17 00:00:00 2001 From: Lucas Gameiro Borges Date: Wed, 22 May 2024 18:52:43 +0000 Subject: [PATCH 6/8] delete-older-than-days as int + refactor --- config.yaml | 3 +-- src/charm.py | 40 +++++++++++++++++++----------- src/constants.py | 1 + tests/integration/test_s3_charm.py | 8 +++--- 4 files changed, 31 insertions(+), 21 deletions(-) diff --git a/config.yaml b/config.yaml index 15ee4cc..9a70a2d 100644 --- a/config.yaml +++ b/config.yaml @@ -37,7 +37,7 @@ options: description: S3 protocol specific API signature. default: '' experimental-delete-older-than-days: - type: string + type: int description: | Full backups can be retained for a number of days. The removal of expired backups happens imediatelly after finishing the first successful backup after @@ -46,4 +46,3 @@ options: depends on this full backup also expires. This option is EXPRERIMENTAL. Allowed values are: from 1 to 9999999. - default: '' diff --git a/src/charm.py b/src/charm.py index 5fc1b75..40e774e 100755 --- a/src/charm.py +++ b/src/charm.py @@ -19,7 +19,14 @@ from ops.charm import ActionEvent, ConfigChangedEvent, RelationChangedEvent, StartEvent from ops.model import ActiveStatus, BlockedStatus -from constants import KEYS_LIST, PEER, S3_LIST_OPTIONS, S3_MANDATORY_OPTIONS, S3_OPTIONS +from constants import ( + KEYS_LIST, + MAX_RETENTION_DAYS, + PEER, + S3_LIST_OPTIONS, + S3_MANDATORY_OPTIONS, + S3_OPTIONS, +) logger = logging.getLogger(__name__) @@ -81,22 +88,25 @@ def _on_config_changed(self, _: ConfigChangedEvent) -> None: # noqa: C901 for option in S3_OPTIONS: # experimental config options should be handled with the "experimental-" prefix if option == "delete-older-than-days" and f"experimental-{option}" in self.config: - config_value = str(self.config[f"experimental-{option}"]) - # reset value if empty config_value - if config_value == "" and self.get_secret("app", option) is not None: - self.set_secret("app", option, None) - update_config.update({option: ""}) - elif config_value != "": - update_config.update({option: config_value}) - self.set_secret("app", option, config_value) + config_value = self.config[f"experimental-{option}"] + # check if new config value is inside allowed range + if config_value > 0 and config_value <= MAX_RETENTION_DAYS: + update_config.update({option: str(config_value)}) + self.set_secret("app", option, str(config_value)) + else: + logger.warning( + "Invalid value %s for config 'delete-older-than-days', ignoring.", + config_value, + ) continue - if option not in self.config: - logger.warning(f"Option {option} is not valid option!") - continue - # skip in case of empty config - if self.config[option] == "": - # reset previous value if present (e.g., juju model-config --reset PARAMETER) + # option possibly removed from the config + # (e.g. 'juju config --reset