Skip to content

Commit f4bf100

Browse files
author
Yogesh Gaikwad
committed
fix and enable repository-hdfs secure tests
Due to recent changes done for converting `repository-hdfs` to test clusters (#41252), the `integTestSecure*` tasks did not depend on `secureHdfsFixture` which when running would fail as the fixture would not be available. This commit adds the dependency of fixture to the task. The `secureHdfsFixture` is an AntFixture which is spawned process. Internally it waits for 30 seconds for the resources to be made available. For my local machine it took almost 45 seconds to be available so I have added the wait time as an input to the AntFixture defaults to 30 seconds and set it to 60 seconds in case of secure hdfs fixture. Another problem while running the `secureHdfsFixture` where it would fail due to port not being privileged port (i.e system port, port < 1024). By default datanode address key tries to find a free port and this would later on fail in case it is running in a secure mode. To address this in case of secure mode we find free port below 1024 and set it in the config. The config `DFSConfigKeys.IGNORE_SECURE_PORTS_FOR_TESTING_KEY` is set to `true` but it did not help. https://fisheye.apache.org/browse/~br=branch-2.8.1/hadoop/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/SecureDataNodeStarter.java?hb=true#to140 The integ test for secure hdfs were disabled for long time and so the changes done in #42090 to fix the tests are also done in this commit.
1 parent 688cf83 commit f4bf100

File tree

5 files changed

+52
-9
lines changed

5 files changed

+52
-9
lines changed

buildSrc/src/main/groovy/org/elasticsearch/gradle/test/AntFixture.groovy

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,9 @@ public class AntFixture extends AntTask implements Fixture {
5858
@Input
5959
boolean useShell = false
6060

61+
@Input
62+
int maxWaitInSeconds = 30
63+
6164
/**
6265
* A flag to indicate whether the fixture should be run in the foreground, or spawned.
6366
* It is protected so subclasses can override (eg RunTask).
@@ -128,7 +131,7 @@ public class AntFixture extends AntTask implements Fixture {
128131

129132
String failedProp = "failed${name}"
130133
// first wait for resources, or the failure marker from the wrapper script
131-
ant.waitfor(maxwait: '30', maxwaitunit: 'second', checkevery: '500', checkeveryunit: 'millisecond', timeoutproperty: failedProp) {
134+
ant.waitfor(maxwait: maxWaitInSeconds, maxwaitunit: 'second', checkevery: '500', checkeveryunit: 'millisecond', timeoutproperty: failedProp) {
132135
or {
133136
resourceexists {
134137
file(file: failureMarker.toString())

plugins/repository-hdfs/build.gradle

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,13 +91,13 @@ for (String fixtureName : ['hdfsFixture', 'haHdfsFixture', 'secureHdfsFixture',
9191
dependsOn project.configurations.hdfsFixture, project(':test:fixtures:krb5kdc-fixture').tasks.postProcessFixture
9292
executable = new File(project.runtimeJavaHome, 'bin/java')
9393
env 'CLASSPATH', "${ -> project.configurations.hdfsFixture.asPath }"
94+
maxWaitInSeconds 60
9495
onlyIf { project(':test:fixtures:krb5kdc-fixture').buildFixture.enabled }
9596
waitCondition = { fixture, ant ->
9697
// the hdfs.MiniHDFS fixture writes the ports file when
9798
// it's ready, so we can just wait for the file to exist
9899
return fixture.portsFile.exists()
99100
}
100-
101101
final List<String> miniHDFSArgs = []
102102

103103
// If it's a secure fixture, then depend on Kerberos Fixture and principals + add the krb5conf to the JVM options
@@ -125,7 +125,7 @@ for (String fixtureName : ['hdfsFixture', 'haHdfsFixture', 'secureHdfsFixture',
125125
}
126126
}
127127

128-
Set disabledIntegTestTaskNames = ['integTestSecure', 'integTestSecureHa']
128+
Set disabledIntegTestTaskNames = []
129129

130130
for (String integTestTaskName : ['integTestHa', 'integTestSecure', 'integTestSecureHa']) {
131131
task "${integTestTaskName}"(type: RestIntegTestTask) {
@@ -136,10 +136,13 @@ for (String integTestTaskName : ['integTestHa', 'integTestSecure', 'integTestSec
136136
enabled = false;
137137
}
138138

139+
if (integTestTaskName.contains("Secure")) {
140+
dependsOn secureHdfsFixture
141+
}
142+
139143
runner {
140144
if (integTestTaskName.contains("Secure")) {
141145
if (disabledIntegTestTaskNames.contains(integTestTaskName) == false) {
142-
dependsOn secureHdfsFixture
143146
nonInputProperties.systemProperty "test.krb5.principal.es", "elasticsearch@${realm}"
144147
nonInputProperties.systemProperty "test.krb5.principal.hdfs", "hdfs/hdfs.build.elastic.co@${realm}"
145148
jvmArgs "-Djava.security.krb5.conf=${krb5conf}"

plugins/repository-hdfs/src/test/resources/rest-api-spec/test/secure_hdfs_repository/30_snapshot_get.yml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,17 +48,17 @@
4848
repository: test_snapshot_get_repository
4949
snapshot: test_snapshot_get
5050

51-
- length: { snapshots: 1 }
52-
- match: { snapshots.0.snapshot : test_snapshot_get }
51+
- length: { responses.0.snapshots: 1 }
52+
- match: { responses.0.snapshots.0.snapshot : test_snapshot_get }
5353

5454
# List snapshot info
5555
- do:
5656
snapshot.get:
5757
repository: test_snapshot_get_repository
5858
snapshot: "*"
5959

60-
- length: { snapshots: 1 }
61-
- match: { snapshots.0.snapshot : test_snapshot_get }
60+
- length: { responses.0.snapshots: 1 }
61+
- match: { responses.0.snapshots.0.snapshot : test_snapshot_get }
6262

6363
# Remove our snapshot
6464
- do:

plugins/repository-hdfs/src/test/resources/rest-api-spec/test/secure_hdfs_repository/30_snapshot_readonly.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
repository: test_snapshot_repository_ro
2424
snapshot: "_all"
2525

26-
- length: { snapshots: 1 }
26+
- length: { responses.0.snapshots: 1 }
2727

2828
# Remove our repository
2929
- do:

test/fixtures/hdfs-fixture/src/main/java/hdfs/MiniHDFS.java

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,9 @@
3434
import org.apache.hadoop.security.UserGroupInformation;
3535

3636
import java.io.File;
37+
import java.io.IOException;
3738
import java.lang.management.ManagementFactory;
39+
import java.net.ServerSocket;
3840
import java.net.URL;
3941
import java.nio.charset.StandardCharsets;
4042
import java.nio.file.Files;
@@ -44,6 +46,7 @@
4446
import java.util.ArrayList;
4547
import java.util.Arrays;
4648
import java.util.List;
49+
import java.util.Random;
4750

4851
/**
4952
* MiniHDFS test fixture. There is a CLI tool, but here we can
@@ -94,11 +97,16 @@ public static void main(String[] args) throws Exception {
9497
cfg.set(DFSConfigKeys.DFS_BLOCK_ACCESS_TOKEN_ENABLE_KEY, "true");
9598
cfg.set(DFSConfigKeys.IGNORE_SECURE_PORTS_FOR_TESTING_KEY, "true");
9699
cfg.set(DFSConfigKeys.DFS_ENCRYPT_DATA_TRANSFER_KEY, "true");
100+
// If we ask port to be allocated automatically, this fails in case of secure hdfs setup
101+
// it needs port to be privileged. See org.apache.hadoop.hdfs.server.datanode.SecureDataNodeStarter
102+
cfg.set(DFSConfigKeys.DFS_DATANODE_ADDRESS_KEY, "0.0.0.0:" + getFreeSocketPort(secure));
103+
cfg.set(DFSConfigKeys.DFS_DATANODE_HTTP_ADDRESS_KEY, "0.0.0.0:" + getFreeSocketPort(secure));
97104
}
98105

99106
UserGroupInformation.setConfiguration(cfg);
100107

101108
MiniDFSCluster.Builder builder = new MiniDFSCluster.Builder(cfg);
109+
builder.checkDataNodeAddrConfig(true);
102110
if (secure) {
103111
builder.nameNodePort(9998);
104112
} else {
@@ -173,6 +181,35 @@ public static void main(String[] args) throws Exception {
173181
tmp = Files.createTempFile(baseDir, null, null);
174182
Files.write(tmp, portFileContent.getBytes(StandardCharsets.UTF_8));
175183
Files.move(tmp, baseDir.resolve(PORT_FILE_NAME), StandardCopyOption.ATOMIC_MOVE);
184+
185+
while(true) {
186+
Thread.sleep(2000);
187+
}
176188
}
177189

190+
191+
/**
192+
* Return a free port number. There is no guarantee it will remain free, so
193+
* it should be used immediately.
194+
* @param secure if {@code true} then returns free port less than 1024.
195+
* @returns a free port if found or else returns 0
196+
*/
197+
private static int getFreeSocketPort(boolean secure) {
198+
int port = 0;
199+
int tries = 0;
200+
do {
201+
try {
202+
port = secure ? new Random().nextInt(1024) : 0;
203+
ServerSocket s = new ServerSocket(port);
204+
s.setReuseAddress(true);
205+
port = s.getLocalPort();
206+
s.close();
207+
return port;
208+
} catch (IOException e) {
209+
// Could not get a free port, continue
210+
}
211+
tries++;
212+
} while (tries < 100 && (port == 0 && !secure) || (secure && port > 1024));
213+
return port;
214+
}
178215
}

0 commit comments

Comments
 (0)