fix(instance): fix pvc names of all instances

apis/core/v1alpha1/tiflash_types.go

@@ -41,6 +41,12 @@ const (
 	DefaultTiFlashPortProxyStatus = 20292
 )
 
+const (
+	// VolumeUsageTypeTiFlashData is the main data dir for the tiflash
+	// The default sub path of this type is ""
+	VolumeUsageTypeTiFlashData VolumeUsageType = "data"
+)
+
 const (
 	TiFlashCondHealth   = "Health"
 	TiFlashHealthReason = "TiFlashHealth"

pkg/configs/tiflash/config.go

@@ -102,14 +102,29 @@ func (c *Config) Overlay(cluster *v1alpha1.Cluster, tiflash *v1alpha1.TiFlash) e
 		c.Security.KeyPath = path.Join(v1alpha1.TiFlashClusterTLSMountPath, corev1.TLSPrivateKeyKey)
 	}
 
-	c.TmpPath = getTmpPath(tiflash)
-	c.Storage.Main.Dir = []string{getMainStorageDir(tiflash)}
-	c.Storage.Raft.Dir = []string{getRaftStorageDir(tiflash)}
+	for i := range tiflash.Spec.Volumes {
+		vol := &tiflash.Spec.Volumes[i]
+		for _, usage := range vol.For {
+			if usage.Type != v1alpha1.VolumeUsageTypeTiFlashData {
+				continue
+			}
+			dataDir := vol.Path
+			if usage.SubPath != "" {
+				dataDir = path.Join(vol.Path, usage.SubPath)
+			}
+
+			c.TmpPath = getTmpPath(dataDir)
+			c.Storage.Main.Dir = []string{getMainStorageDir(dataDir)}
+			c.Storage.Raft.Dir = []string{getRaftStorageDir(dataDir)}
+			c.Flash.Proxy.DataDir = getProxyDataDir(dataDir)
+			c.Logger.Log = GetServerLogPath(dataDir)
+			c.Logger.Errorlog = GetErrorLogPath(dataDir)
+		}
+	}
 
 	c.Flash.ServiceAddr = GetServiceAddr(tiflash)
 	// /etc/tiflash/proxy.toml
 	c.Flash.Proxy.Config = path.Join(v1alpha1.DirNameConfigTiFlash, v1alpha1.ConfigFileTiFlashProxyName)
-	c.Flash.Proxy.DataDir = getProxyDataDir(tiflash)
 	c.Flash.Proxy.Addr = getProxyAddr(tiflash)
 	c.Flash.Proxy.AdvertiseAddr = getProxyAdvertiseAddr(tiflash)
 	c.Flash.Proxy.AdvertiseStatusAddr = getProxyAdvertiseStatusAddr(tiflash)
@@ -118,9 +133,6 @@ func (c *Config) Overlay(cluster *v1alpha1.Cluster, tiflash *v1alpha1.TiFlash) e
 
 	c.Status.MetricsPort = int(tiflash.GetMetricsPort())
 
-	c.Logger.Log = GetServerLogPath(tiflash)
-	c.Logger.Errorlog = GetErrorLogPath(tiflash)
-
 	return nil
 }
 
@@ -217,33 +229,26 @@ func getProxyAdvertiseStatusAddr(tiflash *v1alpha1.TiFlash) string {
 	return fmt.Sprintf("%s.%s.%s:%d", tiflash.PodName(), tiflash.Spec.Subdomain, ns, tiflash.GetProxyStatusPort())
 }
 
