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

Write history as part of layer metadata #877

Merged
merged 19 commits into from
Aug 28, 2018
Merged
Show file tree
Hide file tree
Changes from 13 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 @@ -113,6 +113,9 @@ public void testSteps_forBuildToDockerRegistry()
+ " \"2001/tcp\": {},\n"
+ " \"2002/tcp\": {},\n"
+ " \"3000/udp\": {}"));
String history = new Command("docker", "history", imageReference).run();
Assert.assertThat(history, CoreMatchers.containsString("jib"));
Assert.assertThat(history, CoreMatchers.containsString("bazel build ..."));
Assert.assertEquals(
"Hello, world. An argument.\n", new Command("docker", "run", imageReference).run());
}
Expand All @@ -138,17 +141,18 @@ public void testSteps_forBuildToDockerRegistry_dockerHubBaseImage()
public void testSteps_forBuildToDockerDaemon()
throws IOException, InterruptedException, CacheMetadataCorruptedException, ExecutionException,
CacheDirectoryNotOwnedException, CacheDirectoryCreationException {
String imageReference = "testdocker";
BuildConfiguration buildConfiguration =
getBuildConfiguration(
ImageReference.of("gcr.io", "distroless/java", "latest"),
ImageReference.of(null, "testdocker", null));
ImageReference.of(null, imageReference, null));
Path cacheDirectory = temporaryFolder.newFolder().toPath();
BuildSteps.forBuildToDockerDaemon(
buildConfiguration,
new Caches.Initializer(cacheDirectory, false, cacheDirectory, false))
.run();

