Skip to content

Commit

Permalink
Fix bug where the latest released version of the configurationmodel c…
Browse files Browse the repository at this point in the history
…ould be removed by the cleanup job. (Issue #7324, PR #7412)

# Description

Fix bug where the latest released version of the configurationmodel could be removed by the cleanup job.

closes #7324

# Self Check:

- [x] Attached issue to pull request
- [x] Changelog entry
- [x] Type annotations are present
- [x] Code is clear and sufficiently documented
- [x] No (preventable) type errors (check using make mypy or make mypy-diff)
- [x] Sufficient test cases (reproduces the bug/tests the requested feature)
- [x] Correct, in line with design
- [x] End user documentation is included or an issue is created for end-user documentation (add ref to issue here: )
- [ ] ~~If this PR fixes a race condition in the test suite, also push the fix to the relevant stable branche(s) (see [test-fixes](https://internal.inmanta.com/development/core/tasks/build-master.html#test-fixes) for more info)~~
  • Loading branch information
arnaudsjs authored and inmantaci committed Mar 22, 2024
1 parent 48d08b5 commit 89cf618
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 4 deletions.
@@ -0,0 +1,8 @@
---
description: Fix bug where the latest released version of the configurationmodel could be removed by the cleanup job.
issue-nr: 7324
issue-repo: inmanta-core
change-type: patch
destination-branches: [master, iso7, iso6]
sections:
bugfix: "{{description}}"
2 changes: 1 addition & 1 deletion src/inmanta/data/__init__.py
Expand Up @@ -2676,7 +2676,7 @@ def to_dto(self) -> m.Environment:
default=100,
typ="int",
validator=convert_int,
doc="The number of versions to keep stored in the database",
doc="The number of versions to keep stored in the database, excluding the latest released version.",
),
PROTECTED_ENVIRONMENT: Setting(
name=PROTECTED_ENVIRONMENT,
Expand Down
10 changes: 7 additions & 3 deletions src/inmanta/server/services/orchestrationservice.py
Expand Up @@ -415,14 +415,18 @@ async def _purge_versions(self) -> None:
n_versions = await env_item.get(AVAILABLE_VERSIONS_TO_KEEP, connection=connection)
assert isinstance(n_versions, int)
versions = await data.ConfigurationModel.get_list(
environment=env_item.id, connection=connection, no_status=True
environment=env_item.id, connection=connection, no_status=True, order_by_column="version", order="DESC"
)
if len(versions) > n_versions:
LOGGER.info("Removing %s available versions from environment %s", len(versions) - n_versions, env_item.id)
version_dict = {x.version: x for x in versions}
latest_released_version: Optional[int] = next((v.version for v in versions if v.released), None)
if latest_released_version is not None:
# Never cleanup the latest released version
del version_dict[latest_released_version]
delete_list = sorted(version_dict.keys())
delete_list = delete_list[:-n_versions]

if delete_list:
LOGGER.info("Removing %s available versions from environment %s", len(delete_list), env_item.id)
for v in delete_list:
await version_dict[v].delete_cascade(connection=connection)

Expand Down
71 changes: 71 additions & 0 deletions tests/test_server.py
Expand Up @@ -209,6 +209,9 @@ async def test_create_too_many_versions(client, server, n_versions_to_keep, n_ve
env_1_id = result.result["environment"]["id"]
result = await client.set_setting(tid=env_1_id, id=data.AVAILABLE_VERSIONS_TO_KEEP, value=n_versions_to_keep)
assert result.code == 200
# Make sure we don't have a released version. _purge_versions() always keeps the latest released version.
result = await client.set_setting(env_1_id, AUTO_DEPLOY, False)
assert result.code == 200

# make a second environment to be sure we don't do cross env deletes
# as it is empty, if it leaks, it will likely take everything with it on the other one
Expand Down Expand Up @@ -274,6 +277,68 @@ async def test_create_too_many_versions(client, server, n_versions_to_keep, n_ve
assert len(prvs) == min(n_versions_to_keep, n_versions_to_create) + 1


@pytest.mark.parametrize("has_released_versions", [True, False])
async def test_purge_versions(server, client, environment, has_released_versions: bool) -> None:
"""
Verify that the `OrchestrationService._purge_versions()` method works correctly and that it doesn't cleanup
the latest released version.
"""
result = await client.set_setting(tid=environment, id=data.AUTO_DEPLOY, value="false")
assert result.code == 200

versions = []
for _ in range(5):
version = (await client.reserve_version(environment)).result["data"]
versions.append(version)
res = await client.put_version(
tid=environment,
version=version,
resources=[
{
"id": f"unittest::Resource[internal,name=ok],v={version}",
"name": "root",
"desired_value": "ok",
"send_event": "false",
"purged": False,
"requires": [],
}
],
unknowns=[],
version_info={},
compiler_version=get_compiler_version(),
)
assert res.code == 200

if has_released_versions:
for v in versions[0:2]:
result = await client.release_version(environment, id=v)
assert result.code == 200

result = await client.set_setting(tid=environment, id=data.AVAILABLE_VERSIONS_TO_KEEP, value=3)
assert result.code == 200
await server.get_slice(SLICE_ORCHESTRATION)._purge_versions()

result = await client.list_versions(environment)
assert result.code == 200
assert result.result["count"] == (4 if has_released_versions else 3)
if has_released_versions:
assert {v["version"] for v in result.result["versions"]} == {versions[1], *versions[2:]}
else:
assert {v["version"] for v in result.result["versions"]} == {*versions[2:]}

result = await client.set_setting(tid=environment, id=data.AVAILABLE_VERSIONS_TO_KEEP, value=1)
assert result.code == 200
await server.get_slice(SLICE_ORCHESTRATION)._purge_versions()

result = await client.list_versions(environment)
assert result.code == 200
assert result.result["count"] == (2 if has_released_versions else 1)
if has_released_versions:
assert {v["version"] for v in result.result["versions"]} == {versions[1], *versions[4:]}
else:
assert {v["version"] for v in result.result["versions"]} == {*versions[4:]}


async def test_n_versions_env_setting_scope(client, server):
"""
The AVAILABLE_VERSIONS_TO_KEEP environment setting used to be a global config option.
Expand All @@ -295,11 +360,17 @@ async def test_n_versions_env_setting_scope(client, server):
env_1_id = result.result["environment"]["id"]
result = await client.set_setting(tid=env_1_id, id=data.AVAILABLE_VERSIONS_TO_KEEP, value=n_versions_to_keep_env1)
assert result.code == 200
# Make sure we don't have a released version. _purge_versions() always keeps the latest released version.
result = await client.set_setting(env_1_id, AUTO_DEPLOY, False)
assert result.code == 200

result = await client.create_environment(project_id=project_id, name="env_2")
env_2_id = result.result["environment"]["id"]
result = await client.set_setting(tid=env_2_id, id=data.AVAILABLE_VERSIONS_TO_KEEP, value=n_versions_to_keep_env2)
assert result.code == 200
# Make sure we don't have a released version. _purge_versions() always keeps the latest released version.
result = await client.set_setting(env_2_id, AUTO_DEPLOY, False)
assert result.code == 200

# Create a lot of versions in both environments
for _ in range(n_many_versions):
Expand Down

0 comments on commit 89cf618

Please sign in to comment.