AMBARI-7210. BE: Stack advisor scripts should report python errors in API

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));
     }
 

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);
+  }
+}

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());
     }
   }
 

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());
     }
   }
 

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>() {

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>() {

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)
 

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");
+    }
   }
 
 }

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());