Download extensions only once and if the version changes #488

welandaz · 2024-09-16T22:42:11Z

This change will download Maven extensions only once to the controller node and then use the same copying mechanism we originally had. The only difference is that before we had a predefined location where the extensions where stored (as they were bundled), and now they will be re-downloaded if needed into jenkins-gradle-plugin/cache on the controller node.

The download happens for the first time when injection get's enabled or when a user updates the version they want to inject. We now store a metadata file with a downloaded version and a digest of the JAR which we use for a comparison to determine if we need to copy it to an agent node.

Manual testing was also performed on the locally installed instance in addition to the automated tests we have

welandaz · 2024-09-16T22:46:43Z

src/main/java/hudson/plugins/gradle/injection/MavenExtensionDownloadHandler.java

+import java.util.List;
+import java.util.Map;
+
+public class MavenExtensionDownloadHandler implements MavenInjectionAware {


I tested this logic manually, but I'm also going to add a test as part of this PR for this specific class

alextu · 2024-09-17T13:22:38Z

src/main/java/hudson/plugins/gradle/injection/DevelocityComputerListener.java

                return;
            }

-            injector.inject(computer, globalEnvVars);
+            Map<MavenExtension, String> extensionsDigest = mavenExtensionDownloadHandler.ensureExtensionsDownloaded(


This can be called concurrently (multiple agents launching at the same time). If somehow the extension was not downloaded after saving the configuration, it could get corrupted when downloaded multiple times. If we think it's a realistic issue we can just have ensureExtensionDownloaded making sure the extension is there without downloading it (so no injection in that case). After all now it has to be an explicit action to enable the Maven injection. Wdyt?

I've removed downloading on agent's onOnline event as we discussed.

Everything else stays as it's sequential and synchronized by Jenkins

alextu · 2024-09-17T13:26:43Z

src/main/java/hudson/plugins/gradle/injection/MavenExtensionDownloadHandler.java

+    public Map<MavenExtension, String> ensureExtensionsDownloaded(File root, InjectionConfig injectionConfig) throws IOException {
+        if (!isInjectionDisabledGlobally(injectionConfig)) {
+            Map<MavenExtension, String> extensionsDigest = new HashMap<>();
+            Path parent = root.toPath().resolve(DOWNLOAD_CACHE_DIR);


Suggested change

Path parent = root.toPath().resolve(DOWNLOAD_CACHE_DIR);

Path cacheDir = root.toPath().resolve(DOWNLOAD_CACHE_DIR);

alextu · 2024-09-17T13:26:56Z

src/main/java/hudson/plugins/gradle/injection/MavenExtensionDownloadHandler.java

+        return Collections.emptyMap();
+    }
+
+    private static String getExtensionDigest(InjectionConfig injectionConfig, Path parent, MavenExtension extension) throws IOException {


Suggested change

private static String getExtensionDigest(InjectionConfig injectionConfig, Path parent, MavenExtension extension) throws IOException {

private static String getExtensionDigest(InjectionConfig injectionConfig, Path cacheDir, MavenExtension extension) throws IOException {

alextu · 2024-09-17T13:28:47Z

src/main/java/hudson/plugins/gradle/injection/MavenExtensionDownloadHandler.java

+
+    private static String downloadExtension(
+            InjectionConfig injectionConfig,
+            Path parent,


Suggested change

Path parent,

Path cacheDir,

src/test/groovy/hudson/plugins/gradle/injection/MavenExtensionDownloadHandlerTest.groovy

alextu · 2024-09-17T13:32:59Z

src/test/groovy/hudson/plugins/gradle/injection/MavenExtensionDownloadHandlerTest.groovy

+
+        then:
+        sameExtensions.size() == 2
+        sameExtensions.get(MavenExtension.DEVELOCITY) == originalDevelocityDigest && sameExtensions.get(MavenExtension.CCUD) == originalCcudDigest


🤔 how does it test if it's not re-downloaded?

Just by verifying that the digest of the one downloaded before hasn't changed.
Or do you mean that it can still download it, just the same version again, which will produce same digest?

yes, exactly

Added additional check for lastModified

…DownloadHandlerTest.groovy Co-authored-by: Alexis Tual <atual@gradle.com>

* Download extensions only once and if the version changes * Update tests * Update src/test/groovy/hudson/plugins/gradle/injection/MavenExtensionDownloadHandlerTest.groovy Co-authored-by: Alexis Tual <atual@gradle.com> * Do not download extensions on agents onOnline * Fix CCUD extension handling * Do not set bogus extension repository URL in test * Disable a couple of tests to validate windows failures * Reenable tests again --------- Co-authored-by: Alexis Tual <atual@gradle.com>

Download extensions only once and if the version changes

4622e4c

welandaz added the enhancement label Sep 16, 2024

welandaz requested review from alextu and guylabs September 16, 2024 22:42

welandaz self-assigned this Sep 16, 2024

welandaz commented Sep 16, 2024

View reviewed changes

Update tests

7b84a88

alextu approved these changes Sep 17, 2024

View reviewed changes

Update src/test/groovy/hudson/plugins/gradle/injection/MavenExtension…

f1e91e3

…DownloadHandlerTest.groovy Co-authored-by: Alexis Tual <atual@gradle.com>

guylabs removed their request for review September 17, 2024 15:08

welandaz added 2 commits September 17, 2024 17:51

Do not download extensions on agents onOnline

c349bd6

Fix CCUD extension handling

77799d0

alextu approved these changes Sep 18, 2024

View reviewed changes

welandaz added 3 commits September 18, 2024 12:08

Do not set bogus extension repository URL in test

41aeb2f

Disable a couple of tests to validate windows failures

3a4bb6a

Reenable tests again

654ebcd

welandaz merged commit e435729 into master Sep 18, 2024
20 of 22 checks passed

welandaz deleted the welandaz/download-extensions-once branch September 18, 2024 12:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Download extensions only once and if the version changes #488

Download extensions only once and if the version changes #488

welandaz commented Sep 16, 2024 •

edited

Loading

welandaz Sep 16, 2024 •

edited

Loading

welandaz Sep 17, 2024

alextu Sep 17, 2024

welandaz Sep 17, 2024

alextu Sep 17, 2024

welandaz Sep 17, 2024

alextu Sep 17, 2024

welandaz Sep 17, 2024

alextu Sep 17, 2024

welandaz Sep 17, 2024

alextu Sep 17, 2024

welandaz Sep 17, 2024

alextu Sep 17, 2024

welandaz Sep 17, 2024

	Path parent = root.toPath().resolve(DOWNLOAD_CACHE_DIR);
	Path cacheDir = root.toPath().resolve(DOWNLOAD_CACHE_DIR);

	private static String getExtensionDigest(InjectionConfig injectionConfig, Path parent, MavenExtension extension) throws IOException {
	private static String getExtensionDigest(InjectionConfig injectionConfig, Path cacheDir, MavenExtension extension) throws IOException {

Download extensions only once and if the version changes #488

Download extensions only once and if the version changes #488

Conversation

welandaz commented Sep 16, 2024 • edited Loading

welandaz Sep 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

welandaz commented Sep 16, 2024 •

edited

Loading

welandaz Sep 16, 2024 •

edited

Loading