You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by kh...@apache.org on 2017/06/09 14:19:30 UTC

geode git commit: GEODE-2420: add file-size-limit param to the ExportLogsController

Repository: geode
Updated Branches:
  refs/heads/develop 88825071f -> a658322c9


GEODE-2420: add file-size-limit param to the ExportLogsController

Refactored size and warning for exported logs to be based on fully
expanded size of the exported files


Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/a658322c
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/a658322c
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/a658322c

Branch: refs/heads/develop
Commit: a658322c9994f5f5d69a5b73ed4b67359dbad8f8
Parents: 8882507
Author: Ken Howe <kh...@pivotal.io>
Authored: Mon Jun 5 08:43:39 2017 -0700
Committer: Ken Howe <kh...@pivotal.io>
Committed: Fri Jun 9 07:17:22 2017 -0700

----------------------------------------------------------------------
 .../cli/commands/ExportLogsCommand.java         | 102 ++++----------
 .../cli/functions/SizeExportLogsFunction.java   |   3 +-
 .../internal/cli/i18n/CliStrings.java           |   2 +-
 .../web/controllers/ExportLogController.java    |   5 +-
 .../cli/commands/ExportLogsCommandTest.java     | 141 +++++++++++++++----
 .../cli/commands/ExportLogsDUnitTest.java       |  10 --
 .../commands/ExportLogsFileSizeLimitTest.java   |  87 ------------
 .../cli/commands/ExportLogsStatsDUnitTest.java  |   9 ++
 .../cli/commands/ExportLogsTestSuite.java       |   7 +-
 .../functions/SizeExportLogsFunctionTest.java   |   7 +-
 10 files changed, 162 insertions(+), 211 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/geode/blob/a658322c/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
index 0ff780c..3d68788 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommand.java
@@ -30,7 +30,6 @@ import org.apache.geode.management.internal.cli.functions.ExportLogsFunction;
 import org.apache.geode.management.internal.cli.functions.SizeExportLogsFunction;
 import org.apache.geode.management.internal.cli.i18n.CliStrings;
 import org.apache.geode.management.internal.cli.result.ResultBuilder;
-import org.apache.geode.management.internal.cli.util.BytesToString;
 import org.apache.geode.management.internal.cli.util.ExportLogsCacheWriter;
 import org.apache.geode.management.internal.configuration.utils.ZipUtils;
 import org.apache.geode.management.internal.security.ResourceOperation;
@@ -39,7 +38,6 @@ import org.apache.logging.log4j.Logger;
 import org.springframework.shell.core.annotation.CliCommand;
 import org.springframework.shell.core.annotation.CliOption;
 
-import java.io.File;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -113,32 +111,41 @@ public class ExportLogsCommand implements GfshCommand {
         return ResultBuilder.createUserErrorResult(CliStrings.NO_MEMBERS_FOUND_MESSAGE);
       }
 
