Merge pull request #432 from yoheimuta/support-more-VitessShardTablet…

…Pool

Add a new field called "name" to VitessShardTabletPool as part of x-kubernetes-list-map-keys

deploy/crds/planetscale.com_vitessclusters.yaml

@@ -1501,6 +1501,9 @@ spec:
                                             required:
                                             - resources
                                             type: object
+                                          name:
+                                            default: ""
+                                            type: string
                                           replicas:
                                             format: int32
                                             minimum: 0
@@ -1571,6 +1574,7 @@ spec:
                                       x-kubernetes-list-map-keys:
                                       - type
                                       - cell
+                                      - name
                                       x-kubernetes-list-type: map
                                   required:
                                   - databaseInitScriptSecret
@@ -1896,6 +1900,9 @@ spec:
                                           required:
                                           - resources
                                           type: object
+                                        name:
+                                          default: ""
+                                          type: string
                                         replicas:
                                           format: int32
                                           minimum: 0
@@ -1966,6 +1973,7 @@ spec:
                                     x-kubernetes-list-map-keys:
                                     - type
                                     - cell
+                                    - name
                                     x-kubernetes-list-type: map
                                 required:
                                 - databaseInitScriptSecret

deploy/crds/planetscale.com_vitesskeyspaces.yaml

@@ -549,6 +549,9 @@ spec:
                                       required:
                                       - resources
                                       type: object
+                                    name:
+                                      default: ""
+                                      type: string
                                     replicas:
                                       format: int32
                                       minimum: 0
@@ -619,6 +622,7 @@ spec:
                                 x-kubernetes-list-map-keys:
                                 - type
                                 - cell
+                                - name
                                 x-kubernetes-list-type: map
                             required:
                             - databaseInitScriptSecret
@@ -944,6 +948,9 @@ spec:
                                     required:
                                     - resources
                                     type: object
+                                  name:
+                                    default: ""
+                                    type: string
                                   replicas:
                                     format: int32
                                     minimum: 0
@@ -1014,6 +1021,7 @@ spec:
                               x-kubernetes-list-map-keys:
                               - type
                               - cell
+                              - name
                               x-kubernetes-list-type: map
                           required:
                           - databaseInitScriptSecret

deploy/crds/planetscale.com_vitessshards.yaml

@@ -532,6 +532,9 @@ spec:
                       required:
                       - resources
                       type: object
+                    name:
+                      default: ""
+                      type: string
                     replicas:
                       format: int32
                       minimum: 0
@@ -602,6 +605,7 @@ spec:
                 x-kubernetes-list-map-keys:
                 - type
                 - cell
+                - name
                 x-kubernetes-list-type: map
               topologyReconciliation:
                 properties:

docs/api/index.html

@@ -7066,6 +7066,21 @@ <h3 id="planetscale.com/v2.VitessShardTabletPool">VitessShardTabletPool
 </tr>
 <tr>
 <td>
+<code>name</code></br>
+<em>
+string
+</em>
+</td>
+<td>
+<p>Name is the pool&rsquo;s unique name within the (cell,type) pair.
+This field is optional, and defaults to an empty.
+Assigning different names to this field enables the existence of multiple pools with a specific tablet type in a given cell,
+which can be beneficial for unmanaged tablets.
+Hence, you must specify ExternalDatastore when assigning a name to this field.</p>
+</td>
+</tr>
+<tr>
+<td>
 <code>replicas</code></br>
 <em>
 int32
@@ -7329,7 +7344,7 @@ <h3 id="planetscale.com/v2.VitessShardTemplate">VitessShardTemplate
 <td>
 <p>TabletPools specify groups of tablets in a given cell with a certain
 tablet type and a shared configuration template.</p>
-<p>There must be at most one pool in this list for each (cell,type) pair.
+<p>There must be at most one pool in this list for each (cell,type,name) set.
 Each shard must have at least one &ldquo;replica&rdquo; pool (in at least one cell)
 in order to be able to serve.</p>
 </td>

pkg/apis/planetscale/v2/labels.go

@@ -35,6 +35,9 @@ const (
 	TabletUidLabel = LabelPrefix + "/" + "tablet-uid"
 	// TabletTypeLabel is the key for identifying the Vitess target tablet type for a Pod.
 	TabletTypeLabel = LabelPrefix + "/" + "tablet-type"
+	// TabletPoolNameLabel is the key for identifying the Vitess target pool name within the (cell,type) pair.
+	// This label is applicable to Vitess-unmanaged keyspaces.
+	TabletPoolNameLabel = LabelPrefix + "/" + "pool-name"
 	// TabletIndexLabel is the key for identifying the index of a Vitess tablet within its pool.
 	TabletIndexLabel = LabelPrefix + "/" + "tablet-index"
 

pkg/apis/planetscale/v2/vitessshard_methods.go

@@ -58,9 +58,9 @@ func (t *VitessTabletPoolType) InitTabletType() string {
 	}
 }
 
