refactor(balancer) drop the orderlist property (#2748)

This removes the `orderlist` property from the balancer entity. Due to a different implementation in the dns library, it is no longer required.

from #2748

kong/core/balancer.lua

@@ -176,7 +176,6 @@ local get_balancer = function(target)
     -- no balancer yet (or invalidated) so create a new one
     balancer, err = ring_balancer.new({
         wheelSize = upstream.slots,
-        order = upstream.orderlist,
         dns = dns_client,
       })
 
@@ -222,7 +221,6 @@ local get_balancer = function(target)
       -- for now; create a new balancer from scratch
       balancer, err = ring_balancer.new({
           wheelSize = upstream.slots,
-          order = upstream.orderlist,
           dns = dns_client,
         })
       if not balancer then

kong/dao/migrations/cassandra.lua

@@ -474,4 +474,11 @@ return {
       DROP TABLE nodes;
     ]],
   },
+  {
+    name = "2017-07-28-225000_balancer_orderlist_remove",
+    up = [[
+      ALTER TABLE upstreams DROP orderlist;
+    ]],
+    down = function(_, _, dao) end  -- not implemented
+  },
 }

kong/dao/migrations/postgres.lua

@@ -526,4 +526,11 @@ return {
       DROP INDEX ttls_primary_uuid_value_idx;
     ]]
   },
+  {
+    name = "2017-07-28-225000_balancer_orderlist_remove",
+    up = [[
+      ALTER TABLE upstreams DROP COLUMN IF EXISTS orderlist;
+    ]],
+    down = function(_, _, dao) end  -- not implemented
+  },
 }

kong/dao/schemas/upstreams.lua

@@ -31,13 +31,6 @@ return {
       type = "number",
       default = DEFAULT_SLOTS,
     },
-    orderlist = {
-      -- a list of sequential, but randomly ordered, integer numbers. In the datastore
-      -- because all Kong nodes need the exact-same 'randomness'. If changed, consistency is lost.
-      -- must have exactly `slots` number of entries.
-      type = "array",
-      default = {},
-    }
   },
   self_check = function(schema, config, dao, is_updating)
 
@@ -58,59 +51,6 @@ return {
       return false, Errors.schema(SLOTS_MSG)
     end
 
-    -- check the order array
-    local order = config.orderlist
-    if #order == config.slots then
-      -- array size unchanged, check consistency
-
-      local t = utils.shallow_copy(order)
-      table.sort(t)
-      local count, max = 0, 0
-      for i, v in pairs(t) do
-        if i ~= v then
-          return false, Errors.schema("invalid orderlist")
-        end
-
-        count = count + 1
-        if i > max then
-          max = i
-        end
-      end
-
-      if count ~= config.slots or max ~= config.slots then
-        return false, Errors.schema("invalid orderlist")
-      end
-
-    else
-      -- size mismatch
-      if #order > 0 then
-        -- size given, but doesn't match the size of the also given orderlist
-        return false, Errors.schema("size mismatch between 'slots' and 'orderlist'")
-      end
-
-      -- No list given, generate order array
-      local t = {}
-      for i = 1, config.slots do
-        t[i] = {
-          id = i,
-          order = math.random(1, config.slots),
-        }
-      end
-
-      -- sort the array (we don't check for -accidental- duplicates as the
-      -- id field is used for the order and that one is always unique)
-      table.sort(t, function(a,b)
-        return a.order < b.order
-      end)
-
-      -- replace the created 'record' with only the id
-      for i, v in ipairs(t) do
-        t[i] = v.id
-      end
-
-      config.orderlist = t
-    end
-
     return true
   end,
 }

kong/init.lua

@@ -62,6 +62,10 @@ local kong_error_handlers = require "kong.core.error_handlers"
 
 local ngx              = ngx
 local header           = ngx.header
