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

Add parameters to upstream and upstreammembers #1233

Merged
merged 1 commit into from
Oct 20, 2018
Merged

Add parameters to upstream and upstreammembers #1233

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
75 changes: 64 additions & 11 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,20 @@ nginx::resource::server { 'www.puppetlabs.com':

```puppet
nginx::resource::upstream { 'puppet_rack_app':
members => [
'localhost:3000',
'localhost:3001',
'localhost:3002',
],
members => {
'localhost:3000':
server => 'localhost'
port => 3000
weight => 1
'localhost:3001':
server => 'localhost'
port => 3001
weight => 1
'localhost:3002':
server => 'localhost'
port => 3002
weight => 2
},
}

nginx::resource::server { 'rack.puppetlabs.com':
Expand Down Expand Up @@ -85,6 +94,38 @@ nginx::resource::mailhost { 'domain1.example':
}
```

### Convert upstream members from Array to Hash

The datatype Array for members of a nginx::resource::upstream is replaced by a Hash. The following configuration is no longer valid:

```puppet
nginx::resource::upstream { 'puppet_rack_app':
members => [
'localhost:3000',
'localhost:3001',
'localhost:3002',
],
}
```

From now on, the configuration must look like this:

```puppet
nginx::resource::upstream { 'puppet_rack_app':
members => {
'localhost:3000':
server => 'localhost'
port => 3000
Copy link
Member

Choose a reason for hiding this comment

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

At first glance this can be a little confusing. My first question would be "Why not parse the localhost:3000 into server and port?"

Could we potentially make it an array of hashes so that it looks more like:

nginx::resource::upstream { 'puppet_rack_app':
  members => [
    {
      server => 'localhost',
      port    => 3000,
    },
    {
      server => 'localhost',
      port    => 3001,
    },
    {
      server => 'localhost',
      port    => 3002,
    },
  ]

Does that work or does the parameter rely on each member to have a unique name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mainly cause Unix sockets are accepted too. We would first have to see if it is a valid Unix socket and only if not split at the colon. After that we need to check the parts if they are valid. We almost have to implement the used datatype in code. And at the end we have the problem to name the resources for the defined type which is necessary for exporting upstream members. That's why we need a unique name here.
Should I change the README.md file and use keys that do not consist of server and port? Maybe it's a good idea to use the keys as default for the comment.

'localhost:3001':
server => 'localhost'
port => 3001
'localhost:3002':
server => 'localhost'
port => 3002
},
}
```

## SSL configuration

By default, creating a server resource will only create a HTTP server. To also
Expand Down Expand Up @@ -137,9 +178,15 @@ nginx::nginx_upstreams:
'puppet_rack_app':
ensure: present
members:
- localhost:3000
- localhost:3001
- localhost:3002
'localhost:3000':
server: 'localhost'
port: 3000
'localhost:3001':
server: 'localhost'
port: 3001
'localhost:3002':
server: 'localhost'
port: 3002
nginx::nginx_servers:
'www.puppetlabs.com':
www_root: '/var/www/www.puppetlabs.com'
Expand Down Expand Up @@ -185,9 +232,15 @@ nginx::nginx_upstreams:
'syslog':
upstream_context: 'stream'
members:
- '10.0.0.1:514'
- '10.0.0.2:514'
- '10.0.0.3:514'
'10.0.0.1:514'
server: '10.0.0.1'
port: '514'
'10.0.0.2:514'
server: '10.0.0.2'
port: '514'
'10.0.0.3:514'
server: '10.0.0.3'
port: '514'
```

## Nginx with precompiled Passenger
Expand Down
3 changes: 2 additions & 1 deletion manifests/init.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@
$nginx_mailhosts_defaults = {},
$nginx_streamhosts = {},
$nginx_upstreams = {},
Nginx::UpstreamMemberDefaults $nginx_upstream_defaults = {},
$nginx_servers = {},
$nginx_servers_defaults = {},
Boolean $purge_passenger_repo = true,
Expand All @@ -180,7 +181,7 @@
contain 'nginx::config'
contain 'nginx::service'

create_resources('nginx::resource::upstream', $nginx_upstreams)
create_resources('nginx::resource::upstream', $nginx_upstreams, $nginx_upstream_defaults)
create_resources('nginx::resource::server', $nginx_servers, $nginx_servers_defaults)
create_resources('nginx::resource::location', $nginx_locations, $nginx_locations_defaults)
create_resources('nginx::resource::mailhost', $nginx_mailhosts, $nginx_mailhosts_defaults)
Expand Down
176 changes: 123 additions & 53 deletions manifests/resource/upstream.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,28 @@
# This definition creates a new upstream proxy entry for NGINX
#
# Parameters:
# [*members*] - Array of member URIs for NGINX to connect to. Must follow valid NGINX syntax.
# If omitted, individual members should be defined with nginx::resource::upstream::member
# [*ensure*] - Enables or disables the specified location (present|absent)
# [*upstream_cfg_append*] - Hash of custom directives to put after other directives in upstream
# [*upstream_cfg_prepend*] - It expects a hash with custom directives to put before anything else inside upstream
# [*upstream_fail_timeout*] - Set the fail_timeout for the upstream. Default is 10 seconds - As that is what Nginx does normally.
# [*upstream_max_fails*] - Set the max_fails for the upstream. Default is to use nginx default value which is 1.
# [*context*] - Set the type of this upstream (http|stream).
# [*members*] - Hash of member URIs for NGINX to connect to. Must follow valid NGINX syntax.
# If omitted, individual members should be defined with nginx::resource::upstream::member
# [*members_tag*] - Restrict collecting the exported members for this upstream with a tag.
# [*member_defaults*] - Specify default settings added to each member of this upstream.
# [*hash*] - Activate the hash load balancing method (https://nginx.org/en/docs/http/ngx_http_upstream_module.html#hash).
# [*ip_hash*] - Activate ip_hash for this upstream (https://nginx.org/en/docs/http/ngx_http_upstream_module.html#ip_hash).
# [*keepalive*] - Set the maximum number of idle keepalive connections (https://nginx.org/en/docs/http/ngx_http_upstream_module.html#keepalive).
# [*keepalive_requests*] - Sets the maximum number of requests that can be served through one keepalive connection (https://nginx.org/en/docs/http/ngx_http_upstream_module.html#keepalive_requests).
# [*keepalive_timeout*] - Sets a timeout during which an idle keepalive connection to an upstream server will stay open (https://nginx.org/en/docs/http/ngx_http_upstream_module.html#keepalive_timeout).
# [*least_conn*] - Activate the least_conn load balancing method (https://nginx.org/en/docs/http/ngx_http_upstream_module.html#least_conn).
# [*least_time*] - Activate the least_time load balancing method (https://nginx.org/en/docs/http/ngx_http_upstream_module.html#least_time).
# [*ntlm*] - Allow NTLM authentication (https://nginx.org/en/docs/http/ngx_http_upstream_module.html#ntlm).
# [*queue_max*] - Set the maximum number of queued requests (https://nginx.org/en/docs/http/ngx_http_upstream_module.html#queue).
# [*queue_timeout*] - Set the timeout for the queue (https://nginx.org/en/docs/http/ngx_http_upstream_module.html#queue).
# [*random*] - Activate the random load balancing method (https://nginx.org/en/docs/http/ngx_http_upstream_module.html#random).
# [*statefile*] - Specifies a file that keeps the state of the dynamically configurable group (https://nginx.org/en/docs/http/ngx_http_upstream_module.html#state).
# [*sticky*] - Enables session affinity (https://nginx.org/en/docs/http/ngx_http_upstream_module.html#sticky).
# [*zone*] - Defines the name and optional the size of the shared memory zone (https://nginx.org/en/docs/http/ngx_http_upstream_module.html#zone).
# [*cfg_append*] - Hash of custom directives to put after other directives in upstream
# [*cfg_prepend*] - It expects a hash with custom directives to put before anything else inside upstream
#
# Actions:
#
Expand All @@ -18,82 +33,121 @@
# Sample Usage:
# nginx::resource::upstream { 'proxypass':
# ensure => present,
# members => [
# 'localhost:3000',
# 'localhost:3001',
# 'localhost:3002',
# ],
# members => {
# 'localhost:3001' => {
# server => 'localhost',
# port => 3000,
# },
# 'localhost:3002' => {
# server => 'localhost',
# port => 3002,
# },
# 'localhost:3003' => {
# server => 'localhost',
# port => 3003,
# },
# },
# }
#
# Custom config example to use ip_hash, and 20 keepalive connections
# create a hash with any extra custom config you want.
# $my_config = {
# 'ip_hash' => '',
# 'keepalive' => '20',
# }
# nginx::resource::upstream { 'proxypass':
# ensure => present,
# members => [
# 'localhost:3000',
# 'localhost:3001',
# 'localhost:3002',
# ],
# upstream_cfg_prepend => $my_config,
# ensure => present,
# members => {
# 'localhost:3001' => {
# server => 'localhost',
# port => 3000,
# },
# 'localhost:3002' => {
# server => 'localhost',
# port => 3002,
# },
# 'localhost:3003' => {
# server => 'localhost',
# port => 3003,
# },
# },
# ip_hash => true,
# keepalive => 20,
# }
#
define nginx::resource::upstream (
Optional[Array] $members = undef,
$members_tag = undef,
Enum['present', 'absent'] $ensure = 'present',
Optional[Hash] $upstream_cfg_append = undef,
Optional[Hash] $upstream_cfg_prepend = undef,
$upstream_fail_timeout = '10s',
$upstream_max_fails = undef,
Enum['http', 'stream'] $upstream_context = 'http',
Enum['present', 'absent'] $ensure = 'present',
Enum['http', 'stream'] $context = 'http',
Nginx::UpstreamMembers $members = {},
Copy link
Member

Choose a reason for hiding this comment

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

Same comment as above

Optional[String[1]] $members_tag = undef,
Nginx::UpstreamMemberDefaults $member_defaults = {},
Optional[String[1]] $hash = undef,
Boolean $ip_hash = false,
Optional[Integer[1]] $keepalive = undef,
Optional[Integer[1]] $keepalive_requests = undef,
Optional[Nginx::Time] $keepalive_timeout = undef,
Boolean $least_conn = false,
Optional[Nginx::UpstreamLeastTime] $least_time = undef,
Boolean $ntlm = false,
Optional[Integer] $queue_max = undef,
Optional[Nginx::Time] $queue_timeout = undef,
Optional[String[1]] $random = undef,
Optional[Stdlib::Unixpath] $statefile = undef,
Optional[Nginx::UpstreamSticky] $sticky = undef,
Optional[Nginx::UpstreamZone] $zone = undef,
Nginx::UpstreamCustomParameters $cfg_append = {},
Nginx::UpstreamCustomParameters $cfg_prepend = {},
) {

if ! defined(Class['nginx']) {
fail('You must include the nginx base class before using any defined resources')
}

$root_group = $nginx::root_group

$ensure_real = $ensure ? {
'absent' => absent,
default => present,
if $least_time {
if $context == 'http' and ! ($least_time =~ Nginx::UpstreamLeastTimeHttp) {
fail('The parameter "least_time" does not match the datatype "Nginx::UpstreamLeastTimeHttp"')
}
if $context == 'stream' and ! ($least_time =~ Nginx::UpstreamLeastTimeStream) {
fail('The parameter "least_time" does not match the datatype "Nginx::UpstreamLeastTimeStream"')
}
}

$conf_dir_real = $upstream_context ? {
'stream' => 'conf.stream.d',
default => 'conf.d',
$conf_dir = $context ? {
'stream' => "${nginx::config::conf_dir}/conf.stream.d",
default => "${nginx::config::conf_dir}/conf.d",
}

$conf_dir = "${nginx::config::conf_dir}/${conf_dir_real}"

Concat {
owner => 'root',
group => $root_group,
group => $nginx::root_group,
mode => '0644',
}

concat { "${nginx::conf_dir}/${conf_dir_real}/${name}-upstream.conf":
ensure => $ensure_real,
concat { "${conf_dir}/${name}-upstream.conf":
ensure => $ensure,
notify => Class['::nginx::service'],
require => File[$conf_dir],
}

# Uses: $name, $upstream_cfg_prepend
concat::fragment { "${name}_upstream_header":
target => "${nginx::conf_dir}/${conf_dir_real}/${name}-upstream.conf",
target => "${conf_dir}/${name}-upstream.conf",
order => '10',
content => template('nginx/upstream/upstream_header.erb'),
content => epp('nginx/upstream/upstream_header.epp', {
cfg_prepend => $cfg_prepend,
name => $name,
}),
}

if $members != undef {
# Uses: $members, $upstream_fail_timeout
concat::fragment { "${name}_upstream_members":
target => "${nginx::conf_dir}/${conf_dir_real}/${name}-upstream.conf",
order => '50',
content => template('nginx/upstream/upstream_members.erb'),
$members.each |$member,$values| {
$member_values = merge($member_defaults,$values,{'upstream' => $name,'context' => $context})

if $context == 'stream' and $member_values['route'] {
fail('The parameter "route" is not available for upstreams with context "stream"')
}
if $context == 'stream' and $member_values['state'] and $member_values['state'] == 'drain' {
fail('The state "drain" is not available for upstreams with context "stream"')
}

nginx::resource::upstream::member { $member:
* => $member_values,
}
}
} else {
# Collect exported members:
Expand All @@ -105,8 +159,24 @@
}

concat::fragment { "${name}_upstream_footer":
target => "${nginx::conf_dir}/${conf_dir_real}/${name}-upstream.conf",
target => "${conf_dir}/${name}-upstream.conf",
order => '90',
content => template('nginx/upstream/upstream_footer.erb'),
content => epp('nginx/upstream/upstream_footer.epp', {
Copy link
Member

Choose a reason for hiding this comment

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

My personal preference is for no parameters for epp(), where possible.

Copy link
Member

Choose a reason for hiding this comment

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

I highly prefer them, because that allows scoped variables with proper datatypes.

cfg_append => $cfg_append,
hash => $hash,
ip_hash => $ip_hash,
keepalive => $keepalive,
keepalive_requests => $keepalive_requests,
keepalive_timeout => $keepalive_timeout,
least_conn => $least_conn,
least_time => $least_time,
ntlm => $ntlm,
queue_max => $queue_max,
queue_timeout => $queue_timeout,
random => $random,
statefile => $statefile,
sticky => $sticky,
zone => $zone,
}),
}
}
Loading