Skip to content

Commit

Permalink
[DPE-2804] Conditionally skip backup tests (#310)
Browse files Browse the repository at this point in the history
* Conditionally skip backup tests

* Wrong step output

* Bump libs
  • Loading branch information
dragomirp authored Nov 2, 2023
1 parent 24a755d commit 223ad4c
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 8 deletions.
13 changes: 12 additions & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,19 @@ jobs:
echo Skipping unstable tests
echo "mark_expression=and not unstable" >> "$GITHUB_OUTPUT"
fi
- name: Select test secret usage
id: select-test-secrets
run: |
if [[ "${{ github.event.pull_request.head.repo.full_name }}" == "canonical/postgresql-k8s-operator" ]]
then
echo Running tests using secrets
echo "mark_secrets=" >> "$GITHUB_OUTPUT"
else
echo Skipping tests using secrets
echo "mark_secrets=and not uses_secrets" >> "$GITHUB_OUTPUT"
fi
- name: Run integration tests
run: tox run -e ${{ matrix.tox-environment }} -- -m 'not ${{ matrix.exclude-mark }} ${{ steps.select-test-stability.outputs.mark_expression }}' --keep-models
run: tox run -e ${{ matrix.tox-environment }} -- -m 'not ${{ matrix.exclude-mark }} ${{ steps.select-test-secrets.outputs.mark_secrets }} ${{ steps.select-test-stability.outputs.mark_expression }}' --keep-models
env:
SECRETS_FROM_GITHUB: |
{
Expand Down
34 changes: 27 additions & 7 deletions lib/charms/data_platform_libs/v0/data_interfaces.py
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ 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 = 22
LIBPATCH = 23

PYDEPS = ["ops>=2.0.0"]

Expand Down Expand Up @@ -807,6 +807,9 @@ def _fetch_relation_data_without_secrets(
This is used typically when the Provides side wants to read the Requires side's data,
or when the Requires side may want to read its own data.
"""
if app not in relation.data or not relation.data[app]:
return {}

if fields:
return {k: relation.data[app][k] for k in fields if k in relation.data[app]}
else:
Expand All @@ -830,6 +833,9 @@ def _fetch_relation_data_with_secrets(
normal_fields = []

if not fields:
if app not in relation.data or not relation.data[app]:
return {}

all_fields = list(relation.data[app].keys())
normal_fields = [field for field in all_fields if not self._is_secret_field(field)]

Expand All @@ -853,8 +859,11 @@ def _fetch_relation_data_with_secrets(

def _update_relation_data_without_secrets(
self, app: Application, relation: Relation, data: Dict[str, str]
):
) -> None:
"""Updating databag contents when no secrets are involved."""
if app not in relation.data or relation.data[app] is None:
return

if any(self._is_secret_field(key) for key in data.keys()):
raise SecretsIllegalUpdateError("Can't update secret {key}.")

Expand All @@ -865,8 +874,19 @@ def _delete_relation_data_without_secrets(
self, app: Application, relation: Relation, fields: List[str]
) -> None:
"""Remove databag fields 'fields' from Relation."""
if app not in relation.data or not relation.data[app]:
return

for field in fields:
relation.data[app].pop(field)
try:
relation.data[app].pop(field)
except KeyError:
logger.debug(
"Non-existing field was attempted to be removed from the databag %s, %s",
str(relation.id),
str(field),
)
pass

# Public interface methods
# Handling Relation Fields seamlessly, regardless if in databag or a Juju Secret
Expand All @@ -880,9 +900,6 @@ def get_relation(self, relation_name, relation_id) -> Relation:
"Relation %s %s couldn't be retrieved", relation_name, relation_id
)

if not relation.app:
raise DataInterfacesError("Relation's application missing")

return relation

def fetch_relation_data(
Expand Down Expand Up @@ -1089,7 +1106,10 @@ def _delete_relation_secret(
# Remove secret from the relation if it's fully gone
if not new_content:
field = self._generate_secret_field_name(group)
relation.data[self.local_app].pop(field)
try:
relation.data[self.local_app].pop(field)
except KeyError:
pass

# Return the content that was removed
return True
Expand Down
3 changes: 3 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@ target-version = ["py38"]

# Linting tools configuration
[tool.ruff]
# preview and explicit preview are enabled for CPY001
preview = true
explicit-preview-rules = true
target-version = "py38"
src = ["src", "."]
line-length = 99
Expand Down
8 changes: 8 additions & 0 deletions tests/integration/test_backups.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,12 @@ async def cloud_configs(ops_test: OpsTest, github_secrets) -> None:
bucket_object.delete()


async def test_none() -> None:
"""Empty test so that the suite will not fail if all tests are skippedi."""
pass


@pytest.mark.uses_secrets
@pytest.mark.abort_on_fail
async def test_backup_and_restore(ops_test: OpsTest, cloud_configs: Tuple[Dict, Dict]) -> None:
"""Build and deploy two units of PostgreSQL and then test the backup and restore actions."""
Expand Down Expand Up @@ -203,6 +209,7 @@ async def test_backup_and_restore(ops_test: OpsTest, cloud_configs: Tuple[Dict,
await ops_test.model.remove_application(TLS_CERTIFICATES_APP_NAME, block_until_done=True)


@pytest.mark.uses_secrets
async def test_restore_on_new_cluster(ops_test: OpsTest) -> None:
"""Test that is possible to restore a backup to another PostgreSQL cluster."""
database_app_name = f"new-{DATABASE_APP_NAME}"
Expand Down Expand Up @@ -275,6 +282,7 @@ async def test_restore_on_new_cluster(ops_test: OpsTest) -> None:
connection.close()


@pytest.mark.uses_secrets
async def test_invalid_config_and_recovery_after_fixing_it(
ops_test: OpsTest, cloud_configs: Tuple[Dict, Dict]
) -> None:
Expand Down

0 comments on commit 223ad4c

Please sign in to comment.