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

Support base image layer compressed with zstd #3717

Merged
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ project.ext.dependencyStrings = [
MAVEN_EXTENSION: 'com.google.cloud.tools:jib-maven-plugin-extension-api:0.4.0',

COMMONS_COMPRESS: 'org.apache.commons:commons-compress:1.21',
ZSTD_JNI: 'com.github.luben:zstd-jni:1.5.2-3',
elefeint marked this conversation as resolved.
Show resolved Hide resolved
COMMONS_TEXT: 'org.apache.commons:commons-text:1.9',
JACKSON_DATABIND: 'com.fasterxml.jackson.core:jackson-databind:2.13.3',
JACKSON_DATAFORMAT_YAML: 'com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:2.13.3',
Expand Down
15 changes: 14 additions & 1 deletion docs/faq.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ If a question you have is not answered below, please [submit an issue](/../../is
[How can I examine network traffic?](#how-can-i-examine-network-traffic)\
[How do I view debug logs for Jib?](#how-do-i-view-debug-logs-for-jib)\
[I am seeing `Method Not Found` or `Class Not Found` errors when building.](#i-am-seeing-method-not-found-or-class-not-found-errors-when-building)\
[I am seeing `Unsupported class file major version` when building.](#i-am-seeing-unsupported-class-file-major-version-when-building)
[I am seeing `Unsupported class file major version` when building.](#i-am-seeing-unsupported-class-file-major-version-when-building)\
[I am seeing `NoClassDefFoundError: com/github/luben/zstd/ZstdOutputStream` when building.](#i-am-seeing-noclassdeffounderror-comgithublubenzstdzstdoutputstream-when-building)

**Launch Problems**\
[I am seeing `ImagePullBackoff` on my pods.](#i-am-seeing-imagepullbackoff-on-my-pods-in-minikube)\
Expand Down Expand Up @@ -764,6 +765,18 @@ Jib uses the [ASM library](https://asm.ow2.io/) to examine compiled Java bytecod

Note that although the ASM library is the common cause of this error coming from Jib, it may be due to other reasons. Always check the full stack (`-e` or `-X` for Maven and `--stacktrace` for Gradle) to see where the error is coming from.

### I am seeing `NoClassDefFoundError: com/github/luben/zstd/ZstdOutputStream` when building.

Jib supports base image layers with media-type `application/vnd.oci.image.layer.v1.tar+zstd`, i.e. compressed with zstd algorithm instead of gzip.

However, the dependency to zstd is optional, so pulling such layers will result in:

```
java.lang.NoClassDefFoundError: com/github/luben/zstd/ZstdOutputStream
at org.apache.commons.compress.compressors.zstandard.ZstdCompressorOutputStream.<init>
```

This can be solved by adding a dependency to artifact `com.github.luben:zstd-jni:1.5.2-3` to the plugin.

## Launch problems

Expand Down
10 changes: 10 additions & 0 deletions jib-core/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,22 @@ plugins {
id 'eclipse'
}

java {
// Feature to handle base image layers compressed with zstd instead of gzip
// Will need to re-assess the optional dependency when zstd becomes widely used
registerFeature('zstdSupport') {
usingSourceSet(sourceSets.main)
}
}

dependencies {
api dependencyStrings.BUILD_PLAN
implementation dependencyStrings.GOOGLE_HTTP_CLIENT
implementation dependencyStrings.GOOGLE_HTTP_CLIENT_APACHE_V2
implementation dependencyStrings.GOOGLE_AUTH_LIBRARY_OAUTH2_HTTP

implementation dependencyStrings.COMMONS_COMPRESS
zstdSupportImplementation dependencyStrings.ZSTD_JNI
implementation dependencyStrings.GUAVA
implementation dependencyStrings.JACKSON_DATABIND
implementation dependencyStrings.JACKSON_DATATYPE_JSR310
Expand All @@ -22,6 +31,7 @@ dependencies {
testImplementation dependencyStrings.MOCKITO_CORE
testImplementation dependencyStrings.SLF4J_API
testImplementation dependencyStrings.SYSTEM_RULES
testImplementation dependencyStrings.ZSTD_JNI

integrationTestImplementation dependencyStrings.JBCRYPT
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,10 @@
import java.util.List;
import java.util.concurrent.TimeUnit;
import java.util.function.Predicate;
import java.util.zip.GZIPInputStream;
import java.util.zip.GZIPOutputStream;
import javax.annotation.Nullable;
import org.apache.commons.compress.compressors.CompressorException;
import org.apache.commons.compress.compressors.CompressorStreamFactory;

/** Writes to the default cache storage engine. */
class CacheStorageWriter {
Expand Down Expand Up @@ -159,9 +160,13 @@ static void moveIfDoesNotExist(Path source, Path destination) throws IOException
*/
private static DescriptorDigest getDiffIdByDecompressingFile(Path compressedFile)
throws IOException {
try (InputStream fileInputStream =
new BufferedInputStream(new GZIPInputStream(Files.newInputStream(compressedFile)))) {
return Digests.computeDigest(fileInputStream).getDigest();
try (InputStream in =
CompressorStreamFactory.getSingleton()
.createCompressorInputStream(
new BufferedInputStream(Files.newInputStream(compressedFile)))) {
return Digests.computeDigest(in).getDigest();
} catch (CompressorException e) {
throw new IOException(e);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import com.google.common.io.Resources;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.OutputStream;
import java.net.URISyntaxException;
import java.nio.file.Files;
import java.nio.file.NoSuchFileException;
Expand All @@ -50,8 +51,9 @@
import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;
import java.util.zip.GZIPInputStream;
import java.util.zip.GZIPOutputStream;
import org.apache.commons.compress.compressors.CompressorException;
import org.apache.commons.compress.compressors.CompressorStreamFactory;
import org.hamcrest.CoreMatchers;
import org.hamcrest.MatcherAssert;
import org.junit.Assert;
Expand All @@ -68,6 +70,7 @@ private static BlobDescriptor getDigest(Blob blob) throws IOException {
}

private static Blob compress(Blob blob) {
// Don't use GzipCompressorOutputStream, which has different defaults than GZIPOutputStream
return Blobs.from(
outputStream -> {
try (GZIPOutputStream compressorStream = new GZIPOutputStream(outputStream)) {
Expand All @@ -77,8 +80,28 @@ private static Blob compress(Blob blob) {
false);
}

private static Blob compress(Blob blob, String compressorName) {
return Blobs.from(
outputStream -> {
try (OutputStream compressorStream =
CompressorStreamFactory.getSingleton()
.createCompressorOutputStream(compressorName, outputStream)) {
blob.writeTo(compressorStream);
} catch (CompressorException e) {
throw new RuntimeException(e);
}
},
false);
}

private static Blob decompress(Blob blob) throws IOException {
return Blobs.from(new GZIPInputStream(new ByteArrayInputStream(Blobs.writeToByteArray(blob))));
try {
return Blobs.from(
CompressorStreamFactory.getSingleton()
.createCompressorInputStream(new ByteArrayInputStream(Blobs.writeToByteArray(blob))));
} catch (CompressorException e) {
throw new IOException(e);
}
}

private static <T extends JsonTemplate> T loadJsonResource(String path, Class<T> jsonClass)
Expand All @@ -103,10 +126,27 @@ public void setUp() throws IOException {
@Test
public void testWriteCompressed() throws IOException {
Blob uncompressedLayerBlob = Blobs.from("uncompressedLayerBlob");
Blob compressedLayerBlob = compress(uncompressedLayerBlob);
CachedLayer cachedLayer = cacheStorageWriter.writeCompressed(compressedLayerBlob);

CachedLayer cachedLayer = cacheStorageWriter.writeCompressed(compress(uncompressedLayerBlob));
verifyCachedLayer(cachedLayer, uncompressedLayerBlob, compressedLayerBlob);
}

verifyCachedLayer(cachedLayer, uncompressedLayerBlob);
@Test
public void testWriteZstdCompressed() throws IOException {
Blob uncompressedLayerBlob = Blobs.from("uncompressedLayerBlob");
Blob compressedLayerBlob = compress(uncompressedLayerBlob, CompressorStreamFactory.ZSTANDARD);

CachedLayer cachedLayer = cacheStorageWriter.writeCompressed(compressedLayerBlob);

verifyCachedLayer(cachedLayer, uncompressedLayerBlob, compressedLayerBlob);
}

@Test(expected = IOException.class)
public void testWriteCompressWhenUncompressed() throws IOException {
Blob uncompressedLayerBlob = Blobs.from("uncompressedLayerBlob");
// The detection of compression algorithm will fail
cacheStorageWriter.writeCompressed(uncompressedLayerBlob);
}

@Test
Expand All @@ -117,7 +157,7 @@ public void testWriteUncompressed() throws IOException {

CachedLayer cachedLayer = cacheStorageWriter.writeUncompressed(uncompressedLayerBlob, selector);

verifyCachedLayer(cachedLayer, uncompressedLayerBlob);
verifyCachedLayer(cachedLayer, uncompressedLayerBlob, compress(uncompressedLayerBlob));
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 test is about uncompressed layers, but I noticed the verifyCachedLayer actually uses compression for asserts.
What is even more strange is that this test seems dependant on the gzip compression settings: it passes with JDK GZIPOutputStream, but not with common-compress GzipCompressorOutputStream.

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like a convenience use... @chanseokoh you had a comment in #1678 saying it's a common but unnecessary pattern to "wrap anything in a Blob only for the sake of writing it to an output stream or compute a digest". Do you recall if that convenience shortcut is the reason verifyCachedLayer happens to compress the Blob before getting a descrptor out of it?


// Verifies that the files are present.
Path selectorFile = cacheStorageFiles.getSelectorFile(selector);
Expand Down Expand Up @@ -354,9 +394,10 @@ public void testMoveIfDoesNotExist_exceptionAfterFailure() {
assertThat(exception.getCause()).hasMessageThat().isEqualTo("foo");
}

private void verifyCachedLayer(CachedLayer cachedLayer, Blob uncompressedLayerBlob)
private void verifyCachedLayer(
CachedLayer cachedLayer, Blob uncompressedLayerBlob, Blob compressedLayerBlob)
throws IOException {
BlobDescriptor layerBlobDescriptor = getDigest(compress(uncompressedLayerBlob));
BlobDescriptor layerBlobDescriptor = getDigest(compressedLayerBlob);
DescriptorDigest layerDiffId = getDigest(uncompressedLayerBlob).getDigest();

// Verifies cachedLayer is correct.
Expand Down