Improve testing for skipping promoting host running backup

While at it, also fix goimports in a couple of files.

Signed-off-by: Eduardo J. Ortega U <[email protected]>

go/vt/vtctl/reparentutil/emergency_reparenter.go

@@ -421,9 +421,11 @@ func (erp *EmergencyReparenter) findMostAdvanced(
 	winningPosition := tabletPositions[0]
 
 	// We have already removed the tablets with errant GTIDs before calling this function. At this point our winning position must be a
-	// superset of all the other valid positions. If that is not the case, then we have a split brain scenario, and we should cancel the ERS
+	// superset of all the other valid positions - unless the tablet with that position is taking a backup, in which case it might
+	// have a larger GTID set but we are still not choosing it. If that is not the case, then we have a split brain scenario, and
+	// we should cancel the ERS
 	for i, position := range tabletPositions {
-		if !winningPosition.AtLeast(position) {
+		if !winningPosition.AtLeast(position) && !backingUpTablets[topoproto.TabletAliasString(validTablets[i].Alias)] {
 			return nil, nil, vterrors.Errorf(vtrpc.Code_FAILED_PRECONDITION, "split brain detected between servers - %v and %v", winningPrimaryTablet.Alias, validTablets[i].Alias)
 		}
 	}

go/vt/vtctl/reparentutil/emergency_reparenter_test.go

@@ -2867,6 +2867,55 @@ func TestEmergencyReparenter_findMostAdvanced(t *testing.T) {
 					Uid:  100,
 				},
 			},
+		}, {
+			name: "choose most advanced not backing up",
+			validCandidates: map[string]replication.Position{
+				"zone1-0000000100": positionMostAdvanced,
+				"zone1-0000000101": positionIntermediate1,
+				"zone1-0000000102": positionIntermediate2,
+			},
+			tabletMap: map[string]*topo.TabletInfo{
+				"zone1-0000000100": {
+					Tablet: &topodatapb.Tablet{
+						Alias: &topodatapb.TabletAlias{
+							Cell: "zone1",
+							Uid:  100,
+						},
+					},
+				},
+				"zone1-0000000101": {
+					Tablet: &topodatapb.Tablet{
+						Alias: &topodatapb.TabletAlias{
+							Cell: "zone1",
+							Uid:  101,
+						},
+					},
+				},
+				"zone1-0000000102": {
+					Tablet: &topodatapb.Tablet{
+						Alias: &topodatapb.TabletAlias{
+							Cell: "zone1",
+							Uid:  102,
+						},
+					},
+				},
+				"zone1-0000000404": {
+					Tablet: &topodatapb.Tablet{
+						Alias: &topodatapb.TabletAlias{
+							Cell: "zone1",
+							Uid:  404,
+						},
+						Hostname: "ignored tablet",
+					},
+				},
+			},
+			backingUpTablets: map[string]bool{"zone1-0000000100": true, "zone1-0000000101": false, "zone1-0000000102": false},
+			result: &topodatapb.Tablet{
+				Alias: &topodatapb.TabletAlias{
+					Cell: "zone1",
+					Uid:  102,
+				},
+			},
 		}, {
 			name: "choose most advanced with the best promotion rule",
 			validCandidates: map[string]replication.Position{
@@ -4468,17 +4517,11 @@ func TestEmergencyReparenter_filterValidCandidates(t *testing.T) {
 		}
 	)
 	allTablets := []*topodatapb.Tablet{primaryTablet, replicaTablet, rdonlyTablet, replicaCrossCellTablet, rdonlyCrossCellTablet}
-	noTabletsBackingUp := map[string]bool{
-		topoproto.TabletAliasString(primaryTablet.Alias): false, topoproto.TabletAliasString(replicaTablet.Alias): false,
-		topoproto.TabletAliasString(rdonlyTablet.Alias): false, topoproto.TabletAliasString(replicaCrossCellTablet.Alias): false,
-		topoproto.TabletAliasString(rdonlyCrossCellTablet.Alias): false,
-	}
 	tests := []struct {
 		name             string
 		durability       string
 		validTablets     []*topodatapb.Tablet
 		tabletsReachable []*topodatapb.Tablet
-		tabletsBackingUp map[string]bool
 		prevPrimary      *topodatapb.Tablet
 		opts             EmergencyReparentOptions
 		filteredTablets  []*topodatapb.Tablet
@@ -4489,14 +4532,12 @@ func TestEmergencyReparenter_filterValidCandidates(t *testing.T) {
 			durability:       "none",
 			validTablets:     allTablets,
 			tabletsReachable: allTablets,
-			tabletsBackingUp: noTabletsBackingUp,
 			filteredTablets:  []*topodatapb.Tablet{primaryTablet, replicaTablet, replicaCrossCellTablet},
 		}, {
 			name:             "filter cross cell",
 			durability:       "none",
 			validTablets:     allTablets,
 			tabletsReachable: allTablets,
-			tabletsBackingUp: noTabletsBackingUp,
 			prevPrimary: &topodatapb.Tablet{
 				Alias: &topodatapb.TabletAlias{
 					Cell: "zone-1",
@@ -4511,7 +4552,6 @@ func TestEmergencyReparenter_filterValidCandidates(t *testing.T) {
 			durability:       "cross_cell",
 			validTablets:     []*topodatapb.Tablet{primaryTablet, replicaTablet},
 			tabletsReachable: []*topodatapb.Tablet{primaryTablet, replicaTablet, rdonlyTablet, rdonlyCrossCellTablet},
-			tabletsBackingUp: noTabletsBackingUp,
 			filteredTablets:  nil,
 		}, {
 			name:       "filter mixed",
@@ -4526,14 +4566,12 @@ func TestEmergencyReparenter_filterValidCandidates(t *testing.T) {
 			},
 			validTablets:     allTablets,
 			tabletsReachable: allTablets,
-			tabletsBackingUp: noTabletsBackingUp,
 			filteredTablets:  []*topodatapb.Tablet{replicaCrossCellTablet},
 		}, {
 			name:             "error - requested primary must not",
 			durability:       "none",
 			validTablets:     allTablets,
 			tabletsReachable: allTablets,
-			tabletsBackingUp: noTabletsBackingUp,
 			opts: EmergencyReparentOptions{
 				NewPrimaryAlias: rdonlyTablet.Alias,
 			},
@@ -4554,7 +4592,6 @@ func TestEmergencyReparenter_filterValidCandidates(t *testing.T) {
 			durability:       "cross_cell",
 			validTablets:     allTablets,
 			tabletsReachable: []*topodatapb.Tablet{primaryTablet, replicaTablet, rdonlyTablet, rdonlyCrossCellTablet},
-			tabletsBackingUp: noTabletsBackingUp,
 			opts: EmergencyReparentOptions{
 				NewPrimaryAlias: primaryTablet.Alias,
 			},

go/vt/vtctl/reparentutil/reparent_sorter.go

@@ -18,6 +18,7 @@ package reparentutil
 
 import (
 	"sort"
+
 	"vitess.io/vitess/go/vt/topo/topoproto"
 
 	"vitess.io/vitess/go/mysql/replication"

go/vt/vtctl/reparentutil/reparent_sorter_test.go

@@ -18,6 +18,7 @@ package reparentutil
 
 import (
 	"testing"
+
 	"vitess.io/vitess/go/vt/topo/topoproto"
 
 	"github.com/stretchr/testify/require"