xiaokai-wang / nginx-stream-upsync-module

For stream protocol. syncing upstreams from etcd or consul and so on, dynamically updating backend servers attribute, not need to reload nginx for tcp protocol, stream configure.
175 stars 48 forks source link

Both nginx-upsync-module and nginx-stream-upsync-module in one instance #6

Closed CallMeFoxie closed 8 years ago

CallMeFoxie commented 8 years ago

Hi, we're building a universal upsync proxy and we could use a way to have both built-in at the same time.

The current problem is that the http_parser and json libraries are defined in both and collide during linking (and then one common variable name, that's an easy fix though).

I do not think there's an easy way around other than building a "wrapper" module which compiles only single instance of json+http_parser and then both upsync modules in.

Or if you have some other idea or know of any other possiblye issues, I'll be happy to listen. :)

CallMeFoxie commented 8 years ago

So I did exactly that.

  1. Cloned the two repos into addons/nginx-upsync/nginx-(stream-)upsync-module
  2. Renamed both json_parser and http_parser to a common filename and changed include paths (not necessary though)
  3. Wrote a wrapper config file:
have=NGX_UPSYNC . auto/have
ngx_addon_name=ngx_upsync_module
HTTP_MODULES="$HTTP_MODULES ngx_http_upsync_module"
STREAM_MODULES="$STREAM_MODULES ngx_stream_upsync_module"
NGX_ADDON_SRCS="$NGX_ADDON_SRCS $ngx_addon_dir/nginx-upsync-module/src/ngx_http_upsync_module.c $ngx_addon_dir/nginx-upsync-module/src/ngx_upsync_json.c $ngx_addon_dir/nginx-upsync-module/src/ngx_upsync_parser.c $ngx_addon_dir/nginx-stream-upsync-module/src/ngx_stream_upsync_module.c"
NGX_ADDON_DEPS="$NGX_ADDON_DEPS $ngx_addon_dir/nginx-upsync-module/src/ngx_http_upsync_module.h $ngx_addon_dir/nginx-upsync-module/src/ngx_upsync_json.h $ngx_addon_dir/nginx-upsync-module/src/ngx_upsync_parser.h $ngx_addon_dir/nginx-stream-upsync-module/src/ngx_stream_upsync_module.h"
ngx_feature_libs="-lm"
CORE_LIBS="$CORE_LIBS $ngx_feature_libs"
  1. Renamed variables in the stream: upsync_shared_created -> stream_upsync_shared_created upsync_shared_created0 -> stream_upsync_shared_created0
  2. And compiled with ./configure --add-module=addons/nginx-upsync --without-http_rewrite_module --without-http_gzip_module --prefix=/www/nginx --with-stream

and it works :) I can have both stream AND http upstreams in one.

I can make PR to rename the variables and then create a wrapper git repo, if you want?

xiaokai-wang commented 8 years ago

A pr is welcome.

As all we know, stream_upsync is for TCP and the other one for http, the function is different. Nginx is for tcp/http proxy independently in practical environment, right? And that is the reason I make two independently module.

If the two module as a part of nginx, a wrapper is much better. Thanks all you do. Totally agreed.

xiaokai-wang commented 8 years ago

@CallMeFoxie a new pr?

CallMeFoxie commented 8 years ago

We used both just to avoid 2 separate packages where you have to pick which one you need and have just one universal package.

We have found one issue though, if you have both compiled in and only use http {} or only stream {} it will crash as the context is null :) that has to be fixed as well, will look into it.

CallMeFoxie commented 8 years ago

https://github.com/CallMeFoxie/nginx-upsync/

this is the "master" package :)

Cloned & tested out, seems to be working fine.

gfrankliu commented 6 years ago

I just merged the fix in nginx-stream-upsync-module. Can you check out and try again? Now you can use both nginx-upsync-module and nginx-stream-upsync-module