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

DPE-2758 better messaging when no bucket + ceph testing #332

Merged
merged 18 commits into from
Jan 18, 2024
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
134 changes: 23 additions & 111 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,9 @@ on:
jobs:
lint:
name: Lint
uses: canonical/data-platform-workflows/.github/workflows/lint.yaml@v9.0.2
uses: canonical/data-platform-workflows/.github/workflows/lint.yaml@v9.3.1

unit-test:
strategy:
fail-fast: false
matrix:
juju-version: ["2.9", "3.1"]
name: Unit test charm
runs-on: ubuntu-latest
timeout-minutes: 5
Expand All @@ -35,124 +31,40 @@ jobs:
pipx install poetry
- name: Run tests
run: tox run -e unit
env:
# This env var is only to indicate Juju version to "simulate" in the unit tests
# No libjuju is being actually used in unit testing
LIBJUJU_VERSION_SPECIFIER: ${{ matrix.juju-version }}
- name: Upload Coverage to Codecov
uses: codecov/codecov-action@v3

build:
name: Build charm
uses: canonical/data-platform-workflows/.github/workflows/build_charms_with_cache.yaml@v9.0.2
uses: canonical/data-platform-workflows/.github/workflows/build_charms_with_cache.yaml@v9.3.1
with:
charmcraft-snap-channel: beta

gh-hosted-collect-integration-tests:
name: (GH hosted) Collect integration test groups
needs:
- lint
- unit-test
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v3
- name: Install tox & poetry
run: |
pipx install tox
pipx install poetry
- name: Select test stability level
id: select-test-stability
run: |
if [[ "${{ github.event_name }}" == "schedule" ]]
then
echo Running unstable and stable tests
echo "mark_expression=" >> "$GITHUB_OUTPUT"
else
echo Skipping unstable tests
echo "mark_expression=not unstable" >> "$GITHUB_OUTPUT"
fi
- name: Collect test groups
id: collect-groups
run: tox run -e integration -- tests/integration -m '${{ steps.select-test-stability.outputs.mark_expression }}' --collect-groups
outputs:
groups: ${{ steps.collect-groups.outputs.groups }}

gh-hosted-integration-test:
integration-test:
strategy:
fail-fast: false
matrix:
groups: ${{ fromJSON(needs.gh-hosted-collect-integration-tests.outputs.groups) }}
juju-snap-channel: ["2.9/stable", "3.1/stable"]
include:
- juju-snap-channel: "3.1/stable"
agent-version: "3.1.6"
libjuju-version: "3.2.2"
- juju-snap-channel: "2.9/stable"
agent-version: "2.9.45"
libjuju-version: "2.9.44.1"
exclude:
# Disabling HA tests, as long as we want to have a limited pipeline on Juju3
- juju-snap-channel: "3.1/stable"
groups:
job_name: "high_availability/test_replication.py | group 1"
- juju-snap-channel: "3.1/stable"
groups:
job_name: "high_availability/test_self_healing.py | group 1"
- juju-snap-channel: "3.1/stable"
groups:
job_name: "high_availability/test_upgrade.py | group 1"
- juju-snap-channel: "3.1/stable"
groups:
job_name: "high_availability/test_upgrade_from_stable.py | group 1"
name: ${{ matrix.juju-snap-channel }} - (GH hosted) ${{ matrix.groups.job_name }}
juju:
- agent: 2.9.45
libjuju: ^2
- agent: 3.1.6
name: Integration test charm | ${{ matrix.juju.agent }}
needs:
- lint
- unit-test
- build
- gh-hosted-collect-integration-tests
runs-on: ubuntu-latest
timeout-minutes: 120
steps:
- name: Checkout
uses: actions/checkout@v3
- name: Install tox & poetry
run: |
pipx install tox
pipx install poetry
- name: Setup operator environment
uses: charmed-kubernetes/actions-operator@main
with:
provider: microk8s
channel: "1.28-strict/stable"
bootstrap-options: "--agent-version ${{ matrix.agent-version }}"
juju-channel: ${{ matrix.juju-snap-channel }}
- name: Update python-libjuju version
if: ${{ matrix.libjuju-version == '2.9.44.1' }}
run: poetry add --lock --group integration juju@'${{ matrix.libjuju-version }}'
- name: Download packed charm(s)
uses: actions/download-artifact@v3
with:
name: ${{ needs.build.outputs.artifact-name }}
- name: Select test stability level
id: select-test-stability
run: |
if [[ "${{ github.event_name }}" == "schedule" ]]
then
echo Running unstable and stable tests
echo "mark_expression=" >> "$GITHUB_OUTPUT"
else
echo Skipping unstable tests
echo "mark_expression=not unstable" >> "$GITHUB_OUTPUT"
fi
- name: Run integration tests
run: tox run -e integration -- "${{ matrix.groups.path_to_test_file }}" --group="${{ matrix.groups.group_number }}" -m '${{ steps.select-test-stability.outputs.mark_expression }}'
env:
LIBJUJU_VERSION_SPECIFIER: ${{ matrix.libjuju-version }}
SECRETS_FROM_GITHUB: |
{
"AWS_ACCESS_KEY": "${{ secrets.AWS_ACCESS_KEY }}",
"AWS_SECRET_KEY": "${{ secrets.AWS_SECRET_KEY }}",
"GCP_ACCESS_KEY": "${{ secrets.GCP_ACCESS_KEY }}",
"GCP_SECRET_KEY": "${{ secrets.GCP_SECRET_KEY }}",
}
uses: canonical/data-platform-workflows/.github/workflows/[email protected]
with:
artifact-name: ${{ needs.build.outputs.artifact-name }}
cloud: microk8s
microk8s-snap-channel: 1.28-strict/stable
juju-agent-version: ${{ matrix.juju.agent }}
libjuju-version-constraint: ${{ matrix.juju.libjuju }}
secrets:
integration-test: |
{
"AWS_ACCESS_KEY": "${{ secrets.AWS_ACCESS_KEY }}",
"AWS_SECRET_KEY": "${{ secrets.AWS_SECRET_KEY }}",
"GCP_ACCESS_KEY": "${{ secrets.GCP_ACCESS_KEY }}",
"GCP_SECRET_KEY": "${{ secrets.GCP_SECRET_KEY }}",
}
4 changes: 2 additions & 2 deletions .github/workflows/release.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ jobs:
secrets: inherit
build:
name: Build charm
uses: canonical/data-platform-workflows/.github/workflows/build_charm_without_cache.yaml@v9.0.2
uses: canonical/data-platform-workflows/.github/workflows/build_charm_without_cache.yaml@v9.3.1
with:
charmcraft-snap-channel: beta

