Serve log before termination for smoke tests #4691
Conversation
zpoint
force-pushed
the
dev/zeping/serve_log_before_termination
branch
from
42cbf05 to
23670b2
Compare
|
/smoke-test --kubernetes |
tests/smoke_tests/test_sky_serve.py
Outdated
| # "Services" section), increment a counter. Skip the first line (header) and | ||
| # print the first column (service name) for subsequent lines. | ||
| _SHOW_SERVE_STATUS = ( | ||
| 'sky status --refresh | awk \'' |
There was a problem hiding this comment.
why not use sky serve status and why we use --refresh?
There was a problem hiding this comment.
Updated with sky serve status
tests/smoke_tests/test_sky_serve.py
Outdated
| _SHOW_SERVE_STATUS = ( | ||
| 'sky serve status | awk \'' | ||
| '/^Services$/{{flag=1; next}} ' | ||
| '/^$/{{flag=0}} ' | ||
| 'flag {{if (++count > 1) print $1}}\' | ' | ||
| 'while read -r service_name; do ' | ||
| ' echo "+ sky serve logs --controller $service_name --no-follow"; ' | ||
| ' sky serve logs --controller $service_name --no-follow; ' | ||
| ' echo "+ sky serve logs --load-balancer $service_name --no-follow"; ' | ||
| ' sky serve logs --load-balancer $service_name --no-follow; ' | ||
| 'done; ') |
There was a problem hiding this comment.
This will show the logs for all services? What if there are multiple tests running for skyserve? Wouldn't this mess up the output of the test with outputs from other tests?
Also, would any failure of these commands cause leakage replica resources as service termination commands may not be called afterwards.
There was a problem hiding this comment.
Now pass the {name} to display specific services only.
The command format is cmdA; cmdB; cmdC; .... If cmdA fails, cmdB will still execute — that's how bash runs commands.
|
/smoke-test --managed-jobs |
|
/smoke-test --k test_multi_tenant |
|
/smoke-test -k test_multi_tenant |
1 similar comment
|
/smoke-test -k test_multi_tenant |
|
/smoke-test --kubernetes -k test_managed_jobs_basic |
|
@romilbhardwaj Could u help review? |
| @@ -69,12 +69,19 @@ def _get_service_name() -> str: | |||
| _IP_REGEX = r'([0-9]{1,3}\.){3}[0-9]{1,3}' | |||
| _AWK_ALL_LINES_BELOW_REPLICAS = r'/Replicas/{flag=1; next} flag' | |||
| _SERVICE_LAUNCHING_STATUS_REGEX = 'PROVISIONING\|STARTING' | |||
|
|
|||
| _SHOW_SERVE_STATUS = ( | |||
There was a problem hiding this comment.
Can we add a quick sky serve status before the logs commands?
|
@romilbhardwaj Could you help take another look? |
| @@ -69,12 +69,21 @@ def _get_service_name() -> str: | |||
| _IP_REGEX = r'([0-9]{1,3}\.){3}[0-9]{1,3}' | |||
| _AWK_ALL_LINES_BELOW_REPLICAS = r'/Replicas/{flag=1; next} flag' | |||
| _SERVICE_LAUNCHING_STATUS_REGEX = 'PROVISIONING\|STARTING' | |||
|
|
|||
| _SHOW_SERVE_STATUS = ( | |||
| 'echo "+ sky serve status {name}"; ' | |||
There was a problem hiding this comment.
Are these echo statements necessary? I thought the command being executed was automatically echo'd by our tests
There was a problem hiding this comment.
Our tests didn’t echo the teardown command, and I think it’s better to keep the lower granularity, like the current sky serve status {name}, rather than something like echo "_AWK....1000 characters other command; {sky serve status {name}}".
Tested (run the relevant ones):
bash format.shpytest tests/test_smoke.pypytest tests/test_smoke.py::test_fill_in_the_nameconda deactivate; bash -i tests/backward_compatibility_tests.sh