-// IsMatch indicates whether a tablet pool matches another tablet pool's type and cell.
+// IsMatch indicates whether a tablet pool matches another tablet pool's type, cell and name.
 func (t *VitessShardTabletPool) IsMatch(inputPool *VitessShardTabletPool) bool {
-	return t.Type == inputPool.Type && t.Cell == inputPool.Cell
+	return t.Type == inputPool.Type && t.Cell == inputPool.Cell && t.Name == inputPool.Name
 }
 
 // UsingExternalDatastore indicates whether the VitessShard Spec is using

pkg/apis/planetscale/v2/vitessshard_types.go

@@ -105,14 +105,15 @@ type VitessShardTemplate struct {
 	// TabletPools specify groups of tablets in a given cell with a certain
 	// tablet type and a shared configuration template.
 	//
-	// There must be at most one pool in this list for each (cell,type) pair.
+	// There must be at most one pool in this list for each (cell,type,name) set.
 	// Each shard must have at least one "replica" pool (in at least one cell)
 	// in order to be able to serve.
 	// +patchMergeKey=type
 	// +patchStrategy=merge
 	// +listType=map
 	// +listMapKey=type
 	// +listMapKey=cell
+	// +listMapKey=name
 	TabletPools []VitessShardTabletPool `json:"tabletPools,omitempty" patchStrategy:"merge" patchMergeKey:"type"`
 
 	// DatabaseInitScriptSecret specifies the init_db.sql script file to use for this shard.
@@ -170,6 +171,14 @@ type VitessShardTabletPool struct {
 	// +kubebuilder:validation:Enum=replica;rdonly;externalmaster;externalreplica;externalrdonly
 	Type VitessTabletPoolType `json:"type"`
 
+	// Name is the pool's unique name within the (cell,type) pair.
+	// This field is optional, and defaults to an empty.
+	// Assigning different names to this field enables the existence of multiple pools with a specific tablet type in a given cell,
+	// which can be beneficial for unmanaged tablets.
+	// Hence, you must specify ExternalDatastore when assigning a name to this field.
+	// +kubebuilder:default=""
+	Name string `json:"name,omitempty"`
+
 	// Replicas is the number of tablets to deploy in this pool.
 	// This field is required, although it may be set to 0,
 	// which will scale the pool down to 0 tablets.

pkg/controller/vitessshard/reconcile_disk.go

@@ -53,6 +53,9 @@ func (r *ReconcileVitessShard) reconcileDisk(ctx context.Context, vts *planetsca
 			continue
 		}
 
+		// In cases where there are multiple pools with the same tablet type in a given cell,
+		// there is a possibility of processing the same tablet multiple times.
+		// We permit this to occur as it is practically harmless and simplifies implementation.
 		poolTablets, err := tabletKeysForPool(vts, tabletPool.Cell, tabletPool.Type)
 		if err != nil {
 			return resultBuilder.Error(err)
@@ -129,6 +132,8 @@ func (r *ReconcileVitessShard) claimForTabletPod(ctx context.Context, pod *v1.Po
 	return pvc, nil
 }
 
+// tabletKeysForPool returns the list of targetKeys for a given pool type and cell.
+// Note that this function does not care about the pool's name assignment.
 func tabletKeysForPool(vts *planetscalev2.VitessShard, poolCell string, poolType planetscalev2.VitessTabletPoolType) ([]string, error) {
 	tabletKeys := vts.Status.TabletAliases()
 

pkg/controller/vitessshard/reconcile_tablets.go

@@ -284,6 +284,11 @@ func vttabletSpecs(vts *planetscalev2.VitessShard, parentLabels map[string]strin
 				Uid:  vttablet.UID(pool.Cell, keyspaceName, vts.Spec.KeyRange, pool.Type, uint32(tabletIndex)),
 			}
 
+			// If TabletPools has multiple pools within the same (cell,type) pair, we need to add a pool name to the UID generator.
+			if pool.ExternalDatastore != nil && 0 < len(pool.Name) {
+				tabletAlias.Uid = vttablet.UIDWithPoolName(pool.Cell, keyspaceName, vts.Spec.KeyRange, pool.Type, uint32(tabletIndex), pool.Name)
+			}
+
 			// Copy parent labels map and add tablet-specific labels.
 			labels := make(map[string]string, len(parentLabels)+4)
 			for k, v := range parentLabels {
@@ -293,6 +298,9 @@ func vttabletSpecs(vts *planetscalev2.VitessShard, parentLabels map[string]strin
 			labels[planetscalev2.TabletUidLabel] = strconv.FormatUint(uint64(tabletAlias.Uid), 10)
 			labels[planetscalev2.TabletTypeLabel] = string(pool.Type)
 			labels[planetscalev2.TabletIndexLabel] = strconv.FormatUint(uint64(tabletIndex), 10)
+			if pool.ExternalDatastore != nil {
+				labels[planetscalev2.TabletPoolNameLabel] = pool.Name
+			}
 
 			// Merge ExtraVitessFlags into the tablet spec ExtraFlags field.
 			extraFlags := make(map[string]string)

pkg/operator/vttablet/uid.go

@@ -51,3 +51,21 @@ func UID(cellName, keyspaceName string, shardKeyRange planetscalev2.VitessKeyRan
 	sum := h.Sum(nil)
 	return binary.BigEndian.Uint32(sum[:4])
 }
+
+/*
+UIDWithPoolName function generates a 32-bit unsigned integer similar to the UID function above.
+
+However, it additionally takes the poolName as an input.
+This allows the generation of a unique UID for a tablet that belongs to a different pool
+but shares other common attributes.
+
+To preserve the existing UID, it is recommended to use the UID function instead of this function
+when the poolName is set to its default value of an empty string.
+*/
+func UIDWithPoolName(cellName, keyspaceName string, shardKeyRange planetscalev2.VitessKeyRange,
+	tabletPoolType planetscalev2.VitessTabletPoolType, tabletName uint32, poolName string) uint32 {
+	h := md5.New()
+	fmt.Fprintln(h, cellName, keyspaceName, shardKeyRange.String(), string(tabletPoolType), tabletName, poolName)
+	sum := h.Sum(nil)
+	return binary.BigEndian.Uint32(sum[:4])
+}

pkg/operator/vttablet/uid_test.go

@@ -38,3 +38,21 @@ func TestUIDHash(t *testing.T) {
 		t.Fatalf("UID() = %v, want %v", got, want)
 	}
 }
+
+// TestUIDWithPoolNameHash checks that nobody changed the hash function for UIDWithPoolName().
+func TestUIDWithPoolNameHash(t *testing.T) {
+	cell := "cell"
+	keyspace := "keyspace"
+	keyRange := planetscalev2.VitessKeyRange{Start: "10", End: "20"}
+	tabletType := planetscalev2.ReplicaPoolType
+	tabletName := uint32(1)
+	poolName := "unmanaged-replica-1"
+
+	// DO NOT CHANGE THIS VALUE!
+	// This is intentionally a change-detection test. If it breaks, you messed up.
+	want := uint32(6333720)
+
+	if got := UIDWithPoolName(cell, keyspace, keyRange, tabletType, tabletName, poolName); got != want {
+		t.Fatalf("UIDWithPoolName() = %v, want %v", got, want)
+	}
+}

test/integration/vitesscluster/vitesscluster_test.go

@@ -77,11 +77,28 @@ spec:
             type: replica
             replicas: 3
             mysqld: {}
-            dataVolumeClaimTemplate:
-              accessModes: [ReadWriteOnce]
-              resources:
-                requests:
-                  storage: 1Gi
+            externalDatastore:
+              port: 3306
+              credentialsSecret:
+                name: cluster-config
+                key: db_credentials.json
+          - cell: cell3
+            type: rdonly
+            replicas: 3
+            externalDatastore:
+              port: 3307
+              credentialsSecret:
+                name: cluster-config
+                key: db_credentials.json
+          - cell: cell3
+            type: rdonly
+            replicas: 3
+            name: unmanaged-replica-2
+            externalDatastore:
+              port: 3308
+              credentialsSecret:
+                name: cluster-config
+                key: db_credentials.json
   backup:
     locations:
     - name: vbs1
@@ -181,9 +198,9 @@ func verifyBasicVitessKeyspace(f *framework.Fixture, ns, cluster, keyspace strin
 	f.MustGet(ns, names.JoinWithConstraints(names.DefaultConstraints, cluster, keyspace), &planetscalev2.VitessKeyspace{})
 
 	// VitessKeyspaces create VitessShards.
-	verifyBasicVitessShard(f, ns, cluster, keyspace, "x-80", []int{3, 3, 0})
-	verifyBasicVitessShard(f, ns, cluster, keyspace, "80-x", []int{3, 3, 0})
-	verifyBasicVitessShard(f, ns, cluster, keyspace, "x-x", []int{0, 0, 3})
+	verifyBasicVitessShard(f, ns, cluster, keyspace, "x-80", []int{3, 3})
+	verifyBasicVitessShard(f, ns, cluster, keyspace, "80-x", []int{3, 3})
+	verifyBasicVitessShardExternal(f, ns, cluster, keyspace, "x-x", []int{3, 3, 3})
 }
 
 func verifyBasicVitessShard(f *framework.Fixture, ns, cluster, keyspace, shard string, expectedTabletCount []int) {
@@ -198,10 +215,6 @@ func verifyBasicVitessShard(f *framework.Fixture, ns, cluster, keyspace, shard s
 		Namespace:     ns,
 		LabelSelector: tabletPodSelector(cluster, keyspace, shard, "cell2", "rdonly"),
 	}, expectedTabletCount[1])
-	cell3Pods := f.ExpectPods(&client.ListOptions{
-		Namespace:     ns,
-		LabelSelector: tabletPodSelector(cluster, keyspace, shard, "cell3", "replica"),
-	}, expectedTabletCount[2])
 
 	// Each vttablet Pod should have a PVC.
 	for i := range cell1Pods.Items {
@@ -210,15 +223,30 @@ func verifyBasicVitessShard(f *framework.Fixture, ns, cluster, keyspace, shard s
 	for i := range cell2Pods.Items {
 		f.MustGet(ns, cell2Pods.Items[i].Name, &corev1.PersistentVolumeClaim{})
 	}
-	for i := range cell3Pods.Items {
-		f.MustGet(ns, cell3Pods.Items[i].Name, &corev1.PersistentVolumeClaim{})
-	}
 
 	// VitessShard creates vtbackup-init Pod/PVC.
 	f.MustGet(ns, names.JoinWithConstraints(names.DefaultConstraints, cluster, keyspace, shard, "vtbackup", "init"), &corev1.Pod{})
 	f.MustGet(ns, names.JoinWithConstraints(names.DefaultConstraints, cluster, keyspace, shard, "vtbackup", "init"), &corev1.PersistentVolumeClaim{})
 }
 
+func verifyBasicVitessShardExternal(f *framework.Fixture, ns, cluster, keyspace, shard string, expectedTabletCount []int) {
+	f.MustGet(ns, names.JoinWithConstraints(names.DefaultConstraints, cluster, keyspace, shard), &planetscalev2.VitessShard{})
+
+	// VitessShard creates vttablet Pods.
+	f.ExpectPods(&client.ListOptions{
+		Namespace:     ns,
+		LabelSelector: tabletPodExternalSelector(cluster, keyspace, shard, "cell3", "replica", ""),
+	}, expectedTabletCount[0])
+	f.ExpectPods(&client.ListOptions{
+		Namespace:     ns,
+		LabelSelector: tabletPodExternalSelector(cluster, keyspace, shard, "cell3", "rdonly", ""),
+	}, expectedTabletCount[1])
+	f.ExpectPods(&client.ListOptions{
+		Namespace:     ns,
+		LabelSelector: tabletPodExternalSelector(cluster, keyspace, shard, "cell3", "rdonly", "unmanaged-replica-2"),
+	}, expectedTabletCount[2])
+}
+
 func tabletPodSelector(cluster, keyspace, shard, cell, tabletType string) apilabels.Selector {
 	// This intentionally does NOT use any shared constants because we want the
 	// test to fail if the labels change, since that's a breaking change.
@@ -230,3 +258,16 @@ func tabletPodSelector(cluster, keyspace, shard, cell, tabletType string) apilab
 		"planetscale.com/tablet-type": tabletType,
 	}.AsSelector()
 }
+
+func tabletPodExternalSelector(cluster, keyspace, shard, cell, tabletType, poolName string) apilabels.Selector {
+	// This intentionally does NOT use any shared constants because we want the
+	// test to fail if the labels change, since that's a breaking change.
+	return apilabels.Set{
+		"planetscale.com/cluster":     cluster,
+		"planetscale.com/keyspace":    keyspace,
+		"planetscale.com/shard":       shard,
+		"planetscale.com/cell":        cell,
+		"planetscale.com/tablet-type": tabletType,
+		"planetscale.com/pool-name":   poolName,
+	}.AsSelector()
+}