Copilot commented on code in PR #12906:
URL: https://github.com/apache/apisix/pull/12906#discussion_r2692749042
##########
apisix/discovery/eureka/init.lua:
##########
@@ -192,6 +202,9 @@ local function fetch_full_registry(premature)
end
end
applications = up_apps
+ log.info("successfully updated service registry, services count=",
+ core.table.nkeys(up_apps), "; source=",
+ selected_endpoint and selected_endpoint.url or "unknown")
Review Comment:
The condition `selected_endpoint and selected_endpoint.url or 'unknown'` is
redundant since this log line is only reached when `selected_endpoint` is not
nil (line 170 returns early if nil). Simplify to just `selected_endpoint.url`.
```suggestion
selected_endpoint.url)
```
##########
apisix/discovery/eureka/init.lua:
##########
@@ -210,6 +223,9 @@ function _M.init_worker()
log.info("default_weight:", default_weight, ".")
local fetch_interval = local_conf.discovery.eureka.fetch_interval or 30
log.info("fetch_interval:", fetch_interval, ".")
+
+ endpoints = build_endpoints()
Review Comment:
If `build_endpoints()` returns `nil` due to missing configuration, the
timers will still be started, causing them to fail silently on every execution.
Consider adding error handling to prevent timer initialization when endpoints
cannot be built, similar to how other discovery services handle init failures.
```suggestion
endpoints = build_endpoints()
if not endpoints or #endpoints == 0 then
log.error("failed to init eureka discovery: no valid endpoints " ..
"could be built, please check discovery.eureka
configuration")
return
end
```
##########
t/discovery/eureka.t:
##########
@@ -115,3 +115,53 @@ default_weight:80.
fetch_interval:10.
eureka uri:http://127.0.0.1:8761/eureka/.
connect_timeout:1500, send_timeout:1500, read_timeout:1500.
+
+
+
+=== TEST 4: fallback to next eureka host when current host fails
+--- yaml_config
+apisix:
+ node_listen: 1984
+deployment:
+ role: data_plane
+ role_data_plane:
+ config_provider: yaml
+discovery:
+ eureka:
+ host:
+ - "http://127.0.0.1:20997";
+ - "http://127.0.0.1:8761";
+ prefix: "/eureka/"
+ fetch_interval: 1
+ weight: 80
+ timeout:
+ connect: 1500
+ send: 1500
+ read: 1500
+--- apisix_yaml
+routes:
+ -
+ uri: /eureka/*
+ upstream:
+ service_name: APISIX-EUREKA
+ discovery_type: eureka
+ type: roundrobin
+#END
+--- http_config
+ server {
+ listen 20997;
+
+ location / {
+ return 502;
+ }
+ }
+--- request
+GET /eureka/apps/APISIX-EUREKA
+--- response_body_like
+.*<name>APISIX-EUREKA</name>.*
Review Comment:
The test verifies the response body but doesn't verify that the request was
actually routed through the working endpoint (127.0.0.1:8761) rather than the
failing one (127.0.0.1:20997). Consider adding an assertion to check that the
upstream actually received the request.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
