tests and adjustments

Signed-off-by: Renan Rangel <rrangel@slack-corp.com>

go/vt/mysqlctl/mysqlshellbackupengine.go

@@ -19,7 +19,7 @@ import (
 )
 
 var (
-	// location to store the logical backup
+	// location to store the mysql shell backup
 	mysqlShellBackupLocation = flag.String("mysql_shell_location", "", "location where the backup will be stored")
 	mysqlShellFlags          = flag.String("mysql_shell_flags", "--defaults-file=/dev/null --js -h localhost", "execution flags to pass to mysqlsh binary")
 	// flags to pass through to backup phase
@@ -35,6 +35,8 @@ var (
 		false, "decide if we should drain while taking a backup or continue to serving traffic")
 	mysqlShellSpeedUpRestore = flag.Bool("mysql_shell_speedup_restore",
 		false, "speed up restore by disabling redo logging and double write buffer during the restore process")
+
+	MySQLShellPreCheckError = errors.New("MySQLShellPreCheckError")
 )
 
 // MySQLShellBackupManifest represents a backup.
@@ -50,7 +52,7 @@ type MySQLShellBackupManifest struct {
 	Params string
 }
 
-// LogicalBackupEngine encapsulates the logic to implement the restoration
+// MySQLShellBackupEngine encapsulates the logic to implement the restoration
 // of a mysql-shell based backup.
 type MySQLShellBackupEngine struct {
 }
@@ -101,13 +103,13 @@ func (be *MySQLShellBackupEngine) ExecuteBackup(ctx context.Context, params Back
 
 	cmdWg := &sync.WaitGroup{}
 	cmdWg.Add(2)
-	go scanLinesToLogger("logical stdout", cmdOut, params.Logger, cmdWg.Done)
-	go scanLinesToLogger("logical stderr", cmdErr, params.Logger, cmdWg.Done)
+	go scanLinesToLogger(mysqlShellBackupEngineName+" stdout", cmdOut, params.Logger, cmdWg.Done)
+	go scanLinesToLogger(mysqlShellBackupEngineName+" stderr", cmdErr, params.Logger, cmdWg.Done)
 	cmdWg.Wait()
 
 	// Get exit status.
 	if err := cmd.Wait(); err != nil {
-		return BackupUnusable, vterrors.Wrap(err, "logical failed")
+		return BackupUnusable, vterrors.Wrap(err, mysqlShellBackupEngineName+" failed")
 	}
 
 	// open the MANIFEST
@@ -131,7 +133,7 @@ func (be *MySQLShellBackupEngine) ExecuteBackup(ctx context.Context, params Back
 			FinishedTime: time.Now().UTC().Format(time.RFC3339),
 		},
 
-		// logical backup specific fields
+		// mysql shell backup specific fields
 		BackupLocation: location,
 		Params:         *mysqlShellLoadFlags,
 	}
