You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2021/10/04 01:52:40 UTC
[GitHub] [apisix] nic-chen opened a new pull request #5171: feat: enhance the verification of the configuration
nic-chen opened a new pull request #5171:
URL: https://github.com/apache/apisix/pull/5171
### What this PR does / why we need it:
1. add command `apisix test` to test configuration file
2. when run `apisix stop` or `apisix restart`, regenerate `nginx.conf` to avoid failure to restart
### Pre-submission checklist:
* [x] Did you explain what problem does this PR solve? Or what new features have been added?
* [x] Have you added corresponding test cases?
* [x] Have you modified the corresponding document?
* [x] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix/tree/master#community) first**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #5171: feat: enhance the verification of the configuration
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5171:
URL: https://github.com/apache/apisix/pull/5171#discussion_r723822935
##########
File path: apisix/cli/ops.lua
##########
@@ -891,6 +891,44 @@ local function cleanup()
end
+local function test(env, backup_ngx_conf)
+ -- backup nginx.conf
+ local ngx_conf_path = env.apisix_home .. "/conf/nginx.conf"
+ local ngx_conf_exist = io_open(ngx_conf_path)
+ if ngx_conf_exist then
+ local err = util.execute_cmd_with_error("mv " .. ngx_conf_path .. " "
Review comment:
We can use `os.rename`.
##########
File path: apisix/cli/ops.lua
##########
@@ -891,6 +891,44 @@ local function cleanup()
end
+local function test(env, backup_ngx_conf)
+ -- backup nginx.conf
+ local ngx_conf_path = env.apisix_home .. "/conf/nginx.conf"
+ local ngx_conf_exist = io_open(ngx_conf_path)
Review comment:
We can use `util.is_file_exist`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #5171: feat: enhance the verification of the configuration
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5171:
URL: https://github.com/apache/apisix/pull/5171#discussion_r721002609
##########
File path: apisix/cli/ops.lua
##########
@@ -901,6 +903,8 @@ end
local function stop(env)
cleanup()
+ -- reinit nginx.conf
+ init(env)
Review comment:
This behavior is a break change...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #5171: feat: enhance the verification of the configuration
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5171:
URL: https://github.com/apache/apisix/pull/5171#discussion_r721015329
##########
File path: apisix/cli/ops.lua
##########
@@ -932,6 +936,23 @@ local function reload(env)
end
+local function test(env)
+ -- reinit nginx.conf
+ init(env)
Review comment:
~~Better to backup/restore the current nginx.conf~~ it's ok
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #5171: feat: enhance the verification of the configuration
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5171:
URL: https://github.com/apache/apisix/pull/5171#discussion_r720998258
##########
File path: apisix/cli/ops.lua
##########
@@ -932,6 +936,23 @@ local function reload(env)
end
+local function test(env)
+ -- reinit nginx.conf
+ init(env)
+
+ local test_cmd = env.openresty_args .. [[ -t -q ]]
+ -- When success,
+ -- On linux, os.execute returns 0,
+ -- On macos, os.execute returns 3 values: true, exit, 0, and we need the first.
+ local test_ret = execute((test_cmd))
+ if (test_ret == 0 or test_ret == true) then
+ print("configuration test is successful")
+ return
+ end
+
+ print("configuration test failed")
Review comment:
Need to use `util.die`
##########
File path: t/cli/test_validate_config.sh
##########
@@ -89,3 +89,60 @@ fi
git checkout conf/config-default.yaml
echo "passed: allow configuring node_listen as a number in the default config"
+
+# apisix test
+git checkout conf/config.yaml
+
+out=$(./bin/apisix test 2>&1 || true)
+if ! echo "$out" | grep "configuration test is successful"; then
+ echo "failed: configuration test should be successful"
+ exit 1
+fi
+
+echo "pass: apisix test"
+
+echo '
+nginx_config:
+ main_configuration_snippet: |
+ notexist on;
+' > conf/config.yaml
+
+out=$(./bin/apisix test 2>&1 || true)
+if ! echo "$out" | grep "configuration test failed"; then
+ echo "failed: should test failed when configuration invalid"
+ exit 1
+fi
+
+echo "passed: apisix test(failure scenario)"
+
+# apisix stop and restart
+git checkout conf/config.yaml
+
+./bin/apisix start
+sleep 1 # wait for apisix starts
+
+echo '
+nginx_config:
+ main_configuration_snippet: |
+ notexist on;
+' > conf/config.yaml
+
+out=$(./bin/apisix stop 2>&1 || true)
+if ! echo "$out" | grep "\[emerg\] unknown directive \"notexist\""; then
+ echo "failed: should stop failed when configuration invalid"
+ exit 1
+fi
+
+echo "passed: apisix stop"
+
+
+out=$(./bin/apisix stop 2>&1 || true)
+if ! (echo "$out" | grep "\[emerg\] unknown directive \"notexist\"") && ! (echo "$out" | grep "APISIX is running"); then
+ echo "failed: should restart failed when configuration invalid"
+ exit 1
+fi
+
+echo "passed: apisix restart"
+
+git checkout conf/config.yaml
Review comment:
The configuration will be checked out after the test exits. We don't need to do it ourselves.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #5171: feat: enhance the verification of the configuration
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5171:
URL: https://github.com/apache/apisix/pull/5171#discussion_r721015329
##########
File path: apisix/cli/ops.lua
##########
@@ -932,6 +936,23 @@ local function reload(env)
end
+local function test(env)
+ -- reinit nginx.conf
+ init(env)
Review comment:
Better to backup/restore the current nginx.conf
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] nic-chen commented on pull request #5171: feat: enhance the verification of the configuration
Posted by GitBox <gi...@apache.org>.
nic-chen commented on pull request #5171:
URL: https://github.com/apache/apisix/pull/5171#issuecomment-933299870
@spacewander updated. please review again when you have time. thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #5171: feat: enhance the verification of the configuration
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5171:
URL: https://github.com/apache/apisix/pull/5171#discussion_r723822935
##########
File path: apisix/cli/ops.lua
##########
@@ -891,6 +891,44 @@ local function cleanup()
end
+local function test(env, backup_ngx_conf)
+ -- backup nginx.conf
+ local ngx_conf_path = env.apisix_home .. "/conf/nginx.conf"
+ local ngx_conf_exist = io_open(ngx_conf_path)
+ if ngx_conf_exist then
+ local err = util.execute_cmd_with_error("mv " .. ngx_conf_path .. " "
Review comment:
We can use `os.rename`.
##########
File path: apisix/cli/ops.lua
##########
@@ -891,6 +891,44 @@ local function cleanup()
end
+local function test(env, backup_ngx_conf)
+ -- backup nginx.conf
+ local ngx_conf_path = env.apisix_home .. "/conf/nginx.conf"
+ local ngx_conf_exist = io_open(ngx_conf_path)
Review comment:
We can use `util.is_file_exist`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander merged pull request #5171: feat: enhance the verification of the configuration
Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #5171:
URL: https://github.com/apache/apisix/pull/5171
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org