+local ngx_log          = ngx.log
+local ngx_ERR          = ngx.ERR
+local ngx_CRIT         = ngx.CRIT
+local ngx_DEBUG        = ngx.DEBUG
 local ipairs           = ipairs
 local assert           = assert
 local tostring         = tostring
@@ -73,7 +77,7 @@ local set_more_tries   = ngx_balancer.set_more_tries
 local function load_plugins(kong_conf, dao)
   local in_db_plugins, sorted_plugins = {}, {}
 
-  ngx.log(ngx.DEBUG, "Discovering used plugins")
+  ngx_log(ngx_DEBUG, "Discovering used plugins")
 
   local rows, err_t = dao.plugins:find_all()
   if not rows then
@@ -101,7 +105,7 @@ local function load_plugins(kong_conf, dao)
       return nil, "no configuration schema found for plugin: " .. plugin
     end
 
-    ngx.log(ngx.DEBUG, "Loading plugin: " .. plugin)
+    ngx_log(ngx_DEBUG, "Loading plugin: " .. plugin)
 
     sorted_plugins[#sorted_plugins+1] = {
       name = plugin,
@@ -177,7 +181,7 @@ function Kong.init_worker()
 
   local ok, err = singletons.dao:init_worker()
   if not ok then
-    ngx.log(ngx.CRIT, "could not init DB: ", err)
+    ngx_log(ngx_CRIT, "could not init DB: ", err)
     return
   end
 
@@ -197,7 +201,7 @@ function Kong.init_worker()
     wait_max = 0.5,         -- max wait time before discarding event
   }
   if not ok then
-    ngx.log(ngx.CRIT, "could not start inter-worker events: ", err)
+    ngx_log(ngx_CRIT, "could not start inter-worker events: ", err)
     return
   end
 
@@ -215,7 +219,7 @@ function Kong.init_worker()
     poll_offset             = configuration.db_update_propagation,
   }
   if not cluster_events then
-    ngx.log(ngx.CRIT, "could not create cluster_events: ", err)
+    ngx_log(ngx_CRIT, "could not create cluster_events: ", err)
     return
   end
 
@@ -235,15 +239,15 @@ function Kong.init_worker()
     },
   }
   if not cache then
-    ngx.log(ngx.CRIT, "could not create kong cache: ", err)
+    ngx_log(ngx_CRIT, "could not create kong cache: ", err)
     return
   end
 
   local ok, err = cache:get("router:version", { ttl = 0 }, function()
     return "init"
   end)
   if not ok then
-    ngx.log(ngx.CRIT, "could not set router version in cache: ", err)
+    ngx_log(ngx_CRIT, "could not set router version in cache: ", err)
     return
   end
 
@@ -297,7 +301,7 @@ function Kong.balancer()
 
     local ok, err = balancer_execute(addr)
     if not ok then
