feat: add standalone admin api#12179
Conversation
f4061c4 to
f150a56
Compare
dosubot
Bot
added
size:XL
| @@ -0,0 +1,16 @@ | |||
| { | |||
There was a problem hiding this comment.
how about t/ts/package.json?
move all of the ts related files to folder t/ts
There was a problem hiding this comment.
Reply in the next comment.
| }, | ||
| }; | ||
|
|
||
| describe("Admin - Standalone", () => { |
There was a problem hiding this comment.
this is a new way to write test cases
we can use test::nginx, got the same result, all right?
There was a problem hiding this comment.
This is insufficient for current testing, as the design does not share the configuration between workers in any way, but only broadcasts it through the event mechanism. Any restart of the worker may make the configuration disappear.
The test::nginx always uses reload or restarts the worker, which is bad. The current way is clear and runs entirely on a single instance, as it uses only one TEST section as a nutshell (which means the worker doesn't restart).
I know test::nginx allows no reload, but you can't modify the config because you don't want it to reload, and once you need to modify the config to add a new content_by_lua_block-like block, it has to reload.
This makes it impossible to accurately assert tests with inline lua code, which is detrimental to the tests I'm working on.
Why do I have to use Lua: I want to check a rather complex nested data structure, and regular expressions can't do it simply, or aren't readable. Not to mention the fact that cjson simply can't guarantee stable key ordering in JSON. So I wanted to implement it using Lua assert until I realized the above problem.
Another possible approach is to continue writing dirty shell scripts and treating them as cli tests, which requires us to check the response using similar external software such as jq or yq, which is also very bad. Very cumbersome and error prone.
So I prefer to write these tests in one of the most popular programming languages in the world, even typescript is far more popular to perl or shell, let alone javascript,
| JavaScript | TypeScript | Bash/Shell | Perl |
|---|---|---|---|
| 64.6% | 43.4% | 34.2% | 2.3% |
| The era of Perl is over. |
which allows us to take full advantage of the various libraries available for direct asserting, such as jest, yaml, or other libraries that are currently in use.
This gives more people the opportunity to write tests without any confuses, rather than having to deal with strange test cases mixed with all sorts of weird perl, test::nginx scripts that are both hard to learn and hard to write. As a professional developer on APISIX, I too often have headaches because of this test framework.
Further, this code won't be included in the release at all, it's just a few tests, which won't bother users in any way, and this won't lead to a runtime dependency on nodejs, so I think that's the best route to take.
So, in the long-term, I'd like these ts-implemented tests to be first-class citizens, not stuffed into some folder. As a scripting language, js/ts is not fundamentally different from perl, anything that can be done with test::nginx can be done with them, even fine-grained manipulation of the nginx process for restart or reload, but it's just more popular, more modern, more expressive, better ecologically, and it's time to embrace the future, if only for a little while.
There was a problem hiding this comment.
However, I do realize some additional issues, and I'm switching to shared dict to avoid the problem of not being able to resynchronize the configuration when one of the workers crashes.
There was a problem hiding this comment.
Moved to shared dict
There was a problem hiding this comment.
The test cases don't seem complicated, is there a reason to use JavaScript?
If we want to write new test cases, should we consider using test-nginx or JavaScript?
I am more inclined towards one of them, which is that introducing more language ecosystems actually increases complexity.
There was a problem hiding this comment.
Technically,
I've already explained the extent to which test::nginx doesn't meet my needs: the rigid perl DSL doesn't allow me to fine-tune the response data checker without introducing and learning new dependencies, and the use of content_by_lua blocks means that this forcefully couples the testing process with openresty's reload or restart, which would largely break the shared memory or event broadcast mechanism.
Instead of wasting time on things like Perl or test::nginx (which even implements an HTTP protocol parser of its own, why?), it's better to find another way out.
These tests can be based on any language, even with the efficient but primitive Go. Introducing new testing tools is all about solving ease-of-use and efficiency problems, which are not helped by using technologies with poor expressiveness (forced typing and barren standard libraries) and such a poor audience (two times as many professional developers use Java as use Go, three times as many professional developers use TypeScript as use Go, and four times as many professional developers use JavaScript as use Go).
If we want to write new test cases, should we consider using test-nginx or JavaScript?
There is no clear policy about it, and developers choose familiar technologies to complete their tests. This is not mandatory.
Right now, these new test sets don't have explicit internal APIs for things like modifying configurations, reloading, or restarting openresty, so they're lightweight and focused on testing APISIX for functional modifications using HTTP clients. Some of the more complex features can be introduced incrementally to allow us to implement more varied tests on nodejs until it is ready for the full range of professional developers' testing needs. At that point, we will consider stopping accept new tests using test::nginx.
As such, there is no risk of any immediate deprecation, and tests that already use test::nginx will continue to exist until they are rewritten more appropriately (if one so desires).
I am more inclined towards one of them, which is that introducing more language ecosystems actually increases complexity.
Continuing to invest in technologies that are outdated and hold no promise for development will cause us to accumulate more technical debt and get stuck in a quagmire.
The use of new technologies will bring efficiency gains and greatly increase the number of engineers capable of developing APISIX functionality, and TypeScript is a technology that stands at the right balance of efficiency, maintainability, and adoption.
I think the advantages will outweigh the disadvantages in terms of complexity, and the test framework will be implemented and maintained in the APISIX project so that users will only have to focus on how to write tests and not waste time exploring the boundaries of test::nginx functionality and debugging the odd glitch there (I've had enough of this).
Many features have more standard implementations, and the myriad of dependency libraries will support features that are not in the standard library. None of these are easily solved by Perl or test::nginx which is only a subset of the functionality, and with engineers being forced to learn it or quit APISIX development, I think the choice of which is simpler is obvious.
I don't want to try to solve these problems in-system, which have already been stomped into the dust by other technologies. I don't want to waste my time with technologies that have been buried up to their necks in dirt.
Although I can write code fluently consisting of test::nginx and solve any new problems I encounter. But I don't want to waste any more minutes on this mentally torturous ancient technology.
cc @membphis
Co-authored-by: Traky Deng <[email protected]>
nic-6443
left a comment
nic-6443
left a comment
There was a problem hiding this comment.
I have no issues with this PR itself, but there is a TODO regarding this feature: In the API-driven standalone mode, we need to provide a health status endpoint that returns whether the current APISIX instance has loaded the configuration. This is because in the previous etcd mode and file-based standalone mode, when APISIX exposes ports, it can ensure that all configurations have been loaded. However, in the newly added mode, if such a health status endpoint is not provided, APISIX may expose services while external programs have not yet requested /apisix/admin/configs to submit configurations.
This seems to be as it should be? The 9080 just returns 404 forever until the first configuration is committed. |
@bzp2010 |
|
I think there is also a need to list this mode as experimental in the documentation. to warn users not to blindly use it in production environments, which could be removed before it becomes a GA feature.
Given that there's still some time before the next release, I'd suggest adding it in the PR after that. |
Oh, I see. You are saying that this data plane should not report to Kubernetes that it is in a READY state to prevent the load balancer from distributing traffic to it until the ingress controller commits its first configuration. |
membphis
left a comment
membphis
left a comment
There was a problem hiding this comment.
LGTM, only some minor issues
| if not ok then | ||
| return nil, "failed to save config to shared dict: " .. err | ||
| end | ||
| ngx.log(ngx.NOTICE, "standalone config updated: ", config) |
There was a problem hiding this comment.
pls change to core.log.info(....)
| if err then | ||
| core.log.error("invalid request body: ", req_body, " err: ", err) | ||
| core.response.exit(400, {error_msg = "invalid request body: " .. err, | ||
| req_body = req_body}) |
There was a problem hiding this comment.
it not safe to response the request body
| if not valid then | ||
| core.log.error(err_prefix, err) | ||
| core.response.exit(400, {error_msg = err_prefix .. err, | ||
| req_body = req_body}) |
| if not valid then | ||
| core.log.error(err_prefix, err) | ||
| core.response.exit(400, {error_msg = err_prefix .. err, | ||
| req_body = req_body}) |
| "PUT /v1/agent/service/deregister/service_b1", | ||
| "PUT /v1/agent/service/deregister/service_b2", | ||
| "PUT /v1/agent/service/register\n" . "{\"Checks\": [{\"http\": \"http://baidu.com\",\"interval\": \"1s\"}],\"ID\":\"service_a1\",\"Name\":\"service_a\",\"Tags\":[\"primary\",\"v1\"],\"Address\":\"127.0.0.1\",\"Port\":30511,\"Meta\":{\"service_a_version\":\"4.0\"},\"EnableTagOverride\":false,\"Weights\":{\"Passing\":10,\"Warning\":1}}", | ||
| "PUT /v1/agent/service/register\n" . "{\"Checks\": [{\"http\": \"https://1.1.1.1\",\"interval\": \"1s\"}],\"ID\":\"service_a1\",\"Name\":\"service_a\",\"Tags\":[\"primary\",\"v1\"],\"Address\":\"127.0.0.1\",\"Port\":30511,\"Meta\":{\"service_a_version\":\"4.0\"},\"EnableTagOverride\":false,\"Weights\":{\"Passing\":10,\"Warning\":1}}", |
There was a problem hiding this comment.
do we have to change this?
Seems unrelated to the current PR.
There was a problem hiding this comment.
This thing will cause the CI to fail and needs to be replaced with an endpoint that is accessible in github workflow.
Co-authored-by: Traky Deng <[email protected]>
8 participants
Description
This PR aims to add an Admin API specialized for Standalone mode to allow users to flush in-memory configurations through them.
The user can pass in the configuration as JSON or YAML via HTTP PUT and it will:
This would make the stateless mode even more stateless, and it would no longer even depend on the file system. This will help our Ingress Controller use case.
In addition to the UPDATE API, there is also a GET API for dumping configurations, which will return the current configuration version and configuration contents.
However, the configuration content will be a deep copy replica of the input content, because APISIX will add internal data to the configuration, and since Lua passes table by reference, they will make it impossible to encode the data again. I.e., the data will be exported without any APISIX internal details, just to help you check what the current configuration is.
Scope
The scope of the API is all workers of the current APISIX instance, not the entire APISIX cluster. You need to call them on each instance to make them effective on the entire cluster.
The configuration takes effect in its entirety, so new commits will overwrite the old configuration in its entirety; there is no diff mechanism.
TODO
Checklist