Merge pull request #2857 from ElvinEfendi/more-lua-refactoring-and-tests

remove unnecessary encoding/decoding also fix ipv6 issue

rootfs/etc/nginx/lua/balancer.lua

@@ -55,6 +55,18 @@ local function resolve_external_names(original_backend)
   return backend
 end
 
+local function format_ipv6_endpoints(endpoints)
+  local formatted_endpoints = {}
+  for _, endpoint in ipairs(endpoints) do
+    local formatted_endpoint = endpoint
+    if not endpoint.address:match("^%d+.%d+.%d+.%d+$") then
+      formatted_endpoint.address = string.format("[%s]", endpoint.address)
+    end
+    table.insert(formatted_endpoints, formatted_endpoint)
+  end
+  return formatted_endpoints
+end
+
 local function sync_backend(backend)
   local implementation = get_implementation(backend)
   local balancer = balancers[backend.name]
@@ -79,6 +91,8 @@ local function sync_backend(backend)
     backend = resolve_external_names(backend)
   end
 
+  backend.endpoints = format_ipv6_endpoints(backend.endpoints)
+
   balancer:sync(backend)
 end
 
@@ -135,18 +149,17 @@ function _M.balance()
     return
   end
 
-  local host, port = balancer:balance()
-  if not (host and port) then
-    ngx.log(ngx.WARN,
-      string.format("host or port is missing, balancer: %s, host: %s, port: %s", balancer.name, host, port))
+  local peer = balancer:balance()
+  if not peer then
+    ngx.log(ngx.WARN, "no peer was returned, balancer: " .. balancer.name)
     return
   end
 
   ngx_balancer.set_more_tries(1)
 
-  local ok, err = ngx_balancer.set_current_peer(host, port)
+  local ok, err = ngx_balancer.set_current_peer(peer)
   if not ok then
-    ngx.log(ngx.ERR, "error while setting current upstream peer to " .. tostring(err))
+    ngx.log(ngx.ERR, string.format("error while setting current upstream peer %s: %s", peer, err))
   end
 end
 

rootfs/etc/nginx/lua/balancer/chash.lua

@@ -1,7 +1,6 @@
 local balancer_resty = require("balancer.resty")
 local resty_chash = require("resty.chash")
 local util = require("util")
-local split = require("util.split")
 
 local _M = balancer_resty:new({ factory = resty_chash, name = "chash" })
 
@@ -15,8 +14,7 @@ end
 
 function _M.balance(self)
   local key = util.lua_ngx_var(self.hash_by)
-  local endpoint_string = self.instance:find(key)
-  return split.split_pair(endpoint_string, ":")
+  return self.instance:find(key)
 end
 
 return _M

rootfs/etc/nginx/lua/balancer/ewma.lua

@@ -128,7 +128,8 @@ function _M.balance(self)
     endpoint = pick_and_score(peer_copy, k)
   end
 
-  return endpoint.address, endpoint.port
+  -- TODO(elvinefendi) move this processing to _M.sync
+  return endpoint.address .. ":" .. endpoint.port
 end
 
 function _M.after_balance(_)

rootfs/etc/nginx/lua/balancer/round_robin.lua

@@ -1,7 +1,6 @@
 local balancer_resty = require("balancer.resty")
 local resty_roundrobin = require("resty.roundrobin")
 local util = require("util")
-local split = require("util.split")
 
 local _M = balancer_resty:new({ factory = resty_roundrobin, name = "round_robin" })
 
@@ -14,8 +13,7 @@ function _M.new(self, backend)
 end
 
 function _M.balance(self)
-  local endpoint_string = self.instance:find()
-  return split.split_pair(endpoint_string, ":")
+  return self.instance:find()
 end
 
 return _M

rootfs/etc/nginx/lua/balancer/sticky.lua

@@ -1,7 +1,6 @@
 local balancer_resty = require("balancer.resty")
 local resty_chash = require("resty.chash")
 local util = require("util")
