Switch to the new TabletExternallyReparented. (bitpoke#92)

* Switch to the new TabletExternallyReparented.

Upstream Vitess moved TER from TabletManagerClient to Wrangler. We
stayed on the old one for a while because the new one requires that
users have upgraded to a certain minimum Vitess version. It has been
long enough now that I think we can assume that's the case.

Signed-off-by: Anthony Yeh <enisoc@planetscale.com>

* Set demote_master_type=SPARE on externalmaster tablets.

Signed-off-by: Anthony Yeh <enisoc@planetscale.com>

pkg/controller/vitessshardreplication/reconcile_drain.go

@@ -297,7 +297,7 @@ func (r *ReconcileVitessShard) reconcileDrain(ctx context.Context, vts *planetsc
 
 	var reparentErr error
 	if vts.Spec.UsingExternalDatastore() {
-		reparentErr = r.handleExternalReparent(ctx, vts, wr, newMaster.Tablet, shard.MasterAlias)
+		reparentErr = r.handleExternalReparent(ctx, vts, wr, newMaster.Alias, shard.MasterAlias)
 	} else {
 		reparentErr = wr.PlannedReparentShard(reparentCtx, keyspaceName, vts.Spec.Name, newMaster.Alias, nil, plannedReparentTimeout)
 	}
@@ -313,17 +313,12 @@ func (r *ReconcileVitessShard) reconcileDrain(ctx context.Context, vts *planetsc
 	return resultBuilder.Result()
 }
 
-func (r *ReconcileVitessShard) handleExternalReparent(ctx context.Context, vts *planetscalev2.VitessShard, wr *wrangler.Wrangler, newMasterTablet *topodatapb.Tablet, oldMasterAlias *topodatapb.TabletAlias) error {
-	err := wr.TabletManagerClient().TabletExternallyReparented(ctx, newMasterTablet, "")
+func (r *ReconcileVitessShard) handleExternalReparent(ctx context.Context, vts *planetscalev2.VitessShard, wr *wrangler.Wrangler, newMasterAlias, oldMasterAlias *topodatapb.TabletAlias) error {
+	err := wr.TabletExternallyReparented(ctx, newMasterAlias)
 
 	if err == nil {
-
-		// TODO: Create a specialized function to take the place of TER, that sets the old master as spare, rather than
-		// replica.  Currently TER sets the old master as type replica, and it does so asyncronously.  This may cause
-		// a race where we are left with the old master being of type replica, even though we try to explicitely set it to
-		// be of type spare.
-		// TODO: Related to above todo - we need a more robust way to make sure old masters definitely get set to spare.
-		// A retry may never reach this point.
+		// TODO: Remove this after all externalmaster tablets have been updated
+		// to set the -demote_master_type=SPARE flag.
 		err = wr.ChangeSlaveType(ctx, oldMasterAlias, topodatapb.TabletType_SPARE)
 	}
 

pkg/controller/vitessshardreplication/tablet_externally_reparent.go

@@ -90,21 +90,13 @@ func (r *ReconcileVitessShard) tabletExternallyReparent(ctx context.Context, vts
 		return resultBuilder.Result()
 	}
 
-	// Get the tablet record from topology.
-	ti, err := wr.TopoServer().GetTablet(ctx, masterCandidateAlias)
-	if err != nil {
-		r.recorder.Eventf(vts, corev1.EventTypeWarning, "ExternalMasterShardBlocked", "can't externally reparent shard: %v", err)
-		return resultBuilder.RequeueAfter(replicationRequeueDelay)
-	}
-
-	masterCandidate := ti.Tablet
-
 	// All checks passed. Do TabletExternallyReparented.
-	if err := wr.TabletManagerClient().TabletExternallyReparented(ctx, masterCandidate, ""); err != nil {
+
+	if err := wr.TabletExternallyReparented(ctx, masterCandidateAlias); err != nil {
 		r.recorder.Eventf(vts, corev1.EventTypeWarning, "TabletExternallyReparentedFailed", "failed to externally reparent shard: %v", err)
 		return resultBuilder.RequeueAfter(replicationRequeueDelay)
 	}
 
-	r.recorder.Eventf(vts, corev1.EventTypeNormal, "TabletExternallyReparented", "Externally reparented tablet %v", topoproto.TabletAliasString(masterCandidate.Alias))
+	r.recorder.Eventf(vts, corev1.EventTypeNormal, "TabletExternallyReparented", "Externally reparented tablet %v", topoproto.TabletAliasString(masterCandidateAlias))
 	return resultBuilder.Result()
 }

pkg/operator/vttablet/datastore.go

@@ -18,6 +18,7 @@ package vttablet
 
 import (
 	corev1 "k8s.io/api/core/v1"
+	planetscalev2 "planetscale.dev/vitess-operator/pkg/apis/planetscale/v2"
 	"planetscale.dev/vitess-operator/pkg/operator/lazy"
 	"planetscale.dev/vitess-operator/pkg/operator/secrets"
 	"planetscale.dev/vitess-operator/pkg/operator/vitess"
@@ -109,7 +110,7 @@ func externalDatastoreSSLCAFlags(spec *Spec) vitess.Flags {
 func externalDatastoreFlags(spec *Spec) vitess.Flags {
 	credentialsFile := secrets.Mount(&spec.ExternalDatastore.CredentialsSecret, externalDatastoreCredentialsDirName)
 
-	return vitess.Flags{
+	flags := vitess.Flags{
 		"disable_active_reparents": true,
 		"restore_from_backup":      false,
 		"db_app_user":              spec.ExternalDatastore.User,
@@ -129,4 +130,10 @@ func externalDatastoreFlags(spec *Spec) vitess.Flags {
 		"enforce_strict_trans_tables": false,
 		"vreplication_tablet_type":    vreplicationTabletType,
 	}
+
+	if spec.Type == planetscalev2.ExternalMasterPoolType {
+		flags["demote_master_type"] = "SPARE"
+	}
+
+	return flags
 }