feat: stream subsystem support kubernetes service discovery #8640

ronething · 2023-01-09T13:30:48Z

Description

because of kubernetes service discovery use lua_shared_dict, so i add discovery_shared_dicts with stream suffix to stream subsystem in ngx_tpl.lua.

for test case, i use extra_stream_config to get nodes and return response.

because of openresty version 1.19.3.2 do not implemented ngx.process API for stream subsystem, so ci failed. see https://github.com/apache/apisix/actions/runs/3874260736/jobs/6605244559

i change openresty version from 1.19.3.2 to 1.19.9.1 to solve this problem, refer: https://openresty.org/en/changelog-1019009.html

Checklist

I have explained the need for this PR and the problem it solves
I have explained the changes or the new features added to this PR
I have added tests corresponding to this change
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 first)

spacewander · 2023-01-10T06:48:59Z

apisix/discovery/kubernetes/init.lua

@@ -434,6 +442,9 @@ local function multiple_mode_init(confs)
        end

        local endpoint_dict = ngx.shared["kubernetes-" .. id]
+        if not is_http then
+            endpoint_dict = ngx.shared["kubernetes-" .. id .. "-stream"]
+        end


It would be better to refactor these three changes into a function, instead of three if-else blocks.

spacewander · 2023-01-10T06:52:33Z

ci/linux_openresty_1_19_runner.sh

@@ -17,5 +17,5 @@
 #


-export OPENRESTY_VERSION=1.19.3.2
+export OPENRESTY_VERSION=1.19.9.1


We have promised to support the latest three versions of OpenResty, so we still need to cover 1.19.3.2.
What about updating the test case and only running it when the version is matched, like https://github.com/apache/apisix/blob/release/2.14/t/plugin/ext-plugin/sanity-openresty-1-19.t?

It would be better to refuse to start when the OpenResty is too old and k8s discovery is used in the stream.

We have promised to support the latest three versions of OpenResty, so we still need to cover 1.19.3.2.
What about updating the test case and only running it when the version is matched, like https://github.com/apache/apisix/blob/release/2.14/t/plugin/ext-plugin/sanity-openresty-1-19.t?

ok, i will try it later.

It would be better to refuse to start when the OpenResty is too old and k8s discovery is used in the stream.

Is it a good choice if we add a boolean field like enable_stream(default is false) into apisix/discovery/kubernetes/schema.lua as a property and check OpenResty version when enable_stream is true?

Can you give me some hints? thanks.

something like

apisix/apisix/upstream.lua

Line 425 in 84535af

if conf.pass_host == "node" and conf.nodes and

@spacewander

first, it will cause err like below when start apisix with openresty 1.19.3.2, because local discovery = require("apisix.discovery.init").discovery in apisix/upstream.lua:19 will import discovery/kubernetes/init.lua.

$ ./bin/apisix start /usr/local/openresty-debug//luajit/bin/luajit ./apisix/cli/apisix.lua start nginx: [error] init_by_lua error: /usr/local/openresty-debug/lualib/ngx/process.lua:5: unsupported subsystem: stream stack traceback: [C]: in function 'error' /usr/local/openresty-debug/lualib/resty/core/base.lua:223: in function 'allows_subsystem' /usr/local/openresty-debug/lualib/ngx/process.lua:5: in main chunk [C]: in function 'require' ...ng/Documents/apisix/apisix/discovery/kubernetes/init.lua:29: in main chunk [C]: in function 'require' /home/ronething/Documents/apisix/apisix/discovery/init.lua:28: in main chunk [C]: in function 'require' /home/ronething/Documents/apisix/apisix/upstream.lua:19: in main chunk [C]: in function 'require' /home/ronething/Documents/apisix/apisix/http/service.lua:18: in main chunk [C]: in function 'require' /home/ronething/Documents/apisix/apisix/init.lua:35: in main chunk [C]: in function 'require' init_by_lua:3: in main chunk

then, something like if conf.pass_host == "node" and conf.nodes and in check_upstream_conf function may only print error log when check upstream if in_dp is true, or cause err when create/modify upstream or other resource which need to check upstream_conf both in http and stream subsystem. But we need to refuse to start apisix when the OpenResty is too old and k8s discovery is used in the stream?

so maybe we should consider other solutions? if something i was wrong, please tell me, thanks.

But we need to refuse to start apisix when the OpenResty is too old and k8s discovery is used in the stream?

Yes. Unlike the upstream conf which is a runtime conf, the wrong discovery conf in the yaml can't be corrected without restarting APISIX.

And we need to provide a clear error message so people don't need to debug it.

spacewander · 2023-01-12T05:55:06Z

apisix/discovery/kubernetes/init.lua

@@ -331,9 +332,18 @@ local function start_fetch(handle)
    ngx.timer.at(0, timer_runner)
 end

