Skip to content

Commit

Permalink
Merge pull request #318 from aytuncbeken/JENKINS-44285
Browse files Browse the repository at this point in the history
[JENKINS-44285] Tool Location overwrites are not preserved
  • Loading branch information
carlossg authored Jul 16, 2018
2 parents 63ebaa3 + 0c0e489 commit 3599d4d
Show file tree
Hide file tree
Showing 7 changed files with 217 additions and 22 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.csanchez.jenkins.plugins.kubernetes;

import java.io.IOException;
import java.io.Serializable;
import java.util.ArrayList;
import java.util.Collections;
Expand All @@ -9,9 +10,14 @@
import java.util.Set;
import java.util.logging.Level;
import java.util.logging.Logger;

import javax.annotation.Nonnull;

import edu.umd.cs.findbugs.annotations.NonNull;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import hudson.model.*;
import hudson.slaves.NodeProperty;
import hudson.slaves.NodePropertyDescriptor;
import hudson.util.DescribableList;
import org.apache.commons.lang.StringUtils;
import org.csanchez.jenkins.plugins.kubernetes.model.TemplateEnvVar;
import org.csanchez.jenkins.plugins.kubernetes.volumes.PodVolume;
Expand All @@ -21,19 +27,11 @@
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.DataBoundSetter;

import com.google.common.base.Strings;
import com.google.common.collect.ImmutableMap;

