From aa29b57a99a524e71505d25edcd119b9f3db3708 Mon Sep 17 00:00:00 2001 From: Bhanu Reddy Date: Fri, 10 Apr 2026 17:31:16 +0530 Subject: [PATCH 1/3] RTECO-1006 - Optimize CLI installation: per-pipeline cache, cross-platform OS detection, stale sha256 cleanup --- e2e/local/docker-compose-agents.yml | 24 ++ e2e/local/setup-agents.sh | 261 ++++++++++++++++++ .../plugins/jfrog/ArtifactoryInstaller.java | 2 +- .../plugins/jfrog/BinaryInstaller.java | 164 +++++++++-- 4 files changed, 433 insertions(+), 18 deletions(-) create mode 100644 e2e/local/docker-compose-agents.yml create mode 100755 e2e/local/setup-agents.sh diff --git a/e2e/local/docker-compose-agents.yml b/e2e/local/docker-compose-agents.yml new file mode 100644 index 00000000..e8936134 --- /dev/null +++ b/e2e/local/docker-compose-agents.yml @@ -0,0 +1,24 @@ +services: + agent-linux-amd64: + image: jenkins/inbound-agent:latest + platform: linux/amd64 + container_name: jenkins-agent-amd64 + environment: + JENKINS_URL: http://host.docker.internal:8080 + JENKINS_AGENT_NAME: agent-linux-amd64 + JENKINS_SECRET: ${AGENT_AMD64_SECRET} + JENKINS_AGENT_WORKDIR: /home/jenkins/agent + extra_hosts: + - "host.docker.internal:host-gateway" + + agent-linux-arm64: + image: jenkins/inbound-agent:latest + platform: linux/arm64 + container_name: jenkins-agent-arm64 + environment: + JENKINS_URL: http://host.docker.internal:8080 + JENKINS_AGENT_NAME: agent-linux-arm64 + JENKINS_SECRET: ${AGENT_ARM64_SECRET} + JENKINS_AGENT_WORKDIR: /home/jenkins/agent + extra_hosts: + - "host.docker.internal:host-gateway" diff --git a/e2e/local/setup-agents.sh b/e2e/local/setup-agents.sh new file mode 100755 index 00000000..3926bfb0 --- /dev/null +++ b/e2e/local/setup-agents.sh @@ -0,0 +1,261 @@ +#!/usr/bin/env bash +# Usage: source e2e/setup-agents.sh +# +# Creates Jenkins JNLP agents for cross-platform testing, starts the Docker +# containers, and waits for them to come online. +# +# Prerequisites: +# - Jenkins running on localhost:8080 (admin/password) +# - The jfrog plugin HPI installed in Jenkins +# - Docker with multi-platform support (buildx) +# +# What it does: +# 1. Enables the TCP slave agent listener (port 50000) +# 2. Creates two permanent agents: agent-linux-amd64, agent-linux-arm64 +# 3. Configures the "releases-jfrog-cli" tool (Install from releases.jfrog.io) +# 4. Starts Docker containers that connect back to Jenkins +# 5. Creates a cross-platform test pipeline +# +# After running, trigger the test with: +# curl -X POST http://localhost:8080/job/test-cross-platform/build -u admin:password + +set -euo pipefail + +JENKINS_URL="${JENKINS_URL:-http://localhost:8080}" +JENKINS_USER="${JENKINS_USER:-admin}" +JENKINS_PASS="${JENKINS_PASS:-password}" +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + +get_crumb() { + local cookies="$1" + curl -sf -c "$cookies" "${JENKINS_URL}/crumbIssuer/api/json" \ + -u "${JENKINS_USER}:${JENKINS_PASS}" | python3 -c "import json,sys; print(json.load(sys.stdin)['crumb'])" +} + +run_groovy() { + local script="$1" + local cookies + cookies=$(mktemp) + local crumb + crumb=$(get_crumb "$cookies") + curl -sf -X POST "${JENKINS_URL}/scriptText" \ + -u "${JENKINS_USER}:${JENKINS_PASS}" \ + -b "$cookies" \ + -H "Jenkins-Crumb: ${crumb}" \ + --data-urlencode "script=${script}" + rm -f "$cookies" +} + +echo "==> Checking Jenkins is up..." +if ! curl -sf "${JENKINS_URL}/api/json" -u "${JENKINS_USER}:${JENKINS_PASS}" > /dev/null 2>&1; then + echo "ERROR: Jenkins not reachable at ${JENKINS_URL}" + return 1 +fi + +echo "==> Enabling TCP slave agent listener on port 50000..." +run_groovy ' +import jenkins.model.Jenkins +def jenkins = Jenkins.instance +jenkins.setSlaveAgentPort(50000) +jenkins.save() +println "TCP listener enabled on port: " + jenkins.getSlaveAgentPort() +' + +echo "" +echo "==> Creating agents and retrieving secrets..." +SECRETS=$(run_groovy ' +import jenkins.model.* +import hudson.model.* +import hudson.slaves.* + +def createAgent(String name, String label) { + def jenkins = Jenkins.instance + if (jenkins.getNode(name) != null) { + jenkins.removeNode(jenkins.getNode(name)) + } + def launcher = new JNLPLauncher(true) + def agent = new DumbSlave(name, "/home/jenkins/agent", launcher) + agent.setLabelString(label) + agent.setRetentionStrategy(RetentionStrategy.INSTANCE) + jenkins.addNode(agent) +} + +createAgent("agent-linux-amd64", "linux-amd64") +createAgent("agent-linux-arm64", "linux-arm64") + +Jenkins.instance.nodes.each { node -> + if (node.name.startsWith("agent-linux")) { + def computer = node.toComputer() + if (computer instanceof hudson.slaves.SlaveComputer) { + println "SECRET_${node.name.replace("-", "_").toUpperCase()}=${computer.getJnlpMac()}" + } + } +} +') + +echo "$SECRETS" + +# Parse secrets +export AGENT_AMD64_SECRET=$(echo "$SECRETS" | grep "SECRET_AGENT_LINUX_AMD64" | cut -d= -f2) +export AGENT_ARM64_SECRET=$(echo "$SECRETS" | grep "SECRET_AGENT_LINUX_ARM64" | cut -d= -f2) + +if [ -z "$AGENT_AMD64_SECRET" ] || [ -z "$AGENT_ARM64_SECRET" ]; then + echo "ERROR: Failed to retrieve agent secrets" + return 1 +fi + +echo "" +echo "==> Pulling agent images (both platforms)..." +docker pull --platform linux/amd64 jenkins/inbound-agent:latest +docker pull --platform linux/arm64 jenkins/inbound-agent:latest + +echo "" +echo "==> Starting agent containers..." +docker compose -f "$SCRIPT_DIR/docker-compose-agents.yml" up -d + +echo "" +echo "==> Waiting for agents to come online..." +for i in $(seq 1 30); do + ONLINE_COUNT=$(run_groovy ' + def count = Jenkins.instance.nodes.count { node -> + node.name.startsWith("agent-linux") && node.toComputer()?.isOnline() + } + print count + ') + if [ "$ONLINE_COUNT" = "2" ]; then + echo "==> Both agents are online!" + break + fi + echo " ...waiting for agents ($ONLINE_COUNT/2 online, attempt $i/30)" + sleep 5 +done + +echo "" +echo "==> Configuring JFrog CLI tool..." +run_groovy ' +import jenkins.model.* +import hudson.tools.* +import io.jenkins.plugins.jfrog.* + +def jenkins = Jenkins.instance +def desc = jenkins.getDescriptorByType(JfrogInstallation.DescriptorImpl.class) +def existing = desc.getInstallations()?.toList() ?: [] + +if (!existing.any { it.name == "releases-jfrog-cli" }) { + def installer = new ReleasesInstaller() + def props = new DescribableList(Saveable.NOOP) + props.add(new InstallSourceProperty([installer])) + def tool = new JfrogInstallation("releases-jfrog-cli", "", props) + existing.add(tool) + desc.setInstallations(existing.toArray(new JfrogInstallation[0])) + println "Tool releases-jfrog-cli configured" +} else { + println "Tool releases-jfrog-cli already exists" +} +' + +echo "" +echo "==> Creating cross-platform test pipeline..." +COOKIES=$(mktemp) +CRUMB=$(get_crumb "$COOKIES") + +# Delete if exists +curl -sf -X POST "${JENKINS_URL}/job/test-cross-platform/doDelete" \ + -u "${JENKINS_USER}:${JENKINS_PASS}" -b "$COOKIES" -H "Jenkins-Crumb: ${CRUMB}" 2>/dev/null || true + +CRUMB=$(get_crumb "$COOKIES") +curl -sf -X POST "${JENKINS_URL}/createItem?name=test-cross-platform" \ + -u "${JENKINS_USER}:${JENKINS_PASS}" -b "$COOKIES" -H "Jenkins-Crumb: ${CRUMB}" \ + -H "Content-Type: application/xml" \ + --data-binary @- << 'XMLEOF' + + + Cross-platform JFrog CLI installation test. +Tests: per-pipeline cache, agent OS detection, multi-arch binary download. + + + true + + +XMLEOF + +rm -f "$COOKIES" + +echo "" +echo "==> Setup complete!" +echo "" +echo "Agents:" +run_groovy ' +Jenkins.instance.nodes.each { node -> + def c = node.toComputer() + println " ${node.name}: online=${c?.isOnline()}, label=${node.labelString}" +} +' +echo "" +echo "To run the cross-platform test:" +echo " curl -X POST ${JENKINS_URL}/job/test-cross-platform/build -u ${JENKINS_USER}:${JENKINS_PASS}" +echo "" +echo "To view results:" +echo " curl ${JENKINS_URL}/job/test-cross-platform/lastBuild/consoleText -u ${JENKINS_USER}:${JENKINS_PASS}" +echo "" +echo "To enable FINE logging for debug:" +echo " Manage Jenkins → System Log → Add 'io.jenkins.plugins.jfrog.BinaryInstaller' at FINE" +echo "" +echo "To tear down agents:" +echo " docker compose -f e2e/local/docker-compose-agents.yml down" diff --git a/src/main/java/io/jenkins/plugins/jfrog/ArtifactoryInstaller.java b/src/main/java/io/jenkins/plugins/jfrog/ArtifactoryInstaller.java index 2ee16334..68c155d8 100644 --- a/src/main/java/io/jenkins/plugins/jfrog/ArtifactoryInstaller.java +++ b/src/main/java/io/jenkins/plugins/jfrog/ArtifactoryInstaller.java @@ -56,7 +56,7 @@ public FilePath performInstallation(ToolInstallation tool, Node node, TaskListen throw new IOException("Server id '" + getServerId() + "' doesn't exists."); } String binaryName = Utils.getJfrogCliBinaryName(!node.createLauncher(log).isUnix()); - return performJfrogCliInstallation(getToolLocation(tool, node), log, getVersion(), server, getRepository(), binaryName); + return performJfrogCliInstallation(getToolLocation(tool, node), log, getVersion(), server, getRepository(), binaryName, node.getNodeName()); } /** diff --git a/src/main/java/io/jenkins/plugins/jfrog/BinaryInstaller.java b/src/main/java/io/jenkins/plugins/jfrog/BinaryInstaller.java index e5e7bd93..7459c3b9 100644 --- a/src/main/java/io/jenkins/plugins/jfrog/BinaryInstaller.java +++ b/src/main/java/io/jenkins/plugins/jfrog/BinaryInstaller.java @@ -61,7 +61,28 @@ public abstract class BinaryInstaller extends ToolInstaller { * next access).

