diff --git a/src/main/java/org/jenkinsci/plugins/vSphereCloudSlaveTemplate.java b/src/main/java/org/jenkinsci/plugins/vSphereCloudSlaveTemplate.java index e29d6422..9e89e21b 100644 --- a/src/main/java/org/jenkinsci/plugins/vSphereCloudSlaveTemplate.java +++ b/src/main/java/org/jenkinsci/plugins/vSphereCloudSlaveTemplate.java @@ -82,7 +82,9 @@ public class vSphereCloudSlaveTemplate implements Describable retentionStrategy, final List> nodeProperties, final List guestInfoProperties) { + this.configVersion = CURRENT_CONFIG_VERSION; this.cloneNamePrefix = cloneNamePrefix; this.masterImageName = masterImageName; this.snapshotName = snapshotName; @@ -349,6 +352,38 @@ protected Object readResolve() { LOGGER.log(Level.CONFIG, " - Failed to reconfigure strategy", ex); } } + // Earlier versions of the code may have stored configuration in a + // different way but in the same kind of fields, so we need an explicit + // versioning to know how to mutate the data. + if (configVersion <= 0) { + LOGGER.log(Level.CONFIG, + "{0} loaded old configuration that had hard-coded underscore at the end of the cloneNamePrefix.", + this); + // In version one, the underscore was removed from the code so we + // have to put it in the data (the user is then free to change it to + // something else if they want to). + this.cloneNamePrefix = this.cloneNamePrefix + "_"; + configVersion = 1; + } + // Note: Subsequent changes dependent on configVersion should go above + // this line. + if (configVersion < CURRENT_CONFIG_VERSION) { + throw new IllegalStateException("Internal error: configVersion==" + configVersion + + " at end of readResolve method, but the current config version should be " + + CURRENT_CONFIG_VERSION + + ". Either CURRENT_CONFIG_VERSION is incorrect or the readResolve method is not setting configVersion when it upgrades the data."); + } + if (configVersion > CURRENT_CONFIG_VERSION) { + LOGGER.log(Level.WARNING, + "{0} was defined by a later version of the plugin " + + "(one that saved with configVersion={1}, whereas this version of the plugin is expecting {2}). " + + "The code may not function as expected.", + new Object[]{ + this, + configVersion, + CURRENT_CONFIG_VERSION + }); + } return this; } diff --git a/src/main/java/org/jenkinsci/plugins/vsphere/tools/CloudProvisioningAlgorithm.java b/src/main/java/org/jenkinsci/plugins/vsphere/tools/CloudProvisioningAlgorithm.java index 20f37720..de420758 100644 --- a/src/main/java/org/jenkinsci/plugins/vsphere/tools/CloudProvisioningAlgorithm.java +++ b/src/main/java/org/jenkinsci/plugins/vsphere/tools/CloudProvisioningAlgorithm.java @@ -44,15 +44,17 @@ public static CloudProvisioningRecord findTemplateWithMostFreeCapacity( } /** - * Chooses a name for a new node. + * Chooses a name for a new node. The name will start with the + * {@link vSphereCloudSlaveTemplate#getCloneNamePrefix()}. * * * @param record @@ -70,7 +72,7 @@ public static String findUnusedName(CloudProvisioningRecord record) { final int maxAttempts = hasCap ? (templateInstanceCap) : 100; for (int attempt = 0; attempt < maxAttempts; attempt++) { final String suffix = hasCap ? calcSequentialSuffix(attempt) : calcRandomSuffix(attempt); - final String nodeName = cloneNamePrefix + "_" + suffix; + final String nodeName = cloneNamePrefix + suffix; if (!existingNames.contains(nodeName)) { return nodeName; } @@ -134,4 +136,4 @@ static BigInteger toBigInteger(final long msb, final long lsb) { final BigInteger bigNumber = new BigInteger(bytes); return bigNumber; } -} \ No newline at end of file +} diff --git a/src/main/resources/org/jenkinsci/plugins/vSphereCloudSlaveTemplate/help-cloneNamePrefix.html b/src/main/resources/org/jenkinsci/plugins/vSphereCloudSlaveTemplate/help-cloneNamePrefix.html index 3133efd0..f367da65 100644 --- a/src/main/resources/org/jenkinsci/plugins/vSphereCloudSlaveTemplate/help-cloneNamePrefix.html +++ b/src/main/resources/org/jenkinsci/plugins/vSphereCloudSlaveTemplate/help-cloneNamePrefix.html @@ -1,6 +1,6 @@
Controls the name by which the virtual machine will be known to vSphere. -The plug-in will append an underscore and a unique ID to this prefix in order to generate unique names.

