From 00642faa2a3c3991184e1497fd6ee7fcefbc3d35 Mon Sep 17 00:00:00 2001 From: Paul Farault Date: Wed, 20 Nov 2024 11:07:08 +0100 Subject: [PATCH 1/8] feat: use OperationHostTuple NamedTuple feature --- tdp/core/models/deployment_model.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tdp/core/models/deployment_model.py b/tdp/core/models/deployment_model.py index c64fbb1f..5aac9e52 100644 --- a/tdp/core/models/deployment_model.py +++ b/tdp/core/models/deployment_model.py @@ -349,8 +349,13 @@ def from_stale_hosted_entities( dag = Dag(collections) reconfigure_operations_sorted = list( map( - lambda x: (dag.node_to_operation(x[0], restart=True), x[1]), - dag.topological_sort_key(operation_hosts, key=lambda x: x[0]), + lambda x: ( + dag.node_to_operation(x.operation_name, restart=True), + x.host_name, + ), + dag.topological_sort_key( + operation_hosts, key=lambda x: x.operation_name + ), ) ) @@ -504,7 +509,4 @@ def _get_reconfigure_operation_hosts( if len(operation_hosts) == 0: raise NothingToReconfigureError("No component needs to be reconfigured.") # Sort by hosts to improve readability - return sorted( - operation_hosts, - key=lambda x: f"{x[0]}_{x[1]}", # order by _ - ) + return sorted(operation_hosts, key=lambda x: f"{x.operation_name}_{x.host_name}") From d03309c42972352fd1c492500c2d6680d525b442 Mon Sep 17 00:00:00 2001 From: Paul Farault Date: Wed, 20 Nov 2024 11:03:52 +0100 Subject: [PATCH 2/8] docs: improve _get_reconfigure_operation_hosts docstrings --- tdp/core/models/deployment_model.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tdp/core/models/deployment_model.py b/tdp/core/models/deployment_model.py index 5aac9e52..0b426850 100644 --- a/tdp/core/models/deployment_model.py +++ b/tdp/core/models/deployment_model.py @@ -476,22 +476,24 @@ def _filter_falsy_options(options: dict) -> dict: class OperationHostTuple(NamedTuple): + """Association of an operation string and its optional host.""" + operation_name: str host_name: Optional[str] def _get_reconfigure_operation_hosts( - stale_hosted_entity_statuses: list[HostedEntityStatus], + hosted_entity_statuses: list[HostedEntityStatus], ) -> list[OperationHostTuple]: - """Get the list of reconfigure operations from a list of hosted entities statuses. + """Generate a list of config and restart operation associated with their host. Args: - stale_hosted_entity_statuses: List of stale hosted entities statuses. + hosted_entity_statuses: List of hosted entities statuses. Returns: List of tuple (operation, host) ordered _. """ operation_hosts: set[OperationHostTuple] = set() - for status in stale_hosted_entity_statuses: + for status in hosted_entity_statuses: if status.to_config: operation_hosts.add( OperationHostTuple( From ab80783246c559df1a24510026253ff54cc6bb35 Mon Sep 17 00:00:00 2001 From: Paul Farault Date: Wed, 20 Nov 2024 13:51:26 +0100 Subject: [PATCH 3/8] feat: _get_reconfigure_operation_hosts returns restart operations --- tdp/core/models/deployment_model.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tdp/core/models/deployment_model.py b/tdp/core/models/deployment_model.py index 0b426850..407bdd4d 100644 --- a/tdp/core/models/deployment_model.py +++ b/tdp/core/models/deployment_model.py @@ -350,7 +350,7 @@ def from_stale_hosted_entities( reconfigure_operations_sorted = list( map( lambda x: ( - dag.node_to_operation(x.operation_name, restart=True), + dag.node_to_operation(x.operation_name), x.host_name, ), dag.topological_sort_key( @@ -504,7 +504,7 @@ def _get_reconfigure_operation_hosts( if status.to_restart: operation_hosts.add( OperationHostTuple( - f"{status.entity.name}_start", + f"{status.entity.name}_restart", status.entity.host, ) ) From be670deacfb17e46d4ac1724f20f0d8274a33e19 Mon Sep 17 00:00:00 2001 From: Paul Farault Date: Wed, 20 Nov 2024 11:28:42 +0100 Subject: [PATCH 4/8] feat: use a generator inside of from_stale_hosted_entities --- tdp/core/models/deployment_model.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/tdp/core/models/deployment_model.py b/tdp/core/models/deployment_model.py index 407bdd4d..e926c08c 100644 --- a/tdp/core/models/deployment_model.py +++ b/tdp/core/models/deployment_model.py @@ -347,16 +347,12 @@ def from_stale_hosted_entities( # Sort operations using DAG topological sort. Convert operation name to # Operation instance and replace "start" action by "restart". dag = Dag(collections) - reconfigure_operations_sorted = list( - map( - lambda x: ( - dag.node_to_operation(x.operation_name), - x.host_name, - ), - dag.topological_sort_key( - operation_hosts, key=lambda x: x.operation_name - ), - ) + reconfigure_operations_sorted = map( + lambda x: ( + dag.node_to_operation(x.operation_name), + x.host_name, + ), + dag.topological_sort_key(operation_hosts, key=lambda x: x.operation_name), ) # Generate deployment From 15b44b54db7e50ea29793bf6bcde314b0cf12d4d Mon Sep 17 00:00:00 2001 From: Paul Farault Date: Wed, 20 Nov 2024 11:33:34 +0100 Subject: [PATCH 5/8] feat: merge DeploymentModel.from_stale_hosted_entities and _get_reconfigure_operation_hosts --- tdp/core/models/deployment_model.py | 78 ++++++++++++----------------- 1 file changed, 33 insertions(+), 45 deletions(-) diff --git a/tdp/core/models/deployment_model.py b/tdp/core/models/deployment_model.py index e926c08c..707d9526 100644 --- a/tdp/core/models/deployment_model.py +++ b/tdp/core/models/deployment_model.py @@ -342,17 +342,34 @@ def from_stale_hosted_entities( Raises: NothingToReconfigureError: If no component needs to be reconfigured. """ - operation_hosts = _get_reconfigure_operation_hosts(stale_hosted_entity_statuses) - - # Sort operations using DAG topological sort. Convert operation name to - # Operation instance and replace "start" action by "restart". - dag = Dag(collections) - reconfigure_operations_sorted = map( - lambda x: ( - dag.node_to_operation(x.operation_name), - x.host_name, - ), - dag.topological_sort_key(operation_hosts, key=lambda x: x.operation_name), + operation_hosts: set[OperationHostTuple] = set() + for status in stale_hosted_entity_statuses: + if status.to_config: + operation_hosts.add( + OperationHostTuple( + f"{status.entity.name}_config", + status.entity.host, + ) + ) + if status.to_restart: + operation_hosts.add( + OperationHostTuple( + f"{status.entity.name}_restart", + status.entity.host, + ) + ) + if len(operation_hosts) == 0: + raise NothingToReconfigureError("No component needs to be reconfigured.") + + # Sort by hosts to improve readability + operation_hosts_sorted = sorted( + operation_hosts, key=lambda x: f"{x.operation_name}_{x.host_name}" + ) + + # Sort operations using DAG topological sort. + reconfigure_operations_sorted = Dag(collections).topological_sort_key( + operation_hosts_sorted, + key=lambda x: x.operation_name.replace("_restart", "_start"), ) # Generate deployment @@ -371,7 +388,7 @@ def from_stale_hosted_entities( for operation, host in reconfigure_operations_sorted: deployment.operations.append( OperationModel( - operation=operation.name, + operation=operation, operation_order=operation_order, host=host, extra_vars=None, @@ -379,7 +396,10 @@ def from_stale_hosted_entities( ) ) # Add sleep operation after each "restart" - if rolling_interval is not None and operation.action_name == "restart": + if ( + rolling_interval is not None + and Operation(operation).action_name == "restart" + ): operation_order += 1 deployment.operations.append( OperationModel( @@ -476,35 +496,3 @@ class OperationHostTuple(NamedTuple): operation_name: str host_name: Optional[str] - - -def _get_reconfigure_operation_hosts( - hosted_entity_statuses: list[HostedEntityStatus], -) -> list[OperationHostTuple]: - """Generate a list of config and restart operation associated with their host. - - Args: - hosted_entity_statuses: List of hosted entities statuses. - - Returns: List of tuple (operation, host) ordered _. - """ - operation_hosts: set[OperationHostTuple] = set() - for status in hosted_entity_statuses: - if status.to_config: - operation_hosts.add( - OperationHostTuple( - f"{status.entity.name}_config", - status.entity.host, - ) - ) - if status.to_restart: - operation_hosts.add( - OperationHostTuple( - f"{status.entity.name}_restart", - status.entity.host, - ) - ) - if len(operation_hosts) == 0: - raise NothingToReconfigureError("No component needs to be reconfigured.") - # Sort by hosts to improve readability - return sorted(operation_hosts, key=lambda x: f"{x.operation_name}_{x.host_name}") From 7046ec810cd88b3c466e0b36a19ea32d8f82f0d9 Mon Sep 17 00:00:00 2001 From: Paul Farault Date: Wed, 20 Nov 2024 11:38:27 +0100 Subject: [PATCH 6/8] style: improve from_stale_hosted_entities --- tdp/core/models/deployment_model.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tdp/core/models/deployment_model.py b/tdp/core/models/deployment_model.py index 707d9526..325f1879 100644 --- a/tdp/core/models/deployment_model.py +++ b/tdp/core/models/deployment_model.py @@ -360,18 +360,15 @@ def from_stale_hosted_entities( ) if len(operation_hosts) == 0: raise NothingToReconfigureError("No component needs to be reconfigured.") - # Sort by hosts to improve readability operation_hosts_sorted = sorted( operation_hosts, key=lambda x: f"{x.operation_name}_{x.host_name}" ) - # Sort operations using DAG topological sort. - reconfigure_operations_sorted = Dag(collections).topological_sort_key( + operation_hosts_sorted = Dag(collections).topological_sort_key( operation_hosts_sorted, key=lambda x: x.operation_name.replace("_restart", "_start"), ) - # Generate deployment deployment = DeploymentModel( deployment_type=DeploymentTypeEnum.RECONFIGURE, @@ -385,7 +382,7 @@ def from_stale_hosted_entities( state=DeploymentStateEnum.PLANNED, ) operation_order = 1 - for operation, host in reconfigure_operations_sorted: + for operation, host in operation_hosts_sorted: deployment.operations.append( OperationModel( operation=operation, @@ -410,7 +407,6 @@ def from_stale_hosted_entities( state=OperationStateEnum.PLANNED, ) ) - operation_order += 1 return deployment From 93524b3e46d92c546c3a264d6c4f80ae5a5cd610 Mon Sep 17 00:00:00 2001 From: Paul Farault Date: Wed, 20 Nov 2024 11:42:18 +0100 Subject: [PATCH 7/8] feat: DeploymentModel.from_stale_hosted_entities can take all hosted entities Not only stale one as it is filtering it anyway. --- tdp/cli/commands/plan/reconfigure.py | 4 +--- tdp/core/deployment/deployment_iterator.py | 4 +--- tdp/core/models/deployment_model.py | 6 +++--- test_dag_order/conftest.py | 2 +- 4 files changed, 6 insertions(+), 10 deletions(-) diff --git a/tdp/cli/commands/plan/reconfigure.py b/tdp/cli/commands/plan/reconfigure.py index e1378eb4..1605e6ed 100644 --- a/tdp/cli/commands/plan/reconfigure.py +++ b/tdp/cli/commands/plan/reconfigure.py @@ -38,9 +38,7 @@ def reconfigure( with Dao(db_engine, commit_on_exit=True) as dao: deployment = DeploymentModel.from_stale_hosted_entities( collections=collections, - stale_hosted_entity_statuses=dao.get_hosted_entity_statuses( - filter_stale=True - ), + hosted_entity_statuses=dao.get_hosted_entity_statuses(filter_stale=True), rolling_interval=rolling_interval, ) if preview: diff --git a/tdp/core/deployment/deployment_iterator.py b/tdp/core/deployment/deployment_iterator.py index 241bd018..6d415296 100644 --- a/tdp/core/deployment/deployment_iterator.py +++ b/tdp/core/deployment/deployment_iterator.py @@ -103,9 +103,7 @@ def __init__( self._reconfigure_operations = _group_hosts_by_operation( DeploymentModel.from_stale_hosted_entities( collections=self._collections, - stale_hosted_entity_statuses=[ - status for status in cluster_status.values() if status.is_stale - ], + hosted_entity_statuses=list(cluster_status.values()), ) ) except NothingToReconfigureError: diff --git a/tdp/core/models/deployment_model.py b/tdp/core/models/deployment_model.py index 325f1879..af752675 100644 --- a/tdp/core/models/deployment_model.py +++ b/tdp/core/models/deployment_model.py @@ -329,21 +329,21 @@ def from_operations_hosts_vars( @staticmethod def from_stale_hosted_entities( collections: Collections, - stale_hosted_entity_statuses: list[HostedEntityStatus], + hosted_entity_statuses: list[HostedEntityStatus], rolling_interval: Optional[int] = None, ) -> DeploymentModel: """Generate a deployment plan for stale components. Args: collections: Collections to retrieve the operations from. - stale_hosted_entity_statuses: List of stale hosted entity statuses. + hosted_entity_statuses: List of hosted entity statuses. rolling_interval: Number of seconds to wait between component restart. Raises: NothingToReconfigureError: If no component needs to be reconfigured. """ operation_hosts: set[OperationHostTuple] = set() - for status in stale_hosted_entity_statuses: + for status in hosted_entity_statuses: if status.to_config: operation_hosts.add( OperationHostTuple( diff --git a/test_dag_order/conftest.py b/test_dag_order/conftest.py index 5bcb9313..3163e3e5 100644 --- a/test_dag_order/conftest.py +++ b/test_dag_order/conftest.py @@ -200,7 +200,7 @@ def plan_reconfigure( # return the deployment plan (it is neither persisted in the database nor executed) return DeploymentModel.from_stale_hosted_entities( collections=collections, - stale_hosted_entity_statuses=dao.get_hosted_entity_statuses(filter_stale=True), + hosted_entity_statuses=dao.get_hosted_entity_statuses(filter_stale=True), ) From 068b4af4baacbf2be13fcbaa702361728517c83b Mon Sep 17 00:00:00 2001 From: Paul Farault Date: Wed, 20 Nov 2024 12:10:43 +0100 Subject: [PATCH 8/8] refactor: make Dag._node_to_operation private --- tdp/core/dag.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tdp/core/dag.py b/tdp/core/dag.py index 84584a00..2398cee6 100644 --- a/tdp/core/dag.py +++ b/tdp/core/dag.py @@ -60,7 +60,7 @@ def graph(self) -> nx.DiGraph: """DAG graph.""" return self._graph - def node_to_operation( + def _node_to_operation( self, node: str, restart: bool = False, stop: bool = False ) -> Operation: # ? Restart operations are now stored in collections.operations they can be @@ -152,7 +152,7 @@ def topological_sort( """ return list( map( - lambda node: self.node_to_operation(node, restart=restart, stop=stop), + lambda node: self._node_to_operation(node, restart=restart, stop=stop), self.topological_sort_key(nodes), ) ) @@ -234,7 +234,7 @@ def get_operation_descendants( nodes_filtered = filter(lambda node: node not in nodes, nodes_set) return list( map( - lambda node: self.node_to_operation(node, restart=restart, stop=stop), + lambda node: self._node_to_operation(node, restart=restart, stop=stop), nodes_filtered, ) )