*/ private static final ConcurrentHashMap NODE_INSTALLATION_LOCKS = new ConcurrentHashMap<>(); - + + /** + * Per-pipeline installation verification cache. + * Key: tool path + agent OS + binary name + * Value: pipeline run identifier (derived from the build's log storage) + * + *

Once a tool is verified in a pipeline run, all subsequent stages in the same run + * skip the version check entirely — avoiding repeated HTTP HEAD requests to Artifactory. + * A new pipeline run produces a different run ID, so it gets its own verification. + * The agent OS is included in the key to distinguish same-path installations on + * agents with different architectures (e.g., linux-amd64 vs linux-arm64).

+ */ + private static final ConcurrentHashMap VERIFIED_IN_RUN = new ConcurrentHashMap<>(); + private static final int MAX_CACHE_SIZE = 100; + + /** + * Cache of agent OS details to avoid repeated remote calls. + * Key: tool location remote path + * Value: OS details string (e.g., "linux-amd64", "mac-arm64") + */ + private static final ConcurrentHashMap AGENT_OS_CACHE = new ConcurrentHashMap<>(); + protected BinaryInstaller(String label) { super(label); } @@ -115,14 +136,48 @@ public String getId() { * @throws InterruptedException If installation is interrupted */ public static FilePath performJfrogCliInstallation(FilePath toolLocation, TaskListener log, String version, - JFrogPlatformInstance instance, String repository, String binaryName) + JFrogPlatformInstance instance, String repository, + String binaryName, String nodeName) throws IOException, InterruptedException { FilePath cliPath = toolLocation.child(binaryName); + // Cache agent OS per node+path to handle agents with identical tool paths but different architectures. + String toolPath = toolLocation.getRemote(); + String osCacheKey = nodeName + ":" + toolPath; + String agentOs = AGENT_OS_CACHE.get(osCacheKey); + if (agentOs == null) { + try { + agentOs = toolLocation.act(new MasterToSlaveFileCallable() { + @Override + public String invoke(File f, VirtualChannel channel) throws IOException { + return OsUtils.getOsDetails(); + } + }); + } catch (Exception e) { + LOGGER.warning("Failed to get agent OS details: " + e.getMessage()); + agentOs = "unknown"; + } + if (AGENT_OS_CACHE.size() >= MAX_CACHE_SIZE) { + AGENT_OS_CACHE.clear(); + } + AGENT_OS_CACHE.put(osCacheKey, agentOs); + } + + String cacheKey = nodeName + ":" + toolPath + "/" + agentOs + "/" + binaryName; + LOGGER.fine("Agent OS detected: " + agentOs + " for node: " + nodeName + " tool: " + toolPath); + + // Per-pipeline cache: skip re-verification if already checked in this pipeline run. + String currentRunId = getCurrentRunId(log); + if (currentRunId != null && currentRunId.equals(VERIFIED_IN_RUN.get(cacheKey))) { + LOGGER.fine("CLI already verified in this pipeline run, skipping check for: " + cacheKey); + return toolLocation; + } + // Fast path: binary exists and is already the correct version — skip lock entirely. - if (isValidCliInstallation(cliPath, log) && isCorrectVersion(toolLocation, instance, repository, version, binaryName, log)) { + if (isValidCliInstallation(cliPath, log) && isCorrectVersion(toolLocation, instance, repository, version, binaryName, agentOs, log)) { log.getLogger().println("[BinaryInstaller] CLI already installed and up-to-date, skipping download"); + markVerified(cacheKey, currentRunId); return toolLocation; } @@ -147,8 +202,9 @@ public static FilePath performJfrogCliInstallation(FilePath toolLocation, TaskLi try { // Re-check inside the lock — a concurrent stage may have just finished. boolean validCliExists = isValidCliInstallation(cliPath, log); - if (validCliExists && isCorrectVersion(toolLocation, instance, repository, version, binaryName, log)) { + if (validCliExists && isCorrectVersion(toolLocation, instance, repository, version, binaryName, agentOs, log)) { log.getLogger().println("[BinaryInstaller] CLI was installed by a concurrent stage, skipping download"); + markVerified(cacheKey, currentRunId); return toolLocation; } @@ -162,6 +218,7 @@ public static FilePath performJfrogCliInstallation(FilePath toolLocation, TaskLi JenkinsProxyConfiguration proxyConfiguration = new JenkinsProxyConfiguration(); toolLocation.act(new JFrogCliDownloader(proxyConfiguration, version, instance, log, repository, binaryName)); log.getLogger().println("[BinaryInstaller] CLI installation completed successfully"); + markVerified(cacheKey, currentRunId); } catch (Exception e) { // Download failed. If an older binary is still present, keep the pipeline running. // The upgrade will be retried on the next run. @@ -204,13 +261,62 @@ static int getInstallTimeoutMinutes() { return DEFAULT_INSTALL_TIMEOUT_MINUTES; } + /** + * Extracts a unique pipeline run identifier from the TaskListener. + *

+ * In Pipeline builds, the TaskListener wraps a FileLogStorage whose log file path + * contains the job name and build number (e.g., "jobs/my-job/builds/42/log"). + * This path is guaranteed unique per pipeline run. + *

+ * Falls back to null for non-Pipeline builds (Freestyle, etc.) where the listener + * structure is different — the caller treats null as "don't cache". + *

+ * Fragile: This relies on internal field names in workflow-api and workflow-support. + * Tested with Jenkins {@literal >=} 2.462.3 and workflow-cps. If a future Jenkins update + * renames these fields, the catch block returns null and caching degrades gracefully + * (repeated version checks, no failure). Re-verify after major Jenkins core upgrades. + */ + private static String getCurrentRunId(TaskListener log) { + try { + // Reflection chain (workflow-api / workflow-support internals): + // CloseableTaskListener → mainDelegate (BufferedBuildListener) → out (IndexOutputStream) → this$0 (FileLogStorage) → log (File) + java.lang.reflect.Field mainField = log.getClass().getDeclaredField("mainDelegate"); + mainField.setAccessible(true); + Object buildListener = mainField.get(log); + + java.lang.reflect.Field outField = buildListener.getClass().getDeclaredField("out"); + outField.setAccessible(true); + Object indexOut = outField.get(buildListener); + + java.lang.reflect.Field storageField = indexOut.getClass().getDeclaredField("this$0"); + storageField.setAccessible(true); + Object fileLogStorage = storageField.get(indexOut); + + java.lang.reflect.Field logField = fileLogStorage.getClass().getDeclaredField("log"); + logField.setAccessible(true); + File logFile = (File) logField.get(fileLogStorage); + + return logFile.getPath(); + } catch (Exception e) { + // Non-Pipeline build or different Jenkins version — fall back gracefully + LOGGER.fine("Could not determine pipeline run ID: " + e.getMessage()); + return null; + } + } + + private static void markVerified(String cacheKey, String currentRunId) { + if (currentRunId == null) { + return; + } + if (VERIFIED_IN_RUN.size() >= MAX_CACHE_SIZE) { + VERIFIED_IN_RUN.clear(); + } + VERIFIED_IN_RUN.put(cacheKey, currentRunId); + } + /** * Creates a unique lock key for the installation location. * Version is excluded so all operations targeting the same binary path are serialized. - * - * @param toolLocation Installation directory - * @param binaryName Binary file name - * @return Unique lock key string */ private static String createLockKey(FilePath toolLocation, String binaryName) { try { @@ -267,17 +373,14 @@ public long[] invoke(File file, VirtualChannel channel) { * @param log Task listener for logging * @return true if CLI is the correct version, false otherwise */ - private static boolean isCorrectVersion(FilePath toolLocation, JFrogPlatformInstance instance, - String repository, String version, String binaryName, TaskListener log) { + private static boolean isCorrectVersion(FilePath toolLocation, JFrogPlatformInstance instance, + String repository, String version, String binaryName, + String agentOsDetails, TaskListener log) { try { - // Use the same logic as shouldDownloadTool() from JFrogCliDownloader - // but do it here to avoid unnecessary JFrogCliDownloader.invoke() calls - JenkinsProxyConfiguration proxyConfiguration = new JenkinsProxyConfiguration(); - // Use binaryName to construct the correct URL suffix (handles Windows jf.exe vs Unix jf) - String cliUrlSuffix = String.format("/%s/v2-jf/%s/jfrog-cli-%s/%s", repository, - StringUtils.defaultIfBlank(version, "[RELEASE]"), - OsUtils.getOsDetails(), binaryName); + String cliUrlSuffix = String.format("/%s/v2-jf/%s/jfrog-cli-%s/%s", repository, + StringUtils.defaultIfBlank(version, "[RELEASE]"), + agentOsDetails, binaryName); JenkinsBuildInfoLog buildInfoLog = new JenkinsBuildInfoLog(log); String artifactoryUrl = instance.inferArtifactoryUrl(); @@ -295,6 +398,8 @@ private static boolean isCorrectVersion(FilePath toolLocation, JFrogPlatformInst String expectedSha256 = getArtifactSha256(manager, cliUrlSuffix); if (expectedSha256.isEmpty()) { log.getLogger().println("[BinaryInstaller] WARNING: No SHA256 available from server — cannot verify version, assuming up-to-date (upgrade may be delayed)"); + // Clean up stale 0-byte sha256 file left by older plugin versions + cleanupStaleSha256File(toolLocation, log); return true; } @@ -319,6 +424,31 @@ public Boolean invoke(File f, VirtualChannel channel) throws IOException, Interr } } + /** + * Removes a stale 0-byte sha256 file left behind by older plugin versions. + * The file has no effect on current behaviour but can confuse operators inspecting + * the tool directory. + */ + private static void cleanupStaleSha256File(FilePath toolLocation, TaskListener log) { + try { + toolLocation.act(new MasterToSlaveFileCallable() { + @Override + public Void invoke(File f, VirtualChannel channel) throws IOException { + File sha256File = new File(f, "sha256"); + if (sha256File.exists() && sha256File.length() == 0) { + if (sha256File.delete()) { + log.getLogger().println("[BinaryInstaller] Cleaned up stale 0-byte sha256 file"); + } + } + return null; + } + }); + } catch (Exception e) { + // Best-effort cleanup — not worth failing the build + LOGGER.warning("Failed to clean up stale sha256 file: " + e.getMessage()); + } + } + /** * Get SHA256 hash from Artifactory headers (same logic as in JFrogCliDownloader) */ From 04c415f8d38e64176f9c262f9e6a2b47dad5382f Mon Sep 17 00:00:00 2001 From: Bhanu Reddy Date: Fri, 10 Apr 2026 17:55:00 +0530 Subject: [PATCH 2/3] Fixed spotbugs --- .../plugins/jfrog/BinaryInstaller.java | 29 +++++++++++-------- 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/src/main/java/io/jenkins/plugins/jfrog/BinaryInstaller.java b/src/main/java/io/jenkins/plugins/jfrog/BinaryInstaller.java index 7459c3b9..549897f5 100644 --- a/src/main/java/io/jenkins/plugins/jfrog/BinaryInstaller.java +++ b/src/main/java/io/jenkins/plugins/jfrog/BinaryInstaller.java @@ -145,10 +145,10 @@ public static FilePath performJfrogCliInstallation(FilePath toolLocation, TaskLi // Cache agent OS per node+path to handle agents with identical tool paths but different architectures. String toolPath = toolLocation.getRemote(); String osCacheKey = nodeName + ":" + toolPath; - String agentOs = AGENT_OS_CACHE.get(osCacheKey); - if (agentOs == null) { + evictIfFull(AGENT_OS_CACHE); + String agentOs = AGENT_OS_CACHE.computeIfAbsent(osCacheKey, k -> { try { - agentOs = toolLocation.act(new MasterToSlaveFileCallable() { + return toolLocation.act(new MasterToSlaveFileCallable() { @Override public String invoke(File f, VirtualChannel channel) throws IOException { return OsUtils.getOsDetails(); @@ -156,13 +156,9 @@ public String invoke(File f, VirtualChannel channel) throws IOException { }); } catch (Exception e) { LOGGER.warning("Failed to get agent OS details: " + e.getMessage()); - agentOs = "unknown"; + return "unknown"; } - if (AGENT_OS_CACHE.size() >= MAX_CACHE_SIZE) { - AGENT_OS_CACHE.clear(); - } - AGENT_OS_CACHE.put(osCacheKey, agentOs); - } + }); String cacheKey = nodeName + ":" + toolPath + "/" + agentOs + "/" + binaryName; LOGGER.fine("Agent OS detected: " + agentOs + " for node: " + nodeName + " tool: " + toolPath); @@ -308,12 +304,21 @@ private static void markVerified(String cacheKey, String currentRunId) { if (currentRunId == null) { return; } - if (VERIFIED_IN_RUN.size() >= MAX_CACHE_SIZE) { - VERIFIED_IN_RUN.clear(); - } + evictIfFull(VERIFIED_IN_RUN); VERIFIED_IN_RUN.put(cacheKey, currentRunId); } + /** + * Clears the cache if it has reached {@link #MAX_CACHE_SIZE}. + * This is a simple eviction strategy — the slight race between size() and clear() + * is harmless (worst case: one extra entry before eviction, or an extra cache miss). + */ + private static void evictIfFull(ConcurrentHashMap cache) { + if (cache.size() >= MAX_CACHE_SIZE) { + cache.clear(); + } + } + /** * Creates a unique lock key for the installation location. * Version is excluded so all operations targeting the same binary path are serialized. From a07a522a0df73614021b04fbd24d7bb2ab837de9 Mon Sep 17 00:00:00 2001 From: Bhanu Reddy Date: Mon, 13 Apr 2026 16:09:34 +0530 Subject: [PATCH 3/3] Improved max cache size and empty nodename --- .../plugins/jfrog/BinaryInstaller.java | 63 +++++++++++-------- 1 file changed, 36 insertions(+), 27 deletions(-) diff --git a/src/main/java/io/jenkins/plugins/jfrog/BinaryInstaller.java b/src/main/java/io/jenkins/plugins/jfrog/BinaryInstaller.java index 549897f5..e1b4b72d 100644 --- a/src/main/java/io/jenkins/plugins/jfrog/BinaryInstaller.java +++ b/src/main/java/io/jenkins/plugins/jfrog/BinaryInstaller.java @@ -20,6 +20,9 @@ import java.io.IOException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.ReentrantLock; @@ -63,8 +66,8 @@ public abstract class BinaryInstaller extends ToolInstaller { private static final ConcurrentHashMap NODE_INSTALLATION_LOCKS = new ConcurrentHashMap<>(); /** - * Per-pipeline installation verification cache. - * Key: tool path + agent OS + binary name + * Per-pipeline installation verification cache (LRU, max 1000 entries). + * Key: nodeName + tool path + agent OS + binary name * Value: pipeline run identifier (derived from the build's log storage) * *

Once a tool is verified in a pipeline run, all subsequent stages in the same run @@ -73,15 +76,29 @@ public abstract class BinaryInstaller extends ToolInstaller { * The agent OS is included in the key to distinguish same-path installations on * agents with different architectures (e.g., linux-amd64 vs linux-arm64).

*/ - private static final ConcurrentHashMap VERIFIED_IN_RUN = new ConcurrentHashMap<>(); - private static final int MAX_CACHE_SIZE = 100; + private static final int MAX_CACHE_SIZE = 1000; + private static final Map VERIFIED_IN_RUN = Collections.synchronizedMap( + new LinkedHashMap(MAX_CACHE_SIZE, 0.75f, true) { + @Override + protected boolean removeEldestEntry(Map.Entry eldest) { + return size() > MAX_CACHE_SIZE; + } + }); /** - * Cache of agent OS details to avoid repeated remote calls. - * Key: tool location remote path + * Cache of agent OS details to avoid repeated remote calls (LRU, max 100 entries). + * Key: nodeName + tool location remote path * Value: OS details string (e.g., "linux-amd64", "mac-arm64") */ - private static final ConcurrentHashMap AGENT_OS_CACHE = new ConcurrentHashMap<>(); + private static final Map AGENT_OS_CACHE = Collections.synchronizedMap( + new LinkedHashMap(MAX_CACHE_SIZE, 0.75f, true) { + @Override + protected boolean removeEldestEntry(Map.Entry eldest) { + return size() > MAX_CACHE_SIZE; + } + }); + + private static final String BUILT_IN_NODE = "built-in"; protected BinaryInstaller(String label) { super(label); @@ -142,13 +159,16 @@ public static FilePath performJfrogCliInstallation(FilePath toolLocation, TaskLi FilePath cliPath = toolLocation.child(binaryName); + // Node.getNodeName() returns "" for the built-in (master) node. + String node = StringUtils.defaultIfBlank(nodeName, BUILT_IN_NODE); + // Cache agent OS per node+path to handle agents with identical tool paths but different architectures. String toolPath = toolLocation.getRemote(); - String osCacheKey = nodeName + ":" + toolPath; - evictIfFull(AGENT_OS_CACHE); - String agentOs = AGENT_OS_CACHE.computeIfAbsent(osCacheKey, k -> { + String osCacheKey = node + ":" + toolPath; + String agentOs = AGENT_OS_CACHE.get(osCacheKey); + if (agentOs == null) { try { - return toolLocation.act(new MasterToSlaveFileCallable() { + agentOs = toolLocation.act(new MasterToSlaveFileCallable() { @Override public String invoke(File f, VirtualChannel channel) throws IOException { return OsUtils.getOsDetails(); @@ -156,12 +176,13 @@ public String invoke(File f, VirtualChannel channel) throws IOException { }); } catch (Exception e) { LOGGER.warning("Failed to get agent OS details: " + e.getMessage()); - return "unknown"; + agentOs = "unknown"; } - }); + AGENT_OS_CACHE.put(osCacheKey, agentOs); + } - String cacheKey = nodeName + ":" + toolPath + "/" + agentOs + "/" + binaryName; - LOGGER.fine("Agent OS detected: " + agentOs + " for node: " + nodeName + " tool: " + toolPath); + String cacheKey = node + ":" + toolPath + "/" + agentOs + "/" + binaryName; + LOGGER.fine("Agent OS detected: " + agentOs + " for node: " + node + " tool: " + toolPath); // Per-pipeline cache: skip re-verification if already checked in this pipeline run. String currentRunId = getCurrentRunId(log); @@ -304,21 +325,9 @@ private static void markVerified(String cacheKey, String currentRunId) { if (currentRunId == null) { return; } - evictIfFull(VERIFIED_IN_RUN); VERIFIED_IN_RUN.put(cacheKey, currentRunId); } - /** - * Clears the cache if it has reached {@link #MAX_CACHE_SIZE}. - * This is a simple eviction strategy — the slight race between size() and clear() - * is harmless (worst case: one extra entry before eviction, or an extra cache miss). - */ - private static void evictIfFull(ConcurrentHashMap cache) { - if (cache.size() >= MAX_CACHE_SIZE) { - cache.clear(); - } - } - /** * Creates a unique lock key for the installation location. * Version is excluded so all operations targeting the same binary path are serialized.