Merge pull request #2970 from yueguobin/fix/node-lifecycle-status-flood

fix(controller): stop flooding clients with transitional node statuses

gns3server/controller/node.py

@@ -16,7 +16,6 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import asyncio
-import contextlib
 import copy
 import html
 import logging
@@ -139,7 +138,6 @@ class Node:
         self._node_directory = None
         self._status = "stopped"
         self._status_changed = asyncio.Event()
-        self._transition = None
         self._template_id = template_id
         self._x = 0
         self._y = 0
@@ -907,19 +905,6 @@ class Node:
     async def destroy(self):
         await self.delete()
 
-    @contextlib.asynccontextmanager
-    async def _transitioning(self, transition):
-        """
-        Report a transitional status while a lifecycle call is in flight
-        """
-
-        self._transition = transition
-        self.project.emit_notification("node.updated", self.asdict())
-        try:
-            yield
-        finally:
-            self._transition = None
-
     async def wait_for_status(self, status):
         """
         Wait until the controller side status of the node matches
@@ -954,19 +939,18 @@ class Node:
                     f"Cannot start node '{self._name}': {len(failed_links)} deferred link(s) "
                     "could not be restored. Please try again."
                 )
-        async with self._transitioning("starting"):
-            try:
-                # For IOU: we need to send the licence everytime we start a node
-                if self.node_type == "iou":
-                    license_check = self._project.controller.iou_license.get("license_check", True)
-                    iourc_content = self._project.controller.iou_license.get("iourc_content", None)
-                    await self.post(
-                        "/start", timeout=240, data={"license_check": license_check, "iourc_content": iourc_content}
-                    )
-                else:
-                    await self.post("/start", data=data, timeout=240)
-            except asyncio.TimeoutError:
-                raise ControllerTimeoutError(f"Timeout when starting {self._name}")
+        try:
+            # For IOU: we need to send the licence everytime we start a node
+            if self.node_type == "iou":
+                license_check = self._project.controller.iou_license.get("license_check", True)
+                iourc_content = self._project.controller.iou_license.get("iourc_content", None)
+                await self.post(
+                    "/start", timeout=240, data={"license_check": license_check, "iourc_content": iourc_content}
+                )
+            else:
+                await self.post("/start", data=data, timeout=240)
+        except asyncio.TimeoutError:
+            raise ControllerTimeoutError(f"Timeout when starting {self._name}")
         self._confirm_status("started")
         # A started node may have brought kernel anchors into existence that
         # a link needs to finish wiring (the Ethernet-switch fast path joins
@@ -983,16 +967,15 @@ class Node:
         if self.missing_image:
             return
         stopped = False
-        async with self._transitioning("stopping"):
-            try:
-                await self.post("/stop", timeout=240, dont_connect=True)
-                stopped = True
-            except (ComputeError, ControllerError):
-                # We don't care if a node is down at this step
-                if strict:
-                    raise
-            except asyncio.TimeoutError:
-                raise ControllerTimeoutError(f"Timeout when stopping {self._name}")
+        try:
+            await self.post("/stop", timeout=240, dont_connect=True)
+            stopped = True
+        except (ComputeError, ControllerError):
+            # We don't care if a node is down at this step
+            if strict:
+                raise
+        except asyncio.TimeoutError:
+            raise ControllerTimeoutError(f"Timeout when stopping {self._name}")
         if stopped:
             self._confirm_status("stopped")
 
@@ -1388,7 +1371,7 @@ class Node:
         additional_data = {
             "project_id": self._project.id,
             "command_line": self._command_line,
-            "status": self._transition or self._status,
+            "status": self._status,
             "console_host": str(self._compute.console_host),
             "node_directory": self._node_directory,
             "ports": [port.asdict() for port in self.ports],

tests/api/routes/controller/test_nodes.py

@@ -486,7 +486,7 @@ class TestNodeRoutes:
         response = await client.get(app.url_path_for("get_node", project_id=project.id, node_id=node.id))
         assert response.json()["status"] == "started"
 
-    async def test_start_node_reports_starting_while_in_flight(
+    async def test_start_node_keeps_real_status_while_in_flight(
         self, app: FastAPI, client: AsyncClient, project: Project, compute: Compute, node: Node
     ) -> None:
 
@@ -498,10 +498,11 @@ class TestNodeRoutes:
         compute.post = post
         response = await client.post(app.url_path_for("start_node", project_id=project.id, node_id=node.id), json={})
         assert response.status_code == status.HTTP_200_OK
-        assert seen == ["starting"]
+        # no transitional status is reported while the call is in flight
+        assert seen == ["stopped"]
         assert response.json()["status"] == "started"
 
-    async def test_stop_node_reports_stopping_while_in_flight(
+    async def test_stop_node_keeps_real_status_while_in_flight(
         self, app: FastAPI, client: AsyncClient, project: Project, compute: Compute, node: Node
     ) -> None:
 
@@ -514,7 +515,8 @@ class TestNodeRoutes:
         node._status = "started"
         response = await client.post(app.url_path_for("stop_node", project_id=project.id, node_id=node.id))
         assert response.status_code == status.HTTP_200_OK
-        assert seen == ["stopping"]
+        # no transitional status is reported while the call is in flight
+        assert seen == ["started"]
         assert response.json()["status"] == "stopped"
 
     async def test_start_node_wait_timeout(

tests/api/routes/controller/test_version.py

@@ -17,6 +17,7 @@
 import pytest
 from fastapi import FastAPI, status
 from httpx import AsyncClient
+from starlette.routing import Mount
 
 from gns3server.version import __version__
 
@@ -26,6 +27,14 @@ pytestmark = pytest.mark.asyncio
 class TestVersionRoutes:
     async def test_version_output(self, app: FastAPI, client: AsyncClient) -> None:
 
+        # controller_host is recorded on the mounted compute sub-application
+        # when the controller queries the compute capabilities. Set it
+        # directly so this test does not depend on another test having made
+        # that call first (it failed when run on its own).
+        for route in app.routes:
+            if isinstance(route, Mount) and route.name == "compute":
+                route.app.state.controller_host = "127.0.0.1"
+
         response = await client.get(app.url_path_for("get_version"))
         assert response.status_code == status.HTTP_200_OK
         assert response.json() == {"controller_host": "127.0.0.1", "local": False, "version": __version__}