import hudson.Extension;
import hudson.Util;
import hudson.model.AbstractDescribableImpl;
import hudson.model.Descriptor;
import hudson.model.DescriptorVisibilityFilter;
import hudson.model.Label;
import hudson.model.Node;
import hudson.model.labels.LabelAtom;
import hudson.tools.ToolLocationNodeProperty;
import io.fabric8.kubernetes.api.model.Pod;
import io.fabric8.kubernetes.client.KubernetesClient;
import jenkins.model.Jenkins;
Expand All @@ -43,7 +41,7 @@
*
* @author <a href="mailto:nicolas.deloof@gmail.com">Nicolas De Loof</a>
*/
public class PodTemplate extends AbstractDescribableImpl<PodTemplate> implements Serializable {
public class PodTemplate extends AbstractDescribableImpl<PodTemplate> implements Serializable, Saveable {

private static final long serialVersionUID = 3285310269140845583L;

Expand Down Expand Up @@ -112,7 +110,7 @@ public class PodTemplate extends AbstractDescribableImpl<PodTemplate> implements

private List<PodImagePullSecret> imagePullSecrets = new ArrayList<PodImagePullSecret>();

private transient List<ToolLocationNodeProperty> nodeProperties;
private PodTemplateToolLocation nodeProperties;

private String yaml;

Expand All @@ -137,6 +135,7 @@ public PodTemplate(PodTemplate from) {
this.setVolumes(from.getVolumes());
this.setWorkspaceVolume(from.getWorkspaceVolume());
this.setYaml(from.getYaml());
this.setNodeProperties(from.getNodeProperties());
}

@Deprecated
Expand Down Expand Up @@ -486,18 +485,19 @@ public void setImagePullSecrets(List<PodImagePullSecret> imagePullSecrets) {
}

@DataBoundSetter
public void setNodeProperties(List<ToolLocationNodeProperty> nodeProperties){
this.nodeProperties = nodeProperties;
public void setNodeProperties(List<? extends NodeProperty<?>> properties) {
this.getNodeProperties().clear();
this.getNodeProperties().addAll(properties);
}

@Nonnull
public List<ToolLocationNodeProperty> getNodeProperties(){
if (nodeProperties == null) {
return Collections.emptyList();
}
@NonNull
public PodTemplateToolLocation getNodeProperties(){
if( this.nodeProperties == null)
this.nodeProperties = new PodTemplateToolLocation(this);
return nodeProperties;
}


@Deprecated
public String getResourceRequestMemory() {
return getFirstContainer().map(ContainerTemplate::getResourceRequestMemory).orElse(null);
Expand Down Expand Up @@ -671,6 +671,12 @@ private void optionalField(StringBuilder builder, String label, String value) {
}
}

/**
* Empty implementation of Saveable interface. This interface is used for DescribableList implementation
*/
@Override
public void save() { }

@Extension
public static class DescriptorImpl extends Descriptor<PodTemplate> {

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
Copyright (c) 2018 Aytunc Beken
Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated
documentation files (the "Software"), to deal in the Software without restriction, including without limitation
the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software,
and to permit persons to whom the Software is furnished to do so, subject to the following conditions:
The above copyright notice and this permission notice shall be included in all copies or substantial portions
of the Software.
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO
THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
*/
package org.csanchez.jenkins.plugins.kubernetes;

import hudson.model.Saveable;
import hudson.slaves.NodeProperty;
import hudson.slaves.NodePropertyDescriptor;
import hudson.util.DescribableList;

import java.io.Serializable;
import java.util.Collection;

/**
* Pod Template Tool Location
* This class extends Jenkins DescribableList as implemented in Slave Class. Also implements Serializable interface
* for PodTemplate Class.
* Using DescribableList is not possible directly in PodTemplate because DescribableList is not Serializable.
*
* @author <a href="mailto:aytuncbeken.ab@gmail.com">Aytunc BEKEN</a>
*/
public class PodTemplateToolLocation extends DescribableList<NodeProperty<?>,NodePropertyDescriptor> implements Serializable {

public PodTemplateToolLocation() {}


public PodTemplateToolLocation(DescribableList.Owner owner) {
super(owner);
}

public PodTemplateToolLocation(Saveable owner) {
super(owner);
}

public PodTemplateToolLocation(Saveable owner, Collection<? extends NodeProperty<?>> initialList) {
super(owner,initialList);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@
import javax.annotation.Nonnull;

import org.apache.commons.lang.StringUtils;
import hudson.slaves.NodeProperty;
import hudson.slaves.NodePropertyDescriptor;
import hudson.util.DescribableList;
import jenkins.model.Jenkins;
import org.csanchez.jenkins.plugins.kubernetes.model.TemplateEnvVar;
import org.csanchez.jenkins.plugins.kubernetes.volumes.PodVolume;
import org.csanchez.jenkins.plugins.kubernetes.volumes.workspace.WorkspaceVolume;
Expand Down Expand Up @@ -305,8 +309,7 @@ public static PodTemplate combine(PodTemplate parent, PodTemplate template) {
WorkspaceVolume workspaceVolume = template.isCustomWorkspaceVolumeEnabled() && template.getWorkspaceVolume() != null ? template.getWorkspaceVolume() : parent.getWorkspaceVolume();

//Tool location node properties
List<ToolLocationNodeProperty> toolLocationNodeProperties = new ArrayList<>();
toolLocationNodeProperties.addAll(parent.getNodeProperties());
PodTemplateToolLocation toolLocationNodeProperties = parent.getNodeProperties();
toolLocationNodeProperties.addAll(template.getNodeProperties());

PodTemplate podTemplate = new PodTemplate();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package org.csanchez.jenkins.plugins.kubernetes;

import java.util.Arrays;
import java.util.Collections;

import jenkins.model.JenkinsLocationConfiguration;
import org.csanchez.jenkins.plugins.kubernetes.volumes.EmptyDirVolume;
import org.csanchez.jenkins.plugins.kubernetes.volumes.PodVolume;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
import java.util.Arrays;
import java.util.List;

import hudson.slaves.NodePropertyDescriptor;
import hudson.util.DescribableList;
import org.csanchez.jenkins.plugins.kubernetes.model.KeyValueEnvVar;
import org.csanchez.jenkins.plugins.kubernetes.volumes.EmptyDirVolume;
import org.csanchez.jenkins.plugins.kubernetes.volumes.HostPathVolume;
Expand All @@ -45,6 +47,11 @@
import com.cloudbees.plugins.credentials.SystemCredentialsProvider;
import com.cloudbees.plugins.credentials.impl.UsernamePasswordCredentialsImpl;

import hudson.model.Node;
import hudson.plugins.git.GitTool;
import hudson.slaves.NodeProperty;
import hudson.tools.ToolLocationNodeProperty;
import hudson.tools.ToolLocationNodeProperty.ToolLocation;
import hudson.util.Secret;

/**
Expand Down Expand Up @@ -88,6 +95,21 @@ public void upgradeFrom_0_12() throws Exception {
new KeyValueEnvVar("pod_b_key", "pod_b_value")), templates.get(0).getEnvVars());
}

@Test
@LocalData()
public void upgradeFrom_0_10() throws Exception {
List<PodTemplate> templates = cloud.getTemplates();
PodTemplate template = templates.get(0);
DescribableList<NodeProperty<?>,NodePropertyDescriptor> nodeProperties = template.getNodeProperties();
assertEquals(1, nodeProperties.size());
ToolLocationNodeProperty property = (ToolLocationNodeProperty) nodeProperties.get(0);
assertEquals(1, property.getLocations().size());
ToolLocation location = property.getLocations().get(0);
assertEquals("Default", location.getName());
assertEquals("/custom/path", location.getHome());
assertEquals(GitTool.class, location.getType().clazz);
}

@Test
@LocalData()
public void upgradeFrom_0_8() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,14 @@
import static org.hamcrest.Matchers.*;
import static org.junit.Assert.*;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import hudson.model.Node;
import hudson.slaves.NodeProperty;
import hudson.tools.ToolLocationNodeProperty;
import org.csanchez.jenkins.plugins.kubernetes.model.KeyValueEnvVar;
import org.csanchez.jenkins.plugins.kubernetes.model.SecretEnvVar;
import org.csanchez.jenkins.plugins.kubernetes.volumes.HostPathVolume;
Expand Down Expand Up @@ -614,4 +618,23 @@ public void testValidateLabelTooLong() {
assertFalse(validateLabel("1234567890123456789012345678901234567890123456789012345678901234"));
assertFalse(validateLabel("abcdefghijklmnopqrstuwxyzabcdefghijklmnopqrstuwxyzabcdefghijklmn"));
}

@Test
public void shouldCombineAllToolLocations() {

PodTemplate podTemplate1 = new PodTemplate();
List<ToolLocationNodeProperty> nodeProperties1 = new ArrayList<>();
nodeProperties1.add(new ToolLocationNodeProperty(new ToolLocationNodeProperty.ToolLocation("toolKey1@Test","toolHome1")));
podTemplate1.setNodeProperties(nodeProperties1);

PodTemplate podTemplate2 = new PodTemplate();
List<ToolLocationNodeProperty> nodeProperties2 = new ArrayList<>();
nodeProperties2.add(new ToolLocationNodeProperty(new ToolLocationNodeProperty.ToolLocation("toolKey2@Test","toolHome2")));
podTemplate2.setNodeProperties(nodeProperties2);

PodTemplate result = combine(podTemplate1,podTemplate2);

assertThat(result.getNodeProperties(), hasItems(nodeProperties1.get(0),nodeProperties2.get(0)));

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
<?xml version='1.0' encoding='UTF-8'?>
<hudson>
<disabledAdministrativeMonitors/>
<version>2.7.4</version>
<numExecutors>2</numExecutors>
<mode>NORMAL</mode>
<useSecurity>true</useSecurity>
<authorizationStrategy class="hudson.security.AuthorizationStrategy$Unsecured"/>
<securityRealm class="hudson.security.SecurityRealm$None"/>
<disableRememberMe>false</disableRememberMe>
<projectNamingStrategy class="jenkins.model.ProjectNamingStrategy$DefaultProjectNamingStrategy"/>
<workspaceDir>${JENKINS_HOME}/workspace/${ITEM_FULLNAME}</workspaceDir>
<buildsDir>${ITEM_ROOTDIR}/builds</buildsDir>
<jdks/>
<viewsTabBar class="hudson.views.DefaultViewsTabBar"/>
<myViewsTabBar class="hudson.views.DefaultMyViewsTabBar"/>
<clouds>
<org.csanchez.jenkins.plugins.kubernetes.KubernetesCloud plugin="kubernetes@0.10">
<name>kubernetes</name>
<templates>
<org.csanchez.jenkins.plugins.kubernetes.PodTemplate>
<inheritFrom></inheritFrom>
<name>maven</name>
<instanceCap>2147483647</instanceCap>
<idleMinutes>0</idleMinutes>
<label>maven</label>
<nodeSelector></nodeSelector>
<volumes/>
<containers>
<org.csanchez.jenkins.plugins.kubernetes.ContainerTemplate>
<name>maven</name>
<image>maven:alpine</image>
<privileged>false</privileged>
<alwaysPullImage>false</alwaysPullImage>
<workingDir>/home/jenkins</workingDir>
<command>/bin/sh -c</command>
<args>cat</args>
<ttyEnabled>true</ttyEnabled>
<resourceRequestCpu></resourceRequestCpu>
<resourceRequestMemory></resourceRequestMemory>
<resourceLimitCpu></resourceLimitCpu>
<resourceLimitMemory></resourceLimitMemory>
<envVars/>
</org.csanchez.jenkins.plugins.kubernetes.ContainerTemplate>
</containers>
<envVars/>
<annotations/>
<imagePullSecrets/>
<nodeProperties>
<hudson.tools.ToolLocationNodeProperty>
<locations>
<hudson.tools.ToolLocationNodeProperty_-ToolLocation>
<type>hudson.plugins.git.GitTool$DescriptorImpl</type>
<name>Default</name>
<home>/custom/path</home>
</hudson.tools.ToolLocationNodeProperty_-ToolLocation>
</locations>
</hudson.tools.ToolLocationNodeProperty>
</nodeProperties>
</org.csanchez.jenkins.plugins.kubernetes.PodTemplate>
</templates>
<serverUrl>http://example.com</serverUrl>
<skipTlsVerify>false</skipTlsVerify>
<namespace>default</namespace>
<jenkinsUrl></jenkinsUrl>
<containerCap>10</containerCap>
<retentionTimeout>5</retentionTimeout>
<connectTimeout>0</connectTimeout>
<readTimeout>0</readTimeout>
</org.csanchez.jenkins.plugins.kubernetes.KubernetesCloud>
</clouds>
<quietPeriod>5</quietPeriod>
<scmCheckoutRetryCount>0</scmCheckoutRetryCount>
<views>
<hudson.model.AllView>
<owner class="hudson" reference="../../.."/>
<name>All</name>
<filterExecutors>false</filterExecutors>
<filterQueue>false</filterQueue>
<properties class="hudson.model.View$PropertyList"/>
</hudson.model.AllView>
</views>
<primaryView>All</primaryView>
<slaveAgentPort>-1</slaveAgentPort>
<label></label>
<crumbIssuer class="hudson.security.csrf.DefaultCrumbIssuer">
<excludeClientIPFromCrumb>false</excludeClientIPFromCrumb>
</crumbIssuer>
<nodeProperties/>
<globalNodeProperties/>
</hudson>

0 comments on commit 3599d4d

Please sign in to comment.