Incorrect default timeout values

[rudybroersma]

Affected Puppet, Ruby, OS and module versions/distributions

• Puppet: 2.8.0
• Ruby: 2.3.3p222
• Distribution: Debian Stretch (9)
• Module version: 0.8.0

How to reproduce (e.g Puppet code you use)

class { 'nginx': }

What are you seeing

/etc/nginx/nginx.conf is created with (among some other config) these values:

keepalive_timeout 65;
client_body_timeout 60;
send_timeout 60;
lingering_timeout 5;

What behaviour did you expect instead

We had an issue that we are seeing HTTP 499 errors (client timeout) for requests that dont take very long (eg. only few hundred ms). I noticed there is no indication of what exactly the '65', '60' or '5' is. What I mean by that is that it does not indicate whether it are seconds, minutes, hours or whatever.

When I change the configuration to:

  class { 'nginx':
      keepalive_timeout => '65s',
      keepalive_requests => '100',
      client_body_timeout => '60s',
      send_timeout => '60s',
      lingering_timeout => '5s',
  }

The change becomes:

-  keepalive_timeout   65;
+  keepalive_timeout   65s;
   keepalive_requests  100;
-  client_body_timeout 60;
-  send_timeout        60;
-  lingering_timeout   5;
+  client_body_timeout 60s;
+  send_timeout        60s;
+  lingering_timeout   5s;

And all HTTP 499 timeouts are resolved.

Any additional information you'd like to impart

Looking at the nginx source code it seems to me the values are interpreted by default in miliseconds, eg. the source shows:

    ngx_conf_merge_msec_value(conf->keepalive_timeout,
                              prev->keepalive_timeout, 75000);

    ngx_conf_merge_msec_value(conf->client_body_timeout,
                              prev->client_body_timeout, 60000);

    ngx_conf_merge_msec_value(conf->send_timeout, prev->send_timeout, 60000);

    ngx_conf_merge_msec_value(conf->lingering_timeout,  
                              prev->lingering_timeout, 5000);

Thus, the Puppet module should add 's' to all values or increase the default values by 1000.

Also, the Nginx documentation has all default values explained in seconds. For example see the following snippet from:

http://nginx.org/en/docs/http/ngx_http_core_module.html#send_timeout

Syntax:	send_timeout time;
send_timeout 60s;
http, server, location

The actual issue:

It seems that the Puppet module should indicate timeout values with an 's' suffixed to all default values to indicate seconds. The default (without any suffix) seems to be in miliseconds. The nginx call 'ngx_conf_merge_msec_value' seems to support this.

[ekohl]

Thus, the Puppet module should add 's' to all values or increase the default values by 1000.

I'd prefer adding the s because this matches the nginx defaults and docs. Would you mind submitting a PR to fix this?

[rudybroersma]

Done :)

[alexjfisher]

I'm confused.
A value without a suffix means seconds.

[ekohl]

Fixed in #1138