Skip to content

Commit

Permalink
[JENKINS-60551] Copy constructor test should check all fields
Browse files Browse the repository at this point in the history 
* Default jelly values from java constants
* Apply min values for readTimeout (15 sec) and connectTimeout (5 sec)
* Add missing serverCertificate, readTimeout and capOnlyOnAlivePods to
copy constructor
  • Loading branch information
@Dohbedoh @Vlatombe
Dohbedoh authored and Vlatombe committed Dec 20, 2019
1 parent 0630be7 commit 368ec74
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 24 deletions.
44 changes: 39 additions & 5 deletions src/main/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,10 @@ public class KubernetesCloud extends Cloud {
/** Default timeout for idle workers that don't correctly indicate exit. */
public static final int DEFAULT_RETENTION_TIMEOUT_MINUTES = 5;

public static final int DEFAULT_READ_TIMEOUT_SECONDS = 15;

public static final int DEFAULT_CONNECT_TIMEOUT_SECONDS = 5;

private String defaultsProviderTemplate;

@Nonnull
Expand All @@ -118,8 +122,8 @@ public class KubernetesCloud extends Cloud {
private String credentialsId;
private int containerCap = Integer.MAX_VALUE;
private int retentionTimeout = DEFAULT_RETENTION_TIMEOUT_MINUTES;
private int connectTimeout;
private int readTimeout;
private int connectTimeout = DEFAULT_CONNECT_TIMEOUT_SECONDS;
private int readTimeout = DEFAULT_READ_TIMEOUT_SECONDS;
/** @deprecated Stored as a list of PodLabels */
@Deprecated
private transient Map<String, String> labels;
Expand Down Expand Up @@ -152,6 +156,7 @@ public KubernetesCloud(@NonNull String name, @NonNull KubernetesCloud source) {
this.defaultsProviderTemplate = source.defaultsProviderTemplate;
this.templates.addAll(source.templates);
this.serverUrl = source.serverUrl;
this.serverCertificate = source.serverCertificate;
this.skipTlsVerify = source.skipTlsVerify;
this.addMasterProxyEnvVars = source.addMasterProxyEnvVars;
this.namespace = source.namespace;
Expand All @@ -162,10 +167,12 @@ public KubernetesCloud(@NonNull String name, @NonNull KubernetesCloud source) {
this.containerCap = source.containerCap;
this.retentionTimeout = source.retentionTimeout;
this.connectTimeout = source.connectTimeout;
this.readTimeout = source.readTimeout;
this.usageRestricted = source.usageRestricted;
this.maxRequestsPerHost = source.maxRequestsPerHost;
this.podRetention = source.podRetention;
this.waitForPodSec = source.waitForPodSec;
this.capOnlyOnAlivePods = source.capOnlyOnAlivePods;
setPodLabels(source.podLabels);
}

Expand Down Expand Up @@ -412,7 +419,7 @@ public int getReadTimeout() {

@DataBoundSetter
public void setReadTimeout(int readTimeout) {
this.readTimeout = readTimeout;
this.readTimeout = Math.max(DEFAULT_READ_TIMEOUT_SECONDS, readTimeout);
}

public int getConnectTimeout() {
Expand Down Expand Up @@ -486,7 +493,7 @@ public int getMaxRequestsPerHost() {

@DataBoundSetter
public void setConnectTimeout(int connectTimeout) {
this.connectTimeout = connectTimeout;
this.connectTimeout = Math.max(DEFAULT_CONNECT_TIMEOUT_SECONDS, connectTimeout);
}

/**
Expand Down Expand Up @@ -761,6 +768,7 @@ public static void addAliases() {
}

@RequirePOST
@SuppressWarnings("unused") // used by jelly
public FormValidation doTestConnection(@QueryParameter String name, @QueryParameter String serverUrl, @QueryParameter String credentialsId,
@QueryParameter String serverCertificate,
@QueryParameter boolean skipTlsVerify,
Expand Down Expand Up @@ -790,6 +798,7 @@ public FormValidation doTestConnection(@QueryParameter String name, @QueryParame
}

@RequirePOST
@SuppressWarnings("unused") // used by jelly
public ListBoxModel doFillCredentialsIdItems(@QueryParameter String serverUrl) {
Jenkins.get().checkPermission(Jenkins.ADMINISTER);
return new StandardListBoxModel().withEmptySelection() //
Expand All @@ -813,6 +822,7 @@ public ListBoxModel doFillCredentialsIdItems(@QueryParameter String serverUrl) {
}

@RequirePOST
@SuppressWarnings("unused") // used by jelly
public FormValidation doCheckMaxRequestsPerHostStr(@QueryParameter String value) throws IOException, ServletException {
try {
Integer.parseInt(value);
Expand All @@ -822,6 +832,7 @@ public FormValidation doCheckMaxRequestsPerHostStr(@QueryParameter String value)
}
}

@SuppressWarnings("unused") // used by jelly
public FormValidation doCheckDirectConnection(@QueryParameter boolean value, @QueryParameter String jenkinsUrl) throws IOException, ServletException {
int slaveAgentPort = Jenkins.get().getSlaveAgentPort();
if(slaveAgentPort == -1) return FormValidation.warning(
Expand All @@ -845,6 +856,7 @@ public FormValidation doCheckDirectConnection(@QueryParameter boolean value, @Qu
return FormValidation.ok();
}

@SuppressWarnings("unused") // used by jelly
public FormValidation doCheckJenkinsUrl(@QueryParameter String value, @QueryParameter boolean directConnection) throws IOException, ServletException {
try {
if(!isEmpty(value)) new URL(value);
Expand All @@ -854,6 +866,7 @@ public FormValidation doCheckJenkinsUrl(@QueryParameter String value, @QueryPara
return FormValidation.ok();
}

@SuppressWarnings("unused") // used by jelly
public List<Descriptor<PodRetention>> getAllowedPodRetentions() {
Jenkins jenkins = Jenkins.getInstanceOrNull();
if (jenkins == null) {
Expand All @@ -862,7 +875,7 @@ public List<Descriptor<PodRetention>> getAllowedPodRetentions() {
return DescriptorVisibilityFilter.apply(this, jenkins.getDescriptorList(PodRetention.class));
}

@SuppressWarnings("rawtypes")
@SuppressWarnings({"rawtypes", "unused"}) // used by jelly
public Descriptor getDefaultPodRetention() {
Jenkins jenkins = Jenkins.getInstanceOrNull();
if (jenkins == null) {
Expand All @@ -871,6 +884,25 @@ public Descriptor getDefaultPodRetention() {
return jenkins.getDescriptor(PodRetention.getKubernetesCloudDefault().getClass());
}

@SuppressWarnings("unused") // used by jelly
public int getDefaultReadTimeout() {
return DEFAULT_READ_TIMEOUT_SECONDS;
}

@SuppressWarnings("unused") // used by jelly
public int getDefaultConnectTimeout() {
return DEFAULT_CONNECT_TIMEOUT_SECONDS;
}

@SuppressWarnings("unused") // used by jelly
public int getDefaultRetentionTimeout() {
return DEFAULT_RETENTION_TIMEOUT_MINUTES;
}

public int getDefaultWaitForPod() {
return DEFAULT_WAIT_FOR_POD_SEC;
}

}

@Override
Expand Down Expand Up @@ -913,6 +945,8 @@ private Object readResolve() {
if (podRetention == null) {
podRetention = PodRetention.getKubernetesCloudDefault();
}
setConnectTimeout(connectTimeout);
setReadTimeout(readTimeout);
setRetentionTimeout(retentionTimeout);
if (waitForPodSec == null) {
waitForPodSec = DEFAULT_WAIT_FOR_POD_SEC;
Expand Down
8 changes: 4 additions & 4 deletions src/main/resources/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloud/config.jelly
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,11 @@
</f:entry>

<f:entry title="${%Connection Timeout (seconds)}" field="connectTimeout">
<f:textbox default="5"/>
<f:textbox default="${descriptor.defaultConnectTimeout}"/>
</f:entry>

<f:entry title="${%Read Timeout (seconds)}" field="readTimeout">
<f:textbox default="15"/>
<f:textbox default="${descriptor.defaultReadTimeout}"/>
</f:entry>

<f:entry title="${%Concurrency Limit}" field="containerCapStr">
Expand All @@ -64,12 +64,12 @@
</f:entry>

<f:entry title="Seconds to wait for pod to be running" field="waitForPodSec">
<f:number clazz="required number" min="0" step="1" default="600"/>
<f:number clazz="required number" min="0" step="1" default="${descriptor.defaultWaitForPod}"/>
</f:entry>

<f:advanced>
<f:entry title="${%Container Cleanup Timeout (minutes)}" field="retentionTimeout">
<f:textbox default="5"/>
<f:textbox default="${descriptor.defaultRetentionTimeout}"/>
</f:entry>

<f:entry title="${%Transfer proxy related environment variables from master to agent}" field="addMasterProxyEnvVars">
Expand Down
45 changes: 30 additions & 15 deletions src/test/java/org/csanchez/jenkins/plugins/kubernetes/KubernetesCloudTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static org.junit.Assert.*;

import java.io.IOException;
import java.lang.reflect.InvocationTargetException;
import java.security.KeyStoreException;
import java.security.NoSuchAlgorithmException;
import java.security.UnrecoverableKeyException;
Expand All @@ -26,6 +27,10 @@
import com.gargoylesoftware.htmlunit.html.HtmlInput;
import com.gargoylesoftware.htmlunit.html.HtmlPage;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists;
import org.apache.commons.beanutils.PropertyUtils;
import org.apache.commons.lang3.RandomStringUtils;
import org.apache.commons.lang3.RandomUtils;
import org.csanchez.jenkins.plugins.kubernetes.pod.retention.Always;
import org.csanchez.jenkins.plugins.kubernetes.pod.retention.PodRetention;
import org.csanchez.jenkins.plugins.kubernetes.volumes.EmptyDirVolume;
Expand Down Expand Up @@ -274,28 +279,38 @@ public void testLabels() {
}

@Test
public void copyConstructor() {
public void copyConstructor() throws Exception {
PodTemplate pt = new PodTemplate();
pt.setName("podTemplate");

KubernetesCloud cloud = new KubernetesCloud("name");
cloud.setDefaultsProviderTemplate("default");
ArrayList<String> objectProperties = Lists.newArrayList("templates", "podRetention", "podLabels", "labels");
for (String property: PropertyUtils.describe(cloud).keySet()) {
if (PropertyUtils.isWriteable(cloud, property)) {
Class<?> propertyType = PropertyUtils.getPropertyType(cloud, property);
if (propertyType == String.class) {
if (property.endsWith("Str")) {
// setContainerCapStr
// setMaxRequestsPerHostStr
PropertyUtils.setProperty(cloud, property, RandomStringUtils.randomNumeric(3));
} else {
PropertyUtils.setProperty(cloud, property, RandomStringUtils.randomAlphabetic(10));
}
} else if (propertyType == int.class) {
PropertyUtils.setProperty(cloud, property, RandomUtils.nextInt());
} else if (propertyType == Integer.class) {
PropertyUtils.setProperty(cloud, property, Integer.valueOf(RandomUtils.nextInt()));
} else if (propertyType == boolean.class) {
PropertyUtils.setProperty(cloud, property, RandomUtils.nextBoolean());
} else if (!objectProperties.contains(property)) {
fail("Unhandled field in copy constructor: " + property);
}
}
}
cloud.setTemplates(Collections.singletonList(pt));
cloud.setServerUrl("serverUrl");
cloud.setSkipTlsVerify(true);
cloud.setAddMasterProxyEnvVars(true);
cloud.setNamespace("namespace");
cloud.setJenkinsUrl("jenkinsUrl");
cloud.setJenkinsTunnel("tunnel");
cloud.setCredentialsId("abcd");
cloud.setContainerCapStr("100");
cloud.setRetentionTimeout(1000);
cloud.setConnectTimeout(123);
cloud.setUsageRestricted(true);
cloud.setMaxRequestsPerHostStr("42");
cloud.setPodRetention(new Always());
cloud.setWaitForPodSec(245);
cloud.setPodLabels(PodLabel.listOf("foo", "bar", "cat", "dog"));
cloud.setLabels(ImmutableMap.of("foo", "bar"));

KubernetesCloud copy = new KubernetesCloud("copy", cloud);

Expand Down

0 comments on commit 368ec74

Please sign in to comment.