Alexsander de Souza has proposed merging ~alexsander-souza/maas:lp1995624_suppress_script_results into maas:master.
Commit message: add suppress_failed_script_results() to machine WS fixes LP#1995624 Requested reviews: MAAS Maintainers (maas-maintainers) Related bugs: Bug #1995624 in MAAS: "suppressing script results no longer available on machine listing" https://bugs.launchpad.net/maas/+bug/1995624 For more details, see: https://code.launchpad.net/~alexsander-souza/maas/+git/maas/+merge/433484 -- Your team MAAS Committers is subscribed to branch maas:master.
diff --git a/src/maasserver/websockets/handlers/machine.py b/src/maasserver/websockets/handlers/machine.py index 7572062..380616b 100644 --- a/src/maasserver/websockets/handlers/machine.py +++ b/src/maasserver/websockets/handlers/machine.py @@ -94,6 +94,7 @@ from maasserver.websockets.base import ( ) from maasserver.websockets.handlers.node import node_prefetch, NodeHandler from metadataserver.enum import HARDWARE_TYPE, RESULT_TYPE +from metadataserver.models.scriptresult import ScriptResult from provisioningserver.certificates import Certificate from provisioningserver.logger import LegacyLogger from provisioningserver.rpc.exceptions import UnknownPowerType @@ -251,6 +252,7 @@ class MachineHandler(NodeHandler): "filter_groups", "filter_options", "count", + "suppress_failed_script_results", ] form = AdminMachineWithMACAddressesForm exclude = [ @@ -312,11 +314,11 @@ class MachineHandler(NodeHandler): super().__init__(*args, **kwargs) self._deployed = False - def get_queryset(self, for_list=False): + def get_queryset(self, for_list=False, perm=NodePermission.view): """Return `QuerySet` for devices only viewable by `user`.""" return Machine.objects.get_nodes( self.user, - NodePermission.view, + perm, from_nodes=super().get_queryset(for_list=for_list), ) @@ -1219,3 +1221,21 @@ class MachineHandler(NodeHandler): if "filter" in params: qs = self._filter(qs, "list", params["filter"]) return {"count": qs.count()} + + def suppress_failed_script_results(self, params): + qs = self.get_queryset(for_list=True) + system_ids = self._filter( + qs, "list", params.get("filter") + ).values_list("system_id", flat=True) + script_results = self._get_latest_failed_testing_script_results( + system_ids + ) + script_result_ids = [ + script_result.id for script_result in script_results + ] + ScriptResult.objects.filter( + id__in=script_result_ids, + ).update(suppressed=True) + return { + "success_count": len(script_result_ids), + } diff --git a/src/maasserver/websockets/handlers/node.py b/src/maasserver/websockets/handlers/node.py index 218a113..3290e09 100644 --- a/src/maasserver/websockets/handlers/node.py +++ b/src/maasserver/websockets/handlers/node.py @@ -1149,32 +1149,25 @@ class NodeHandler(TimestampedModelHandler): def set_script_result_suppressed(self, params): """Set suppressed for the ScriptResult ids.""" - node = self.get_object(params) - if not self.user.has_perm(NodePermission.admin, node): - raise HandlerPermissionError() + node = self.get_object(params, NodePermission.admin) script_result_ids = params.get("script_result_ids") - ScriptResult.objects.filter(id__in=script_result_ids).update( - suppressed=True - ) + ScriptResult.objects.filter( + script_set__node__system_id=node.system_id, + id__in=script_result_ids, + ).update(suppressed=True) def set_script_result_unsuppressed(self, params): """Set unsuppressed for the ScriptResult ids.""" - node = self.get_object(params) - if not self.user.has_perm(NodePermission.admin, node): - raise HandlerPermissionError() + node = self.get_object(params, NodePermission.admin) script_result_ids = params.get("script_result_ids") - ScriptResult.objects.filter(id__in=script_result_ids).update( - suppressed=False - ) - - def get_latest_failed_testing_script_results(self, params): - """Return a dictionary with Nodes system_ids mapped to a list of - the latest failed ScriptResults.""" - node_result_handler = NodeResultHandler(self.user, {}, None) - system_ids = params.get("system_ids") - - # Create the node to script result mappings. - script_result_mappings = {} + ScriptResult.objects.filter( + script_set__node__system_id=node.system_id, + id__in=script_result_ids, + ).update(suppressed=False) + + def _get_latest_failed_testing_script_results( + self, system_ids: Iterable[str] + ) -> Iterable[ScriptResult]: script_results = ( ScriptResult.objects.filter( script_set__node__system_id__in=system_ids, @@ -1199,26 +1192,34 @@ class NodeHandler(TimestampedModelHandler): "script_set__node_id", "script_name", "physical_blockdevice_id" ) ) + node_script_results = [ + s for s in script_results if s.status in SCRIPT_STATUS_FAILED + ] + return node_script_results - for system_id in system_ids: - # Need to evaluate QuerySet first to get latest script results, - # then filter by results that have failed - node_script_results = [ - s - for s in script_results - if s.status in SCRIPT_STATUS_FAILED - and (s.script_set.node.system_id == system_id) - ] - - for script_result in node_script_results: - if system_id not in script_result_mappings: - script_result_mappings[system_id] = [] + def get_latest_failed_testing_script_results(self, params): + """Return a dictionary with Nodes system_ids mapped to a list of + the latest failed ScriptResults.""" + node_result_handler = NodeResultHandler(self.user, {}, None) + if "filter" in params: + qs = self.get_queryset(for_list=True) + system_ids = self._filter( + qs, "list", params.get("filter") + ).values_list("system_id", flat=True) + else: + system_ids = params.get("system_ids") - mapping = node_result_handler.dehydrate( - script_result, {}, for_list=True - ) - mapping["id"] = script_result.id - script_result_mappings[system_id].append(mapping) + script_results = self._get_latest_failed_testing_script_results( + system_ids + ) + script_result_mappings = {} + for script_result in script_results: + system_id = script_result.script_set.node.system_id + mapping = node_result_handler.dehydrate( + script_result, {}, for_list=True + ) + mapping["id"] = script_result.id + script_result_mappings.setdefault(system_id, []).append(mapping) return script_result_mappings def _update_tags(self, node, tag_ids): diff --git a/src/maasserver/websockets/handlers/tests/test_machine.py b/src/maasserver/websockets/handlers/tests/test_machine.py index 7c39ba3..5e90c0a 100644 --- a/src/maasserver/websockets/handlers/tests/test_machine.py +++ b/src/maasserver/websockets/handlers/tests/test_machine.py @@ -607,6 +607,7 @@ class TestMachineHandler(MAASServerTestCase): "refetch", "update_blockdevice_filesystem", "update_partition_filesystem", + "set_script_result_suppressed_value", ] [ self.assertIn(attr, MachineHandler.Meta.allowed_methods) @@ -5634,6 +5635,34 @@ class TestMachineHandlerUpdateFilesystem(MAASServerTestCase): blockdevice = reload_object(blockdevice) self.assertEqual(blockdevice.tags, []) + +class TestMachineHandlerSuppressScriptResult(MAASServerTestCase): + def create_script_results(self, nodes, count=5): + script_results = [] + for node in nodes: + script = factory.make_Script() + for _ in range(count): + script_set = factory.make_ScriptSet( + result_type=script.script_type, node=node + ) + factory.make_ScriptResult(script=script, script_set=script_set) + + script_set = factory.make_ScriptSet( + result_type=script.script_type, node=node + ) + script_result = factory.make_ScriptResult( + script=script, + script_set=script_set, + status=random.choice( + list(SCRIPT_STATUS_FAILED.union({SCRIPT_STATUS.PASSED})) + ), + ) + if script.script_type == SCRIPT_TYPE.TESTING and ( + script_result.status in SCRIPT_STATUS_FAILED + ): + script_results.append(script_result) + return script_results + def test_set_script_result_suppressed(self): owner = factory.make_admin() node = factory.make_Node(owner=owner) @@ -5686,6 +5715,16 @@ class TestMachineHandlerUpdateFilesystem(MAASServerTestCase): params, ) + def test_suppress_failed_script_results(self): + owner = factory.make_User() + handler = MachineHandler(owner, {}, None) + nodes = [factory.make_Node(owner=owner) for _ in range(10)] + failed_results = self.create_script_results(nodes) + result = handler.suppress_failed_script_results( + {"filter": {"owner": owner.username}} + ) + self.assertEqual(len(failed_results), result["success_count"]) + def test_set_script_result_unsuppressed(self): owner = factory.make_admin() node = factory.make_Node(owner=owner) @@ -5746,43 +5785,27 @@ class TestMachineHandlerUpdateFilesystem(MAASServerTestCase): node_result_handler = NodeResultHandler(owner, {}, None) nodes = [factory.make_Node(owner=owner) for _ in range(10)] - script_results = [] - for node in nodes: - script = factory.make_Script() - for run in range(10): - script_set = factory.make_ScriptSet( - result_type=script.script_type, node=node - ) - factory.make_ScriptResult(script=script, script_set=script_set) + script_results = self.create_script_results(nodes) - script_set = factory.make_ScriptSet( - result_type=script.script_type, node=node - ) - script_result = factory.make_ScriptResult( - script=script, - script_set=script_set, - status=random.choice( - list(SCRIPT_STATUS_FAILED.union({SCRIPT_STATUS.PASSED})) - ), - ) - if script.script_type == SCRIPT_TYPE.TESTING and ( - script_result.status in SCRIPT_STATUS_FAILED - ): - script_results.append(script_result) + requests = [ + {"system_ids": [node.system_id for node in nodes]}, + {"filter": {"owner": owner.username}}, + ] - actual = handler.get_latest_failed_testing_script_results( - {"system_ids": [node.system_id for node in nodes]} - ) - expected = {} - for script_result in script_results: - if script_result.script_set.node.system_id not in expected: - expected[script_result.script_set.node.system_id] = [] - mapping = node_result_handler.dehydrate( - script_result, {}, for_list=True - ) - mapping["id"] = script_result.id - expected[script_result.script_set.node.system_id].append(mapping) - self.assertEqual(actual, expected) + for request in requests: + actual = handler.get_latest_failed_testing_script_results(request) + expected = {} + for script_result in script_results: + if script_result.script_set.node.system_id not in expected: + expected[script_result.script_set.node.system_id] = [] + mapping = node_result_handler.dehydrate( + script_result, {}, for_list=True + ) + mapping["id"] = script_result.id + expected[script_result.script_set.node.system_id].append( + mapping + ) + self.assertEqual(actual, expected) def test_get_latest_failed_testing_script_results_num_queries(self): # Prevent RBAC from making a query.
-- Mailing list: https://launchpad.net/~sts-sponsors Post to : sts-sponsors@lists.launchpad.net Unsubscribe : https://launchpad.net/~sts-sponsors More help : https://help.launchpad.net/ListHelp