Fix locking logic and related tests

Signed-off-by: Rohit Nayak <rohit@planetscale.com>

go/vt/topo/keyspace_routing_rules_lock.go

@@ -19,6 +19,9 @@ package topo
 import (
 	"context"
 	"fmt"
+	"path"
+
+	"vitess.io/vitess/go/vt/log"
 )
 
 // KeyspaceRoutingRulesLock is a wrapper over TopoLock, to serialize updates to the keyspace routing rules.
@@ -29,15 +32,41 @@ type KeyspaceRoutingRulesLock struct {
 	sourceKeyspace string
 }
 
+func checkAndCreateLocksFile(ctx context.Context, ts *Server) error {
+	topoPath := path.Join(KeyspaceRoutingRulesPath, "lock")
+	_, _, err := ts.GetGlobalCell().Get(ctx, topoPath)
+	if IsErrType(err, NoNode) {
+		log.Infof("Creating keyspace routing rules file %s", topoPath)
+		_, err = ts.globalCell.Create(ctx, topoPath, []byte("lock file for keyspace routing rules"))
+		if err != nil {
+			log.Errorf("Failed to create keyspace routing rules lock file: %v", err)
+		} else {
+			_, _, err := ts.GetGlobalCell().Get(ctx, topoPath)
+			if err != nil {
+				log.Errorf("Failed to read keyspace routing rules lock file: %v", err)
+			}
+		}
+	}
+	return err
+}
+
 func NewKeyspaceRoutingRulesLock(ctx context.Context, ts *Server, sourceKeyspace string) (*KeyspaceRoutingRulesLock, error) {
 	if sourceKeyspace == "" {
 		return nil, fmt.Errorf("sourceKeyspace is not specified")
 	}
+
+	// TODO: check if this can be done better: catch is that we never explicitly create keyspaces routing rules file,
+	// unless required.
+	if err := checkAndCreateLocksFile(ctx, ts); err != nil {
+		log.Errorf("Failed to create keyspace routing rules lock file: %v", err)
+		return nil, err
+	}
+
 	return &KeyspaceRoutingRulesLock{
 		TopoLock: &TopoLock{
-			Root:   "", // global
-			Key:    KeyspaceRoutingRulesFile,
-			Action: "KeyspaceRoutingRulesLock",
+			Root:   KeyspaceRoutingRulesPath,
+			Key:    "",
+			Action: "Lock",
 			Name:   fmt.Sprintf("KeyspaceRoutingRules for %s", sourceKeyspace),
 			ts:     ts,
 		},

go/vt/topo/keyspace_routing_rules_lock_test.go

@@ -32,10 +32,8 @@ func TestKeyspaceRoutingRulesLock(t *testing.T) {
 	defer cancel()
 	ts := memorytopo.NewServer(ctx, "zone1")
 	defer ts.Close()
-	conn := ts.GetGlobalConn()
+	conn := ts.GetGlobalCell()
 	require.NotNil(t, conn)
-	_, err := conn.Create(ctx, topo.KeyspaceRoutingRulesFile, []byte(""))
-	require.NoError(t, err)
 
 	currentTopoLockTimeout := topo.LockTimeout
 	topo.LockTimeout = testLockTimeout

go/vt/topo/server.go

@@ -84,13 +84,15 @@ const (
 
 // Path for all object types.
 const (
-	CellsPath             = "cells"
-	CellsAliasesPath      = "cells_aliases"
-	KeyspacesPath         = "keyspaces"
-	ShardsPath            = "shards"
-	TabletsPath           = "tablets"
-	MetadataPath          = "metadata"
-	ExternalClusterVitess = "vitess"
+	CellsPath                   = "cells"
+	CellsAliasesPath            = "cells_aliases"
+	KeyspacesPath               = "keyspaces"
+	ShardsPath                  = "shards"
+	TabletsPath                 = "tablets"
+	MetadataPath                = "metadata"
+	ExternalClusterVitess       = "vitess"
+	KeyspaceRoutingRulesPath    = "keyspaces_routing_rules"
+	KeyspaceRoutingRulesLockDir = "lock"
 )
 
 // Factory is a factory interface to create Conn objects.
@@ -352,7 +354,7 @@ func (ts *Server) Close() {
 	ts.cellConns = make(map[string]cellConn)
 }
 
-func (ts *Server) GetGlobalConn() Conn {
+func (ts *Server) GetGlobalCell() Conn {
 	return ts.globalCell
 }
 

go/vt/topo/topo_lock_test.go

@@ -36,7 +36,7 @@ func TestTopoLockTimeout(t *testing.T) {
 	defer cancel()
 	ts := memorytopo.NewServer(ctx, "zone1")
 	defer ts.Close()
-	conn := ts.GetGlobalConn()
+	conn := ts.GetGlobalCell()
 	require.NotNil(t, conn)
 	_, err := conn.Create(ctx, "root/key1", []byte("value"))
 	if err != nil {
@@ -67,7 +67,7 @@ func TestTopoLockBasic(t *testing.T) {
 	defer cancel()
 	ts := memorytopo.NewServer(ctx, "zone1")
 	defer ts.Close()
-	conn := ts.GetGlobalConn()
+	conn := ts.GetGlobalCell()
 	require.NotNil(t, conn)
 	_, err := conn.Create(ctx, "root/key1", []byte("value"))
 	if err != nil {

go/vt/vtctl/workflow/utils_test.go

@@ -0,0 +1,21 @@
+package workflow
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"vitess.io/vitess/go/vt/topo/memorytopo"
+)
+
+func TestUpdateKeyspaceRoutingRule(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	ts := memorytopo.NewServer(ctx, "zone1")
+	defer ts.Close()
+	routes := make(map[string]string)
+	routes["from"] = "to"
+	err := updateKeyspaceRoutingRule(ctx, ts, "ks", routes)
+	require.NoError(t, err)
+}