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

Fix local base image temp directory cleanup #2018

Merged
merged 5 commits into from
Sep 24, 2019
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
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