Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: determine platform details from local docker installation for jibDockerBuild #4249

Merged
merged 18 commits into from
May 30, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,13 @@ void save(ImageReference imageReference, Path outputPath, Consumer<Long> written
* @throws InterruptedException if the {@code docker inspect} process was interrupted
*/
ImageDetails inspect(ImageReference imageReference) throws IOException, InterruptedException;

/**
* Gets the os and architecture of the local docker installation.
blakeli0 marked this conversation as resolved.
Show resolved Hide resolved
*
* @return the os type and architecture of the image
* @throws IOException if an I/O exception occurs or {@code docker info} failed
* @throws InterruptedException if the {@code docker info} process was interrupted
*/
DockerInfoDetails info() throws IOException, InterruptedException;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Copyright 2024 Google LLC.
*
* 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
*
* http://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 com.google.cloud.tools.jib.api;

import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.cloud.tools.jib.json.JsonTemplate;

/** Contains the os and architecture from {@code docker info}. */
@JsonIgnoreProperties(ignoreUnknown = true)
public class DockerInfoDetails implements JsonTemplate {
mpeddada1 marked this conversation as resolved.
Show resolved Hide resolved

@JsonProperty("OSType")
private String osType = "";

@JsonProperty("Architecture")
private String architecture = "";

public String getOsType() {
return osType;
}

public String getArchitecture() {
return architecture;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import com.google.cloud.tools.jib.api.DescriptorDigest;
import com.google.cloud.tools.jib.api.DockerClient;
import com.google.cloud.tools.jib.api.DockerInfoDetails;
import com.google.cloud.tools.jib.blob.BlobDescriptor;
import com.google.cloud.tools.jib.builder.ProgressEventDispatcher;
import com.google.cloud.tools.jib.builder.steps.LocalBaseImageSteps.LocalImage;
Expand All @@ -38,6 +39,7 @@
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListeningExecutorService;
import com.google.common.util.concurrent.MoreExecutors;
import java.io.IOException;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Collection;
Expand All @@ -52,6 +54,7 @@
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Future;
import java.util.function.Consumer;
import java.util.logging.Logger;
import java.util.stream.Collectors;
import javax.annotation.Nullable;

Expand All @@ -64,6 +67,8 @@
*/
public class StepsRunner {

Logger logger = Logger.getLogger(StepResults.class.getName());

/** Holds the individual step results. */
private static class StepResults {

Expand Down Expand Up @@ -427,6 +432,7 @@ private void buildImages(ProgressEventDispatcher.Factory progressDispatcherFacto
Image baseImage = entry.getKey();
List<Future<PreparedLayer>> baseLayers = entry.getValue();

// Image is immutable once built.
Future<Image> builtImage =
buildImage(baseImage, baseLayers, progressDispatcher.newChildProducer());
baseImagesAndBuiltImages.put(baseImage, builtImage);
Expand Down Expand Up @@ -616,11 +622,7 @@ private void loadDocker(
results.buildResult =
executorService.submit(
() -> {
Verify.verify(
results.baseImagesAndBuiltImages.get().size() == 1,
"multi-platform image building not supported when pushing to Docker engine");
Image builtImage =
results.baseImagesAndBuiltImages.get().values().iterator().next().get();
Image builtImage = fetchBaseImageForLocalBuild(dockerClient);
return new LoadDockerStep(
buildContext, progressDispatcherFactory, dockerClient, builtImage)
.call();
Expand All @@ -647,4 +649,28 @@ private void writeTarFile(
private <E> List<Future<E>> scheduleCallables(ImmutableList<? extends Callable<E>> callables) {
return callables.stream().map(executorService::submit).collect(Collectors.toList());
}

private String computeArchitecture(String architecture) {
if (architecture.equals("x86_64")) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where do we get the mapping between the architecture from docker info and the architecture needed for docker build? Are x86_64 and aarch64 all we need? Also please add unit tests for all the cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mapping was retrieved from https://docs.docker.com/engine/install/#supported-platforms. From the documented architectures, "x86_64" and "aarch64" appear to be the only ones with alternative naming? I've also added a source code comment.

Done, added unit testing in StepsRunnerTest.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, so the root cause is that the architecture returned from docker info is different from the architecture used by docker build. If that's the case, maybe we can change the method name to something like mapDockerInfoArchitechtureToDockerBuildArchitechture? Or toDockerBuildArchitecture(String dockerInfoArchitecture) if we think the previous one is too verbose?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right, the naming doesn't seem to be standardized everywhere. While docker info results in x86_64 on my machine, docker manifest inspect on images uses amd64 which is supposed to be equivalent (docker/docs#4295).
Renaming the method is a good idea. Alternatively, what are your thoughts on normalizeArchitecture() or standardizeArchitecture()? I found a couple of references that do something similar:
(1)https://github.com/containerd/platforms/blob/c1438e911ac7596426105350652fe267d0fb8a03/database.go#L76
(2)https://github.com/openzipkin/zipkin/blob/2f0a26fd4e3f3ea24e9fb30f55a84ebbc194129c/build-bin/docker/docker_arch#L7

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestions! I like normalizeArchitecture().

return "amd64";
} else if (architecture.equals("aarch64")) {
return "arm64";
}
return architecture;
}

private Image fetchBaseImageForLocalBuild(DockerClient dockerClient)
throws IOException, InterruptedException, ExecutionException {
DockerInfoDetails dockerInfoDetails = dockerClient.info();
String osType = dockerInfoDetails.getOsType();
String dockerArchitecture = computeArchitecture(dockerInfoDetails.getArchitecture());
for (Map.Entry<Image, Future<Image>> imageEntry :
blakeli0 marked this conversation as resolved.
Show resolved Hide resolved
results.baseImagesAndBuiltImages.get().entrySet()) {
Image image = imageEntry.getValue().get();
if (image.getArchitecture().equals(dockerArchitecture) && image.getOs().equals(osType)) {
return image;
}
}
return results.baseImagesAndBuiltImages.get().values().iterator().next().get();
blakeli0 marked this conversation as resolved.
Show resolved Hide resolved
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.cloud.tools.jib.api.DescriptorDigest;
import com.google.cloud.tools.jib.api.DockerClient;
import com.google.cloud.tools.jib.api.DockerInfoDetails;
import com.google.cloud.tools.jib.api.ImageDetails;
import com.google.cloud.tools.jib.api.ImageReference;
import com.google.cloud.tools.jib.http.NotifyingOutputStream;
Expand Down Expand Up @@ -184,6 +185,17 @@ public boolean supported(Map<String, String> parameters) {
return true;
}

@Override
public DockerInfoDetails info() throws IOException, InterruptedException {
// Runs 'docker info'.
Process infoProcess = docker("info", "-f", "{{json .}}");
if (infoProcess.waitFor() != 0) {
throw new IOException(
"'docker info' command failed with error: " + getStderrOutput(infoProcess));
}
return JsonTemplateMapper.readJson(infoProcess.getInputStream(), DockerInfoDetails.class);
}

@Override
public String load(ImageTarball imageTarball, Consumer<Long> writtenByteCountListener)
throws InterruptedException, IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.google.cloud.tools.jib.api.ImageDetails;
import com.google.cloud.tools.jib.api.ImageReference;
import com.google.cloud.tools.jib.image.ImageTarball;
import com.google.cloud.tools.jib.json.JsonTemplate;
import java.io.IOException;
import java.nio.file.Path;
import java.util.Map;
Expand Down Expand Up @@ -47,4 +48,9 @@ public ImageDetails inspect(ImageReference imageReference)
throws IOException, InterruptedException {
return null;
}

@Override
public JsonTemplate info() throws IOException, InterruptedException {
return null;
}
}
Loading