Skip to content

Commit d7d6ec8

Browse files
smengclanuengineer
smengcl
authored and
anuengineer
committed
HDDS-2128. Make ozone sh command work with OM HA service ids (#1445)
1 parent 5363730 commit d7d6ec8

File tree

2 files changed

+359
-1
lines changed

2 files changed

+359
-1
lines changed

‎hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/ozShell/TestOzoneShellHA.java‎

Lines changed: 343 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,343 @@
1+
/**
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
package org.apache.hadoop.ozone.ozShell;
19+
20+
import com.google.common.base.Strings;
21+
import org.apache.hadoop.fs.FileUtil;
22+
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
23+
import org.apache.hadoop.ozone.MiniOzoneCluster;
24+
import org.apache.hadoop.ozone.MiniOzoneHAClusterImpl;
25+
import org.apache.hadoop.ozone.OmUtils;
26+
import org.apache.hadoop.ozone.om.OMConfigKeys;
27+
import org.apache.hadoop.ozone.web.ozShell.OzoneShell;
28+
import org.apache.hadoop.test.GenericTestUtils;
29+
import org.junit.After;
30+
import org.junit.AfterClass;
31+
import org.junit.Assert;
32+
import org.junit.Before;
33+
import org.junit.BeforeClass;
34+
import org.junit.Rule;
35+
import org.junit.Test;
36+
import org.junit.rules.Timeout;
37+
import org.slf4j.Logger;
38+
import org.slf4j.LoggerFactory;
39+
import picocli.CommandLine;
40+
import picocli.CommandLine.ExecutionException;
41+
import picocli.CommandLine.IExceptionHandler2;
42+
import picocli.CommandLine.ParameterException;
43+
import picocli.CommandLine.ParseResult;
44+
import picocli.CommandLine.RunLast;
45+
46+
import java.io.ByteArrayOutputStream;
47+
import java.io.File;
48+
import java.io.PrintStream;
49+
import java.util.Arrays;
50+
import java.util.Collection;
51+
import java.util.List;
52+
import java.util.UUID;
53+
54+
import static org.junit.Assert.fail;
55+
56+
/**
57+
* This class tests Ozone sh shell command.
58+
* Inspired by TestS3Shell
59+
*/
60+
public class TestOzoneShellHA {
61+
62+
private static final Logger LOG =
63+
LoggerFactory.getLogger(TestOzoneShellHA.class);
64+
65+
/**
66+
* Set the timeout for every test.
67+
*/
68+
@Rule
69+
public Timeout testTimeout = new Timeout(300000);
70+
71+
private static File baseDir;
72+
private static OzoneConfiguration conf = null;
73+
private static MiniOzoneCluster cluster = null;
74+
private static OzoneShell ozoneShell = null;
75+
76+
private final ByteArrayOutputStream out = new ByteArrayOutputStream();
77+
private final ByteArrayOutputStream err = new ByteArrayOutputStream();
78+
private static final PrintStream OLD_OUT = System.out;
79+
private static final PrintStream OLD_ERR = System.err;
80+
81+
private static String omServiceId;
82+
private static String clusterId;
83+
private static String scmId;
84+
private static int numOfOMs;
85+
86+
/**
87+
* Create a MiniOzoneCluster for testing with using distributed Ozone
88+
* handler type.
89+
*
90+
* @throws Exception
91+
*/
92+
@BeforeClass
93+
public static void init() throws Exception {
94+
conf = new OzoneConfiguration();
95+
96+
String path = GenericTestUtils.getTempPath(
97+
TestOzoneShellHA.class.getSimpleName());
98+
baseDir = new File(path);
99+
baseDir.mkdirs();
100+
ozoneShell = new OzoneShell();
101+
102+
// Init HA cluster
103+
omServiceId = "om-service-test1";
104+
numOfOMs = 3;
105+
clusterId = UUID.randomUUID().toString();
106+
scmId = UUID.randomUUID().toString();
107+
cluster = MiniOzoneCluster.newHABuilder(conf)
108+
.setClusterId(clusterId)
109+
.setScmId(scmId)
110+
.setOMServiceId(omServiceId)
111+
.setNumOfOzoneManagers(numOfOMs)
112+
.build();
113+
conf.setQuietMode(false);
114+
cluster.waitForClusterToBeReady();
115+
}
116+
117+
/**
118+
* shutdown MiniOzoneCluster.
119+
*/
120+
@AfterClass
121+
public static void shutdown() {
122+
if (cluster != null) {
123+
cluster.shutdown();
124+
}
125+
126+
if (baseDir != null) {
127+
FileUtil.fullyDelete(baseDir, true);
128+
}
129+
}
130+
131+
@Before
132+
public void setup() {
133+
System.setOut(new PrintStream(out));
134+
System.setErr(new PrintStream(err));
135+
}
136+
137+
@After
138+
public void reset() {
139+
// reset stream after each unit test
140+
out.reset();
141+
err.reset();
142+
143+
// restore system streams
144+
System.setOut(OLD_OUT);
145+
System.setErr(OLD_ERR);
146+
}
147+
148+
private void execute(OzoneShell shell, String[] args) {
149+
LOG.info("Executing OzoneShell command with args {}", Arrays.asList(args));
150+
CommandLine cmd = shell.getCmd();
151+
152+
IExceptionHandler2<List<Object>> exceptionHandler =
153+
new IExceptionHandler2<List<Object>>() {
154+
@Override
155+
public List<Object> handleParseException(ParameterException ex,
156+
String[] args) {
157+
throw ex;
158+
}
159+
160+
@Override
161+
public List<Object> handleExecutionException(ExecutionException ex,
162+
ParseResult parseRes) {
163+
throw ex;
164+
}
165+
};
166+
167+
// Since there is no elegant way to pass Ozone config to the shell,
168+
// the idea is to use 'set' to place those OM HA configs.
169+
String[] argsWithHAConf = getHASetConfStrings(args);
170+
171+
cmd.parseWithHandlers(new RunLast(), exceptionHandler, argsWithHAConf);
172+
}
173+
174+
/**
175+
* Execute command, assert exception message and returns true if error
176+
* was thrown.
177+
*/
178+
private void executeWithError(OzoneShell shell, String[] args,
179+
String expectedError) {
180+
if (Strings.isNullOrEmpty(expectedError)) {
181+
execute(shell, args);
182+
} else {
183+
try {
184+
execute(shell, args);
185+
fail("Exception is expected from command execution " + Arrays
186+
.asList(args));
187+
} catch (Exception ex) {
188+
if (!Strings.isNullOrEmpty(expectedError)) {
189+
Throwable exceptionToCheck = ex;
190+
if (exceptionToCheck.getCause() != null) {
191+
exceptionToCheck = exceptionToCheck.getCause();
192+
}
193+
Assert.assertTrue(
194+
String.format(
195+
"Error of OzoneShell code doesn't contain the " +
196+
"exception [%s] in [%s]",
197+
expectedError, exceptionToCheck.getMessage()),
198+
exceptionToCheck.getMessage().contains(expectedError));
199+
}
200+
}
201+
}
202+
}
203+
204+
/**
205+
* @return the leader OM's Node ID in the MiniOzoneHACluster.
206+
*
207+
* TODO: This should be put into MiniOzoneHAClusterImpl in the future.
208+
* This helper function is similar to the one in TestOzoneFsHAURLs.
209+
*/
210+
private String getLeaderOMNodeId() {
211+
Collection<String> omNodeIds = OmUtils.getOMNodeIds(conf, omServiceId);
212+
assert(omNodeIds.size() == numOfOMs);
213+
MiniOzoneHAClusterImpl haCluster = (MiniOzoneHAClusterImpl) cluster;
214+
// Note: this loop may be implemented inside MiniOzoneHAClusterImpl
215+
for (String omNodeId : omNodeIds) {
216+
// Find the leader OM
217+
if (!haCluster.getOzoneManager(omNodeId).isLeader()) {
218+
continue;
219+
}
220+
return omNodeId;
221+
}
222+
return null;
223+
}
224+
225+
private String getSetConfStringFromConf(String key) {
226+
return String.format("--set=%s=%s", key, conf.get(key));
227+
}
228+
229+
private String generateSetConfString(String key, String value) {
230+
return String.format("--set=%s=%s", key, value);
231+
}
232+
233+
/**
234+
* Helper function to get a String array to be fed into OzoneShell.
235+
* @param numOfArgs Additional number of arguments after the HA conf string,
236+
* this translates into the number of empty array elements
237+
* after the HA conf string.
238+
* @return String array.
239+
*/
240+
private String[] getHASetConfStrings(int numOfArgs) {
241+
assert(numOfArgs >= 0);
242+
String[] res = new String[1 + 1 + numOfOMs + numOfArgs];
243+
final int indexOmServiceIds = 0;
244+
final int indexOmNodes = 1;
245+
final int indexOmAddressStart = 2;
246+
247+
res[indexOmServiceIds] = getSetConfStringFromConf(
248+
OMConfigKeys.OZONE_OM_SERVICE_IDS_KEY);
249+
250+
String omNodesKey = OmUtils.addKeySuffixes(
251+
OMConfigKeys.OZONE_OM_NODES_KEY, omServiceId);
252+
String omNodesVal = conf.get(omNodesKey);
253+
res[indexOmNodes] = generateSetConfString(omNodesKey, omNodesVal);
254+
255+
String[] omNodesArr = omNodesVal.split(",");
256+
// Sanity check
257+
assert(omNodesArr.length == numOfOMs);
258+
for (int i = 0; i < numOfOMs; i++) {
259+
res[indexOmAddressStart + i] =
260+
getSetConfStringFromConf(OmUtils.addKeySuffixes(
261+
OMConfigKeys.OZONE_OM_ADDRESS_KEY, omServiceId, omNodesArr[i]));
262+
}
263+
264+
return res;
265+
}
266+
267+
/**
268+
* Helper function to create a new set of arguments that contains HA configs.
269+
* @param existingArgs Existing arguments to be fed into OzoneShell command.
270+
* @return String array.
271+
*/
272+
private String[] getHASetConfStrings(String[] existingArgs) {
273+
// Get a String array populated with HA configs first
274+
String[] res = getHASetConfStrings(existingArgs.length);
275+
276+
int indexCopyStart = res.length - existingArgs.length;
277+
// Then copy the existing args to the returned String array
278+
for (int i = 0; i < existingArgs.length; i++) {
279+
res[indexCopyStart + i] = existingArgs[i];
280+
}
281+
return res;
282+
}
283+
284+
/**
285+
* Tests ozone sh command URI parsing with volume and bucket create commands.
286+
*/
287+
@Test
288+
public void testOzoneShCmdURIs() {
289+
// Test case 1: ozone sh volume create /volume
290+
// Expectation: Failure.
291+
String[] args = new String[] {"volume", "create", "/volume"};
292+
executeWithError(ozoneShell, args,
293+
"Service ID or host name must not be omitted");
294+
295+
// Get leader OM node RPC address from ozone.om.address.omServiceId.omNode
296+
String omLeaderNodeId = getLeaderOMNodeId();
297+
String omLeaderNodeAddrKey = OmUtils.addKeySuffixes(
298+
OMConfigKeys.OZONE_OM_ADDRESS_KEY, omServiceId, omLeaderNodeId);
299+
String omLeaderNodeAddr = conf.get(omLeaderNodeAddrKey);
300+
String omLeaderNodeAddrWithoutPort = omLeaderNodeAddr.split(":")[0];
301+
302+
// Test case 2: ozone sh volume create o3://om1/volume2
303+
// Expectation: Success.
304+
// Note: For now it seems OzoneShell is only trying the default port 9862
305+
// instead of using the port defined in ozone.om.address (as ozone fs does).
306+
// So the test will fail before this behavior is fixed.
307+
// TODO: Fix this behavior, then uncomment the execute() below.
308+
String setOmAddress = "--set=" + OMConfigKeys.OZONE_OM_ADDRESS_KEY + "="
309+
+ omLeaderNodeAddr;
310+
args = new String[] {setOmAddress,
311+
"volume", "create", "o3://" + omLeaderNodeAddrWithoutPort + "/volume2"};
312+
//execute(ozoneShell, args);
313+
314+
// Test case 3: ozone sh volume create o3://om1:port/volume3
315+
// Expectation: Success.
316+
args = new String[] {
317+
"volume", "create", "o3://" + omLeaderNodeAddr + "/volume3"};
318+
execute(ozoneShell, args);
319+
320+
// Test case 4: ozone sh volume create o3://id1/volume
321+
// Expectation: Success.
322+
args = new String[] {"volume", "create", "o3://" + omServiceId + "/volume"};
323+
execute(ozoneShell, args);
324+
325+
// Test case 5: ozone sh volume create o3://id1:port/volume
326+
// Expectation: Failure.
327+
args = new String[] {"volume", "create",
328+
"o3://" + omServiceId + ":9862" + "/volume"};
329+
executeWithError(ozoneShell, args, "does not use port information");
330+
331+
// Test case 6: ozone sh bucket create /volume/bucket
332+
// Expectation: Failure.
333+
args = new String[] {"bucket", "create", "/volume/bucket"};
334+
executeWithError(ozoneShell, args,
335+
"Service ID or host name must not be omitted");
336+
337+
// Test case 7: ozone sh bucket create o3://om1/volume/bucket
338+
// Expectation: Success.
339+
args = new String[] {
340+
"bucket", "create", "o3://" + omServiceId + "/volume/bucket"};
341+
execute(ozoneShell, args);
342+
}
343+
}

‎hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/web/ozShell/OzoneAddress.java‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.net.URISyntaxException;
2323

2424
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
25+
import org.apache.hadoop.ozone.OmUtils;
2526
import org.apache.hadoop.ozone.client.OzoneClient;
2627
import org.apache.hadoop.ozone.client.OzoneClientException;
2728
import org.apache.hadoop.ozone.client.OzoneClientFactory;
@@ -92,13 +93,27 @@ public OzoneClient createClient(OzoneConfiguration conf)
9293
} else if (scheme.equals(OZONE_RPC_SCHEME)) {
9394
if (ozoneURI.getHost() != null && !ozoneURI.getAuthority()
9495
.equals(EMPTY_HOST)) {
95-
if (ozoneURI.getPort() == -1) {
96+
if (OmUtils.isOmHAServiceId(conf, ozoneURI.getHost())) {
97+
// When host is an HA service ID
98+
if (ozoneURI.getPort() != -1) {
99+
throw new OzoneClientException(
100+
"Port " + ozoneURI.getPort() + " specified in URI but host '"
101+
+ ozoneURI.getHost() + "' is a logical (HA) OzoneManager "
102+
+ "and does not use port information.");
103+
}
104+
client = OzoneClientFactory.getRpcClient(ozoneURI.getHost(), conf);
105+
} else if (ozoneURI.getPort() == -1) {
96106
client = OzoneClientFactory.getRpcClient(ozoneURI.getHost());
97107
} else {
98108
client = OzoneClientFactory
99109
.getRpcClient(ozoneURI.getHost(), ozoneURI.getPort(), conf);
100110
}
101111
} else {
112+
// When host is not specified
113+
if (OmUtils.isServiceIdsDefined(conf)) {
114+
throw new OzoneClientException("Service ID or host name must not"
115+
+ " be omitted when ozone.om.service.ids is defined.");
116+
}
102117
client = OzoneClientFactory.getRpcClient(conf);
103118
}
104119
} else {

0 commit comments

Comments
 (0)