-local split = require("util.split")
 local ck = require("resty.cookie")
 
 local _M = balancer_resty:new({ factory = resty_chash, name = "sticky" })
@@ -56,7 +55,7 @@ local function pick_random(instance)
   return instance:next(index)
 end
 
-local function sticky_endpoint_string(self)
+function _M.balance(self)
   local cookie, err = ck:new()
   if not cookie then
     ngx.log(ngx.ERR, err)
@@ -73,9 +72,4 @@ local function sticky_endpoint_string(self)
   return self.instance:find(key)
 end
 
-function _M.balance(self)
-  local endpoint_string = sticky_endpoint_string(self)
-  return split.split_pair(endpoint_string, ":")
-end
-
 return _M

rootfs/etc/nginx/lua/test/balancer/chash_test.lua

@@ -21,9 +21,8 @@ describe("Balancer chash", function()
       }
       local instance = balancer_chash:new(backend)
 
-      local host, port = instance:balance()
-      assert.equal("10.184.7.40", host)
-      assert.equal("8080", port)
+      local peer = instance:balance()
+      assert.equal("10.184.7.40:8080", peer)
     end)
   end)
 end)

rootfs/etc/nginx/lua/test/balancer/ewma_test.lua

@@ -11,9 +11,8 @@ describe("Balancer ewma", function()
       }
       local instance = balancer_ewma:new(backend)
 
-      local host, port = instance:balance()
-      assert.equal("10.184.7.40", host)
-      assert.equal("8080", port)
+      local peer = instance:balance()
+      assert.equal("10.184.7.40:8080", peer)
     end)
 
     it("picks the endpoint with lowest score when there two of them", function()
@@ -32,9 +31,8 @@ describe("Balancer ewma", function()
       ngx.shared.balancer_ewma_last_touched_at.get = function(self, key) return t end
 
 
-      local host, port = instance:balance()
-      assert.equal("10.184.97.100", host)
-      assert.equal("8080", port)
+      local peer = instance:balance()
+      assert.equal("10.184.97.100:8080", peer)
     end)
   end)
 

rootfs/etc/nginx/lua/test/balancer_test.lua

@@ -115,6 +115,31 @@ describe("Balancer", function()
       assert.stub(mock_instance.sync).was_called_with(mock_instance, expected_backend)
     end)
 
+    it("wraps IPv6 addresses into square brackets", function()
+      local backend = {
+        name = "exmaple-com",
+        endpoints = {
+          { address = "::1", port = "8080", maxFails = 0, failTimeout = 0 },
+          { address = "192.168.1.1", port = "8080", maxFails = 0, failTimeout = 0 },
+        }
+      }
+      local expected_backend = {
+        name = "exmaple-com",
+        endpoints = {
+          { address = "[::1]", port = "8080", maxFails = 0, failTimeout = 0 },
+          { address = "192.168.1.1", port = "8080", maxFails = 0, failTimeout = 0 },
+        }
+      }
+
+      local mock_instance = { sync = function(backend) end }
+      setmetatable(mock_instance, implementation)
+      implementation.new = function(self, backend) return mock_instance end
+      assert.has_no.errors(function() balancer.sync_backend(backend) end)
+      stub(mock_instance, "sync")
+      assert.has_no.errors(function() balancer.sync_backend(backend) end)
+      assert.stub(mock_instance.sync).was_called_with(mock_instance, expected_backend)
+    end)
+
     it("replaces the existing balancer when load balancing config changes for backend", function()
       assert.has_no.errors(function() balancer.sync_backend(backend) end)
 

rootfs/etc/nginx/lua/test/run.lua

@@ -29,4 +29,7 @@ ngx.socket.tcp = function(...)
   return socket
 end
 
+ngx.log = function(...) end
+ngx.print = function(...) end
+
 require "busted.runner"({ standalone = false })