From f2f118df9e29bc639462a01bf90c27aab2b3b45f Mon Sep 17 00:00:00 2001 From: Edward Oakes Date: Thu, 7 May 2020 16:45:20 -0500 Subject: [PATCH] [serve] Clear serve cluster state between tests. (#8357) --- python/ray/serve/master.py | 21 ++++++++++----------- python/ray/serve/tests/conftest.py | 14 +++++++++++++- python/ray/serve/tests/test_api.py | 10 +++++----- 3 files changed, 28 insertions(+), 17 deletions(-) diff --git a/python/ray/serve/master.py b/python/ray/serve/master.py index a33871989..66e66d489 100644 --- a/python/ray/serve/master.py +++ b/python/ray/serve/master.py @@ -416,6 +416,10 @@ class ServeMaster: """Fetched by the router on startup.""" return self.workers + def get_all_backends(self): + """Used for validation by the API client.""" + return list(self.backends.keys()) + def get_all_endpoints(self): """Used for validation by the API client.""" return [endpoint for endpoint, methods in self.routes.values()] @@ -501,26 +505,21 @@ class ServeMaster: async with self.write_lock: # This method must be idempotent. We should validate that the # specified endpoint exists on the client. - if endpoint not in self.traffic_policies: - logger.info("Endpoint '{}' doesn't exist".format(endpoint)) - return - - # Remove the traffic policy entry. - del self.traffic_policies[endpoint] - for route, (route_endpoint, _) in self.routes.items(): if route_endpoint == endpoint: route_to_delete = route break else: - # This should never happen, we either add to or delete from - # both self.traffic_policies and self.routes. - assert False, "No route found for endpoint '{}'".format( - endpoint) + logger.info("Endpoint '{}' doesn't exist".format(endpoint)) + return # Remove the routing entry. del self.routes[route_to_delete] + # Remove the traffic policy entry if it exists. + if endpoint in self.traffic_policies: + del self.traffic_policies[endpoint] + self.endpoints_to_remove.append(endpoint) # NOTE(edoakes): we must write a checkpoint before pushing the diff --git a/python/ray/serve/tests/conftest.py b/python/ray/serve/tests/conftest.py index 0d8f19811..8e3048868 100644 --- a/python/ray/serve/tests/conftest.py +++ b/python/ray/serve/tests/conftest.py @@ -3,12 +3,24 @@ import os import pytest from ray import serve +from ray.serve.utils import retry_actor_failures if os.environ.get("RAY_SERVE_INTENTIONALLY_CRASH", False): serve.master._CRASH_AFTER_CHECKPOINT_PROBABILITY = 0.5 @pytest.fixture(scope="session") -def serve_instance(): +def _shared_serve_instance(): serve.init(blocking=True, ray_init_kwargs={"num_cpus": 36}) yield + + +@pytest.fixture +def serve_instance(_shared_serve_instance): + yield + master = serve.api._get_master_actor() + # Clear all state between tests to avoid naming collisions. + for endpoint in retry_actor_failures(master.get_all_endpoints): + serve.delete_endpoint(endpoint) + for backend in retry_actor_failures(master.get_all_backends): + serve.delete_backend(backend) diff --git a/python/ray/serve/tests/test_api.py b/python/ray/serve/tests/test_api.py index 43171373c..913012ebc 100644 --- a/python/ray/serve/tests/test_api.py +++ b/python/ray/serve/tests/test_api.py @@ -40,24 +40,24 @@ def test_e2e(serve_instance): def test_call_method(serve_instance): - serve.create_endpoint("call-method", "/call-method") + serve.create_endpoint("endpoint", "/api") class CallMethod: def method(self, request): return "hello" - serve.create_backend("call-method", CallMethod) - serve.set_traffic("call-method", {"call-method": 1.0}) + serve.create_backend("backend", CallMethod) + serve.set_traffic("endpoint", {"backend": 1.0}) # Test HTTP path. resp = requests.get( - "http://127.0.0.1:8000/call-method", + "http://127.0.0.1:8000/api", timeout=1, headers={"X-SERVE-CALL-METHOD": "method"}) assert resp.text == "hello" # Test serve handle path. - handle = serve.get_handle("call-method") + handle = serve.get_handle("endpoint") assert ray.get(handle.options("method").remote()) == "hello"