diff --git a/jib-core/src/main/java/com/google/cloud/tools/jib/cache/CacheWriter.java b/jib-core/src/main/java/com/google/cloud/tools/jib/cache/CacheWriter.java index 21aa29a2e7..69b55f06ae 100644 --- a/jib-core/src/main/java/com/google/cloud/tools/jib/cache/CacheWriter.java +++ b/jib-core/src/main/java/com/google/cloud/tools/jib/cache/CacheWriter.java @@ -27,9 +27,9 @@ import java.io.BufferedOutputStream; 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; @@ -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 = diff --git a/jib-core/src/test/java/com/google/cloud/tools/jib/cache/CacheWriterTest.java b/jib-core/src/test/java/com/google/cloud/tools/jib/cache/CacheWriterTest.java index 2de09e2fd1..3f70700e7e 100644 --- a/jib-core/src/test/java/com/google/cloud/tools/jib/cache/CacheWriterTest.java +++ b/jib-core/src/test/java/com/google/cloud/tools/jib/cache/CacheWriterTest.java @@ -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; @@ -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; @@ -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 = @@ -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 stream = Files.walk(temporaryCacheDirectory.getRoot().toPath())) { + return stream.filter(CacheWriterTest::isTarGz).count(); + } + } + + private long getTarGzModifiedTimeInCache() throws IOException { + try (Stream 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); @@ -142,9 +173,6 @@ 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 = @@ -152,7 +180,8 @@ private ExpectedLayer getExpectedLayer() throws IOException { 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(); @@ -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(