Refactor/tests (Kong#1350)

* Tests (fixes and missing implementations)
* Brings back anonymous reports in nginx.conf
* Checks for ulimit

kong/cmd/utils/prefix_handler.lua

@@ -7,8 +7,10 @@ local pl_utils = require "pl.utils"
 local pl_file = require "pl.file"
 local pl_path = require "pl.path"
 local pl_dir = require "pl.dir"
+local socket = require "socket"
 local utils = require "kong.tools.utils"
 local log = require "kong.cmd.utils.log"
+local constants = require "kong.constants"
 local fmt = string.format
 
 -- script from old services.serf module
@@ -71,12 +73,19 @@ local function gen_default_ssl_cert(kong_config)
   return true
 end
 
+local function get_ulimit()
+  local ok, _, stdout, stderr = pl_utils.executeex "ulimit -n"
+  if not ok then return nil, stderr end
+  return tonumber(pl_stringx.strip(stdout))
+end
+
 local function gather_system_infos(compile_env)
   local infos = {}
 
-  local ok, _, stdout, stderr = pl_utils.executeex "ulimit -n"
-  if not ok then return nil, stderr end
-  infos.ulimit = pl_stringx.strip(stdout)
+  local ulimit, err = get_ulimit()
+  if not ulimit then return nil, err end
+  infos.worker_rlimit = ulimit
+  infos.worker_connections = ulimit > 16384 and 16384 or ulimit
 
   return infos
 end
@@ -97,13 +106,22 @@ local function compile_conf(kong_config, conf_template, env_t)
   if kong_config.dnsmasq then
     compile_env["dns_resolver"] = "127.0.0.1:"..kong_config.dnsmasq_port
   end
+
+  local infos, err = gather_system_infos()
+  if not infos then return nil, err end
   if kong_config.nginx_optimizations then
-    local infos, err = gather_system_infos()
-    if not infos then return nil, err end
     compile_env = pl_tablex.merge(compile_env, infos,  true) -- union
   end
 
-  -- merge compile env with kong_config and additional env values
+  if kong_config.anonymous_reports then
+    -- If there is no internet connection, disable this feature
+    if socket.dns.toip(constants.SYSLOG.ADDRESS) then
+      compile_env["syslog_reports"] = string.format("error_log syslog:server=%s:%d error;", 
+                                                    constants.SYSLOG.ADDRESS, 
+                                                    constants.SYSLOG.PORT)
+    end
+  end
+
   compile_env = pl_tablex.merge(compile_env, kong_config, true) -- union
   compile_env = pl_tablex.merge(compile_env, env_t, true) -- union
 
@@ -171,6 +189,13 @@ local function prepare_prefix(kong_config)
     end
   end
 
+  -- check ulimit
+  local ulimit, err = get_ulimit()
+  if not ulimit then return nil, err end
+  if ulimit < 4096 then
+    log.warn(string.format("ulimit is currently set to \"%d\". For better performance set it to at least \"4096\" using \"ulimit -n\"", ulimit))
+  end
+
   -- write NGINX conf
   local nginx_conf, err = compile_nginx_conf(kong_config)
   if not nginx_conf then return nil, err end

kong/cmd/utils/serf_signals.lua

@@ -63,7 +63,7 @@ function _M.start(kong_config, dao)
     ["-advertise"] = kong_config.cluster_advertise,
     ["-encrypt"] = kong_config.cluster_encrypt_key,
     ["-log-level"] = "err",
-    ["-profile"] = "wan",
+    ["-profile"] = kong_config.cluster_profile,
     ["-node"] = serf.node_name,
     ["-event-handler"] = "member-join,member-leave,member-failed,"
                        .."member-update,member-reap,user:"

kong/conf_loader.lua

@@ -70,6 +70,7 @@ local CONF_INFERENCES = {
   cassandra_ssl = {typ = "boolean"},
   cassandra_ssl_verify = {typ = "boolean"},
 
+  cluster_profile = {enum = {"local", "lan", "wan"}},
   cluster_ttl_on_failure = {typ = "number"},
 
   dnsmasq = {typ = "boolean"},

kong/constants.lua

@@ -37,5 +37,9 @@ return {
       "month",
       "year"
     }
+  },
+  SYSLOG = {
+    ADDRESS = "kong-hf.mashape.com",
+    PORT = 61828
   }
 }

kong/core/reports.lua

@@ -6,6 +6,7 @@ local pl_utils = require "pl.utils"
 local pl_stringx = require "pl.stringx"
 local resty_lock = require "resty.lock"
 local singletons = require "kong.singletons"
