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

[MISC] Update snap and renovate fixes #432

Merged
merged 13 commits into from
Apr 11, 2024
62 changes: 61 additions & 1 deletion .github/renovate.json5
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"$schema": "https://docs.renovatebot.com/renovate-schema.json",
"extends": ["github>canonical/data-platform//renovate_presets/charm.json5"],
"reviewers": ["dragomirp", "marceloneppel", "taurus-forever"],
"reviewers": ["dragomirp", "lucasgameiroborges", "marceloneppel", "taurus-forever"],
"packageRules": [
// Later rules override earlier rules
{
Expand All @@ -10,6 +10,66 @@
}, {
"matchPackageNames": ["python"],
"allowedVersions": "<3.11"
},
{
"matchDepNames": ["Juju 2"],
"matchPackageNames": ["juju/juju"],
"allowedVersions": "<3.0.0",
"extractVersion": "^v(?<version>.*)$",
"groupName": "Juju 2"
},
{
"matchDepNames": ["Juju 3"],
"matchPackageNames": ["juju/juju"],
"allowedVersions": "<3.2.0",
"extractVersion": "^v(?<version>.*)$",
"groupName": "Juju 3"
},
{
"matchDepNames": ["libjuju 2"],
"matchPackageNames": ["juju"],
"matchManagers": ["regex"],
"matchDatasources": ["pypi"],
"versioning": "loose",
"allowedVersions": "<3",
"groupName": "Juju 2"
}
],
"regexManagers": [
{
"customType": "regex",
"fileMatch": ["^(workflow-templates|\\.github/workflows)/[^/]+\\.ya?ml$"],
"matchStrings": [
"(- agent: )(?<currentValue>.*?) +# renovate: latest juju 2"
],
"depNameTemplate": "Juju 2",
"packageNameTemplate": "juju/juju",
"datasourceTemplate": "github-releases",
"versioningTemplate": "loose",
"extractVersionTemplate": "Juju release"
},
{
"customType": "regex",
"fileMatch": ["^(workflow-templates|\\.github/workflows)/[^/]+\\.ya?ml$"],
"matchStrings": [
"(- agent: )(?<currentValue>.*?) +# renovate: latest juju 3"
],
"depNameTemplate": "Juju 3",
"packageNameTemplate": "juju/juju",
"datasourceTemplate": "github-releases",
"versioningTemplate": "loose",
"extractVersionTemplate": "Juju release"
},
{
"customType": "regex",
"fileMatch": ["^(workflow-templates|\\.github/workflows)/[^/]+\\.ya?ml$"],
"matchStrings": [
"(libjuju: )==(?<currentValue>.*?) +# renovate: latest libjuju 2"
],
"depNameTemplate": "libjuju 2",
"packageNameTemplate": "juju",
"datasourceTemplate": "pypi",
"versioningTemplate": "loose"
}
]
}
6 changes: 3 additions & 3 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,10 @@ jobs:
fail-fast: false
matrix:
juju:
- agent: 2.9.46
libjuju: ^2
- agent: 2.9.46 # renovate: latest juju 2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The backup tests fail on 2.9.47. Charm cannot be redeployed with the same name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This seems to be fixed with juju 2.9.49, but there are issues with the subordinates test. I will update in a separate PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

I cannot find such fixes mentioned in Juju 2.9.47+ release notes: https://discourse.charmhub.io/t/roadmap-releases/5064
Maybe this... strange... Will ask Juju Team to improve release notes as 2.9.48 is missing there completely.

