voxpupuli / puppet-nginx

Puppet Module to manage NGINX on various UNIXes
https://forge.puppet.com/puppet/nginx
MIT License
470 stars 881 forks source link

Exported members and streams #1059

Open cova-fe opened 7 years ago

cova-fe commented 7 years ago

I'm using the feature of exporting members to add servers to a stream. However face issues when upstream resource tries to collect exported resources, as concat searches in /etc/nginx/conf.d/whatever-upstream.conf instead of /etc/nginx/conf.stream.d/whatever.

Looking at upstream/member.pp code, there is a

target  => "${::nginx::conf_dir}/conf.d/${upstream}-upstream.conf",

shouldn't be the same as upstream.pp, that uses the parameter $upstream_context to decide where to place the conf file, something like

target  => "${::nginx::conf_dir}/${conf_dir_real}/${upstream}-upstream.conf",

(with related logic, of course?)

I'm testing a small patch as described, but maybe I got the whole logic wrong (very likely, I fear...). Any hint?

igalic commented 7 years ago

I actually think the logic should be simplified, or unified, not duplicated.

It took us at $co a while to figure out what we're doing wrong when using this module, when we didn't set nginx::stream => on

a lot of the concept that we're exposing are very unintuitive

cova-fe commented 7 years ago

I see your point; however stream flag is indeed on (as the needed stream code block is present in conf files.)

Unifying it seems to be a good idea, however I wonder how this can be done in an exported resource, that is declared with only a @@nginx::resource::upstream::member and no other references to the same module classes...

igalic commented 7 years ago

possibly related, of the things i'm missing in @@nginx::resource::upstream::member is an assignment of port or protocol… https://github.com/voxpupuli/puppet-nginx/issues/1055

cova-fe commented 7 years ago

see https://github.com/voxpupuli/puppet-nginx/pull/1061

AranVinkItility commented 6 years ago

Just lost a lot of time because settings stream => on on the init of the module is not documented in the README.

It should throw an error in my opinion when defining streams when stream is not set to on