bzp2010 commented on code in PR #12200:
URL: https://github.com/apache/apisix/pull/12200#discussion_r2097023697


##########
apisix/core/config_etcd.lua:
##########
@@ -65,9 +68,7 @@ local err_etcd_grpc_engine_timeout = "context deadline 
exceeded"
 local err_etcd_grpc_ngx_timeout = "timeout"
 local err_etcd_unhealthy_all = "has no healthy etcd endpoint available"
 local health_check_shm_name = "etcd-cluster-health-check"
-if not is_http then
-    health_check_shm_name = health_check_shm_name .. "-stream"
-end

Review Comment:
   Old code in config etcd should not be modified. Just add our new code.



##########
apisix/cli/ngx_tpl.lua:
##########
@@ -70,6 +70,7 @@ lua {
     {% if standalone_with_admin_api then %}
     lua_shared_dict standalone-config {* 
meta.lua_shared_dict["standalone-config"] *};
     {% end %}
+    lua_shared_dict status-report {* meta.lua_shared_dict["status-report"] *};

Review Comment:
   ```suggestion
       {% if status then %}
       lua_shared_dict status-report {* meta.lua_shared_dict["status-report"] 
*};
       {% end %}
   ```



##########
conf/config.yaml.example:
##########
@@ -132,7 +132,10 @@ apisix:
   events:                             # Event distribution module configuration
     module: lua-resty-events          # Sets the name of the events module 
used.
                                       # Supported module: 
lua-resty-worker-events and lua-resty-events
-
+# status:                       # When enabled, APISIX will provide `/status` 
and `/status/ready` endpoints
+  #   ip: 127.0.0.1               # /status endpoint will return 200 status 
code if APISIX has successfully started and running correctly
+  #   port: 7085                  # /status/ready endpoint will return 503 
status code if none of the workers received data from etcd

Review Comment:
   ```suggestion
     #   port: 7085                  # /status/ready endpoint will return 503 
status code if any of the workers do not receive config from etcd
   ```



##########
apisix/init.lua:
##########
@@ -868,6 +869,56 @@ local function healthcheck_passive(api_ctx)
 end
 
 
+function _M.status()
+    core.response.exit(200, core.json.encode({
+        status = "ok" }),
+        { ["Content-Type"] = "application/json"
+    })
+end
+
+function _M.status_ready()
+    local local_conf = core.config.local_conf()
+    local provider = core.table.try_read_attr(local_conf, "deployment",
+                                              "role_traditional", 
"config_provider") or
+                    core.table.try_read_attr(local_conf, "deployment",
+                                              "role_data_plane", 
"config_provider")
+    if provider == "yaml" or provider == "etcd" then
+        local status_shdict = ngx.shared["status-report"]
+        local pids = status_shdict:get_keys()
+        local errors = {}
+
+        for _, pid in ipairs(pids) do
+            local ready = status_shdict:get(pid)
+            if not ready then
+                core.log.warn("worker pid: ", pid, " has not received 
configuration")
+                core.table.insert(errors, "worker pid: " .. pid ..
+                                  " has not received configuration")
+                break
+            end
+        end

Review Comment:
   The pid does not apply to this scenario. Please use worker ids consistently 
and check if all workers are reported as healthy with `ngx.worker.count`.



##########
apisix/core/config_yaml.lua:
##########
@@ -73,6 +75,16 @@ local apisix_yaml
 local apisix_yaml_raw -- save a deepcopy of the latest configuration for API
 local apisix_yaml_mtime
 
+local function sync_status_to_shdict(status)
+    if process.type() ~= "worker" then
+        return
+    end
+    local status_shdict = ngx.shared[status_report_shared_dict_name]
+    local pid = worker_id()

Review Comment:
   rename the `pid`



##########
apisix/core/config_etcd.lua:
##########
@@ -482,6 +483,20 @@ local function short_key(self, str)
 end
 
 
+local function sync_status_to_shdict(status)
+    local local_conf = config_local.local_conf()
+    if not local_conf.apisix.status then
+        return
+    end
+    if process.type() ~= "worker" then
+        return
+    end
+    local status_shdict = ngx.shared[status_report_shared_dict_name]
+    local pid = worker_pid()

Review Comment:
   Use `ngx.worker.id`? The pid does not apply to this scenario.



##########
conf/config.yaml.example:
##########
@@ -132,7 +132,10 @@ apisix:
   events:                             # Event distribution module configuration
     module: lua-resty-events          # Sets the name of the events module 
used.
                                       # Supported module: 
lua-resty-worker-events and lua-resty-events
-
+# status:                       # When enabled, APISIX will provide `/status` 
and `/status/ready` endpoints
+  #   ip: 127.0.0.1               # /status endpoint will return 200 status 
code if APISIX has successfully started and running correctly
+  #   port: 7085                  # /status/ready endpoint will return 503 
status code if none of the workers received data from etcd
+                                  # or (standalone mode) the config isn't 
loaded yet either via file or ADMIN API.

Review Comment:
   ```suggestion
                                     # or (standalone mode) the config isn't 
loaded yet either via file or Admin API.
   ```



##########
apisix/init.lua:
##########
@@ -868,6 +869,56 @@ local function healthcheck_passive(api_ctx)
 end
 
 
+function _M.status()
+    core.response.exit(200, core.json.encode({
+        status = "ok" }),
+        { ["Content-Type"] = "application/json"
+    })
+end
+
+function _M.status_ready()
+    local local_conf = core.config.local_conf()
+    local provider = core.table.try_read_attr(local_conf, "deployment",
+                                              "role_traditional", 
"config_provider") or
+                    core.table.try_read_attr(local_conf, "deployment",
+                                              "role_data_plane", 
"config_provider")

Review Comment:
   ```
   local role = core.table.try_read_attr(local_conf, "deployment", "role")
   local provider = core.table.try_read_attr(local_conf, "deployment",
                                                 "role_" .. role, 
"config_provider")
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscr...@apisix.apache.org

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

Reply via email to