Skip to content

Commit

Permalink
Do not overwrite existing layer TAR (#729)
Browse files Browse the repository at this point in the history
  • Loading branch information
@chanseokoh
chanseokoh authored Jul 27, 2018
1 parent 02104b3 commit 51b20c4
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 40 deletions.
16 changes: 11 additions & 5 deletions jib-core/src/main/java/com/google/cloud/tools/jib/cache/CacheWriter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@
import java.io.BufferedOutputStream;

This comment has been minimized.

Sign in to view

Copy link
@Bulldog6600

Bulldog6600 Jul 28, 2018

java.nio.file.FileAlreadyExistsException;
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.FileAlreadyExistsException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;
import java.nio.file.attribute.FileTime;
import java.time.Instant;
import java.util.zip.GZIPInputStream;
Expand Down Expand Up @@ -76,11 +76,17 @@ public CachedLayerWithMetadata writeLayer(ReproducibleLayerBuilder reproducibleL
compressorStream.close();
BlobDescriptor compressedBlobDescriptor = compressedDigestOutputStream.toBlobDescriptor();

// Renames the temporary layer file to the correct filename. If the file already exists, we
// skip renaming and use the existing file. This happens if a new layer happens to have the
// same content as a previously-cached layer.
// Renames the temporary layer file to the correct filename.
Path layerFile = getLayerFile(compressedBlobDescriptor.getDigest());
Files.move(tempLayerFile, layerFile, StandardCopyOption.REPLACE_EXISTING);
try {
Files.move(tempLayerFile, layerFile);
} catch (FileAlreadyExistsException ignored) {
// If the file already exists, we skip renaming and use the existing file. This happens if a
// new layer happens to have the same content as a previously-cached layer.
//
// Do not attempt to remove the try-catch block with the idea of checking file existence
// before moving; there can be concurrent file moves.
}

CachedLayer cachedLayer = new CachedLayer(layerFile, compressedBlobDescriptor, diffId);
LayerMetadata layerMetadata =
Expand Down
94 changes: 59 additions & 35 deletions jib-core/src/test/java/com/google/cloud/tools/jib/cache/CacheWriterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,18 @@
import com.google.cloud.tools.jib.image.ReproducibleLayerBuilder;
import com.google.cloud.tools.jib.image.UnwrittenLayer;
import com.google.common.collect.ImmutableList;
import com.google.common.io.CharStreams;
import com.google.common.io.ByteStreams;
import com.google.common.io.CountingOutputStream;
import com.google.common.io.Resources;
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStreamReader;
import java.net.URISyntaxException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Collections;
import java.util.stream.Stream;
import java.util.zip.GZIPInputStream;
import java.util.zip.GZIPOutputStream;
import org.junit.Assert;
Expand All @@ -50,9 +49,14 @@
/** Tests for {@link CacheWriter}. */
public class CacheWriterTest {

@Rule public TemporaryFolder temporaryCacheDirectory = new TemporaryFolder();
private static boolean isTarGz(Path path) {
return path.toString().endsWith(".tar.gz");
}

@Rule public final TemporaryFolder temporaryCacheDirectory = new TemporaryFolder();

private Cache testCache;
private CacheWriter cacheWriter;

private Path resourceBlob;

Expand All @@ -74,17 +78,14 @@ public void setUp() throws CacheMetadataCorruptedException, IOException, URISynt
Path cacheDirectory = temporaryCacheDirectory.newFolder().toPath();

testCache = Cache.init(cacheDirectory);
// Writes resourceBlob as a layer to the cache.
cacheWriter = new CacheWriter(testCache);

resourceBlob = Paths.get(Resources.getResource("blobA").toURI());
}

@Test
public void testWriteLayer_unwritten() throws IOException {
ExpectedLayer expectedLayer = getExpectedLayer();

// Writes resourceBlob as a layer to the cache.
CacheWriter cacheWriter = new CacheWriter(testCache);

UnwrittenLayer unwrittenLayer = new UnwrittenLayer(Blobs.from(resourceBlob));

ReproducibleLayerBuilder mockReproducibleLayerBuilder =
Expand All @@ -94,34 +95,64 @@ public void testWriteLayer_unwritten() throws IOException {
.thenReturn(
ImmutableList.of(
new LayerEntry(
ImmutableList.of(Paths.get("some", "source", "file")),
"/some/extraction/path")));
ImmutableList.of(Paths.get("some/source/file")), "/some/extraction/path")));

CachedLayerWithMetadata cachedLayerWithMetadata =
cacheWriter.writeLayer(mockReproducibleLayerBuilder);
testCache.addCachedLayersWithMetadataToMetadata(
Collections.singletonList(cachedLayerWithMetadata));

CachedLayerWithMetadata layerInMetadata = testCache.getUpdatedMetadata().getLayers().get(0);
Assert.assertNotNull(layerInMetadata.getMetadata());
Assert.assertEquals(1, layerInMetadata.getMetadata().getEntries().size());
LayerMetadata layerMetadata = testCache.getUpdatedMetadata().getLayers().get(0).getMetadata();
Assert.assertNotNull(layerMetadata);
Assert.assertEquals(1, layerMetadata.getEntries().size());
Assert.assertEquals(
Collections.singletonList(Paths.get("some", "source", "file").toString()),
layerInMetadata.getMetadata().getEntries().get(0).getSourceFilesStrings());
Collections.singletonList(Paths.get("some/source/file").toString()),
layerMetadata.getEntries().get(0).getSourceFilesStrings());
Assert.assertEquals(
"/some/extraction/path",
layerInMetadata.getMetadata().getEntries().get(0).getExtractionPath());
"/some/extraction/path", layerMetadata.getEntries().get(0).getExtractionPath());

