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

Does this work with nginx 1.20.2? #227

Closed
prabumohan opened this issue Mar 8, 2022 · 10 comments
Closed

Does this work with nginx 1.20.2? #227

prabumohan opened this issue Mar 8, 2022 · 10 comments

Comments

@prabumohan
Copy link

No description provided.

@aminvakil
Copy link
Contributor

Yes.

@prabumohan
Copy link
Author

wow ... to be honest i didnt expect a response ! Thanks for the quick response .

I'm stuck with this error
/usr/local/src/nginx/nginx-module-vts/src/ngx_http_vhost_traffic_status_display_json.c: In function ‘ngx_http_vhost_traffic_status_display_set_upstream_group’:
/usr/local/src/nginx/nginx-module-vts/src/ngx_http_vhost_traffic_status_display_json.c:586:59: error: ‘ngx_http_upstream_rr_peer_t’ has no member named ‘check_index’
if (ngx_http_upstream_check_peer_down(peer->check_index)) {
^
make[1]: *** [objs/addon/src/ngx_http_vhost_traffic_status_display_json.o] Error 1
make[1]: Leaving directory `/usr/local/src/nginx/build/nginx-1.20.0'
make: *** [build] Error 2

Any help would be much appricated

@testn
Copy link

testn commented Mar 17, 2022

Did you have nginx_upstream_check_module?

--add-module=../nginx_upstream_check_module-master

@prabumohan
Copy link
Author

yes
https://github.com/yaoweibin/nginx_upstream_check_module
For this module when i check health for upstream they report correct whereas Nginx Vhost Traffic Status always saya Up for all upstream servers though they are down

Is that expected ?

@testn
Copy link

testn commented Mar 17, 2022

@prabumohan
Copy link
Author

i did and hence got the active health check working.. the issue now is for Nginx-module-vts module still flags all upstream servers up in its status page

@testn
Copy link

testn commented Mar 17, 2022

@prabumohan
Copy link
Author

yes check is setup...
image
as per the screenshot as you can see it reports up but actual its down

@vozlt
Copy link
Owner

vozlt commented Sep 5, 2022

@prabumohan
I've never tested nginx_upstream_check_module.(#134 (comment))
The peer->check_index variable is defined in the nginx_upstream_check_module and it looks like it can be used only by patching it in nginx.
If it continues to be a problem during compilation,, remove it as below!

--- ngx_http_vhost_traffic_status_display_json.c        2021-01-25 00:21:14.610002559 +0900
+++ ngx_http_vhost_traffic_status_display_json.c.o      2022-09-05 23:43:21.015685286 +0900
@@ -10,10 +10,6 @@
 #include "ngx_http_vhost_traffic_status_display_json.h"
 #include "ngx_http_vhost_traffic_status_display.h"
 
-#if (NGX_HTTP_UPSTREAM_CHECK)
-#include "ngx_http_upstream_check_module.h"
-#endif
-
 
 u_char *
 ngx_http_vhost_traffic_status_display_set_main(ngx_http_request_t *r,
@@ -582,17 +578,7 @@ ngx_http_vhost_traffic_status_display_se
                 usn.max_fails = peer->max_fails;
                 usn.fail_timeout = peer->fail_timeout;
                 usn.backup = 0;
-#if (NGX_HTTP_UPSTREAM_CHECK)
-                if (ngx_http_upstream_check_peer_down(peer->check_index)) {
-                    usn.down = 1;
-
-                } else {
-                    usn.down = 0;
-                }
-#else
                 usn.down = (peer->fails >= peer->max_fails || peer->down);
-#endif
-
 #if nginx_version > 1007001
                 usn.name = peer->name;
 #endif

@u5surf
Copy link
Collaborator

u5surf commented Mar 9, 2023

@prabumohan
Hi, It seems the same issue of this.
#262 (comment)
We consider that it is quite tricky because it is a little complicated mechanism when the peer->fails is counted on pure nginx upstream.

So that a lot of people confuse what to do and not to recognize the necessary of the upstream status collect module such as nginx_upstream_check_module.

To check the copatiblity of it and this module, we also consider that it should prepare the procedure how to setup with it in this README and the combined test case in this CI before too long.

u5surf added a commit to u5surf/nginx-module-vts that referenced this issue Mar 12, 2023
u5surf added a commit to u5surf/nginx-module-vts that referenced this issue Mar 12, 2023
@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

5 participants