+local constants = require "kong.constants"
 local concat = table.concat
 local udp_sock = ngx.socket.udp
 
@@ -51,8 +52,8 @@ system_infos = get_system_infos()
 local function send(t, host, port)
   if not enabled then return end
   t = t or {}
-  host = host or "kong-hf.mashape.com"
-  port = port or 61828
+  host = host or constants.SYSLOG.ADDRESS
+  port = port or constants.SYSLOG.PORT
 
   local buf = {}
   for k, v in pairs(system_infos) do

kong/templates/kong_defaults.lua

@@ -39,6 +39,7 @@ cluster_listen = 0.0.0.0:7946
 cluster_listen_rpc = 127.0.0.1:7373
 cluster_advertise = NONE
 cluster_encrypt_key = NONE
+cluster_profile = wan
 cluster_ttl_on_failure = 3600
 
 dnsmasq = on

kong/templates/nginx.lua

@@ -4,10 +4,17 @@ daemon ${{NGINX_DAEMON}};
 
 pid pids/nginx.pid;
 error_log logs/error.log ${{LOG_LEVEL}};
+> if anonymous_reports then
+${{SYSLOG_REPORTS}}
+> end
+
+> if nginx_optimizations then
+worker_rlimit_nofile ${{WORKER_RLIMIT}};
+> end
 
 events {
 > if nginx_optimizations then
-    worker_connections ${{ULIMIT}};
+    worker_connections ${{WORKER_CONNECTIONS}};
     multi_accept on;
 > end
 }

kong/tools/responses.lua

@@ -124,7 +124,7 @@ local function send_response(status_code)
     elseif content then
       ngx.say(cjson.encode({ message = content }))
     end
-
+    
     return ngx.exit(status_code)
   end
 end

spec/01-unit/01-conf/02-prefix_handler_spec.lua

@@ -96,7 +96,9 @@ describe("NGINX conf compiler", function()
       local conf = assert(conf_loader(nil, {
         nginx_optimizations = false,
       }))
+
       local nginx_conf = prefix_handler.compile_nginx_conf(conf)
+      assert.not_matches("worker_rlimit_nofile %d+;", nginx_conf)
       assert.not_matches("worker_connections %d+;", nginx_conf)
       assert.not_matches("multi_accept on;", nginx_conf)
     end)
@@ -105,9 +107,24 @@ describe("NGINX conf compiler", function()
         nginx_optimizations = true,
       }))
       local nginx_conf = prefix_handler.compile_nginx_conf(conf)
+      assert.matches("worker_rlimit_nofile %d+;", nginx_conf)
       assert.matches("worker_connections %d+;", nginx_conf)
       assert.matches("multi_accept on;", nginx_conf)
     end)
