Merge pull request #6600 from nic-6443/backend-sync-503-fix

Bugfix: some requests fail with 503 when nginx reload

rootfs/etc/nginx/lua/balancer.lua

@@ -107,6 +107,12 @@ local function sync_backend(backend)
     return
   end
 
+  if is_backend_with_external_name(backend) then
+    backend = resolve_external_names(backend)
+  end
+
+  backend.endpoints = format_ipv6_endpoints(backend.endpoints)
+
   local implementation = get_implementation(backend)
   local balancer = balancers[backend.name]
 
@@ -126,24 +132,21 @@ local function sync_backend(backend)
     return
   end
 
-  if is_backend_with_external_name(backend) then
-    backend = resolve_external_names(backend)
-  end
-
-  backend.endpoints = format_ipv6_endpoints(backend.endpoints)
-
   balancer:sync(backend)
 end
 
+local function sync_backends_with_external_name()
+  for _, backend_with_external_name in pairs(backends_with_external_name) do
+    sync_backend(backend_with_external_name)
+  end
+end
+
 local function sync_backends()
   local raw_backends_last_synced_at = configuration.get_raw_backends_last_synced_at()
   ngx.update_time()
   local current_timestamp = ngx.time()
   if current_timestamp - backends_last_synced_at < BACKENDS_FORCE_SYNC_INTERVAL
       and raw_backends_last_synced_at <= backends_last_synced_at then
-    for _, backend_with_external_name in pairs(backends_with_external_name) do
-      sync_backend(backend_with_external_name)
-    end
     return
   end
 
@@ -161,12 +164,13 @@ local function sync_backends()
 
   local balancers_to_keep = {}
   for _, new_backend in ipairs(new_backends) do
-    sync_backend(new_backend)
-    balancers_to_keep[new_backend.name] = balancers[new_backend.name]
     if is_backend_with_external_name(new_backend) then
       local backend_with_external_name = util.deepcopy(new_backend)
       backends_with_external_name[backend_with_external_name.name] = backend_with_external_name
+    else
+      sync_backend(new_backend)
     end
+    balancers_to_keep[new_backend.name] = true
   end
 
   for backend_name, _ in pairs(balancers) do
@@ -272,11 +276,12 @@ local function get_balancer()
 end
 
 function _M.init_worker()
-  -- when worker starts, sync backends without delay
-  -- we call it in timer because for endpoints that require
+  -- when worker starts, sync non ExternalName backends without delay
+  sync_backends()
+  -- we call sync_backends_with_external_name in timer because for endpoints that require
   -- DNS resolution it needs to use socket which is not available in
   -- init_worker phase
-  local ok, err = ngx.timer.at(0, sync_backends)
+  local ok, err = ngx.timer.at(0, sync_backends_with_external_name)
   if not ok then
     ngx.log(ngx.ERR, "failed to create timer: ", err)
   end
@@ -285,6 +290,11 @@ function _M.init_worker()
   if not ok then
     ngx.log(ngx.ERR, "error when setting up timer.every for sync_backends: ", err)
   end
+  ok, err = ngx.timer.every(BACKENDS_SYNC_INTERVAL, sync_backends_with_external_name)
+  if not ok then
+    ngx.log(ngx.ERR, "error when setting up timer.every for sync_backends_with_external_name: ",
+            err)
+  end
 end
 
 function _M.rewrite()

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

@@ -1,3 +1,5 @@
+local cjson = require("cjson.safe")
+local util = require("util")
 
 local balancer, expected_implementations, backends
 local original_ngx = ngx
@@ -339,7 +341,9 @@ describe("Balancer", function()
       local mock_instance = { sync = function(backend) end }
       setmetatable(mock_instance, implementation)
       implementation.new = function(self, backend) return mock_instance end
+      local s = spy.on(implementation, "new")
       assert.has_no.errors(function() balancer.sync_backend(backend) end)
+      assert.spy(s).was_called_with(implementation, expected_backend)
       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)
@@ -364,9 +368,11 @@ describe("Balancer", function()
       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)
+      local s = spy.on(implementation, "new")
+      assert.has_no.errors(function() balancer.sync_backend(util.deepcopy(backend)) end)
+      assert.spy(s).was_called_with(implementation, expected_backend)
       stub(mock_instance, "sync")
-      assert.has_no.errors(function() balancer.sync_backend(backend) end)
+      assert.has_no.errors(function() balancer.sync_backend(util.deepcopy(backend)) end)
       assert.stub(mock_instance.sync).was_called_with(mock_instance, expected_backend)
     end)
 
@@ -400,4 +406,38 @@ describe("Balancer", function()
       assert.stub(mock_instance.sync).was_called_with(mock_instance, backend)
     end)
   end)
+
+  describe("sync_backends()", function()
+
+    after_each(function()
+      reset_ngx()
+    end)
+
+    it("sync backends", function()
+      backends = {
+        {
+          name = "access-router-production-web-80", port = "80", secure = false,
+          sslPassthrough = false,
+          endpoints = {
+            { address = "10.184.7.40", port = "8080", maxFails = 0, failTimeout = 0 },
+            { address = "10.184.97.100", port = "8080", maxFails = 0, failTimeout = 0 },
+            { address = "10.184.98.239", port = "8080", maxFails = 0, failTimeout = 0 },
+          },
+          sessionAffinityConfig = { name = "", cookieSessionAffinity = { name = "" } },
+          trafficShapingPolicy = {
+            weight = 0,
+            header = "",
+            headerValue = "",
+            cookie = ""
+          },
+        }
+      }
+      mock_ngx({ var = { proxy_upstream_name = "access-router-production-web-80" }, ctx = { } })
+      ngx.shared.configuration_data:set("backends", cjson.encode(backends))
+      reset_balancer()
+      balancer.init_worker()
+      assert.not_equal(balancer.get_balancer(), nil)
+    end)
+
+  end)
 end)