Fixing container inspect error (mageddo#599)

* fix inspect error

* bump docker-java version

Author: defung

build.gradle

@@ -68,8 +68,8 @@ dependencies {
   implementation('dev.failsafe:failsafe:3.3.1')
   implementation("io.github.resilience4j:resilience4j-circuitbreaker:2.2.0")
 
-  implementation('com.github.docker-java:docker-java-core:3.3.4')
-  implementation('com.github.docker-java:docker-java-transport-httpclient5:3.3.4')
+  implementation('com.github.docker-java:docker-java-core:3.4.0')
+  implementation('com.github.docker-java:docker-java-transport-httpclient5:3.4.0')
 
   implementation('info.picocli:picocli:4.7.6')
   implementation('com.fasterxml.jackson.datatype:jackson-datatype-jsr310:2.17.1')

src/main/java/com/mageddo/dnsproxyserver/docker/dataprovider/ContainerFacade.java

@@ -4,12 +4,13 @@
 import com.github.dockerjava.api.model.Container;
 
 import java.util.List;
+import java.util.Optional;
 
 public interface ContainerFacade {
 
   Container findById(String containerId);
 
   List<Container> findActiveContainers();
 
-  InspectContainerResponse inspect(String id);
+  Optional<InspectContainerResponse> inspect(String id);
 }

src/main/java/com/mageddo/dnsproxyserver/docker/dataprovider/ContainerFacadeDefault.java

@@ -2,6 +2,7 @@
 
 import com.github.dockerjava.api.DockerClient;
 import com.github.dockerjava.api.command.InspectContainerResponse;
+import com.github.dockerjava.api.exception.NotFoundException;
 import com.github.dockerjava.api.model.Container;
 import com.mageddo.dnsproxyserver.docker.application.Containers;
 import lombok.RequiredArgsConstructor;
@@ -12,6 +13,7 @@
 import javax.inject.Singleton;
 import java.util.Collections;
 import java.util.List;
+import java.util.Optional;
 
 @Slf4j
 @Default
@@ -44,7 +46,15 @@ public List<Container> findActiveContainers() {
   }
 
   @Override
-  public InspectContainerResponse inspect(String id) {
-    return this.dockerClient.inspectContainerCmd(id).exec();
+  public Optional<InspectContainerResponse> inspect(String id) {
+    try {
+      return Optional.of(this.dockerClient.inspectContainerCmd(id).exec());
+    } catch (NotFoundException e) {
+      log.warn("status=container-not-found, action=inspect-container, containerId={}", id);
+    } catch (Exception e) {
+      log.warn("status=unexpected-exception, action=inspect-container, containerId={}, msg={}", id, e.getMessage(), e);
+    }
+
+    return Optional.empty();
   }
 }

src/main/java/com/mageddo/dnsproxyserver/solver/docker/dataprovider/ContainerDAODefault.java

@@ -28,7 +28,7 @@ public class ContainerDAODefault implements ContainerDAO {
   public List<Container> findActiveContainersMatching(HostnameQuery query) {
     return this.containerFacade.findActiveContainers()
       .stream()
-      .map(it -> this.containerFacade.inspect(it.getId()))
+      .flatMap(it -> this.containerFacade.inspect(it.getId()).stream())
       .filter(ContainerHostnameMatcher.buildPredicate(query))
       .map(ContainerMapper::of)
       .toList();

src/main/java/com/mageddo/dnsproxyserver/solver/docker/dataprovider/DpsContainerDAODefault.java

@@ -55,7 +55,7 @@ public Container findDPSContainer() {
     return containers
       .stream()
       .findFirst()
-      .map(it -> this.containerFacade.inspect(it.getId()))
+      .flatMap(it -> this.containerFacade.inspect(it.getId()))
       .map(ContainerMapper::of)
       .orElse(null);
   }

src/test/java/com/mageddo/dnsproxyserver/docker/ContainerFacadeDefaultTest.java

@@ -0,0 +1,152 @@
+package com.mageddo.dnsproxyserver.docker;
+
+import com.github.dockerjava.api.DockerClient;
+import com.github.dockerjava.api.exception.NotFoundException;
+import com.github.dockerjava.api.model.Container;
+import com.mageddo.dnsproxyserver.docker.dataprovider.ContainerFacadeDefault;
+import testing.templates.docker.ContainerTemplates;
+import testing.templates.docker.DockerClientTemplates;
+
+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 testing.templates.docker.InspectContainerResponseTemplates;
+
+import java.util.ArrayList;
+import java.util.Optional;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.doThrow;
+
+@ExtendWith(MockitoExtension.class)
+class ContainerFacadeDefaultTest {
+
+  ContainerFacadeDefault dao;
+
+  DockerClient dockerClient;
+
+  @BeforeEach
+  void before(){
+    this.dockerClient = DockerClientTemplates.buildSpy();
+    this.dao = new ContainerFacadeDefault(this.dockerClient);
+  }
+
+  @Test
+  void mustFindContainerById(){
+    // arrange
+    final var mockReturn = new ArrayList<Container>();
+    mockReturn.add(ContainerTemplates.buildDpsContainer());
+    mockReturn.add(ContainerTemplates.buildRegularContainerCoffeeMakerCheckout());
+
+    final var containerId = mockReturn.get(0).getId();
+
+    final var inspectContainerCmd = this.dockerClient.listContainersCmd();
+    doReturn(mockReturn)
+      .when(inspectContainerCmd)
+      .exec()
+    ;
+
+    // act
+    final var container = this.dao.findById(containerId);
+
+    // assert
+    assertEquals(mockReturn.get(0), container);
+  }
+
+  @Test
+  void mustReturnNullWhenFindContainerByIdNotFound(){
+    // arrange
+    final var mockReturn = new ArrayList<Container>();
+
+    final var containerId = "abc123";
+
+    final var listContainerCmd = this.dockerClient.listContainersCmd();
+    doReturn(mockReturn)
+      .when(listContainerCmd)
+      .exec()
+    ;
+
+    // act
+    final var container = this.dao.findById(containerId);
+
+    // assert
+    assertNull(container);
+  }
+
+  @Test
+  void mustListActiveContainers(){
+    // arrange
+    final var expected = new ArrayList<Container>();
+    expected.add(ContainerTemplates.buildRegularContainerCoffeeMakerCheckout());
+
+    final var listContainerCmd = this.dockerClient.listContainersCmd();
+    doReturn(expected)
+      .when(listContainerCmd)
+      .exec()
+    ;
+
+    // act
+    final var findActiveContainersResponse = this.dao.findActiveContainers();
+
+    // assert
+    assertEquals(expected, findActiveContainersResponse);
+  }
+
+  @Test
+  void mustInspectContainerById(){
+    // arrange
+    final var expected = InspectContainerResponseTemplates.ngixWithDefaultBridgeNetworkOnly();
+    final var containerId = expected.getId();
+
+    final var inspectContainerCmd = this.dockerClient.inspectContainerCmd(containerId);
+    doReturn(expected)
+      .when(inspectContainerCmd)
+      .exec()
+    ;
+
+    // act
+    final var inspectResponse = this.dao.inspect(containerId).orElseThrow();
+
+    // assert
+    assertEquals(expected, inspectResponse);
+  }
+
+  @Test
+  void mustNotThrowErrorWhenInspectContainerNotFound(){
+    // arrange
+    final var containerId = "a39bba9a8bab2899";
+
+    final var inspectContainerCmd = this.dockerClient.inspectContainerCmd(containerId);
+    doThrow(new NotFoundException("Container not found"))
+      .when(inspectContainerCmd)
+      .exec()
+    ;
+
+    // act
+    final var container = this.dao.inspect(containerId);
+
+    // assert
+    assertEquals(Optional.empty(), container);
+  }
+
+  @Test
+  void mustNotThrowErrorWhenInspectContainerFails(){
+    // arrange
+    final var containerId = "a39bba9a8bab28aa";
+
+    final var inspectContainerCmd = this.dockerClient.inspectContainerCmd(containerId);
+    doThrow(new NullPointerException("Unexpected failure"))
+      .when(inspectContainerCmd)
+      .exec()
+    ;
+
+    // act
+    final var container = this.dao.inspect(containerId);
+
+    // assert
+    assertEquals(Optional.empty(), container);
+  }
+}

src/test/java/testing/mocks/DockerClientStub.java

@@ -3,25 +3,60 @@
 import com.github.dockerjava.api.DockerClientDelegate;
 import com.github.dockerjava.api.command.ConnectToNetworkCmd;
 import com.github.dockerjava.api.command.DockerCmdSyncExec;
+import com.github.dockerjava.api.command.InspectContainerCmd;
+import com.github.dockerjava.api.command.ListContainersCmd;
 import com.github.dockerjava.core.command.ConnectToNetworkCmdImpl;
+import com.github.dockerjava.core.command.InspectContainerCmdImpl;
+import com.github.dockerjava.core.command.ListContainersCmdImpl;
 import lombok.Getter;
 
+import javax.annotation.Nonnull;
+
+import java.util.HashMap;
+import java.util.Map;
+
 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;
+  private final ListContainersCmd listContainersCmd;
+  private final Map<String, InspectContainerCmd> inspectContainerCmdMap;
+
+  private final DockerCmdSyncExec<ConnectToNetworkCmd, Void> connectToNetworkExecution;
+  private final ListContainersCmd.Exec listContainersExecution;
+  private final InspectContainerCmd.Exec inspectContainerExecution;
 
   public DockerClientStub() {
-    this.execution = mock(DockerCmdSyncExec.class);
-    this.connectToNetworkCmd = spy(new ConnectToNetworkCmdImpl((DockerCmdSyncExec<ConnectToNetworkCmd, Void>) this.execution));
+    this.connectToNetworkExecution = mock(DockerCmdSyncExec.class);
+    this.listContainersExecution = mock(ListContainersCmd.Exec.class);
+    this.inspectContainerExecution = mock(InspectContainerCmd.Exec.class);
+
+    this.connectToNetworkCmd = spy(new ConnectToNetworkCmdImpl(this.connectToNetworkExecution));
+    this.listContainersCmd = spy(new ListContainersCmdImpl(this.listContainersExecution));
+    this.inspectContainerCmdMap = new HashMap<>();
   }
 
   @Override
   public ConnectToNetworkCmd connectToNetworkCmd() {
     return this.connectToNetworkCmd;
   }
+
+  @Override
+  public ListContainersCmd listContainersCmd() {
+    return this.listContainersCmd;
+  }
+
+  @Override
+  public InspectContainerCmd inspectContainerCmd(@Nonnull String containerId) {
+    if (this.inspectContainerCmdMap.containsKey(containerId)) {
+      return this.inspectContainerCmdMap.get(containerId);
+    } else {
+      final var inspectCmd = spy(new InspectContainerCmdImpl(this.inspectContainerExecution, containerId));
+      inspectContainerCmdMap.put(containerId, inspectCmd);
+      return inspectCmd;
+    }
+  }
 }