Skip to content

Commit

Permalink
Exit with different error codes when consistency-check fails
Browse files Browse the repository at this point in the history
  • Loading branch information
@spacecowboy
spacecowboy committed Mar 14, 2017
1 parent d33f4e2 commit 0c4907d
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 25 deletions.
13 changes: 9 additions & 4 deletions community/command-line/src/main/java/org/neo4j/commandline/admin/AdminTool.java
View file
Expand Up @@ -149,7 +149,7 @@ private void unexpected( RuntimeException e )

private void commandFailed( CommandFailed e )
{
failure( "command failed", e );
failure( "command failed", e, e.code() );
}

private void failure()
Expand All @@ -158,18 +158,23 @@ private void failure()
}

private void failure( String message, Exception e )
{
failure( message, e, 1 );
}

private void failure( String message, Exception e, int code )
{
if ( debug )
{
outsideWorld.printStacktrace( e );
}
failure( format( "%s: %s", message, e.getMessage() ) );
failure( format( "%s: %s", message, e.getMessage() ), code );
}

private void failure( String message )
private void failure( String message, int code )
{
outsideWorld.stdErrLine( message );
outsideWorld.exit( 1 );
outsideWorld.exit( code );
}

private void success()
Expand Down
20 changes: 17 additions & 3 deletions community/command-line/src/main/java/org/neo4j/commandline/admin/CommandFailed.java
View file
Expand Up @@ -21,18 +21,32 @@

public class CommandFailed extends Exception
{
public CommandFailed( String message, Exception cause )
private final int code;

public CommandFailed( String message, Throwable cause, int code )
{
super( message, cause );
this.code = code;
}

public CommandFailed( Throwable cause )
public CommandFailed( String message, Throwable cause )
{
super( cause );
this( message, cause, 1 );
}

public CommandFailed( String message )
{
this( message, 1 );
}

public CommandFailed( String message, int code )
{
super( message );
this.code = code;
}

public int code()
{
return code;
}
}
10 changes: 7 additions & 3 deletions enterprise/backup/src/main/java/org/neo4j/backup/OnlineBackupCommand.java
View file
Expand Up @@ -249,12 +249,16 @@ public void execute( String[] args ) throws IncorrectUsage, CommandFailed
if ( !ccResult.isSuccessful() )
{
throw new CommandFailed( String.format( "Inconsistencies found. See '%s' for details.",
ccResult.reportFile() ) );
ccResult.reportFile() ), 3 );
}
}
catch ( Exception e )
catch ( Throwable e )
{
throw new CommandFailed( "Failed to do consistency check on backup: " + e.getMessage(), e );
if ( e instanceof CommandFailed )
{
throw (CommandFailed) e;
}
throw new CommandFailed( "Failed to do consistency check on backup: " + e.getMessage(), e, 2 );
}
}

Expand Down
81 changes: 72 additions & 9 deletions enterprise/backup/src/test/java/org/neo4j/backup/OnlineBackupCommandTest.java
View file
Expand Up @@ -19,6 +19,9 @@
*/
package org.neo4j.backup;

import org.hamcrest.BaseMatcher;
import org.hamcrest.Description;
import org.hamcrest.Matcher;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
Expand All @@ -45,6 +48,7 @@
import org.neo4j.kernel.configuration.Config;
import org.neo4j.test.rule.TestDirectory;

import static java.lang.String.format;
import static java.util.Collections.singletonList;
import static java.util.concurrent.TimeUnit.HOURS;
import static java.util.concurrent.TimeUnit.MINUTES;
Expand Down Expand Up @@ -168,6 +172,7 @@ public void nonExistingBackupDirThrows() throws CommandFailed, IncorrectUsage, B
final String path = path( "/Idontexist/sasdfasdfa" );
expected.expect( CommandFailed.class );
expected.expectMessage( "Directory '" + path + "' does not exist." );
expected.expect( exitCode( 1 ) );
execute( backupDir( path ), "--name=mybackup" );
}

Expand Down Expand Up @@ -238,17 +243,33 @@ public void shouldNotAskForConsistencyCheckIfSpecifiedIncremental() throws Excep
}

@Test
public void failedCCIsReported() throws Exception
public void inconsistentCCIsReportedWithExitCode3() throws Exception

