diff --git a/apis/core/v1alpha1/tiflash_types.go b/apis/core/v1alpha1/tiflash_types.go index fe09572cab..41baf0b810 100644 --- a/apis/core/v1alpha1/tiflash_types.go +++ b/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" diff --git a/pkg/configs/tiflash/config.go b/pkg/configs/tiflash/config.go index 4b3358bbf2..5d1ff09293 100644 --- a/pkg/configs/tiflash/config.go +++ b/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) } diff --git a/pkg/configs/tiflash/config_test.go b/pkg/configs/tiflash/config_test.go index c1f902e8e3..1982d50c4e 100644 --- a/pkg/configs/tiflash/config_test.go +++ b/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, + }, + }, }, }, }, diff --git a/pkg/controllers/pd/tasks/pvc_test.go b/pkg/controllers/pd/tasks/pvc_test.go index 1933144dde..cd863b9589 100644 --- a/pkg/controllers/pd/tasks/pvc_test.go +++ b/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 }), diff --git a/pkg/controllers/pd/tasks/util.go b/pkg/controllers/pd/tasks/util.go index d0281664b9..51f5602f39 100644 --- a/pkg/controllers/pd/tasks/util.go +++ b/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 { diff --git a/pkg/controllers/tidb/tasks/cm_test.go b/pkg/controllers/tidb/tasks/cm_test.go new file mode 100644 index 0000000000..17742a2203 --- /dev/null +++ b/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) + } + }) + } +} diff --git a/pkg/controllers/tidb/tasks/pvc.go b/pkg/controllers/tidb/tasks/pvc.go index 380e996328..5ba0b83a6f 100644 --- a/pkg/controllers/tidb/tasks/pvc.go +++ b/pkg/controllers/tidb/tasks/pvc.go @@ -23,12 +23,13 @@ import ( "github.com/pingcap/tidb-operator/apis/core/v1alpha1" "github.com/pingcap/tidb-operator/pkg/client" + "github.com/pingcap/tidb-operator/pkg/controllers/common" maputil "github.com/pingcap/tidb-operator/pkg/utils/map" "github.com/pingcap/tidb-operator/pkg/utils/task/v3" "github.com/pingcap/tidb-operator/pkg/volumes" ) -func TaskPVC(state *ReconcileContext, logger logr.Logger, c client.Client, vm volumes.Modifier) task.Task { +func TaskPVC(state common.TiDBState, logger logr.Logger, c client.Client, vm volumes.Modifier) task.Task { return task.NameTaskFunc("PVC", func(ctx context.Context) task.Result { pvcs := newPVCs(state.TiDB()) if wait, err := volumes.SyncPVCs(ctx, c, pvcs, vm, logger); err != nil { diff --git a/pkg/controllers/tidb/tasks/pvc_test.go b/pkg/controllers/tidb/tasks/pvc_test.go new file mode 100644 index 0000000000..f1fb423f37 --- /dev/null +++ b/pkg/controllers/tidb/tasks/pvc_test.go @@ -0,0 +1,165 @@ +// 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/go-logr/logr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/resource" + + "github.com/pingcap/tidb-operator/apis/core/v1alpha1" + "github.com/pingcap/tidb-operator/pkg/client" + "github.com/pingcap/tidb-operator/pkg/controllers/common" + "github.com/pingcap/tidb-operator/pkg/utils/fake" + "github.com/pingcap/tidb-operator/pkg/utils/task/v3" + "github.com/pingcap/tidb-operator/pkg/volumes" +) + +func TestTaskPVC(t *testing.T) { + cases := []struct { + desc string + state common.TiDBState + pvcs []*corev1.PersistentVolumeClaim + unexpectedErr bool + + expectedStatus task.Status + expectedPVCNum int + }{ + { + desc: "no pvc", + state: &state{ + tidb: fake.FakeObj[v1alpha1.TiDB]("aaa-xxx"), + }, + expectedStatus: task.SComplete, + expectedPVCNum: 0, + }, + { + desc: "create a data vol", + state: &state{ + tidb: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiDB) *v1alpha1.TiDB { + obj.Spec.Volumes = []v1alpha1.Volume{ + { + Name: "data", + Storage: resource.MustParse("10Gi"), + }, + } + return obj + }), + }, + expectedStatus: task.SComplete, + expectedPVCNum: 1, + }, + { + desc: "has a data vol", + state: &state{ + tidb: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiDB) *v1alpha1.TiDB { + obj.Spec.Volumes = []v1alpha1.Volume{ + { + Name: "data", + Storage: resource.MustParse("10Gi"), + }, + } + return obj + }), + }, + pvcs: []*corev1.PersistentVolumeClaim{ + fake.FakeObj("data-aaa-tidb-xxx", func(obj *corev1.PersistentVolumeClaim) *corev1.PersistentVolumeClaim { + obj.Status.Phase = corev1.ClaimBound + return obj + }), + }, + expectedStatus: task.SComplete, + expectedPVCNum: 1, + }, + { + desc: "has a data vol, but failed to apply", + state: &state{ + tidb: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiDB) *v1alpha1.TiDB { + obj.Spec.Volumes = []v1alpha1.Volume{ + { + Name: "data", + Storage: resource.MustParse("10Gi"), + }, + } + return obj + }), + }, + pvcs: []*corev1.PersistentVolumeClaim{ + fake.FakeObj("data-aaa-tidb-xxx", func(obj *corev1.PersistentVolumeClaim) *corev1.PersistentVolumeClaim { + obj.Status.Phase = corev1.ClaimBound + return obj + }), + }, + unexpectedErr: true, + + expectedStatus: task.SFail, + expectedPVCNum: 1, + }, + } + + 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()) + fc := client.NewFakeClient(objs...) + for _, obj := range c.pvcs { + require.NoError(tt, fc.Apply(ctx, obj), c.desc) + } + + ctrl := gomock.NewController(tt) + vm := volumes.NewMockModifier(ctrl) + expectedPVCs := newPVCs(c.state.TiDB()) + for _, expected := range expectedPVCs { + for _, current := range c.pvcs { + if current.Name == expected.Name { + vm.EXPECT().GetActualVolume(ctx, expected, current).Return(&volumes.ActualVolume{ + Desired: &volumes.DesiredVolume{}, + PVC: current, + }, nil) + vm.EXPECT().ShouldModify(ctx, &volumes.ActualVolume{ + Desired: &volumes.DesiredVolume{}, + PVC: current, + }).Return(false) + } + } + } + + if c.unexpectedErr { + // cannot update pvc + fc.WithError("patch", "*", errors.NewInternalError(fmt.Errorf("fake internal err"))) + } + + res, done := task.RunTask(ctx, TaskPVC(c.state, logr.Discard(), fc, vm)) + assert.Equal(tt, c.expectedStatus.String(), res.Status().String(), res.Message()) + assert.False(tt, done, c.desc) + + pvcs := corev1.PersistentVolumeClaimList{} + require.NoError(tt, fc.List(ctx, &pvcs), c.desc) + assert.Len(tt, pvcs.Items, c.expectedPVCNum, c.desc) + }) + } +} diff --git a/pkg/controllers/tidb/tasks/util.go b/pkg/controllers/tidb/tasks/util.go index 0940a5c54c..3fc4502fb6 100644 --- a/pkg/controllers/tidb/tasks/util.go +++ b/pkg/controllers/tidb/tasks/util.go @@ -26,10 +26,8 @@ func ConfigMapName(tidbName string) string { 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 "tidb-" + podName - } - return "tidb-" + podName + "-" + volName + // NOTE: for v1, should use component as volName of data, e.g. tidb + return volName + "-" + podName } // TiDBServiceURL returns the service URL of a tidb member. diff --git a/pkg/controllers/tiflash/tasks/cm.go b/pkg/controllers/tiflash/tasks/cm.go index 9597b3615b..2643b51e02 100644 --- a/pkg/controllers/tiflash/tasks/cm.go +++ b/pkg/controllers/tiflash/tasks/cm.go @@ -57,6 +57,7 @@ func TaskConfigMap(state *ReconcileContext, c client.Client) task.Task { return task.Fail().With("tiflash proxy config cannot be encoded: %w", err) } + // TODO(liubo02): config hash should be generated by tiflash config + proxy config state.ConfigHash, err = hasher.GenerateHash(state.TiFlash().Spec.Config) if err != nil { return task.Fail().With("failed to generate hash for `tiflash.spec.config`: %w", err) diff --git a/pkg/controllers/tiflash/tasks/cm_test.go b/pkg/controllers/tiflash/tasks/cm_test.go new file mode 100644 index 0000000000..7de7fa11bc --- /dev/null +++ b/pkg/controllers/tiflash/tasks/cm_test.go @@ -0,0 +1,213 @@ +// 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{ + tiflash: fake.FakeObj[v1alpha1.TiFlash]("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{ + tiflash: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + 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: "invalid proxy config", + state: &ReconcileContext{ + State: &state{ + tiflash: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Spec.ProxyConfig = `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: "config with managed field", + state: &ReconcileContext{ + State: &state{ + tiflash: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Spec.Config = `tmp_path = '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: "proxy config with managed field", + state: &ReconcileContext{ + State: &state{ + tiflash: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Spec.ProxyConfig = `server.status-addr = '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{ + tiflash: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + 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-tiflash-xxx", + }, + }, + }, + expectedStatus: task.SComplete, + }, + { + desc: "update config map failed", + state: &ReconcileContext{ + State: &state{ + tiflash: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + 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-tiflash-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.TiFlash(), 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-tiflash-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) + } + }) + } +} diff --git a/pkg/controllers/tiflash/tasks/pod.go b/pkg/controllers/tiflash/tasks/pod.go index 9ffb0896bc..4b3e2c7382 100644 --- a/pkg/controllers/tiflash/tasks/pod.go +++ b/pkg/controllers/tiflash/tasks/pod.go @@ -17,6 +17,7 @@ package tasks import ( "context" "fmt" + "path" "path/filepath" corev1 "k8s.io/api/core/v1" @@ -94,19 +95,16 @@ func newPod(cluster *v1alpha1.Cluster, tiflash *v1alpha1.TiFlash, configHash str }, } - var firstMount *corev1.VolumeMount + var dataMount *corev1.VolumeMount + var dataDir string for i := range tiflash.Spec.Volumes { vol := &tiflash.Spec.Volumes[i] - name := v1alpha1.NamePrefix + "tiflash" - if vol.Name != "" { - name = name + "-" + vol.Name - } + name := v1alpha1.NamePrefix + "-" + vol.Name vols = append(vols, corev1.Volume{ Name: name, VolumeSource: corev1.VolumeSource{ PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ - // the format is "data{i}-tiflash-xxx" to compatible with TiDB Operator v1 - ClaimName: PersistentVolumeClaimName(tiflash.PodName(), i), + ClaimName: PersistentVolumeClaimName(tiflash.PodName(), vol.Name), }, }, }) @@ -115,8 +113,14 @@ func newPod(cluster *v1alpha1.Cluster, tiflash *v1alpha1.TiFlash, configHash str MountPath: vol.Path, } mounts = append(mounts, mount) - if i == 0 { - firstMount = &mount + for _, usage := range vol.For { + if usage.Type == v1alpha1.VolumeUsageTypeTiFlashData { + dataMount = &mount + dataDir = vol.Path + if usage.SubPath != "" { + dataDir = path.Join(vol.Path, usage.SubPath) + } + } } } @@ -154,8 +158,8 @@ func newPod(cluster *v1alpha1.Cluster, tiflash *v1alpha1.TiFlash, configHash str Subdomain: tiflash.Spec.Subdomain, NodeSelector: tiflash.Spec.Topology, InitContainers: []corev1.Container{ - *buildLogTailerContainer(tiflash, v1alpha1.TiFlashServerLogContainerName, tiflashcfg.GetServerLogPath(tiflash), firstMount), - *buildLogTailerContainer(tiflash, v1alpha1.TiFlashErrorLogContainerName, tiflashcfg.GetErrorLogPath(tiflash), firstMount), + *buildLogTailerContainer(tiflash, v1alpha1.TiFlashServerLogContainerName, tiflashcfg.GetServerLogPath(dataDir), dataMount), + *buildLogTailerContainer(tiflash, v1alpha1.TiFlashErrorLogContainerName, tiflashcfg.GetErrorLogPath(dataDir), dataMount), }, Containers: []corev1.Container{ { diff --git a/pkg/controllers/tiflash/tasks/pvc.go b/pkg/controllers/tiflash/tasks/pvc.go index fd84b9eaa0..4ce4815c11 100644 --- a/pkg/controllers/tiflash/tasks/pvc.go +++ b/pkg/controllers/tiflash/tasks/pvc.go @@ -24,12 +24,13 @@ import ( "github.com/pingcap/tidb-operator/apis/core/v1alpha1" "github.com/pingcap/tidb-operator/pkg/client" + "github.com/pingcap/tidb-operator/pkg/controllers/common" maputil "github.com/pingcap/tidb-operator/pkg/utils/map" "github.com/pingcap/tidb-operator/pkg/utils/task/v3" "github.com/pingcap/tidb-operator/pkg/volumes" ) -func TaskPVC(state *ReconcileContext, logger logr.Logger, c client.Client, vm volumes.Modifier) task.Task { +func TaskPVC(state common.TiFlashState, logger logr.Logger, c client.Client, vm volumes.Modifier) task.Task { return task.NameTaskFunc("PVC", func(ctx context.Context) task.Result { pvcs := newPVCs(state.TiFlash()) if wait, err := volumes.SyncPVCs(ctx, c, pvcs, vm, logger); err != nil { @@ -48,8 +49,7 @@ func newPVCs(tiflash *v1alpha1.TiFlash) []*corev1.PersistentVolumeClaim { vol := &tiflash.Spec.Volumes[i] pvcs = append(pvcs, &corev1.PersistentVolumeClaim{ ObjectMeta: metav1.ObjectMeta{ - // the format is "data{i}-tiflash-xxx" to compatible with TiDB Operator v1 - Name: PersistentVolumeClaimName(tiflash.PodName(), i), + Name: PersistentVolumeClaimName(tiflash.PodName(), vol.Name), Namespace: tiflash.Namespace, Labels: maputil.Merge(tiflash.Labels, map[string]string{ v1alpha1.LabelKeyInstance: tiflash.Name, diff --git a/pkg/controllers/tiflash/tasks/pvc_test.go b/pkg/controllers/tiflash/tasks/pvc_test.go new file mode 100644 index 0000000000..3a8bf0359c --- /dev/null +++ b/pkg/controllers/tiflash/tasks/pvc_test.go @@ -0,0 +1,165 @@ +// 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/go-logr/logr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/resource" + + "github.com/pingcap/tidb-operator/apis/core/v1alpha1" + "github.com/pingcap/tidb-operator/pkg/client" + "github.com/pingcap/tidb-operator/pkg/controllers/common" + "github.com/pingcap/tidb-operator/pkg/utils/fake" + "github.com/pingcap/tidb-operator/pkg/utils/task/v3" + "github.com/pingcap/tidb-operator/pkg/volumes" +) + +func TestTaskPVC(t *testing.T) { + cases := []struct { + desc string + state common.TiFlashState + pvcs []*corev1.PersistentVolumeClaim + unexpectedErr bool + + expectedStatus task.Status + expectedPVCNum int + }{ + { + desc: "no pvc", + state: &state{ + tiflash: fake.FakeObj[v1alpha1.TiFlash]("aaa-xxx"), + }, + expectedStatus: task.SComplete, + expectedPVCNum: 0, + }, + { + desc: "create a data vol", + state: &state{ + tiflash: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Spec.Volumes = []v1alpha1.Volume{ + { + Name: "data", + Storage: resource.MustParse("10Gi"), + }, + } + return obj + }), + }, + expectedStatus: task.SComplete, + expectedPVCNum: 1, + }, + { + desc: "has a data vol", + state: &state{ + tiflash: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Spec.Volumes = []v1alpha1.Volume{ + { + Name: "data", + Storage: resource.MustParse("10Gi"), + }, + } + return obj + }), + }, + pvcs: []*corev1.PersistentVolumeClaim{ + fake.FakeObj("data-aaa-tiflash-xxx", func(obj *corev1.PersistentVolumeClaim) *corev1.PersistentVolumeClaim { + obj.Status.Phase = corev1.ClaimBound + return obj + }), + }, + expectedStatus: task.SComplete, + expectedPVCNum: 1, + }, + { + desc: "has a data vol, but failed to apply", + state: &state{ + tiflash: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Spec.Volumes = []v1alpha1.Volume{ + { + Name: "data", + Storage: resource.MustParse("10Gi"), + }, + } + return obj + }), + }, + pvcs: []*corev1.PersistentVolumeClaim{ + fake.FakeObj("data-aaa-tiflash-xxx", func(obj *corev1.PersistentVolumeClaim) *corev1.PersistentVolumeClaim { + obj.Status.Phase = corev1.ClaimBound + return obj + }), + }, + unexpectedErr: true, + + expectedStatus: task.SFail, + expectedPVCNum: 1, + }, + } + + 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.TiFlash()) + fc := client.NewFakeClient(objs...) + for _, obj := range c.pvcs { + require.NoError(tt, fc.Apply(ctx, obj), c.desc) + } + + ctrl := gomock.NewController(tt) + vm := volumes.NewMockModifier(ctrl) + expectedPVCs := newPVCs(c.state.TiFlash()) + for _, expected := range expectedPVCs { + for _, current := range c.pvcs { + if current.Name == expected.Name { + vm.EXPECT().GetActualVolume(ctx, expected, current).Return(&volumes.ActualVolume{ + Desired: &volumes.DesiredVolume{}, + PVC: current, + }, nil) + vm.EXPECT().ShouldModify(ctx, &volumes.ActualVolume{ + Desired: &volumes.DesiredVolume{}, + PVC: current, + }).Return(false) + } + } + } + + if c.unexpectedErr { + // cannot update pvc + fc.WithError("patch", "*", errors.NewInternalError(fmt.Errorf("fake internal err"))) + } + + res, done := task.RunTask(ctx, TaskPVC(c.state, logr.Discard(), fc, vm)) + assert.Equal(tt, c.expectedStatus.String(), res.Status().String(), res.Message()) + assert.False(tt, done, c.desc) + + pvcs := corev1.PersistentVolumeClaimList{} + require.NoError(tt, fc.List(ctx, &pvcs), c.desc) + assert.Len(tt, pvcs.Items, c.expectedPVCNum, c.desc) + }) + } +} diff --git a/pkg/controllers/tiflash/tasks/util.go b/pkg/controllers/tiflash/tasks/util.go index d4e6d3d357..2588dd07c2 100644 --- a/pkg/controllers/tiflash/tasks/util.go +++ b/pkg/controllers/tiflash/tasks/util.go @@ -16,11 +16,12 @@ package tasks import "fmt" -func ConfigMapName(tiflashName string) string { - return tiflashName +func ConfigMapName(podName string) string { + return podName } -func PersistentVolumeClaimName(tiflashName string, volIndex int) string { +func PersistentVolumeClaimName(podName, volName string) string { // ref: https://github.com/pingcap/tidb-operator/blob/486cc85c8380efc4f36b3125a1abba9e3146a2c8/pkg/apis/pingcap/v1alpha1/helpers.go#L105 - return fmt.Sprintf("data%d-%s", volIndex, tiflashName) + // NOTE: for v1, volName should be data0, data1, ... + return fmt.Sprintf("%s-%s", volName, podName) } diff --git a/pkg/controllers/tikv/tasks/cm_test.go b/pkg/controllers/tikv/tasks/cm_test.go new file mode 100644 index 0000000000..c29f09bf92 --- /dev/null +++ b/pkg/controllers/tikv/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{ + tikv: fake.FakeObj[v1alpha1.TiKV]("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{ + tikv: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + 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{ + tikv: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Spec.Config = `server.addr = '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{ + tikv: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + 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-tikv-xxx", + }, + }, + }, + expectedStatus: task.SComplete, + }, + { + desc: "update config map failed", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + 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-tikv-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.TiKV(), 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-tikv-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) + } + }) + } +} diff --git a/pkg/controllers/tikv/tasks/pvc.go b/pkg/controllers/tikv/tasks/pvc.go index 7ccda0133e..325fcdeed8 100644 --- a/pkg/controllers/tikv/tasks/pvc.go +++ b/pkg/controllers/tikv/tasks/pvc.go @@ -23,12 +23,13 @@ import ( "github.com/pingcap/tidb-operator/apis/core/v1alpha1" "github.com/pingcap/tidb-operator/pkg/client" + "github.com/pingcap/tidb-operator/pkg/controllers/common" maputil "github.com/pingcap/tidb-operator/pkg/utils/map" "github.com/pingcap/tidb-operator/pkg/utils/task/v3" "github.com/pingcap/tidb-operator/pkg/volumes" ) -func TaskPVC(state *ReconcileContext, logger logr.Logger, c client.Client, vm volumes.Modifier) task.Task { +func TaskPVC(state common.TiKVState, logger logr.Logger, c client.Client, vm volumes.Modifier) task.Task { return task.NameTaskFunc("PVC", func(ctx context.Context) task.Result { pvcs := newPVCs(state.TiKV()) if wait, err := volumes.SyncPVCs(ctx, c, pvcs, vm, logger); err != nil { diff --git a/pkg/controllers/tikv/tasks/pvc_test.go b/pkg/controllers/tikv/tasks/pvc_test.go new file mode 100644 index 0000000000..4d6895697e --- /dev/null +++ b/pkg/controllers/tikv/tasks/pvc_test.go @@ -0,0 +1,165 @@ +// 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/go-logr/logr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/resource" + + "github.com/pingcap/tidb-operator/apis/core/v1alpha1" + "github.com/pingcap/tidb-operator/pkg/client" + "github.com/pingcap/tidb-operator/pkg/controllers/common" + "github.com/pingcap/tidb-operator/pkg/utils/fake" + "github.com/pingcap/tidb-operator/pkg/utils/task/v3" + "github.com/pingcap/tidb-operator/pkg/volumes" +) + +func TestTaskPVC(t *testing.T) { + cases := []struct { + desc string + state common.TiKVState + pvcs []*corev1.PersistentVolumeClaim + unexpectedErr bool + + expectedStatus task.Status + expectedPVCNum int + }{ + { + desc: "no pvc", + state: &state{ + tikv: fake.FakeObj[v1alpha1.TiKV]("aaa-xxx"), + }, + expectedStatus: task.SComplete, + expectedPVCNum: 0, + }, + { + desc: "create a data vol", + state: &state{ + tikv: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Spec.Volumes = []v1alpha1.Volume{ + { + Name: "data", + Storage: resource.MustParse("10Gi"), + }, + } + return obj + }), + }, + expectedStatus: task.SComplete, + expectedPVCNum: 1, + }, + { + desc: "has a data vol", + state: &state{ + tikv: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Spec.Volumes = []v1alpha1.Volume{ + { + Name: "data", + Storage: resource.MustParse("10Gi"), + }, + } + return obj + }), + }, + pvcs: []*corev1.PersistentVolumeClaim{ + fake.FakeObj("data-aaa-tikv-xxx", func(obj *corev1.PersistentVolumeClaim) *corev1.PersistentVolumeClaim { + obj.Status.Phase = corev1.ClaimBound + return obj + }), + }, + expectedStatus: task.SComplete, + expectedPVCNum: 1, + }, + { + desc: "has a data vol, but failed to apply", + state: &state{ + tikv: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Spec.Volumes = []v1alpha1.Volume{ + { + Name: "data", + Storage: resource.MustParse("10Gi"), + }, + } + return obj + }), + }, + pvcs: []*corev1.PersistentVolumeClaim{ + fake.FakeObj("data-aaa-tikv-xxx", func(obj *corev1.PersistentVolumeClaim) *corev1.PersistentVolumeClaim { + obj.Status.Phase = corev1.ClaimBound + return obj + }), + }, + unexpectedErr: true, + + expectedStatus: task.SFail, + expectedPVCNum: 1, + }, + } + + 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.TiKV()) + fc := client.NewFakeClient(objs...) + for _, obj := range c.pvcs { + require.NoError(tt, fc.Apply(ctx, obj), c.desc) + } + + ctrl := gomock.NewController(tt) + vm := volumes.NewMockModifier(ctrl) + expectedPVCs := newPVCs(c.state.TiKV()) + for _, expected := range expectedPVCs { + for _, current := range c.pvcs { + if current.Name == expected.Name { + vm.EXPECT().GetActualVolume(ctx, expected, current).Return(&volumes.ActualVolume{ + Desired: &volumes.DesiredVolume{}, + PVC: current, + }, nil) + vm.EXPECT().ShouldModify(ctx, &volumes.ActualVolume{ + Desired: &volumes.DesiredVolume{}, + PVC: current, + }).Return(false) + } + } + } + + if c.unexpectedErr { + // cannot update pvc + fc.WithError("patch", "*", errors.NewInternalError(fmt.Errorf("fake internal err"))) + } + + res, done := task.RunTask(ctx, TaskPVC(c.state, logr.Discard(), fc, vm)) + assert.Equal(tt, c.expectedStatus.String(), res.Status().String(), res.Message()) + assert.False(tt, done, c.desc) + + pvcs := corev1.PersistentVolumeClaimList{} + require.NoError(tt, fc.List(ctx, &pvcs), c.desc) + assert.Len(tt, pvcs.Items, c.expectedPVCNum, c.desc) + }) + } +} diff --git a/pkg/controllers/tikv/tasks/util.go b/pkg/controllers/tikv/tasks/util.go index fac5809fe7..2dfb41b42a 100644 --- a/pkg/controllers/tikv/tasks/util.go +++ b/pkg/controllers/tikv/tasks/util.go @@ -29,10 +29,8 @@ func ConfigMapName(tikvName string) string { 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 "tikv-" + podName - } - return "tikv-" + podName + "-" + volName + // NOTE: for v1, should use component as volName of data, e.g. tikv + return fmt.Sprintf("%s-%s", volName, podName) } func DeletePodWithGracePeriod(ctx context.Context, c client.Client, pod *corev1.Pod, regionCount int) error {