Skip to content

Commit ce174dc

Browse files
olivergondzaolivergondza
committed
Merge branch 'master' into node-identification-by-identity
2 parents 297c3c2 + a6334fe commit ce174dc

File tree

8 files changed

+92
-119
lines changed

8 files changed

+92
-119
lines changed

‎plugin/pom.xml‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
</parent>
88

99
<artifactId>openstack-cloud</artifactId>
10-
<version>2.59-SNAPSHOT</version>
10+
<version>2.60-SNAPSHOT</version>
1111
<packaging>hpi</packaging>
1212
<name>OpenStack Cloud Plugin</name>
1313
<description>Allows Jenkins to build on OpenStack nodes</description>
@@ -108,6 +108,11 @@
108108
<artifactId>instance-identity</artifactId>
109109
<version>2.2</version>
110110
</dependency>
111+
<dependency>
112+
<groupId>io.jenkins.plugins</groupId>
113+
<artifactId>caffeine-api</artifactId>
114+
<version>2.9.1-23.v51c4e2c879c8</version>
115+
</dependency>
111116

112117
<!-- Hardcoding upper-bound versions of dependencies -->
113118
<dependency>

‎plugin/src/main/java/jenkins/plugins/openstack/compute/JCloudsBuildWrapper.java‎

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,10 @@
2020
import java.util.ArrayList;
2121
import java.util.List;
2222
import java.util.Map;
23+
import java.util.Objects;
2324
import java.util.concurrent.Callable;
2425
import java.util.concurrent.atomic.AtomicInteger;
26+
import java.util.stream.Collectors;
2527

2628
import javax.annotation.Nonnull;
2729

@@ -34,8 +36,6 @@
3436
import org.kohsuke.stapler.DataBoundConstructor;
3537
import org.openstack4j.model.compute.Server;
3638

37-
import com.google.common.base.Function;
38-
import com.google.common.collect.Iterables;
3939
import com.google.common.util.concurrent.MoreExecutors;
4040

