Skip to content

Commit f8d3db2

Browse files
andyb-elasticandyb-elastic
committed
code review fixes
Set windows images ids from gradle project properties instead of environment variables Make vagrant test plugin configuration nonstatic so applications of the plugin don't stomp each other Make the plugin aware of which boxes make up the universe of ones that we test, versus what images are actually available from the configuration at runtime, and fail if those don't match up. Add tasks to list all boxes and available boxes so it's clear to the user what configuration gradle is seeing.
1 parent 0a5b7dd commit f8d3db2

File tree

2 files changed

+113
-66
lines changed

2 files changed

+113
-66
lines changed

‎TESTING.asciidoc

+29-19
Original file line numberDiff line numberDiff line change
@@ -335,18 +335,20 @@ the tar, zip, and deb packages and all the plugins. It will then run the tests
335335
on ubuntu-1404 and centos-7. We chose those two distributions as the default
336336
because they cover deb and rpm packaging and SyvVinit and systemd.
337337

338-
You can choose which boxes to test by setting the `-Pvagrant.boxes` project property. For
339-
example, to test all boxes, run `./gradlew -Pvagrant.boxes=all`. For a complete list of
340-
available boxes, run `./gradlew :qa:vagrant:tasks --all`. All of the following values are
341-
supported by the `vagrant.boxes` property
338+
You can choose which boxes to test by setting the `-Pvagrant.boxes` project property. All of
339+
the valid options for this property are:
342340

343341
* `sample` - The default, only chooses ubuntu-1404 and centos-7
344-
* `all` - All configured boxes, i.e. all linux boxes and any configured Windows
345-
boxes.
346-
* `linux-all` - All linux boxes.
347-
* `windows-all` - All configured Windows boxes. If there are none configured when this
348-
value is set, the build will fail.
349342
* List of box names, comma separated (e.g. `oel-7,fedora-26`) - Chooses exactly the boxes listed.
343+
* `linux-all` - All linux boxes.
344+
* `windows-all` - All Windows boxes. If there are any Windows boxes which do not
345+
have images available when this value is provided, the build will fail.
346+
* `all` - All boxes we test. If there are any boxes (e.g. Windows) which do not have images
347+
available when this value is provided, the build will fail.
348+
349+
For a complete list of boxes on which tests can be run, run `./gradlew :qa:vagrant:listAllBoxes`.
350+
For a list of boxes that have images available from your configuration, run
351+
`./gradlew :qa:vagrant:listAvailableBoxes`
350352

351353
Note that if you interrupt gradle in the middle of running these tasks, any boxes started
352354
will remain running and you'll have to stop them manually with `./gradlew stop` or
@@ -389,19 +391,27 @@ Testing on Windows requires the https://github.com/criteo/vagrant-winrm[vagrant-
389391
vagrant plugin install vagrant-winrm
390392
------------------------------------
391393

392-
Specify the image IDs of the Windows boxes to gradle with the following environment
393-
variables. These are used by both the gradle build and Vagrant, so if they're exported
394-
the Windows boxes will be recognized when you run gradle commands, or Vagrant commands
395-
directly.
394+
Specify the image IDs of the Windows boxes to gradle with the following project
395+
properties. They can be set in `~/.gradle/gradle.properties` like
396396

397-
* `VAGRANT_WINDOWS_2012R2_BOX`
398-
* `VAGRANT_WINDOWS_2016_BOX`
397+
------------------------------------
398+
vagrant.windows-2012r2.id=my-image-id
399+
vagrant.windows-2016.id=another-image-id
400+
------------------------------------
401+
402+
or passed on the command line like `-Pvagrant.windows-2012r2.id=my-image-id`
403+
`-Pvagrant.windows-2016=another-image-id`
399404

400-
These variables are required for Windows support in all gradle tasks that
405+
These properties are required for Windows support in all gradle tasks that
401406
handle packaging tests. Either or both may be specified. Remember that to run tests
402-
on these boxes, they still need to be included in the value of `-Pvagrant.boxes`.
403-
When these properties are present, passing `-Pvagrant.boxes=all` will include the
404-
Windows boxes.
407+
on these boxes, the project property `vagrant.boxes` still needs to be set to a
408+
value that will include them.
409+
410+
If you're running vagrant commands outside of gradle, specify the Windows boxes
411+
with the environment variables
412+
413+
* `VAGRANT_WINDOWS_2012R2_BOX`
414+
* `VAGRANT_WINDOWS_2016_BOX`
405415