@@ -245,13 +247,13 @@ func (be *MySQLShellBackupEngine) ExecuteRestore(ctx context.Context, params Res
 
 	cmdWg := &sync.WaitGroup{}
 	cmdWg.Add(2)
-	go scanLinesToLogger("logical stdout", cmdOut, params.Logger, cmdWg.Done)
-	go scanLinesToLogger("logical stderr", cmdErr, params.Logger, cmdWg.Done)
+	go scanLinesToLogger(mysqlShellBackupEngineName+" stdout", cmdOut, params.Logger, cmdWg.Done)
+	go scanLinesToLogger(mysqlShellBackupEngineName+" stderr", cmdErr, params.Logger, cmdWg.Done)
 	cmdWg.Wait()
 
 	// Get the exit status.
 	if err := cmd.Wait(); err != nil {
-		return nil, vterrors.Wrap(err, "logical failed")
+		return nil, vterrors.Wrap(err, mysqlShellBackupEngineName+" failed")
 	}
 	params.Logger.Infof("%s completed successfully", mysqlShellBackupBinaryName)
 
@@ -288,33 +290,33 @@ func (be *MySQLShellBackupEngine) ShouldDrainForBackup(req *tabletmanagerdatapb.
 
 func (be *MySQLShellBackupEngine) backupPreCheck() error {
 	if *mysqlShellBackupLocation == "" {
-		return errors.New("no backup location set via --mysql_shell_location")
+		return fmt.Errorf("%w: no backup location set via --mysql_shell_location", MySQLShellPreCheckError)
 	}
 
-	if *mysqlShellFlags == "" {
-		return errors.New("at least the --js flag is required")
+	if *mysqlShellFlags == "" || !strings.Contains(*mysqlShellFlags, "--js") {
+		return fmt.Errorf("%w: at least the --js flag is required", MySQLShellPreCheckError)
 	}
 
 	return nil
 }
 
 func (be *MySQLShellBackupEngine) restorePreCheck() error {
 	if *mysqlShellFlags == "" {
-		return errors.New("at least the --js flag is required")
+		return fmt.Errorf("%w: at least the --js flag is required", MySQLShellPreCheckError)
 	}
 
 	loadFlags := map[string]interface{}{}
 	err := json.Unmarshal([]byte(*mysqlShellLoadFlags), &loadFlags)
 	if err != nil {
-		return errors.New("unable to parse JSON of load flags")
+		return fmt.Errorf("%w: unable to parse JSON of load flags", MySQLShellPreCheckError)
 	}
 
 	if val, ok := loadFlags["updateGtidSet"]; !ok || val != "replace" {
-		return errors.New("mysql-shell needs to restore with updateGtidSet set to \"replace\" to work with Vitess")
+		return fmt.Errorf("%w: mysql-shell needs to restore with updateGtidSet set to \"replace\" to work with Vitess", MySQLShellPreCheckError)
 	}
 
 	if val, ok := loadFlags["progressFile"]; !ok || val != "" {
-		return errors.New("\"progressFile\" needs to be empty as vitess always starts a restore from scratch")
+		return fmt.Errorf("%w: \"progressFile\" needs to be empty as vitess always starts a restore from scratch", MySQLShellPreCheckError)
 	}
 
 	return nil

go/vt/mysqlctl/mysqlshellbackupengine_test.go

@@ -0,0 +1,69 @@
+package mysqlctl
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	tabletmanagerdatapb "vitess.io/vitess/go/vt/proto/tabletmanagerdata"
+)
+
+func TestMySQLShellBackupRestorePreCheck(t *testing.T) {
+	original := *mysqlShellLoadFlags
+	defer func() { *mysqlShellLoadFlags = original }()
+
+	engine := MySQLShellBackupEngine{}
+	tests := []struct {
+		name  string
+		flags string
+		err   error
+	}{
+		{
+			"empty load flags",
+			`{}`,
+			MySQLShellPreCheckError,
+		},
+		{
+			"only updateGtidSet",
+			`{"updateGtidSet": "replace"}`,
+			MySQLShellPreCheckError,
+		},
+		{
+			"only progressFile",
+			`{"progressFile": ""}`,
+			MySQLShellPreCheckError,
+		},
+		{
+			"both values but unsupported values",
+			`{"updateGtidSet": "append", "progressFile": "/tmp/test1"}`,
+			MySQLShellPreCheckError,
+		},
+		{
+			"supported values",
+			`{"updateGtidSet": "replace", "progressFile": ""}`,
+			nil,
+		},
+	}
+
+	for _, tt := range tests {
+		*mysqlShellLoadFlags = tt.flags
+		assert.ErrorIs(t, engine.restorePreCheck(), tt.err)
+	}
+
+}
+
+func TestShouldDrainForBackupMySQLShell(t *testing.T) {
+	original := *mysqlShellBackupShouldDrain
+	defer func() { *mysqlShellBackupShouldDrain = original }()
+
+	engine := MySQLShellBackupEngine{}
+
+	*mysqlShellBackupShouldDrain = false
+
+	assert.False(t, engine.ShouldDrainForBackup(nil))
+	assert.False(t, engine.ShouldDrainForBackup(&tabletmanagerdatapb.BackupRequest{}))
+
+	*mysqlShellBackupShouldDrain = true
+
+	assert.True(t, engine.ShouldDrainForBackup(nil))
+	assert.True(t, engine.ShouldDrainForBackup(&tabletmanagerdatapb.BackupRequest{}))
+}