Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(ops): check process running with posix.signal insteadof lsof #7006

Merged
merged 15 commits into from
May 20, 2022
Merged

feat(ops): check process running with posix.signal insteadof lsof #7006

merged 15 commits into from
May 20, 2022

Conversation

kwanhur
Copy link
Contributor

@kwanhur kwanhur commented May 8, 2022 •
edited
Loading

Description

Partial OS no lsof tool, so if use lsof to check APISIX running or not will fail.

Now send signal none(0) to check via luaposix.

  • remove shell command lsof dependency
  • use signal SIGNONE(0) check process running or not

Fixes #6977

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

@kwanhur kwanhur dismissed a stale review via e897939 May 8, 2022 13:04
@kwanhur
feat(ops): check process running with posix.signal insteadof lsof
9bab5b9 
* remove shell command lsof dependency resolve apache#6977
* use signal SIGNONE(0) check process running or not

Signed-off-by: kwanhur <huang_hua2012@163.com>
@kwanhur kwanhur marked this pull request as ready for review May 8, 2022 14:20
@kwanhur
test(ops): cases check apisix running
21b9bc3 
Signed-off-by: kwanhur <huang_hua2012@163.com>
tzssangglass
tzssangglass reviewed May 9, 2022
View reviewed changes
apisix/cli/ops.lua Outdated Show resolved Hide resolved
tzssangglass
tzssangglass reviewed May 9, 2022
View reviewed changes
apisix/cli/ops.lua Outdated Show resolved Hide resolved
tokers
tokers previously approved these changes May 10, 2022
View reviewed changes
spacewander
spacewander reviewed May 10, 2022
View reviewed changes
apisix/cli/ops.lua Outdated Show resolved Hide resolved
t/cli/test_ops.sh Outdated Show resolved Hide resolved
@kwanhur
Revert "test(ops): cases check apisix running"
1de9ef3 
This reverts commit 21b9bc3.

Signed-off-by: kwanhur <huang_hua2012@163.com>
@kwanhur
feat(ops): use posix.errno constant ESRCH
e4ecd65 
Signed-off-by: kwanhur <huang_hua2012@163.com>
spacewander
spacewander reviewed May 12, 2022
View reviewed changes
apisix/cli/ops.lua Show resolved Hide resolved
apisix/cli/ops.lua Outdated Show resolved Hide resolved
apisix/cli/ops.lua Show resolved Hide resolved
@kwanhur
feat(ops): remove redundant blank line
f9298e1 
Signed-off-by: kwanhur <huang_hua2012@163.com>
@kwanhur
test(ops): update fake pid 66666 to check running or not
c8ee3bb 
Signed-off-by: kwanhur <huang_hua2012@163.com>
@kwanhur
Revert "test(ops): update fake pid 66666 to check running or not"
41364ed 
This reverts commit c8ee3bb.

Signed-off-by: kwanhur <huang_hua2012@163.com>
@kwanhur
feat(ops): return invalid when pid negative
b58dbf9 
Signed-off-by: kwanhur <huang_hua2012@163.com>
tzssangglass
tzssangglass previously approved these changes May 14, 2022
View reviewed changes
@kwanhur
Copy link
Contributor Author

kwanhur commented May 14, 2022

Any test case need to cover no such process? It seems no similar in test_cli_main.sh.

@kwanhur
feat(ops): return invalid when pid zero
055f5bc 
Signed-off-by: kwanhur <huang_hua2012@163.com>
@spacewander
Copy link
Member

Please make the CI pass, thanks!

@kwanhur
test(ops): check running when run repeatedly
3e8c4d5 
Signed-off-by: kwanhur <huang_hua2012@163.com>
@kwanhur
test(ops): remove unused test file
d1801ca 
Signed-off-by: kwanhur <huang_hua2012@163.com>
@kwanhur
Copy link
Contributor Author

kwanhur commented May 15, 2022

CI action run failed on t/plugin/traffic-split.t 6441580676 and t/plugin/api-breaker.t 6441580676 6441580789

Plz trigger to rerun them :-)

@spacewander
Copy link
Member

Let's merge master to make CI pass.