Expand All @@ -43,7 +43,7 @@ jobs:
- lib-check
- ci-tests
- build
uses: canonical/data-platform-workflows/.github/workflows/release_charm.yaml@v9.0.2
uses: canonical/data-platform-workflows/.github/workflows/release_charm.yaml@v9.3.1
with:
channel: 8.0/edge
artifact-name: ${{ needs.build.outputs.artifact-name }}
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/sync_issue_to_jira.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ on:
jobs:
sync:
name: Sync GitHub issue to Jira
uses: canonical/data-platform-workflows/.github/workflows/sync_issue_to_jira.yaml@v9.0.2
uses: canonical/data-platform-workflows/.github/workflows/sync_issue_to_jira.yaml@v9.3.1
with:
jira-base-url: https://warthogs.atlassian.net
jira-project-key: DPE
Expand Down
6 changes: 0 additions & 6 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,6 @@ tox run -e unit # unit tests
tox run -e integration # integration tests
tox # runs 'lint' and 'unit' environments
```
Tests by default are using Juju 3. In case tests are to be run against Juju 3, the following
environment variable should be defined with a valid `juju` Python library version:

```
export LIBJUJU_VERSION_SPECIFIER=2.9.44.1
```

## Build charm

Expand Down
12 changes: 7 additions & 5 deletions lib/charms/mysql/v0/backups.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ def is_unit_blocked(self) -> bool:
import datetime
import logging
import pathlib
from typing import Dict, List, Optional, Tuple
import typing
from typing import Dict, List, Optional, Tuple

from charms.data_platform_libs.v0.s3 import S3Requirer
from charms.mysql.v0.mysql import (
Expand Down Expand Up @@ -96,7 +96,7 @@ def is_unit_blocked(self) -> bool:

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


if typing.TYPE_CHECKING:
Expand Down Expand Up @@ -220,9 +220,11 @@ def _on_list_backups(self, event: ActionEvent) -> None:
logger.info("Listing backups in the specified s3 path")
backups = sorted(list_backups_in_s3_path(s3_parameters), key=lambda pair: pair[0])
event.set_results({"backups": self._format_backups_list(backups)})
except Exception:
error_message = "Failed to retrieve backup ids from S3"
logger.exception(error_message)
except Exception as e:
error_message = (
e.message if hasattr(e, "message") else "Failed to retrieve backup ids from S3"
)
logger.error(error_message)
Comment on lines -223 to +227
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider using a more specific exception type instead

event.fail(error_message)

# ------------------ Create Backup ------------------
Expand Down
23 changes: 19 additions & 4 deletions lib/charms/mysql/v0/s3_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,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 = 6

# botocore/urllib3 clutter the logs when on debug
logging.getLogger("botocore").setLevel(logging.WARNING)
logging.getLogger("urllib3").setLevel(logging.WARNING)


def upload_content_to_s3(content: str, content_path: str, s3_parameters: Dict) -> bool:
Expand All @@ -55,6 +59,7 @@ def upload_content_to_s3(content: str, content_path: str, s3_parameters: Dict) -
)

s3 = session.resource("s3", endpoint_url=s3_parameters["endpoint"])

bucket = s3.Bucket(s3_parameters["bucket"])

with tempfile.NamedTemporaryFile() as temp_file:
Expand Down Expand Up @@ -89,7 +94,7 @@ def _compile_backups_from_file_ids(
return backups


def list_backups_in_s3_path(s3_parameters: Dict) -> List[Tuple[str, str]]:
def list_backups_in_s3_path(s3_parameters: Dict) -> List[Tuple[str, str]]: # noqa: C901
"""Retrieve subdirectories in an S3 path.

Args:
Expand Down Expand Up @@ -147,9 +152,19 @@ def list_backups_in_s3_path(s3_parameters: Dict) -> List[Tuple[str, str]]:

return _compile_backups_from_file_ids(metadata_ids, md5_ids, log_ids)
except Exception as e:
try:
# botocore raises dynamically generated exceptions
# with a response attribute. We can use this to
# set a more meaningful error message.
if e.response["Error"]["Code"] == "NoSuchBucket":
message = f"Bucket {s3_parameters['bucket']} does not exist"
setattr(e, "message", message)
raise
except (KeyError, AttributeError):
pass
paulomach marked this conversation as resolved.
Show resolved Hide resolved
# default handling exposes exception
logger.exception(
f"Failed to list subdirectories in S3 bucket={s3_parameters['bucket']}, path={s3_parameters['path']}",
exc_info=e,
f"Failed to list subdirectories in S3 bucket={s3_parameters['bucket']}, path={s3_parameters['path']}"
)
raise

Expand Down
Loading
Loading