-func GetServerLogPath(tiflash *v1alpha1.TiFlash) string {
-	return fmt.Sprintf("%s/logs/server.log", getDefaultMountPath(tiflash))
-}
-
-func GetErrorLogPath(tiflash *v1alpha1.TiFlash) string {
-	return fmt.Sprintf("%s/logs/error.log", getDefaultMountPath(tiflash))
+func GetServerLogPath(dataDir string) string {
+	return fmt.Sprintf("%s/logs/server.log", dataDir)
 }
 
-func getTmpPath(tiflash *v1alpha1.TiFlash) string {
-	return fmt.Sprintf("%s/tmp", getDefaultMountPath(tiflash))
+func GetErrorLogPath(dataDir string) string {
+	return fmt.Sprintf("%s/logs/error.log", dataDir)
 }
 
-func getMainStorageDir(tiflash *v1alpha1.TiFlash) string {
-	return fmt.Sprintf("%s/db", getDefaultMountPath(tiflash))
+func getTmpPath(dataDir string) string {
+	return fmt.Sprintf("%s/tmp", dataDir)
 }
 
-func getRaftStorageDir(tiflash *v1alpha1.TiFlash) string {
-	return fmt.Sprintf("%s/kvstore", getDefaultMountPath(tiflash))
+func getMainStorageDir(dataDir string) string {
+	return fmt.Sprintf("%s/db", dataDir)
 }
 
-func getProxyDataDir(tiflash *v1alpha1.TiFlash) string {
-	return fmt.Sprintf("%s/proxy", getDefaultMountPath(tiflash))
+func getRaftStorageDir(dataDir string) string {
+	return fmt.Sprintf("%s/kvstore", dataDir)
 }
 
-// in TiDB Operator v1, we mount the first data volume to /data0,
-// so for an existing TiFlash cluster, we should set the first data volume mount path to /data0.
-func getDefaultMountPath(tiflash *v1alpha1.TiFlash) string {
-	vol := tiflash.Spec.Volumes[0]
-	return vol.Path
+func getProxyDataDir(dataDir string) string {
+	return fmt.Sprintf("%s/proxy", dataDir)
 }

pkg/configs/tiflash/config_test.go

@@ -110,6 +110,11 @@ func TestOverlay(t *testing.T) {
 					{
 						Name: "data",
 						Path: "/data0",
+						For: []v1alpha1.VolumeUsage{
+							{
+								Type: v1alpha1.VolumeUsageTypeTiFlashData,
+							},
+						},
 					},
 				},
 			},

pkg/controllers/pd/tasks/pod_test.go

@@ -307,7 +307,7 @@ func TestTaskPod(t *testing.T) {
 					pod: fake.FakeObj("aaa-pd-xxx", func(obj *corev1.Pod) *corev1.Pod {
 						obj.Labels = map[string]string{
 							v1alpha1.LabelKeyConfigHash:  "old",
-							v1alpha1.LabelKeyPodSpecHash: "7cd7474797",
+							v1alpha1.LabelKeyPodSpecHash: "6d6499ffc7",
 						}
 						return obj
 					}),
@@ -316,7 +316,6 @@ func TestTaskPod(t *testing.T) {
 			},
 
 			expectedPodIsTerminating: true,
-			expectUpdatedPod:         false,
 			expectedStatus:           task.SWait,
 		},
 		{

pkg/controllers/pd/tasks/pvc_test.go

@@ -83,7 +83,7 @@ func TestTaskPVC(t *testing.T) {
 				}),
 			},
 			pvcs: []*corev1.PersistentVolumeClaim{
-				fake.FakeObj("pd-aaa-pd-xxx-data", func(obj *corev1.PersistentVolumeClaim) *corev1.PersistentVolumeClaim {
+				fake.FakeObj("data-aaa-pd-xxx", func(obj *corev1.PersistentVolumeClaim) *corev1.PersistentVolumeClaim {
 					obj.Status.Phase = corev1.ClaimBound
 					return obj
 				}),
@@ -105,7 +105,7 @@ func TestTaskPVC(t *testing.T) {
 				}),
 			},
 			pvcs: []*corev1.PersistentVolumeClaim{
-				fake.FakeObj("pd-aaa-pd-xxx-data", func(obj *corev1.PersistentVolumeClaim) *corev1.PersistentVolumeClaim {
+				fake.FakeObj("data-aaa-pd-xxx", func(obj *corev1.PersistentVolumeClaim) *corev1.PersistentVolumeClaim {
 					obj.Status.Phase = corev1.ClaimBound
 					return obj
 				}),

pkg/controllers/pd/tasks/util.go

@@ -25,10 +25,8 @@ import (
 
 func PersistentVolumeClaimName(podName, volName string) string {
 	// ref: https://github.com/pingcap/tidb-operator/blob/v1.6.0/pkg/apis/pingcap/v1alpha1/helpers.go#L92
-	if volName == "" {
-		return "pd-" + podName
-	}
-	return "pd-" + podName + "-" + volName
+	// NOTE: for v1, should use component as volName of data, e.g. pd
+	return volName + "-" + podName
 }
 
 func LongestHealthPeer(pd *v1alpha1.PD, peers []*v1alpha1.PD) string {

pkg/controllers/tidb/tasks/cm_test.go

@@ -0,0 +1,179 @@
+// Copyright 2024 PingCAP, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package tasks
+
+import (
+	"context"
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"github.com/pingcap/tidb-operator/apis/core/v1alpha1"
+	"github.com/pingcap/tidb-operator/pkg/client"
+	"github.com/pingcap/tidb-operator/pkg/utils/fake"
+	"github.com/pingcap/tidb-operator/pkg/utils/task/v3"
+)
+
+const fakePDAddr = "any string, useless in test"
+
+func TestTaskConfigMap(t *testing.T) {
+	cases := []struct {
+		desc          string
+		state         *ReconcileContext
+		objs          []client.Object
+		unexpectedErr bool
+		invalidConfig bool
+
+		expectedStatus task.Status
+	}{
+		{
+			desc: "no config",
+			state: &ReconcileContext{
+				State: &state{
+					tidb: fake.FakeObj[v1alpha1.TiDB]("aaa-xxx"),
+					cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster {
+						obj.Status.PD = fakePDAddr
+						return obj
+					}),
+				},
+			},
+			expectedStatus: task.SComplete,
+		},
+		{
+			desc: "invalid config",
+			state: &ReconcileContext{
+				State: &state{
+					tidb: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiDB) *v1alpha1.TiDB {
+						obj.Spec.Config = `invalid`
+						return obj
+					}),
+					cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster {
+						obj.Status.PD = fakePDAddr
+						return obj
+					}),
+				},
+			},
+			invalidConfig:  true,
+			expectedStatus: task.SFail,
+		},
+		{
+			desc: "with managed field",
+			state: &ReconcileContext{
+				State: &state{
+					tidb: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiDB) *v1alpha1.TiDB {
+						obj.Spec.Config = `store = 'xxx'`
+						return obj
+					}),
+					cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster {
+						obj.Status.PD = fakePDAddr
+						return obj
+					}),
+				},
+			},
+			invalidConfig:  true,
+			expectedStatus: task.SFail,
+		},
+		{
+			desc: "has config map",
+			state: &ReconcileContext{
+				State: &state{
+					tidb: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiDB) *v1alpha1.TiDB {
+						return obj
+					}),
+					cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster {
+						obj.Status.PD = fakePDAddr
+						return obj
+					}),
+				},
+			},
+			objs: []client.Object{
+				&corev1.ConfigMap{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "aaa-tidb-xxx",
+					},
+				},
+			},
+			expectedStatus: task.SComplete,
+		},
+		{
+			desc: "update config map failed",
+			state: &ReconcileContext{
+				State: &state{
+					tidb: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiDB) *v1alpha1.TiDB {
+						return obj
+					}),
+					cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster {
+						obj.Status.PD = fakePDAddr
+						return obj
+					}),
+				},
+			},
+			objs: []client.Object{
+				&corev1.ConfigMap{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "aaa-tidb-xxx",
+					},
+				},
+			},
+			unexpectedErr: true,
+
+			expectedStatus: task.SFail,
+		},
+	}
+
+	for i := range cases {
+		c := &cases[i]
+		t.Run(c.desc, func(tt *testing.T) {
+			tt.Parallel()
+
+			ctx := context.Background()
+			var objs []client.Object
+			objs = append(objs, c.state.TiDB(), c.state.Cluster())
+			fc := client.NewFakeClient(objs...)
+			for _, obj := range c.objs {
+				require.NoError(tt, fc.Apply(ctx, obj), c.desc)
+			}
+
+			if c.unexpectedErr {
+				// cannot update svc
+				fc.WithError("patch", "*", errors.NewInternalError(fmt.Errorf("fake internal err")))
+			}
+
+			res, done := task.RunTask(ctx, TaskConfigMap(c.state, fc))
+			assert.Equal(tt, c.expectedStatus.String(), res.Status().String(), res.Message())
+			assert.False(tt, done, c.desc)
+
+			if !c.invalidConfig {
+				// config hash should be set
+				assert.NotEmpty(tt, c.state.ConfigHash, c.desc)
+			}
+
+			if c.expectedStatus == task.SComplete {
+				cm := corev1.ConfigMap{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "aaa-tidb-xxx",
+					},
+				}
+				require.NoError(tt, fc.Get(ctx, client.ObjectKeyFromObject(&cm), &cm), c.desc)
+				assert.Equal(tt, c.state.ConfigHash, cm.Labels[v1alpha1.LabelKeyConfigHash], c.desc)
+			}
+		})
+	}
+}