4141
public class JCloudsBuildWrapper extends BuildWrapper {
@@ -60,17 +60,13 @@ public Environment setUp(final AbstractBuild build, Launcher launcher, final Bui
6060
final ServerScope.Build scope = new ServerScope.Build(build);
6161

6262
// eagerly lookup node supplier so that errors occur before we attempt to provision things
63-
Iterable<NodePlan> nodePlans = Iterables.transform(instancesToRun, new Function<InstancesToRun, NodePlan>() {
64-
65-
@SuppressWarnings("unchecked")
66-
public NodePlan apply(InstancesToRun instance) {
67-
JCloudsCloud cloud = JCloudsCloud.getByName(instance.cloudName);
68-
String templateName = Util.replaceMacro(instance.getActualTemplateName(), build.getBuildVariableResolver());
69-
JCloudsSlaveTemplate template = cloud.getTemplate(templateName);
70-
if (template == null) throw new IllegalArgumentException("No such template " + templateName);
71-
return new NodePlan(cloud, template, instance.count, scope);
72-
}
73-
});
63+
Iterable<NodePlan> nodePlans = instancesToRun.stream().map(instance -> {
64+
JCloudsCloud cloud = JCloudsCloud.getByName(Objects.requireNonNull(instance.cloudName));
65+
String templateName = Util.replaceMacro(instance.getActualTemplateName(), build.getBuildVariableResolver());
66+
JCloudsSlaveTemplate template = cloud.getTemplate(templateName);
67+
if (template == null) throw new IllegalArgumentException("No such template " + templateName);
68+
return new NodePlan(cloud, template, instance.count, scope);
69+
}).collect(Collectors.toList());
7470

7571
ListeningExecutorService executor = MoreExecutors.listeningDecorator(Computer.threadPoolForRemoting);
7672
final ImmutableList.Builder<RunningNode> cloudTemplateNodeBuilder = ImmutableList.builder();
@@ -89,7 +85,7 @@ public NodePlan apply(InstancesToRun instance) {
8985

9086
ListenableFuture<Server> provisionTemplate = executor.submit(nodePlan.getNodeSupplier());
9187

92-
Futures.addCallback(provisionTemplate, new FutureCallback<Server>() {
88+
FutureCallback<Server> callback = new FutureCallback<Server>() {
9389
public void onSuccess(Server result) {
9490
if (result != null) {
9591
synchronized (cloudTemplateNodeBuilder) {
@@ -108,7 +104,8 @@ public void onFailure(@Nonnull Throwable t) {
108104
index, nodePlan.getCloud(), nodePlan.getTemplate(), Functions.printThrowable(t)
109105
);
110106
}
111-
});
107+
};
108+
Futures.addCallback(provisionTemplate, callback, MoreExecutors.directExecutor());
112109

113110
plannedInstancesBuilder.add(provisionTemplate);
114111
}

‎plugin/src/main/java/jenkins/plugins/openstack/compute/JCloudsSlave.java‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
package jenkins.plugins.openstack.compute;
22

3+
import com.github.benmanes.caffeine.cache.Cache;
4+
import com.github.benmanes.caffeine.cache.Caffeine;
5+
import com.google.common.collect.ImmutableList;
36
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
47
import hudson.EnvVars;
58
import hudson.Extension;
@@ -31,10 +34,6 @@
3134
import org.openstack4j.model.compute.Flavor;
3235
import org.openstack4j.model.compute.Server;
3336

34-
import com.google.common.cache.Cache;
35-
import com.google.common.cache.CacheBuilder;
36-
import com.google.common.collect.ImmutableList;
37-
3837
import javax.annotation.CheckForNull;
3938
import javax.annotation.Nonnull;
4039
import java.io.IOException;
@@ -44,9 +43,8 @@
4443
import java.util.Map;
4544
import java.util.NoSuchElementException;
4645
import java.util.Objects;
47-
import java.util.concurrent.Callable;
48-
import java.util.concurrent.ExecutionException;
4946
import java.util.concurrent.TimeUnit;
47+
import java.util.function.Function;
5048
import java.util.logging.Level;
5149
import java.util.logging.Logger;
5250

@@ -188,7 +186,7 @@ protected Object readResolve() {
188186

189187
/** Creates a cache where data will be kept for a short duration before being discarded. */
190188
private static @Nonnull <K, V> Cache<K, V> makeCache() {
191-
return CacheBuilder.newBuilder().expireAfterWrite(5, TimeUnit.MINUTES).build();
189+
return Caffeine.newBuilder().expireAfterWrite(5, TimeUnit.MINUTES).build();
192190
}
193191

194192
/**
@@ -220,7 +218,7 @@ protected Object readResolve() {
220218
*/
221219
@Restricted(DoNotUse.class) // Jelly
222220
public @Nonnull Map<String, String> getLiveOpenstackServerDetails() {
223-
return getCachableData("liveData", this::readLiveOpenstackServerDetails);
221+
return getCachableData("liveData", (String unused) -> readLiveOpenstackServerDetails());
224222
}
225223

226224
private @Nonnull Map<String, String> readLiveOpenstackServerDetails() {
@@ -304,15 +302,15 @@ private static void putIfNotNullOrEmpty(
304302
}
305303

306304
/** Gets something from the cache, loading it into the cache if necessary. */
307-
private @Nonnull Map<String, String> getCachableData(@Nonnull final String key, @Nonnull final Callable<Map<String, String>> dataloader) {
305+
private @Nonnull Map<String, String> getCachableData(@Nonnull final String key, @Nonnull final Function<String, Map<String, String>> dataloader) {
308306
try {
309-
return cache.get(key, dataloader);
310-
} catch (ExecutionException e) {
307+
return Objects.requireNonNull(cache.get(key, dataloader));
308+
} catch (RuntimeException e) { // Propagated from cacheMissFunction
311309
final Throwable cause = e.getCause();
312310
if (cause instanceof RuntimeException) {
313311
throw (RuntimeException) cause;
314312
}
315-
throw new RuntimeException(e);
313+
throw e;
316314
}
317315
}
318316

‎plugin/src/main/java/jenkins/plugins/openstack/compute/internal/Openstack.java‎

Lines changed: 43 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -23,64 +23,32 @@
2323
*/
2424
package jenkins.plugins.openstack.compute.internal;
2525

26-
import java.io.File;
27-
import java.io.IOException;
28-
import java.nio.charset.Charset;
29-
import java.security.interfaces.RSAPublicKey;
30-
import java.util.ArrayList;
31-
import java.util.Collection;
32-
import java.util.Collections;
33-
import java.util.Comparator;
34-
import java.util.Date;
35-
import java.util.HashMap;
36-
import java.util.HashSet;
37-
import java.util.List;
38-
import java.util.Map;
39-
import java.util.NoSuchElementException;
40-
import java.util.Objects;
41-
import java.util.Optional;
42-
import java.util.TreeMap;
43-
import java.util.TreeSet;
44-
import java.util.concurrent.Callable;
45-
import java.util.concurrent.ExecutionException;
46-
import java.util.concurrent.TimeUnit;
47-
import java.util.logging.Level;
48-
import java.util.logging.Logger;
49-
import java.util.stream.Collectors;
50-
51-
import javax.annotation.CheckForNull;
52-
import javax.annotation.Nonnegative;
53-
import javax.annotation.Nonnull;
54-
import javax.annotation.concurrent.ThreadSafe;
55-
5626
import com.cloudbees.plugins.credentials.common.PasswordCredentials;
27+
import com.github.benmanes.caffeine.cache.Cache;
28+
import com.github.benmanes.caffeine.cache.Caffeine;
5729
import com.google.common.annotations.VisibleForTesting;
58-
import com.google.common.base.Charsets;
59-
import com.google.common.base.MoreObjects;
60-
import com.google.common.cache.Cache;
61-
import com.google.common.cache.CacheBuilder;
62-
import com.google.common.util.concurrent.UncheckedExecutionException;
6330
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
6431
import hudson.Extension;
6532
import hudson.ExtensionList;
6633
import hudson.ExtensionPoint;
6734
import hudson.Util;
68-
import hudson.remoting.Which;
6935
import hudson.util.FormValidation;
36+
import jenkins.model.Jenkins;
7037
import jenkins.plugins.openstack.compute.auth.OpenstackCredential;
38+
import org.apache.commons.codec.Charsets;
7139
import org.apache.commons.codec.binary.Base64;
7240
import org.apache.commons.codec.digest.DigestUtils;
7341
import org.jenkinsci.main.modules.instance_identity.InstanceIdentity;
7442
import org.kohsuke.accmod.Restricted;
7543
import org.kohsuke.accmod.restrictions.NoExternalUse;
7644
import org.openstack4j.api.Builders;
77-
import org.openstack4j.api.networking.NetFloatingIPService;
78-
import org.openstack4j.api.networking.NetworkingService;
79-
import org.openstack4j.core.transport.Config;
8045
import org.openstack4j.api.OSClient;
8146
import org.openstack4j.api.client.IOSClientBuilder;
8247
import org.openstack4j.api.compute.ServerService;
8348
import org.openstack4j.api.exceptions.ResponseException;
49+
import org.openstack4j.api.networking.NetFloatingIPService;
50+
import org.openstack4j.api.networking.NetworkingService;
51+
import org.openstack4j.core.transport.Config;
8452
import org.openstack4j.model.common.ActionResponse;
8553
import org.openstack4j.model.compute.Address;
8654
import org.openstack4j.model.compute.Fault;
@@ -94,16 +62,38 @@
9462
import org.openstack4j.model.image.v2.Image;
9563
import org.openstack4j.model.network.NetFloatingIP;
9664
import org.openstack4j.model.network.Network;
97-
import org.openstack4j.model.network.Router;
9865
import org.openstack4j.model.network.Port;
66+
import org.openstack4j.model.network.Router;
9967
import org.openstack4j.model.network.ext.NetworkIPAvailability;
10068
import org.openstack4j.model.network.options.PortListOptions;
10169
import org.openstack4j.model.storage.block.Volume;
10270
import org.openstack4j.model.storage.block.Volume.Status;
10371
import org.openstack4j.model.storage.block.VolumeSnapshot;
10472
import org.openstack4j.openstack.OSFactory;
10573

106-
import jenkins.model.Jenkins;
74+
import javax.annotation.CheckForNull;
75+
import javax.annotation.Nonnegative;
76+
import javax.annotation.Nonnull;
77+
import javax.annotation.concurrent.ThreadSafe;
78+
import java.util.ArrayList;
79+
import java.util.Collection;
80+
import java.util.Collections;
81+
import java.util.Comparator;
82+
import java.util.Date;
83+
import java.util.HashMap;
84+
import java.util.HashSet;
85+
import java.util.List;
86+
import java.util.Map;
87+
import java.util.NoSuchElementException;
88+
import java.util.Objects;
89+
import java.util.Optional;
90+
import java.util.TreeMap;
91+
import java.util.TreeSet;
92+
import java.util.concurrent.TimeUnit;
93+
import java.util.function.Function;
94+
import java.util.logging.Level;
95+
import java.util.logging.Logger;
96+
import java.util.stream.Collectors;
10797

10898
/**
10999
* Encapsulate {@link OSClient}.
@@ -858,7 +848,7 @@ private static void debug(@Nonnull String msg, @Nonnull String... args) {
858848

859849
@Restricted(NoExternalUse.class) // Extension point just for testing
860850
public static abstract class FactoryEP implements ExtensionPoint {
861-
private final transient @Nonnull Cache<String, Openstack> cache = CacheBuilder.newBuilder()
851+
private final transient @Nonnull Cache<String, Openstack> cache = Caffeine.newBuilder()
862852
// There is no clear reasoning behind particular expiration policy except that individual instances can
863853
// have different token expiration time, which is something guava does not support. This expiration needs
864854
// to be implemented separately.
@@ -885,12 +875,18 @@ public static abstract class FactoryEP implements ExtensionPoint {
885875
+ (auth instanceof PasswordCredentials ? ((PasswordCredentials) auth).getPassword().getEncryptedValue() + '\n' : "")
886876
+ region);
887877
final FactoryEP ep = ExtensionList.lookup(FactoryEP.class).get(0);
888-
final Callable<Openstack> cacheMissFunction = () -> ep.getOpenstack(endPointUrl, ignoreSsl, auth, region);
878+
final Function<String, Openstack> cacheMissFunction = (String unused) -> {
879+
try {
880+
return ep.getOpenstack(endPointUrl, ignoreSsl, auth, region);
881+
} catch (FormValidation ex) {
882+
throw new RuntimeException(ex);
883+
}
884+
};
889885

890-
// Get an instance, creating a new one if necessary.
891886
try {
892-
return ep.cache.get(fingerprint, cacheMissFunction);
893-
} catch (UncheckedExecutionException | ExecutionException e) {
887+
// cacheMissFunction is guaranteed to return nonnull
888+
return Objects.requireNonNull(ep.cache.get(fingerprint, cacheMissFunction));
889+
} catch (RuntimeException e) { // Propagated from cacheMissFunction
894890
// Exception was thrown when creating a new instance.
895891
final Throwable cause = e.getCause();
896892
if (cause instanceof FormValidation) {
@@ -899,7 +895,7 @@ public static abstract class FactoryEP implements ExtensionPoint {
899895
if (cause instanceof RuntimeException) {
900896
throw (RuntimeException) cause;
901897
}
902-
throw new RuntimeException(e);
898+
throw e;
903899
}
904900
}
905901

@@ -1012,16 +1008,4 @@ private SessionClientV3Provider(OSClient.OSClientV3 toStore, String usedRegion,
10121008
}
10131009
}
10141010
}
1015-
1016-
static {
1017-
// Log where guava is coming from. This can not be reliably tested as jenkins-test-harness, hpi:run and actual
1018-
// jenkins deployed plugin have different classloader environments. Messing around with maven-hpi-plugin opts can
1019-
// fix or break any of that and there is no regression test to catch that.
1020-
try {
1021-
File path = Which.jarFile(MoreObjects.ToStringHelper.class);
1022-
LOGGER.info("com.google.common.base.Objects loaded from " + path);
1023-
} catch (IOException e) {
1024-
LOGGER.log(Level.WARNING, "Unable to get source of com.google.common.base.Objects", e);
1025-
}
1026-
}
10271011
}

‎plugin/src/test/java/jenkins/plugins/openstack/compute/JCloudsCloudTest.java‎

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
import com.gargoylesoftware.htmlunit.html.HtmlForm;
1212
import com.gargoylesoftware.htmlunit.html.HtmlFormUtil;
1313
import com.gargoylesoftware.htmlunit.html.HtmlPage;
14-
import com.google.common.cache.Cache;
14+
import com.github.benmanes.caffeine.cache.Cache;
1515
import hudson.ExtensionList;
1616
import hudson.model.Item;
1717
import hudson.model.Label;
@@ -49,7 +49,6 @@
4949
import org.kohsuke.stapler.StaplerRequest;
5050
import org.mockito.stubbing.Answer;
5151
import org.openstack4j.api.OSClient;
52-
import org.openstack4j.openstack.compute.domain.NovaServer;
5352

5453
import javax.annotation.Nonnull;
5554
import java.io.IOException;
@@ -62,18 +61,15 @@
6261
import static org.hamcrest.MatcherAssert.assertThat;
6362
import static org.hamcrest.Matchers.anyOf;
6463
import static org.hamcrest.Matchers.containsString;
65-
import static org.hamcrest.Matchers.empty;
6664
import static org.hamcrest.Matchers.instanceOf;
6765
import static org.hamcrest.Matchers.not;
6866
import static org.hamcrest.Matchers.sameInstance;
6967
import static org.junit.Assert.assertEquals;
7068
import static org.junit.Assert.assertNotEquals;
7169
import static org.junit.Assert.assertNull;
7270
import static org.junit.Assert.assertSame;
73-
import static org.junit.Assert.assertTrue;
7471
import static org.junit.Assert.fail;
7572
import static org.mockito.Matchers.any;
76-
import static org.mockito.Mockito.CALLS_REAL_METHODS;
7773
import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
7874
import static org.mockito.Mockito.mock;
7975
import static org.mockito.Mockito.times;
@@ -86,13 +82,6 @@ public class JCloudsCloudTest {
8682
@Rule
8783
public PluginTestRule j = new PluginTestRule();
8884

89-
@Test @Issue("JENKINS-39282") // The problem does not manifest in jenkins-test-harness - created as a regression test
90-
public void guavaLeak() {
91-
NovaServer server = mock(NovaServer.class, CALLS_REAL_METHODS);
92-
server.id = "424242";
93-
assertThat(server.toString(), containsString("424242"));
94-
}
95-
9685
@Test
9786
public void failToTestConnection() {
9887
DescriptorImpl desc = j.getCloudDescriptor();

0 commit comments

Comments
 (0)