+local function get_endpoint_dict(shm)


We can pass the id as the argument, and put the constant kubernetes in this function? There is no need to repeat it.

…_discovery_kubernetes

spacewander · 2023-01-16T06:30:07Z

apisix/discovery/kubernetes/init.lua

@@ -520,6 +520,12 @@ end


 function _M.init_worker()
+    if not support_process then
+        core.log.error("kubernetes discovery not support in subsystem: "
+                       .. ngx.config.subsystem


We can use ',' here

@spacewander do you mean something like below? .. ngx.config.subsystem .. ", "

if not support_process then core.log.error("kubernetes discovery not support in subsystem: " .. ngx.config.subsystem .. ", " .. "please check if your openresty version >= 1.19.9.1 or not") return end

Err. What I mean is that we can use ',' instead of '..'.

ok, i see. I will change it later.

ronething · 2023-01-18T03:49:52Z

@tokers @soulbird Could you please take a look, thanks.

* upstream/master: (67 commits) fix: grpc-transcode plugin: fix map data population (apache#8731) change(jwt-auth): unify apisix/core/vault.lua and apisix/secret/vault.lua (apache#8660) feat: stream subsystem support consul_kv service discovery (apache#8633) fix(proxy-mirror): use with uri rewrite (apache#8718) ci: move helper script to the right dir (apache#8691) refactor(pubsub): simpify the get_cmd implementation (apache#8608) feat: stream subsystem support kubernetes service discovery (apache#8640) docs: fix deployment links (apache#8714) fix: remove backslash before slash when encoding (apache#8684) ci: kafka should register port in the zookeeper same as exposed (apache#8672) docs: fix plugin config naming (apache#8701) docs: fix code block (apache#8700) docs: rename kms to secret (apache#8697) docs: replace transparent logos with white background logos (apache#8689) fix: upgrade lua-resty-etcd to 1.10.3 (apache#8668) fix: upgrade casbin to 1.41.3 to improve performance (apache#8676) chore: make send_stream_request more clear (apache#8627) feat: stream subsystem support nacos service discovery (apache#8584) feat: stream subsystem support dns service discovery (apache#8593) refactor(admin): refactor resource routes (apache#8611) ...

ronething added 4 commits January 9, 2023 18:40

feat: stream subsystem support kubernetes service discovery

5e115f4

feat: test cases

8c0f345

fix: ci

66079cd

chore: remove code line

34f37b8

ronething marked this pull request as ready for review January 10, 2023 04:08

spacewander reviewed Jan 10, 2023

View reviewed changes

fix: test case

cafcbfd

spacewander reviewed Jan 12, 2023

View reviewed changes

ronething added 6 commits January 12, 2023 14:15

fix: review

54473dc

fix: print error log if not support ngx.process

3501d50

docs: add note

ffb80e8

Merge remote-tracking branch 'upstream/master' into feat/stream_route…

fd31cb3

…_discovery_kubernetes

fix: ci

2c74653

fix: ci

336149e

ronething requested a review from spacewander January 16, 2023 03:01

spacewander reviewed Jan 16, 2023

View reviewed changes

fix: review

4d875a3

ronething requested a review from spacewander January 17, 2023 08:47

spacewander approved these changes Jan 18, 2023

View reviewed changes

soulbird approved these changes Jan 18, 2023

View reviewed changes

starsz approved these changes Jan 20, 2023

View reviewed changes

spacewander merged commit 9c899b8 into apache:master Jan 28, 2023

ronething deleted the feat/stream_route_discovery_kubernetes branch January 28, 2023 08:29

ronething mentioned this pull request Feb 10, 2023

feat: stream subsystem support tars service discovery #8826

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: stream subsystem support kubernetes service discovery #8640

feat: stream subsystem support kubernetes service discovery #8640

ronething commented Jan 9, 2023 •

edited

Loading

spacewander Jan 10, 2023

spacewander Jan 10, 2023

ronething Jan 10, 2023 •

edited

Loading

spacewander Jan 12, 2023

ronething Jan 12, 2023 •

edited

Loading

spacewander Jan 13, 2023

spacewander Jan 12, 2023

spacewander Jan 16, 2023

ronething Jan 16, 2023

spacewander Jan 17, 2023

ronething Jan 17, 2023

ronething commented Jan 18, 2023

feat: stream subsystem support kubernetes service discovery #8640

feat: stream subsystem support kubernetes service discovery #8640

Conversation

ronething commented Jan 9, 2023 • edited Loading

Description

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ronething Jan 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ronething Jan 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ronething commented Jan 18, 2023

ronething commented Jan 9, 2023 •

edited

Loading

ronething Jan 10, 2023 •

edited

Loading

ronething Jan 12, 2023 •

edited

Loading