AMBARI-7210. BE: Stack advisor scripts should report python errors in API
Project: http://git-wip-us.apache.org/repos/asf/ambari/repo Commit: http://git-wip-us.apache.org/repos/asf/ambari/commit/d49c1964 Tree: http://git-wip-us.apache.org/repos/asf/ambari/tree/d49c1964 Diff: http://git-wip-us.apache.org/repos/asf/ambari/diff/d49c1964 Branch: refs/heads/branch-alerts-dev Commit: d49c19645d9ef134491b0c225a9235def1a5ac99 Parents: 32cb4c5 Author: Srimanth Gunturi <sgunt...@hortonworks.com> Authored: Mon Sep 8 13:01:59 2014 -0700 Committer: Srimanth Gunturi <sgunt...@hortonworks.com> Committed: Mon Sep 8 13:20:04 2014 -0700 ---------------------------------------------------------------------- .../stackadvisor/StackAdvisorHelper.java | 6 +-- .../StackAdvisorRequestException.java | 27 ++++++++++++ .../stackadvisor/StackAdvisorRunner.java | 34 +++++++++++---- .../commands/StackAdvisorCommand.java | 14 +++---- .../RecommendationResourceProvider.java | 9 +++- .../internal/ValidationResourceProvider.java | 9 +++- .../src/main/resources/scripts/stack_advisor.py | 8 +++- .../stackadvisor/StackAdvisorRunnerTest.java | 44 +++++++++++++------- .../commands/StackAdvisorCommandTest.java | 29 ++++++------- 9 files changed, 124 insertions(+), 56 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ambari/blob/d49c1964/ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorHelper.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorHelper.java b/ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorHelper.java index 60cdd52..9e683f0 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorHelper.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorHelper.java @@ -59,7 +59,7 @@ public class StackAdvisorHelper { * Returns validation (component-layout or configurations) result for the * request. * - * @param validationRequest the validation request + * @param request the validation request * @return {@link ValidationResponse} instance * @throws StackAdvisorException in case of stack advisor script errors */ @@ -83,7 +83,7 @@ public class StackAdvisorHelper { command = new GetConfigurationValidationCommand(recommendationsDir, stackAdvisorScript, requestId, saRunner, metaInfo); } else { - throw new StackAdvisorException(String.format("Unsupported request type, type=%s", + throw new StackAdvisorRequestException(String.format("Unsupported request type, type=%s", requestType)); } @@ -118,7 +118,7 @@ public class StackAdvisorHelper { command = new GetConfigurationRecommnedationCommand(recommendationsDir, stackAdvisorScript, requestId, saRunner, metaInfo); } else { - throw new StackAdvisorException(String.format("Unsupported request type, type=%s", + throw new StackAdvisorRequestException(String.format("Unsupported request type, type=%s", requestType)); } http://git-wip-us.apache.org/repos/asf/ambari/blob/d49c1964/ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorRequestException.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorRequestException.java b/ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorRequestException.java new file mode 100644 index 0000000..93e6d7b --- /dev/null +++ b/ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorRequestException.java @@ -0,0 +1,27 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ambari.server.api.services.stackadvisor; + +@SuppressWarnings("serial") +public class StackAdvisorRequestException extends StackAdvisorException { + + public StackAdvisorRequestException(String message) { + super(message); + } +} http://git-wip-us.apache.org/repos/asf/ambari/blob/d49c1964/ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorRunner.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorRunner.java b/ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorRunner.java index c57f54e..ee7dcc2 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorRunner.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorRunner.java @@ -41,10 +41,9 @@ public class StackAdvisorRunner { * @param script stack advisor script * @param saCommandType {@link StackAdvisorCommandType} to run. * @param actionDirectory directory for the action - * @return {@code true} if script completed successfully, {@code false} - * otherwise. */ - public boolean runScript(String script, StackAdvisorCommandType saCommandType, File actionDirectory) { + public void runScript(String script, StackAdvisorCommandType saCommandType, File actionDirectory) + throws StackAdvisorException { LOG.info(String.format("Script=%s, actionDirectory=%s, command=%s", script, actionDirectory, saCommandType)); @@ -62,23 +61,40 @@ public class StackAdvisorRunner { LOG.info("Stack-advisor output={}, error={}", outputFile, errorFile); int exitCode = process.waitFor(); + String outMessage; + String errMessage = null; try { - String outMessage = FileUtils.readFileToString(new File(outputFile)); - String errMessage = FileUtils.readFileToString(new File(errorFile)); + outMessage = FileUtils.readFileToString(new File(outputFile)).trim(); + errMessage = FileUtils.readFileToString(new File(errorFile)).trim(); LOG.info("Stack advisor output files"); LOG.info(" advisor script stdout: {}", outMessage); LOG.info(" advisor script stderr: {}", errMessage); } catch (IOException io) { LOG.error("Error in reading script log files", io); } - - return exitCode == 0; + if (exitCode > 0) { + String errorMessage; + if (errMessage != null) { + errorMessage = errMessage.substring(errMessage.lastIndexOf("\n")); + } else { + errorMessage = "Error occurred during stack advisor execution"; + } + switch (exitCode) { + case 1: + throw new StackAdvisorRequestException(errorMessage); + case 2: + throw new StackAdvisorException(errorMessage); + } + } } finally { process.destroy(); } + } catch (StackAdvisorException ex) { + throw ex; } catch (Exception ioe) { - LOG.error("Error executing stack advisor", ioe); - return false; + String message = "Error executing stack advisor: "; + LOG.error(message, ioe); + throw new StackAdvisorException(message + ioe.getMessage()); } } http://git-wip-us.apache.org/repos/asf/ambari/blob/d49c1964/ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/commands/StackAdvisorCommand.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/commands/StackAdvisorCommand.java b/ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/commands/StackAdvisorCommand.java index bc98c4c..6353978 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/commands/StackAdvisorCommand.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/api/services/stackadvisor/commands/StackAdvisorCommand.java @@ -224,21 +224,17 @@ public abstract class StackAdvisorCommand<T extends StackAdvisorResponse> extend FileUtils.writeStringToFile(new File(requestDirectory, "services.json"), adjusted.servicesJSON); - boolean success = saRunner.runScript(stackAdvisorScript, getCommandType(), requestDirectory); - if (!success) { - String message = "Stack advisor script finished with errors"; - LOG.warn(message); - throw new StackAdvisorException(message); - } - + saRunner.runScript(stackAdvisorScript, getCommandType(), requestDirectory); String result = FileUtils.readFileToString(new File(requestDirectory, getResultFileName())); T response = this.mapper.readValue(result, this.type); return updateResponse(request, setRequestId(response)); + } catch (StackAdvisorException ex) { + throw ex; } catch (Exception e) { - String message = "Error occured during stack advisor command invocation"; + String message = "Error occured during stack advisor command invocation: "; LOG.warn(message, e); - throw new StackAdvisorException(message, e); + throw new StackAdvisorException(message + e.getMessage()); } } http://git-wip-us.apache.org/repos/asf/ambari/blob/d49c1964/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/RecommendationResourceProvider.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/RecommendationResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/RecommendationResourceProvider.java index 0463076..b722825 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/RecommendationResourceProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/RecommendationResourceProvider.java @@ -33,6 +33,7 @@ import javax.ws.rs.core.Response.Status; import org.apache.ambari.server.AmbariException; import org.apache.ambari.server.api.services.stackadvisor.StackAdvisorException; import org.apache.ambari.server.api.services.stackadvisor.StackAdvisorRequest; +import org.apache.ambari.server.api.services.stackadvisor.StackAdvisorRequestException; import org.apache.ambari.server.api.services.stackadvisor.recommendations.RecommendationResponse; import org.apache.ambari.server.api.services.stackadvisor.recommendations.RecommendationResponse.BindingHostGroup; import org.apache.ambari.server.api.services.stackadvisor.recommendations.RecommendationResponse.HostGroup; @@ -90,10 +91,14 @@ public class RecommendationResourceProvider extends StackAdvisorResourceProvider final RecommendationResponse response; try { response = saHelper.recommend(recommendationRequest); - } catch (StackAdvisorException e) { - LOG.warn("Error occured during component-layout recommnedation", e); + } catch (StackAdvisorRequestException e) { + LOG.warn("Error occured during recommnedation", e); throw new WebApplicationException(Response.status(Status.BAD_REQUEST).entity(e.getMessage()) .build()); + } catch (StackAdvisorException e) { + LOG.warn("Error occured during recommnedation", e); + throw new WebApplicationException(Response.status(Status.INTERNAL_SERVER_ERROR).entity(e.getMessage()) + .build()); } Resource recommendation = createResources(new Command<Resource>() { http://git-wip-us.apache.org/repos/asf/ambari/blob/d49c1964/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ValidationResourceProvider.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ValidationResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ValidationResourceProvider.java index 017abae..941fb19 100644 --- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ValidationResourceProvider.java +++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ValidationResourceProvider.java @@ -33,6 +33,7 @@ import javax.ws.rs.core.Response.Status; import org.apache.ambari.server.AmbariException; import org.apache.ambari.server.api.services.stackadvisor.StackAdvisorException; import org.apache.ambari.server.api.services.stackadvisor.StackAdvisorRequest; +import org.apache.ambari.server.api.services.stackadvisor.StackAdvisorRequestException; import org.apache.ambari.server.api.services.stackadvisor.validations.ValidationResponse; import org.apache.ambari.server.api.services.stackadvisor.validations.ValidationResponse.ValidationItem; import org.apache.ambari.server.controller.AmbariManagementController; @@ -82,10 +83,14 @@ public class ValidationResourceProvider extends StackAdvisorResourceProvider { final ValidationResponse response; try { response = saHelper.validate(validationRequest); - } catch (StackAdvisorException e) { - LOG.warn("Error occured during component-layout validation", e); + } catch (StackAdvisorRequestException e) { + LOG.warn("Error occurred during validation", e); throw new WebApplicationException(Response.status(Status.BAD_REQUEST).entity(e.getMessage()) .build()); + } catch (StackAdvisorException e) { + LOG.warn("Error occurred during validation", e); + throw new WebApplicationException(Response.status(Status.INTERNAL_SERVER_ERROR).entity(e.getMessage()) + .build()); } Resource validation = createResources(new Command<Resource>() { http://git-wip-us.apache.org/repos/asf/ambari/blob/d49c1964/ambari-server/src/main/resources/scripts/stack_advisor.py ---------------------------------------------------------------------- diff --git a/ambari-server/src/main/resources/scripts/stack_advisor.py b/ambari-server/src/main/resources/scripts/stack_advisor.py index 566fe7f..2293d99 100755 --- a/ambari-server/src/main/resources/scripts/stack_advisor.py +++ b/ambari-server/src/main/resources/scripts/stack_advisor.py @@ -145,8 +145,12 @@ def instantiateStackAdvisor(stackName, stackVersion, parentVersions): if __name__ == '__main__': try: main(sys.argv) - except Exception, e: + except StackAdvisorException as stack_exception: traceback.print_exc() - print "Error occured in stack advisor.\nError details: {0}".format(str(e)) + print "Error occured in stack advisor.\nError details: {0}".format(str(stack_exception)) sys.exit(1) + except Exception as e: + traceback.print_exc() + print "Error occured in stack advisor.\nError details: {0}".format(str(e)) + sys.exit(2) http://git-wip-us.apache.org/repos/asf/ambari/blob/d49c1964/ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorRunnerTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorRunnerTest.java b/ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorRunnerTest.java index 6afe5c4..0e49b7f 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorRunnerTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/StackAdvisorRunnerTest.java @@ -20,6 +20,7 @@ package org.apache.ambari.server.api.services.stackadvisor; import static org.easymock.EasyMock.expect; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; import static org.powermock.api.easymock.PowerMock.createNiceMock; import static org.powermock.api.easymock.PowerMock.replay; import static org.powermock.api.support.membermodification.MemberModifier.stub; @@ -56,8 +57,8 @@ public class StackAdvisorRunnerTest { temp.delete(); } - @Test - public void testRunScript_processStartThrowsException_returnFalse() throws IOException { + @Test(expected = StackAdvisorException.class) + public void testRunScript_processStartThrowsException_returnFalse() throws Exception { String script = "echo"; StackAdvisorCommandType saCommandType = StackAdvisorCommandType.RECOMMEND_COMPONENT_LAYOUT; File actionDirectory = temp.newFolder("actionDir"); @@ -68,14 +69,11 @@ public class StackAdvisorRunnerTest { .toReturn(processBuilder); expect(processBuilder.start()).andThrow(new IOException()); replay(processBuilder); - boolean result = saRunner.runScript(script, saCommandType, actionDirectory); - - assertEquals(false, result); + saRunner.runScript(script, saCommandType, actionDirectory); } - @Test - public void testRunScript_processExitCodeNonZero_returnFalse() throws IOException, - InterruptedException { + @Test(expected = StackAdvisorRequestException.class) + public void testRunScript_processExitCode1_returnFalse() throws Exception { String script = "echo"; StackAdvisorCommandType saCommandType = StackAdvisorCommandType.RECOMMEND_COMPONENT_LAYOUT; File actionDirectory = temp.newFolder("actionDir"); @@ -88,14 +86,28 @@ public class StackAdvisorRunnerTest { expect(processBuilder.start()).andReturn(process); expect(process.waitFor()).andReturn(1); replay(processBuilder, process); - boolean result = saRunner.runScript(script, saCommandType, actionDirectory); + saRunner.runScript(script, saCommandType, actionDirectory); + } + + @Test(expected = StackAdvisorException.class) + public void testRunScript_processExitCode2_returnFalse() throws Exception { + String script = "echo"; + StackAdvisorCommandType saCommandType = StackAdvisorCommandType.RECOMMEND_COMPONENT_LAYOUT; + File actionDirectory = temp.newFolder("actionDir"); + ProcessBuilder processBuilder = createNiceMock(ProcessBuilder.class); + Process process = createNiceMock(Process.class); + StackAdvisorRunner saRunner = new StackAdvisorRunner(); - assertEquals(false, result); + stub(PowerMock.method(StackAdvisorRunner.class, "prepareShellCommand")) + .toReturn(processBuilder); + expect(processBuilder.start()).andReturn(process); + expect(process.waitFor()).andReturn(2); + replay(processBuilder, process); + saRunner.runScript(script, saCommandType, actionDirectory); } @Test - public void testRunScript_processExitCodeZero_returnTrue() throws IOException, - InterruptedException { + public void testRunScript_processExitCodeZero_returnTrue() throws Exception { String script = "echo"; StackAdvisorCommandType saCommandType = StackAdvisorCommandType.RECOMMEND_COMPONENT_LAYOUT; File actionDirectory = temp.newFolder("actionDir"); @@ -108,9 +120,11 @@ public class StackAdvisorRunnerTest { expect(processBuilder.start()).andReturn(process); expect(process.waitFor()).andReturn(0); replay(processBuilder, process); - boolean result = saRunner.runScript(script, saCommandType, actionDirectory); - - assertEquals(true, result); + try { + saRunner.runScript(script, saCommandType, actionDirectory); + } catch (StackAdvisorException ex) { + fail("Should not fail with StackAdvisorException"); + } } } http://git-wip-us.apache.org/repos/asf/ambari/blob/d49c1964/ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/commands/StackAdvisorCommandTest.java ---------------------------------------------------------------------- diff --git a/ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/commands/StackAdvisorCommandTest.java b/ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/commands/StackAdvisorCommandTest.java index 1a19c46..3f21bce 100644 --- a/ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/commands/StackAdvisorCommandTest.java +++ b/ambari-server/src/test/java/org/apache/ambari/server/api/services/stackadvisor/commands/StackAdvisorCommandTest.java @@ -23,6 +23,7 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyString; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; @@ -41,6 +42,7 @@ import org.apache.ambari.server.api.services.AmbariMetaInfo; import org.apache.ambari.server.api.services.stackadvisor.StackAdvisorException; import org.apache.ambari.server.api.services.stackadvisor.StackAdvisorRequest; import org.apache.ambari.server.api.services.stackadvisor.StackAdvisorRequest.StackAdvisorRequestBuilder; +import org.apache.ambari.server.api.services.stackadvisor.StackAdvisorRequestException; import org.apache.ambari.server.api.services.stackadvisor.StackAdvisorResponse; import org.apache.ambari.server.api.services.stackadvisor.StackAdvisorRunner; import org.apache.ambari.server.api.services.stackadvisor.commands.StackAdvisorCommand.StackAdvisorData; @@ -113,8 +115,8 @@ public class StackAdvisorCommandTest { doReturn(servicesJSON).when(command).getServicesInformation(request); doReturn(data).when(command) .adjust(any(StackAdvisorData.class), any(StackAdvisorRequest.class)); - when(saRunner.runScript(any(String.class), any(StackAdvisorCommandType.class), any(File.class))) - .thenReturn(false); + doThrow(new StackAdvisorRequestException("error")).when(saRunner) + .runScript(any(String.class), any(StackAdvisorCommandType.class), any(File.class)); command.invoke(request); assertTrue(false); @@ -138,8 +140,8 @@ public class StackAdvisorCommandTest { doReturn("{\"services\" : \"HDFS\"").when(command).getServicesInformation(request); doThrow(new WebApplicationException()).when(command).adjust(any(StackAdvisorData.class), any(StackAdvisorRequest.class)); - when(saRunner.runScript(any(String.class), any(StackAdvisorCommandType.class), any(File.class))) - .thenReturn(false); + doThrow(new StackAdvisorException("error")).when(saRunner) + .runScript(any(String.class), any(StackAdvisorCommandType.class), any(File.class)); command.invoke(request); assertTrue(false); @@ -168,16 +170,15 @@ public class StackAdvisorCommandTest { doReturn(servicesJSON).when(command).getServicesInformation(request); doReturn(data).when(command) .adjust(any(StackAdvisorData.class), any(StackAdvisorRequest.class)); - when(saRunner.runScript(any(String.class), any(StackAdvisorCommandType.class), any(File.class))) - .thenAnswer(new Answer<Boolean>() { - public Boolean answer(InvocationOnMock invocation) throws Throwable { - String resultFilePath = String.format("%s/%s", requestId, command.getResultFileName()); - File resultFile = new File(recommendationsDir, resultFilePath); - resultFile.getParentFile().mkdirs(); - FileUtils.writeStringToFile(resultFile, testResourceString); - return true; - } - }); + doAnswer(new Answer() { + public Object answer(InvocationOnMock invocation) throws Throwable { + String resultFilePath = String.format("%s/%s", requestId, command.getResultFileName()); + File resultFile = new File(recommendationsDir, resultFilePath); + resultFile.getParentFile().mkdirs(); + FileUtils.writeStringToFile(resultFile, testResourceString); + return null; + } + }).when(saRunner).runScript(any(String.class), any(StackAdvisorCommandType.class), any(File.class)); TestResource result = command.invoke(request); assertEquals(expected, result.getType());