Skip to content

Commit

Permalink
Remove deployed and result from configuration model (Issue #8252,…
Browse files Browse the repository at this point in the history
… PR #8320)

# Description

Strip  version and status

closes #8252

# Self Check:

Strike through any lines that are not applicable (`~~line~~`) then check the box

- [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
wouterdb authored and inmantaci committed Nov 7, 2024
1 parent 781914b commit c595ef6
Show file tree
Hide file tree
Showing 25 changed files with 1,794 additions and 475 deletions.
2 changes: 1 addition & 1 deletion changelogs/unreleased/8196-remove_status.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ issue-nr: 8196
change-type: major
destination-branches: [master]
sections:
upgrade-note: >
upgrade-note: |
All api endpoints reporting deployment status for specific versions of resources have been removed.
| api endpoint | change | alternative |
Expand Down
19 changes: 19 additions & 0 deletions changelogs/unreleased/8252-remove-deploy-status.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
description: Remove `deployed` and `result` from configuration model
issue-nr: 8252
change-type: major
destination-branches: [master]
sections:
upgrade-note: |
The fields ``deployed`` and ``result`` have been removed from all API endpoint and tools that report on versions.
Deploy status information is no longer maintained for individual versions, but only for the latest, active version.
| api endpoint | change | alternative |
| ------------ | ------ | ------------|
| `GET /api/v1/version` | removal of the `deployed` and `result` fields from the response | `GET /api/v2/resource/` |
| `GET /api/v1/version/<id>` | removal of the `deployed` and `result` fields from the response | `GET /api/v2/resource` |
| `POST /api/v1/version/<id>` | removal of the `deployed` and `result` fields from the response | `GET /api/v2/resource` |
| command | change |
|---------| ------ |
| `inmanta-cli version release` | removal of the "Deployed" and "# Done" columns from the output and "State" field now reports the same state as the corresponding page in the web-console |
| `inmanta-cli version list` | removal of the "Deployed" and "# Done" columns from the output and "State" field now reports the same state as the corresponding page in the web-console |
16 changes: 1 addition & 15 deletions mypy-baseline.txt
Original file line number Diff line number Diff line change
Expand Up @@ -603,20 +603,6 @@ src/inmanta/data/__init__.py:0: note: def [_T] __init__(self) -> list[_T]
src/inmanta/data/__init__.py:0: note: def [_T] __init__(self, Iterable[_T], /) -> list[_T]
src/inmanta/data/__init__.py:0: error: "object" has no attribute "__iter__"; maybe "__dir__" or "__str__"? (not iterable) [attr-defined]
src/inmanta/data/__init__.py:0: error: Argument 1 to "__init__" of "BaseDocument" has incompatible type "**dict[str, object]"; expected "bool" [arg-type]
src/inmanta/data/__init__.py:0: error: Need type annotation for "_status" (hint: "_status: dict[<type>, <type>] = ...") [var-annotated]
src/inmanta/data/__init__.py:0: error: Invalid index type "str" for "str"; expected type "SupportsIndex | slice" [index]
src/inmanta/data/__init__.py:0: error: Signature of "get_list" incompatible with supertype "BaseDocument" [override]
src/inmanta/data/__init__.py:0: note: Superclass:
src/inmanta/data/__init__.py:0: note: @classmethod
src/inmanta/data/__init__.py:0: note: def get_list(cls, *, order_by_column: str | None = ..., order: str | None = ..., limit: int | None = ..., offset: int | None = ..., no_obj: bool | None = ..., lock: RowLockMode | None = ..., connection: Connection | None = ..., **query: object) -> Coroutine[Any, Any, list[ConfigurationModel]]
src/inmanta/data/__init__.py:0: note: Subclass:
src/inmanta/data/__init__.py:0: note: @classmethod
src/inmanta/data/__init__.py:0: note: def get_list(cls, *, order_by_column: str | None = ..., order: str | None = ..., limit: int | None = ..., offset: int | None = ..., no_obj: bool | None = ..., lock: RowLockMode | None = ..., connection: Connection | None = ..., no_status: bool = ..., **query: object) -> Coroutine[Any, Any, list[ConfigurationModel]]
src/inmanta/data/__init__.py:0: error: Argument 1 to "_get_status_field" of "ConfigurationModel" has incompatible type "object"; expected "UUID" [arg-type]
src/inmanta/data/__init__.py:0: error: Argument 2 to "_get_status_field" of "ConfigurationModel" has incompatible type "object"; expected "str" [arg-type]
src/inmanta/data/__init__.py:0: error: Argument 1 to "_get_status_field" of "ConfigurationModel" has incompatible type "object"; expected "UUID" [arg-type]
src/inmanta/data/__init__.py:0: error: Argument 2 to "_get_status_field" of "ConfigurationModel" has incompatible type "object"; expected "str" [arg-type]
src/inmanta/data/__init__.py:0: error: Incompatible types in assignment (expression has type "object", variable has type "int") [assignment]
src/inmanta/data/__init__.py:0: error: Argument 1 to "append" of "list" has incompatible type "ConfigurationModel"; expected "dict[str, object]" [arg-type]
src/inmanta/data/__init__.py:0: error: Incompatible return value type (got "list[dict[str, object]]", expected "list[ConfigurationModel]") [return-value]
src/inmanta/data/__init__.py:0: error: Item "None" of "RowLockMode | None" has no attribute "value" [union-attr]
Expand All @@ -626,7 +612,6 @@ src/inmanta/data/__init__.py:0: note: Possible overload variants:
src/inmanta/data/__init__.py:0: note: def __new__(cls, str | Buffer | SupportsInt | SupportsIndex | SupportsTrunc = ..., /) -> int
src/inmanta/data/__init__.py:0: note: def __new__(cls, str | bytes | bytearray, /, base: SupportsIndex) -> int
src/inmanta/data/__init__.py:0: error: Incompatible return value type (got "list[object]", expected "list[str]") [return-value]
src/inmanta/data/__init__.py:0: error: Enum index should be a string (actual index type "object") [misc]
src/inmanta/data/__init__.py:0: error: "object" has no attribute "__iter__"; maybe "__dir__" or "__str__"? (not iterable) [attr-defined]
src/inmanta/data/__init__.py:0: error: Unsupported left operand type for > ("object") [operator]
src/inmanta/data/__init__.py:0: error: Incompatible types in assignment (expression has type "object", variable has type "int") [assignment]
Expand Down Expand Up @@ -1132,4 +1117,5 @@ src/inmanta/model.py:0: error: "Value" has no attribute "to_dict" [attr-defined
src/inmanta/main.py:0: error: Returning Any from function declared to return "dict[str, Any] | None" [no-any-return]
src/inmanta/main.py:0: error: Invalid index type "str" for "str"; expected type "SupportsIndex | slice" [index]
src/inmanta/main.py:0: error: Value of type "dict[str, Any] | None" is not indexable [index]
src/inmanta/main.py:0: error: Value of type "dict[str, Any] | None" is not indexable [index]
src/inmanta/main.py:0: error: Need type annotation for "bar" [var-annotated]
3 changes: 2 additions & 1 deletion src/inmanta/agent/agent_new.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,10 +209,11 @@ def create_executor_manager(self) -> executor.ExecutorManager[executor.Executor]
async def stop(self) -> None:
if self.working:
await self.stop_working()
if self._db_monitor:
await self._db_monitor.stop()
threadpools_to_join = [self.thread_pool]
await self.executor_manager.join(threadpools_to_join, const.SHUTDOWN_GRACE_IOLOOP * 0.9)
self.thread_pool.shutdown(wait=False)

await join_threadpools(threadpools_to_join)
await super().stop()

Expand Down
125 changes: 6 additions & 119 deletions src/inmanta/data/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
import inmanta.db.versions
from crontab import CronTab
from inmanta import const, resources, util
from inmanta.const import DATETIME_MIN_UTC, DONE_STATES, UNDEPLOYABLE_NAMES, AgentStatus, LogLevel, ResourceState
from inmanta.const import DATETIME_MIN_UTC, UNDEPLOYABLE_NAMES, AgentStatus, LogLevel, ResourceState
from inmanta.data import model as m
from inmanta.data import schema
from inmanta.data.model import (
Expand Down Expand Up @@ -5446,8 +5446,6 @@ class ConfigurationModel(BaseDocument):
pip_config: Optional[PipConfig] = None

released: bool = False
deployed: bool = False
result: const.VersionState = const.VersionState.pending
version_info: Optional[dict[str, object]] = None
is_suitable_for_partial_compiles: bool

Expand All @@ -5459,20 +5457,10 @@ class ConfigurationModel(BaseDocument):

def __init__(self, **kwargs: object) -> None:
super().__init__(**kwargs)
self._status = {}
self._done = 0

@classmethod
def get_valid_field_names(cls) -> list[str]:
return super().get_valid_field_names() + ["status", "model"]

@property
def done(self) -> int:
# Keep resources which are deployed in done, even when a repair operation
# changes its state to deploying again.
if self.deployed:
return self.total
return self._done
return super().get_valid_field_names() + ["model"]

@classmethod
async def create_for_partial_compile(
Expand Down Expand Up @@ -5601,8 +5589,6 @@ async def create_for_partial_compile(
skipped_for_undeployable,
partial_base,
released,
deployed,
result,
is_suitable_for_partial_compiles,
pip_config
"""
Expand All @@ -5627,18 +5613,6 @@ async def create_for_partial_compile(
fields = {name: val for name, val in result.items() if name != "base_version_found"}
return cls(from_postgres=True, **fields)

@classmethod
async def _get_status_field(cls, environment: uuid.UUID, values: str) -> dict[str, str]:
"""
This field is required to ensure backward compatibility on the API.
"""
result = {}
values = json.loads(values)
for value_entry in values:
entry_uuid = str(uuid.uuid5(environment, value_entry["id"]))
result[entry_uuid] = value_entry
return result

@classmethod
async def get_list(
cls,
Expand All @@ -5650,7 +5624,6 @@ async def get_list(
no_obj: Optional[bool] = None,
lock: Optional[RowLockMode] = None,
connection: Optional[asyncpg.connection.Connection] = None,
no_status: bool = False, # don't load the status field
**query: object,
) -> list["ConfigurationModel"]:
# sanitize and validate order parameters
Expand All @@ -5668,24 +5641,15 @@ async def get_list(
if offset is not None:
offset = int(offset)

transient_states = ",".join(["$" + str(i) for i in range(1, len(const.TRANSIENT_STATES) + 1)])
transient_states_values = [cls._get_value(s) for s in const.TRANSIENT_STATES]
(filterstr, values) = cls._get_composed_filter(col_name_prefix="c", offset=len(transient_states_values) + 1, **query)
values = transient_states_values + values
(filterstr, values) = cls._get_composed_filter(col_name_prefix="c", offset=1, **query)
values = values
where_statement = f"WHERE {filterstr} " if filterstr else ""
order_by_statement = f"ORDER BY {order_by_column} {order} " if order_by_column else ""
limit_statement = f"LIMIT {limit} " if limit is not None and limit > 0 else ""
offset_statement = f"OFFSET {offset} " if offset is not None and offset > 0 else ""
lock_statement = f" {lock.value} " if lock is not None else ""
query_string = f"""SELECT c.*,
SUM(CASE WHEN r.status NOT IN({transient_states}) THEN 1 ELSE 0 END) AS done,
to_json(array(SELECT jsonb_build_object('status', r2.status, 'id', r2.resource_id)
FROM {Resource.table_name()} AS r2
WHERE c.environment=r2.environment AND c.version=r2.model
)
) AS status
FROM {cls.table_name()} AS c LEFT OUTER JOIN {Resource.table_name()} AS r
ON c.environment = r.environment AND c.version = r.model
query_string = f"""SELECT c.*
FROM {cls.table_name()} AS c
{where_statement}
GROUP BY c.environment, c.version
{order_by_statement}
Expand All @@ -5697,30 +5661,12 @@ async def get_list(
for in_record in query_result:
record = dict(in_record)
if no_obj:
if no_status:
record["status"] = {}
else:
record["status"] = await cls._get_status_field(record["environment"], record["status"])
result.append(record)
else:
done = record.pop("done")
if no_status:
status = {}
record.pop("status")
else:
status = await cls._get_status_field(record["environment"], record.pop("status"))
obj = cls(from_postgres=True, **record)
obj._done = done
obj._status = status
result.append(obj)
return result

def to_dict(self) -> JsonType:
dct = BaseDocument.to_dict(self)
dct["status"] = dict(self._status)
dct["done"] = self.done
return dct

@classmethod
async def version_exists(cls, environment: uuid.UUID, version: int) -> bool:
query = f"""SELECT 1
Expand Down Expand Up @@ -5883,65 +5829,6 @@ def get_skipped_for_undeployable(self) -> list[m.ResourceIdStr]:
"""
return self.skipped_for_undeployable

async def mark_done(self, *, connection: Optional[asyncpg.connection.Connection] = None) -> None:
"""mark this deploy as done"""
subquery = f"""(EXISTS(
SELECT 1
FROM {Resource.table_name()}
WHERE environment=$1 AND model=$2 AND status != $3
))::boolean
"""
query = f"""UPDATE {self.table_name()}
SET
deployed=True, result=(CASE WHEN {subquery} THEN $4::versionstate ELSE $5::versionstate END)
WHERE environment=$1 AND version=$2 RETURNING result
"""
values = [
self._get_value(self.environment),
self._get_value(self.version),
self._get_value(const.ResourceState.deployed),
self._get_value(const.VersionState.failed),
self._get_value(const.VersionState.success),
]
result = await self._fetchval(query, *values, connection=connection)
self.result = const.VersionState[result]
self.deployed = True

@classmethod
async def mark_done_if_done(
cls, environment: uuid.UUID, version: int, connection: Optional[asyncpg.connection.Connection] = None
) -> None:
async with cls.get_connection(connection) as con:
"""
Performs the query to mark done if done. Expects to be called outside of any transaction that writes resource state
in order to prevent race conditions.
"""
async with con.transaction():
query = f"""UPDATE {ConfigurationModel.table_name()}
SET deployed=True,
result=(CASE WHEN (
EXISTS(SELECT 1
FROM {Resource.table_name()}
WHERE environment=$1 AND model=$2 AND status != $3)
)::boolean
THEN $4::versionstate
ELSE $5::versionstate END
)
WHERE environment=$1 AND version=$2 AND
total=(SELECT COUNT(*)
FROM {Resource.table_name()}
WHERE environment=$1 AND model=$2 AND status = any($6::resourcestate[])
)"""
values = [
cls._get_value(environment),
cls._get_value(version),
cls._get_value(ResourceState.deployed),
cls._get_value(const.VersionState.failed),
cls._get_value(const.VersionState.success),
cls._get_value(DONE_STATES),
]
await cls._execute_query(query, *values, connection=con)

@classmethod
async def get_increment(
cls, environment: uuid.UUID, version: int, *, connection: Optional[Connection] = None
Expand Down
29 changes: 29 additions & 0 deletions src/inmanta/db/versions/v202410310.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
"""
Copyright 2024 Inmanta
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
Contact: [email protected]
"""

from asyncpg import Connection


async def update(connection: Connection) -> None:
"""
Drop the deployed and result columns from configurationmodel.
"""
schema = """
ALTER TABLE configurationmodel DROP deployed, DROP result;
"""
await connection.execute(schema)
Loading

0 comments on commit c595ef6

Please sign in to comment.