Re: [PR] fix: enable issue of endpointslices for k8s discovery [apisix]
Baoyuantop merged PR #11654: URL: https://github.com/apache/apisix/pull/11654 -- 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]
Re: [PR] fix: enable issue of endpointslices for k8s discovery [apisix]
ChuanFF commented on code in PR #11654:
URL: https://github.com/apache/apisix/pull/11654#discussion_r2276483415
##
apisix/discovery/kubernetes/init.lua:
##
@@ -60,32 +61,34 @@ local function on_endpoint_slices_modified(handle, endpoint)
core.table.clear(endpoint_buffer)
local endpointslices = endpoint.endpoints
-for _, endpointslice in ipairs(endpointslices or {}) do
-if endpointslice.addresses then
-local addresses = endpointslices.addresses
-for _, port in ipairs(endpoint.ports or {}) do
-local port_name
-if port.name then
-port_name = port.name
-elseif port.targetPort then
-port_name = tostring(port.targetPort)
-else
-port_name = tostring(port.port)
-end
-
-if endpointslice.conditions and endpointslice.condition.ready
then
-local nodes = endpoint_buffer[port_name]
-if nodes == nil then
-nodes = core.table.new(0, #endpointslices * #addresses)
-endpoint_buffer[port_name] = nodes
+if type(endpointslices) == "table" then
Review Comment:
it may be ngx.null
--
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]
Re: [PR] fix: enable issue of endpointslices for k8s discovery [apisix]
Baoyuantop commented on code in PR #11654:
URL: https://github.com/apache/apisix/pull/11654#discussion_r2275570708
##
apisix/discovery/kubernetes/init.lua:
##
@@ -60,32 +61,34 @@ local function on_endpoint_slices_modified(handle, endpoint)
core.table.clear(endpoint_buffer)
local endpointslices = endpoint.endpoints
-for _, endpointslice in ipairs(endpointslices or {}) do
-if endpointslice.addresses then
-local addresses = endpointslices.addresses
-for _, port in ipairs(endpoint.ports or {}) do
-local port_name
-if port.name then
-port_name = port.name
-elseif port.targetPort then
-port_name = tostring(port.targetPort)
-else
-port_name = tostring(port.port)
-end
-
-if endpointslice.conditions and endpointslice.condition.ready
then
-local nodes = endpoint_buffer[port_name]
-if nodes == nil then
-nodes = core.table.new(0, #endpointslices * #addresses)
-endpoint_buffer[port_name] = nodes
+if type(endpointslices) == "table" then
Review Comment:
Hi @slayer321, can you explain 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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] fix: enable issue of endpointslices for k8s discovery [apisix]
AlinsRan commented on code in PR #11654:
URL: https://github.com/apache/apisix/pull/11654#discussion_r2272213874
##
apisix/discovery/kubernetes/init.lua:
##
@@ -60,32 +61,34 @@ local function on_endpoint_slices_modified(handle, endpoint)
core.table.clear(endpoint_buffer)
local endpointslices = endpoint.endpoints
-for _, endpointslice in ipairs(endpointslices or {}) do
-if endpointslice.addresses then
-local addresses = endpointslices.addresses
-for _, port in ipairs(endpoint.ports or {}) do
-local port_name
-if port.name then
-port_name = port.name
-elseif port.targetPort then
-port_name = tostring(port.targetPort)
-else
-port_name = tostring(port.port)
-end
-
-if endpointslice.conditions and endpointslice.condition.ready
then
-local nodes = endpoint_buffer[port_name]
-if nodes == nil then
-nodes = core.table.new(0, #endpointslices * #addresses)
-endpoint_buffer[port_name] = nodes
+if type(endpointslices) == "table" then
Review Comment:
Why add this extra judgment? Is there any problem with the previous handling
of `endpointslices or {}`?
--
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]
Re: [PR] fix: enable issue of endpointslices for k8s discovery [apisix]
Baoyuantop commented on PR #11654: URL: https://github.com/apache/apisix/pull/11654#issuecomment-3167150320 > Hey @Baoyuantop , should we merge the PR . I will remind other maintainers again to review it, one more approval is needed. -- 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]
Re: [PR] fix: enable issue of endpointslices for k8s discovery [apisix]
slayer321 commented on PR #11654: URL: https://github.com/apache/apisix/pull/11654#issuecomment-3166543879 Hey @Baoyuantop , should we merge the 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix: enable issue of endpointslices for k8s discovery [apisix]
slayer321 commented on code in PR #11654: URL: https://github.com/apache/apisix/pull/11654#discussion_r697355 ## t/kubernetes/discovery/kubernetes3.t: ## @@ -165,19 +165,19 @@ _EOC_ if op.op == "replace_endpointslices" then method = "PATCH" -path = "/apis/discovery.k8s.io/namespaces/" .. op.namespace .. "/endpointslices/" .. op.name +path = "/apis/discovery.k8s.io/v1/namespaces/" .. op.namespace .. "/endpointslices/" .. op.name Review Comment: As this is the standard way to use the [k8s API](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#endpointslice-v1-discovery-k8s-io) . -- 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]
Re: [PR] fix: enable issue of endpointslices for k8s discovery [apisix]
slayer321 commented on code in PR #11654: URL: https://github.com/apache/apisix/pull/11654#discussion_r697355 ## t/kubernetes/discovery/kubernetes3.t: ## @@ -165,19 +165,19 @@ _EOC_ if op.op == "replace_endpointslices" then method = "PATCH" -path = "/apis/discovery.k8s.io/namespaces/" .. op.namespace .. "/endpointslices/" .. op.name +path = "/apis/discovery.k8s.io/v1/namespaces/" .. op.namespace .. "/endpointslices/" .. op.name Review Comment: As this is the standard way to use the [k8s API](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#endpointslice-v1-discovery-k8s-io) . This change was made. -- 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]
Re: [PR] fix: enable issue of endpointslices for k8s discovery [apisix]
Baoyuantop commented on code in PR #11654: URL: https://github.com/apache/apisix/pull/11654#discussion_r2220873336 ## t/kubernetes/discovery/kubernetes3.t: ## @@ -165,19 +165,19 @@ _EOC_ if op.op == "replace_endpointslices" then method = "PATCH" -path = "/apis/discovery.k8s.io/namespaces/" .. op.namespace .. "/endpointslices/" .. op.name +path = "/apis/discovery.k8s.io/v1/namespaces/" .. op.namespace .. "/endpointslices/" .. op.name Review Comment: Can you explain why these changes were made? ## docs/zh/latest/discovery/kubernetes.md: ## @@ -300,7 +300,7 @@ A: Kubernetes 服务发现只使用特权进程监听 Kubernetes Endpoints，然 **Q: [_ServiceAccount_](https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/) 需要的权限有哪些？** -A: [_ServiceAccount_](https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/) 需要集群级 [ get,list,watch ] endpoints 资源的的权限，其声明式定义如下： +A: [_ServiceAccount_](https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/) 需要集群级 [ get,list,watch ] endpoints and endpointslices 资源的的权限，其声明式定义如下： Review Comment: ```suggestion A: [_ServiceAccount_](https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/) 需要集群级 [ get,list,watch ] endpoints 和 endpointslices 资源的的权限，其声明式定义如下： ``` -- 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]
Re: [PR] fix: enable issue of endpointslices for k8s discovery [apisix]
slayer321 commented on code in PR #11654: URL: https://github.com/apache/apisix/pull/11654#discussion_r2217695246 ## t/kubernetes/discovery/kubernetes3.t: ## @@ -381,6 +381,8 @@ POST /operators Content-type: application/json --- response_body_like .*"name":"default/kubernetes".* +--- error-log +attempt to get length of local 'endpointslices' (a userdata value) Review Comment: It's been around a year now since I raised this PR. I also don't remember the exact reason I added the error log. I have removed it for now and tested it locally ..test if passing. ## t/kubernetes/discovery/kubernetes3.t: ## @@ -381,6 +381,8 @@ POST /operators Content-type: application/json --- response_body_like .*"name":"default/kubernetes".* +--- error-log +attempt to get length of local 'endpointslices' (a userdata value) Review Comment: It's been around a year now since I raised this PR. I also don't remember the exact reason I added the error log. I have removed it for now and tested it locally ..test is passing. -- 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]
Re: [PR] fix: enable issue of endpointslices for k8s discovery [apisix]
Baoyuantop commented on code in PR #11654: URL: https://github.com/apache/apisix/pull/11654#discussion_r2206800215 ## t/kubernetes/discovery/kubernetes3.t: ## @@ -381,6 +381,8 @@ POST /operators Content-type: application/json --- response_body_like .*"name":"default/kubernetes".* +--- error-log +attempt to get length of local 'endpointslices' (a userdata value) Review Comment: I don't quite understand why this assertion needs to be added here. -- 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]
Re: [PR] fix: enable issue of endpointslices for k8s discovery [apisix]
Baoyuantop commented on code in PR #11654: URL: https://github.com/apache/apisix/pull/11654#discussion_r2179161815 ## t/kubernetes/discovery/kubernetes3.t: ## @@ -381,21 +381,22 @@ POST /operators Content-type: application/json --- response_body_like .*"name":"default/kubernetes".* - +--- error-log +attempt to get length of local 'endpointslices' (a userdata value) Review Comment: ```suggestion attempt to get length of local 'endpointslices' (a userdata value) ``` -- 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]
Re: [PR] fix: enable issue of endpointslices for k8s discovery [apisix]
slayer321 commented on PR #11654: URL: https://github.com/apache/apisix/pull/11654#issuecomment-3026420654 Hey @Baoyuantop , the [linting error](https://github.com/apache/apisix/actions/runs/15989732289/job/45104853361?pr=11654) that I coming . I tried running it locally but it is showing skipped ``` ╰─ ./reindex ../t/kubernetes/discovery/kubernetes3.t reindex: ../t/kubernetes/discovery/kubernetes3.t:skipped. ``` Also another [CI build](https://github.com/apache/apisix/actions/runs/15989732308/job/45104853757?pr=11654) that is failing I'm not sure the reason for that. I thought of re running the CI.. but I see that I can't rerun 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix: enable issue of endpointslices for k8s discovery [apisix]
Baoyuantop commented on PR #11654: URL: https://github.com/apache/apisix/pull/11654#issuecomment-3006913648 Hi @slayer321, there is a failed CI that needs to be 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix: enable issue of endpointslices for k8s discovery [apisix]
Baoyuantop commented on PR #11654: URL: https://github.com/apache/apisix/pull/11654#issuecomment-2995500130 There are some code lint errors. -- 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]
Re: [PR] fix: enable issue of endpointslices for k8s discovery [apisix]
slayer321 commented on PR #11654: URL: https://github.com/apache/apisix/pull/11654#issuecomment-2994954952 Hi @Baoyuantop , I have fixed CI changes and pushed 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix: enable issue of endpointslices for k8s discovery [apisix]
Baoyuantop commented on PR #11654: URL: https://github.com/apache/apisix/pull/11654#issuecomment-2994753985 Hi @slayer321, can you fix the failed CI? -- 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]
Re: [PR] fix: enable issue of endpointslices for k8s discovery [apisix]
slayer321 commented on PR #11654: URL: https://github.com/apache/apisix/pull/11654#issuecomment-2990999492 Hey @Baoyuantop , I have rebase with master but I think to run CI it needs some approval. -- 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]
Re: [PR] fix: enable issue of endpointslices for k8s discovery [apisix]
Baoyuantop commented on PR #11654: URL: https://github.com/apache/apisix/pull/11654#issuecomment-2990224874 Hi @slayer321, I reopened this pull request. Can you merge the master branch and re-execute this CI? -- 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]
[PR] fix: enable issue of endpointslices for k8s discovery [apisix]
slayer321 opened a new pull request, #11654: URL: https://github.com/apache/apisix/pull/11654 ### Description use `conf.watch_endpoint_slices` to enable endpointslices whenever it is set to true. Fixed the test. Fixes #11631 ### Checklist - [x] I have explained the need for this PR and the problem it solves - [x] I have explained the changes or the new features added to this PR - [x] I have added tests corresponding to this change - [x] I have updated the documentation to reflect this change - [ ] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix: enable issue of endpointslices for k8s discovery [apisix]
github-actions[bot] closed pull request #11654: fix: enable issue of endpointslices for k8s discovery URL: https://github.com/apache/apisix/pull/11654 -- 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]
Re: [PR] fix: enable issue of endpointslices for k8s discovery [apisix]
github-actions[bot] commented on PR #11654: URL: https://github.com/apache/apisix/pull/11654#issuecomment-2626819943 This pull request/issue has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] fix: enable issue of endpointslices for k8s discovery [apisix]
github-actions[bot] commented on PR #11654: URL: https://github.com/apache/apisix/pull/11654#issuecomment-2567528290 This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 4 weeks if no further activity occurs. If you think that's incorrect or this pull request should instead be reviewed, please simply write any comment. Even if closed, you can still revive the PR at any time or discuss it on the [email protected] list. Thank you for your contributions. -- 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]
[PR] fix: enable issue of endpointslices for k8s discovery [apisix]
slayer321 opened a new pull request, #11654: URL: https://github.com/apache/apisix/pull/11654 ### Description use `conf.watch_endpoint_slices` to enable endpointslices whenever it is set to true. Fixed the test. Fixes #11631 ### Checklist - [x] I have explained the need for this PR and the problem it solves - [x] I have explained the changes or the new features added to this PR - [x] I have added tests corresponding to this change - [x] I have updated the documentation to reflect this change - [ ] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