406416
=== Testing VMs are disposable
407417

‎buildSrc/src/main/groovy/org/elasticsearch/gradle/vagrant/VagrantTestPlugin.groovy

+84-47
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,12 @@ import org.gradle.api.tasks.Delete
1313
import org.gradle.api.tasks.Exec
1414
import org.gradle.api.tasks.TaskState
1515

16+
import static java.util.Collections.unmodifiableList
17+
1618
class VagrantTestPlugin implements Plugin<Project> {
1719

18-
/** All available boxes **/
19-
static List<String> LINUX_BOXES = [
20+
/** All Linux boxes that we test. These are all always supplied **/
21+
static final List<String> LINUX_BOXES = unmodifiableList([
2022
'centos-6',
2123
'centos-7',
2224
'debian-8',
@@ -29,49 +31,53 @@ class VagrantTestPlugin implements Plugin<Project> {
2931
'sles-12',
3032
'ubuntu-1404',
3133
'ubuntu-1604'
32-
]
34+
])
3335

34-
/** Windows boxes that are available - map of box names to image IDs **/
35-
static List<String> WINDOWS_BOXES = []
36+
/** All Windows boxes that we test, which may or may not be supplied **/
37+
static final List<String> WINDOWS_BOXES = unmodifiableList([
38+
'windows-2012r2',
39+
'windows-2016'
40+
])
3641

37-
/** All available boxes **/
38-
static List<String> BOXES
42+
/** All boxes that we test, some of which may not be supplied **/
43+
static final List<String> ALL_BOXES = unmodifiableList(LINUX_BOXES + WINDOWS_BOXES)
3944

4045
/** Boxes used when sampling the tests **/
41-
static List<String> SAMPLE = [
46+
static final List<String> SAMPLE = unmodifiableList([
4247
'centos-7',
43-
'ubuntu-1404',
44-
]
48+
'ubuntu-1404'
49+
])
50+
51+
/** Boxes that have been supplied and are available for testing **/
52+
List<String> AVAILABLE_BOXES = []
4553

4654
/** extra env vars to pass to vagrant for box configuration **/
47-
static Map<String, String> VAGRANT_BOX_ENV_VARS = [:]
55+
Map<String, String> VAGRANT_BOX_ENV_VARS = [:]
4856

4957
/** All distributions to bring into test VM, whether or not they are used **/
50-
static List<String> DISTRIBUTIONS = [
58+
static final List<String> DISTRIBUTIONS = unmodifiableList([
5159
'archives:tar',
5260
'archives:oss-tar',
5361
'packages:rpm',
5462
'packages:oss-rpm',
5563
'packages:deb',
5664
'packages:oss-deb'
57-
]
65+
])
5866

5967
/** Packages onboarded for upgrade tests **/
60-
static List<String> UPGRADE_FROM_ARCHIVES = ['rpm', 'deb']
68+
static final List<String> UPGRADE_FROM_ARCHIVES = unmodifiableList(['rpm', 'deb'])
6169

6270
private static final PACKAGING_CONFIGURATION = 'packaging'
6371
private static final PACKAGING_TEST_CONFIGURATION = 'packagingTest'
6472
private static final BATS = 'bats'
6573
private static final String BATS_TEST_COMMAND ="cd \$PACKAGING_ARCHIVES && sudo bats --tap \$BATS_TESTS/*.$BATS"
6674
private static final String PLATFORM_TEST_COMMAND ="rm -rf ~/elasticsearch && rsync -r /elasticsearch/ ~/elasticsearch && cd ~/elasticsearch && ./gradlew test integTest"
6775

68-
static {
69-
collectAvailableBoxes()
70-
}
71-
7276
@Override
7377
void apply(Project project) {
7478

79+
collectAvailableBoxes(project)
80+
7581
// Creates the Vagrant extension for the project
7682
project.extensions.create('esvagrant', VagrantPropertiesExtension, listSelectedBoxes(project))
7783

@@ -86,12 +92,17 @@ class VagrantTestPlugin implements Plugin<Project> {
8692
createVagrantTasks(project)
8793

8894
if (project.extensions.esvagrant.boxes == null || project.extensions.esvagrant.boxes.size() == 0) {
89-
throw new InvalidUserDataException('Vagrant boxes cannot be null or empty for esvagrant')
95+
throw new InvalidUserDataException('Must specify at least one vagrant box')
9096
}
9197

9298
for (String box : project.extensions.esvagrant.boxes) {
93-
if (BOXES.contains(box) == false) {
94-
throw new InvalidUserDataException("Vagrant box [${box}] not found, available virtual machines are ${BOXES}")
99+
if (ALL_BOXES.contains(box) == false) {
100+
throw new InvalidUserDataException("Vagrant box [${box}] is unknown to this plugin. Valid boxes are ${ALL_BOXES}")
101+
}
102+
103+
if (AVAILABLE_BOXES.contains(box) == false) {
104+
throw new InvalidUserDataException("Vagrant box [${box}] is not available because an image is not supplied for it. " +
105+
"Available boxes with supplied images are ${AVAILABLE_BOXES}")
95106
}
96107
}
97108

@@ -102,26 +113,28 @@ class VagrantTestPlugin implements Plugin<Project> {
102113
/**
103114
* Enumerate all the boxes that we know about and could possibly choose to test
104115
*/
105-
private static void collectAvailableBoxes() {
106-
String windows_2012r2_box = System.getenv('VAGRANT_WINDOWS_2012R2_BOX')
116+
private void collectAvailableBoxes(Project project) {
117+
// these images are hardcoded in the Vagrantfile and are always available
118+
AVAILABLE_BOXES.addAll(LINUX_BOXES)
119+
120+
// these images need to be provided at runtime
121+
String windows_2012r2_box = project.getProperties().get('vagrant.windows-2012r2.id')
107122
if (windows_2012r2_box != null && windows_2012r2_box.isEmpty() == false) {
108-
WINDOWS_BOXES.add('windows-2012r2')
123+
AVAILABLE_BOXES.add('windows-2012r2')
109124
VAGRANT_BOX_ENV_VARS['VAGRANT_WINDOWS_2012R2_BOX'] = windows_2012r2_box
110125
}
111126

112-
String windows_2016_box = System.getenv('VAGRANT_WINDOWS_2016_BOX')
127+
String windows_2016_box = project.getProperties().get('vagrant.windows-2016.id')
113128
if (windows_2016_box != null && windows_2016_box.isEmpty() == false) {
114-
WINDOWS_BOXES.add('windows-2016')
129+
AVAILABLE_BOXES.add('windows-2016')
115130
VAGRANT_BOX_ENV_VARS['VAGRANT_WINDOWS_2016_BOX'] = windows_2016_box
116131
}
117-
118-
BOXES = LINUX_BOXES + WINDOWS_BOXES
119132
}
120133

121134
/**
122135
* Enumerate all the boxes that we have chosen to test
123136
*/
124-
private List<String> listSelectedBoxes(Project project) {
137+
private static List<String> listSelectedBoxes(Project project) {
125138
String vagrantBoxes = project.getProperties().get('vagrant.boxes', 'sample')
126139
switch (vagrantBoxes) {
127140
case 'sample':
@@ -131,7 +144,9 @@ class VagrantTestPlugin implements Plugin<Project> {
131144
case 'windows-all':
132145
return WINDOWS_BOXES
133146
case 'all':
134-
return BOXES
147+
return ALL_BOXES
148+
case '':
149+
return []
135150
default:
136151
return vagrantBoxes.split(',')
137152
}
@@ -314,36 +329,57 @@ class VagrantTestPlugin implements Plugin<Project> {
314329
private static void createPackagingTestTask(Project project) {
315330
project.tasks.create('packagingTest') {
316331
group 'Verification'
317-
description "Tests yum/apt packages using vagrant and bats.\n" +
318-
" Specify the vagrant boxes to test using the gradle property 'vagrant.boxes'.\n" +
319-
" 'sample' can be used to test a single yum and apt box. 'all' can be used to\n" +
320-
" test all available boxes. The available boxes are: \n" +
321-
" ${BOXES}"
332+
description "Tests distribution installation on different platforms using vagrant. See TESTING.asciidoc for details."
322333
dependsOn 'vagrantCheckVersion'
323334
}
324335
}
325336

326337
private static void createPlatformTestTask(Project project) {
327338
project.tasks.create('platformTest') {
328339
group 'Verification'
329-
description "Test unit and integ tests on different platforms using vagrant.\n" +
330-
" Specify the vagrant boxes to test using the gradle property 'vagrant.boxes'.\n" +
331-
" 'all' can be used to test all available boxes. The available boxes are: \n" +
332-
" ${BOXES}"
340+
description "Test unit and integ tests on different platforms using vagrant. See TESTING.asciidoc for details. This test " +
341+
"is unmaintained."
342+
dependsOn 'vagrantCheckVersion'
343+
}
344+
}
345+
346+
private void createBoxListTasks(Project project) {
347+
project.tasks.create('listAllBoxes') {
348+
group 'Verification'
349+
description 'List all vagrant boxes which can be tested by this plugin'
350+
doLast {
351+
println("All vagrant boxes supported by ${project.path}")
352+
for (String box : ALL_BOXES) {
353+
println(box)
354+
}
355+
}
356+
dependsOn 'vagrantCheckVersion'
357+
}
358+
359+
project.tasks.create('listAvailableBoxes') {
360+
group 'Verification'
361+
description 'List all vagrant boxes which are available for testing'
362+
doLast {
363+
println("All vagrant boxes available to ${project.path}")
364+
for (String box : AVAILABLE_BOXES) {
365+
println(box)
366+
}
367+
}
333368
dependsOn 'vagrantCheckVersion'
334369
}
335370
}
336371

337-
private static void createVagrantTasks(Project project) {
372+
private void createVagrantTasks(Project project) {
338373
createCleanTask(project)
339374
createStopTask(project)
340375
createSmokeTestTask(project)
341376
createPrepareVagrantTestEnvTask(project)
342377
createPackagingTestTask(project)
343378
createPlatformTestTask(project)
379+
createBoxListTasks(project)
344380
}
345381

346-
private static void createVagrantBoxesTasks(Project project) {
382+
private void createVagrantBoxesTasks(Project project) {
347383
assert project.extensions.esvagrant.boxes != null
348384

349385
assert project.tasks.stop != null
@@ -375,10 +411,11 @@ class VagrantTestPlugin implements Plugin<Project> {
375411
'VAGRANT_CWD' : "${project.rootDir.absolutePath}",
376412
'VAGRANT_VAGRANTFILE' : 'Vagrantfile',
377413
'VAGRANT_PROJECT_DIR' : "${project.projectDir.absolutePath}"
378-
] + VAGRANT_BOX_ENV_VARS
414+
]
415+
vagrantEnvVars.putAll(VAGRANT_BOX_ENV_VARS)
379416

380417
// Each box gets it own set of tasks
381-
for (String box : BOXES) {
418+
for (String box : AVAILABLE_BOXES) {
382419
String boxTask = box.capitalize().replace('-', '')
383420

384421
// always add a halt task for all boxes, so clean makes sure they are all shutdown
@@ -445,15 +482,15 @@ class VagrantTestPlugin implements Plugin<Project> {
445482
finalizedBy halt
446483
}
447484
vagrantSmokeTest.dependsOn(smoke)
448-
if (box in LINUX_BOXES) {
485+
if (LINUX_BOXES.contains(box)) {
449486
smoke.commandLine = ['vagrant', 'ssh', box, '--command',
450487
"set -o pipefail && echo 'Hello from ${project.path}' | sed -ue 's/^/ ${box}: /'"]
451488
} else {
452489
smoke.commandLine = ['vagrant', 'winrm', box, '--command',
453490
"Write-Host ' ${box}: Hello from ${project.path}'"]
454491
}
455492

456-
if (box in LINUX_BOXES) {
493+
if (LINUX_BOXES.contains(box)) {
457494
Task batsPackagingTest = project.tasks.create("vagrant${boxTask}#batsPackagingTest", BatsOverVagrantTask) {
458495
remoteCommand BATS_TEST_COMMAND
459496
boxName box
@@ -486,7 +523,7 @@ class VagrantTestPlugin implements Plugin<Project> {
486523
project.extensions.esvagrant.testClass != null
487524
}
488525

489-
if (box in LINUX_BOXES) {
526+
if (LINUX_BOXES.contains(box)) {
490527
javaPackagingTest.command = 'ssh'
491528
javaPackagingTest.args = ['--command', 'bash "$PACKAGING_TESTS/run-tests.sh"']
492529
} else {
@@ -509,7 +546,7 @@ class VagrantTestPlugin implements Plugin<Project> {
509546
* This test is unmaintained and was created to run on Linux. We won't allow it to run on Windows
510547
* until it's been brought back into maintenance
511548
*/
512-
if (box in LINUX_BOXES) {
549+
if (LINUX_BOXES.contains(box)) {
513550
Task platform = project.tasks.create("vagrant${boxTask}#platformTest", VagrantCommandTask) {
514551
command 'ssh'
515552
boxName box

0 commit comments

Comments
 (0)