@kwanhur
Copy link
Contributor Author

kwanhur commented May 16, 2022

Had merged. Plz rerun this action.

cc @spacewander @tokers @tzssangglass

@tokers
Copy link
Contributor

tokers commented May 17, 2022

@kwanhur Please make the CI pass, thanks!

@tzssangglass
Copy link
Member

Had merged. Plz rerun this action.

done

@kwanhur
test(ops): case operation not permitted
bd64795 
Signed-off-by: kwanhur <huang_hua2012@163.com>
spacewander
spacewander reviewed May 19, 2022
View reviewed changes
t/cli/test_main.sh Outdated
@@ -706,6 +706,26 @@ fi
./bin/apisix stop
echo "pass: ignore stale nginx.pid"

# check operation not permitted
sudo make run
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we replace this test with a test that checks no such process? It seems we don't have test to cover it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added.

@kwanhur
test(ops): case no corresponding process
772d395 
Signed-off-by: kwanhur <huang_hua2012@163.com>
@kwanhur
Copy link
Contributor Author

kwanhur commented May 19, 2022

These failed workflows take error on compilation, plz trigger to re-run them, thanks very much. @tzssangglass

../ngx_lua-0.10.20/src/ngx_http_lua_common.h:26:10: fatal error: ngx_meta_lua_api.h: No such file or directory
 #include "ngx_meta_lua_api.h"
          ^~~~~~~~~~~~~~~~~~~~
compilation terminated.

spacewander
spacewander previously approved these changes May 20, 2022
View reviewed changes
t/cli/test_main.sh Outdated Show resolved Hide resolved
@spacewander spacewander merged commit 1a6f1b8 into apache:master May 20, 2022
Liu-Junlin pushed a commit to Liu-Junlin/apisix that referenced this pull request May 20, 2022
@kwanhur @spacewander @Liu-Junlin
feat(ops): check process running with posix.signal insteadof lsof (ap…
5252f32 
…ache#7006)

Signed-off-by: kwanhur <huang_hua2012@163.com>
Co-authored-by: 罗泽轩 <spacewanderlzx@gmail.com>
hongbinhsu pushed a commit to fitphp/apix that referenced this pull request May 24, 2022
Merge branch 'upsgrteam/master' into github/master
70f0ff9 
* upsgrteam/master: (351 commits)
  fix(proxy-cache): bypass when method mismatch cache_method (apache#7111)
  chore(script): support to install dependencies under arm64 (apache#7091)
  chore(ci): use the latest build script for apisix-base (apache#7090)
  fix(batch-requests): ignore "unix:" in the configuration (apache#7106)
  fix: install dependencies issues (apache#7092)
  feat(ops): use lua libs to backup config file insteadof shell command (apache#7048)
  test: reduce CI failure caused by flaky tests (apache#7085)
  chore(ci): move set_dns.sh to ci dir (apache#7089)
  feat: release 2.14.0 (apache#7057)
  docs: update "Tracers" Plugins (apache#7086)
  docs: update "Traffic" Plugin docs 3 (apache#7064)
  docs: update "Serverless" Plugins (apache#7076)
  feat(ops): check process running with posix.signal insteadof lsof (apache#7006)
  docs: modify how-to-build filename (apache#7087)
  docs: fix link of hot-reload in docs (apache#7081)
  chore(ci): apt update before install (apache#7080)
  docs: add pubsub develop example for kafka (apache#7059)
  ci: enable rebase in some situation (apache#7074)
  fix: redirect http to https but port not change (apache#7065)
  ci: make it pass under OpenResty 1.21 (apache#7067)
  ...
spacewander added a commit that referenced this pull request Jun 30, 2022
@kwanhur @spacewander
feat(ops): check process running with posix.signal insteadof lsof (#7006
eea2b7f 
)

Signed-off-by: kwanhur <huang_hua2012@163.com>
Co-authored-by: 罗泽轩 <spacewanderlzx@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@spacewander spacewander spacewander approved these changes

@tzssangglass tzssangglass tzssangglass approved these changes

@tokers tokers tokers left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

help request: lsof command not found while launching
4 participants
@kwanhur @spacewander @tokers @tzssangglass