{
final Path path = Paths.get( "/foo/bar" );

when( consistencyCheckService.runFullConsistencyCheck( any(), any(), any(), any(), any(),
anyBoolean(), eq( new File( "." ).getCanonicalFile() ), any( CheckConsistencyConfig.class ) ) ).thenReturn(
ConsistencyCheckService.Result.failure( path.toFile() ) );
anyBoolean(), eq( new File( "." ).getCanonicalFile() ), any( CheckConsistencyConfig.class ) ) )
.thenReturn(
ConsistencyCheckService.Result.failure( path.toFile() ) );

expected.expect( CommandFailed.class );
expected.expectMessage( "Inconsistencies found. See '" + path + "' for details." );
expected.expect( exitCode( 3 ) );
execute( "--check-consistency=true", backupDir(), "--name=mybackup" );
}

@Test
public void errorInCCIsReportedWithExitCode2() throws Exception

{
when( consistencyCheckService.runFullConsistencyCheck( any(), any(), any(), any(), any(),
anyBoolean(), eq( new File( "." ).getCanonicalFile() ), any( CheckConsistencyConfig.class ) ) )
.thenThrow( new RuntimeException( "craassh" ) );

expected.expect( CommandFailed.class );
expected.expectMessage( "Failed to do consistency check on backup: craassh" );
expected.expect( exitCode( 2 ) );
execute( "--check-consistency=true", backupDir(), "--name=mybackup" );
}

Expand Down Expand Up @@ -317,7 +338,7 @@ public void shouldFallbackToFullIfIncrementalFails() throws Exception
when( outsideWorld.fileSystem() ).thenReturn( mockFs );
File dir = testDirectory.directory( "ccInc" );
when( mockFs.isDirectory( eq( dir.getParentFile() ) ) ).thenReturn( true );
when( mockFs.listFiles( eq( dir ) ) ).thenReturn( new File[]{dir} );
when( mockFs.listFiles( eq( dir ) ) ).thenReturn( new File[]{ dir } );
when( backupService.doIncrementalBackup( any(), anyInt(), any(), anyLong(), any() ) )
.thenThrow( new RuntimeException( "nah-ah" ) );

Expand All @@ -341,19 +362,57 @@ public void failToRenameIsReported() throws Exception
when( outsideWorld.fileSystem() ).thenReturn( mockFs );
File dir = testDirectory.directory( "ccInc" );
when( mockFs.isDirectory( eq( dir.getParentFile() ) ) ).thenReturn( true );
when( mockFs.listFiles( eq( dir ) ) ).thenReturn( new File[]{dir} );
when( mockFs.listFiles( eq( dir ) ) ).thenReturn( new File[]{ dir } );
when( backupService.doIncrementalBackup( any(), anyInt(), any(), anyLong(), any() ) )
.thenThrow( new RuntimeException( "nah-ah" ) );

doThrow( new IOException( "kaboom" ) ).when( mockFs ).renameFile( any(), any() );

expected.expectMessage( "Failed to move old backup out of the way: kaboom" );
expected.expect( CommandFailed.class );
expected.expect( exitCode( 1 ) );

execute( "--cc-report-dir=" + dir.getParent(), backupDir( dir.getParent() ),
"--name=" + dir.getName() );
}

private Matcher<CommandFailed> exitCode( final int expectedCode )
{
return new BaseMatcher<CommandFailed>()
{
@Override
public void describeTo( Description description )
{
description.appendText( "expected exit code " ).appendValue( expectedCode );
}

@Override
public void describeMismatch( Object o, Description description )
{
if ( o instanceof CommandFailed )
{
CommandFailed e = (CommandFailed) o;
description.appendText( "but it was " ).appendValue( e.code() );
}
else
{
description.appendText( "but exception is not a CommandFailed exception" );
}
}

@Override
public boolean matches( Object o )
{
if ( o instanceof CommandFailed )
{
CommandFailed e = (CommandFailed) o;
return expectedCode == e.code();
}
return false;
}
};
}

@Test
public void shouldNotFallbackToFullIfSpecified() throws Exception

Expand All @@ -365,6 +424,7 @@ public void shouldNotFallbackToFullIfSpecified() throws Exception

expected.expectMessage( "Backup failed: nah-ah" );
expected.expect( CommandFailed.class );
expected.expect( exitCode( 1 ) );