-      ngx.log(ngx.ERR, "failed to retry the dns/balancer resolver for ",
+      ngx_log(ngx_ERR, "failed to retry the dns/balancer resolver for ",
               tostring(addr.host), "' with: ", tostring(err))
 
       return responses.send(500)
@@ -315,9 +319,11 @@ function Kong.balancer()
   current_try.port = addr.port
 
   -- set the targets as resolved
+  ngx_log(ngx_DEBUG, "setting address (try ", addr.try_count, "): ",
+                     addr.ip, ":", addr.port)
   local ok, err = set_current_peer(addr.ip, addr.port)
   if not ok then
-    ngx.log(ngx.ERR, "failed to set the current peer (address: ",
+    ngx_log(ngx_ERR, "failed to set the current peer (address: ",
             tostring(addr.ip), " port: ", tostring(addr.port),"): ",
             tostring(err))
 
@@ -328,7 +334,7 @@ function Kong.balancer()
                          addr.send_timeout / 1000,
                          addr.read_timeout / 1000)
   if not ok then
-    ngx.log(ngx.ERR, "could not set upstream timeouts: ", err)
+    ngx_log(ngx_ERR, "could not set upstream timeouts: ", err)
   end
 
   core.balancer.after()

spec/01-unit/007-entities_schemas_spec.lua

@@ -5,7 +5,6 @@ local targets_schema = require "kong.dao.schemas.targets"
 local upstreams_schema = require "kong.dao.schemas.upstreams"
 local validations = require "kong.dao.schemas_validation"
 local validate_entity = validations.validate_entity
-local utils = require "kong.tools.utils"
 
 describe("Entities Schemas", function()
 
@@ -781,94 +780,6 @@ describe("Entities Schemas", function()
       end
     end)
 
-    it("should require (optional) orderlist to be a proper list", function()
-      local data, valid, errors, check
-      local function validate_order(list, size)
-        assert(type(list) == "table", "expected list table, got " .. type(list))
-        assert(next(list), "table is empty")
-        assert(type(size) == "number", "expected size number, got " .. type(size))
-        assert(size > 0, "expected size to be > 0")
-        local c = {}
-        local max = 0
-        for i,v in pairs(list) do  --> note: pairs, not ipairs!!
-          if i > max then max = i end
-          c[i] = v
-        end
-        assert(max == size, "highest key is not equal to the size")
-        table.sort(c)
-        max = 0
-        for i, v in ipairs(c) do
-          assert(i == v, "expected sorted table to have equal keys and values")
-          if i>max then max = i end
-        end
-        assert(max == size, "expected array, but got list with holes")
-      end
-
-      for _ = 1, 20 do  -- have Kong generate 20 random sized arrays and verify them
-        data = {
-          name = "valid.host.name",
-          slots = math.random(slots_min, slots_max)
-        }
-        valid, errors, check = validate_entity(data, upstreams_schema)
-        assert.is_true(valid)
-        assert.is_nil(errors)
-        assert.is_nil(check)
-        validate_order(data.orderlist, data.slots)
-      end
-
-      local lst = { 9,7,5,3,1,2,4,6,8,10 }   -- a valid list
-      data = {
-        name = "valid.host.name",
-        slots = 10,
-        orderlist = utils.shallow_copy(lst)
-      }
-      valid, errors, check = validate_entity(data, upstreams_schema)
-      assert.is_true(valid)
-      assert.is_nil(errors)
-      assert.is_nil(check)
-      assert.same(lst, data.orderlist)
-
-      data = {
-        name = "valid.host.name",
-        slots = 10,
-        orderlist = { 9,7,5,3,1,2,4,6,8 }   -- too short (9)
-      }
-      valid, errors, check = validate_entity(data, upstreams_schema)
-      assert.is_false(valid)
-      assert.is_nil(errors)
-      assert.are.equal("size mismatch between 'slots' and 'orderlist'",check.message)
-
-      data = {
-        name = "valid.host.name",
-        slots = 10,
-        orderlist = { 9,7,5,3,1,2,4,6,8,10,11 }   -- too long (11)
-      }
-      valid, errors, check = validate_entity(data, upstreams_schema)
-      assert.is_false(valid)
-      assert.is_nil(errors)
-      assert.are.equal("size mismatch between 'slots' and 'orderlist'",check.message)
-
-      data = {
-        name = "valid.host.name",
-        slots = 10,
-        orderlist = { 9,7,5,3,1,2,4,6,8,8 }   -- a double value (2x 8, no 10)
-      }
-      valid, errors, check = validate_entity(data, upstreams_schema)
-      assert.is_false(valid)
-      assert.is_nil(errors)
-      assert.are.equal("invalid orderlist",check.message)
-
-      data = {
-        name = "valid.host.name",
-        slots = 10,
-        orderlist = { 9,7,5,3,1,2,4,6,8,11 }   -- a hole (10 missing)
-      }
-      valid, errors, check = validate_entity(data, upstreams_schema)
-      assert.is_false(valid)
-      assert.is_nil(errors)
-      assert.are.equal("invalid orderlist",check.message)
-    end)
-
   end)
 
   --