+The plug-in will append a unique ID to this prefix in order to generate unique names.

Note: You must ensure that no virtual machines (other than those created by this template) have names starting with this prefix.

\ No newline at end of file diff --git a/src/test/java/org/jenkinsci/plugins/vsphere/tools/CloudProvisioningAlgorithmTest.java b/src/test/java/org/jenkinsci/plugins/vsphere/tools/CloudProvisioningAlgorithmTest.java index 785e36fb..02f2af5b 100644 --- a/src/test/java/org/jenkinsci/plugins/vsphere/tools/CloudProvisioningAlgorithmTest.java +++ b/src/test/java/org/jenkinsci/plugins/vsphere/tools/CloudProvisioningAlgorithmTest.java @@ -200,8 +200,8 @@ public void findUnusedNameGivenZeroOfTwoExistsThenReturnsOneThenTwo() { // Given final CloudProvisioningRecord record = createInstance(2, 0, 0); final String prefix = record.getTemplate().getCloneNamePrefix(); - final String expected1 = prefix + "_1"; - final String expected2 = prefix + "_2"; + final String expected1 = prefix + "1"; + final String expected2 = prefix + "2"; // When final String actual1 = CloudProvisioningAlgorithm.findUnusedName(record); @@ -219,10 +219,10 @@ public void findUnusedNameGivenMiddleOfThreeStillExistsThenReturnsOneThenThree() // Given final CloudProvisioningRecord record = createInstance(3, 0, 0); final String prefix = record.getTemplate().getCloneNamePrefix(); - final String expected1 = prefix + "_1"; - final String unwanted = prefix + "_2"; + final String expected1 = prefix + "1"; + final String unwanted = prefix + "2"; record.setCurrentlyUnwanted(unwanted, false); - final String expected2 = prefix + "_3"; + final String expected2 = prefix + "3"; // When final String actual1 = CloudProvisioningAlgorithm.findUnusedName(record); @@ -240,9 +240,9 @@ public void findUnusedNameGivenNoSpaceThenThrowsIllegalStateException() { // Given final CloudProvisioningRecord record = createInstance(3, 0, 0); final String prefix = record.getTemplate().getCloneNamePrefix(); - final String unwanted = prefix + "_1"; - final String active = prefix + "_2"; - final String planned = prefix + "_3"; + final String unwanted = prefix + "1"; + final String active = prefix + "2"; + final String planned = prefix + "3"; record.setCurrentlyUnwanted(unwanted, false); record.addCurrentlyActive(active); record.addCurrentlyPlanned(planned); @@ -264,7 +264,7 @@ public void findUnusedNameGivenOneOfTwoHasEndedThenReturnsOne() { // Given final CloudProvisioningRecord record = createInstance(2, 0, 0); final String prefix = record.getTemplate().getCloneNamePrefix(); - final String expected = prefix + "_1"; + final String expected = prefix + "1"; // When final String actual1 = CloudProvisioningAlgorithm.findUnusedName(record); @@ -296,7 +296,7 @@ public void findUnusedNameGivenUncappedInstancesThenReturnsUniqueNames() { // Then final List uniques = new ArrayList(new LinkedHashSet(actuals)); assertThat(actuals, equalTo(uniques)); - assertThat(actuals, everyItem(startsWith(prefix + "_"))); + assertThat(actuals, everyItem(startsWith(prefix))); } private CloudProvisioningRecord createInstance(int capacity, int provisioned, int planned) {