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 2020/10/12 07:05:49 UTC

[GitHub] [apisix] tokers opened a new pull request #2395: improve: refactor apisix command line tool

tokers opened a new pull request #2395:
URL: https://github.com/apache/apisix/pull/2395


   ### What this PR does / why we need it:
   
   This PR refactors the APISIX command line tool, splitting it into several files at `apisix/cmd` dir, which enhances the readability and maintainability.
   
   ### Pre-submission checklist:
   
   * [x] Did you explain what problem does this PR solve? Or what new features have been added?
   * [ ] Have you added corresponding test cases?
   * [x] Have you modified the corresponding document?
   * [x] Is this PR backward compatible?
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers commented on a change in pull request #2395: improve: refactor apisix command line tool

Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #2395:
URL: https://github.com/apache/apisix/pull/2395#discussion_r507353856



##########
File path: apisix/cmd/ops.lua
##########
@@ -0,0 +1,294 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local env = require "apisix.cmd.env"
+local etcd = require "apisix.cmd.etcd"
+local file = require "apisix.cmd.file"
+local util = require "apisix.cmd.util"
+local ngx_tpl = require "apisix.cmd.ngx_tpl"
+local template = require "resty.template"
+
+local type = type
+local pairs = pairs
+local print = print
+local ipairs = ipairs
+local tonumber = tonumber
+local tostring = tostring
+local str_find = string.find
+local str_sub = string.sub
+local max = math.max
+local popen = io.popen
+local execute = os.execute
+local error = error
+local stderr = io.stderr
+local openresty_args = [[openresty  -p ]] .. env.apisix_home .. [[ -c ]]
+                       .. env.apisix_home .. [[/conf/nginx.conf]]
+
+local _M = {}
+
+
+local function get_openresty_version()
+    local str = "nginx version: openresty/"
+    local ret = util.execute_cmd("openresty -v 2>&1")
+    local pos = str_find(ret, str)
+    if pos then
+        return str_sub(ret, pos + #str)
+    end
+
+    str = "nginx version: nginx/"
+    ret = util.execute_cmd("openresty -v 2>&1")
+    pos = str_find(ret, str)
+    if pos then
+        return str_sub(ret, pos + #str)
+    end
+
+    return nil
+end
+
+
+local function check_version(cur_ver_s, need_ver_s)
+    local cur_vers = util.split(cur_ver_s, [[.]])
+    local need_vers = util.split(need_ver_s, [[.]])
+    local len = max(#cur_vers, #need_vers)
+
+    for i = 1, len do
+        local cur_ver = tonumber(cur_vers[i]) or 0
+        local need_ver = tonumber(need_vers[i]) or 0
+        if cur_ver > need_ver then
+            return true
+        end
+
+        if cur_ver < need_ver then
+            return false
+        end
+    end
+
+    return true
+end
+
+
+function _M.init()
+    if env.is_root_path then
+        print("Warning! Running apisix under /root is only suitable for "
+              .. "development environments and it is dangerous to do so."
+              .. "It is recommended to run APISIX in a directory other than "
+              .. "/root.")
+    end
+
+    -- read_yaml_conf
+    local yaml_conf, err = file.read_yaml_conf()
+    if not yaml_conf then
+        error("failed to read local yaml config of apisix: " .. err)
+    end
+
+    -- check the Admin API token
+    local checked_admin_key = false
+    if yaml_conf.apisix.enable_admin and yaml_conf.apisix.allow_admin then
+        for _, allow_ip in ipairs(yaml_conf.apisix.allow_admin) do
+            if allow_ip == "127.0.0.0/24" then
+                checked_admin_key = true
+                break
+            end
+        end
+    end
+
+    if yaml_conf.apisix.enable_admin and not checked_admin_key then
+        local help = [[
+
+%s
+Please modify "admin_key" in conf/config.yaml .
+
+]]
+        if type(yaml_conf.apisix.admin_key) ~= "table" or
+           #yaml_conf.apisix.admin_key == 0
+        then
+            return util.die(help:format("ERROR: missing valid Admin API token."))
+        end
+
+        for _, admin in ipairs(yaml_conf.apisix.admin_key) do
+            if type(admin.key) == "table" then
+                admin.key = ""
+            else
+                admin.key = tostring(admin.key)
+            end
+
+            if admin.key == "" then
+                return util.die(help:format("ERROR: missing valid Admin API token."),
+                                "\n")
+            end
+
+            if admin.key == "edd1c9f034335f136f87ad84b625c8f1" then
+                local msg = help:format([[WARNING: using fixed Admin API token has security risk.]])
+                return util.die(msg, "\n")
+            end
+        end
+    end
+
+    local with_module_status = true
+
+    local or_ver = util.execute_cmd("openresty -V 2>&1")
+    if or_ver and not or_ver:find("http_stub_status_module", 1, true) then
+        stderr:write("'http_stub_status_module' module is missing in ",
+                     "your openresty, please check it out. Without this ",
+                     "module, there will be fewer monitoring indicators.\n")
+
+        with_module_status = false
+    end
+
+    local enabled_plugins = {}
+    for i, name in ipairs(yaml_conf.plugins) do
+        enabled_plugins[name] = true
+    end
+
+    if enabled_plugins["proxy-cache"] and not yaml_conf.apisix.proxy_cache then
+        error("missing apisix.proxy_cache for plugin proxy-cache")
+    end
+
+    -- Using template.render
+    local sys_conf = {
+        lua_path           = env.pkg_path_org,
+        lua_cpath          = env.pkg_cpath_org,
+        apisix_lua_home    = env.apisix_home,
+        os_name            = util.trim(util.execute_cmd("uname")),
+        with_module_status = with_module_status,
+        error_log          = { level = "warn" },
+        enabled_plugins    = enabled_plugins,
+    }
+
+    if not yaml_conf.apisix then
+        error("failed to read `apisix` field from yaml file")
+    end
+
+    if not yaml_conf.nginx_config then
+        error("failed to read `nginx_config` field from yaml file")
+    end
+
+    if util.is_32bit_arch() then
+        sys_conf["worker_rlimit_core"] = "4G"
+    else
+        sys_conf["worker_rlimit_core"] = "16G"
+    end
+
+    for k,v in pairs(yaml_conf.apisix) do
+        sys_conf[k] = v
+    end
+
+    for k,v in pairs(yaml_conf.nginx_config) do
+        sys_conf[k] = v
+    end
+
+    local wrn = sys_conf["worker_rlimit_nofile"]
+    local wc = sys_conf["event"]["worker_connections"]
+    if not wrn or wrn <= wc then
+        -- ensure the number of fds is slightly larger than the number of conn
+        sys_conf["worker_rlimit_nofile"] = wc + 128
+    end
+
+    if sys_conf["enable_dev_mode"] == true then
+        sys_conf["worker_processes"] = 1
+        sys_conf["enable_reuseport"] = false
+
+    elseif tonumber(sys_conf["worker_processes"]) == nil then
+        sys_conf["worker_processes"] = "auto"
+    end
+
+    local dns_resolver = sys_conf["dns_resolver"]
+    if not dns_resolver or #dns_resolver == 0 then
+        local dns_addrs, err = util.local_dns_resolver("/etc/resolv.conf")
+        if not dns_addrs then
+            return util.die("failed to import local DNS: ", err)
+        end
+
+        if #dns_addrs == 0 then
+            return util.die("local DNS is empty")
+        end
+
+        sys_conf["dns_resolver"] = dns_addrs
+    end
+
+    local conf_render = template.compile(ngx_tpl)
+    local ngxconf = conf_render(sys_conf)
+
+    local ok, err = file.write_file(env.apisix_home .. "/conf/nginx.conf",
+                                    ngxconf)
+    if not ok then
+        return util.die("failed to update nginx.conf: ", err)
+    end
+
+    local op_ver = get_openresty_version()
+    if op_ver == nil then
+        return util.die("can not find openresty\n")
+    end
+
+    local need_ver = "1.15.8"
+    if not check_version(op_ver, need_ver) then
+        return util.die("openresty version must >=", need_ver, " current ", op_ver, "\n")
+    end
+end
+
+
+function _M.start(...)
+    -- check running
+    local pid = file.read_file(env.pid_path)
+    if pid then
+        local hd = popen("lsof -p " .. pid)
+        local res = hd:read("*a")
+        if res and res ~= "" then
+            print("APISIX is running...")
+            return nil
+        end
+    end
+
+    _M.init(...)
+    etcd.init(...)
+
+    execute(openresty_args)
+end
+
+
+function _M.stop()
+    execute(openresty_args .. [[ -s stop]])
+end
+
+
+function _M.restart()
+    _M.stop()
+    _M.start()
+end
+
+
+function _M.reload()
+    -- reinit nginx.conf
+    _M.init()
+
+    local test_cmd = 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
+        local cmd = openresty_args .. [[ -s reload]]
+        execute(cmd)
+        return
+    end
+
+    util.die("test openresty failed")
+end
+
+
+return _M

Review comment:
       The help and version sub command locate at `bin/apisix` file since the don't belong to the scope of `ops`.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers edited a comment on pull request #2395: improve: refactor apisix command line tool

Posted by GitBox <gi...@apache.org>.
tokers edited a comment on pull request #2395:
URL: https://github.com/apache/apisix/pull/2395#issuecomment-711486895


   Rebased the newest master branch and reviewed the historical changes for the old `bin/apisix` file after the birth of this branch and applied them, also code was tweaked according to the code review comments. @spacewander @membphis 


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] membphis commented on pull request #2395: improve: refactor apisix command line tool

Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2395:
URL: https://github.com/apache/apisix/pull/2395#issuecomment-711573927


   CI failed, please take a look at the output: https://github.com/apache/apisix/pull/2395/checks?check_run_id=1272985327#step:9:43


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers commented on a change in pull request #2395: improve: refactor apisix command line tool

Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #2395:
URL: https://github.com/apache/apisix/pull/2395#discussion_r509835614



##########
File path: .luacheckrc
##########
@@ -2,3 +2,6 @@ std = "ngx_lua"
 unused_args = false
 redefined = false
 max_line_length = 100
+exclude_files = {
+    "apisix/cmd/ngx_tpl.lua",

Review comment:
       Both of them are OK for me, It's up to the personal coding habits.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers commented on a change in pull request #2395: improve: refactor apisix command line tool

Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #2395:
URL: https://github.com/apache/apisix/pull/2395#discussion_r507372495



##########
File path: apisix/cmd/etcd.lua
##########
@@ -0,0 +1,196 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local dkjson = require "dkjson"
+local file = require "apisix.cmd.file"
+local util = require "apisix.cmd.util"
+local env = require "apisix.cmd.env"
+
+local base64_encode = require("base64").encode
+
+local type = type
+local ipairs = ipairs
+local print = print
+local tonumber = tonumber
+local str_format = string.format
+
+local _M = {}
+
+
+local function parse_semantic_version(ver)
+    local errmsg = "invalid semantic version: " .. ver
+
+    local parts = util.split(ver, "-")
+    if #parts > 2 then
+        return nil, errmsg
+    end
+
+    if #parts == 2 then
+        ver = parts[1]
+    end
+
+    local fields = util.split(ver, ".")
+    if #fields ~= 3 then
+        return nil, errmsg
+    end
+
+    local major = tonumber(fields[1])
+    local minor = tonumber(fields[2])
+    local patch = tonumber(fields[3])
+
+    if not (major and minor and patch) then
+        return nil, errmsg
+    end
+
+    return {
+        major = major,
+        minor = minor,
+        patch = patch,
+    }
+end
+
+
+local function compare_semantic_version(v1, v2)
+    local ver1, err = parse_semantic_version(v1)
+    if not ver1 then
+        return nil, err
+    end
+
+    local ver2, err = parse_semantic_version(v2)
+    if not ver2 then
+        return nil, err
+    end
+
+    if ver1.major ~= ver2.major then
+        return ver1.major < ver2.major
+    end
+
+    if ver1.minor ~= ver2.minor then
+        return ver1.minor < ver2.minor
+    end
+
+    return ver1.patch < ver2.patch
+end
+
+
+function _M.init(show_output)
+    -- read_yaml_conf
+    local yaml_conf, err = file.read_yaml_conf()
+    if not yaml_conf then
+        return util.die("failed to read local yaml config of apisix: ",
+                        err)
+    end
+
+    if not yaml_conf.apisix then
+        return util.die("failed to read `apisix` field from yaml file ",
+                        "while initializing etcd")
+    end
+
+    if yaml_conf.apisix.config_center ~= "etcd" then
+        return true
+    end
+
+    if not yaml_conf.etcd then
+        return util.die("failed to read `etcd` field from yaml file ",
+                        "while initializing etcd")
+    end
+
+    --convert old single etcd config to multiple etcd config
+    if type(yaml_conf.etcd.host) == "string" then
+        yaml_conf.etcd.host = { yaml_conf.etcd.host }
+    end
+
+    local etcd_conf = yaml_conf.etcd
+    local timeout = yaml_conf.etcd.timeout or 3
+    local host_count = #(yaml_conf.etcd.host)
+    local uri
+
+    for index, host in ipairs(yaml_conf.etcd.host) do
+        -- check the etcd cluster version
+        uri = etcd_conf.host[1] .. "/version"

Review comment:
       Yeah, to prevent the very corner case like the network partition.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers commented on pull request #2395: improve: refactor apisix command line tool

Posted by GitBox <gi...@apache.org>.
tokers commented on pull request #2395:
URL: https://github.com/apache/apisix/pull/2395#issuecomment-707083816


   > many thx @tokers for your nice job. that is a big change, need time to read it first.
   > 
   > please wait for more time.
   
   OK, great care must be taken for these big changes.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers commented on a change in pull request #2395: improve: refactor apisix command line tool

Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #2395:
URL: https://github.com/apache/apisix/pull/2395#discussion_r509835614



##########
File path: .luacheckrc
##########
@@ -2,3 +2,6 @@ std = "ngx_lua"
 unused_args = false
 redefined = false
 max_line_length = 100
+exclude_files = {
+    "apisix/cmd/ngx_tpl.lua",

Review comment:
       Both of them are OK for me, it's up to the personal coding habits.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers commented on pull request #2395: improve: refactor apisix command line tool

Posted by GitBox <gi...@apache.org>.
tokers commented on pull request #2395:
URL: https://github.com/apache/apisix/pull/2395#issuecomment-717211974


   I am still debugging this problem, it's seems the `plugin_metadata` cannot be fetched in the test case 10 (plugin-metadata.t)


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers commented on pull request #2395: improve: refactor apisix command line tool

Posted by GitBox <gi...@apache.org>.
tokers commented on pull request #2395:
URL: https://github.com/apache/apisix/pull/2395#issuecomment-711486895


   Rebased the newest master branch and reviewed the historical changes for the old `bin/apisix` file before the birth of this branch and applied them, also code was tweaked according to the code review comments. @spacewander @membphis 


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] spacewander commented on a change in pull request #2395: improve: refactor apisix command line tool

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #2395:
URL: https://github.com/apache/apisix/pull/2395#discussion_r507376609



##########
File path: bin/apisix
##########
@@ -798,333 +41,30 @@ version:    print the version of apisix
 end
 
 
-local checked_admin_key = false
-local function init()
-    if is_root_path then
-        print('Warning! Running apisix under /root is only suitable for development environments'
-            .. ' and it is dangerous to do so. It is recommended to run APISIX in a directory other than /root.')
-    end
-
-    -- read_yaml_conf
-    local yaml_conf, err = read_yaml_conf()
-    if not yaml_conf then
-        error("failed to read local yaml config of apisix: " .. err)
-    end
-    -- print("etcd: ", yaml_conf.etcd.host)
-
-    -- check the Admin API token
-    if yaml_conf.apisix.enable_admin and yaml_conf.apisix.allow_admin then
-        for _, allow_ip in ipairs(yaml_conf.apisix.allow_admin) do
-            if allow_ip == "127.0.0.0/24" then
-                checked_admin_key = true
-            end
-        end
-    end
-
-    if yaml_conf.apisix.enable_admin and not checked_admin_key then
-        checked_admin_key = true
-        local help = [[
-
-%s
-Please modify "admin_key" in conf/config.yaml .
-
-]]
-        if type(yaml_conf.apisix.admin_key) ~= "table" or
-           #yaml_conf.apisix.admin_key == 0
-        then
-            io.stderr:write(help:format("ERROR: missing valid Admin API token."))
-            os.exit(1)
-        end
-
-        for _, admin in ipairs(yaml_conf.apisix.admin_key) do
-            if type(admin.key) == "table" then
-                admin.key = ""
-            else
-                admin.key = tostring(admin.key)
-            end
-
-            if admin.key == "" then
-                io.stderr:write(help:format("ERROR: missing valid Admin API token."), "\n")
-                os.exit(1)
-            end
-
-            if admin.key == "edd1c9f034335f136f87ad84b625c8f1" then
-                io.stderr:write(
-                    help:format([[WARNING: using fixed Admin API token has security risk.]]),
-                    "\n"
-                )
-            end
-        end
-    end
-
-    local or_ver = execute_cmd("openresty -V 2>&1")
-    local with_module_status = true
-    if or_ver and not or_ver:find("http_stub_status_module", 1, true) then
-        io.stderr:write("'http_stub_status_module' module is missing in ",
-                        "your openresty, please check it out. Without this ",
-                        "module, there will be fewer monitoring indicators.\n")
-        with_module_status = false
-    end
-
-    local enabled_plugins = {}
-    for i, name in ipairs(yaml_conf.plugins) do
-        enabled_plugins[name] = true
-    end
-
-    if enabled_plugins["proxy-cache"] and not yaml_conf.apisix.proxy_cache then
-        error("missing apisix.proxy_cache for plugin proxy-cache")
-    end
-
-    -- Using template.render
-    local sys_conf = {
-        lua_path = pkg_path_org,
-        lua_cpath = pkg_cpath_org,
-        os_name = trim(execute_cmd("uname")),
-        apisix_lua_home = apisix_home,
-        with_module_status = with_module_status,
-        error_log = {level = "warn"},
-        enabled_plugins = enabled_plugins,
-    }
-
-    if not yaml_conf.apisix then
-        error("failed to read `apisix` field from yaml file")
-    end
-
-    if not yaml_conf.nginx_config then
-        error("failed to read `nginx_config` field from yaml file")
-    end
-
-    if is_32bit_arch() then
-        sys_conf["worker_rlimit_core"] = "4G"
-    else
-        sys_conf["worker_rlimit_core"] = "16G"
-    end
-
-    for k,v in pairs(yaml_conf.apisix) do
-        sys_conf[k] = v
-    end
-    for k,v in pairs(yaml_conf.nginx_config) do
-        sys_conf[k] = v
-    end
-
-    local wrn = sys_conf["worker_rlimit_nofile"]
-    local wc = sys_conf["event"]["worker_connections"]
-    if not wrn or wrn <= wc then
-        -- ensure the number of fds is slightly larger than the number of conn
-        sys_conf["worker_rlimit_nofile"] = wc + 128
-    end
-
-    if(sys_conf["enable_dev_mode"] == true) then
-        sys_conf["worker_processes"] = 1
-        sys_conf["enable_reuseport"] = false
-    elseif tonumber(sys_conf["worker_processes"]) == nil then
-        sys_conf["worker_processes"] = "auto"
-    end
-
-    local dns_resolver = sys_conf["dns_resolver"]
-    if not dns_resolver or #dns_resolver == 0 then
-        local dns_addrs, err = local_dns_resolver("/etc/resolv.conf")
-        if not dns_addrs then
-            error("failed to import local DNS: " .. err)
-        end
-
-        if #dns_addrs == 0 then
-            error("local DNS is empty")
-        end
-        sys_conf["dns_resolver"] = dns_addrs
-    end
-
-    local conf_render = template.compile(ngx_tpl)
-    local ngxconf = conf_render(sys_conf)
-
-    local ok, err = write_file(apisix_home .. "/conf/nginx.conf", ngxconf)
-    if not ok then
-        error("failed to update nginx.conf: " .. err)
-    end
-
-    local op_ver = get_openresty_version()
-    if op_ver == nil then
-        io.stderr:write("can not find openresty\n")
-        return
-    end
-
-    local need_ver = "1.15.8"
-    if not check_version(op_ver, need_ver) then
-        io.stderr:write("openresty version must >=", need_ver, " current ", op_ver, "\n")
-        return
-    end
-end
-_M.init = init
-
-local function init_etcd(show_output)
-    -- read_yaml_conf
-    local yaml_conf, err = read_yaml_conf()
-    if not yaml_conf then
-        error("failed to read local yaml config of apisix: " .. err)
-    end
-
-    if not yaml_conf.apisix then
-        error("failed to read `apisix` field from yaml file when init etcd")
-    end
-
-    if yaml_conf.apisix.config_center ~= "etcd" then
-        return true
-    end
-
-    if not yaml_conf.etcd then
-        error("failed to read `etcd` field from yaml file when init etcd")
-    end
-
-    local etcd_conf = yaml_conf.etcd
-
-    local timeout = etcd_conf.timeout or 3
-    local uri
-    --convert old single etcd config to multiple etcd config
-    if type(yaml_conf.etcd.host) == "string" then
-        yaml_conf.etcd.host = {yaml_conf.etcd.host}
-    end
-
-    local cluster_version
-    local host_count = #(yaml_conf.etcd.host)
-    local dkjson = require("dkjson")
-
-    -- check the etcd cluster version
-    for index, host in ipairs(yaml_conf.etcd.host) do
-        uri = host .. "/version"
-        local cmd = string.format("curl -s -m %d %s", timeout * 2, uri)
-        local res = execute_cmd(cmd)
-        local errmsg = string.format("got malformed version message: \"%s\" from etcd\n", res)
-        local body, _, err = dkjson.decode(res)
-        if err then
-            io.stderr:write(errmsg)
-            os.exit(1)
-        end
-
-        local cluster_version = body["etcdcluster"]
-        if not cluster_version then
-            io.stderr:write(errmsg)
-            os.exit(1)
-        end
-
-        if compare_semantic_version(cluster_version, min_etcd_version) then
-            io.stderr:write("etcd cluster version ".. cluster_version ..
-                            " is less than the required version ".. min_etcd_version ..
-                            ", please upgrade your etcd cluster\n")
-            os.exit(1)
-        end
-    end
-
-    local etcd_ok = false
-    for index, host in ipairs(yaml_conf.etcd.host) do
-        local is_success = true
-
-        for _, dir_name in ipairs({"/routes", "/upstreams", "/services",
-                                   "/plugins", "/consumers", "/node_status",
-                                   "/ssl", "/global_rules", "/stream_routes",
-                                   "/proto"}) do
-            local key =  (etcd_conf.prefix or "") .. dir_name .. "/"
-
-            local base64_encode = require("base64").encode
-            local uri = host .. "/v3/kv/put"
-            local post_json = '{"value":"' .. base64_encode("init_dir") ..  '", "key":"' .. base64_encode(key) .. '"}'
-            local cmd = "curl " .. uri .. " -X POST -d '" .. post_json
-                        .. "' --connect-timeout " .. timeout
-                        .. " --max-time " .. timeout * 2 .. " --retry 1 2>&1"
-
-            local res = execute_cmd(cmd)
-            if (etcd_version == "v3" and not res:find("OK", 1, true)) then
-                is_success = false
-                if (index == host_count) then
-                    error(cmd .. "\n" .. res)
-                end
-
-                break
-            end
-
-            if show_output then
-                print(cmd)
-                print(res)
-            end
-        end
-
-        if is_success then
-            etcd_ok = true
-            break
-        end
-    end
-
-    if not etcd_ok then
-        error("none of the configured etcd works well")
-    end
-end
-_M.init_etcd = init_etcd
-
-local openresty_args = [[openresty  -p ]] .. apisix_home .. [[ -c ]]
-                       .. apisix_home .. [[/conf/nginx.conf]]
-
-function _M.start(...)
-    -- check running
-    local pid_path = apisix_home .. "/logs/nginx.pid"
-    local pid, err = read_file(pid_path)
-    if pid then
-        local hd = io.popen("lsof -p " .. pid)
-        local res = hd:read("*a")
-        if res and res ~= "" then
-            print("APISIX is running...")
-            return nil
-        end
-    end
-
-    init(...)
-    init_etcd(...)
-
-    local cmd = openresty_args
-    -- print(cmd)
-    os.execute(cmd)
-end
-
-function _M.stop()
-    local cmd = openresty_args .. [[ -s stop]]
-    -- print(cmd)
-    os.execute(cmd)
-end
-
-function _M.restart()
-  _M.stop()
-  _M.start()
+local function version()
+    print(ver['VERSION'])
 end
 
-function _M.reload()
-    -- reinit nginx.conf
-    init()
-
-    local test_cmd = 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 = os.execute((test_cmd))
-    if (test_ret == 0 or test_ret == true) then
-        local cmd = openresty_args .. [[ -s reload]]
-        -- print(cmd)
-        os.execute(cmd)
-        return
-    end
-    print("test openresty failed")
-end
 
-function _M.version()
-    local ver = require("apisix.core.version")
-    print(ver['VERSION'])
-end
+local command = {
+    version = version,
+    help = help,
+    init = ops.init,
+    init_etcd = etcd.init,
+    start = ops.start,
+    stop = ops.stop,
+    restart = ops.restart,
+    reload = ops.reload,
+}
 
-local cmd_action = arg[1]
-if not cmd_action then
-    return _M.help()
+local action = arg[1]
+if not action then
+    return help()
 end
 
-if not _M[cmd_action] then
-    print("invalid argument: ", cmd_action, "\n")
+if not command[action] then
+    util.die("unknown sub-command: ", action, "\n")

Review comment:
       Please also integrate https://github.com/apache/apisix/commit/1fe4e50f5a153edc738efbe92aba3f910e5c888e into this refactor.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] membphis commented on pull request #2395: improve: refactor apisix command line tool

Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2395:
URL: https://github.com/apache/apisix/pull/2395#issuecomment-714226293


   @tokers please rebase your branch, CI failed


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers commented on pull request #2395: improve: refactor apisix command line tool

Posted by GitBox <gi...@apache.org>.
tokers commented on pull request #2395:
URL: https://github.com/apache/apisix/pull/2395#issuecomment-714178118


   @membphis Fixed.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] spacewander commented on a change in pull request #2395: improve: refactor apisix command line tool

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #2395:
URL: https://github.com/apache/apisix/pull/2395#discussion_r506123444



##########
File path: apisix/cmd/env.lua
##########
@@ -0,0 +1,77 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local util = require "apisix.cmd.util"
+
+local execute_cmd   = util.execute_cmd
+local trim          = util.trim
+local pcall         = pcall
+local error         = error
+local str_match     = string.match
+local pkg_cpath_org = package.cpath
+local pkg_path_org  = package.path
+local apisix_home   = "/usr/local/apisix"

Review comment:
       Style: constant definition should not be here.

##########
File path: apisix/cmd/ops.lua
##########
@@ -0,0 +1,294 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local env = require "apisix.cmd.env"
+local etcd = require "apisix.cmd.etcd"
+local file = require "apisix.cmd.file"
+local util = require "apisix.cmd.util"
+local ngx_tpl = require "apisix.cmd.ngx_tpl"
+local template = require "resty.template"
+
+local type = type
+local pairs = pairs
+local print = print
+local ipairs = ipairs
+local tonumber = tonumber
+local tostring = tostring
+local str_find = string.find
+local str_sub = string.sub
+local max = math.max
+local popen = io.popen
+local execute = os.execute
+local error = error
+local stderr = io.stderr
+local openresty_args = [[openresty  -p ]] .. env.apisix_home .. [[ -c ]]

Review comment:
       Style: constant definition should be put a few lines after here.

##########
File path: apisix/cmd/file.lua
##########
@@ -0,0 +1,130 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local yaml = require "tinyyaml"
+local env = require "apisix.cmd.env"
+local profile = require "apisix.core.profile"
+
+local pairs = pairs
+local type = type
+local open = io.open
+local str_find = string.find
+local str_gmatch = string.gmatch
+local _M = {}

Review comment:
       Need blank lines before `_M`.

##########
File path: apisix/cmd/ops.lua
##########
@@ -0,0 +1,294 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local env = require "apisix.cmd.env"
+local etcd = require "apisix.cmd.etcd"
+local file = require "apisix.cmd.file"
+local util = require "apisix.cmd.util"
+local ngx_tpl = require "apisix.cmd.ngx_tpl"
+local template = require "resty.template"
+
+local type = type
+local pairs = pairs
+local print = print
+local ipairs = ipairs
+local tonumber = tonumber
+local tostring = tostring
+local str_find = string.find
+local str_sub = string.sub
+local max = math.max
+local popen = io.popen
+local execute = os.execute
+local error = error
+local stderr = io.stderr
+local openresty_args = [[openresty  -p ]] .. env.apisix_home .. [[ -c ]]
+                       .. env.apisix_home .. [[/conf/nginx.conf]]
+
+local _M = {}
+
+
+local function get_openresty_version()
+    local str = "nginx version: openresty/"
+    local ret = util.execute_cmd("openresty -v 2>&1")
+    local pos = str_find(ret, str)
+    if pos then
+        return str_sub(ret, pos + #str)
+    end
+
+    str = "nginx version: nginx/"
+    ret = util.execute_cmd("openresty -v 2>&1")
+    pos = str_find(ret, str)
+    if pos then
+        return str_sub(ret, pos + #str)
+    end
+
+    return nil
+end
+
+
+local function check_version(cur_ver_s, need_ver_s)
+    local cur_vers = util.split(cur_ver_s, [[.]])
+    local need_vers = util.split(need_ver_s, [[.]])
+    local len = max(#cur_vers, #need_vers)
+
+    for i = 1, len do
+        local cur_ver = tonumber(cur_vers[i]) or 0
+        local need_ver = tonumber(need_vers[i]) or 0
+        if cur_ver > need_ver then
+            return true
+        end
+
+        if cur_ver < need_ver then
+            return false
+        end
+    end
+
+    return true
+end
+
+
+function _M.init()
+    if env.is_root_path then
+        print("Warning! Running apisix under /root is only suitable for "
+              .. "development environments and it is dangerous to do so."
+              .. "It is recommended to run APISIX in a directory other than "
+              .. "/root.")
+    end
+
+    -- read_yaml_conf
+    local yaml_conf, err = file.read_yaml_conf()
+    if not yaml_conf then
+        error("failed to read local yaml config of apisix: " .. err)
+    end
+
+    -- check the Admin API token
+    local checked_admin_key = false
+    if yaml_conf.apisix.enable_admin and yaml_conf.apisix.allow_admin then
+        for _, allow_ip in ipairs(yaml_conf.apisix.allow_admin) do
+            if allow_ip == "127.0.0.0/24" then
+                checked_admin_key = true
+                break
+            end
+        end
+    end
+
+    if yaml_conf.apisix.enable_admin and not checked_admin_key then
+        local help = [[
+
+%s
+Please modify "admin_key" in conf/config.yaml .
+
+]]
+        if type(yaml_conf.apisix.admin_key) ~= "table" or
+           #yaml_conf.apisix.admin_key == 0
+        then
+            return util.die(help:format("ERROR: missing valid Admin API token."))
+        end
+
+        for _, admin in ipairs(yaml_conf.apisix.admin_key) do
+            if type(admin.key) == "table" then
+                admin.key = ""
+            else
+                admin.key = tostring(admin.key)
+            end
+
+            if admin.key == "" then
+                return util.die(help:format("ERROR: missing valid Admin API token."),
+                                "\n")
+            end
+
+            if admin.key == "edd1c9f034335f136f87ad84b625c8f1" then
+                local msg = help:format([[WARNING: using fixed Admin API token has security risk.]])

Review comment:
       Should use `ERROR` here since we will exit.

##########
File path: apisix/cmd/ops.lua
##########
@@ -0,0 +1,294 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local env = require "apisix.cmd.env"
+local etcd = require "apisix.cmd.etcd"
+local file = require "apisix.cmd.file"
+local util = require "apisix.cmd.util"
+local ngx_tpl = require "apisix.cmd.ngx_tpl"
+local template = require "resty.template"
+
+local type = type
+local pairs = pairs
+local print = print
+local ipairs = ipairs
+local tonumber = tonumber
+local tostring = tostring
+local str_find = string.find
+local str_sub = string.sub
+local max = math.max
+local popen = io.popen
+local execute = os.execute
+local error = error
+local stderr = io.stderr
+local openresty_args = [[openresty  -p ]] .. env.apisix_home .. [[ -c ]]
+                       .. env.apisix_home .. [[/conf/nginx.conf]]
+
+local _M = {}
+
+
+local function get_openresty_version()
+    local str = "nginx version: openresty/"
+    local ret = util.execute_cmd("openresty -v 2>&1")
+    local pos = str_find(ret, str)

Review comment:
       Plain string find is required.

##########
File path: apisix/cmd/ops.lua
##########
@@ -0,0 +1,294 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local env = require "apisix.cmd.env"
+local etcd = require "apisix.cmd.etcd"
+local file = require "apisix.cmd.file"
+local util = require "apisix.cmd.util"
+local ngx_tpl = require "apisix.cmd.ngx_tpl"
+local template = require "resty.template"
+
+local type = type
+local pairs = pairs
+local print = print
+local ipairs = ipairs
+local tonumber = tonumber
+local tostring = tostring
+local str_find = string.find
+local str_sub = string.sub
+local max = math.max
+local popen = io.popen
+local execute = os.execute
+local error = error
+local stderr = io.stderr
+local openresty_args = [[openresty  -p ]] .. env.apisix_home .. [[ -c ]]
+                       .. env.apisix_home .. [[/conf/nginx.conf]]
+
+local _M = {}
+
+
+local function get_openresty_version()
+    local str = "nginx version: openresty/"
+    local ret = util.execute_cmd("openresty -v 2>&1")
+    local pos = str_find(ret, str)
+    if pos then
+        return str_sub(ret, pos + #str)
+    end
+
+    str = "nginx version: nginx/"
+    ret = util.execute_cmd("openresty -v 2>&1")
+    pos = str_find(ret, str)
+    if pos then
+        return str_sub(ret, pos + #str)
+    end
+
+    return nil
+end
+
+
+local function check_version(cur_ver_s, need_ver_s)
+    local cur_vers = util.split(cur_ver_s, [[.]])
+    local need_vers = util.split(need_ver_s, [[.]])
+    local len = max(#cur_vers, #need_vers)
+
+    for i = 1, len do
+        local cur_ver = tonumber(cur_vers[i]) or 0
+        local need_ver = tonumber(need_vers[i]) or 0
+        if cur_ver > need_ver then
+            return true
+        end
+
+        if cur_ver < need_ver then
+            return false
+        end
+    end
+
+    return true
+end
+
+
+function _M.init()
+    if env.is_root_path then
+        print("Warning! Running apisix under /root is only suitable for "
+              .. "development environments and it is dangerous to do so."
+              .. "It is recommended to run APISIX in a directory other than "
+              .. "/root.")
+    end
+
+    -- read_yaml_conf
+    local yaml_conf, err = file.read_yaml_conf()
+    if not yaml_conf then
+        error("failed to read local yaml config of apisix: " .. err)
+    end
+
+    -- check the Admin API token
+    local checked_admin_key = false
+    if yaml_conf.apisix.enable_admin and yaml_conf.apisix.allow_admin then
+        for _, allow_ip in ipairs(yaml_conf.apisix.allow_admin) do
+            if allow_ip == "127.0.0.0/24" then
+                checked_admin_key = true
+                break
+            end
+        end
+    end
+
+    if yaml_conf.apisix.enable_admin and not checked_admin_key then
+        local help = [[
+
+%s
+Please modify "admin_key" in conf/config.yaml .
+
+]]
+        if type(yaml_conf.apisix.admin_key) ~= "table" or
+           #yaml_conf.apisix.admin_key == 0
+        then
+            return util.die(help:format("ERROR: missing valid Admin API token."))
+        end
+
+        for _, admin in ipairs(yaml_conf.apisix.admin_key) do
+            if type(admin.key) == "table" then
+                admin.key = ""
+            else
+                admin.key = tostring(admin.key)
+            end
+
+            if admin.key == "" then
+                return util.die(help:format("ERROR: missing valid Admin API token."),
+                                "\n")
+            end
+
+            if admin.key == "edd1c9f034335f136f87ad84b625c8f1" then
+                local msg = help:format([[WARNING: using fixed Admin API token has security risk.]])
+                return util.die(msg, "\n")
+            end
+        end
+    end
+
+    local with_module_status = true
+
+    local or_ver = util.execute_cmd("openresty -V 2>&1")
+    if or_ver and not or_ver:find("http_stub_status_module", 1, true) then
+        stderr:write("'http_stub_status_module' module is missing in ",
+                     "your openresty, please check it out. Without this ",
+                     "module, there will be fewer monitoring indicators.\n")
+
+        with_module_status = false
+    end
+
+    local enabled_plugins = {}
+    for i, name in ipairs(yaml_conf.plugins) do
+        enabled_plugins[name] = true
+    end
+
+    if enabled_plugins["proxy-cache"] and not yaml_conf.apisix.proxy_cache then
+        error("missing apisix.proxy_cache for plugin proxy-cache")
+    end
+
+    -- Using template.render
+    local sys_conf = {
+        lua_path           = env.pkg_path_org,
+        lua_cpath          = env.pkg_cpath_org,
+        apisix_lua_home    = env.apisix_home,
+        os_name            = util.trim(util.execute_cmd("uname")),
+        with_module_status = with_module_status,
+        error_log          = { level = "warn" },
+        enabled_plugins    = enabled_plugins,
+    }
+
+    if not yaml_conf.apisix then
+        error("failed to read `apisix` field from yaml file")
+    end
+
+    if not yaml_conf.nginx_config then
+        error("failed to read `nginx_config` field from yaml file")
+    end
+
+    if util.is_32bit_arch() then
+        sys_conf["worker_rlimit_core"] = "4G"
+    else
+        sys_conf["worker_rlimit_core"] = "16G"
+    end
+
+    for k,v in pairs(yaml_conf.apisix) do
+        sys_conf[k] = v
+    end
+
+    for k,v in pairs(yaml_conf.nginx_config) do
+        sys_conf[k] = v
+    end
+
+    local wrn = sys_conf["worker_rlimit_nofile"]
+    local wc = sys_conf["event"]["worker_connections"]
+    if not wrn or wrn <= wc then
+        -- ensure the number of fds is slightly larger than the number of conn
+        sys_conf["worker_rlimit_nofile"] = wc + 128
+    end
+
+    if sys_conf["enable_dev_mode"] == true then
+        sys_conf["worker_processes"] = 1
+        sys_conf["enable_reuseport"] = false
+
+    elseif tonumber(sys_conf["worker_processes"]) == nil then
+        sys_conf["worker_processes"] = "auto"
+    end
+
+    local dns_resolver = sys_conf["dns_resolver"]
+    if not dns_resolver or #dns_resolver == 0 then
+        local dns_addrs, err = util.local_dns_resolver("/etc/resolv.conf")
+        if not dns_addrs then
+            return util.die("failed to import local DNS: ", err)
+        end
+
+        if #dns_addrs == 0 then
+            return util.die("local DNS is empty")
+        end
+
+        sys_conf["dns_resolver"] = dns_addrs
+    end
+
+    local conf_render = template.compile(ngx_tpl)
+    local ngxconf = conf_render(sys_conf)
+
+    local ok, err = file.write_file(env.apisix_home .. "/conf/nginx.conf",
+                                    ngxconf)
+    if not ok then
+        return util.die("failed to update nginx.conf: ", err)
+    end
+
+    local op_ver = get_openresty_version()
+    if op_ver == nil then
+        return util.die("can not find openresty\n")
+    end
+
+    local need_ver = "1.15.8"
+    if not check_version(op_ver, need_ver) then
+        return util.die("openresty version must >=", need_ver, " current ", op_ver, "\n")
+    end
+end
+
+
+function _M.start(...)
+    -- check running
+    local pid = file.read_file(env.pid_path)
+    if pid then
+        local hd = popen("lsof -p " .. pid)
+        local res = hd:read("*a")
+        if res and res ~= "" then
+            print("APISIX is running...")
+            return nil
+        end
+    end
+
+    _M.init(...)
+    etcd.init(...)
+
+    execute(openresty_args)
+end
+
+
+function _M.stop()
+    execute(openresty_args .. [[ -s stop]])
+end
+
+
+function _M.restart()
+    _M.stop()
+    _M.start()
+end
+
+
+function _M.reload()
+    -- reinit nginx.conf
+    _M.init()
+
+    local test_cmd = 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
+        local cmd = openresty_args .. [[ -s reload]]
+        execute(cmd)
+        return
+    end
+
+    util.die("test openresty failed")
+end
+
+
+return _M

Review comment:
       Missing `_M.help` and `_M.version`?

##########
File path: apisix/cmd/etcd.lua
##########
@@ -0,0 +1,196 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local dkjson = require "dkjson"
+local file = require "apisix.cmd.file"
+local util = require "apisix.cmd.util"
+local env = require "apisix.cmd.env"
+
+local base64_encode = require("base64").encode
+
+local type = type
+local ipairs = ipairs
+local print = print
+local tonumber = tonumber
+local str_format = string.format
+
+local _M = {}
+
+
+local function parse_semantic_version(ver)
+    local errmsg = "invalid semantic version: " .. ver
+
+    local parts = util.split(ver, "-")
+    if #parts > 2 then
+        return nil, errmsg
+    end
+
+    if #parts == 2 then
+        ver = parts[1]
+    end
+
+    local fields = util.split(ver, ".")
+    if #fields ~= 3 then
+        return nil, errmsg
+    end
+
+    local major = tonumber(fields[1])
+    local minor = tonumber(fields[2])
+    local patch = tonumber(fields[3])
+
+    if not (major and minor and patch) then
+        return nil, errmsg
+    end
+
+    return {
+        major = major,
+        minor = minor,
+        patch = patch,
+    }
+end
+
+
+local function compare_semantic_version(v1, v2)
+    local ver1, err = parse_semantic_version(v1)
+    if not ver1 then
+        return nil, err
+    end
+
+    local ver2, err = parse_semantic_version(v2)
+    if not ver2 then
+        return nil, err
+    end
+
+    if ver1.major ~= ver2.major then
+        return ver1.major < ver2.major
+    end
+
+    if ver1.minor ~= ver2.minor then
+        return ver1.minor < ver2.minor
+    end
+
+    return ver1.patch < ver2.patch
+end
+
+
+function _M.init(show_output)
+    -- read_yaml_conf
+    local yaml_conf, err = file.read_yaml_conf()
+    if not yaml_conf then
+        return util.die("failed to read local yaml config of apisix: ",
+                        err)
+    end
+
+    if not yaml_conf.apisix then
+        return util.die("failed to read `apisix` field from yaml file ",
+                        "while initializing etcd")
+    end
+
+    if yaml_conf.apisix.config_center ~= "etcd" then
+        return true
+    end
+
+    if not yaml_conf.etcd then
+        return util.die("failed to read `etcd` field from yaml file ",
+                        "while initializing etcd")
+    end
+
+    --convert old single etcd config to multiple etcd config
+    if type(yaml_conf.etcd.host) == "string" then
+        yaml_conf.etcd.host = { yaml_conf.etcd.host }
+    end
+
+    local etcd_conf = yaml_conf.etcd
+    local timeout = yaml_conf.etcd.timeout or 3
+    local host_count = #(yaml_conf.etcd.host)
+    local uri
+
+    for index, host in ipairs(yaml_conf.etcd.host) do
+        -- check the etcd cluster version
+        uri = etcd_conf.host[1] .. "/version"

Review comment:
       The target host should be different in the loop?

##########
File path: apisix/cmd/file.lua
##########
@@ -0,0 +1,130 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local yaml = require "tinyyaml"
+local env = require "apisix.cmd.env"
+local profile = require "apisix.core.profile"
+
+local pairs = pairs
+local type = type
+local open = io.open
+local str_find = string.find
+local str_gmatch = string.gmatch
+local _M = {}
+
+
+local function tab_is_array(t)
+    local count = 0
+    for k,v in pairs(t) do
+        count = count + 1
+    end
+
+    return #t == count
+end
+
+
+local merge_conf
+merge_conf = function(base, new_tab)
+    for key, val in pairs(new_tab) do
+        if type(val) == "table" then
+            if tab_is_array(val) then
+                base[key] = val
+            else
+                merge_conf(base[key], val)
+            end
+
+        else
+            base[key] = val
+        end
+    end
+
+    return base
+end
+
+
+local function is_empty_yaml_line(line)
+    return line == '' or str_find(line, '^%s*$') or str_find(line, '^%s*#')
+end
+
+
+function _M.write_file(file_path, data)
+    local file, err = open(file_path, "w+")
+    if not file then
+        return false, "failed to open file: " .. file_path .. ", error info:" .. err
+    end
+
+    file:write(data)

Review comment:
       Should check the returned value of `write`.

##########
File path: apisix/cmd/file.lua
##########
@@ -0,0 +1,130 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local yaml = require "tinyyaml"
+local env = require "apisix.cmd.env"
+local profile = require "apisix.core.profile"
+
+local pairs = pairs
+local type = type
+local open = io.open
+local str_find = string.find
+local str_gmatch = string.gmatch
+local _M = {}
+
+
+local function tab_is_array(t)
+    local count = 0
+    for k,v in pairs(t) do
+        count = count + 1
+    end
+
+    return #t == count
+end
+
+
+local merge_conf
+merge_conf = function(base, new_tab)
+    for key, val in pairs(new_tab) do
+        if type(val) == "table" then
+            if tab_is_array(val) then
+                base[key] = val
+            else
+                merge_conf(base[key], val)
+            end
+
+        else
+            base[key] = val
+        end
+    end
+
+    return base
+end
+
+
+local function is_empty_yaml_line(line)
+    return line == '' or str_find(line, '^%s*$') or str_find(line, '^%s*#')
+end
+
+
+function _M.write_file(file_path, data)
+    local file, err = open(file_path, "w+")
+    if not file then
+        return false, "failed to open file: " .. file_path .. ", error info:" .. err
+    end
+
+    file:write(data)
+    file:close()
+
+    return true
+end
+
+
+function _M.read_file(file_path)
+    local file, err = open(file_path, "rb")
+    if not file then
+        return false, "failed to open file: " .. file_path .. ", error info:" .. err
+    end
+
+    local data = file:read("*all")

Review comment:
       Should check the returned value of `read`.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] membphis commented on a change in pull request #2395: improve: refactor apisix command line tool

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2395:
URL: https://github.com/apache/apisix/pull/2395#discussion_r509880180



##########
File path: rockspec/apisix-master-0.rockspec
##########
@@ -20,8 +20,8 @@ version = "master-0"
 supported_platforms = {"linux", "macosx"}
 
 source = {
-    url = "git://github.com/apache/apisix",
-    branch = "master",
+    url = "git://github.com/tokers/apisix",
+    branch = "improve/cmd_refactoring",

Review comment:
       which CI will fail? I think we need to resolve this 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers commented on a change in pull request #2395: improve: refactor apisix command line tool

Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #2395:
URL: https://github.com/apache/apisix/pull/2395#discussion_r514948544



##########
File path: apisix/cli/env.lua
##########
@@ -0,0 +1,77 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local util = require("apisix.cli.util")
+
+local execute_cmd   = util.execute_cmd
+local trim          = util.trim
+local pcall         = pcall
+local error         = error
+local str_match     = string.match
+local pkg_cpath_org = package.cpath
+local pkg_path_org  = package.path
+
+local apisix_home   = "/usr/local/apisix"
+local pkg_cpath = apisix_home .. "/deps/lib64/lua/5.1/?.so;"
+                  .. apisix_home .. "/deps/lib/lua/5.1/?.so;;"
+local pkg_path  = apisix_home .. "/deps/share/lua/5.1/?.lua;;"
+
+
+-- only for developer, use current folder as working space
+local is_root_path = false
+local script_path = arg[0]
+if script_path:sub(1, 2) == './' then
+    apisix_home = trim(execute_cmd("pwd"))
+    if not apisix_home then
+        error("failed to fetch current path")
+    end
+
+    if str_match(apisix_home .. "/", '^/root/') then

Review comment:
       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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers commented on pull request #2395: improve: refactor apisix command line tool

Posted by GitBox <gi...@apache.org>.
tokers commented on pull request #2395:
URL: https://github.com/apache/apisix/pull/2395#issuecomment-706958472


   Temporarily modify the branch from `master` to `improve/cmd_refactoring` file to pass the CI. Should be reset after merging.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers commented on a change in pull request #2395: improve: refactor apisix command line tool

Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #2395:
URL: https://github.com/apache/apisix/pull/2395#discussion_r509905087



##########
File path: rockspec/apisix-master-0.rockspec
##########
@@ -20,8 +20,8 @@ version = "master-0"
 supported_platforms = {"linux", "macosx"}
 
 source = {
-    url = "git://github.com/apache/apisix",
-    branch = "master",
+    url = "git://github.com/tokers/apisix",
+    branch = "improve/cmd_refactoring",

Review comment:
       The `apisix_cli_test.sh`.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers commented on a change in pull request #2395: improve: refactor apisix command line tool

Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #2395:
URL: https://github.com/apache/apisix/pull/2395#discussion_r509835277



##########
File path: apisix/cmd/env.lua
##########
@@ -0,0 +1,77 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local util = require "apisix.cmd.util"

Review comment:
       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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers commented on pull request #2395: improve: refactor apisix command line tool

Posted by GitBox <gi...@apache.org>.
tokers commented on pull request #2395:
URL: https://github.com/apache/apisix/pull/2395#issuecomment-712537171


   @membphis I'm investigating this failure, it's a bit weird


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] membphis commented on pull request #2395: improve: refactor apisix command line tool

Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2395:
URL: https://github.com/apache/apisix/pull/2395#issuecomment-717248751


   > I am still debugging this problem, it's seems the `plugin_metadata` cannot be fetched in the test case 10 (plugin-metadata.t) 😂
   
   https://github.com/apache/apisix/pull/2541 
   
   Is it a related PR?


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers commented on pull request #2395: improve: refactor apisix command line tool

Posted by GitBox <gi...@apache.org>.
tokers commented on pull request #2395:
URL: https://github.com/apache/apisix/pull/2395#issuecomment-712541082


   > CI failed, please take a look at the output: https://github.com/apache/apisix/pull/2395/checks?check_run_id=1272985327#step:9:43
   
   @membphis Fixed.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers removed a comment on pull request #2395: improve: refactor apisix command line tool

Posted by GitBox <gi...@apache.org>.
tokers removed a comment on pull request #2395:
URL: https://github.com/apache/apisix/pull/2395#issuecomment-712537171


   @membphis I'm investigating this failure, it's a bit weird


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers commented on pull request #2395: improve: refactor apisix command line tool

Posted by GitBox <gi...@apache.org>.
tokers commented on pull request #2395:
URL: https://github.com/apache/apisix/pull/2395#issuecomment-706958566


   @membphis 


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] membphis commented on pull request #2395: improve: refactor apisix command line tool

Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2395:
URL: https://github.com/apache/apisix/pull/2395#issuecomment-707024027


   many thx @tokers for your nice job. that is a big change, need time to read it first. 
   
   please wait for more time.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] membphis commented on a change in pull request #2395: improve: refactor apisix command line tool

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2395:
URL: https://github.com/apache/apisix/pull/2395#discussion_r509027656



##########
File path: .luacheckrc
##########
@@ -2,3 +2,6 @@ std = "ngx_lua"
 unused_args = false
 redefined = false
 max_line_length = 100
+exclude_files = {
+    "apisix/cmd/ngx_tpl.lua",

Review comment:
       `cmd` -> `cli`
   
   @tokers What do you think?

##########
File path: apisix/cmd/env.lua
##########
@@ -0,0 +1,77 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local util = require "apisix.cmd.util"

Review comment:
       code style: `require("apisix.cmd.util")`

##########
File path: rockspec/apisix-master-0.rockspec
##########
@@ -20,8 +20,8 @@ version = "master-0"
 supported_platforms = {"linux", "macosx"}
 
 source = {
-    url = "git://github.com/apache/apisix",
-    branch = "master",
+    url = "git://github.com/tokers/apisix",
+    branch = "improve/cmd_refactoring",

Review comment:
       why we need to change this?

##########
File path: apisix/cmd/env.lua
##########
@@ -0,0 +1,77 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local util = require "apisix.cmd.util"

Review comment:
       and please fix similar points




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers commented on a change in pull request #2395: improve: refactor apisix command line tool

Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #2395:
URL: https://github.com/apache/apisix/pull/2395#discussion_r507395837



##########
File path: bin/apisix
##########
@@ -798,333 +41,30 @@ version:    print the version of apisix
 end
 
 
-local checked_admin_key = false
-local function init()
-    if is_root_path then
-        print('Warning! Running apisix under /root is only suitable for development environments'
-            .. ' and it is dangerous to do so. It is recommended to run APISIX in a directory other than /root.')
-    end
-
-    -- read_yaml_conf
-    local yaml_conf, err = read_yaml_conf()
-    if not yaml_conf then
-        error("failed to read local yaml config of apisix: " .. err)
-    end
-    -- print("etcd: ", yaml_conf.etcd.host)
-
-    -- check the Admin API token
-    if yaml_conf.apisix.enable_admin and yaml_conf.apisix.allow_admin then
-        for _, allow_ip in ipairs(yaml_conf.apisix.allow_admin) do
-            if allow_ip == "127.0.0.0/24" then
-                checked_admin_key = true
-            end
-        end
-    end
-
-    if yaml_conf.apisix.enable_admin and not checked_admin_key then
-        checked_admin_key = true
-        local help = [[
-
-%s
-Please modify "admin_key" in conf/config.yaml .
-
-]]
-        if type(yaml_conf.apisix.admin_key) ~= "table" or
-           #yaml_conf.apisix.admin_key == 0
-        then
-            io.stderr:write(help:format("ERROR: missing valid Admin API token."))
-            os.exit(1)
-        end
-
-        for _, admin in ipairs(yaml_conf.apisix.admin_key) do
-            if type(admin.key) == "table" then
-                admin.key = ""
-            else
-                admin.key = tostring(admin.key)
-            end
-
-            if admin.key == "" then
-                io.stderr:write(help:format("ERROR: missing valid Admin API token."), "\n")
-                os.exit(1)
-            end
-
-            if admin.key == "edd1c9f034335f136f87ad84b625c8f1" then
-                io.stderr:write(
-                    help:format([[WARNING: using fixed Admin API token has security risk.]]),
-                    "\n"
-                )
-            end
-        end
-    end
-
-    local or_ver = execute_cmd("openresty -V 2>&1")
-    local with_module_status = true
-    if or_ver and not or_ver:find("http_stub_status_module", 1, true) then
-        io.stderr:write("'http_stub_status_module' module is missing in ",
-                        "your openresty, please check it out. Without this ",
-                        "module, there will be fewer monitoring indicators.\n")
-        with_module_status = false
-    end
-
-    local enabled_plugins = {}
-    for i, name in ipairs(yaml_conf.plugins) do
-        enabled_plugins[name] = true
-    end
-
-    if enabled_plugins["proxy-cache"] and not yaml_conf.apisix.proxy_cache then
-        error("missing apisix.proxy_cache for plugin proxy-cache")
-    end
-
-    -- Using template.render
-    local sys_conf = {
-        lua_path = pkg_path_org,
-        lua_cpath = pkg_cpath_org,
-        os_name = trim(execute_cmd("uname")),
-        apisix_lua_home = apisix_home,
-        with_module_status = with_module_status,
-        error_log = {level = "warn"},
-        enabled_plugins = enabled_plugins,
-    }
-
-    if not yaml_conf.apisix then
-        error("failed to read `apisix` field from yaml file")
-    end
-
-    if not yaml_conf.nginx_config then
-        error("failed to read `nginx_config` field from yaml file")
-    end
-
-    if is_32bit_arch() then
-        sys_conf["worker_rlimit_core"] = "4G"
-    else
-        sys_conf["worker_rlimit_core"] = "16G"
-    end
-
-    for k,v in pairs(yaml_conf.apisix) do
-        sys_conf[k] = v
-    end
-    for k,v in pairs(yaml_conf.nginx_config) do
-        sys_conf[k] = v
-    end
-
-    local wrn = sys_conf["worker_rlimit_nofile"]
-    local wc = sys_conf["event"]["worker_connections"]
-    if not wrn or wrn <= wc then
-        -- ensure the number of fds is slightly larger than the number of conn
-        sys_conf["worker_rlimit_nofile"] = wc + 128
-    end
-
-    if(sys_conf["enable_dev_mode"] == true) then
-        sys_conf["worker_processes"] = 1
-        sys_conf["enable_reuseport"] = false
-    elseif tonumber(sys_conf["worker_processes"]) == nil then
-        sys_conf["worker_processes"] = "auto"
-    end
-
-    local dns_resolver = sys_conf["dns_resolver"]
-    if not dns_resolver or #dns_resolver == 0 then
-        local dns_addrs, err = local_dns_resolver("/etc/resolv.conf")
-        if not dns_addrs then
-            error("failed to import local DNS: " .. err)
-        end
-
-        if #dns_addrs == 0 then
-            error("local DNS is empty")
-        end
-        sys_conf["dns_resolver"] = dns_addrs
-    end
-
-    local conf_render = template.compile(ngx_tpl)
-    local ngxconf = conf_render(sys_conf)
-
-    local ok, err = write_file(apisix_home .. "/conf/nginx.conf", ngxconf)
-    if not ok then
-        error("failed to update nginx.conf: " .. err)
-    end
-
-    local op_ver = get_openresty_version()
-    if op_ver == nil then
-        io.stderr:write("can not find openresty\n")
-        return
-    end
-
-    local need_ver = "1.15.8"
-    if not check_version(op_ver, need_ver) then
-        io.stderr:write("openresty version must >=", need_ver, " current ", op_ver, "\n")
-        return
-    end
-end
-_M.init = init
-
-local function init_etcd(show_output)
-    -- read_yaml_conf
-    local yaml_conf, err = read_yaml_conf()
-    if not yaml_conf then
-        error("failed to read local yaml config of apisix: " .. err)
-    end
-
-    if not yaml_conf.apisix then
-        error("failed to read `apisix` field from yaml file when init etcd")
-    end
-
-    if yaml_conf.apisix.config_center ~= "etcd" then
-        return true
-    end
-
-    if not yaml_conf.etcd then
-        error("failed to read `etcd` field from yaml file when init etcd")
-    end
-
-    local etcd_conf = yaml_conf.etcd
-
-    local timeout = etcd_conf.timeout or 3
-    local uri
-    --convert old single etcd config to multiple etcd config
-    if type(yaml_conf.etcd.host) == "string" then
-        yaml_conf.etcd.host = {yaml_conf.etcd.host}
-    end
-
-    local cluster_version
-    local host_count = #(yaml_conf.etcd.host)
-    local dkjson = require("dkjson")
-
-    -- check the etcd cluster version
-    for index, host in ipairs(yaml_conf.etcd.host) do
-        uri = host .. "/version"
-        local cmd = string.format("curl -s -m %d %s", timeout * 2, uri)
-        local res = execute_cmd(cmd)
-        local errmsg = string.format("got malformed version message: \"%s\" from etcd\n", res)
-        local body, _, err = dkjson.decode(res)
-        if err then
-            io.stderr:write(errmsg)
-            os.exit(1)
-        end
-
-        local cluster_version = body["etcdcluster"]
-        if not cluster_version then
-            io.stderr:write(errmsg)
-            os.exit(1)
-        end
-
-        if compare_semantic_version(cluster_version, min_etcd_version) then
-            io.stderr:write("etcd cluster version ".. cluster_version ..
-                            " is less than the required version ".. min_etcd_version ..
-                            ", please upgrade your etcd cluster\n")
-            os.exit(1)
-        end
-    end
-
-    local etcd_ok = false
-    for index, host in ipairs(yaml_conf.etcd.host) do
-        local is_success = true
-
-        for _, dir_name in ipairs({"/routes", "/upstreams", "/services",
-                                   "/plugins", "/consumers", "/node_status",
-                                   "/ssl", "/global_rules", "/stream_routes",
-                                   "/proto"}) do
-            local key =  (etcd_conf.prefix or "") .. dir_name .. "/"
-
-            local base64_encode = require("base64").encode
-            local uri = host .. "/v3/kv/put"
-            local post_json = '{"value":"' .. base64_encode("init_dir") ..  '", "key":"' .. base64_encode(key) .. '"}'
-            local cmd = "curl " .. uri .. " -X POST -d '" .. post_json
-                        .. "' --connect-timeout " .. timeout
-                        .. " --max-time " .. timeout * 2 .. " --retry 1 2>&1"
-
-            local res = execute_cmd(cmd)
-            if (etcd_version == "v3" and not res:find("OK", 1, true)) then
-                is_success = false
-                if (index == host_count) then
-                    error(cmd .. "\n" .. res)
-                end
-
-                break
-            end
-
-            if show_output then
-                print(cmd)
-                print(res)
-            end
-        end
-
-        if is_success then
-            etcd_ok = true
-            break
-        end
-    end
-
-    if not etcd_ok then
-        error("none of the configured etcd works well")
-    end
-end
-_M.init_etcd = init_etcd
-
-local openresty_args = [[openresty  -p ]] .. apisix_home .. [[ -c ]]
-                       .. apisix_home .. [[/conf/nginx.conf]]
-
-function _M.start(...)
-    -- check running
-    local pid_path = apisix_home .. "/logs/nginx.pid"
-    local pid, err = read_file(pid_path)
-    if pid then
-        local hd = io.popen("lsof -p " .. pid)
-        local res = hd:read("*a")
-        if res and res ~= "" then
-            print("APISIX is running...")
-            return nil
-        end
-    end
-
-    init(...)
-    init_etcd(...)
-
-    local cmd = openresty_args
-    -- print(cmd)
-    os.execute(cmd)
-end
-
-function _M.stop()
-    local cmd = openresty_args .. [[ -s stop]]
-    -- print(cmd)
-    os.execute(cmd)
-end
-
-function _M.restart()
-  _M.stop()
-  _M.start()
+local function version()
+    print(ver['VERSION'])
 end
 
-function _M.reload()
-    -- reinit nginx.conf
-    init()
-
-    local test_cmd = 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 = os.execute((test_cmd))
-    if (test_ret == 0 or test_ret == true) then
-        local cmd = openresty_args .. [[ -s reload]]
-        -- print(cmd)
-        os.execute(cmd)
-        return
-    end
-    print("test openresty failed")
-end
 
-function _M.version()
-    local ver = require("apisix.core.version")
-    print(ver['VERSION'])
-end
+local command = {
+    version = version,
+    help = help,
+    init = ops.init,
+    init_etcd = etcd.init,
+    start = ops.start,
+    stop = ops.stop,
+    restart = ops.restart,
+    reload = ops.reload,
+}
 
-local cmd_action = arg[1]
-if not cmd_action then
-    return _M.help()
+local action = arg[1]
+if not action then
+    return help()
 end
 
-if not _M[cmd_action] then
-    print("invalid argument: ", cmd_action, "\n")
+if not command[action] then
+    util.die("unknown sub-command: ", action, "\n")

Review comment:
       OK, already did it.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers commented on a change in pull request #2395: improve: refactor apisix command line tool

Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #2395:
URL: https://github.com/apache/apisix/pull/2395#discussion_r509835158



##########
File path: rockspec/apisix-master-0.rockspec
##########
@@ -20,8 +20,8 @@ version = "master-0"
 supported_platforms = {"linux", "macosx"}
 
 source = {
-    url = "git://github.com/apache/apisix",
-    branch = "master",
+    url = "git://github.com/tokers/apisix",
+    branch = "improve/cmd_refactoring",

Review comment:
       The `linux_apisix_master_luarocks_runner.sh` always fetches the master branch of APISIX which doesn't contains the `apisix/cmd` directory and causes the failure of CI, it's just a temporary change and will be reset before merging into master.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] spacewander commented on a change in pull request #2395: improve: refactor apisix command line tool

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #2395:
URL: https://github.com/apache/apisix/pull/2395#discussion_r514094203



##########
File path: apisix/cli/env.lua
##########
@@ -0,0 +1,77 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+local util = require("apisix.cli.util")
+
+local execute_cmd   = util.execute_cmd
+local trim          = util.trim
+local pcall         = pcall
+local error         = error
+local str_match     = string.match
+local pkg_cpath_org = package.cpath
+local pkg_path_org  = package.path
+
+local apisix_home   = "/usr/local/apisix"
+local pkg_cpath = apisix_home .. "/deps/lib64/lua/5.1/?.so;"
+                  .. apisix_home .. "/deps/lib/lua/5.1/?.so;;"
+local pkg_path  = apisix_home .. "/deps/share/lua/5.1/?.lua;;"
+
+
+-- only for developer, use current folder as working space
+local is_root_path = false
+local script_path = arg[0]
+if script_path:sub(1, 2) == './' then
+    apisix_home = trim(execute_cmd("pwd"))
+    if not apisix_home then
+        error("failed to fetch current path")
+    end
+
+    if str_match(apisix_home .. "/", '^/root/') then

Review comment:
       We can use plain `string.find` like this issue suggested: https://github.com/apache/apisix/issues/1268




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers edited a comment on pull request #2395: improve: refactor apisix command line tool

Posted by GitBox <gi...@apache.org>.
tokers edited a comment on pull request #2395:
URL: https://github.com/apache/apisix/pull/2395#issuecomment-717211974


   I am still debugging this problem, it's seems the `plugin_metadata` cannot be fetched in the test case 10 (plugin-metadata.t) 😂


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers commented on pull request #2395: improve: refactor apisix command line tool

Posted by GitBox <gi...@apache.org>.
tokers commented on pull request #2395:
URL: https://github.com/apache/apisix/pull/2395#issuecomment-714461021


   @membphis I'm stucking in this weird CI failed problem, it may cost me some time to solve this.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] tokers edited a comment on pull request #2395: improve: refactor apisix command line tool

Posted by GitBox <gi...@apache.org>.
tokers edited a comment on pull request #2395:
URL: https://github.com/apache/apisix/pull/2395#issuecomment-714461021


   @membphis I was stucked in this weird CI failed problem, it may cost me some time to solve this.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] membphis commented on pull request #2395: improve: refactor apisix command line tool

Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2395:
URL: https://github.com/apache/apisix/pull/2395#issuecomment-710051556


   @tokers please rebase your branch


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [apisix] membphis commented on pull request #2395: improve: refactor apisix command line tool

Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2395:
URL: https://github.com/apache/apisix/pull/2395#issuecomment-714854804


   > I was stucked in this weird CI failed problem, it may cost me some time to solve this.
   
   Do not worry about this, we will help you  ^_^


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org