-      if (parseFileSizeLimit(fileSizeLimit) > 0) {
+      long userSpecifiedLimit = parseFileSizeLimit(fileSizeLimit);
+      if (userSpecifiedLimit > 0) {
         // Get estimated size of exported logs from all servers before exporting anything
         for (DistributedMember server : targetMembers) {
           SizeExportLogsFunction.Args args = new SizeExportLogsFunction.Args(start, end, logLevel,
               onlyLogLevel, logsOnly, statsOnly);
 
           List<Object> results = (List<Object>) estimateLogSize(args, server).getResult();
-          long estimatedSize = 0;
           if (!results.isEmpty()) {
-            List<?> res = (List<?>) results.get(0);
-            if (res.get(0) instanceof Long) {
-              estimatedSize = (Long) res.get(0);
+            if (results.get(0) instanceof Long) {
+              long estimatedSize = (Long) results.get(0);
+              logger.info("Received estimated export size from member {}: {}", server.getId(),
+                  estimatedSize);
+              totalEstimatedExportSize += estimatedSize;
+            } else if (results.get(0) instanceof ManagementException) {
+              ManagementException exception = (ManagementException) results.get(0);
+              return ResultBuilder.createUserErrorResult(exception.getMessage());
             }
           }
-          logger.info("Received estimated export size from member {}: {}", server.getId(),
-              estimatedSize);
-          totalEstimatedExportSize += estimatedSize;
         }
 
-        // The sum of the estimated export sizes from each member should not exceed the
-        // disk available on the locator
-        try {
-          checkIfExportLogsOverflowsDisk("locator", parseFileSizeLimit(fileSizeLimit),
-              totalEstimatedExportSize, getLocalDiskAvailable());
-        } catch (ManagementException e) {
-          return ResultBuilder.createUserErrorResult(e.getMessage());
+        // first check if totalEstimate file size exceeds available disk space on locator
+        if (totalEstimatedExportSize > getLocalDiskAvailable()) {
+          return ResultBuilder.createUserErrorResult(
+              "Estimated logs size will exceed the available disk space on the locator.");
+        }
+        // then check if total estimated file size exceeds user specified value
+        if (totalEstimatedExportSize > userSpecifiedLimit) {
+          StringBuilder sb = new StringBuilder();
+          sb.append("Estimated exported logs expanded file size = ")
+              .append(totalEstimatedExportSize).append(", ")
+              .append(CliStrings.EXPORT_LOGS__FILESIZELIMIT).append(" = ")
+              .append(userSpecifiedLimit).append(
+                  ". To disable exported logs file size check use option \"--file-size-limit=0\".");
+          return ResultBuilder.createUserErrorResult(sb.toString());
         }
       }
 
@@ -193,14 +200,8 @@ public class ExportLogsCommand implements GfshCommand {
 
       logger.info("Zipping into: " + exportedLogsZipFile.toString());
       ZipUtils.zipDirectory(exportedLogsDir, exportedLogsZipFile);
-      try {
-        checkFileSizeWithinLimit(parseFileSizeLimit(fileSizeLimit), exportedLogsZipFile.toFile());
-      } catch (ManagementException e) {
-        FileUtils.deleteQuietly(exportedLogsZipFile.toFile());
-        return ResultBuilder.createUserErrorResult(e.getMessage());
-      } finally {
-        FileUtils.deleteDirectory(tempDir.toFile());
-      }
+      FileUtils.deleteDirectory(tempDir.toFile());
+
       result = ResultBuilder.createInfoResult(exportedLogsZipFile.toString());
     } catch (Exception ex) {
       logger.error(ex.getMessage(), ex);
@@ -208,7 +209,9 @@ public class ExportLogsCommand implements GfshCommand {
     } finally {
       ExportLogsFunction.destroyExportLogsRegion(cache);
     }
-    logger.debug("Exporting logs returning = {}", result);
+    if (logger.isDebugEnabled()) {
+      logger.debug("Exporting logs returning = {}", result);
+    }
     return result;
   }
 
@@ -229,13 +232,6 @@ public class ExportLogsCommand implements GfshCommand {
   /**
    * Wrapper to enable stubbing of static method call for unit testing
    */
-  long getLocalDiskSize() {
-    return FileUtils.getUserDirectory().getTotalSpace();
-  }
-
-  /**
-   * Wrapper to enable stubbing of static method call for unit testing
-   */
   long getLocalDiskAvailable() {
     return FileUtils.getUserDirectory().getUsableSpace();
   }
@@ -243,7 +239,7 @@ public class ExportLogsCommand implements GfshCommand {
   /**
    * Returns file size limit in bytes
    */
-  private long parseFileSizeLimit(String fileSizeLimit) {
+  long parseFileSizeLimit(String fileSizeLimit) {
     if (StringUtils.isEmpty(fileSizeLimit)) {
       return 0;
     }
@@ -254,44 +250,6 @@ public class ExportLogsCommand implements GfshCommand {
     return sizeLimit * byteMultiplier;
   }
 
-  /**
-   * @throws ManagementException if checking is enabled (fileSizeLimit > 0) and file size is over
-   *         fileSizeLimit bytes
-   */
-  void checkFileSizeWithinLimit(long fileSizeLimitBytes, File file) {
-    if (fileSizeLimitBytes > 0) {
-      if (FileUtils.sizeOf(file) > fileSizeLimitBytes) {
-        StringBuilder sb = new StringBuilder();
-        sb.append("Exported logs zip file size = ").append(FileUtils.sizeOf(file)).append(", ")
-            .append(CliStrings.EXPORT_LOGS__FILESIZELIMIT).append(" = ").append(fileSizeLimitBytes)
-            .append(
-                ". To disable exported logs file size check use option \"--file-size-limit=0\".");
-        throw new ManagementException(sb.toString()); // FileTooBigException
-      }
-    }
-  }
-
-
-  /**
-   * @throws ManagementException if export file size checking is enabled (fileSizeLimit > 0) and the
-   *         space required on a cluster member to filter and zip up files to be exported exceeds
-   *         the disk space available
-   */
-  void checkIfExportLogsOverflowsDisk(String memberName, long fileSizeLimitBytes,
-      long estimatedSize, long diskAvailable) {
-    if (fileSizeLimitBytes > 0) {
-      StringBuilder sb = new StringBuilder();
-      BytesToString bytesToString = new BytesToString();
-      if (estimatedSize > diskAvailable) {
-        sb.append("Estimated disk space required (").append(bytesToString.of(estimatedSize))
-            .append(") to consolidate logs on member ").append(memberName)
-            .append(" will exceed available disk space (").append(bytesToString.of(diskAvailable))
-            .append(")");
-        throw new ManagementException(sb.toString()); // FileTooBigException
-      }
-    }
-  }
-
   static int parseSize(String diskSpaceLimit) {
     Matcher matcher = DISK_SPACE_LIMIT_PATTERN.matcher(diskSpaceLimit);
     if (matcher.matches()) {

http://git-wip-us.apache.org/repos/asf/geode/blob/a658322c/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunction.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunction.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunction.java
index 57355c0..9d1eff5 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunction.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunction.java
@@ -16,7 +16,6 @@ package org.apache.geode.management.internal.cli.functions;
 
 import java.io.File;
 import java.io.IOException;
-import java.util.Arrays;
 
 import org.apache.geode.management.ManagementException;
 import org.apache.geode.management.internal.cli.util.BytesToString;
@@ -49,7 +48,7 @@ public class SizeExportLogsFunction extends ExportLogsFunction implements Functi
 
       BytesToString bytesToString = new BytesToString();
       if (estimatedSize == 0 || estimatedSize < diskAvailable) {
-        context.getResultSender().lastResult(Arrays.asList(estimatedSize));
+        context.getResultSender().lastResult(estimatedSize);
       } else {
         StringBuilder sb = new StringBuilder().append("Estimated disk space required (")
             .append(bytesToString.of(estimatedSize)).append(") to consolidate logs on member ")

http://git-wip-us.apache.org/repos/asf/geode/blob/a658322c/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java b/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
index 9f68d3a..a7a3742 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/cli/i18n/CliStrings.java
@@ -1442,7 +1442,7 @@ public class CliStrings {
   public static final String EXPORT_LOGS__STATSONLY__HELP = "Whether to only export statistics";
   public static final String EXPORT_LOGS__FILESIZELIMIT = "file-size-limit";
   public static final String EXPORT_LOGS__FILESIZELIMIT__HELP =
-      "Limits size of the file that can be exported. Specify zero for no limit. Value is in megabytes by default or [k|m|g|t] may be specified.";
+      "Limits total unzipped size of the exported files. Specify zero for no limit. Value is in megabytes by default or [k|m|g|t] may be specified.";
   public static final String EXPORT_LOGS__FILESIZELIMIT__SPECIFIED_DEFAULT = "0";
   public static final String EXPORT_LOGS__FILESIZELIMIT__UNSPECIFIED_DEFAULT = "100m";
 

http://git-wip-us.apache.org/repos/asf/geode/blob/a658322c/geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ExportLogController.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ExportLogController.java b/geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ExportLogController.java
index a369c6e..ddfd936 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ExportLogController.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/ExportLogController.java
@@ -57,7 +57,9 @@ public class ExportLogController extends AbstractCommandsController {
       @RequestParam(value = CliStrings.EXPORT_LOGS__LOGSONLY,
           required = false) final boolean logsOnly,
       @RequestParam(value = CliStrings.EXPORT_LOGS__STATSONLY,
-          required = false) final boolean statsOnly) {
+          required = false) final boolean statsOnly,
+      @RequestParam(value = CliStrings.EXPORT_LOGS__FILESIZELIMIT,
+          required = false) final String fileSizeLimit) {
     final CommandStringBuilder command = new CommandStringBuilder(CliStrings.EXPORT_LOGS);
 
     command.addOption(CliStrings.EXPORT_LOGS__DIR, decode(directory));
@@ -79,6 +81,7 @@ public class ExportLogController extends AbstractCommandsController {
     command.addOption(CliStrings.EXPORT_LOGS__MERGELOG, String.valueOf(mergeLog));
     command.addOption(CliStrings.EXPORT_LOGS__LOGSONLY, String.valueOf(logsOnly));
     command.addOption(CliStrings.EXPORT_LOGS__STATSONLY, String.valueOf(statsOnly));
+    command.addOption(CliStrings.EXPORT_LOGS__FILESIZELIMIT, fileSizeLimit);
 
     if (hasValue(startTime)) {
       command.addOption(CliStrings.EXPORT_LOGS__STARTTIME, startTime);

http://git-wip-us.apache.org/repos/asf/geode/blob/a658322c/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java
index 16549e7..7cb9dc3 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsCommandTest.java
@@ -32,13 +32,14 @@ import org.apache.geode.internal.cache.InternalCache;
 import org.apache.geode.management.ManagementException;
 import org.apache.geode.management.cli.Result;
 import org.apache.geode.management.internal.cli.functions.SizeExportLogsFunction;
+import org.apache.geode.management.internal.cli.result.CommandResult;
+import org.apache.geode.management.internal.cli.util.BytesToString;
 import org.apache.geode.test.junit.categories.UnitTest;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 import org.mockito.Matchers;
 
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -121,39 +122,78 @@ public class ExportLogsCommandTest {
   }
 
   @Test
-  public void sizeTooBigOnMember_sizeChecksDisabled_returnsFalse() throws Exception {
-    final Cache mockCache = mock(Cache.class);
-    final DistributedMember mockDistributedMember = mock(DistributedMember.class);
-    final Execution mockFunctionExecutor = mock(Execution.class);
-    final ExportLogsCommand cmd =
-        createExportLogsCommand(mockCache, mockDistributedMember, mockFunctionExecutor);
-    cmd.checkIfExportLogsOverflowsDisk("clusterMember", 0, MEGABYTE + 1024, MEGABYTE);
+  public void parseSizeLimit_sizeWithoutUnit_shouldReturnMegabytesSize() throws Exception {
+    ExportLogsCommand exportCmd = new ExportLogsCommand();
+    assertThat(exportCmd.parseFileSizeLimit("1000")).isEqualTo(1000 * MEGABYTE);
   }
 
   @Test
-  public void sizeOKOnMember_sizeChecksEnabled_doesNotThrow() throws Exception {
-    final Cache mockCache = mock(Cache.class);
-    final DistributedMember mockDistributedMember = mock(DistributedMember.class);
-    final Execution mockFunctionExecutor = mock(Execution.class);
-    final ExportLogsCommand cmd =
-        createExportLogsCommand(mockCache, mockDistributedMember, mockFunctionExecutor);
-    cmd.checkIfExportLogsOverflowsDisk("clusterMember", 10 * MEGABYTE, MEGABYTE - 1024, MEGABYTE);
+  public void parseSizeLimit_sizeWith_K_shouldReturnKilobytesSize() throws Exception {
+    ExportLogsCommand exportCmd = new ExportLogsCommand();
+    assertThat(exportCmd.parseFileSizeLimit("1000k")).isEqualTo(1000 * KILOBYTE);
   }
 
   @Test
-  public void sizeTooBigOnMember_sizeChecksEnabled_shouldThrow() throws Exception {
-    final Cache mockCache = mock(Cache.class);
-    final DistributedMember mockDistributedMember = mock(DistributedMember.class);
-    final Execution mockFunctionExecutor = mock(Execution.class);
-    final ExportLogsCommand cmd =
-        createExportLogsCommand(mockCache, mockDistributedMember, mockFunctionExecutor);
-    assertThatThrownBy(() -> cmd.checkIfExportLogsOverflowsDisk("clusterMember", 10 * MEGABYTE,
-        MEGABYTE + 1024, MEGABYTE)).isInstanceOf(ManagementException.class);
+  public void parseSizeLimit_sizeWith_M_shouldReturnMegabytesSize() throws Exception {
+    ExportLogsCommand exportCmd = new ExportLogsCommand();
+    assertThat(exportCmd.parseFileSizeLimit("1000m")).isEqualTo(1000 * MEGABYTE);
   }
 
   @Test
-  public void sizeFromAllMembers_greaterThanLocalDiskAvailable_shouldReturnErrorResult()
-      throws Exception {
+  public void parseSizeLimit_sizeWith_G_shouldReturnMegabytesSize() throws Exception {
+    ExportLogsCommand exportCmd = new ExportLogsCommand();
+    assertThat(exportCmd.parseFileSizeLimit("1000g")).isEqualTo(1000 * GIGABYTE);
+  }
+
+  @Test
+  public void parseSizeLimit_sizeWith_T_shouldReturnMegabytesSize() throws Exception {
+    ExportLogsCommand exportCmd = new ExportLogsCommand();
+    assertThat(exportCmd.parseFileSizeLimit("1000t")).isEqualTo(1000 * TERABYTE);
+  }
+
+  @Test
+  public void testTotalEstimateSizeExceedsLocatorAvailableDisk() throws Exception {
+    final InternalCache mockCache = mock(InternalCache.class);
+    final ExportLogsCommand realCmd = new ExportLogsCommand();
+    ExportLogsCommand spyCmd = spy(realCmd);
+
+    String start = null;
+    String end = null;
+    String logLevel = null;
+    boolean onlyLogLevel = false;
+    boolean logsOnly = false;
+    boolean statsOnly = false;
+
+    InternalDistributedMember member1 = new InternalDistributedMember("member1", 12345);
+    InternalDistributedMember member2 = new InternalDistributedMember("member2", 98765);
+    member1.getNetMember().setName("member1");
+    member2.getNetMember().setName("member2");
+    Set<DistributedMember> testMembers = new HashSet<>();
+    testMembers.add(member1);
+    testMembers.add(member2);
+
+    ResultCollector testResults1 = new CustomCollector();
+    testResults1.addResult(member1, 75 * MEGABYTE);
+    ResultCollector testResults2 = new CustomCollector();
+    testResults2.addResult(member2, 60 * MEGABYTE);
+
+    doReturn(mockCache).when(spyCmd).getCache();
+    doReturn(testMembers).when(spyCmd).getMembers(null, null);
+    doReturn(testResults1).when(spyCmd)
+        .estimateLogSize(Matchers.any(SizeExportLogsFunction.Args.class), eq(member1));
+    doReturn(testResults2).when(spyCmd)
+        .estimateLogSize(Matchers.any(SizeExportLogsFunction.Args.class), eq(member2));
+    doReturn(10 * MEGABYTE).when(spyCmd).getLocalDiskAvailable();
+
+    CommandResult res = (CommandResult) spyCmd.exportLogs("working dir", null, null, logLevel,
+        onlyLogLevel, false, start, end, logsOnly, statsOnly, "125m");
+    assertThat(res.getStatus()).isEqualTo(Result.Status.ERROR);
+    assertThat(res.toJson())
+        .contains("Estimated logs size will exceed the available disk space on the locator");
+  }
+
+  @Test
+  public void testTotalEstimateSizeExceedsUserSpecifiedValue() throws Exception {
     final InternalCache mockCache = mock(InternalCache.class);
     final ExportLogsCommand realCmd = new ExportLogsCommand();
     ExportLogsCommand spyCmd = spy(realCmd);
@@ -174,9 +214,9 @@ public class ExportLogsCommandTest {
     testMembers.add(member2);
 
     ResultCollector testResults1 = new CustomCollector();
-    testResults1.addResult(member1, Arrays.asList(75 * MEGABYTE));
+    testResults1.addResult(member1, 75 * MEGABYTE);
     ResultCollector testResults2 = new CustomCollector();
-    testResults2.addResult(member2, Arrays.asList(60 * MEGABYTE));
+    testResults2.addResult(member2, 60 * MEGABYTE);
 
     doReturn(mockCache).when(spyCmd).getCache();
     doReturn(testMembers).when(spyCmd).getMembers(null, null);
@@ -184,12 +224,51 @@ public class ExportLogsCommandTest {
         .estimateLogSize(Matchers.any(SizeExportLogsFunction.Args.class), eq(member1));
     doReturn(testResults2).when(spyCmd)
         .estimateLogSize(Matchers.any(SizeExportLogsFunction.Args.class), eq(member2));
-    doReturn(125 * MEGABYTE).when(spyCmd).getLocalDiskAvailable();
-    doReturn(GIGABYTE).when(spyCmd).getLocalDiskSize();
+    doReturn(GIGABYTE).when(spyCmd).getLocalDiskAvailable();
+
+    CommandResult res = (CommandResult) spyCmd.exportLogs("working dir", null, null, logLevel,
+        onlyLogLevel, false, start, end, logsOnly, statsOnly, "125m");
+    assertThat(res.getStatus()).isEqualTo(Result.Status.ERROR);
+    assertThat(res.toJson()).contains(
+        "Estimated exported logs expanded file size = 141557760, file-size-limit = 131072000");
+  }
+
+  @Test
+  public void estimateLogSizeExceedsServerDisk() throws Exception {
+    final InternalCache mockCache = mock(InternalCache.class);
+    final ExportLogsCommand realCmd = new ExportLogsCommand();
+    ExportLogsCommand spyCmd = spy(realCmd);
+
+    String start = null;
+    String end = null;
+    String logLevel = null;
+    boolean onlyLogLevel = false;
+    boolean logsOnly = false;
+    boolean statsOnly = false;
+
+    InternalDistributedMember member1 = new InternalDistributedMember("member1", 12345);
+    member1.getNetMember().setName("member1");
+    Set<DistributedMember> testMembers = new HashSet<>();
+    testMembers.add(member1);
+
+    BytesToString bytesToString = new BytesToString();
+    ResultCollector testResults1 = new CustomCollector();
+    StringBuilder sb = new StringBuilder().append("Estimated disk space required (")
+        .append(bytesToString.of(GIGABYTE)).append(") to consolidate logs on member ")
+        .append(member1.getName()).append(" will exceed available disk space (")
+        .append(bytesToString.of(500 * MEGABYTE)).append(")");
+    testResults1.addResult(member1, new ManagementException(sb.toString()));
+
+    doReturn(mockCache).when(spyCmd).getCache();
+    doReturn(testMembers).when(spyCmd).getMembers(null, null);
+    doReturn(testResults1).when(spyCmd)
+        .estimateLogSize(Matchers.any(SizeExportLogsFunction.Args.class), eq(member1));
 
-    Result res = spyCmd.exportLogs("working dir", null, null, logLevel, onlyLogLevel, false, start,
-        end, logsOnly, statsOnly, "125m");
+    CommandResult res = (CommandResult) spyCmd.exportLogs("working dir", null, null, logLevel,
+        onlyLogLevel, false, start, end, logsOnly, statsOnly, "125m");
     assertThat(res.getStatus()).isEqualTo(Result.Status.ERROR);
+    assertThat(res.toJson()).contains(
+        "Estimated disk space required (1 GB) to consolidate logs on member member1 will exceed available disk space (500 MB)");
   }
 
   private ExportLogsCommand createExportLogsCommand(final Cache cache,

http://git-wip-us.apache.org/repos/asf/geode/blob/a658322c/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java
index 8609b3a..045e13e 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsDUnitTest.java
@@ -28,9 +28,7 @@ import org.apache.geode.cache.Cache;
 import org.apache.geode.distributed.ConfigurationProperties;
 import org.apache.geode.internal.cache.GemFireCacheImpl;
 import org.apache.geode.internal.logging.LogService;
-import org.apache.geode.management.cli.Result;
 import org.apache.geode.management.internal.cli.functions.ExportLogsFunction;
-import org.apache.geode.management.internal.cli.result.CommandResult;
 import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
 import org.apache.geode.management.internal.configuration.utils.ZipUtils;
 import org.apache.geode.test.dunit.IgnoredException;
@@ -166,8 +164,6 @@ public class ExportLogsDUnitTest {
   @Test
   public void testExportWithExactLogLevelFilter() throws Exception {
     gfshConnector.executeAndVerifyCommand("export logs --log-level=info --only-log-level=true");
-
-
     Set<String> acceptedLogLevels = Stream.of("info").collect(toSet());
     verifyZipFileContents(acceptedLogLevels);
   }
@@ -180,12 +176,6 @@ public class ExportLogsDUnitTest {
   }
 
   @Test
-  public void testExportedZipFileTooBig() throws Exception {
-    CommandResult result = gfshConnector.executeCommand("export logs --file-size-limit=10k");
-    assertThat(result.getStatus()).isEqualTo(Result.Status.ERROR);
-  }
-
-  @Test
   public void testExportWithNoFilters() throws Exception {
     gfshConnector.executeAndVerifyCommand("export logs --log-level=all");
 

http://git-wip-us.apache.org/repos/asf/geode/blob/a658322c/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java
deleted file mode 100644
index 09ee08d..0000000
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsFileSizeLimitTest.java
+++ /dev/null
@@ -1,87 +0,0 @@
-/*
- * 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.geode.management.internal.cli.commands;
-
-import static org.apache.geode.management.internal.cli.commands.ExportLogsCommand.MEGABYTE;
-import static org.assertj.core.api.Assertions.*;
-
-import org.apache.commons.io.FileUtils;
-import org.apache.geode.test.junit.categories.IntegrationTest;
-import org.junit.Before;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.experimental.categories.Category;
-import org.junit.rules.TemporaryFolder;
-import org.junit.rules.TestName;
-
-import java.io.File;
-import java.io.IOException;
-import java.io.PrintWriter;
-
-@Category(IntegrationTest.class)
-public class ExportLogsFileSizeLimitTest {
-
-  private File dir;
-
-  @Rule
-  public TemporaryFolder temporaryFolder = new TemporaryFolder();
-
-  @Rule
-  public TestName testName = new TestName();
-
-  @Before
-  public void before() throws Exception {
-    this.dir = this.temporaryFolder.getRoot();
-  }
-
-  @Test
-  public void sizeOverLimitThrows() throws Exception {
-    File file = new File(this.dir, this.testName.getMethodName());
-    fillUpFile(file, MEGABYTE * 2);
-
-    ExportLogsCommand exportLogsCommand = new ExportLogsCommand();
-
-    assertThatThrownBy(() -> exportLogsCommand.checkFileSizeWithinLimit(MEGABYTE, file));
-  }
-
-  @Test
-  public void sizeLessThanLimitIsOk() throws Exception {
-    File file = new File(this.dir, this.testName.getMethodName());
-    fillUpFile(file, MEGABYTE / 2);
-
-    ExportLogsCommand exportLogsCommand = new ExportLogsCommand();
-
-    exportLogsCommand.checkFileSizeWithinLimit(MEGABYTE, file);
-  }
-
-  @Test
-  public void sizeZeroIsUnlimitedSize() throws Exception {
-    File file = new File(this.dir, this.testName.getMethodName());
-    fillUpFile(file, MEGABYTE * 2);
-
-    ExportLogsCommand exportLogsCommand = new ExportLogsCommand();
-
-    exportLogsCommand.checkFileSizeWithinLimit(0, file);
-  }
-
-  private void fillUpFile(File file, long sizeInBytes) throws IOException {
-    PrintWriter writer = new PrintWriter(file, "UTF-8");
-    while (FileUtils.sizeOf(file) < sizeInBytes) {
-      writer.println("this is a line of data in the file");
-    }
-    writer.close();
-  }
-
-}

http://git-wip-us.apache.org/repos/asf/geode/blob/a658322c/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsStatsDUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsStatsDUnitTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsStatsDUnitTest.java
index 44a0362..33439e6 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsStatsDUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsStatsDUnitTest.java
@@ -26,6 +26,8 @@ import com.google.common.collect.Sets;
 
 import org.apache.geode.distributed.ConfigurationProperties;
 import org.apache.geode.internal.AvailablePortHelper;
+import org.apache.geode.management.cli.Result;
+import org.apache.geode.management.internal.cli.result.CommandResult;
 import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
 import org.apache.geode.test.dunit.rules.GfshShellConnectionRule;
 import org.apache.geode.test.dunit.rules.LocatorServerStartupRule;
@@ -138,6 +140,13 @@ public class ExportLogsStatsDUnitTest {
     assertThat(output).contains("No files to be exported");
   }
 
+  @Test
+  public void testExportedZipFileTooBig() throws Exception {
+    connectIfNeeded();
+    CommandResult result = connector.executeCommand("export logs --file-size-limit=10k");
+    assertThat(result.getStatus()).isEqualTo(Result.Status.ERROR);
+  }
+
   protected String getZipPathFromCommandResult(String message) {
     return message.replaceAll("Logs exported to the connected member's file system: ", "").trim();
   }

http://git-wip-us.apache.org/repos/asf/geode/blob/a658322c/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java
index 4e1dac0..b1f3ed7 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsTestSuite.java
@@ -28,10 +28,9 @@ import org.junit.runners.Suite;
  */
 
 @Ignore
-@Suite.SuiteClasses({ExportLogsCommandTest.class, ExportLogsFileSizeLimitTest.class,
-    ExportLogsIntegrationTest.class, ExportLogsDUnitTest.class, SizeExportLogsFunctionTest.class,
-    SizeExportLogsFunctionFileTest.class, LogSizerTest.class, LogExporterTest.class,
-    LogExporterIntegrationTest.class})
+@Suite.SuiteClasses({ExportLogsCommandTest.class, ExportLogsDUnitTest.class,
+    SizeExportLogsFunctionTest.class, SizeExportLogsFunctionFileTest.class, LogSizerTest.class,
+    LogExporterTest.class, LogExporterIntegrationTest.class, ExportLogsIntegrationTest.class})
 @RunWith(Suite.class)
 public class ExportLogsTestSuite {
 }

http://git-wip-us.apache.org/repos/asf/geode/blob/a658322c/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionTest.java
index cc5e7d5..96fa733 100644
--- a/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionTest.java
+++ b/geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/SizeExportLogsFunctionTest.java
@@ -138,12 +138,13 @@ public class SizeExportLogsFunctionTest {
 
     assertThat(results).isNotNull();
     assertThat(results.size()).isEqualTo(1);
-    List<?> result = (List<?>) results.get(0);
+    Object result = results.get(0);
     assertThat(result).isNotNull();
+    assertThat(result).isInstanceOf(Long.class);
     if (minExpected == maxExpected) {
-      assertThat(((Long) result.get(0))).isEqualTo(minExpected);
+      assertThat(((Long) result)).isEqualTo(minExpected);
     }
-    assertThat(((Long) result.get(0))).isGreaterThanOrEqualTo(minExpected)
+    assertThat(((Long) result)).isGreaterThanOrEqualTo(minExpected)
         .isLessThanOrEqualTo(maxExpected);
   }