+    it("compiles without anonymous reports", function()
+      local conf = assert(conf_loader(nil, {
+        anonymous_reports = false,
+      }))
+      local nginx_conf = prefix_handler.compile_nginx_conf(conf)
+      assert.not_matches("error_log syslog:server=.+ error;", nginx_conf)
+    end)
+    it("compiles with anonymous reports", function()
+      local conf = assert(conf_loader(nil, {
+        anonymous_reports = true,
+      }))
+      local nginx_conf = prefix_handler.compile_nginx_conf(conf)
+      assert.matches("error_log syslog:server=.+:61828 error;", nginx_conf)
+    end)
   end)
 
   describe("prepare_prefix()", function()

spec/02-integration/01-cmd/02-start_stop_spec.lua

@@ -1,11 +1,6 @@
 local helpers = require "spec.helpers"
-local pl_dir = require "pl.dir"
-local pl_path = require "pl.path"
 
 describe("kong start/stop", function()
-  setup(function()
-    helpers.prepare_prefix()
-  end)
   teardown(function()
     helpers.kill_all()
     helpers.clean_prefix()

spec/02-integration/01-cmd/03-compile_spec.lua

@@ -1,6 +1,10 @@
 local helpers = require "spec.helpers"
+local prefix_handler = require "kong.cmd.utils.prefix_handler"
 
 describe("kong compile", function()
+  setup(function()
+    assert(prefix_handler.prepare_prefix(helpers.test_conf))
+  end)
   it("compiles a Kong NGINX config", function()
     local _, stderr, stdout = helpers.kong_exec "compile"
     assert.equal("", stderr)

spec/02-integration/01-cmd/04-reload_spec.lua

@@ -3,7 +3,6 @@ local helpers = require "spec.helpers"
 describe("kong reload", function()
   setup(function()
     helpers.kill_all()
-    helpers.prepare_prefix()
   end)
   teardown(function()
     helpers.kill_all()

spec/02-integration/03-admin_api/01-kong_routes_spec.lua

@@ -5,7 +5,6 @@ describe("Admin API", function()
   local client
   setup(function()
     helpers.kill_all()
-    assert(helpers.prepare_prefix())
     assert(helpers.start_kong())
 
     client = assert(helpers.http_client("127.0.0.1", helpers.test_conf.admin_port, 10000))

spec/02-integration/03-admin_api/02-apis_routes_spec.lua

@@ -12,7 +12,6 @@ describe("Admin API", function()
   local client
   setup(function()
     helpers.kill_all()
-    assert(helpers.prepare_prefix())
     assert(helpers.start_kong())
 
     client = assert(helpers.http_client("127.0.0.1", helpers.test_conf.admin_port))

spec/02-integration/03-admin_api/03-consumers_routes_spec.lua

@@ -12,7 +12,6 @@ describe("Admin API", function()
   local client
   setup(function()
     helpers.kill_all()
-    assert(helpers.prepare_prefix())
     assert(helpers.start_kong())
     client = assert(helpers.http_client("127.0.0.1", helpers.test_conf.admin_port))
   end)

spec/02-integration/03-admin_api/04-plugins_routes_spec.lua

@@ -5,7 +5,6 @@ describe("Admin API", function()
   local client
   setup(function()
     helpers.kill_all()
-    assert(helpers.prepare_prefix())
     assert(helpers.start_kong())
 
     client = assert(helpers.http_client("127.0.0.1", helpers.test_conf.admin_port))
@@ -15,7 +14,7 @@ describe("Admin API", function()
       client:close()
     end
     helpers.stop_kong()
-    helpers.clean_prefix()
+    --helpers.clean_prefix()
   end)
 
   describe("/plugins/enabled", function()
@@ -157,7 +156,7 @@ describe("Admin API", function()
         local json = cjson.decode(body)
         assert.is_table(json.fields)
       end)
-      it("returns 404 on invalid plugin", function()
+      it("#only returns 404 on invalid plugin", function()
         local res = assert(client:send {
           method = "GET",
           path = "/plugins/schema/foobar",

spec/02-integration/03-admin_api/05-cache_routes_spec.lua

@@ -6,7 +6,6 @@ describe("Admin API", function()
   local client, proxy_client
   setup(function()
     helpers.kill_all()
-    assert(helpers.prepare_prefix())
     assert(helpers.start_kong())
 
     client = assert(helpers.http_client("127.0.0.1", helpers.test_conf.admin_port))

spec/02-integration/03-admin_api/06-cluster_routes_spec.lua

@@ -5,7 +5,6 @@ describe("Admin API", function()
   local client
   setup(function()
     helpers.kill_all()
-    assert(helpers.prepare_prefix())
     assert(helpers.start_kong())
 
     client = assert(helpers.http_client("127.0.0.1", helpers.test_conf.admin_port))

spec/02-integration/04-core/03-hooks_spec.lua → spec/02-integration/04-core/02-hooks_spec.lua

@@ -12,7 +12,6 @@ describe("Core Hooks", function()
   local consumer, api1, api2, basic_auth2, api3, rate_limiting_consumer
   before_each(function()
     helpers.kill_all()
-    assert(helpers.prepare_prefix())
 
     consumer = assert(helpers.dao.consumers:insert {
       username = "consumer1"

spec/02-integration/05-proxy/01-resolver_spec.lua

@@ -6,8 +6,6 @@ describe("Resolver", function()
   local client
   setup(function()
     helpers.kill_all()
-    helpers.dao:truncate_tables()
-    assert(helpers.prepare_prefix())
 
     -- request_host
     assert(helpers.dao.apis:insert {

spec/02-integration/05-proxy/02-real_ip_spec.lua

@@ -13,7 +13,6 @@ describe("Real IP proxying", function()
       strip_request_path = true,
       upstream_url = "http://mockbin.com"
     })
-    assert(helpers.prepare_prefix())
     assert(helpers.start_kong())
     client = assert(helpers.http_client("127.0.0.1", helpers.test_conf.proxy_port))
   end)

spec/02-integration/05-proxy/03-plugins_triggering_spec.lua

@@ -31,7 +31,6 @@ describe("Plugins triggering", function()
       }
     })
 
-    assert(helpers.prepare_prefix())
     assert(helpers.start_kong())
     client = assert(helpers.http_client("127.0.0.1", helpers.test_conf.proxy_port))
   end)