Merge pull request vitessio#8084 from tinyspeck/am_reparent_tablet_pr…

…event_self_reparent

[vtctldserver] Add guard against self-reparent, plus misc updates

go/vt/vtctl/grpcvtctldserver/server.go

@@ -1151,7 +1151,7 @@ func (s *VtctldServer) ReparentTablet(ctx context.Context, req *vtctldatapb.Repa
  }
 
  if !shard.HasMaster() {
- return nil, vterrors.Errorf(vtrpc.Code_FAILED_PRECONDITION, "no master tablet for shard %v/%v", tablet.Keyspace, tablet.Shard)
+ return nil, vterrors.Errorf(vtrpc.Code_FAILED_PRECONDITION, "no primary tablet for shard %v/%v", tablet.Keyspace, tablet.Shard)
  }
 
  shardPrimary, err := s.ts.GetTablet(ctx, shard.MasterAlias)
@@ -1164,7 +1164,11 @@ func (s *VtctldServer) ReparentTablet(ctx context.Context, req *vtctldatapb.Repa
  }
 
  if shardPrimary.Keyspace != tablet.Keyspace || shardPrimary.Shard != tablet.Shard {
- return nil, vterrors.Errorf(vtrpc.Code_FAILED_PRECONDITION, "master %v and potential replica %v not in same keypace shard (%v/%v)", topoproto.TabletAliasString(shard.MasterAlias), topoproto.TabletAliasString(req.Tablet), tablet.Keyspace, tablet.Shard)
+ return nil, vterrors.Errorf(vtrpc.Code_FAILED_PRECONDITION, "primary %v and potential replica %v not in same keypace shard (%v/%v)", topoproto.TabletAliasString(shard.MasterAlias), topoproto.TabletAliasString(req.Tablet), tablet.Keyspace, tablet.Shard)
+ }
+
+ if topoproto.TabletAliasEqual(req.Tablet, shardPrimary.Alias) {
+ return nil, vterrors.Errorf(vtrpc.Code_FAILED_PRECONDITION, "cannot ReparentTablet current shard primary (%v) onto itself", topoproto.TabletAliasString(req.Tablet))
  }
 
  if err := s.tmc.SetMaster(ctx, tablet.Tablet, shard.MasterAlias, 0, "", false); err != nil {

go/vt/vtctl/grpcvtctldserver/server_test.go

@@ -4580,6 +4580,44 @@ func TestReparentTablet(t *testing.T) {
  expected: nil,
  shouldErr: true,
  },
+ {
+ name: "requested tablet is shard primary",
+ tablets: []*topodatapb.Tablet{
+ {
+ Alias: &topodatapb.TabletAlias{
+ Cell: "zone1",
+ Uid: 100,
+ },
+ Type: topodatapb.TabletType_MASTER,
+ Keyspace: "testkeyspace",
+ Shard: "-",
+ },
+ },
+ shards: []*vtctldatapb.Shard{
+ {
+ Keyspace: "testkeyspace",
+ Name: "-",
+ Shard: &topodatapb.Shard{
+ MasterAlias: &topodatapb.TabletAlias{
+ Cell: "zone1",
+ Uid: 100,
+ },
+ MasterTermStartTime: &vttime.Time{
+ Seconds: 1010,
+ },
+ IsMasterServing: true,
+ },
+ },
+ },
+ req: &vtctldatapb.ReparentTabletRequest{
+ Tablet: &topodatapb.TabletAlias{
+ Cell: "zone1",
+ Uid: 100,
+ },
+ },
+ expected: nil,
+ shouldErr: true,
+ },
  {
  name: "tmc.SetMaster failure",
  tmc: &testutil.TabletManagerClient{

go/vt/wrangler/reparent.go

@@ -99,38 +99,10 @@ func (wr *Wrangler) ShardReplicationStatuses(ctx context.Context, keyspace, shar
 // master, based on the current replication position. If there is no
 // match, it will fail.
 func (wr *Wrangler) ReparentTablet(ctx context.Context, tabletAlias *topodatapb.TabletAlias) error {
- // Get specified tablet.
- // Get current shard master tablet.
- // Sanity check they are in the same keyspace/shard.
- // Issue a SetMaster to the tablet.
- ti, err := wr.ts.GetTablet(ctx, tabletAlias)
- if err != nil {
- return err
- }
-
- shardInfo, err := wr.ts.GetShard(ctx, ti.Keyspace, ti.Shard)
- if err != nil {
- return err
- }
- if !shardInfo.HasMaster() {
- return fmt.Errorf("no master tablet for shard %v/%v", ti.Keyspace, ti.Shard)
- }
-
- masterTi, err := wr.ts.GetTablet(ctx, shardInfo.MasterAlias)
- if err != nil {
- return err
- }
-
- // Basic sanity checking.
- if masterTi.Type != topodatapb.TabletType_MASTER {
- return fmt.Errorf("TopologyServer has inconsistent state for shard master %v", topoproto.TabletAliasString(shardInfo.MasterAlias))
- }
- if masterTi.Keyspace != ti.Keyspace || masterTi.Shard != ti.Shard {
- return fmt.Errorf("master %v and potential replica not in same keyspace/shard", topoproto.TabletAliasString(shardInfo.MasterAlias))
- }
-
- // and do the remote command
- return wr.tmc.SetMaster(ctx, ti.Tablet, shardInfo.MasterAlias, 0, "", false)
+ _, err := wr.vtctld.ReparentTablet(ctx, &vtctldatapb.ReparentTabletRequest{
+ Tablet: tabletAlias,
+ })
+ return err
 }
 
 // InitShardMaster will make the provided tablet the master for the shard.