Skip to content

Commit

Permalink
Fix local base image temp directory cleanup (#2018)
Browse files Browse the repository at this point in the history
  • Loading branch information
TadCordle authored Sep 24, 2019
1 parent d92f83f commit 8d6efa5
Show file tree
Hide file tree
Showing 9 changed files with 138 additions and 123 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
import com.google.cloud.tools.jib.configuration.BuildConfiguration;
import com.google.cloud.tools.jib.docker.json.DockerManifestEntryTemplate;
import com.google.cloud.tools.jib.event.progress.ThrottledAccumulatingConsumer;
import com.google.cloud.tools.jib.filesystem.FileOperations;
import com.google.cloud.tools.jib.filesystem.TempDirectoryProvider;
import com.google.cloud.tools.jib.http.NotifyingOutputStream;
import com.google.cloud.tools.jib.image.Image;
import com.google.cloud.tools.jib.image.LayerCountMismatchException;
Expand Down Expand Up @@ -88,26 +88,28 @@ static boolean isGzipped(Path path) throws IOException {
private final BuildConfiguration buildConfiguration;
private final Path tarPath;
private final ProgressEventDispatcher.Factory progressEventDispatcherFactory;
private final TempDirectoryProvider tempDirectoryProvider;

ExtractTarStep(
BuildConfiguration buildConfiguration,
Path tarPath,
ProgressEventDispatcher.Factory progressEventDispatcherFactory) {
ProgressEventDispatcher.Factory progressEventDispatcherFactory,
TempDirectoryProvider tempDirectoryProvider) {
this.buildConfiguration = buildConfiguration;
this.tarPath = tarPath;
this.progressEventDispatcherFactory = progressEventDispatcherFactory;
this.tempDirectoryProvider = tempDirectoryProvider;
}

@Override
public LocalImage call()
throws IOException, LayerCountMismatchException, BadContainerConfigurationFormatException,
CacheCorruptedException {
Path destination = Files.createTempDirectory("jib-extract-tar");
Path destination = tempDirectoryProvider.newDirectory();
try (TimerEventDispatcher ignored =
new TimerEventDispatcher(
buildConfiguration.getEventHandlers(),
"Extracting tar " + tarPath + " into " + destination)) {
FileOperations.deleteRecursiveOnExit(destination);
TarExtractor.extract(tarPath, destination);

InputStream manifestStream = Files.newInputStream(destination.resolve("manifest.json"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,8 @@
import com.google.cloud.tools.jib.configuration.BuildConfiguration;
import com.google.cloud.tools.jib.docker.DockerClient;
import com.google.cloud.tools.jib.event.progress.ThrottledAccumulatingConsumer;
import com.google.cloud.tools.jib.filesystem.FileOperations;
import com.google.cloud.tools.jib.filesystem.TempDirectoryProvider;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.concurrent.Callable;

Expand All @@ -34,20 +33,22 @@ public class SaveDockerStep implements Callable<Path> {
private final BuildConfiguration buildConfiguration;
private final DockerClient dockerClient;
private final ProgressEventDispatcher.Factory progressEventDispatcherFactory;
private final TempDirectoryProvider tempDirectoryProvider;

SaveDockerStep(
BuildConfiguration buildConfiguration,
DockerClient dockerClient,
ProgressEventDispatcher.Factory progressEventDispatcherFactory) {
ProgressEventDispatcher.Factory progressEventDispatcherFactory,
TempDirectoryProvider tempDirectoryProvider) {
this.buildConfiguration = buildConfiguration;
this.dockerClient = dockerClient;
this.progressEventDispatcherFactory = progressEventDispatcherFactory;
this.tempDirectoryProvider = tempDirectoryProvider;
}

@Override
public Path call() throws IOException, InterruptedException {
Path outputDir = Files.createTempDirectory("jib-docker-save");
FileOperations.deleteRecursiveOnExit(outputDir);
Path outputDir = tempDirectoryProvider.newDirectory();
Path outputPath = outputDir.resolve("out.tar");
ImageReference imageReference = buildConfiguration.getBaseImageConfiguration().getImage();
try (TimerEventDispatcher ignored =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.google.cloud.tools.jib.configuration.BuildConfiguration;
import com.google.cloud.tools.jib.configuration.ImageConfiguration;
import com.google.cloud.tools.jib.docker.DockerClient;
import com.google.cloud.tools.jib.filesystem.TempDirectoryProvider;
import com.google.cloud.tools.jib.global.JibSystemProperties;
import com.google.cloud.tools.jib.http.Authorization;
import com.google.cloud.tools.jib.image.Image;
Expand Down Expand Up @@ -102,6 +103,7 @@ private static <E> List<E> realizeFutures(List<Future<E>> futures)

private final ExecutorService executorService;
private final BuildConfiguration buildConfiguration;
private final TempDirectoryProvider tempDirectoryProvider = new TempDirectoryProvider();

// We save steps to run by wrapping each step into a Runnable, only because of the unfortunate
// chicken-and-egg situation arising from using ProgressEventDispatcher. The current
Expand Down Expand Up @@ -181,6 +183,9 @@ public BuildResult run() throws ExecutionException, InterruptedException {
unrolled = (ExecutionException) unrolled.getCause();
}
throw unrolled;

} finally {
tempDirectoryProvider.close();
}
}

Expand Down Expand Up @@ -239,7 +244,10 @@ private void saveDocker() {
results.tarPath =
executorService.submit(
new SaveDockerStep(
buildConfiguration, dockerClient.get(), childProgressDispatcherFactory));
buildConfiguration,
dockerClient.get(),
childProgressDispatcherFactory,
tempDirectoryProvider));
}

private void extractTar() {
Expand All @@ -249,7 +257,10 @@ private void extractTar() {
executorService.submit(
() ->
new ExtractTarStep(
buildConfiguration, results.tarPath.get(), childProgressDispatcherFactory)
buildConfiguration,
results.tarPath.get(),
childProgressDispatcherFactory,
tempDirectoryProvider)
.call());
results.baseImageAndAuth =
executorService.submit(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import com.google.cloud.tools.jib.blob.Blobs;
import com.google.cloud.tools.jib.filesystem.FileOperations;
import com.google.cloud.tools.jib.filesystem.LockFile;
import com.google.cloud.tools.jib.filesystem.TemporaryDirectory;
import com.google.cloud.tools.jib.filesystem.TempDirectoryProvider;
import com.google.cloud.tools.jib.hash.CountingDigestOutputStream;
import com.google.cloud.tools.jib.hash.Digests;
import com.google.cloud.tools.jib.image.json.BuildableManifestTemplate;
Expand Down Expand Up @@ -171,9 +171,9 @@ CachedLayer writeCompressed(Blob compressedLayerBlob) throws IOException {

// Creates the temporary directory.
Files.createDirectories(cacheStorageFiles.getTemporaryDirectory());
try (TemporaryDirectory temporaryDirectory =
new TemporaryDirectory(cacheStorageFiles.getTemporaryDirectory())) {
Path temporaryLayerDirectory = temporaryDirectory.getDirectory();
try (TempDirectoryProvider tempDirectoryProvider = new TempDirectoryProvider()) {
Path temporaryLayerDirectory =
tempDirectoryProvider.newDirectory(cacheStorageFiles.getTemporaryDirectory());

// Writes the layer file to the temporary directory.
WrittenLayer writtenLayer =
Expand Down Expand Up @@ -220,9 +220,9 @@ CachedLayer writeUncompressed(Blob uncompressedLayerBlob, @Nullable DescriptorDi
// Creates the temporary directory. The temporary directory must be in the same FileStore as the
// final location for Files.move to work.
Files.createDirectories(cacheStorageFiles.getTemporaryDirectory());
try (TemporaryDirectory temporaryDirectory =
new TemporaryDirectory(cacheStorageFiles.getTemporaryDirectory())) {
Path temporaryLayerDirectory = temporaryDirectory.getDirectory();
try (TempDirectoryProvider tempDirectoryProvider = new TempDirectoryProvider()) {
Path temporaryLayerDirectory =
tempDirectoryProvider.newDirectory(cacheStorageFiles.getTemporaryDirectory());

// Writes the layer file to the temporary directory.
WrittenLayer writtenLayer =
Expand Down Expand Up @@ -261,9 +261,9 @@ CachedLayer writeUncompressed(Blob uncompressedLayerBlob, @Nullable DescriptorDi
CachedLayer writeTarLayer(DescriptorDigest diffId, Blob compressedBlob) throws IOException {
Files.createDirectories(cacheStorageFiles.getLocalDirectory());
Files.createDirectories(cacheStorageFiles.getTemporaryDirectory());
try (TemporaryDirectory temporaryDirectory =
new TemporaryDirectory(cacheStorageFiles.getTemporaryDirectory())) {
Path temporaryLayerDirectory = temporaryDirectory.getDirectory();
try (TempDirectoryProvider tempDirectoryProvider = new TempDirectoryProvider()) {
Path temporaryLayerDirectory =
tempDirectoryProvider.newDirectory(cacheStorageFiles.getTemporaryDirectory());
Path temporaryLayerFile = cacheStorageFiles.getTemporaryLayerFile(temporaryLayerDirectory);

BlobDescriptor layerBlobDescriptor;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@
package com.google.cloud.tools.jib.filesystem;

import com.google.common.collect.ImmutableList;
import com.google.common.io.MoreFiles;
import com.google.common.io.RecursiveDeleteOption;
import java.io.IOException;
import java.io.OutputStream;
import java.nio.channels.Channels;
Expand Down Expand Up @@ -87,24 +85,5 @@ public static OutputStream newLockingOutputStream(Path file) throws IOException
return Channels.newOutputStream(channel);
}

/**
* Sets up a shutdown hook that tries to delete a file or directory.
*
* @param path the path to the file or directory
*/
public static void deleteRecursiveOnExit(Path path) {
Runtime.getRuntime()
.addShutdownHook(
new Thread(
() -> {
if (Files.exists(path)) {
try {
MoreFiles.deleteRecursively(path, RecursiveDeleteOption.ALLOW_INSECURE);
} catch (IOException ignored) {
}
}
}));
}

private FileOperations() {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*
* Copyright 2019 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.filesystem;

import com.google.common.io.MoreFiles;
import com.google.common.io.RecursiveDeleteOption;
import java.io.Closeable;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;

/** Creates temporary directories and deletes them all when closed. */
public class TempDirectoryProvider implements Closeable {

private final Set<Path> directories = Collections.synchronizedSet(new HashSet<>());

/**
* Creates a new temporary directory.
*
* @return the path to the temporary directory
* @throws IOException if creating the directory fails
*/
public Path newDirectory() throws IOException {
Path path = Files.createTempDirectory(null);
directories.add(path);
return path;
}

/**
* Creates a new temporary directory.
*
* @param parentDirectory the directory to create the temp directory inside
* @return the path to the temporary directory
* @throws IOException if creating the directory fails
*/
public Path newDirectory(Path parentDirectory) throws IOException {
Path path = Files.createTempDirectory(parentDirectory, null);
directories.add(path);
return path;
}

@Override
public void close() {
for (Path path : directories) {
if (Files.exists(path)) {
try {
MoreFiles.deleteRecursively(path, RecursiveDeleteOption.ALLOW_INSECURE);
} catch (IOException ignored) {
}
}
}
directories.clear();
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.google.cloud.tools.jib.cache.CacheCorruptedException;
import com.google.cloud.tools.jib.configuration.BuildConfiguration;
import com.google.cloud.tools.jib.event.EventHandlers;
import com.google.cloud.tools.jib.filesystem.TempDirectoryProvider;
import com.google.cloud.tools.jib.image.LayerCountMismatchException;
import com.google.cloud.tools.jib.image.json.BadContainerConfigurationFormatException;
import com.google.common.io.Resources;
Expand Down Expand Up @@ -73,7 +74,12 @@ public void testCall_validDocker()
BadContainerConfigurationFormatException, IOException, CacheCorruptedException {
Path dockerBuild = getResource("core/extraction/docker-save.tar");
LocalImage result =
new ExtractTarStep(buildConfiguration, dockerBuild, progressEventDispatcherFactory).call();
new ExtractTarStep(
buildConfiguration,
dockerBuild,
progressEventDispatcherFactory,
new TempDirectoryProvider())
.call();

Mockito.verify(progressEventDispatcher, Mockito.times(2)).newChildProducer();
Assert.assertEquals(2, result.layers.size());
Expand All @@ -98,7 +104,12 @@ public void testCall_validTar()
BadContainerConfigurationFormatException, IOException, CacheCorruptedException {
Path tarBuild = getResource("core/extraction/jib-image.tar");
LocalImage result =
new ExtractTarStep(buildConfiguration, tarBuild, progressEventDispatcherFactory).call();
new ExtractTarStep(
buildConfiguration,
tarBuild,
progressEventDispatcherFactory,
new TempDirectoryProvider())
.call();

Mockito.verify(progressEventDispatcher, Mockito.times(2)).newChildProducer();
Assert.assertEquals(2, result.layers.size());
Expand Down
Loading

0 comments on commit 8d6efa5

Please sign in to comment.