execute( "--fallback-to-full=false", backupDir( dir.getParent() ), "--name=" + dir.getName() );
}
Expand All @@ -376,7 +436,7 @@ public void renamingOldBackupIncrements() throws Exception
File dir = testDirectory.directory( "ccInc" );
when( outsideWorld.fileSystem() ).thenReturn( mockFs );
when( mockFs.isDirectory( eq( dir.getParentFile() ) ) ).thenReturn( true );
when( mockFs.listFiles( eq( dir ) ) ).thenReturn( new File[]{dir} );
when( mockFs.listFiles( eq( dir ) ) ).thenReturn( new File[]{ dir } );
for ( int i = 1; i < 50; i++ )
{
when( mockFs.fileExists( eq( new File( dir.getParentFile(), "ccInc.err." + i ) ) ) ).thenReturn( true );
Expand Down Expand Up @@ -404,7 +464,7 @@ public void renamingOldBackupIncrementsOnlySoFar() throws Exception
File dir = testDirectory.directory( "ccInc" );
when( outsideWorld.fileSystem() ).thenReturn( mockFs );
when( mockFs.isDirectory( eq( dir.getParentFile() ) ) ).thenReturn( true );
when( mockFs.listFiles( eq( dir ) ) ).thenReturn( new File[]{dir} );
when( mockFs.listFiles( eq( dir ) ) ).thenReturn( new File[]{ dir } );
for ( int i = 1; i < MAX_OLD_BACKUPS; i++ )
{
when( mockFs.fileExists( eq( new File( dir.getParentFile(), "ccInc.err." + i ) ) ) ).thenReturn( true );
Expand All @@ -414,6 +474,7 @@ public void renamingOldBackupIncrementsOnlySoFar() throws Exception

expected.expect( CommandFailed.class );
expected.expectMessage( "ailed to move old backup out of the way: too many old backups." );
expected.expect( exitCode( 1 ) );
execute( "--cc-report-dir=" + dir.getParent(), backupDir( dir.getParent() ), "--name=" + dir.getName() );
}

Expand All @@ -422,7 +483,7 @@ public void shouldSpecifyReportDirIfSpecified() throws Exception
{
File reportDir = testDirectory.directory( "ccreport" );
when( consistencyCheckService.runFullConsistencyCheck( any(), any(), any(), any(), any(), anyBoolean(),
any(CheckConsistencyConfig.class) ) ).thenReturn( ConsistencyCheckService.Result.success( null ) );
any( CheckConsistencyConfig.class ) ) ).thenReturn( ConsistencyCheckService.Result.success( null ) );

execute( "--check-consistency", backupDir(), "--name=mybackup",
"--cc-report-dir=" + reportDir );
Expand All @@ -443,6 +504,7 @@ public void fullFailureIsReported() throws Exception

expected.expect( CommandFailed.class );
expected.expectMessage( "Backup failed: nope" );
expected.expect( exitCode( 1 ) );
execute( backupDir( dir.getParent() ), "--name=" + dir.getName() );
}

Expand All @@ -461,6 +523,7 @@ public void reportDirMustExist() throws Exception
final String path = path( "/aalivnmoimzlckmvPDK" );
expected.expect( CommandFailed.class );
expected.expectMessage( "Directory '" + path + "' does not exist." );
expected.expect( exitCode( 1 ) );
execute( "--check-consistency", backupDir(), "--name=mybackup",
"--cc-report-dir=" + path );
}
Expand Down Expand Up @@ -539,7 +602,7 @@ public void shouldPrintNiceHelp() throws Throwable
usage.printUsageForCommand( new OnlineBackupCommandProvider(), ps::println );

assertEquals(
String.format( "usage: neo4j-admin backup --backup-dir=<backup-path> --name=<graph.db-backup>%n" +
format( "usage: neo4j-admin backup --backup-dir=<backup-path> --name=<graph.db-backup>%n" +
" [--from=<address>] [--fallback-to-full[=<true|false>]]%n" +
" [--timeout=<timeout>]%n" +
" [--check-consistency[=<true|false>]]%n" +
Expand Down
8 changes: 2 additions & 6 deletions .../causal-clustering/src/main/java/org/neo4j/commandline/dbms/UnbindFromClusterCommand.java
View file
Expand Up @@ -103,13 +103,9 @@ public void execute( String[] args ) throws IncorrectUsage, CommandFailed
{
throw new IncorrectUsage( e.getMessage() );
}
catch ( UnbindFailureException | CannotWriteException | IOException e )
catch ( UnbindFailureException | CannotWriteException | IOException | ClusterStateException e )
{
throw new CommandFailed( "Unbind failed: " + e.getMessage(), e );
}
catch ( ClusterStateException e )
{
throw new CommandFailed( e );
throw new CommandFailed( e.getMessage(), e );
}
}

Expand Down

0 comments on commit 0c4907d

Please sign in to comment.