Merge pull request #44412 from nosan
* pr/44412: Polish "Improve logging in DockerApi" Improve logging in DockerApi Closes gh-44412
This commit is contained in:
@@ -21,6 +21,7 @@ import java.util.List;
|
||||
import java.util.function.Consumer;
|
||||
|
||||
import org.springframework.boot.buildpack.platform.docker.DockerApi;
|
||||
import org.springframework.boot.buildpack.platform.docker.DockerLog;
|
||||
import org.springframework.boot.buildpack.platform.docker.TotalProgressEvent;
|
||||
import org.springframework.boot.buildpack.platform.docker.TotalProgressPullListener;
|
||||
import org.springframework.boot.buildpack.platform.docker.TotalProgressPushListener;
|
||||
@@ -75,7 +76,7 @@ public class Builder {
|
||||
* @param log a logger used to record output
|
||||
*/
|
||||
public Builder(BuildLog log) {
|
||||
this(log, new DockerApi(), null);
|
||||
this(log, new DockerApi(null, BuildLogAdapter.get(log)), null);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -85,8 +86,8 @@ public class Builder {
|
||||
* @since 2.4.0
|
||||
*/
|
||||
public Builder(BuildLog log, DockerConfiguration dockerConfiguration) {
|
||||
this(log, new DockerApi((dockerConfiguration != null) ? dockerConfiguration.getHost() : null),
|
||||
dockerConfiguration);
|
||||
this(log, new DockerApi((dockerConfiguration != null) ? dockerConfiguration.getHost() : null,
|
||||
BuildLogAdapter.get(log)), dockerConfiguration);
|
||||
}
|
||||
|
||||
Builder(BuildLog log, DockerApi docker, DockerConfiguration dockerConfiguration) {
|
||||
@@ -262,6 +263,40 @@ public class Builder {
|
||||
|
||||
}
|
||||
|
||||
/**
|
||||
* A {@link DockerLog} implementation that adapts to an {@link AbstractBuildLog}.
|
||||
*/
|
||||
static final class BuildLogAdapter implements DockerLog {
|
||||
|
||||
private final AbstractBuildLog log;
|
||||
|
||||
private BuildLogAdapter(AbstractBuildLog log) {
|
||||
this.log = log;
|
||||
}
|
||||
|
||||
@Override
|
||||
public void log(String message) {
|
||||
this.log.log(message);
|
||||
}
|
||||
|
||||
/**
|
||||
* Creates{@link DockerLog} instance based on the provided {@link BuildLog}.
|
||||
* <p>
|
||||
* If the provided {@link BuildLog} instance is an {@link AbstractBuildLog}, the
|
||||
* method returns a {@link BuildLogAdapter}, otherwise it returns a default
|
||||
* {@link DockerLog#toSystemOut()}.
|
||||
* @param log the {@link BuildLog} instance to delegate
|
||||
* @return a {@link DockerLog} instance for logging
|
||||
*/
|
||||
static DockerLog get(BuildLog log) {
|
||||
if (log instanceof AbstractBuildLog abstractBuildLog) {
|
||||
return new BuildLogAdapter(abstractBuildLog);
|
||||
}
|
||||
return DockerLog.toSystemOut();
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
/**
|
||||
* {@link BuildpackResolverContext} implementation for the {@link Builder}.
|
||||
*/
|
||||
|
||||
@@ -87,7 +87,7 @@ public class DockerApi {
|
||||
* Create a new {@link DockerApi} instance.
|
||||
*/
|
||||
public DockerApi() {
|
||||
this(HttpTransport.create(null));
|
||||
this(HttpTransport.create(null), DockerLog.toSystemOut());
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -96,21 +96,34 @@ public class DockerApi {
|
||||
* @since 2.4.0
|
||||
*/
|
||||
public DockerApi(DockerHostConfiguration dockerHost) {
|
||||
this(HttpTransport.create(dockerHost));
|
||||
this(HttpTransport.create(dockerHost), DockerLog.toSystemOut());
|
||||
}
|
||||
|
||||
/**
|
||||
* Create a new {@link DockerApi} instance.
|
||||
* @param dockerHost the Docker daemon host information
|
||||
* @param log a logger used to record output
|
||||
* @since 3.5.0
|
||||
*/
|
||||
public DockerApi(DockerHostConfiguration dockerHost, DockerLog log) {
|
||||
this(HttpTransport.create(dockerHost), log);
|
||||
}
|
||||
|
||||
/**
|
||||
* Create a new {@link DockerApi} instance backed by a specific {@link HttpTransport}
|
||||
* implementation.
|
||||
* @param http the http implementation
|
||||
* @param log a logger used to record output
|
||||
*/
|
||||
DockerApi(HttpTransport http) {
|
||||
DockerApi(HttpTransport http, DockerLog log) {
|
||||
Assert.notNull(http, "'http' must not be null");
|
||||
Assert.notNull(log, "'log' must not be null");
|
||||
this.http = http;
|
||||
this.jsonStream = new JsonStream(SharedObjectMapper.get());
|
||||
this.image = new ImageApi();
|
||||
this.container = new ContainerApi();
|
||||
this.volume = new VolumeApi();
|
||||
this.system = new SystemApi();
|
||||
this.system = new SystemApi(log);
|
||||
}
|
||||
|
||||
private HttpTransport http() {
|
||||
@@ -485,7 +498,10 @@ public class DockerApi {
|
||||
*/
|
||||
class SystemApi {
|
||||
|
||||
SystemApi() {
|
||||
private final DockerLog log;
|
||||
|
||||
SystemApi(DockerLog log) {
|
||||
this.log = log;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -502,6 +518,7 @@ public class DockerApi {
|
||||
}
|
||||
}
|
||||
catch (Exception ex) {
|
||||
this.log.log("Warning: Failed to determine Docker API version: " + ex.getMessage());
|
||||
// fall through to return default value
|
||||
}
|
||||
return UNKNOWN_API_VERSION;
|
||||
|
||||
@@ -0,0 +1,54 @@
|
||||
/*
|
||||
* Copyright 2012-2025 the original author or authors.
|
||||
*
|
||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* https://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
|
||||
package org.springframework.boot.buildpack.platform.docker;
|
||||
|
||||
import java.io.PrintStream;
|
||||
|
||||
/**
|
||||
* Callback interface used to provide {@link DockerApi} output logging.
|
||||
*
|
||||
* @author Dmytro Nosan
|
||||
* @since 3.5.0
|
||||
* @see #toSystemOut()
|
||||
*/
|
||||
public interface DockerLog {
|
||||
|
||||
/**
|
||||
* Logs a given message.
|
||||
* @param message the message to log
|
||||
*/
|
||||
void log(String message);
|
||||
|
||||
/**
|
||||
* Factory method that returns a {@link DockerLog} the outputs to {@link System#out}.
|
||||
* @return {@link DockerLog} instance that logs to system out
|
||||
*/
|
||||
static DockerLog toSystemOut() {
|
||||
return to(System.out);
|
||||
}
|
||||
|
||||
/**
|
||||
* Factory method that returns a {@link DockerLog} the outputs to a given
|
||||
* {@link PrintStream}.
|
||||
* @param out the print stream used to output the log
|
||||
* @return {@link DockerLog} instance that logs to the given print stream
|
||||
*/
|
||||
static DockerLog to(PrintStream out) {
|
||||
return out::println;
|
||||
}
|
||||
|
||||
}
|
||||
@@ -25,10 +25,12 @@ import org.junit.jupiter.api.Test;
|
||||
import org.mockito.ArgumentCaptor;
|
||||
import org.mockito.stubbing.Answer;
|
||||
|
||||
import org.springframework.boot.buildpack.platform.build.Builder.BuildLogAdapter;
|
||||
import org.springframework.boot.buildpack.platform.docker.DockerApi;
|
||||
import org.springframework.boot.buildpack.platform.docker.DockerApi.ContainerApi;
|
||||
import org.springframework.boot.buildpack.platform.docker.DockerApi.ImageApi;
|
||||
import org.springframework.boot.buildpack.platform.docker.DockerApi.VolumeApi;
|
||||
import org.springframework.boot.buildpack.platform.docker.DockerLog;
|
||||
import org.springframework.boot.buildpack.platform.docker.TotalProgressPullListener;
|
||||
import org.springframework.boot.buildpack.platform.docker.configuration.DockerConfiguration;
|
||||
import org.springframework.boot.buildpack.platform.docker.transport.DockerEngineException;
|
||||
@@ -75,6 +77,26 @@ class BuilderTests {
|
||||
assertThat(builder).isNotNull();
|
||||
}
|
||||
|
||||
@Test
|
||||
void createDockerApiWithLogDockerLogDelegate() {
|
||||
Builder builder = new Builder(BuildLog.toSystemOut());
|
||||
assertThat(builder).isNotNull();
|
||||
assertThat(builder).extracting("docker")
|
||||
.extracting("system")
|
||||
.extracting("log")
|
||||
.isInstanceOf(BuildLogAdapter.class);
|
||||
}
|
||||
|
||||
@Test
|
||||
void createDockerApiWithLogDockerSystemOutDelegate() {
|
||||
Builder builder = new Builder(mock(BuildLog.class));
|
||||
assertThat(builder).isNotNull();
|
||||
assertThat(builder).extracting("docker")
|
||||
.extracting("system")
|
||||
.extracting("log")
|
||||
.isInstanceOf(DockerLog.toSystemOut().getClass());
|
||||
}
|
||||
|
||||
@Test
|
||||
void buildWhenRequestIsNullThrowsException() {
|
||||
Builder builder = new Builder();
|
||||
|
||||
@@ -59,6 +59,8 @@ import org.springframework.boot.buildpack.platform.io.Content;
|
||||
import org.springframework.boot.buildpack.platform.io.IOConsumer;
|
||||
import org.springframework.boot.buildpack.platform.io.Owner;
|
||||
import org.springframework.boot.buildpack.platform.io.TarArchive;
|
||||
import org.springframework.boot.testsupport.system.CapturedOutput;
|
||||
import org.springframework.boot.testsupport.system.OutputCaptureExtension;
|
||||
import org.springframework.util.LinkedMultiValueMap;
|
||||
import org.springframework.util.MultiValueMap;
|
||||
|
||||
@@ -82,7 +84,7 @@ import static org.mockito.Mockito.times;
|
||||
* @author Rafael Ceccone
|
||||
* @author Moritz Halbritter
|
||||
*/
|
||||
@ExtendWith(MockitoExtension.class)
|
||||
@ExtendWith({ MockitoExtension.class, OutputCaptureExtension.class })
|
||||
class DockerApiTests {
|
||||
|
||||
private static final String API_URL = "/v" + DockerApi.API_VERSION;
|
||||
@@ -108,7 +110,7 @@ class DockerApiTests {
|
||||
|
||||
@BeforeEach
|
||||
void setup() {
|
||||
this.dockerApi = new DockerApi(this.http);
|
||||
this.dockerApi = new DockerApi(this.http, DockerLog.toSystemOut());
|
||||
}
|
||||
|
||||
private HttpTransport http() {
|
||||
@@ -732,9 +734,10 @@ class DockerApiTests {
|
||||
}
|
||||
|
||||
@Test
|
||||
void getApiVersionWithExceptionReturnsUnknownVersion() throws Exception {
|
||||
void getApiVersionWithExceptionReturnsUnknownVersion(CapturedOutput output) throws Exception {
|
||||
given(http().head(eq(new URI(PING_URL)))).willThrow(new IOException("simulated error"));
|
||||
assertThat(this.api.getApiVersion()).isEqualTo(DockerApi.UNKNOWN_API_VERSION);
|
||||
assertThat(output).contains("Warning: Failed to determine Docker API version: simulated error");
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -0,0 +1,51 @@
|
||||
/*
|
||||
* Copyright 2012-2025 the original author or authors.
|
||||
*
|
||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* https://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
|
||||
package org.springframework.boot.buildpack.platform.docker;
|
||||
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.api.extension.ExtendWith;
|
||||
|
||||
import org.springframework.boot.testsupport.system.CapturedOutput;
|
||||
import org.springframework.boot.testsupport.system.OutputCaptureExtension;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
|
||||
/**
|
||||
* Tests for {@link DockerLog}.
|
||||
*
|
||||
* @author Dmytro nosan
|
||||
*/
|
||||
@ExtendWith(OutputCaptureExtension.class)
|
||||
class DockerLogTests {
|
||||
|
||||
@Test
|
||||
void toSystemOutPrintsToSystemOut(CapturedOutput output) {
|
||||
DockerLog logger = DockerLog.toSystemOut();
|
||||
logger.log("Hello world");
|
||||
assertThat(output.getErr()).isEmpty();
|
||||
assertThat(output.getOut()).contains("Hello world");
|
||||
}
|
||||
|
||||
@Test
|
||||
void toPrintsToOutput(CapturedOutput output) {
|
||||
DockerLog logger = DockerLog.to(System.err);
|
||||
logger.log("Hello world");
|
||||
assertThat(output.getOut()).isEmpty();
|
||||
assertThat(output.getErr()).contains("Hello world");
|
||||
}
|
||||
|
||||
}
|
||||
Reference in New Issue
Block a user