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.

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

Only discovers one of upstream->servers->addrs #189

Closed
nicholaschiasson opened this issue Oct 7, 2020 · 1 comment
Closed

Only discovers one of upstream->servers->addrs #189

nicholaschiasson opened this issue Oct 7, 2020 · 1 comment

Comments

@nicholaschiasson
Copy link

Nginx's ngx_http_upstream_server_t type supports holding multiple ngx_addr_ts, but this module only considers one of them for the server.

I am working on this PR to allow the jdomain module to integrate better with this module and others by leveraging Nginx server. I discovered this issue with one of my test cases in which I define an upstream using jdomain which will resolve to multiple servers under the hood. Even in that case however, I found that the vhost traffic status page only shows one element for the server which has 2 addresses.

Below is my test case (apologies for verbosity and random context here):

__DATA__

=== TEST 1: Virtual host traffic status
--- init
`echo 'local-data: "example.com 1 A 127.0.0.2"' > /etc/unbound_local_zone.conf &&
echo 'local-data: "example.com 1 A 127.0.0.3"' >> /etc/unbound_local_zone.conf &&
unbound-control reload` or die $!;
--- http_config
vhost_traffic_status_zone;
resolver 127.0.0.88;
upstream upstream_test {
	jdomain example.com port=8000 interval=32;
}
server {
	listen 127.0.0.2:8000;
	return 201 'Pass 1';
}
server {
	listen 127.0.0.3:8000;
	return 202 'Pass 2';
}
--- config
location = / {
	proxy_pass http://upstream_test;
}
location /status {
	vhost_traffic_status_display;
}
--- request eval
["GET /", "GET /", "GET /", "GET /", "GET /", "GET /", "GET /"]
--- error_code eval
[201, 202, 201, 202, 201, 202, 201]
--- response_body_like eval
["Pass 1", "Pass 2", "Pass 1", "Pass 2", "Pass 1", "Pass 2", "Pass 1"]

In that test, we first set our DNS server to return 2 IPs for example.com:

$ dig example.com

; <<>> DiG 9.11.4-P2-RedHat-9.11.4-9.P2.amzn2.0.4 <<>> example.com
;; global options: +cmd
;; Got answer:
;; ->>HEADER<<- opcode: QUERY, status: NOERROR, id: 3262
;; flags: qr aa rd ra; QUERY: 1, ANSWER: 2, AUTHORITY: 0, ADDITIONAL: 1

;; OPT PSEUDOSECTION:
; EDNS: version: 0, flags:; udp: 4096
;; QUESTION SECTION:
;example.com.                   IN      A

;; ANSWER SECTION:
example.com.            1       IN      A       127.0.0.2
example.com.            1       IN      A       127.0.0.3

;; Query time: 0 msec
;; SERVER: 127.0.0.88#53(127.0.0.88)
;; WHEN: Wed Oct 07 15:25:08 UTC 2020
;; MSG SIZE  rcvd: 72

We then hit the server 7 times expecting the round robin load balancing to behave correctly, which it does.

Afterward, I have a test case which I omitted here to verify the contents of the vhost traffic status json response.

However, here is the status displayed:

image

We can see that there is only an upstream server for 127.0.0.2:8000, and not 127.0.0.3:8000. What's more, there were 7 requests sent by the test suite, 4 targeting 127.0.0.2:8000 and 3 targeting 127.0.0.3:8000 if round robin worked correctly. The report shows only 4 requests total, so I can safely assume it's not just grouping all traffic stats for the addresses in the server by the first address name, but it's really only tracking stats for the first address.

For now, I will be taking a break from things and submitting my PR for jdomain regardless of if this gets addressed. But I think I may return with a PR for this issue myself.

Thanks! 🙂

@u5surf
Copy link
Collaborator

u5surf commented Apr 6, 2023

@nicholaschiasson Hi, I consider that it seems to be similar to your issue.
can you fix if you try this patch?
#259

@u5surf u5surf closed this as completed Jul 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants