Repository: mageddo/dns-proxy-server
Pull request #411 (2023-04-07 19:09:33)

Fix: DPS Network autoconnect stops working 
Fixes: #408


Patch:
diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md
index d460c5e5..11f9c349 100644
--- a/RELEASE-NOTES.md
+++ b/RELEASE-NOTES.md
@@ -11,6 +11,7 @@ for more details
 * Fixing solver NPE on unsupported query types
 * Option to not comment out existing nameservers from resolv.conf [see the docs][1]
 * Option to disable remote solvers, see [the docs][2] for more details.
+* Fixing DPS auto connect feature stops working suddenly, see #408
 
 [1]: http://mageddo.github.io/dns-proxy-server/3.15/en/3-configuration/
 [2]: https://mageddo.github.io/dns-proxy-server/3.15/en/3-configuration/#no-remote-servers
diff --git a/docs/content/1-getting-started/versioning.md b/docs/content/1-getting-started/versioning.md
index d8823103..1eebb61f 100644
--- a/docs/content/1-getting-started/versioning.md
+++ b/docs/content/1-getting-started/versioning.md
@@ -5,7 +5,7 @@ weight: 3
 
 ## DPS Versioning
 
-DPS uses [semantic versioning][1] for releasing. The latest stable version is the one which is marked
+DPS uses [semantic versioning][1] \[[2][6]\] for releasing. The latest stable version is the one which is marked
 as "latest" label at Github releases or the latest docker tag. [Click here][2] to see the binary releases at Github.
 
 ## Promotion to the latest (Stable)
@@ -35,3 +35,4 @@ DPS release process was inpired on [Gradle][4] and Debian releasing process.
 [3]: https://hub.docker.com/r/defreitas/dns-proxy-server/t
 [4]: https://docs.gradle.org/7.6.1/release-notes.html
 [5]: https://github.com/mageddo/dns-proxy-server/blob/master/RELEASE-NOTES.md
+[6]: https://semver.org/
diff --git a/gradle.properties b/gradle.properties
index d4660e5b..88334ed6 100644
--- a/gradle.properties
+++ b/gradle.properties
@@ -1,1 +1,1 @@
-version=3.15.11-snapshot
+version=3.15.12-snapshot
diff --git a/src/main/java/com/mageddo/dnsproxyserver/docker/DockerNetworkDAO.java b/src/main/java/com/mageddo/dnsproxyserver/docker/DockerNetworkDAO.java
index 564d5e6e..28256abd 100644
--- a/src/main/java/com/mageddo/dnsproxyserver/docker/DockerNetworkDAO.java
+++ b/src/main/java/com/mageddo/dnsproxyserver/docker/DockerNetworkDAO.java
@@ -3,6 +3,7 @@ package com.mageddo.dnsproxyserver.docker;
 import com.github.dockerjava.api.model.Container;
 import com.github.dockerjava.api.model.ContainerNetwork;
 import com.github.dockerjava.api.model.Network;
+import com.mageddo.dnsproxyserver.docker.domain.NetworkConnectionStatus;
 import org.apache.commons.lang3.tuple.Pair;
 
 import java.util.List;
@@ -20,7 +21,10 @@ public interface DockerNetworkDAO {
 
   void disconnect(String networkId, String containerId);
 
-  void connect(String networkNameOrId, String containerId);
+  /**
+   * Lenient network connect to a container.
+   */
+  NetworkConnectionStatus connect(String networkNameOrId, String containerId);
 
   void connect(String networkNameOrId, String containerId, String ip);
 
diff --git a/src/main/java/com/mageddo/dnsproxyserver/docker/DockerNetworkDAODefault.java b/src/main/java/com/mageddo/dnsproxyserver/docker/DockerNetworkDAODefault.java
index c9d42ca7..5a99c6a2 100644
--- a/src/main/java/com/mageddo/dnsproxyserver/docker/DockerNetworkDAODefault.java
+++ b/src/main/java/com/mageddo/dnsproxyserver/docker/DockerNetworkDAODefault.java
@@ -1,10 +1,12 @@
 package com.mageddo.dnsproxyserver.docker;
 
 import com.github.dockerjava.api.DockerClient;
+import com.github.dockerjava.api.exception.DockerException;
 import com.github.dockerjava.api.model.Container;
 import com.github.dockerjava.api.model.ContainerNetwork;
 import com.github.dockerjava.api.model.Network;
 import com.mageddo.commons.lang.Objects;
+import com.mageddo.dnsproxyserver.docker.domain.NetworkConnectionStatus;
 import com.mageddo.net.Networks;
 import lombok.AllArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
@@ -18,6 +20,9 @@ import javax.inject.Singleton;
 import java.util.List;
 import java.util.function.Predicate;
 
+import static com.mageddo.dnsproxyserver.docker.domain.NetworkConnectionStatus.ALREADY_CONNECTED;
+import static com.mageddo.dnsproxyserver.docker.domain.NetworkConnectionStatus.CONNECTED;
+
 @Slf4j
 @Default
 @Singleton
@@ -82,14 +87,22 @@ public class DockerNetworkDAODefault implements DockerNetworkDAO {
   }
 
   @Override
-  public void connect(String networkNameOrId, String containerId) {
-    this.dockerClient
-      .connectToNetworkCmd()
-      .withNetworkId(networkNameOrId)
-      .withContainerId(containerId)
-      .exec()
-    ;
-    log.info("status=connected, networkNameOrId={}, containerId={}", networkNameOrId, containerId);
+  public NetworkConnectionStatus connect(String networkNameOrId, String containerId) {
+    try {
+      this.dockerClient
+        .connectToNetworkCmd()
+        .withNetworkId(networkNameOrId)
+        .withContainerId(containerId)
+        .exec()
+      ;
+      log.debug("status=connected, networkNameOrId={}, containerId={}", networkNameOrId, containerId);
+      return CONNECTED;
+    } catch (DockerException e) {
+      if (e.getMessage().contains("already exists in network")) {
+        return ALREADY_CONNECTED;
+      }
+      throw e;
+    }
   }
 
   @Override
diff --git a/src/main/java/com/mageddo/dnsproxyserver/docker/DockerNetworkService.java b/src/main/java/com/mageddo/dnsproxyserver/docker/DockerNetworkService.java
index 8069f2e7..6bee6b36 100644
--- a/src/main/java/com/mageddo/dnsproxyserver/docker/DockerNetworkService.java
+++ b/src/main/java/com/mageddo/dnsproxyserver/docker/DockerNetworkService.java
@@ -71,6 +71,7 @@ public class DockerNetworkService {
       log.info("status=won't connect dps container using conventional mode, containerId={}", containerId);
       return;
     }
-    this.networkDAO.connect(networkName, containerId);
+    final var status = this.networkDAO.connect(networkName, containerId);
+    log.info("status={}, networkName={}, containerId={}", status, networkName, containerId);
   }
 }
diff --git a/src/main/java/com/mageddo/dnsproxyserver/docker/EventListener.java b/src/main/java/com/mageddo/dnsproxyserver/docker/EventListener.java
index bc6c1b2c..4e5b4af7 100644
--- a/src/main/java/com/mageddo/dnsproxyserver/docker/EventListener.java
+++ b/src/main/java/com/mageddo/dnsproxyserver/docker/EventListener.java
@@ -57,15 +57,19 @@ public class EventListener implements StartupEvent {
 
       @Override
       public void onNext(Event event) {
-        log.debug(
-          "status=event, id={}, action={}, type={}, status={}, event={}",
-          event.getId(), event.getAction(), event.getType(), event.getStatus(), event
-        );
-        if (StringUtils.equals(event.getAction(), "start")) {
-          networkService.connect(NETWORK_DPS, event.getId());
-          return;
+        try {
+          log.debug(
+            "status=event, id={}, action={}, type={}, status={}, event={}",
+            event.getId(), event.getAction(), event.getType(), event.getStatus(), event
+          );
+          if (StringUtils.equals(event.getAction(), "start")) {
+            networkService.connect(NETWORK_DPS, event.getId());
+            return;
+          }
+          log.debug("status=eventIgnored, event={}", event);
+        } catch (Throwable e){
+          log.warn("status=errorWhenProcessingEvent, msg={}, event={}", e.getMessage(), event, e);
         }
-        log.debug("status=eventIgnore, event={}", event);
       }
 
       @Override
@@ -76,7 +80,7 @@ public class EventListener implements StartupEvent {
       public void onComplete() {
       }
     };
-    dockerClient
+    this.dockerClient
       .eventsCmd()
 //      .withEventFilter("start", "die", "stop", "destroy")
       .withEventFilter("start")
diff --git a/src/main/java/com/mageddo/dnsproxyserver/docker/domain/NetworkConnectionStatus.java b/src/main/java/com/mageddo/dnsproxyserver/docker/domain/NetworkConnectionStatus.java
new file mode 100644
index 00000000..761054e0
--- /dev/null
+++ b/src/main/java/com/mageddo/dnsproxyserver/docker/domain/NetworkConnectionStatus.java
@@ -0,0 +1,7 @@
+package com.mageddo.dnsproxyserver.docker.domain;
+
+public enum NetworkConnectionStatus {
+  ALREADY_CONNECTED,
+  CONNECTED,
+  ;
+}


Test patch:
diff --git a/src/test/java/com/mageddo/dnsproxyserver/docker/DockerNetworkDAODefaultTest.java b/src/test/java/com/mageddo/dnsproxyserver/docker/DockerNetworkDAODefaultTest.java
new file mode 100644
index 00000000..a6a5d380
--- /dev/null
+++ b/src/test/java/com/mageddo/dnsproxyserver/docker/DockerNetworkDAODefaultTest.java
@@ -0,0 +1,62 @@
+package com.mageddo.dnsproxyserver.docker;
+
+import com.github.dockerjava.api.DockerClient;
+import com.github.dockerjava.api.exception.DockerException;
+import com.mageddo.dnsproxyserver.templates.docker.DockerClientTemplates;
+import com.mageddo.http.HttpStatus;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.junit.jupiter.MockitoExtension;
+
+import static com.mageddo.dnsproxyserver.docker.domain.NetworkConnectionStatus.ALREADY_CONNECTED;
+import static com.mageddo.dnsproxyserver.docker.domain.NetworkConnectionStatus.CONNECTED;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.mockito.Mockito.doThrow;
+
+@ExtendWith(MockitoExtension.class)
+class DockerNetworkDAODefaultTest {
+
+  DockerNetworkDAODefault dao;
+
+  DockerClient dockerClient;
+
+  @BeforeEach
+  void before(){
+    this.dockerClient = DockerClientTemplates.buildSpy();
+    this.dao = new DockerNetworkDAODefault(this.dockerClient);
+  }
+
+  @Test
+  void mustConnectContainerIsToNetwork(){
+    // arrange
+    final var netName = "dps";
+    final var containerId = "a39bba9a8bab2899";
+
+    // act
+    final var status = this.dao.connect(netName, containerId);
+
+    // assert
+    assertEquals(CONNECTED, status);
+  }
+
+  @Test
+  void mustNotThrowErrorWhenContainerIsAlreadyConnectedToNetwork(){
+    // arrange
+    final var netName = "dps";
+    final var containerId = "a39bba9a8bab2899";
+
+    final var connectToNetworkCmd = this.dockerClient.connectToNetworkCmd();
+    doThrow(new DockerException("endpoint with name cobaia already exists in network dps", HttpStatus.FORBIDDEN))
+      .when(connectToNetworkCmd)
+      .exec()
+    ;
+
+    // act
+    final var status = this.dao.connect(netName, containerId);
+
+    // assert
+    assertEquals(ALREADY_CONNECTED, status);
+  }
+
+}
diff --git a/src/test/java/com/mageddo/dnsproxyserver/templates/docker/DockerClientTemplates.java b/src/test/java/com/mageddo/dnsproxyserver/templates/docker/DockerClientTemplates.java
new file mode 100644
index 00000000..d77d66e1
--- /dev/null
+++ b/src/test/java/com/mageddo/dnsproxyserver/templates/docker/DockerClientTemplates.java
@@ -0,0 +1,10 @@
+package com.mageddo.dnsproxyserver.templates.docker;
+
+import org.mockito.Mockito;
+import testing.mocks.DockerClientStub;
+
+public class DockerClientTemplates {
+  public static DockerClientStub buildSpy(){
+    return Mockito.spy(new DockerClientStub());
+  }
+}
diff --git a/src/test/java/testing/mocks/DockerClientStub.java b/src/test/java/testing/mocks/DockerClientStub.java
new file mode 100644
index 00000000..d89a292b
--- /dev/null
+++ b/src/test/java/testing/mocks/DockerClientStub.java
@@ -0,0 +1,27 @@
+package testing.mocks;
+
+import com.github.dockerjava.api.DockerClientDelegate;
+import com.github.dockerjava.api.command.ConnectToNetworkCmd;
+import com.github.dockerjava.api.command.DockerCmdSyncExec;
+import com.github.dockerjava.core.command.ConnectToNetworkCmdImpl;
+import lombok.Getter;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+
+@Getter
+public class DockerClientStub extends DockerClientDelegate {
+
+  private final ConnectToNetworkCmd connectToNetworkCmd;
+  private final DockerCmdSyncExec<?, Void> execution;
+
+  public DockerClientStub() {
+    this.execution = mock(DockerCmdSyncExec.class);
+    this.connectToNetworkCmd = spy(new ConnectToNetworkCmdImpl((DockerCmdSyncExec<ConnectToNetworkCmd, Void>) this.execution));
+  }
+
+  @Override
+  public ConnectToNetworkCmd connectToNetworkCmd() {
+    return this.connectToNetworkCmd;
+  }
+}