Skip to content

Allow specific version to be deleted from cache #4

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
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 @@ -202,7 +202,7 @@ static List<URL> retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriev
FilePath libDir = new FilePath(execution.getOwner().getRootDir()).child("libs/" + record.getDirectoryName());
Boolean shouldCache = cachingConfiguration != null;
final FilePath versionCacheDir = new FilePath(LibraryCachingConfiguration.getGlobalLibrariesCacheDir(), record.getDirectoryName());
ReentrantReadWriteLock retrieveLock = getReadWriteLockFor(record.getDirectoryName());
ReentrantReadWriteLock retrieveLock = getReadWriteLockFor(record.getName());
Copy link
Author

Choose a reason for hiding this comment

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

This is to make it consistent across accesses because record.getName() is no longer equal to record.getDirectoryName().

final FilePath lastReadFile = new FilePath(versionCacheDir, LibraryCachingConfiguration.LAST_READ_FILE);

if(shouldCache && cachingConfiguration.isExcluded(version)) {
Expand Down Expand Up @@ -238,7 +238,7 @@ static List<URL> retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriev
}

if (retrieve) {
listener.getLogger().println("Caching library " + name + "@" + version);
listener.getLogger().println("Caching library " + name + "@" + version);
versionCacheDir.mkdirs();
retriever.retrieve(name, version, changelog, versionCacheDir, run, listener);
}
Expand All @@ -251,7 +251,7 @@ static List<URL> retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriev
}

