From 63611a2a7ae16ad551e7006fb63460ea42f92faa Mon Sep 17 00:00:00 2001 From: landscapepainter Date: Mon, 12 Aug 2024 00:42:10 +0000 Subject: [PATCH 1/8] fix test_smoke.py::test_skyserve_new_autoscaler_update --- ...fter.yaml => new_autoscaler_after.yaml.j2} | 4 +- ...ore.yaml => new_autoscaler_before.yaml.j2} | 4 +- tests/test_smoke.py | 76 ++++++++++++------- 3 files changed, 53 insertions(+), 31 deletions(-) rename tests/skyserve/update/{new_autoscaler_after.yaml => new_autoscaler_after.yaml.j2} (89%) rename tests/skyserve/update/{new_autoscaler_before.yaml => new_autoscaler_before.yaml.j2} (72%) diff --git a/tests/skyserve/update/new_autoscaler_after.yaml b/tests/skyserve/update/new_autoscaler_after.yaml.j2 similarity index 89% rename from tests/skyserve/update/new_autoscaler_after.yaml rename to tests/skyserve/update/new_autoscaler_after.yaml.j2 index f5a2e552f67..0a6ce2f9935 100644 --- a/tests/skyserve/update/new_autoscaler_after.yaml +++ b/tests/skyserve/update/new_autoscaler_after.yaml.j2 @@ -8,7 +8,7 @@ service: base_ondemand_fallback_replicas: 1 resources: - cloud: gcp + cloud: {{generic_cloud}} ports: 8081 use_spot: true cpus: 2+ @@ -22,4 +22,4 @@ run: | # blue-green update. sleep 120 fi - python3 server.py + python3 server.py --port 8081 diff --git a/tests/skyserve/update/new_autoscaler_before.yaml b/tests/skyserve/update/new_autoscaler_before.yaml.j2 similarity index 72% rename from tests/skyserve/update/new_autoscaler_before.yaml rename to tests/skyserve/update/new_autoscaler_before.yaml.j2 index a91c3cd230a..531c1c2664c 100644 --- a/tests/skyserve/update/new_autoscaler_before.yaml +++ b/tests/skyserve/update/new_autoscaler_before.yaml.j2 @@ -5,10 +5,10 @@ service: replicas: 2 resources: - cloud: gcp + cloud: {{generic_cloud}} ports: 8081 cpus: 2+ workdir: examples/serve/http_server -run: python3 server.py +run: python3 server.py --port 8081 diff --git a/tests/test_smoke.py b/tests/test_smoke.py index 33280e1fd4c..d7566738859 100644 --- a/tests/test_smoke.py +++ b/tests/test_smoke.py @@ -3981,33 +3981,55 @@ def test_skyserve_new_autoscaler_update(mode: str, generic_cloud: str): (2, False, 'READY')]) + _check_service_version(name, "1"), ] - test = Test( - 'test-skyserve-new-autoscaler-update', - [ - f'sky serve up -n {name} --cloud {generic_cloud} -y tests/skyserve/update/new_autoscaler_before.yaml', - _SERVE_WAIT_UNTIL_READY.format(name=name, replica_num=2) + - _check_service_version(name, "1"), - f'{_SERVE_ENDPOINT_WAIT.format(name=name)}; ' - 's=$(curl http://$endpoint); echo "$s"; echo "$s" | grep "Hi, SkyPilot here"', - f'sky serve update {name} --cloud {generic_cloud} --mode {mode} -y tests/skyserve/update/new_autoscaler_after.yaml', - # Wait for update to be registered - f'sleep 90', - wait_until_no_pending, - _check_replica_in_status( - name, [(4, True, _SERVICE_LAUNCHING_STATUS_REGEX + '\|READY'), - (1, False, _SERVICE_LAUNCHING_STATUS_REGEX), - (2, False, 'READY')]), - *update_check, - _SERVE_WAIT_UNTIL_READY.format(name=name, replica_num=5), - f'{_SERVE_ENDPOINT_WAIT.format(name=name)}; ' - 'curl http://$endpoint | grep "Hi, SkyPilot here"', - _check_replica_in_status(name, [(4, True, 'READY'), - (1, False, 'READY')]), - ], - _TEARDOWN_SERVICE.format(name=name), - timeout=20 * 60, - ) - run_one_test(test) + + before_template_str = pathlib.Path( + 'tests/skyserve/update/new_autoscaler_before.yaml.j2').read_text() + after_template_str = pathlib.Path( + 'tests/skyserve/update/new_autoscaler_after.yaml.j2').read_text() + before_template = jinja2.Template(before_template_str) + after_template = jinja2.Template(after_template_str) + before_content = before_template.render(generic_cloud=generic_cloud) + after_content = after_template.render(generic_cloud=generic_cloud) + with tempfile.NamedTemporaryFile( + suffix='.yaml', + mode='w' + ) as before_file, tempfile.NamedTemporaryFile( + suffix='.yaml', + mode='w' + ) as after_file: + before_file.write(before_content) + before_file.flush() + before_file_path = before_file.name + after_file.write(after_content) + after_file.flush() + after_file_path = after_file.name + test = Test( + 'test-skyserve-new-autoscaler-update', + [ + f'sky serve up -n {name} --cloud {generic_cloud} -y {before_file_path}', + _SERVE_WAIT_UNTIL_READY.format(name=name, replica_num=2) + + _check_service_version(name, "1"), + f'{_SERVE_ENDPOINT_WAIT.format(name=name)}; ' + 's=$(curl http://$endpoint); echo "$s"; echo "$s" | grep "Hi, SkyPilot here"', + f'sky serve update {name} --cloud {generic_cloud} --mode {mode} -y {after_file_path}', + # Wait for update to be registered + f'sleep 90', + wait_until_no_pending, + _check_replica_in_status( + name, [(4, True, _SERVICE_LAUNCHING_STATUS_REGEX + '\|READY'), + (1, False, _SERVICE_LAUNCHING_STATUS_REGEX), + (2, False, 'READY')]), + *update_check, + _SERVE_WAIT_UNTIL_READY.format(name=name, replica_num=5), + f'{_SERVE_ENDPOINT_WAIT.format(name=name)}; ' + 'curl http://$endpoint | grep "Hi, SkyPilot here"', + _check_replica_in_status(name, [(4, True, 'READY'), + (1, False, 'READY')]), + ], + _TEARDOWN_SERVICE.format(name=name), + timeout=20 * 60, + ) + run_one_test(test) @pytest.mark.serve From 5fe603a8897dc140a59760380cea798aa6eeb4ab Mon Sep 17 00:00:00 2001 From: landscapepainter Date: Mon, 12 Aug 2024 01:23:19 +0000 Subject: [PATCH 2/8] nit --- tests/test_smoke.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_smoke.py b/tests/test_smoke.py index d7566738859..16a4b1e4cb4 100644 --- a/tests/test_smoke.py +++ b/tests/test_smoke.py @@ -3952,7 +3952,7 @@ def test_skyserve_update_autoscale(generic_cloud: str): @pytest.mark.parametrize('mode', ['rolling', 'blue_green']) def test_skyserve_new_autoscaler_update(mode: str, generic_cloud: str): """Test skyserve with update that changes autoscaler""" - name = _get_service_name() + mode + name = f'{_get_service_name()}-{mode}' wait_until_no_pending = ( f's=$(sky serve status {name}); echo "$s"; ' From 93fbd388542a1d34c6897e2810230972d9900b25 Mon Sep 17 00:00:00 2001 From: landscapepainter Date: Mon, 12 Aug 2024 02:11:58 +0000 Subject: [PATCH 3/8] format --- tests/test_smoke.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/tests/test_smoke.py b/tests/test_smoke.py index 16a4b1e4cb4..9acf15185cf 100644 --- a/tests/test_smoke.py +++ b/tests/test_smoke.py @@ -3991,12 +3991,9 @@ def test_skyserve_new_autoscaler_update(mode: str, generic_cloud: str): before_content = before_template.render(generic_cloud=generic_cloud) after_content = after_template.render(generic_cloud=generic_cloud) with tempfile.NamedTemporaryFile( - suffix='.yaml', - mode='w' - ) as before_file, tempfile.NamedTemporaryFile( - suffix='.yaml', - mode='w' - ) as after_file: + suffix='.yaml', + mode='w') as before_file, tempfile.NamedTemporaryFile( + suffix='.yaml', mode='w') as after_file: before_file.write(before_content) before_file.flush() before_file_path = before_file.name @@ -4015,10 +4012,11 @@ def test_skyserve_new_autoscaler_update(mode: str, generic_cloud: str): # Wait for update to be registered f'sleep 90', wait_until_no_pending, - _check_replica_in_status( - name, [(4, True, _SERVICE_LAUNCHING_STATUS_REGEX + '\|READY'), - (1, False, _SERVICE_LAUNCHING_STATUS_REGEX), - (2, False, 'READY')]), + _check_replica_in_status(name, [ + (4, True, _SERVICE_LAUNCHING_STATUS_REGEX + '\|READY'), + (1, False, _SERVICE_LAUNCHING_STATUS_REGEX), + (2, False, 'READY') + ]), *update_check, _SERVE_WAIT_UNTIL_READY.format(name=name, replica_num=5), f'{_SERVE_ENDPOINT_WAIT.format(name=name)}; ' From eed43e4b332212770021c52e2ca7e61516457383 Mon Sep 17 00:00:00 2001 From: landscapepainter Date: Thu, 15 Aug 2024 03:25:47 +0000 Subject: [PATCH 4/8] move out the comment from run at new_autoscaler_after.yaml --- tests/skyserve/update/new_autoscaler_after.yaml.j2 | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/skyserve/update/new_autoscaler_after.yaml.j2 b/tests/skyserve/update/new_autoscaler_after.yaml.j2 index 0a6ce2f9935..792a1cdd032 100644 --- a/tests/skyserve/update/new_autoscaler_after.yaml.j2 +++ b/tests/skyserve/update/new_autoscaler_after.yaml.j2 @@ -15,11 +15,11 @@ resources: workdir: examples/serve/http_server +# Sleep for the last replica in the test_skyserve_new_autoscaler_update +# so that we can check the behavior difference between rolling and +# blue-green update. run: | if [ $SKYPILOT_SERVE_REPLICA_ID -eq 7 ]; then - # Sleep for the last replica in the test_skyserve_new_autoscaler_update - # so that we can check the behavior difference between rolling and - # blue-green update. sleep 120 fi python3 server.py --port 8081 From 58f34e3db381fc3cefcd9ffaabdeb607c07971a8 Mon Sep 17 00:00:00 2001 From: landscapepainter Date: Thu, 15 Aug 2024 03:44:51 +0000 Subject: [PATCH 5/8] revert comment --- tests/skyserve/update/new_autoscaler_after.yaml.j2 | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/skyserve/update/new_autoscaler_after.yaml.j2 b/tests/skyserve/update/new_autoscaler_after.yaml.j2 index 792a1cdd032..0a6ce2f9935 100644 --- a/tests/skyserve/update/new_autoscaler_after.yaml.j2 +++ b/tests/skyserve/update/new_autoscaler_after.yaml.j2 @@ -15,11 +15,11 @@ resources: workdir: examples/serve/http_server -# Sleep for the last replica in the test_skyserve_new_autoscaler_update -# so that we can check the behavior difference between rolling and -# blue-green update. run: | if [ $SKYPILOT_SERVE_REPLICA_ID -eq 7 ]; then + # Sleep for the last replica in the test_skyserve_new_autoscaler_update + # so that we can check the behavior difference between rolling and + # blue-green update. sleep 120 fi python3 server.py --port 8081 From 1789432266f54f67de4b5d21176086481adfafbf Mon Sep 17 00:00:00 2001 From: landscapepainter Date: Sat, 17 Aug 2024 00:39:45 +0000 Subject: [PATCH 6/8] revert templating --- ...fter.yaml.j2 => new_autoscaler_after.yaml} | 1 - ...ore.yaml.j2 => new_autoscaler_before.yaml} | 1 - tests/test_smoke.py | 73 +++++++------------ 3 files changed, 27 insertions(+), 48 deletions(-) rename tests/skyserve/update/{new_autoscaler_after.yaml.j2 => new_autoscaler_after.yaml} (95%) rename tests/skyserve/update/{new_autoscaler_before.yaml.j2 => new_autoscaler_before.yaml} (88%) diff --git a/tests/skyserve/update/new_autoscaler_after.yaml.j2 b/tests/skyserve/update/new_autoscaler_after.yaml similarity index 95% rename from tests/skyserve/update/new_autoscaler_after.yaml.j2 rename to tests/skyserve/update/new_autoscaler_after.yaml index 0a6ce2f9935..2d12d3ef109 100644 --- a/tests/skyserve/update/new_autoscaler_after.yaml.j2 +++ b/tests/skyserve/update/new_autoscaler_after.yaml @@ -8,7 +8,6 @@ service: base_ondemand_fallback_replicas: 1 resources: - cloud: {{generic_cloud}} ports: 8081 use_spot: true cpus: 2+ diff --git a/tests/skyserve/update/new_autoscaler_before.yaml.j2 b/tests/skyserve/update/new_autoscaler_before.yaml similarity index 88% rename from tests/skyserve/update/new_autoscaler_before.yaml.j2 rename to tests/skyserve/update/new_autoscaler_before.yaml index 531c1c2664c..793221080ae 100644 --- a/tests/skyserve/update/new_autoscaler_before.yaml.j2 +++ b/tests/skyserve/update/new_autoscaler_before.yaml @@ -5,7 +5,6 @@ service: replicas: 2 resources: - cloud: {{generic_cloud}} ports: 8081 cpus: 2+ diff --git a/tests/test_smoke.py b/tests/test_smoke.py index 9acf15185cf..6c979bfd4e9 100644 --- a/tests/test_smoke.py +++ b/tests/test_smoke.py @@ -3982,52 +3982,33 @@ def test_skyserve_new_autoscaler_update(mode: str, generic_cloud: str): _check_service_version(name, "1"), ] - before_template_str = pathlib.Path( - 'tests/skyserve/update/new_autoscaler_before.yaml.j2').read_text() - after_template_str = pathlib.Path( - 'tests/skyserve/update/new_autoscaler_after.yaml.j2').read_text() - before_template = jinja2.Template(before_template_str) - after_template = jinja2.Template(after_template_str) - before_content = before_template.render(generic_cloud=generic_cloud) - after_content = after_template.render(generic_cloud=generic_cloud) - with tempfile.NamedTemporaryFile( - suffix='.yaml', - mode='w') as before_file, tempfile.NamedTemporaryFile( - suffix='.yaml', mode='w') as after_file: - before_file.write(before_content) - before_file.flush() - before_file_path = before_file.name - after_file.write(after_content) - after_file.flush() - after_file_path = after_file.name - test = Test( - 'test-skyserve-new-autoscaler-update', - [ - f'sky serve up -n {name} --cloud {generic_cloud} -y {before_file_path}', - _SERVE_WAIT_UNTIL_READY.format(name=name, replica_num=2) + - _check_service_version(name, "1"), - f'{_SERVE_ENDPOINT_WAIT.format(name=name)}; ' - 's=$(curl http://$endpoint); echo "$s"; echo "$s" | grep "Hi, SkyPilot here"', - f'sky serve update {name} --cloud {generic_cloud} --mode {mode} -y {after_file_path}', - # Wait for update to be registered - f'sleep 90', - wait_until_no_pending, - _check_replica_in_status(name, [ - (4, True, _SERVICE_LAUNCHING_STATUS_REGEX + '\|READY'), - (1, False, _SERVICE_LAUNCHING_STATUS_REGEX), - (2, False, 'READY') - ]), - *update_check, - _SERVE_WAIT_UNTIL_READY.format(name=name, replica_num=5), - f'{_SERVE_ENDPOINT_WAIT.format(name=name)}; ' - 'curl http://$endpoint | grep "Hi, SkyPilot here"', - _check_replica_in_status(name, [(4, True, 'READY'), - (1, False, 'READY')]), - ], - _TEARDOWN_SERVICE.format(name=name), - timeout=20 * 60, - ) - run_one_test(test) + test = Test( + 'test-skyserve-new-autoscaler-update', + [ + f'sky serve up -n {name} --cloud {generic_cloud} -y tests/skyserve/update/new_autoscaler_before.yaml', + _SERVE_WAIT_UNTIL_READY.format(name=name, replica_num=2) + + _check_service_version(name, "1"), + f'{_SERVE_ENDPOINT_WAIT.format(name=name)}; ' + 's=$(curl http://$endpoint); echo "$s"; echo "$s" | grep "Hi, SkyPilot here"', + f'sky serve update {name} --cloud {generic_cloud} --mode {mode} -y tests/skyserve/update/new_autoscaler_after.yaml', + # Wait for update to be registered + f'sleep 90', + wait_until_no_pending, + _check_replica_in_status( + name, [(4, True, _SERVICE_LAUNCHING_STATUS_REGEX + '\|READY'), + (1, False, _SERVICE_LAUNCHING_STATUS_REGEX), + (2, False, 'READY')]), + *update_check, + _SERVE_WAIT_UNTIL_READY.format(name=name, replica_num=5), + f'{_SERVE_ENDPOINT_WAIT.format(name=name)}; ' + 'curl http://$endpoint | grep "Hi, SkyPilot here"', + _check_replica_in_status(name, [(4, True, 'READY'), + (1, False, 'READY')]), + ], + _TEARDOWN_SERVICE.format(name=name), + timeout=20 * 60, + ) + run_one_test(test) @pytest.mark.serve From 8bc1c409c3113fa3ca6d54440d7537dc8c07077b Mon Sep 17 00:00:00 2001 From: landscapepainter Date: Sat, 17 Aug 2024 00:44:03 +0000 Subject: [PATCH 7/8] nit --- tests/test_smoke.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_smoke.py b/tests/test_smoke.py index 6c979bfd4e9..beab0622838 100644 --- a/tests/test_smoke.py +++ b/tests/test_smoke.py @@ -3981,7 +3981,6 @@ def test_skyserve_new_autoscaler_update(mode: str, generic_cloud: str): (2, False, 'READY')]) + _check_service_version(name, "1"), ] - test = Test( 'test-skyserve-new-autoscaler-update', [ From ad1f941412dac633f2403385d4a945d01e9eab74 Mon Sep 17 00:00:00 2001 From: landscapepainter Date: Sat, 17 Aug 2024 00:57:44 +0000 Subject: [PATCH 8/8] nit --- tests/test_smoke.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_smoke.py b/tests/test_smoke.py index beab0622838..874e51e0f9c 100644 --- a/tests/test_smoke.py +++ b/tests/test_smoke.py @@ -3982,7 +3982,7 @@ def test_skyserve_new_autoscaler_update(mode: str, generic_cloud: str): _check_service_version(name, "1"), ] test = Test( - 'test-skyserve-new-autoscaler-update', + f'test-skyserve-new-autoscaler-update-{mode}', [ f'sky serve up -n {name} --cloud {generic_cloud} -y tests/skyserve/update/new_autoscaler_before.yaml', _SERVE_WAIT_UNTIL_READY.format(name=name, replica_num=2) +