fix: cleanup stale custom configs with apisix start#9701
Conversation
Signed-off-by: revolyssup <[email protected]>
|
Please make the ci pass |
Signed-off-by: revolyssup <[email protected]>
juststillthinking
added
the
wait for update
Signed-off-by: revolyssup <[email protected]>
|
@monkeyDluffy6017 please approve ci |
lingsamuel
removed
the
wait for update
|
Could you try to add a test case like this? |
Okay I'll add similar tests cases here. |
Signed-off-by: revolyssup <[email protected]>
|
@monkeyDluffy6017 I added the tests. Please approve CI |
Signed-off-by: revolyssup <[email protected]>
Signed-off-by: revolyssup <[email protected]>
Signed-off-by: revolyssup <[email protected]>
Signed-off-by: revolyssup <[email protected]>
| end | ||
| end | ||
| end | ||
|
|
There was a problem hiding this comment.
@monkeyDluffy6017 Removed redundant lines. Please approve CI again.
| # remove stale conf server sock | ||
| touch conf/config_listen.sock | ||
| ./bin/apisix start | ||
| sleep 0.5 |
There was a problem hiding this comment.
Why do you sleep so long?
There was a problem hiding this comment.
The tests were failing intermittently on CI as well as my machine. They passed only after giving a long enough sleep between start and stop.
There was a problem hiding this comment.
I don't think it takes that long, our ci test is too slow now, please reduce the time
There was a problem hiding this comment.
Okay Im bringing it to 2.5 but I have seen this test fail a lot due to less sleep time so I hope 2.5 is not too slow, not too fast.
Signed-off-by: revolyssup <[email protected]>
|
update the CI help us to check the codeStyle @Sn0rt |
Signed-off-by: Ashish Tiwari <[email protected]>
Signed-off-by: Ashish Tiwari <[email protected]>
Signed-off-by: Ashish Tiwari <[email protected]>
|
LGTM @monkeyDluffy6017 pls help me to review thx |
| echo "pass: no corresponding process" | ||
| sleep 0.5 | ||
|
|
||
| # check running when run repeatedly |
There was a problem hiding this comment.
why do you remove this comment?
There was a problem hiding this comment.
Was removed by mistake. Added it back
Signed-off-by: revolyssup <[email protected]>
| else | ||
| -- Cleanup any custom config from previous runs | ||
| cleanup() |
There was a problem hiding this comment.
I think we should not do this in every start, try to catch the error info of util.execute_cmd(env.openresty_args) and do the cleanup
There was a problem hiding this comment.
You mean to check the error of util.execute_cmd(env.openresty_args) and if the error is related to cleanup then cleanup and run util.execute_cmd(env.openresty_args) again?
There was a problem hiding this comment.
No need to run again, call the function cleanup()
There was a problem hiding this comment.
@monkeyDluffy6017 This is an initialization error and doesn't come from the openresty process. So I think we should check the error of init(env) and do the cleanup in case of error. What do you think?
|
|
||
| make stop | ||
| echo "pass: check APISIX running" | ||
|
|
There was a problem hiding this comment.
Why do you remove this blank, the below is another test case
juststillthinking
added
wait for update
Signed-off-by: revolyssup <[email protected]>
Signed-off-by: revolyssup <[email protected]>
Signed-off-by: revolyssup <[email protected]>
Signed-off-by: revolyssup <[email protected]>
Signed-off-by: revolyssup <[email protected]>
Signed-off-by: revolyssup <[email protected]>
Signed-off-by: Ashish Tiwari <[email protected]>
Signed-off-by: Ashish Tiwari <[email protected]>
Signed-off-by: revolyssup <[email protected]>
Signed-off-by: revolyssup <[email protected]>
Signed-off-by: revolyssup <[email protected]>
Signed-off-by: revolyssup <[email protected]>
Signed-off-by: revolyssup <[email protected]>
|
I will close this pr because it is fixed by #9846 |
4 participants
Description
Fixes #9700
Checklist