diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index c303e1a07..fe2351753 100644 --- a/RELEASE-NOTES.md +++ b/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, diff --git a/gradle.properties b/gradle.properties index b701727f8..8995b94a1 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1 +1 @@ -version=3.32.2-snapshot +version=3.32.3-snapshot diff --git a/src/main/java/com/mageddo/dnsproxyserver/docker/dataprovider/ContainerFacade.java b/src/main/java/com/mageddo/dnsproxyserver/docker/dataprovider/ContainerFacade.java index ec0fb41f4..7b8e94602 100644 --- a/src/main/java/com/mageddo/dnsproxyserver/docker/dataprovider/ContainerFacade.java +++ b/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 findActiveContainers(); InspectContainerResponse inspect(String id); + + InspectContainerResponse safeInspect(String id); + + Stream inspectFilteringValidContainers(List containers); } diff --git a/src/main/java/com/mageddo/dnsproxyserver/docker/dataprovider/ContainerFacadeDefault.java b/src/main/java/com/mageddo/dnsproxyserver/docker/dataprovider/ContainerFacadeDefault.java index e5ab88fa2..662343723 100644 --- a/src/main/java/com/mageddo/dnsproxyserver/docker/dataprovider/ContainerFacadeDefault.java +++ b/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 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 inspectFilteringValidContainers(List containers) { + return containers + .stream() + .map(com.github.dockerjava.api.model.Container::getId) + .map(this::safeInspect) + .filter(Objects::nonNull) + ; + } } diff --git a/src/main/java/com/mageddo/dnsproxyserver/solver/docker/dataprovider/ContainerDAODefault.java b/src/main/java/com/mageddo/dnsproxyserver/solver/docker/dataprovider/ContainerDAODefault.java index 7c4ec0723..cb592ae1e 100644 --- a/src/main/java/com/mageddo/dnsproxyserver/solver/docker/dataprovider/ContainerDAODefault.java +++ b/src/main/java/com/mageddo/dnsproxyserver/solver/docker/dataprovider/ContainerDAODefault.java @@ -26,9 +26,8 @@ public class ContainerDAODefault implements ContainerDAO { @Override public List 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(); diff --git a/src/main/java/com/mageddo/dnsproxyserver/solver/docker/dataprovider/DpsContainerDAODefault.java b/src/main/java/com/mageddo/dnsproxyserver/solver/docker/dataprovider/DpsContainerDAODefault.java index bcc8904e1..69a988d4c 100644 --- a/src/main/java/com/mageddo/dnsproxyserver/solver/docker/dataprovider/DpsContainerDAODefault.java +++ b/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); } diff --git a/src/test/java/com/mageddo/dnsproxyserver/docker/dataprovider/ContainerFacadeDefaultCompTest.java b/src/test/java/com/mageddo/dnsproxyserver/docker/dataprovider/ContainerFacadeDefaultCompTest.java new file mode 100644 index 000000000..610fb877e --- /dev/null +++ b/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(); + 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(); + + 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(); + 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); + } + + +} diff --git a/src/test/java/com/mageddo/dnsproxyserver/docker/dataprovider/ContainerFacadeDefaultTest.java b/src/test/java/com/mageddo/dnsproxyserver/docker/dataprovider/ContainerFacadeDefaultTest.java new file mode 100644 index 000000000..34f980466 --- /dev/null +++ b/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()); + } +} diff --git a/src/test/java/testing/mocks/DockerClientStub.java b/src/test/java/testing/mocks/DockerClientStub.java index d89a292bd..ec63838ed 100644 --- a/src/test/java/testing/mocks/DockerClientStub.java +++ b/src/test/java/testing/mocks/DockerClientStub.java @@ -3,9 +3,18 @@ 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; @@ -13,15 +22,41 @@ public class DockerClientStub extends DockerClientDelegate { private final ConnectToNetworkCmd connectToNetworkCmd; - private final DockerCmdSyncExec execution; + private final ListContainersCmd listContainersCmd; + private final Map inspectContainerCmdMap; + + private final DockerCmdSyncExec connectToNetworkExecution; + private final ListContainersCmd.Exec listContainersExecution; + private final InspectContainerCmd.Exec inspectContainerExecution; public DockerClientStub() { - this.execution = mock(DockerCmdSyncExec.class); - this.connectToNetworkCmd = spy(new ConnectToNetworkCmdImpl((DockerCmdSyncExec) 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; + } + } }