lastReadFile.touch(System.currentTimeMillis());
versionCacheDir.withSuffix("-name.txt").write(name, "UTF-8");
versionCacheDir.withSuffix("-name.txt").write(name + "@" + version, "UTF-8");
versionCacheDir.copyRecursiveTo(libDir);
} finally {
retrieveLock.readLock().unlock();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,13 @@ public LibraryCachingCleanup() {
@Override protected void execute(TaskListener listener) throws IOException, InterruptedException {
FilePath globalCacheDir = LibraryCachingConfiguration.getGlobalLibrariesCacheDir();
for (FilePath library : globalCacheDir.list()) {
if (!removeIfExpiredCacheDirectory(library)) {
// Prior to the SECURITY-2586 fix, library caches had a two-level directory structure.
// These caches will never be used again, so we delete any that we find.
for (FilePath version: library.list()) {
if (version.child(LibraryCachingConfiguration.LAST_READ_FILE).exists()) {
library.deleteRecursive();
break;
for (FilePath versionDir : library.listDirectories()) {
if (!removeIfExpiredCacheDirectory(versionDir)) {
FilePath parent = versionDir.getParent();
if (parent != null) {
parent.deleteRecursive();
}
break;
}
}
}
Expand All @@ -48,14 +47,15 @@ public LibraryCachingCleanup() {
*/
private boolean removeIfExpiredCacheDirectory(FilePath library) throws IOException, InterruptedException {
final FilePath lastReadFile = new FilePath(library, LibraryCachingConfiguration.LAST_READ_FILE);
if (lastReadFile.exists()) {
if (lastReadFile.exists() && library.withSuffix("-name.txt").exists()) {
ReentrantReadWriteLock retrieveLock = LibraryAdder.getReadWriteLockFor(library.getName());
retrieveLock.writeLock().lockInterruptibly();
try {
if (System.currentTimeMillis() - lastReadFile.lastModified() > TimeUnit.DAYS.toMillis(EXPIRE_AFTER_READ_DAYS)) {

library.deleteRecursive();
library.withSuffix("-name.txt").delete();
FilePath parent = library.getParent();
if (parent != null) {
parent.deleteRecursive();
}
}
} finally {
retrieveLock.writeLock().unlock();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,46 +89,61 @@ public static FilePath getGlobalLibrariesCacheDir() {
}

@Extension public static class DescriptorImpl extends Descriptor<LibraryCachingConfiguration> {
public FormValidation doClearCache(@QueryParameter String name, @QueryParameter boolean forceDelete) throws InterruptedException {
public FormValidation doClearCache(@QueryParameter String name, @QueryParameter String cachedLibraryRef, @QueryParameter boolean forceDelete) throws InterruptedException {
Jenkins.get().checkPermission(Jenkins.ADMINISTER);

String cacheDirName = null;
try {
if (LibraryCachingConfiguration.getGlobalLibrariesCacheDir().exists()) {
for (FilePath libraryNamePath : LibraryCachingConfiguration.getGlobalLibrariesCacheDir().list("*-name.txt")) {
// Libraries configured in distinct locations may have the same name. Since only admins are allowed here, this is not a huge issue, but it is probably unexpected.
String cacheName;
try (InputStream stream = libraryNamePath.read()) {
cacheName = IOUtils.toString(stream, StandardCharsets.UTF_8);
}
if (cacheName.equals(name)) {
FilePath libraryCachePath = LibraryCachingConfiguration.getGlobalLibrariesCacheDir()
.child(libraryNamePath.getName().replace("-name.txt", ""));
if (forceDelete) {
LOGGER.log(Level.FINER, "Force deleting cache for {0}", name);
libraryCachePath.deleteRecursive();
libraryNamePath.delete();
} else {
LOGGER.log(Level.FINER, "Safe deleting cache for {0}", name);
ReentrantReadWriteLock retrieveLock = LibraryAdder.getReadWriteLockFor(libraryCachePath.getName());
if (retrieveLock.writeLock().tryLock(10, TimeUnit.SECONDS)) {
try {
libraryCachePath.deleteRecursive();
libraryNamePath.delete();
} finally {
retrieveLock.writeLock().unlock();
outer: for (FilePath libraryCache : LibraryCachingConfiguration.getGlobalLibrariesCacheDir().listDirectories()) {
for (FilePath libraryNamePath : libraryCache.list("*-name.txt")) {
if (libraryNamePath.readToString().startsWith(name + "@")) {
FilePath libraryCachePath = libraryNamePath.getParent();
if (libraryCachePath != null) {
FilePath versionCachePath = new FilePath(libraryCachePath, libraryNamePath.getName().replace("-name.txt", ""));
LOGGER.log(Level.FINER, "Safe deleting cache for {0}", name);
ReentrantReadWriteLock retrieveLock = LibraryAdder.getReadWriteLockFor(libraryCachePath.getName());
if (forceDelete || retrieveLock.writeLock().tryLock(10, TimeUnit.SECONDS)) {
if (forceDelete) {
LOGGER.log(Level.FINER, "Force deleting cache for {0}", name);
} else {
LOGGER.log(Level.FINER, "Safe deleting cache for {0}", name);
}
try {
if (StringUtils.isNotEmpty(cachedLibraryRef)) {
if (libraryNamePath.readToString().equals(name + "@" + cachedLibraryRef)) {
cacheDirName = name + "@" + cachedLibraryRef;
libraryNamePath.delete();
versionCachePath.deleteRecursive();
break outer;
}
} else {
cacheDirName = name;
libraryCachePath.deleteRecursive();
break outer;
}
} finally {
if (!forceDelete) {
retrieveLock.writeLock().unlock();
}
}
} else {
return FormValidation.error("The cache dir could not be deleted because it is currently being used by another thread. Please try again.");
}
} else {
return FormValidation.error("The cache dir could not be deleted because it is currently being used by another thread. Please try again.");
}
}
}
}
}
} catch (IOException ex) {
return FormValidation.error(ex, "The cache dir was not deleted successfully");
return FormValidation.error(ex, String.format("The cache dir %s was not deleted successfully", cacheDirName));
}

if (cacheDirName == null) {
return FormValidation.ok(String.format("The version %s was not found for library %s.", cachedLibraryRef, name));
} else {
return FormValidation.ok(String.format("The cache dir %s was deleted successfully.", cacheDirName));
}
return FormValidation.ok("The cache dir was deleted successfully.");
}

}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

package org.jenkinsci.plugins.workflow.libs;

import java.io.File;
import java.util.Collections;
import java.util.Set;
import java.util.TreeSet;
Expand Down Expand Up @@ -62,7 +63,7 @@ public final class LibraryRecord {
this.trusted = trusted;
this.changelog = changelog;
this.cachingConfiguration = cachingConfiguration;
this.directoryName = directoryNameFor(name, version, String.valueOf(trusted), source);
this.directoryName = directoryNameFor(name, String.valueOf(trusted), source) + File.separator + directoryNameFor(version);
}

@Exported
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,12 @@ THE SOFTWARE.
<f:textbox />
</f:entry>
<j:if test="${h.hasPermission(app.ADMINISTER)}">
<f:entry title="${%Force clear cache}" field="forceDelete">
<f:entry title="${%Clear cache for ref}" field="cachedLibraryRef">
<f:textbox />
</f:entry>
<f:entry title="${%Force clear cache}" field="forceDelete">
<f:checkbox/>
</f:entry>
<f:validateButton title="${%Clear cache}" progress="${%Clearing...}" method="clearCache" with="name,forceDelete" />
<f:validateButton title="${%Clear cache}" progress="${%Clearing...}" method="clearCache" with="name,cachedLibraryRef,forceDelete" />
</j:if>
</j:jelly>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<div>
Specifies a specific version to clear the cache for. An empty value will clear the cache for all versions.
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,16 @@ public void smokes() throws Throwable {
assertThat(new File(cache.getRemote()), anExistingDirectory());
// Run LibraryCachingCleanup and show that cache is not deleted.
ExtensionList.lookupSingleton(LibraryCachingCleanup.class).execute(StreamTaskListener.fromStderr());
assertThat(new File(cache.getParent().getRemote()), anExistingDirectory());
assertThat(new File(cache.getRemote()), anExistingDirectory());
assertThat(new File(cache.withSuffix("-name.txt").getRemote()), anExistingFile());
// Run LibraryCachingCleanup after modifying LAST_READ_FILE to be an old date and and show that cache is deleted.
long oldMillis = ZonedDateTime.now().minusDays(LibraryCachingCleanup.EXPIRE_AFTER_READ_DAYS + 1).toInstant().toEpochMilli();
cache.child(LibraryCachingConfiguration.LAST_READ_FILE).touch(oldMillis);
ExtensionList.lookupSingleton(LibraryCachingCleanup.class).execute(StreamTaskListener.fromStderr());
assertThat(new File(cache.getRemote()), not(anExistingDirectory()));
assertThat(new File(cache.withSuffix("-name.txt").getRemote()), not(anExistingDirectory()));
assertThat(new File(cache.getParent().getRemote()), not(anExistingDirectory()));
assertThat(new File(cache.withSuffix("-name.txt").getRemote()), not(anExistingFile()));

}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
import hudson.ExtensionList;
import hudson.FilePath;
import java.io.File;
import java.util.Arrays;
import java.util.List;
import jenkins.plugins.git.GitSCMSource;
import jenkins.plugins.git.GitSampleRepoRule;
import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
Expand Down Expand Up @@ -157,29 +159,68 @@

@Test
public void clearCache() throws Exception {
List<FilePath> caches = setupLibraryCaches();
FilePath cache = caches.get(0);
FilePath cache2 = caches.get(1);
assertThat("Must be different paths", cache, not(equalTo(cache2)));
assertThat(new File(cache.getParent().getRemote()), anExistingDirectory());
assertThat(new File(cache.getRemote()), anExistingDirectory());
assertThat(new File(cache2.getRemote()), anExistingDirectory());
assertThat(new File(cache.withSuffix("-name.txt").getRemote()), anExistingFile());
assertThat(cache.withSuffix("-name.txt").readToString(), equalTo("library@master"));
assertThat(cache2.withSuffix("-name.txt").readToString(), equalTo("library@feature/something"));
// Clear the cache. TODO: Would be more realistic to set up security and use WebClient.

Check warning on line 172 in src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingConfigurationTest.java

View check run for this annotation

ci.jenkins.io / Open Tasks Scanner

TODO

NORMAL: Would be more realistic to set up security and use WebClient.
ExtensionList.lookupSingleton(LibraryCachingConfiguration.DescriptorImpl.class).doClearCache("library", "", false);
assertThat(new File(cache.getParent().getRemote()), not(anExistingDirectory()));
assertThat(new File(cache.withSuffix("-name.txt").getRemote()), not(anExistingFile()));
}

@Test
public void clearCacheVersion() throws Exception {

List<FilePath> caches = setupLibraryCaches();
FilePath cache = caches.get(0);
FilePath cache2 = caches.get(1);
assertThat(new File(cache.getRemote()), anExistingDirectory());
// Clear the cache. TODO: Would be more realistic to set up security and use WebClient.

Check warning on line 185 in src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryCachingConfigurationTest.java

View check run for this annotation

ci.jenkins.io / Open Tasks Scanner

TODO

NORMAL: Would be more realistic to set up security and use WebClient.
ExtensionList.lookupSingleton(LibraryCachingConfiguration.DescriptorImpl.class).doClearCache("library", "master", false);
assertThat(new File(cache.getParent().getRemote()), anExistingDirectory());
assertThat(new File(cache.getRemote()), not(anExistingDirectory()));
assertThat(new File(cache.withSuffix("-name.txt").getRemote()), not(anExistingFile()));
//Other cache has not been touched
assertThat(new File(cache2.getRemote()), anExistingDirectory());
assertThat(new File(cache2.withSuffix("-name.txt").getRemote()), anExistingFile());
}


private List<FilePath> setupLibraryCaches() throws Exception {
sampleRepo.init();
sampleRepo.write("vars/foo.groovy", "def call() { echo 'foo' }");
sampleRepo.git("add", "vars");
sampleRepo.git("commit", "--message=init");
sampleRepo.git("branch", "feature/something");
LibraryConfiguration config = new LibraryConfiguration("library",
new SCMSourceRetriever(new GitSCMSource(null, sampleRepo.toString(), "", "*", "", true)));
config.setDefaultVersion("master");
config.setImplicit(true);
config.setImplicit(false);
config.setCachingConfiguration(new LibraryCachingConfiguration(30, null));
config.setAllowVersionOverride(true);
GlobalLibraries.get().getLibraries().add(config);
// Run build and check that cache gets created.
WorkflowJob p = r.createProject(WorkflowJob.class);
p.setDefinition(new CpsFlowDefinition("foo()", true));
p.setDefinition(new CpsFlowDefinition("library identifier: 'library', changelog:false\n\nfoo()", true));
WorkflowRun b = r.buildAndAssertSuccess(p);
WorkflowJob p2 = r.createProject(WorkflowJob.class);
p2.setDefinition(new CpsFlowDefinition("library identifier: 'library@feature/something', changelog:false\n\nfoo()", true));
WorkflowRun b2 = r.buildAndAssertSuccess(p2);
LibrariesAction action = b.getAction(LibrariesAction.class);
LibraryRecord record = action.getLibraries().get(0);
LibrariesAction action2 = b2.getAction(LibrariesAction.class);
LibraryRecord record2 = action2.getLibraries().get(0);

FilePath cache = LibraryCachingConfiguration.getGlobalLibrariesCacheDir().child(record.getDirectoryName());
assertThat(new File(cache.getRemote()), anExistingDirectory());
assertThat(new File(cache.withSuffix("-name.txt").getRemote()), anExistingFile());
// Clear the cache. TODO: Would be more realistic to set up security and use WebClient.
ExtensionList.lookupSingleton(LibraryCachingConfiguration.DescriptorImpl.class).doClearCache("library", false);
assertThat(new File(cache.getRemote()), not(anExistingDirectory()));
assertThat(new File(cache.withSuffix("-name.txt").getRemote()), not(anExistingFile()));
FilePath cache2 = LibraryCachingConfiguration.getGlobalLibrariesCacheDir().child(record2.getDirectoryName());

return Arrays.asList(cache, cache2);
}

}
Loading
Loading