Merge pull request #605 from mageddo/develop

Develop Merge

RELEASE-NOTES.md

@@ -1,3 +1,6 @@
+## 3.32.3
+* Ignoring containers which fails to inspect. #599
+
 ## 3.32.2
 * Exposing Stub Solver domain config, see the docs. #545
 
@@ -31,7 +34,7 @@
 
 ## 3.30.0
 * Module to beans which need to initialize on app startup, different of StartupEvent,
- Eager are not coupled to DPS logic.
+  Eager are not coupled to DPS logic.
 
 ## 3.29.0
 * Implementing an IntTest which can validate the happy path of all DPS solvers,

gradle.properties

@@ -1 +1 @@
-version=3.32.2-snapshot
+version=3.32.3-snapshot

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

@@ -4,6 +4,7 @@
 import com.github.dockerjava.api.model.Container;
 
 import java.util.List;
+import java.util.stream.Stream;
 
 public interface ContainerFacade {
 
@@ -12,4 +13,8 @@ public interface ContainerFacade {
   List<Container> findActiveContainers();
 
   InspectContainerResponse inspect(String id);
+
+  InspectContainerResponse safeInspect(String id);
+
+  Stream<InspectContainerResponse> inspectFilteringValidContainers(List<Container> containers);
 }

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,8 @@
 import javax.inject.Singleton;
 import java.util.Collections;
 import java.util.List;
+import java.util.Objects;
+import java.util.stream.Stream;
 
 @Slf4j
 @Default
@@ -47,4 +50,26 @@ public List<Container> findActiveContainers() {
   public InspectContainerResponse inspect(String id) {
     return this.dockerClient.inspectContainerCmd(id).exec();
   }
+
+  @Override
+  public InspectContainerResponse safeInspect(String id) {
+    try {
+      return this.inspect(id);
+    } 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 null;
+  }
+
+  @Override
+  public Stream<InspectContainerResponse> inspectFilteringValidContainers(List<Container> containers) {
+    return containers
+      .stream()
+      .map(com.github.dockerjava.api.model.Container::getId)
+      .map(this::safeInspect)
+      .filter(Objects::nonNull)
+      ;
+  }
 }

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

@@ -26,9 +26,8 @@ public class ContainerDAODefault implements ContainerDAO {
 
   @Override
   public List<Container> findActiveContainersMatching(HostnameQuery query) {
-    return this.containerFacade.findActiveContainers()
-      .stream()
-      .map(it -> this.containerFacade.inspect(it.getId()))
+    final var containersToFilter = this.containerFacade.findActiveContainers();
+    return this.containerFacade.inspectFilteringValidContainers(containersToFilter)
       .filter(ContainerHostnameMatcher.buildPredicate(query))
       .map(ContainerMapper::of)
       .toList();

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

@@ -52,10 +52,8 @@ public Container findDPSContainer() {
     } else {
       log.debug("dpsContainersFound={}", containers.size());
     }
-    return containers
-      .stream()
+    return this.containerFacade.inspectFilteringValidContainers(containers)
       .findFirst()
-      .map(it -> this.containerFacade.inspect(it.getId()))
       .map(ContainerMapper::of)
       .orElse(null);
   }

src/test/java/com/mageddo/dnsproxyserver/docker/dataprovider/ContainerFacadeDefaultCompTest.java

@@ -0,0 +1,114 @@
+package com.mageddo.dnsproxyserver.docker.dataprovider;
+
+import com.github.dockerjava.api.DockerClient;
+import com.github.dockerjava.api.model.Container;
+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.ContainerTemplates;
+import testing.templates.docker.DockerClientTemplates;
+import testing.templates.docker.InspectContainerResponseTemplates;
+
+import java.util.ArrayList;
+
+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.spy;
+
+@ExtendWith(MockitoExtension.class)
+class ContainerFacadeDefaultCompTest {
+
+  ContainerFacadeDefault dao;
+
+  DockerClient dockerClient;
+
+  @BeforeEach
+  void before(){
+    this.dockerClient = DockerClientTemplates.buildSpy();
+    this.dao = spy(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);
+
+    // assert
+    assertEquals(expected, inspectResponse);
+  }
+
+
+}

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

@@ -0,0 +1,77 @@
+package com.mageddo.dnsproxyserver.docker.dataprovider;
+
+import com.github.dockerjava.api.exception.NotFoundException;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.InjectMocks;
+import org.mockito.Spy;
+import org.mockito.junit.jupiter.MockitoExtension;
+import testing.templates.docker.ContainerTemplates;
+import testing.templates.docker.InspectContainerResponseTemplates;
+
+import java.util.List;
+
+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 {
+
+  @Spy
+  @InjectMocks
+  ContainerFacadeDefault facade;
+
+  @Test
+  void mustNotThrowErrorWhenInspectContainerGetNotFound() {
+    // arrange
+    final var containerId = "a39bba9a8bab2899";
+
+    doThrow(new NotFoundException("Container not found"))
+      .when(this.facade).inspect(containerId)
+    ;
+
+    // act
+    final var container = this.facade.safeInspect(containerId);
+
+    // assert
+    assertNull(container);
+  }
+
+  @Test
+  void mustNotThrowErrorWhenInspectContainerFails() {
+    // arrange
+    final var containerId = "a39bba9a8bab28aa";
+
+    doThrow(new NullPointerException("Unexpected failure"))
+      .when(this.facade).inspect(containerId)
+    ;
+
+    // act
+    final var container = this.facade.safeInspect(containerId);
+
+    // assert
+    assertNull(container);
+  }
+
+  @Test
+  void mustFilterNullContainerInspections() {
+    final var c1 = ContainerTemplates.buildRegularContainerCoffeeMakerCheckout();
+    final var c2 = ContainerTemplates.buildDpsContainer();
+    final var containers = List.of(c1, c2);
+
+    doReturn(InspectContainerResponseTemplates.withDpsLabel())
+      .when(this.facade).safeInspect(c1.getId())
+    ;
+
+    doReturn(null)
+      .when(this.facade).safeInspect(c2.getId())
+    ;
+
+    final var filtered = this.facade.inspectFilteringValidContainers(containers)
+      .toList();
+
+    assertEquals(1, filtered.size());
+  }
+}

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;
+    }
+  }
 }