libjuju: ==2.9.46.1 # renovate: latest libjuju 2
allure: false
- agent: 3.1.7
- agent: 3.1.7 # renovate: latest juju 3
allure: true
name: Integration test charm | ${{ matrix.juju.agent }}
needs:
Expand Down
4 changes: 2 additions & 2 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ pytest-github-secrets = {git = "https://github.com/canonical/data-platform-workf
pytest-operator = "^0.34.0"
pytest-operator-cache = {git = "https://github.com/canonical/data-platform-workflows", tag = "v13.1.0", subdirectory = "python/pytest_plugins/pytest_operator_cache"}
pytest-operator-groups = {git = "https://github.com/canonical/data-platform-workflows", tag = "v13.1.0", subdirectory = "python/pytest_plugins/pytest_operator_groups"}
juju = "^3.3.0.0"
# renovate caret doesn't work: https://github.com/renovatebot/renovate/issues/26940
juju = "<=3.4.0.0"
boto3 = "*"
tenacity = "*"
landscape-api-py3 = "^0.9.0"
Expand Down
6 changes: 5 additions & 1 deletion src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -1390,7 +1390,11 @@ def push_tls_files_to_workload(self) -> bool:
if cert is not None:
self._patroni.render_file(f"{PATRONI_CONF_PATH}/{TLS_CERT_FILE}", cert, 0o600)

return self.update_config()
try:
return self.update_config()
except Exception:
logger.exception("TLS files failed to push. Error in config update")
return False
Comment on lines +1393 to +1397
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't really check for exceptions or return value of update_config() in most places, we should most probably go over and handle such cases.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah.... this is what we saw with @beliaev-maksim yesterday on a troubleshooting. Tnx for the quick fix here!


def _reboot_on_detached_storage(self, event: EventBase) -> None:
"""Reboot on detached storage.
Expand Down
2 changes: 1 addition & 1 deletion src/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
SNAP_PACKAGES = [
(
POSTGRESQL_SNAP_NAME,
{"revision": {"aarch64": "105", "x86_64": "104"}, "channel": "14/stable"},
{"revision": {"aarch64": "110", "x86_64": "111"}, "channel": "14/stable"},
)
]

Expand Down
38 changes: 13 additions & 25 deletions tests/integration/test_backups.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,8 @@ async def cloud_configs(ops_test: OpsTest, github_secrets) -> None:

@pytest.mark.group(1)
@pytest.mark.abort_on_fail
async def test_backup(ops_test: OpsTest, cloud_configs: Tuple[Dict, Dict]) -> None:
async def test_backup(ops_test: OpsTest, cloud_configs: Tuple[Dict, Dict], charm) -> None:
"""Build and deploy two units of PostgreSQL and then test the backup and restore actions."""
# Build the PostgreSQL charm.
charm = await ops_test.build_charm(".")

# Deploy S3 Integrator and TLS Certificates Operator.
await ops_test.model.deploy(S3_INTEGRATOR_APP_NAME)
await ops_test.model.deploy(TLS_CERTIFICATES_APP_NAME, config=TLS_CONFIG, channel=TLS_CHANNEL)
Expand Down Expand Up @@ -126,7 +123,7 @@ async def test_backup(ops_test: OpsTest, cloud_configs: Tuple[Dict, Dict]) -> No
await action.wait()
async with ops_test.fast_forward(fast_interval="60s"):
await ops_test.model.wait_for_idle(
apps=[database_app_name, S3_INTEGRATOR_APP_NAME], status="active", timeout=1200
apps=[database_app_name, S3_INTEGRATOR_APP_NAME], status="active", timeout=1500
)

primary = await get_primary(ops_test, f"{database_app_name}/0")
Expand Down Expand Up @@ -279,14 +276,14 @@ async def test_backup(ops_test: OpsTest, cloud_configs: Tuple[Dict, Dict]) -> No

# Remove the database app.
await ops_test.model.remove_application(database_app_name, block_until_done=True)

# Remove the TLS operator.
await ops_test.model.remove_application(TLS_CERTIFICATES_APP_NAME, block_until_done=True)


@pytest.mark.group(1)
async def test_restore_on_new_cluster(ops_test: OpsTest, github_secrets) -> None:
async def test_restore_on_new_cluster(ops_test: OpsTest, github_secrets, charm) -> None:
"""Test that is possible to restore a backup to another PostgreSQL cluster."""
charm = await ops_test.build_charm(".")
previous_database_app_name = f"{DATABASE_APP_NAME}-gcp"
database_app_name = f"new-{DATABASE_APP_NAME}"
await ops_test.model.deploy(charm, application_name=previous_database_app_name)
Expand Down Expand Up @@ -355,12 +352,9 @@ async def test_restore_on_new_cluster(ops_test: OpsTest, github_secrets) -> None

# Wait for the restore to complete.
async with ops_test.fast_forward():
await wait_for_idle_on_blocked(
ops_test,
database_app_name,
0,
S3_INTEGRATOR_APP_NAME,
ANOTHER_CLUSTER_REPOSITORY_ERROR_MESSAGE,
unit = ops_test.model.units.get(f"{database_app_name}/0")
await ops_test.model.block_until(
lambda: unit.workload_status_message == ANOTHER_CLUSTER_REPOSITORY_ERROR_MESSAGE
)
Comment on lines +355 to 358
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Model fails to idle with the new snap. I tried with one minute ffwd interval, but it didn't help.


# Check that the backup was correctly restored by having only the first created table.
Expand Down Expand Up @@ -402,12 +396,9 @@ async def test_invalid_config_and_recovery_after_fixing_it(
)
await action.wait()
logger.info("waiting for the database charm to become blocked")
await wait_for_idle_on_blocked(
ops_test,
database_app_name,
0,
S3_INTEGRATOR_APP_NAME,
FAILED_TO_ACCESS_CREATE_BUCKET_ERROR_MESSAGE,
unit = ops_test.model.units.get(f"{database_app_name}/0")
await ops_test.model.block_until(
lambda: unit.workload_status_message == FAILED_TO_ACCESS_CREATE_BUCKET_ERROR_MESSAGE
)

# Provide valid backup configurations, but from another cluster repository.
Expand All @@ -421,12 +412,9 @@ async def test_invalid_config_and_recovery_after_fixing_it(
)
await action.wait()
logger.info("waiting for the database charm to become blocked")
await wait_for_idle_on_blocked(
ops_test,
database_app_name,
0,
S3_INTEGRATOR_APP_NAME,
ANOTHER_CLUSTER_REPOSITORY_ERROR_MESSAGE,
unit = ops_test.model.units.get(f"{database_app_name}/0")
await ops_test.model.block_until(
lambda: unit.workload_status_message == ANOTHER_CLUSTER_REPOSITORY_ERROR_MESSAGE
)

# Provide valid backup configurations, with another path in the S3 bucket.
Expand Down
Loading