verifyCachedLayerIsExpected(getExpectedLayer(), cachedLayerWithMetadata);
}

// Windows file overwrite issue: https://github.com/GoogleContainerTools/jib/issues/719
@Test
public void testWriteLayer_doesNotOverwriteExistingTarGz()
throws IOException, InterruptedException {
// Writes resourceBlob as a layer to the cache.
UnwrittenLayer unwrittenLayer = new UnwrittenLayer(Blobs.from(resourceBlob));

ReproducibleLayerBuilder layerBuilder = Mockito.mock(ReproducibleLayerBuilder.class);
Mockito.when(layerBuilder.build()).thenReturn(unwrittenLayer);
LayerEntry layerEntry =
new LayerEntry(ImmutableList.of(Paths.get("some/source/file")), "/some/extraction/path");
Mockito.when(layerBuilder.getLayerEntries()).thenReturn(ImmutableList.of(layerEntry));

verifyCachedLayerIsExpected(expectedLayer, cachedLayerWithMetadata);
cacheWriter.writeLayer(layerBuilder);
Assert.assertEquals(1, getTarGzCountInCache());
long tarGzModifiedTime = getTarGzModifiedTimeInCache();

Thread.sleep(1000); // to have different modified time
cacheWriter.writeLayer(layerBuilder);
Assert.assertEquals(1, getTarGzCountInCache());
Assert.assertEquals(tarGzModifiedTime, getTarGzModifiedTimeInCache());
}

private long getTarGzCountInCache() throws IOException {
try (Stream<Path> stream = Files.walk(temporaryCacheDirectory.getRoot().toPath())) {
return stream.filter(CacheWriterTest::isTarGz).count();
}
}

private long getTarGzModifiedTimeInCache() throws IOException {
try (Stream<Path> stream = Files.walk(temporaryCacheDirectory.getRoot().toPath())) {
return stream.filter(CacheWriterTest::isTarGz).findFirst().get().toFile().lastModified();
}
}

@Test
public void testGetLayerOutputStream() throws IOException {
ExpectedLayer expectedLayer = getExpectedLayer();

// Writes resourceBlob as a layer to the cache.
CacheWriter cacheWriter = new CacheWriter(testCache);

CountingOutputStream layerOutputStream =
cacheWriter.getLayerOutputStream(expectedLayer.blobDescriptor.getDigest());
expectedLayer.blob.writeTo(layerOutputStream);
Expand All @@ -142,17 +173,15 @@ public void testGetLayerOutputStream() throws IOException {
* file
*/
private ExpectedLayer getExpectedLayer() throws IOException {
String expectedBlobAString =
new String(Files.readAllBytes(resourceBlob), StandardCharsets.UTF_8);

// Gets the expected content descriptor, diff ID, and compressed BLOB.
ByteArrayOutputStream compressedBlobOutputStream = new ByteArrayOutputStream();
CountingDigestOutputStream compressedDigestOutputStream =
new CountingDigestOutputStream(compressedBlobOutputStream);
CountingDigestOutputStream uncompressedDigestOutputStream;
try (GZIPOutputStream compressorStream = new GZIPOutputStream(compressedDigestOutputStream)) {
uncompressedDigestOutputStream = new CountingDigestOutputStream(compressorStream);
uncompressedDigestOutputStream.write(expectedBlobAString.getBytes(StandardCharsets.UTF_8));
byte[] expectedBlobABytes = Files.readAllBytes(resourceBlob);
uncompressedDigestOutputStream.write(expectedBlobABytes);
}

BlobDescriptor expectedBlobADescriptor = compressedDigestOutputStream.toBlobDescriptor();
Expand All @@ -171,15 +200,10 @@ private void verifyCachedLayerIsExpected(ExpectedLayer expectedLayer, CachedLaye
// Reads the cached layer back.
Path compressedBlobFile = cachedLayer.getContentFile();

try (InputStreamReader fileReader =
new InputStreamReader(
new GZIPInputStream(Files.newInputStream(compressedBlobFile)),
StandardCharsets.UTF_8)) {
String decompressedString = CharStreams.toString(fileReader);

String expectedBlobAString =
new String(Files.readAllBytes(resourceBlob), StandardCharsets.UTF_8);
Assert.assertEquals(expectedBlobAString, decompressedString);
try (GZIPInputStream in = new GZIPInputStream(Files.newInputStream(compressedBlobFile))) {
byte[] decompressedBytes = ByteStreams.toByteArray(in);
byte[] expectedBlobABytes = Files.readAllBytes(resourceBlob);
Assert.assertArrayEquals(expectedBlobABytes, decompressedBytes);
Assert.assertEquals(
expectedLayer.blobDescriptor.getSize(), cachedLayer.getBlobDescriptor().getSize());
Assert.assertEquals(
Expand Down

0 comments on commit 51b20c4

Please sign in to comment.