Skip to content

feat(stream): support stream subsystem discovery#4488

Closed
zhixiongdu027 wants to merge 5 commits into
apache:masterfrom
zhixiongdu027:master
Closed

feat(stream): support stream subsystem discovery#4488
zhixiongdu027 wants to merge 5 commits into
apache:masterfrom
zhixiongdu027:master

Conversation

@zhixiongdu027

@zhixiongdu027 zhixiongdu027 commented Jun 26, 2021

Copy link
Copy Markdown
Contributor

Signed-off-by: adugeek [email protected]

What this PR does / why we need it:

resolve: #4469

int function init.lua:stream_init_worker() not call discovery.init_worker(),
so, we can't use discovery in stream subsystem.
by this pr, we can slove it
of course, the current and future discovery components need to ensure they can work in the stream subsystem

Pre-submission checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

@tokers

tokers commented Jun 27, 2021

Copy link
Copy Markdown
Contributor

@adugeek Please add some test cases to verify the changes are effective.

Comment thread apisix/init.lua
local discovery = require("apisix.discovery.init").discovery
if discovery and discovery.init_worker then
discovery.init_worker()
end

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can add a test with the DNS discovery:

=== TEST 2: A

You can setup it with

if [ ! -f "build-cache/coredns_1_8_1" ]; then

Comment thread t/discovery/dns/sanity.t Outdated
--- stream_response
hello world
--- no_error_log
[error] No newline at end of file

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should add the EOS symbol. Please check your editor.

@tokers

tokers commented Jul 5, 2021

Copy link
Copy Markdown
Contributor

@adugeek Please check out the CI status.

@zhixiongdu027

Copy link
Copy Markdown
Contributor Author

@tokers sorry, busying in the past week.
and it takes a little time to learn "how to write test case"

Comment thread t/discovery/dns/sanity.t Outdated
@Yiyiyimu

Yiyiyimu commented Jul 8, 2021

Copy link
Copy Markdown
Member

@adugeek CI failed, please recheck

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add discovery.init_worker() call in init.lua.stream_init_worker()

4 participants