Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Converted balancer and backend to APIcast::Blackbox #1418

Closed
wants to merge 1 commit into from
Closed

Converted balancer and backend to APIcast::Blackbox #1418

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 31 additions & 7 deletions t/backend.t
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should delete this file because it really doesn't check anything.
cc @eguzki

Copy link
Member

Choose a reason for hiding this comment

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

APIcast has configured a virtualhost backend, listening on 8081 and configured with conf.d/backend.conf. Not really used in production, but widely used in dev environments to mock backend and in the tests environments.

It makes sense to me to test that this virtualhost backend exists and returns what the mocked backend does, which is return a 200 OK with the body transactions authrep!

The other alternative is to remove the virtualhost backend and run some mock server in another container for tests. Then it would make sense to remove this test IMO.

Anyway, the new implementation of this backend.t is not testing the backend virtualhost. The request should hit the backend virtualhost directly.

Copy link
Contributor Author

@hector-vido hector-vido Nov 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@eguzki or @tkan145 any ideas on how to add a different port in the --- request section?
Since the port is randomized in Test/APIcast/Blackbox.pm I can't find a way to talk directly with the backend listening in 8081.
Don't know if this is the right path, but I added a section for sites_d:

--- sites_d
server {
  listen *:8081;
  server_name backend;
  include "/opt/app-root/src/gateway/conf.d/backend.conf";
}

It is working from outside the test script.

Original file line number Diff line number Diff line change
@@ -1,17 +1,41 @@
use lib 't';
use Test::APIcast 'no_plan';
use Test::APIcast::Blackbox 'no_plan';

repeat_each(1);

run_tests();

__DATA__

=== TEST 1: backend
This is just a simple demonstration of the
=== TEST 1: This is just a simple demonstration of the
echo directive provided by ngx_http_echo_module.
--- config
include $TEST_NGINX_BACKEND_CONFIG;
--- configuration
{
"services" : [
{
"id": 42,
"backend_version": 1,
"proxy" : {
"api_backend": "http://test:$TEST_NGINX_SERVER_PORT/",
"proxy_rules": [
{ "pattern" : "/", "http_method" : "GET", "metric_system_name" : "bar", "delta" : 1}
]
}
}
]
}
--- backend
location /transactions/authrep.xml {
content_by_lua_block {
ngx.exit(200)
}
}
--- upstream
location / {
echo 'yay, api backend';
}
--- request
GET /transactions/authrep.xml
GET /?user_key=value
--- response_body
transactions authrep!
yay, api backend
--- error_code: 200
43 changes: 32 additions & 11 deletions t/balancer.t
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

This test is covered in #1416 . A small suggestion is to check existing PR(s) to avoid duplicating efforts. Good work on this one though.

Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use lib 't';
use Test::APIcast 'no_plan';
use Test::APIcast::Blackbox 'no_plan';

$ENV{TEST_NGINX_HTTP_CONFIG} = "$Test::APIcast::path/http.d/*.conf";
$ENV{RESOLVER} = '127.0.1.1:5353';
Expand All @@ -19,19 +19,40 @@ Balancing different hosts does not leak memory.
init_by_lua_block {
require('resty.balancer.round_robin').cache_size = 1
}
--- config
location = /t {
content_by_lua_block {
local round_robin = require('resty.balancer.round_robin')
local balancer = round_robin.new()
--- configuration
{
"services" : [
{
"id": 42,
"backend_version": 1,
"proxy" : {
"api_backend": "http://test:$TEST_NGINX_SERVER_PORT/",
"proxy_rules": [
{ "pattern" : "/", "http_method" : "GET", "metric_system_name" : "bar", "delta" : 1 }
]
}
}
]
}
--- backend
location /transactions/authrep.xml {
content_by_lua_block {
ngx.exit(200)
}
}
--- upstream
location = / {
content_by_lua_block {
local round_robin = require('resty.balancer.round_robin')
local balancer = round_robin.new()

local peers = { hash = ngx.var.request_id, cur = 1, 1, 2 }
local peer = round_robin.call(peers)
local peers = { hash = ngx.var.request_id, cur = 1, 1, 2 }
local peer = round_robin.call(peers)

ngx.print(peer)
}
ngx.print(peer)
}
}
--- pipelined_requests eval
[ "GET /t", "GET /t" ]
[ "GET /?user_key=value", "GET /?user_key=value" ]
--- response_body eval
[ "1", "1" ]