String dockerContainerConfig = new Command("docker", "inspect", "testdocker").run();
String dockerContainerConfig = new Command("docker", "inspect", imageReference).run();
Assert.assertThat(
dockerContainerConfig,
CoreMatchers.containsString(
Expand All @@ -165,8 +169,11 @@ public void testSteps_forBuildToDockerDaemon()
+ " \"key1\": \"value1\",\n"
+ " \"key2\": \"value2\"\n"
+ " }"));
String history = new Command("docker", "history", imageReference).run();
Assert.assertThat(history, CoreMatchers.containsString("jib"));
Assert.assertThat(history, CoreMatchers.containsString("bazel build ..."));
Assert.assertEquals(
"Hello, world. An argument.\n", new Command("docker", "run", "testdocker").run());
"Hello, world. An argument.\n", new Command("docker", "run", imageReference).run());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,15 @@
import com.google.cloud.tools.jib.cache.CachedLayer;
import com.google.cloud.tools.jib.configuration.BuildConfiguration;
import com.google.cloud.tools.jib.configuration.ContainerConfiguration;
import com.google.cloud.tools.jib.image.HistoryEntry;
import com.google.cloud.tools.jib.image.Image;
import com.google.cloud.tools.jib.image.Layer;
import com.google.cloud.tools.jib.image.LayerPropertyNotFoundException;
import com.google.common.collect.ImmutableList;
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.ListeningExecutorService;
import java.time.Instant;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.Callable;
Expand Down Expand Up @@ -94,22 +96,43 @@ private Image<CachedLayer> afterCachedLayersSteps()
try (Timer ignored = new Timer(buildConfiguration.getBuildLogger(), DESCRIPTION)) {
// Constructs the image.
Image.Builder<CachedLayer> imageBuilder = Image.builder();
for (PullAndCacheBaseImageLayerStep pullAndCacheBaseImageLayerStep :
NonBlockingSteps.get(pullAndCacheBaseImageLayersStep)) {
Image<Layer> baseImage = NonBlockingSteps.get(pullBaseImageStep).getBaseImage();
ContainerConfiguration containerConfiguration =
buildConfiguration.getContainerConfiguration();

// Base image layers
List<PullAndCacheBaseImageLayerStep> baseImageLayers =
NonBlockingSteps.get(pullAndCacheBaseImageLayersStep);
for (PullAndCacheBaseImageLayerStep pullAndCacheBaseImageLayerStep : baseImageLayers) {
imageBuilder.addLayer(NonBlockingSteps.get(pullAndCacheBaseImageLayerStep));
}
for (BuildAndCacheApplicationLayerStep buildAndCacheApplicationLayerStep :
buildAndCacheApplicationLayerSteps) {
imageBuilder.addLayer(NonBlockingSteps.get(buildAndCacheApplicationLayerStep));
}

// Parameters that we passthrough from the base image
Image<Layer> baseImage = NonBlockingSteps.get(pullBaseImageStep).getBaseImage();
// Passthrough config and count non-empty history entries
int nonEmptyLayerCount = 0;
for (HistoryEntry historyObject : baseImage.getHistory()) {
imageBuilder.addHistory(historyObject);
if (!historyObject.isEmptyLayer()) {
nonEmptyLayerCount++;
}
}
imageBuilder.addEnvironment(baseImage.getEnvironment());
imageBuilder.addLabels(baseImage.getLabels());

ContainerConfiguration containerConfiguration =
buildConfiguration.getContainerConfiguration();
// Add history elements for non-empty layers that don't have one yet
String layerCreationTime =
containerConfiguration == null
? Instant.EPOCH.toString()
: containerConfiguration.getCreationTime().toString();
for (int count = 0; count < baseImageLayers.size() - nonEmptyLayerCount; count++) {
imageBuilder.addHistory(new HistoryEntry(layerCreationTime, null, null, null));
}

// Add built layers/configuration
for (BuildAndCacheApplicationLayerStep buildAndCacheApplicationLayerStep :
buildAndCacheApplicationLayerSteps) {
imageBuilder.addLayer(NonBlockingSteps.get(buildAndCacheApplicationLayerStep));
imageBuilder.addHistory(new HistoryEntry(layerCreationTime, "Jib", "jib", null));
}
if (containerConfiguration != null) {
imageBuilder.addEnvironment(containerConfiguration.getEnvironmentMap());
imageBuilder.setCreated(containerConfiguration.getCreationTime());
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
/*
* Copyright 2018 Google LLC. All rights reserved.
*
* 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.image;

import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.google.cloud.tools.jib.json.JsonTemplate;
import java.util.Objects;
import javax.annotation.Nullable;

/** Represents an item in the container configuration's {@code history} list. */
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add a link to https://github.com/opencontainers/image-spec/blob/master/config.md#properties and pointing specifically to the history field.

@JsonIgnoreProperties(ignoreUnknown = true)
public class HistoryEntry implements JsonTemplate {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this might better belong under the image.json package, next to ContainerConfigurationTemplate.

I'm thinking it might make more sense to call this ContainerConfigurationHistory and have it manage the actual JSON templates. Then, Image will have a ContainerConfigurationHistory and not have to directly manage HistoryEntry JSON templates.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll move it to the package, but I'm just going to make an issue for the second point for now.

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 timestamp at which the image was created. */
Copy link
Contributor

Choose a reason for hiding this comment

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

A comment about this needing to be formatted according to RFC 3339, section 5.6

private String created;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to be making the created field non-nullable (in the config spec, it is optional)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was referencing this and it didn't seem optional (no "omitempty").

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, interesting - okay, let's match that behavior then since we don't want to break compatibility with Docker.

Copy link
Member

Choose a reason for hiding this comment

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

But on the flip side we may error out reading an OCI-conformant image that has no created field. Right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can name this in code style and use JsonProperty here too. Something like creationTimestamp is more descriptive.


/** The name of the author specified when committing the image. */
@Nullable private String author;

/** The command used while building the image. */
Copy link
Contributor

Choose a reason for hiding this comment

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

s/while building/to build/?

@JsonProperty("created_by")
@Nullable
private String createdBy;

/** Whether or not the layer is empty ({@code @Nullable Boolean} to make field optional). */
Copy link
Contributor

Choose a reason for hiding this comment

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

This field is actually about whether or not this history entry corresponds to an actual layer in the container.

@JsonProperty("empty_layer")
@Nullable
private Boolean emptyLayer;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could be named like isLayer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that naming is too different from the field it corresponds to and could get confusing.


public HistoryEntry() {
this("1970-01-01T00:00:00Z", null, null, null);
}

public HistoryEntry(
String created,
@Nullable String author,
@Nullable String createdBy,
@Nullable Boolean emptyLayer) {
this.author = author;
this.created = created;
this.createdBy = createdBy;
this.emptyLayer = emptyLayer;
}

/**
* Returns whether or not the history object corresponds to an empty layer.
*
* @return {@code true} if the history object corresponds to an empty layer
*/
@JsonIgnore
public boolean isEmptyLayer() {
return emptyLayer == null ? false : emptyLayer;
}

@Override
public boolean equals(Object other) {
if (this == other) {
return true;
}
if (other instanceof HistoryEntry) {
HistoryEntry otherHistory = (HistoryEntry) other;
return otherHistory.created.equals(created)
&& Objects.equals(otherHistory.author, author)
&& Objects.equals(otherHistory.createdBy, createdBy)
&& Objects.equals(otherHistory.emptyLayer, emptyLayer);
}
return false;
}

@Override
public int hashCode() {
return Objects.hash(author, created, createdBy, emptyLayer);
}
}
22 changes: 22 additions & 0 deletions jib-core/src/main/java/com/google/cloud/tools/jib/image/Image.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ public class Image<T extends Layer> {
public static class Builder<T extends Layer> {

private final ImageLayers.Builder<T> imageLayersBuilder = ImageLayers.builder();
private ImmutableList.Builder<HistoryEntry> historyBuilder = ImmutableList.builder();
private ImmutableMap.Builder<String, String> environmentBuilder = ImmutableMap.builder();
private ImmutableMap.Builder<String, String> labelsBuilder = ImmutableMap.builder();

Expand Down Expand Up @@ -145,10 +146,22 @@ public Builder<T> addLayer(T layer) throws LayerPropertyNotFoundException {
return this;
}

/**
* Adds a history element to the image.
*
* @param history the history object to add
* @return this
*/
public Builder<T> addHistory(HistoryEntry history) {
historyBuilder.add(history);
return this;
}

public Image<T> build() {
return new Image<>(
created,
imageLayersBuilder.build(),
historyBuilder.build(),
environmentBuilder.build(),
entrypoint,
javaArguments,
Expand All @@ -167,6 +180,9 @@ public static <T extends Layer> Builder<T> builder() {
/** The layers of the image, in the order in which they are applied. */
private final ImageLayers<T> layers;

/** The commands used to build each layer of the image */
private final ImmutableList<HistoryEntry> history;

/** Environment variable definitions for running the image, in the format {@code NAME=VALUE}. */
@Nullable private final ImmutableMap<String, String> environment;

Expand All @@ -185,13 +201,15 @@ public static <T extends Layer> Builder<T> builder() {
private Image(
@Nullable Instant created,
ImageLayers<T> layers,
ImmutableList<HistoryEntry> history,
@Nullable ImmutableMap<String, String> environment,
@Nullable ImmutableList<String> entrypoint,
@Nullable ImmutableList<String> javaArguments,
@Nullable ImmutableList<Port> exposedPorts,
@Nullable ImmutableMap<String, String> labels) {
this.created = created;
this.layers = layers;
this.history = history;
this.environment = environment;
this.entrypoint = entrypoint;
this.javaArguments = javaArguments;
Expand Down Expand Up @@ -232,4 +250,8 @@ public ImmutableMap<String, String> getLabels() {
public ImmutableList<T> getLayers() {
return layers.getLayers();
}

public ImmutableList<HistoryEntry> getHistory() {
return history;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.google.cloud.tools.jib.image.DescriptorDigest;
import com.google.cloud.tools.jib.image.HistoryEntry;
import com.google.cloud.tools.jib.json.JsonTemplate;
import com.google.common.annotations.VisibleForTesting;
import java.util.ArrayList;
Expand All @@ -42,6 +43,18 @@
* "ExposedPorts": { "6000/tcp":{}, "8000/tcp":{}, "9000/tcp":{} }
* "Labels": { "com.example.label": "value" }
* },
* "history": [
* {
* "author": "Jib",
* "created": "1970-01-01T00:00:00Z",
* "created_by": "jib"
* },
* {
* "author": "Jib",
* "created": "1970-01-01T00:00:00Z",
* "created_by": "jib"
* }
* ]
* "rootfs": {
* "diff_ids": [
* "sha256:2aebd096e0e237b447781353379722157e6c2d434b9ec5a0d63f2a6f07cf90c2",
Expand Down Expand Up @@ -70,6 +83,9 @@ public class ContainerConfigurationTemplate implements JsonTemplate {
/** Execution parameters that should be used as a base when running the container. */
private final ConfigurationObjectTemplate config = new ConfigurationObjectTemplate();

/** Build commands used to create the image. */
Copy link
Contributor

Choose a reason for hiding this comment

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

"Describes the history of each layer"?

private final List<HistoryEntry> history = new ArrayList<>();

/** Layer content digests that are used to build the container filesystem. */
private final RootFilesystemObjectTemplate rootfs = new RootFilesystemObjectTemplate();

Expand Down Expand Up @@ -137,10 +153,18 @@ public void addLayerDiffId(DescriptorDigest diffId) {
rootfs.diff_ids.add(diffId);
}

public void addHistory(HistoryEntry historyEntry) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: since history is a plural list of HistoryEntrys, perhaps this method name should be addToHistory instead

history.add(historyEntry);
}

List<DescriptorDigest> getDiffIds() {
return rootfs.diff_ids;
}

List<HistoryEntry> getHistory() {
return history;
}

@Nullable
String getCreated() {
return created;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.google.cloud.tools.jib.cache.CachedLayer;
import com.google.cloud.tools.jib.configuration.Port;
import com.google.cloud.tools.jib.image.DescriptorDigest;
import com.google.cloud.tools.jib.image.HistoryEntry;
import com.google.cloud.tools.jib.image.Image;
import com.google.cloud.tools.jib.json.JsonTemplateMapper;
import com.google.common.annotations.VisibleForTesting;
Expand Down Expand Up @@ -115,6 +116,11 @@ public Blob getContainerConfigurationBlob() {
template.addLayerDiffId(layer.getDiffId());
}

// Adds the history.
for (HistoryEntry historyObject : image.getHistory()) {
template.addHistory(historyObject);
}

// Sets the creation time. Instant#toString() returns an ISO-8601 formatted string.
template.setCreated(image.getCreated() == null ? null : image.getCreated().toString());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.google.cloud.tools.jib.configuration.Port.Protocol;
import com.google.cloud.tools.jib.image.DescriptorDigest;
import com.google.cloud.tools.jib.image.DigestOnlyLayer;
import com.google.cloud.tools.jib.image.HistoryEntry;
import com.google.cloud.tools.jib.image.Image;
import com.google.cloud.tools.jib.image.Layer;
import com.google.cloud.tools.jib.image.LayerCountMismatchException;
Expand Down Expand Up @@ -110,6 +111,7 @@ public static Image<Layer> toImage(
}

List<DescriptorDigest> diffIds = containerConfigurationTemplate.getDiffIds();
List<HistoryEntry> historyObjects = containerConfigurationTemplate.getHistory();

if (layers.size() != diffIds.size()) {
throw new LayerCountMismatchException(
Expand All @@ -124,6 +126,9 @@ public static Image<Layer> toImage(

imageBuilder.addLayer(new ReferenceLayer(noDiffIdLayer.getBlobDescriptor(), diffId));
}
for (HistoryEntry historyObject : historyObjects) {
imageBuilder.addHistory(historyObject);
}

if (containerConfigurationTemplate.getCreated() != null) {
try {
Expand Down
Loading