Address gemini code review

Author: mesudip

nginx_proxy/WebServer.py

@@ -250,7 +250,7 @@ def disconnect(self, network, container, scope):
             if self.config_data.has_backend(container):
                 try:
                     backend = BackendTarget.from_container(self.client.containers.get(container))
-                    if not self.update_backend(backend):
+                    if not self.update_backend(backend, replace_existing=True):
                         self.remove_backend(
                             container
                         )  # remove_backend not implemented yet, using remove_container (it takes ID)
@@ -286,23 +286,23 @@ def connect(self, network, container, scope):
                     # print(f"Skipping network connect for service task container {container}")
                     return
                 backend = BackendTarget.from_container(container_obj)
-                self.update_backend(backend)
+                self.update_backend(backend, replace_existing=True)
             except docker.errors.NotFound:
                 return
             except (KeyboardInterrupt, SystemExit):
                 raise
             except Exception as e:
                 print(f"Error processing connect for container {container}: {e}", file=sys.stderr)
 
-    def update_backend(self, backend: BackendTarget):
+    def update_backend(self, backend: BackendTarget, replace_existing: bool = False):
         """
         Rescan the backend to detect changes. And update nginx configuration if necessary.
         :param backend: BackendTarget object
         :return: true if state change affected the nginx configuration else false
         """
         try:
             existing_backend = self.config_data.has_backend(backend.id)
-            if existing_backend and backend.type != "service":
+            if existing_backend and backend.type != "service" and not replace_existing:
                 return False
 
             removed = None

tests/integration/conftest.py

@@ -85,8 +85,8 @@ def test_network(docker_client: docker.DockerClient, swarm_mode):
 
 @pytest.fixture(
     scope="session",
-    params=["ignore", "exclude", "enable", "strict"],
-    ids=["swarm_ignore", "swarm_exclude", "swarm_enable", "swarm_strict"],
+    params=["ignore", "exclude", "enable", "prefer-local", "strict"],
+    ids=["swarm_ignore", "swarm_exclude", "swarm_enable", "swarm_prefer_local", "swarm_strict"],
 )
 def swarm_mode(request):
     return request.param

tests/integration/test_nginx_proxy.py

@@ -19,7 +19,7 @@ def is_reachable(swarm_mode, backend_type):
     """
     Determines if a backend should be reachable based on swarm mode and backend type.
     """
-    if swarm_mode in ("enable", "ignore"):
+    if swarm_mode in ("enable", "ignore", "prefer-local"):
         return True
     if swarm_mode == "strict" and backend_type == "service":
         return True

tests/unit/test_web_server.py

@@ -134,6 +134,22 @@ def test_update_backend_ignores_existing_container_backend(web_server):
     mock_throttle.assert_not_called()
 
 
+def test_update_backend_replaces_existing_container_backend_when_requested(web_server):
+    web_server.networks = {"frontend": "frontend-id", "frontend-id": "frontend"}
+    web_server.config_data = ProxyConfigData()
+    existing = _backend_target("container1", "old.example.com", "172.18.0.2")
+    updated = _backend_target("container1", "new.example.com", "172.18.0.3")
+    web_server.register_backend(existing)
+
+    with patch.object(web_server.throttler, "throttle") as mock_throttle:
+        changed = web_server.update_backend(updated, replace_existing=True)
+
+    assert changed is True
+    assert web_server.config_data.getHost("old.example.com").isempty()
+    assert web_server.config_data.getHost("new.example.com") is not None
+    mock_throttle.assert_called_once()
+
+
 def test_update_backend_replaces_existing_service_backend(web_server):
     web_server.networks = {"frontend": "frontend-id", "frontend-id": "frontend"}
     web_server.config_data = ProxyConfigData()

tests/unit/test_webserver_events.py

@@ -359,6 +359,34 @@ def test_webserver_remove_network(docker_client: DockerTestClient, nginx: DummyN
     expect_server_down(nginx, hostname)
 
 
+def test_webserver_disconnect_keeps_backend_when_another_proxy_network_remains(
+    docker_client: DockerTestClient, webserver: WebServer, nginx: DummyNginx
+):
+    container_name = "multi_reachable_network_container"
+    hostname = "multi-reachable-network.example.com"
+    env = {
+        "VIRTUAL_HOST": hostname,
+    }
+
+    alt_network = docker_client.networks.create("frontend_alt")
+    webserver.networks[alt_network.id] = alt_network.name
+    webserver.networks[alt_network.name] = alt_network.id
+
+    container = docker_client.containers.run("nginx:alpine", name=container_name, environment=env, network="frontend")
+    time.sleep(0.2)
+
+    expect_server_up(nginx, hostname)
+
+    alt_network.connect(container.id)
+    time.sleep(0.2)
+
+    frontend_network = docker_client.networks.get("frontend")
+    frontend_network.disconnect(container.id)
+    time.sleep(0.2)
+
+    expect_server_up(nginx, hostname)
+
+
 def test_webserver_recreate_same_name_container_with_different_host(docker_client: DockerTestClient, nginx: DummyNginx):
     container_name = "test_container"
     old_hostname = "old.recreate.example.com"