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/pod_test.go b/pkg/controllers/pd/tasks/pod_test.go index eabe13cbec..908f769264 100644 --- a/pkg/controllers/pd/tasks/pod_test.go +++ b/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, }, { 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/finalizer_test.go b/pkg/controllers/tidb/tasks/finalizer_test.go new file mode 100644 index 0000000000..51212969c5 --- /dev/null +++ b/pkg/controllers/tidb/tasks/finalizer_test.go @@ -0,0 +1,279 @@ +// 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" + "strconv" + "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" +) + +func TestTaskFinalizerDel(t *testing.T) { + now := metav1.Now() + cases := []struct { + desc string + state *ReconcileContext + subresources []client.Object + unexpectedErr bool + + expectedStatus task.Status + expectedObj *v1alpha1.TiDB + }{ + { + desc: "no sub resources, no finalizer", + state: &ReconcileContext{ + State: &state{ + tidb: fake.FakeObj("aaa", func(obj *v1alpha1.TiDB) *v1alpha1.TiDB { + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + obj.SetDeletionTimestamp(&now) + return obj + }), + }, + }, + + expectedStatus: task.SComplete, + expectedObj: fake.FakeObj("aaa", func(obj *v1alpha1.TiDB) *v1alpha1.TiDB { + return obj + }), + }, + { + desc: "has sub resources, has finalizer", + state: &ReconcileContext{ + State: &state{ + tidb: fake.FakeObj("aaa", func(obj *v1alpha1.TiDB) *v1alpha1.TiDB { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + obj.SetDeletionTimestamp(&now) + return obj + }), + }, + }, + subresources: fakeSubresources("Pod", "ConfigMap", "PersistentVolumeClaim"), + + expectedStatus: task.SWait, + expectedObj: fake.FakeObj("aaa", func(obj *v1alpha1.TiDB) *v1alpha1.TiDB { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + }, + { + desc: "has sub resources, has finalizer, failed to del subresources(pod)", + state: &ReconcileContext{ + State: &state{ + tidb: fake.FakeObj("aaa", func(obj *v1alpha1.TiDB) *v1alpha1.TiDB { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + obj.SetDeletionTimestamp(&now) + return obj + }), + }, + }, + subresources: fakeSubresources("Pod"), + + unexpectedErr: true, + + expectedStatus: task.SFail, + expectedObj: fake.FakeObj("aaa", func(obj *v1alpha1.TiDB) *v1alpha1.TiDB { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + }, + { + desc: "has sub resources, has finalizer, failed to del subresources(cm)", + state: &ReconcileContext{ + State: &state{ + tidb: fake.FakeObj("aaa", func(obj *v1alpha1.TiDB) *v1alpha1.TiDB { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + obj.SetDeletionTimestamp(&now) + return obj + }), + }, + }, + subresources: fakeSubresources("ConfigMap"), + + unexpectedErr: true, + + expectedStatus: task.SFail, + expectedObj: fake.FakeObj("aaa", func(obj *v1alpha1.TiDB) *v1alpha1.TiDB { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + }, + { + desc: "has sub resources, has finalizer, failed to del subresources(pvc)", + state: &ReconcileContext{ + State: &state{ + tidb: fake.FakeObj("aaa", func(obj *v1alpha1.TiDB) *v1alpha1.TiDB { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + obj.SetDeletionTimestamp(&now) + return obj + }), + }, + }, + subresources: fakeSubresources("PersistentVolumeClaim"), + + unexpectedErr: true, + + expectedStatus: task.SFail, + expectedObj: fake.FakeObj("aaa", func(obj *v1alpha1.TiDB) *v1alpha1.TiDB { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + }, + { + desc: "no sub resources, has finalizer", + state: &ReconcileContext{ + State: &state{ + tidb: fake.FakeObj("aaa", func(obj *v1alpha1.TiDB) *v1alpha1.TiDB { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + obj.SetDeletionTimestamp(&now) + return obj + }), + }, + }, + + expectedStatus: task.SComplete, + expectedObj: fake.FakeObj("aaa", func(obj *v1alpha1.TiDB) *v1alpha1.TiDB { + obj.Finalizers = []string{} + return obj + }), + }, + { + desc: "no sub resources, has finalizer, failed to remove finalizer", + state: &ReconcileContext{ + State: &state{ + tidb: fake.FakeObj("aaa", func(obj *v1alpha1.TiDB) *v1alpha1.TiDB { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + obj.SetDeletionTimestamp(&now) + return obj + }), + }, + }, + unexpectedErr: true, + + expectedStatus: task.SFail, + }, + } + + for i := range cases { + c := &cases[i] + t.Run(c.desc, func(tt *testing.T) { + tt.Parallel() + + objs := []client.Object{ + c.state.TiDB(), + } + + objs = append(objs, c.subresources...) + + fc := client.NewFakeClient(objs...) + if c.unexpectedErr { + // cannot remove finalizer + fc.WithError("update", "*", errors.NewInternalError(fmt.Errorf("fake internal err"))) + // cannot delete sub resources + fc.WithError("delete", "*", errors.NewInternalError(fmt.Errorf("fake internal err"))) + } + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + res, done := task.RunTask(ctx, TaskFinalizerDel(c.state, fc)) + assert.Equal(tt, c.expectedStatus.String(), res.Status().String(), c.desc) + assert.False(tt, done, c.desc) + + // no need to check update result + if c.unexpectedErr { + return + } + + pd := &v1alpha1.TiDB{} + require.NoError(tt, fc.Get(ctx, client.ObjectKey{Name: "aaa"}, pd), c.desc) + assert.Equal(tt, c.expectedObj, pd, c.desc) + }) + } +} + +func fakeSubresources(types ...string) []client.Object { + var objs []client.Object + for i, t := range types { + var obj client.Object + switch t { + case "Pod": + obj = fake.FakeObj(strconv.Itoa(i), func(obj *corev1.Pod) *corev1.Pod { + obj.Labels = map[string]string{ + v1alpha1.LabelKeyManagedBy: v1alpha1.LabelValManagedByOperator, + v1alpha1.LabelKeyInstance: "aaa", + v1alpha1.LabelKeyCluster: "", + v1alpha1.LabelKeyComponent: v1alpha1.LabelValComponentTiDB, + } + return obj + }) + case "ConfigMap": + obj = fake.FakeObj(strconv.Itoa(i), func(obj *corev1.ConfigMap) *corev1.ConfigMap { + obj.Labels = map[string]string{ + v1alpha1.LabelKeyManagedBy: v1alpha1.LabelValManagedByOperator, + v1alpha1.LabelKeyInstance: "aaa", + v1alpha1.LabelKeyCluster: "", + v1alpha1.LabelKeyComponent: v1alpha1.LabelValComponentTiDB, + } + return obj + }) + case "PersistentVolumeClaim": + obj = fake.FakeObj(strconv.Itoa(i), func(obj *corev1.PersistentVolumeClaim) *corev1.PersistentVolumeClaim { + obj.Labels = map[string]string{ + v1alpha1.LabelKeyManagedBy: v1alpha1.LabelValManagedByOperator, + v1alpha1.LabelKeyInstance: "aaa", + v1alpha1.LabelKeyCluster: "", + v1alpha1.LabelKeyComponent: v1alpha1.LabelValComponentTiDB, + } + return obj + }) + } + if obj != nil { + objs = append(objs, obj) + } + } + + return objs +} diff --git a/pkg/controllers/tidb/tasks/pod.go b/pkg/controllers/tidb/tasks/pod.go index c74a1d21a9..29e8a85280 100644 --- a/pkg/controllers/tidb/tasks/pod.go +++ b/pkg/controllers/tidb/tasks/pod.go @@ -73,7 +73,7 @@ func TaskPod(state *ReconcileContext, c client.Client) task.Task { } state.PodIsTerminating = true - return task.Complete().With("pod is deleting") + return task.Wait().With("pod is deleting") } else if res == k8s.CompareResultUpdate { logger.Info("will update the pod in place") if err := c.Apply(ctx, expected); err != nil { diff --git a/pkg/controllers/tidb/tasks/pod_test.go b/pkg/controllers/tidb/tasks/pod_test.go new file mode 100644 index 0000000000..223de30212 --- /dev/null +++ b/pkg/controllers/tidb/tasks/pod_test.go @@ -0,0 +1,275 @@ +// 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" + + "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 fakeVersion = "v1.2.3" + +func TestTaskPod(t *testing.T) { + cases := []struct { + desc string + state *ReconcileContext + objs []client.Object + // if true, cannot apply pod + unexpectedErr bool + + expectUpdatedPod bool + expectedPodIsTerminating bool + expectedStatus task.Status + }{ + { + desc: "no pod", + state: &ReconcileContext{ + State: &state{ + tidb: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiDB) *v1alpha1.TiDB { + obj.Spec.Version = fakeVersion + return obj + }), + cluster: fake.FakeObj[v1alpha1.Cluster]("aaa"), + }, + }, + + expectUpdatedPod: true, + expectedStatus: task.SComplete, + }, + { + desc: "no pod, failed to apply", + state: &ReconcileContext{ + State: &state{ + tidb: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiDB) *v1alpha1.TiDB { + obj.Spec.Version = fakeVersion + return obj + }), + cluster: fake.FakeObj[v1alpha1.Cluster]("aaa"), + }, + }, + unexpectedErr: true, + + expectedStatus: task.SFail, + }, + { + desc: "pod spec changed", + state: &ReconcileContext{ + State: &state{ + tidb: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiDB) *v1alpha1.TiDB { + obj.Spec.Version = fakeVersion + return obj + }), + cluster: fake.FakeObj[v1alpha1.Cluster]("aaa"), + pod: fake.FakeObj("aaa-tidb-xxx", func(obj *corev1.Pod) *corev1.Pod { + return obj + }), + }, + }, + + expectedPodIsTerminating: true, + expectedStatus: task.SWait, + }, + { + desc: "pod spec changed, failed to delete", + state: &ReconcileContext{ + State: &state{ + tidb: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiDB) *v1alpha1.TiDB { + obj.Spec.Version = fakeVersion + return obj + }), + cluster: fake.FakeObj[v1alpha1.Cluster]("aaa"), + pod: fake.FakeObj("aaa-tidb-xxx", func(obj *corev1.Pod) *corev1.Pod { + return obj + }), + }, + }, + unexpectedErr: true, + + expectedStatus: task.SFail, + }, + { + desc: "pod spec hash not changed, config changed, hot reload policy", + state: &ReconcileContext{ + State: &state{ + tidb: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiDB) *v1alpha1.TiDB { + obj.Spec.Version = fakeVersion + obj.Spec.UpdateStrategy.Config = v1alpha1.ConfigUpdateStrategyHotReload + return obj + }), + cluster: fake.FakeObj[v1alpha1.Cluster]("aaa"), + pod: fake.FakeObj("aaa-tidb-xxx", func(obj *corev1.Pod) *corev1.Pod { + obj.Labels = map[string]string{ + v1alpha1.LabelKeyConfigHash: "newest", + v1alpha1.LabelKeyPodSpecHash: "5656465c8c", + } + return obj + }), + }, + ConfigHash: "newest", + }, + + expectUpdatedPod: true, + expectedStatus: task.SComplete, + }, + { + desc: "pod spec hash not changed, config changed, restart policy", + state: &ReconcileContext{ + State: &state{ + tidb: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiDB) *v1alpha1.TiDB { + obj.Spec.Version = fakeVersion + obj.Spec.UpdateStrategy.Config = v1alpha1.ConfigUpdateStrategyRestart + return obj + }), + cluster: fake.FakeObj[v1alpha1.Cluster]("aaa"), + pod: fake.FakeObj("aaa-tidb-xxx", func(obj *corev1.Pod) *corev1.Pod { + obj.Labels = map[string]string{ + v1alpha1.LabelKeyConfigHash: "old", + v1alpha1.LabelKeyPodSpecHash: "5656465c8c", + } + return obj + }), + }, + ConfigHash: "newest", + }, + + expectedPodIsTerminating: true, + expectedStatus: task.SWait, + }, + { + desc: "pod spec hash not changed, pod labels changed, config not changed", + state: &ReconcileContext{ + State: &state{ + tidb: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiDB) *v1alpha1.TiDB { + obj.Spec.Version = fakeVersion + obj.Spec.UpdateStrategy.Config = v1alpha1.ConfigUpdateStrategyRestart + return obj + }), + cluster: fake.FakeObj[v1alpha1.Cluster]("aaa"), + pod: fake.FakeObj("aaa-tidb-xxx", func(obj *corev1.Pod) *corev1.Pod { + obj.Labels = map[string]string{ + v1alpha1.LabelKeyConfigHash: "newest", + v1alpha1.LabelKeyPodSpecHash: "5656465c8c", + "xxx": "yyy", + } + return obj + }), + }, + ConfigHash: "newest", + }, + + expectUpdatedPod: true, + expectedStatus: task.SComplete, + }, + { + desc: "pod spec hash not changed, pod labels changed, config not changed, apply failed", + state: &ReconcileContext{ + State: &state{ + tidb: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiDB) *v1alpha1.TiDB { + obj.Spec.Version = fakeVersion + obj.Spec.UpdateStrategy.Config = v1alpha1.ConfigUpdateStrategyRestart + return obj + }), + cluster: fake.FakeObj[v1alpha1.Cluster]("aaa"), + pod: fake.FakeObj("aaa-tidb-xxx", func(obj *corev1.Pod) *corev1.Pod { + obj.Labels = map[string]string{ + v1alpha1.LabelKeyConfigHash: "newest", + v1alpha1.LabelKeyPodSpecHash: "5656465c8c", + "xxx": "yyy", + } + return obj + }), + }, + ConfigHash: "newest", + }, + unexpectedErr: true, + + expectedStatus: task.SFail, + }, + { + desc: "all are not changed", + state: &ReconcileContext{ + State: &state{ + tidb: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiDB) *v1alpha1.TiDB { + obj.Spec.Version = fakeVersion + obj.Spec.UpdateStrategy.Config = v1alpha1.ConfigUpdateStrategyRestart + return obj + }), + cluster: fake.FakeObj[v1alpha1.Cluster]("aaa"), + pod: fake.FakeObj("aaa-tidb-xxx", func(obj *corev1.Pod) *corev1.Pod { + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstance: "aaa-xxx", + v1alpha1.LabelKeyConfigHash: "newest", + v1alpha1.LabelKeyPodSpecHash: "5656465c8c", + } + return obj + }), + }, + ConfigHash: "newest", + }, + + expectedStatus: task.SComplete, + }, + } + + 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()) + if c.state.Pod() != nil { + objs = append(objs, c.state.Pod()) + } + fc := client.NewFakeClient(objs...) + for _, obj := range c.objs { + require.NoError(tt, fc.Apply(ctx, obj), c.desc) + } + + if c.unexpectedErr { + // cannot update pod + fc.WithError("patch", "*", errors.NewInternalError(fmt.Errorf("fake internal err"))) + fc.WithError("delete", "*", errors.NewInternalError(fmt.Errorf("fake internal err"))) + } + + res, done := task.RunTask(ctx, TaskPod(c.state, fc)) + assert.Equal(tt, c.expectedStatus.String(), res.Status().String(), res.Message()) + assert.False(tt, done, c.desc) + + assert.Equal(tt, c.expectedPodIsTerminating, c.state.PodIsTerminating, c.desc) + + if c.expectUpdatedPod { + expectedPod := newPod(c.state.Cluster(), c.state.TiDB(), c.state.GracefulWaitTimeInSeconds, c.state.ConfigHash) + actual := c.state.Pod().DeepCopy() + actual.Kind = "" + actual.APIVersion = "" + actual.ManagedFields = nil + assert.Equal(tt, expectedPod, actual, 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/status.go b/pkg/controllers/tidb/tasks/status.go index 4666010845..f7fe1bf93d 100644 --- a/pkg/controllers/tidb/tasks/status.go +++ b/pkg/controllers/tidb/tasks/status.go @@ -32,6 +32,8 @@ const ( ) // TODO(liubo02): extract to common task +// +//nolint:gocyclo // refactor if possible func TaskStatus(state *ReconcileContext, c client.Client) task.Task { return task.NameTaskFunc("Status", func(ctx context.Context) task.Result { needUpdate := false @@ -63,10 +65,17 @@ func TaskStatus(state *ReconcileContext, c client.Client) task.Task { } } - if !state.Healthy || !v1alpha1.IsUpToDate(state.TiDB()) { - // can we only rely on Pod status events to trigger the retry? - // TODO(liubo02): change to task.Wait - return task.Retry(defaultTaskWaitDuration).With("tidb may not be healthy, requeue to retry") + if !healthy { + // TODO(liubo02): delete pod should retrigger the events, try to change to wait + if state.PodIsTerminating { + return task.Retry(defaultTaskWaitDuration).With("pod may be terminating, requeue to retry") + } + + if pod != nil && statefulset.IsPodRunningAndReady(pod) { + return task.Retry(defaultTaskWaitDuration).With("tidb is not healthy, requeue to retry") + } + + return task.Wait().With("pod of tidb is not ready, wait") } return task.Complete().With("status is synced") @@ -107,9 +116,6 @@ func syncSuspendCond(tidb *v1alpha1.TiDB) bool { // TODO: move to utils func SetIfChanged[T comparable](dst *T, src T) bool { - if src == *new(T) { - return false - } if *dst != src { *dst = src return true diff --git a/pkg/controllers/tidb/tasks/status_test.go b/pkg/controllers/tidb/tasks/status_test.go new file mode 100644 index 0000000000..e0bcf4b066 --- /dev/null +++ b/pkg/controllers/tidb/tasks/status_test.go @@ -0,0 +1,332 @@ +// 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 ( + newRevision = "new" + oldRevision = "old" + + fakeTiDBName = "aaa-xxx" +) + +func TestTaskStatus(t *testing.T) { + now := metav1.Now() + cases := []struct { + desc string + state *ReconcileContext + unexpectedErr bool + + expectedStatus task.Status + expectedObj *v1alpha1.TiDB + }{ + { + desc: "no pod but healthy", + state: &ReconcileContext{ + State: &state{ + tidb: fake.FakeObj(fakeTiDBName, func(obj *v1alpha1.TiDB) *v1alpha1.TiDB { + obj.Generation = 3 + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: newRevision, + } + obj.Status.CurrentRevision = "keep" + obj.Status.ObservedGeneration = 3 + return obj + }), + }, + Healthy: true, + }, + + expectedStatus: task.SWait, + expectedObj: fake.FakeObj(fakeTiDBName, func(obj *v1alpha1.TiDB) *v1alpha1.TiDB { + obj.Generation = 3 + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: newRevision, + } + + obj.Status.ObservedGeneration = 3 + obj.Status.UpdateRevision = newRevision + obj.Status.CurrentRevision = "keep" + obj.Status.Conditions = []metav1.Condition{ + { + Type: v1alpha1.CondHealth, + Status: metav1.ConditionFalse, + ObservedGeneration: 3, + Reason: "Unhealthy", + Message: "instance is not healthy", + }, + { + Type: v1alpha1.CondSuspended, + Status: metav1.ConditionFalse, + ObservedGeneration: 3, + Reason: v1alpha1.ReasonUnsuspended, + Message: "instace is not suspended", + }, + } + + return obj + }), + }, + { + desc: "pod is healthy", + state: &ReconcileContext{ + State: &state{ + tidb: fake.FakeObj(fakeTiDBName, func(obj *v1alpha1.TiDB) *v1alpha1.TiDB { + obj.Generation = 3 + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: newRevision, + } + return obj + }), + pod: fake.FakeObj("aaa-tidb-xxx", func(obj *corev1.Pod) *corev1.Pod { + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: oldRevision, + } + obj.Status.Phase = corev1.PodRunning + obj.Status.Conditions = append(obj.Status.Conditions, corev1.PodCondition{ + Type: corev1.PodReady, + Status: corev1.ConditionTrue, + }) + return obj + }), + }, + Healthy: true, + }, + + expectedStatus: task.SComplete, + expectedObj: fake.FakeObj(fakeTiDBName, func(obj *v1alpha1.TiDB) *v1alpha1.TiDB { + obj.Generation = 3 + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: newRevision, + } + + obj.Status.ObservedGeneration = 3 + obj.Status.UpdateRevision = newRevision + obj.Status.CurrentRevision = oldRevision + obj.Status.Conditions = []metav1.Condition{ + { + Type: v1alpha1.CondHealth, + Status: metav1.ConditionTrue, + ObservedGeneration: 3, + Reason: "Healthy", + Message: "instance is healthy", + }, + { + Type: v1alpha1.CondSuspended, + Status: metav1.ConditionFalse, + ObservedGeneration: 3, + Reason: v1alpha1.ReasonUnsuspended, + Message: "instace is not suspended", + }, + } + + return obj + }), + }, + { + desc: "pod is deleting", + state: &ReconcileContext{ + State: &state{ + tidb: fake.FakeObj(fakeTiDBName, func(obj *v1alpha1.TiDB) *v1alpha1.TiDB { + obj.Generation = 3 + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: newRevision, + } + return obj + }), + pod: fake.FakeObj("aaa-tidb-xxx", func(obj *corev1.Pod) *corev1.Pod { + obj.SetDeletionTimestamp(&now) + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: oldRevision, + } + obj.Status.Phase = corev1.PodRunning + obj.Status.Conditions = append(obj.Status.Conditions, corev1.PodCondition{ + Type: corev1.PodReady, + Status: corev1.ConditionTrue, + }) + return obj + }), + }, + PodIsTerminating: true, + Healthy: true, + }, + + expectedStatus: task.SRetry, + expectedObj: fake.FakeObj(fakeTiDBName, func(obj *v1alpha1.TiDB) *v1alpha1.TiDB { + obj.Generation = 3 + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: newRevision, + } + + obj.Status.ObservedGeneration = 3 + obj.Status.UpdateRevision = newRevision + obj.Status.Conditions = []metav1.Condition{ + { + Type: v1alpha1.CondHealth, + Status: metav1.ConditionFalse, + ObservedGeneration: 3, + Reason: "Unhealthy", + Message: "instance is not healthy", + }, + { + Type: v1alpha1.CondSuspended, + Status: metav1.ConditionFalse, + ObservedGeneration: 3, + Reason: v1alpha1.ReasonUnsuspended, + Message: "instace is not suspended", + }, + } + + return obj + }), + }, + { + desc: "pod is ready but tidb is not healthy", + state: &ReconcileContext{ + State: &state{ + tidb: fake.FakeObj(fakeTiDBName, func(obj *v1alpha1.TiDB) *v1alpha1.TiDB { + obj.Generation = 3 + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: newRevision, + } + return obj + }), + pod: fake.FakeObj("aaa-tidb-xxx", func(obj *corev1.Pod) *corev1.Pod { + obj.SetDeletionTimestamp(&now) + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: oldRevision, + } + obj.Status.Phase = corev1.PodRunning + obj.Status.Conditions = append(obj.Status.Conditions, corev1.PodCondition{ + Type: corev1.PodReady, + Status: corev1.ConditionTrue, + }) + return obj + }), + }, + }, + + expectedStatus: task.SRetry, + expectedObj: fake.FakeObj(fakeTiDBName, func(obj *v1alpha1.TiDB) *v1alpha1.TiDB { + obj.Generation = 3 + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: newRevision, + } + + obj.Status.ObservedGeneration = 3 + obj.Status.UpdateRevision = newRevision + obj.Status.Conditions = []metav1.Condition{ + { + Type: v1alpha1.CondHealth, + Status: metav1.ConditionFalse, + ObservedGeneration: 3, + Reason: "Unhealthy", + Message: "instance is not healthy", + }, + { + Type: v1alpha1.CondSuspended, + Status: metav1.ConditionFalse, + ObservedGeneration: 3, + Reason: v1alpha1.ReasonUnsuspended, + Message: "instace is not suspended", + }, + } + + return obj + }), + }, + { + desc: "failed to update status", + state: &ReconcileContext{ + State: &state{ + tidb: fake.FakeObj(fakeTiDBName, func(obj *v1alpha1.TiDB) *v1alpha1.TiDB { + obj.Generation = 3 + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: newRevision, + } + return obj + }), + pod: fake.FakeObj("aaa-tidb-xxx", func(obj *corev1.Pod) *corev1.Pod { + obj.SetDeletionTimestamp(&now) + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: oldRevision, + } + obj.Status.Phase = corev1.PodRunning + obj.Status.Conditions = append(obj.Status.Conditions, corev1.PodCondition{ + Type: corev1.PodReady, + Status: corev1.ConditionTrue, + }) + return obj + }), + }, + }, + unexpectedErr: true, + + expectedStatus: task.SFail, + }, + } + + for i := range cases { + c := &cases[i] + t.Run(c.desc, func(tt *testing.T) { + tt.Parallel() + + var objs []client.Object + objs = append(objs, c.state.TiDB()) + if c.state.Pod() != nil { + objs = append(objs, c.state.Pod()) + } + fc := client.NewFakeClient(objs...) + if c.unexpectedErr { + fc.WithError("*", "*", errors.NewInternalError(fmt.Errorf("fake internal err"))) + } + + ctx := context.Background() + res, done := task.RunTask(ctx, TaskStatus(c.state, fc)) + assert.Equal(tt, c.expectedStatus.String(), res.Status().String(), c.desc) + assert.False(tt, done, c.desc) + + // no need to check update result + if c.unexpectedErr { + return + } + + obj := &v1alpha1.TiDB{} + require.NoError(tt, fc.Get(ctx, client.ObjectKey{Name: fakeTiDBName}, obj), c.desc) + conds := obj.Status.Conditions + for i := range conds { + cond := &conds[i] + cond.LastTransitionTime = metav1.Time{} + } + assert.Equal(tt, c.expectedObj, obj, 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_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/ctx.go b/pkg/controllers/tiflash/tasks/ctx.go index 7e91bc1571..161a8de1b6 100644 --- a/pkg/controllers/tiflash/tasks/ctx.go +++ b/pkg/controllers/tiflash/tasks/ctx.go @@ -22,7 +22,6 @@ import ( "github.com/pingcap/kvproto/pkg/metapb" tiflashconfig "github.com/pingcap/tidb-operator/pkg/configs/tiflash" - "github.com/pingcap/tidb-operator/pkg/pdapi/v1" pdv1 "github.com/pingcap/tidb-operator/pkg/timanager/apis/pd/v1" pdm "github.com/pingcap/tidb-operator/pkg/timanager/pd" "github.com/pingcap/tidb-operator/pkg/utils/task/v3" @@ -31,7 +30,7 @@ import ( type ReconcileContext struct { State - PDClient pdapi.PDClient + PDClient pdm.PDClient Store *pdv1.Store StoreID string @@ -55,7 +54,7 @@ func TaskContextInfoFromPD(state *ReconcileContext, cm pdm.PDClientManager) task if !ok { return task.Complete().With("pd client is not registered") } - state.PDClient = c.Underlay() + state.PDClient = c if !c.HasSynced() { return task.Complete().With("store info is not synced, just wait for next sync") diff --git a/pkg/controllers/tiflash/tasks/finalizer.go b/pkg/controllers/tiflash/tasks/finalizer.go index 972a475965..04e4d0ac99 100644 --- a/pkg/controllers/tiflash/tasks/finalizer.go +++ b/pkg/controllers/tiflash/tasks/finalizer.go @@ -69,7 +69,7 @@ func TaskFinalizerDel(state *ReconcileContext, c client.Client) task.Task { } default: // get store info successfully and the store still exists - if err := state.PDClient.DeleteStore(ctx, state.StoreID); err != nil { + if err := state.PDClient.Underlay().DeleteStore(ctx, state.StoreID); err != nil { return task.Fail().With("cannot delete store %s: %v", state.StoreID, err) } diff --git a/pkg/controllers/tiflash/tasks/finalizer_test.go b/pkg/controllers/tiflash/tasks/finalizer_test.go new file mode 100644 index 0000000000..f19dce6524 --- /dev/null +++ b/pkg/controllers/tiflash/tasks/finalizer_test.go @@ -0,0 +1,504 @@ +// 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" + "strconv" + "testing" + + "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" + 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/pdapi/v1" + pdm "github.com/pingcap/tidb-operator/pkg/timanager/pd" + "github.com/pingcap/tidb-operator/pkg/utils/fake" + "github.com/pingcap/tidb-operator/pkg/utils/task/v3" +) + +func TestTaskFinalizerDel(t *testing.T) { + now := metav1.Now() + cases := []struct { + desc string + state *ReconcileContext + subresources []client.Object + needDelStore bool + unexpectedDelStoreErr bool + unexpectedErr bool + + expectedStatus task.Status + expectedObj *v1alpha1.TiFlash + }{ + { + desc: "cluster is deleting, no sub resources, no finalizer", + state: &ReconcileContext{ + State: &state{ + tiflash: fake.FakeObj("aaa", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + obj.SetDeletionTimestamp(&now) + return obj + }), + }, + }, + + expectedStatus: task.SComplete, + expectedObj: fake.FakeObj("aaa", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + return obj + }), + }, + { + desc: "cluster is deleting, has sub resources, has finalizer", + state: &ReconcileContext{ + State: &state{ + tiflash: fake.FakeObj("aaa", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + obj.SetDeletionTimestamp(&now) + return obj + }), + }, + }, + subresources: fakeSubresources("Pod", "ConfigMap", "PersistentVolumeClaim"), + + expectedStatus: task.SWait, + expectedObj: fake.FakeObj("aaa", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + }, + { + desc: "cluster is deleting, has sub resources, has finalizer, failed to del subresources(pod)", + state: &ReconcileContext{ + State: &state{ + tiflash: fake.FakeObj("aaa", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + obj.SetDeletionTimestamp(&now) + return obj + }), + }, + }, + subresources: fakeSubresources("Pod"), + + unexpectedErr: true, + + expectedStatus: task.SFail, + expectedObj: fake.FakeObj("aaa", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + }, + { + desc: "cluster is deleting, has sub resources, has finalizer, failed to del subresources(cm)", + state: &ReconcileContext{ + State: &state{ + tiflash: fake.FakeObj("aaa", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + obj.SetDeletionTimestamp(&now) + return obj + }), + }, + }, + subresources: fakeSubresources("ConfigMap"), + + unexpectedErr: true, + + expectedStatus: task.SFail, + expectedObj: fake.FakeObj("aaa", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + }, + { + desc: "cluster is deleting, has sub resources, has finalizer, failed to del subresources(pvc)", + state: &ReconcileContext{ + State: &state{ + tiflash: fake.FakeObj("aaa", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + obj.SetDeletionTimestamp(&now) + return obj + }), + }, + }, + subresources: fakeSubresources("PersistentVolumeClaim"), + + unexpectedErr: true, + + expectedStatus: task.SFail, + expectedObj: fake.FakeObj("aaa", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + }, + { + desc: "cluster is deleting, no sub resources, has finalizer", + state: &ReconcileContext{ + State: &state{ + tiflash: fake.FakeObj("aaa", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + obj.SetDeletionTimestamp(&now) + return obj + }), + }, + }, + + expectedStatus: task.SComplete, + expectedObj: fake.FakeObj("aaa", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Finalizers = []string{} + return obj + }), + }, + { + desc: "cluster is deleting, no sub resources, has finalizer, failed to remove finalizer", + state: &ReconcileContext{ + State: &state{ + tiflash: fake.FakeObj("aaa", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + obj.SetDeletionTimestamp(&now) + return obj + }), + }, + }, + unexpectedErr: true, + + expectedStatus: task.SFail, + expectedObj: fake.FakeObj("aaa", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + return obj + }), + }, + { + desc: "cluster is not deleting, store is removing", + state: &ReconcileContext{ + State: &state{ + tiflash: fake.FakeObj("aaa", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + return obj + }), + }, + StoreState: v1alpha1.StoreStateRemoving, + }, + + expectedStatus: task.SRetry, + expectedObj: fake.FakeObj("aaa", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + }, + { + desc: "cluster is not deleting, store is removed, no subresources, no finalizer", + state: &ReconcileContext{ + State: &state{ + tiflash: fake.FakeObj("aaa", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + return obj + }), + }, + StoreState: v1alpha1.StoreStateRemoved, + }, + + expectedStatus: task.SComplete, + expectedObj: fake.FakeObj("aaa", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + return obj + }), + }, + { + desc: "cluster is not deleting, store is removed, no subresources, has finalizer", + state: &ReconcileContext{ + State: &state{ + tiflash: fake.FakeObj("aaa", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + return obj + }), + }, + StoreState: v1alpha1.StoreStateRemoved, + }, + + expectedStatus: task.SComplete, + expectedObj: fake.FakeObj("aaa", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Finalizers = []string{} + return obj + }), + }, + { + desc: "cluster is not deleting, store is removed, no subresources, has finalizer, failed to remove finalizer", + state: &ReconcileContext{ + State: &state{ + tiflash: fake.FakeObj("aaa", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + return obj + }), + }, + StoreState: v1alpha1.StoreStateRemoved, + }, + unexpectedErr: true, + + expectedStatus: task.SFail, + }, + { + desc: "cluster is not deleting, store is removed, has subresources", + state: &ReconcileContext{ + State: &state{ + tiflash: fake.FakeObj("aaa", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + return obj + }), + }, + StoreState: v1alpha1.StoreStateRemoved, + }, + subresources: fakeSubresources("Pod"), + + expectedStatus: task.SWait, + expectedObj: fake.FakeObj("aaa", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + }, + { + desc: "cluster is not deleting, store is removed, has subresources, failed to del subresources", + state: &ReconcileContext{ + State: &state{ + tiflash: fake.FakeObj("aaa", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + return obj + }), + }, + StoreState: v1alpha1.StoreStateRemoved, + }, + subresources: fakeSubresources("Pod"), + unexpectedErr: true, + + expectedStatus: task.SFail, + }, + { + desc: "cluster is not deleting, store id is empty, no subresources, has finalizer", + state: &ReconcileContext{ + State: &state{ + tiflash: fake.FakeObj("aaa", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + return obj + }), + }, + }, + + expectedStatus: task.SComplete, + expectedObj: fake.FakeObj("aaa", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Finalizers = []string{} + return obj + }), + }, + { + desc: "cluster is not deleting, store exists", + state: &ReconcileContext{ + State: &state{ + tiflash: fake.FakeObj("aaa", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + return obj + }), + }, + StoreID: "xxx", + }, + needDelStore: true, + + expectedStatus: task.SRetry, + expectedObj: fake.FakeObj("aaa", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + }, + { + desc: "cluster is not deleting, store exists, failed to del store", + state: &ReconcileContext{ + State: &state{ + tiflash: fake.FakeObj("aaa", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + return obj + }), + }, + StoreID: "xxx", + }, + needDelStore: true, + unexpectedDelStoreErr: true, + + expectedStatus: task.SFail, + expectedObj: fake.FakeObj("aaa", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + }, + } + + for i := range cases { + c := &cases[i] + t.Run(c.desc, func(tt *testing.T) { + tt.Parallel() + + objs := []client.Object{ + c.state.TiFlash(), + } + + objs = append(objs, c.subresources...) + + fc := client.NewFakeClient(objs...) + if c.unexpectedErr { + // cannot remove finalizer + fc.WithError("update", "*", errors.NewInternalError(fmt.Errorf("fake internal err"))) + // cannot delete sub resources + fc.WithError("delete", "*", errors.NewInternalError(fmt.Errorf("fake internal err"))) + } + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + var acts []action + if c.needDelStore { + var retErr error + if c.unexpectedDelStoreErr { + retErr = fmt.Errorf("fake err") + } + acts = append(acts, deleteStore(ctx, c.state.StoreID, retErr)) + } + + pdc := NewFakePDClient(tt, acts...) + c.state.PDClient = pdc + + res, done := task.RunTask(ctx, TaskFinalizerDel(c.state, fc)) + assert.Equal(tt, c.expectedStatus.String(), res.Status().String(), c.desc) + assert.False(tt, done, c.desc) + + // no need to check update result + if c.unexpectedErr || c.unexpectedDelStoreErr { + return + } + + pd := &v1alpha1.TiFlash{} + require.NoError(tt, fc.Get(ctx, client.ObjectKey{Name: "aaa"}, pd), c.desc) + assert.Equal(tt, c.expectedObj, pd, c.desc) + }) + } +} + +func NewFakePDClient(t *testing.T, acts ...action) pdm.PDClient { + ctrl := gomock.NewController(t) + pdc := pdm.NewMockPDClient(ctrl) + for _, act := range acts { + act(ctrl, pdc) + } + + return pdc +} + +type action func(ctrl *gomock.Controller, pdc *pdm.MockPDClient) + +func deleteStore(ctx context.Context, name string, err error) action { + return func(ctrl *gomock.Controller, pdc *pdm.MockPDClient) { + underlay := pdapi.NewMockPDClient(ctrl) + pdc.EXPECT().Underlay().Return(underlay) + underlay.EXPECT().DeleteStore(ctx, name).Return(err) + } +} + +func fakeSubresources(types ...string) []client.Object { + var objs []client.Object + for i, t := range types { + var obj client.Object + switch t { + case "Pod": + obj = fake.FakeObj(strconv.Itoa(i), func(obj *corev1.Pod) *corev1.Pod { + obj.Labels = map[string]string{ + v1alpha1.LabelKeyManagedBy: v1alpha1.LabelValManagedByOperator, + v1alpha1.LabelKeyInstance: "aaa", + v1alpha1.LabelKeyCluster: "", + v1alpha1.LabelKeyComponent: v1alpha1.LabelValComponentTiFlash, + } + return obj + }) + case "ConfigMap": + obj = fake.FakeObj(strconv.Itoa(i), func(obj *corev1.ConfigMap) *corev1.ConfigMap { + obj.Labels = map[string]string{ + v1alpha1.LabelKeyManagedBy: v1alpha1.LabelValManagedByOperator, + v1alpha1.LabelKeyInstance: "aaa", + v1alpha1.LabelKeyCluster: "", + v1alpha1.LabelKeyComponent: v1alpha1.LabelValComponentTiFlash, + } + return obj + }) + case "PersistentVolumeClaim": + obj = fake.FakeObj(strconv.Itoa(i), func(obj *corev1.PersistentVolumeClaim) *corev1.PersistentVolumeClaim { + obj.Labels = map[string]string{ + v1alpha1.LabelKeyManagedBy: v1alpha1.LabelValManagedByOperator, + v1alpha1.LabelKeyInstance: "aaa", + v1alpha1.LabelKeyCluster: "", + v1alpha1.LabelKeyComponent: v1alpha1.LabelValComponentTiFlash, + } + return obj + }) + } + if obj != nil { + objs = append(objs, obj) + } + } + + return objs +} diff --git a/pkg/controllers/tiflash/tasks/pod.go b/pkg/controllers/tiflash/tasks/pod.go index 9ffb0896bc..65b563bbfe 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" @@ -60,7 +61,7 @@ func TaskPod(state *ReconcileContext, c client.Client) task.Task { } state.PodIsTerminating = true - return task.Complete().With("pod is deleting") + return task.Wait().With("pod is deleting") } else if res == k8s.CompareResultUpdate { logger.Info("will update the pod in place") if err := c.Apply(ctx, expected); err != nil { @@ -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{ { @@ -216,13 +220,15 @@ func buildLogTailerContainer(tiflash *v1alpha1.TiFlash, containerName, logFile s Name: containerName, Image: img, RestartPolicy: &restartPolicy, - VolumeMounts: []corev1.VolumeMount{*mount}, Command: []string{ "sh", "-c", fmt.Sprintf("touch %s; tail -n0 -F %s;", logFile, logFile), }, } + if mount != nil { + c.VolumeMounts = append(c.VolumeMounts, *mount) + } if tiflash.Spec.LogTailer != nil { c.Resources = k8s.GetResourceRequirements(tiflash.Spec.LogTailer.Resources) } diff --git a/pkg/controllers/tiflash/tasks/pod_test.go b/pkg/controllers/tiflash/tasks/pod_test.go new file mode 100644 index 0000000000..5212716c97 --- /dev/null +++ b/pkg/controllers/tiflash/tasks/pod_test.go @@ -0,0 +1,275 @@ +// 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" + + "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 fakeVersion = "v1.2.3" + +func TestTaskPod(t *testing.T) { + cases := []struct { + desc string + state *ReconcileContext + objs []client.Object + // if true, cannot apply pod + unexpectedErr bool + + expectUpdatedPod bool + expectedPodIsTerminating bool + expectedStatus task.Status + }{ + { + desc: "no pod", + state: &ReconcileContext{ + State: &state{ + tiflash: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Spec.Version = fakeVersion + return obj + }), + cluster: fake.FakeObj[v1alpha1.Cluster]("aaa"), + }, + }, + + expectUpdatedPod: true, + expectedStatus: task.SComplete, + }, + { + desc: "no pod, failed to apply", + state: &ReconcileContext{ + State: &state{ + tiflash: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Spec.Version = fakeVersion + return obj + }), + cluster: fake.FakeObj[v1alpha1.Cluster]("aaa"), + }, + }, + unexpectedErr: true, + + expectedStatus: task.SFail, + }, + { + desc: "pod spec changed", + state: &ReconcileContext{ + State: &state{ + tiflash: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Spec.Version = fakeVersion + return obj + }), + cluster: fake.FakeObj[v1alpha1.Cluster]("aaa"), + pod: fake.FakeObj("aaa-tiflash-xxx", func(obj *corev1.Pod) *corev1.Pod { + return obj + }), + }, + }, + + expectedPodIsTerminating: true, + expectedStatus: task.SWait, + }, + { + desc: "pod spec changed, failed to delete", + state: &ReconcileContext{ + State: &state{ + tiflash: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Spec.Version = fakeVersion + return obj + }), + cluster: fake.FakeObj[v1alpha1.Cluster]("aaa"), + pod: fake.FakeObj("aaa-tiflash-xxx", func(obj *corev1.Pod) *corev1.Pod { + return obj + }), + }, + }, + unexpectedErr: true, + + expectedStatus: task.SFail, + }, + { + desc: "pod spec hash not changed, config changed, hot reload policy", + state: &ReconcileContext{ + State: &state{ + tiflash: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Spec.Version = fakeVersion + obj.Spec.UpdateStrategy.Config = v1alpha1.ConfigUpdateStrategyHotReload + return obj + }), + cluster: fake.FakeObj[v1alpha1.Cluster]("aaa"), + pod: fake.FakeObj("aaa-tiflash-xxx", func(obj *corev1.Pod) *corev1.Pod { + obj.Labels = map[string]string{ + v1alpha1.LabelKeyConfigHash: "newest", + v1alpha1.LabelKeyPodSpecHash: "556796549d", + } + return obj + }), + }, + ConfigHash: "newest", + }, + + expectUpdatedPod: true, + expectedStatus: task.SComplete, + }, + { + desc: "pod spec hash not changed, config changed, restart policy", + state: &ReconcileContext{ + State: &state{ + tiflash: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Spec.Version = fakeVersion + obj.Spec.UpdateStrategy.Config = v1alpha1.ConfigUpdateStrategyRestart + return obj + }), + cluster: fake.FakeObj[v1alpha1.Cluster]("aaa"), + pod: fake.FakeObj("aaa-tiflash-xxx", func(obj *corev1.Pod) *corev1.Pod { + obj.Labels = map[string]string{ + v1alpha1.LabelKeyConfigHash: "old", + v1alpha1.LabelKeyPodSpecHash: "556796549d", + } + return obj + }), + }, + ConfigHash: "newest", + }, + + expectedPodIsTerminating: true, + expectedStatus: task.SWait, + }, + { + desc: "pod spec hash not changed, pod labels changed, config not changed", + state: &ReconcileContext{ + State: &state{ + tiflash: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Spec.Version = fakeVersion + obj.Spec.UpdateStrategy.Config = v1alpha1.ConfigUpdateStrategyRestart + return obj + }), + cluster: fake.FakeObj[v1alpha1.Cluster]("aaa"), + pod: fake.FakeObj("aaa-tiflash-xxx", func(obj *corev1.Pod) *corev1.Pod { + obj.Labels = map[string]string{ + v1alpha1.LabelKeyConfigHash: "newest", + v1alpha1.LabelKeyPodSpecHash: "556796549d", + "xxx": "yyy", + } + return obj + }), + }, + ConfigHash: "newest", + }, + + expectUpdatedPod: true, + expectedStatus: task.SComplete, + }, + { + desc: "pod spec hash not changed, pod labels changed, config not changed, apply failed", + state: &ReconcileContext{ + State: &state{ + tiflash: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Spec.Version = fakeVersion + obj.Spec.UpdateStrategy.Config = v1alpha1.ConfigUpdateStrategyRestart + return obj + }), + cluster: fake.FakeObj[v1alpha1.Cluster]("aaa"), + pod: fake.FakeObj("aaa-tiflash-xxx", func(obj *corev1.Pod) *corev1.Pod { + obj.Labels = map[string]string{ + v1alpha1.LabelKeyConfigHash: "newest", + v1alpha1.LabelKeyPodSpecHash: "556796549d", + "xxx": "yyy", + } + return obj + }), + }, + ConfigHash: "newest", + }, + unexpectedErr: true, + + expectedStatus: task.SFail, + }, + { + desc: "all are not changed", + state: &ReconcileContext{ + State: &state{ + tiflash: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Spec.Version = fakeVersion + obj.Spec.UpdateStrategy.Config = v1alpha1.ConfigUpdateStrategyRestart + return obj + }), + cluster: fake.FakeObj[v1alpha1.Cluster]("aaa"), + pod: fake.FakeObj("aaa-tiflash-xxx", func(obj *corev1.Pod) *corev1.Pod { + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstance: "aaa-xxx", + v1alpha1.LabelKeyConfigHash: "newest", + v1alpha1.LabelKeyPodSpecHash: "556796549d", + } + return obj + }), + }, + ConfigHash: "newest", + }, + + expectedStatus: task.SComplete, + }, + } + + 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()) + if c.state.Pod() != nil { + objs = append(objs, c.state.Pod()) + } + fc := client.NewFakeClient(objs...) + for _, obj := range c.objs { + require.NoError(tt, fc.Apply(ctx, obj), c.desc) + } + + if c.unexpectedErr { + // cannot update pod + fc.WithError("patch", "*", errors.NewInternalError(fmt.Errorf("fake internal err"))) + fc.WithError("delete", "*", errors.NewInternalError(fmt.Errorf("fake internal err"))) + } + + res, done := task.RunTask(ctx, TaskPod(c.state, fc)) + assert.Equal(tt, c.expectedStatus.String(), res.Status().String(), res.Message()) + assert.False(tt, done, c.desc) + + assert.Equal(tt, c.expectedPodIsTerminating, c.state.PodIsTerminating, c.desc) + + if c.expectUpdatedPod { + expectedPod := newPod(c.state.Cluster(), c.state.TiFlash(), c.state.ConfigHash) + actual := c.state.Pod().DeepCopy() + actual.Kind = "" + actual.APIVersion = "" + actual.ManagedFields = nil + assert.Equal(tt, expectedPod, actual, c.desc) + } + }) + } +} 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/status.go b/pkg/controllers/tiflash/tasks/status.go index 56a40c17cf..25d84ce694 100644 --- a/pkg/controllers/tiflash/tasks/status.go +++ b/pkg/controllers/tiflash/tasks/status.go @@ -44,8 +44,7 @@ func TaskStatus(state *ReconcileContext, c client.Client) task.Task { if pod != nil && statefulset.IsPodRunningAndReady(pod) && !state.PodIsTerminating && - state.Store != nil && - state.Store.NodeState == v1alpha1.StoreStateServing { + state.StoreState == v1alpha1.StoreStateServing { healthy = true } needUpdate = syncHealthCond(tiflash, healthy) || needUpdate @@ -66,11 +65,13 @@ func TaskStatus(state *ReconcileContext, c client.Client) task.Task { } } + if state.PodIsTerminating { + return task.Retry(defaultTaskWaitDuration).With("pod may be terminating, requeue to retry") + } + // TODO: use a condition to refactor it - if tiflash.Status.ID == "" || tiflash.Status.State != v1alpha1.StoreStateServing || !v1alpha1.IsUpToDate(tiflash) { - // can we only rely on the PD member events for this condition? - // TODO(liubo02): change to task.Wait - return task.Retry(defaultTaskWaitDuration).With("tiflash may not be initialized, retry") + if !healthy || tiflash.Status.ID == "" { + return task.Wait().With("tiflash may not be synced, wait") } return task.Complete().With("status is synced") diff --git a/pkg/controllers/tiflash/tasks/status_test.go b/pkg/controllers/tiflash/tasks/status_test.go new file mode 100644 index 0000000000..9c0a61a7e0 --- /dev/null +++ b/pkg/controllers/tiflash/tasks/status_test.go @@ -0,0 +1,346 @@ +// 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" + pdv1 "github.com/pingcap/tidb-operator/pkg/timanager/apis/pd/v1" + "github.com/pingcap/tidb-operator/pkg/utils/fake" + "github.com/pingcap/tidb-operator/pkg/utils/task/v3" +) + +const ( + newRevision = "new" + oldRevision = "old" + + fakeTiFlashName = "aaa-xxx" +) + +func TestTaskStatus(t *testing.T) { + now := metav1.Now() + cases := []struct { + desc string + state *ReconcileContext + unexpectedErr bool + + expectedStatus task.Status + expectedObj *v1alpha1.TiFlash + }{ + { + desc: "no pod but healthy", + state: &ReconcileContext{ + State: &state{ + tiflash: fake.FakeObj(fakeTiFlashName, func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Generation = 3 + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: newRevision, + } + obj.Status.CurrentRevision = "keep" + obj.Status.ObservedGeneration = 3 + return obj + }), + }, + Store: &pdv1.Store{}, + StoreID: fakeTiFlashName, + StoreState: v1alpha1.StoreStateServing, + }, + + expectedStatus: task.SWait, + expectedObj: fake.FakeObj(fakeTiFlashName, func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Generation = 3 + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: newRevision, + } + + obj.Status.ObservedGeneration = 3 + obj.Status.ID = fakeTiFlashName + obj.Status.State = v1alpha1.StoreStateServing + obj.Status.UpdateRevision = newRevision + obj.Status.CurrentRevision = "keep" + obj.Status.Conditions = []metav1.Condition{ + { + Type: v1alpha1.CondHealth, + Status: metav1.ConditionFalse, + ObservedGeneration: 3, + Reason: "Unhealthy", + Message: "instance is not healthy", + }, + { + Type: v1alpha1.CondSuspended, + Status: metav1.ConditionFalse, + ObservedGeneration: 3, + Reason: v1alpha1.ReasonUnsuspended, + Message: "instace is not suspended", + }, + } + + return obj + }), + }, + { + desc: "pod is healthy", + state: &ReconcileContext{ + State: &state{ + tiflash: fake.FakeObj(fakeTiFlashName, func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Generation = 3 + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: newRevision, + } + return obj + }), + pod: fake.FakeObj("aaa-tiflash-xxx", func(obj *corev1.Pod) *corev1.Pod { + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: oldRevision, + } + obj.Status.Phase = corev1.PodRunning + obj.Status.Conditions = append(obj.Status.Conditions, corev1.PodCondition{ + Type: corev1.PodReady, + Status: corev1.ConditionTrue, + }) + return obj + }), + }, + Store: &pdv1.Store{}, + StoreID: fakeTiFlashName, + StoreState: v1alpha1.StoreStateServing, + }, + + expectedStatus: task.SComplete, + expectedObj: fake.FakeObj(fakeTiFlashName, func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Generation = 3 + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: newRevision, + } + + obj.Status.ObservedGeneration = 3 + obj.Status.ID = fakeTiFlashName + obj.Status.State = v1alpha1.StoreStateServing + obj.Status.UpdateRevision = newRevision + obj.Status.CurrentRevision = oldRevision + obj.Status.Conditions = []metav1.Condition{ + { + Type: v1alpha1.CondHealth, + Status: metav1.ConditionTrue, + ObservedGeneration: 3, + Reason: "Healthy", + Message: "instance is healthy", + }, + { + Type: v1alpha1.CondSuspended, + Status: metav1.ConditionFalse, + ObservedGeneration: 3, + Reason: v1alpha1.ReasonUnsuspended, + Message: "instace is not suspended", + }, + } + + return obj + }), + }, + { + desc: "pod is deleting", + state: &ReconcileContext{ + State: &state{ + tiflash: fake.FakeObj(fakeTiFlashName, func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Generation = 3 + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: newRevision, + } + return obj + }), + pod: fake.FakeObj("aaa-tiflash-xxx", func(obj *corev1.Pod) *corev1.Pod { + obj.SetDeletionTimestamp(&now) + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: oldRevision, + } + obj.Status.Phase = corev1.PodRunning + obj.Status.Conditions = append(obj.Status.Conditions, corev1.PodCondition{ + Type: corev1.PodReady, + Status: corev1.ConditionTrue, + }) + return obj + }), + }, + Store: &pdv1.Store{}, + StoreID: fakeTiFlashName, + PodIsTerminating: true, + }, + + expectedStatus: task.SRetry, + expectedObj: fake.FakeObj(fakeTiFlashName, func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Generation = 3 + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: newRevision, + } + + obj.Status.ObservedGeneration = 3 + obj.Status.ID = fakeTiFlashName + obj.Status.UpdateRevision = newRevision + obj.Status.Conditions = []metav1.Condition{ + { + Type: v1alpha1.CondHealth, + Status: metav1.ConditionFalse, + ObservedGeneration: 3, + Reason: "Unhealthy", + Message: "instance is not healthy", + }, + { + Type: v1alpha1.CondSuspended, + Status: metav1.ConditionFalse, + ObservedGeneration: 3, + Reason: v1alpha1.ReasonUnsuspended, + Message: "instace is not suspended", + }, + } + + return obj + }), + }, + { + desc: "not serving", + state: &ReconcileContext{ + State: &state{ + tiflash: fake.FakeObj(fakeTiFlashName, func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Generation = 3 + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: newRevision, + } + return obj + }), + pod: fake.FakeObj("aaa-tiflash-xxx", func(obj *corev1.Pod) *corev1.Pod { + obj.SetDeletionTimestamp(&now) + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: oldRevision, + } + obj.Status.Phase = corev1.PodRunning + obj.Status.Conditions = append(obj.Status.Conditions, corev1.PodCondition{ + Type: corev1.PodReady, + Status: corev1.ConditionTrue, + }) + return obj + }), + }, + Store: &pdv1.Store{}, + StoreID: fakeTiFlashName, + }, + + expectedStatus: task.SWait, + expectedObj: fake.FakeObj(fakeTiFlashName, func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Generation = 3 + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: newRevision, + } + + obj.Status.ObservedGeneration = 3 + obj.Status.ID = fakeTiFlashName + obj.Status.UpdateRevision = newRevision + obj.Status.Conditions = []metav1.Condition{ + { + Type: v1alpha1.CondHealth, + Status: metav1.ConditionFalse, + ObservedGeneration: 3, + Reason: "Unhealthy", + Message: "instance is not healthy", + }, + { + Type: v1alpha1.CondSuspended, + Status: metav1.ConditionFalse, + ObservedGeneration: 3, + Reason: v1alpha1.ReasonUnsuspended, + Message: "instace is not suspended", + }, + } + + return obj + }), + }, + { + desc: "failed to update status", + state: &ReconcileContext{ + State: &state{ + tiflash: fake.FakeObj(fakeTiFlashName, func(obj *v1alpha1.TiFlash) *v1alpha1.TiFlash { + obj.Generation = 3 + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: newRevision, + } + return obj + }), + pod: fake.FakeObj("aaa-tiflash-xxx", func(obj *corev1.Pod) *corev1.Pod { + obj.SetDeletionTimestamp(&now) + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: oldRevision, + } + obj.Status.Phase = corev1.PodRunning + obj.Status.Conditions = append(obj.Status.Conditions, corev1.PodCondition{ + Type: corev1.PodReady, + Status: corev1.ConditionTrue, + }) + return obj + }), + }, + }, + unexpectedErr: true, + + expectedStatus: task.SFail, + }, + } + + for i := range cases { + c := &cases[i] + t.Run(c.desc, func(tt *testing.T) { + tt.Parallel() + + var objs []client.Object + objs = append(objs, c.state.TiFlash()) + if c.state.Pod() != nil { + objs = append(objs, c.state.Pod()) + } + fc := client.NewFakeClient(objs...) + if c.unexpectedErr { + fc.WithError("*", "*", errors.NewInternalError(fmt.Errorf("fake internal err"))) + } + + ctx := context.Background() + res, done := task.RunTask(ctx, TaskStatus(c.state, fc)) + assert.Equal(tt, c.expectedStatus.String(), res.Status().String(), c.desc) + assert.False(tt, done, c.desc) + + // no need to check update result + if c.unexpectedErr { + return + } + + obj := &v1alpha1.TiFlash{} + require.NoError(tt, fc.Get(ctx, client.ObjectKey{Name: fakeTiFlashName}, obj), c.desc) + conds := obj.Status.Conditions + for i := range conds { + cond := &conds[i] + cond.LastTransitionTime = metav1.Time{} + } + assert.Equal(tt, c.expectedObj, obj, c.desc) + }) + } +} diff --git a/pkg/controllers/tiflash/tasks/store_labels.go b/pkg/controllers/tiflash/tasks/store_labels.go index 4515b9ac01..7cb5289a38 100644 --- a/pkg/controllers/tiflash/tasks/store_labels.go +++ b/pkg/controllers/tiflash/tasks/store_labels.go @@ -47,7 +47,7 @@ func TaskStoreLabels(state *ReconcileContext, c client.Client) task.Task { } // TODO: too many API calls to PD? - pdCfg, err := state.PDClient.GetConfig(ctx) + pdCfg, err := state.PDClient.Underlay().GetConfig(ctx) if err != nil { return task.Fail().With("failed to get pd config: %s", err) } @@ -66,7 +66,7 @@ func TaskStoreLabels(state *ReconcileContext, c client.Client) task.Task { if err != nil { return task.Fail().With("failed to parse store id %s: %s", state.StoreID, err) } - set, err := state.PDClient.SetStoreLabels(ctx, storeID, storeLabels) + set, err := state.PDClient.Underlay().SetStoreLabels(ctx, storeID, storeLabels) if err != nil { return task.Fail().With("failed to set store labels: %s", err) } else if set { diff --git a/pkg/controllers/tiflash/tasks/util.go b/pkg/controllers/tiflash/tasks/util.go index 4ece7ad6b6..7e159c12ac 100644 --- a/pkg/controllers/tiflash/tasks/util.go +++ b/pkg/controllers/tiflash/tasks/util.go @@ -18,11 +18,12 @@ 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/ctx.go b/pkg/controllers/tikv/tasks/ctx.go index c938a0f04b..7767ba52e4 100644 --- a/pkg/controllers/tikv/tasks/ctx.go +++ b/pkg/controllers/tikv/tasks/ctx.go @@ -20,7 +20,6 @@ import ( "k8s.io/apimachinery/pkg/api/errors" kvcfg "github.com/pingcap/tidb-operator/pkg/configs/tikv" - "github.com/pingcap/tidb-operator/pkg/pdapi/v1" pdv1 "github.com/pingcap/tidb-operator/pkg/timanager/apis/pd/v1" pdm "github.com/pingcap/tidb-operator/pkg/timanager/pd" "github.com/pingcap/tidb-operator/pkg/utils/task/v3" @@ -29,7 +28,7 @@ import ( type ReconcileContext struct { State - PDClient pdapi.PDClient + PDClient pdm.PDClient StoreExists bool StoreID string @@ -38,6 +37,8 @@ type ReconcileContext struct { Store *pdv1.Store + IsPDAvailable bool + // ConfigHash stores the hash of **user-specified** config (i.e.`.Spec.Config`), // which will be used to determine whether the config has changed. // This ensures that our config overlay logic will not restart the tidb cluster unexpectedly. @@ -55,11 +56,12 @@ func TaskContextInfoFromPD(state *ReconcileContext, cm pdm.PDClientManager) task if !ok { return task.Complete().With("pd client is not registered") } - state.PDClient = c.Underlay() + state.PDClient = c if !c.HasSynced() { return task.Complete().With("store info is not synced, just wait for next sync") } + state.IsPDAvailable = true s, err := c.Stores().Get(kvcfg.GetAdvertiseClientURLs(state.TiKV())) if err != nil { @@ -74,7 +76,7 @@ func TaskContextInfoFromPD(state *ReconcileContext, cm pdm.PDClientManager) task if state.Cluster().ShouldSuspendCompute() { return task.Complete().With("cluster is suspending") } - scheduler, err := state.PDClient.GetEvictLeaderScheduler(ctx, state.StoreID) + scheduler, err := state.PDClient.Underlay().GetEvictLeaderScheduler(ctx, state.StoreID) if err != nil { return task.Fail().With("pd is unexpectedly crashed: %w", err) } diff --git a/pkg/controllers/tikv/tasks/evict_leader.go b/pkg/controllers/tikv/tasks/evict_leader.go index 1f91d43750..01e170f17b 100644 --- a/pkg/controllers/tikv/tasks/evict_leader.go +++ b/pkg/controllers/tikv/tasks/evict_leader.go @@ -27,14 +27,14 @@ func TaskEvictLeader(state *ReconcileContext) task.Task { return task.Complete().With("store has been deleted or not created") case state.PodIsTerminating: if !state.LeaderEvicting { - if err := state.PDClient.BeginEvictLeader(ctx, state.StoreID); err != nil { + if err := state.PDClient.Underlay().BeginEvictLeader(ctx, state.StoreID); err != nil { return task.Fail().With("cannot add evict leader scheduler: %v", err) } } return task.Complete().With("ensure evict leader scheduler exists") default: if state.LeaderEvicting { - if err := state.PDClient.EndEvictLeader(ctx, state.StoreID); err != nil { + if err := state.PDClient.Underlay().EndEvictLeader(ctx, state.StoreID); err != nil { return task.Fail().With("cannot remove evict leader scheduler: %v", err) } } diff --git a/pkg/controllers/tikv/tasks/finalizer.go b/pkg/controllers/tikv/tasks/finalizer.go index 1c6c60b763..2b0ff1bc94 100644 --- a/pkg/controllers/tikv/tasks/finalizer.go +++ b/pkg/controllers/tikv/tasks/finalizer.go @@ -70,7 +70,7 @@ func TaskFinalizerDel(state *ReconcileContext, c client.Client) task.Task { } default: // get store info successfully and the store still exists - if err := state.PDClient.DeleteStore(ctx, state.StoreID); err != nil { + if err := state.PDClient.Underlay().DeleteStore(ctx, state.StoreID); err != nil { return task.Fail().With("cannot delete store %s: %v", state.StoreID, err) } diff --git a/pkg/controllers/tikv/tasks/finalizer_test.go b/pkg/controllers/tikv/tasks/finalizer_test.go new file mode 100644 index 0000000000..709b3ecc64 --- /dev/null +++ b/pkg/controllers/tikv/tasks/finalizer_test.go @@ -0,0 +1,504 @@ +// 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" + "strconv" + "testing" + + "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" + 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/pdapi/v1" + pdm "github.com/pingcap/tidb-operator/pkg/timanager/pd" + "github.com/pingcap/tidb-operator/pkg/utils/fake" + "github.com/pingcap/tidb-operator/pkg/utils/task/v3" +) + +func TestTaskFinalizerDel(t *testing.T) { + now := metav1.Now() + cases := []struct { + desc string + state *ReconcileContext + subresources []client.Object + needDelStore bool + unexpectedDelStoreErr bool + unexpectedErr bool + + expectedStatus task.Status + expectedObj *v1alpha1.TiKV + }{ + { + desc: "cluster is deleting, no sub resources, no finalizer", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj("aaa", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + obj.SetDeletionTimestamp(&now) + return obj + }), + }, + }, + + expectedStatus: task.SComplete, + expectedObj: fake.FakeObj("aaa", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + return obj + }), + }, + { + desc: "cluster is deleting, has sub resources, has finalizer", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj("aaa", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + obj.SetDeletionTimestamp(&now) + return obj + }), + }, + }, + subresources: fakeSubresources("Pod", "ConfigMap", "PersistentVolumeClaim"), + + expectedStatus: task.SWait, + expectedObj: fake.FakeObj("aaa", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + }, + { + desc: "cluster is deleting, has sub resources, has finalizer, failed to del subresources(pod)", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj("aaa", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + obj.SetDeletionTimestamp(&now) + return obj + }), + }, + }, + subresources: fakeSubresources("Pod"), + + unexpectedErr: true, + + expectedStatus: task.SFail, + expectedObj: fake.FakeObj("aaa", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + }, + { + desc: "cluster is deleting, has sub resources, has finalizer, failed to del subresources(cm)", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj("aaa", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + obj.SetDeletionTimestamp(&now) + return obj + }), + }, + }, + subresources: fakeSubresources("ConfigMap"), + + unexpectedErr: true, + + expectedStatus: task.SFail, + expectedObj: fake.FakeObj("aaa", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + }, + { + desc: "cluster is deleting, has sub resources, has finalizer, failed to del subresources(pvc)", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj("aaa", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + obj.SetDeletionTimestamp(&now) + return obj + }), + }, + }, + subresources: fakeSubresources("PersistentVolumeClaim"), + + unexpectedErr: true, + + expectedStatus: task.SFail, + expectedObj: fake.FakeObj("aaa", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + }, + { + desc: "cluster is deleting, no sub resources, has finalizer", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj("aaa", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + obj.SetDeletionTimestamp(&now) + return obj + }), + }, + }, + + expectedStatus: task.SComplete, + expectedObj: fake.FakeObj("aaa", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Finalizers = []string{} + return obj + }), + }, + { + desc: "cluster is deleting, no sub resources, has finalizer, failed to remove finalizer", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj("aaa", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + obj.SetDeletionTimestamp(&now) + return obj + }), + }, + }, + unexpectedErr: true, + + expectedStatus: task.SFail, + expectedObj: fake.FakeObj("aaa", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + return obj + }), + }, + { + desc: "cluster is not deleting, store is removing", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj("aaa", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + return obj + }), + }, + StoreState: v1alpha1.StoreStateRemoving, + }, + + expectedStatus: task.SRetry, + expectedObj: fake.FakeObj("aaa", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + }, + { + desc: "cluster is not deleting, store is removed, no subresources, no finalizer", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj("aaa", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + return obj + }), + }, + StoreState: v1alpha1.StoreStateRemoved, + }, + + expectedStatus: task.SComplete, + expectedObj: fake.FakeObj("aaa", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + return obj + }), + }, + { + desc: "cluster is not deleting, store is removed, no subresources, has finalizer", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj("aaa", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + return obj + }), + }, + StoreState: v1alpha1.StoreStateRemoved, + }, + + expectedStatus: task.SComplete, + expectedObj: fake.FakeObj("aaa", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Finalizers = []string{} + return obj + }), + }, + { + desc: "cluster is not deleting, store is removed, no subresources, has finalizer, failed to remove finalizer", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj("aaa", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + return obj + }), + }, + StoreState: v1alpha1.StoreStateRemoved, + }, + unexpectedErr: true, + + expectedStatus: task.SFail, + }, + { + desc: "cluster is not deleting, store is removed, has subresources", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj("aaa", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + return obj + }), + }, + StoreState: v1alpha1.StoreStateRemoved, + }, + subresources: fakeSubresources("Pod"), + + expectedStatus: task.SWait, + expectedObj: fake.FakeObj("aaa", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + }, + { + desc: "cluster is not deleting, store is removed, has subresources, failed to del subresources", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj("aaa", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + return obj + }), + }, + StoreState: v1alpha1.StoreStateRemoved, + }, + subresources: fakeSubresources("Pod"), + unexpectedErr: true, + + expectedStatus: task.SFail, + }, + { + desc: "cluster is not deleting, store id is empty, no subresources, has finalizer", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj("aaa", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + return obj + }), + }, + }, + + expectedStatus: task.SComplete, + expectedObj: fake.FakeObj("aaa", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Finalizers = []string{} + return obj + }), + }, + { + desc: "cluster is not deleting, store exists", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj("aaa", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + return obj + }), + }, + StoreID: "xxx", + }, + needDelStore: true, + + expectedStatus: task.SRetry, + expectedObj: fake.FakeObj("aaa", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + }, + { + desc: "cluster is not deleting, store exists, failed to del store", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj("aaa", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + cluster: fake.FakeObj("cluster", func(obj *v1alpha1.Cluster) *v1alpha1.Cluster { + return obj + }), + }, + StoreID: "xxx", + }, + needDelStore: true, + unexpectedDelStoreErr: true, + + expectedStatus: task.SFail, + expectedObj: fake.FakeObj("aaa", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Finalizers = append(obj.Finalizers, v1alpha1.Finalizer) + return obj + }), + }, + } + + for i := range cases { + c := &cases[i] + t.Run(c.desc, func(tt *testing.T) { + tt.Parallel() + + objs := []client.Object{ + c.state.TiKV(), + } + + objs = append(objs, c.subresources...) + + fc := client.NewFakeClient(objs...) + if c.unexpectedErr { + // cannot remove finalizer + fc.WithError("update", "*", errors.NewInternalError(fmt.Errorf("fake internal err"))) + // cannot delete sub resources + fc.WithError("delete", "*", errors.NewInternalError(fmt.Errorf("fake internal err"))) + } + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + var acts []action + if c.needDelStore { + var retErr error + if c.unexpectedDelStoreErr { + retErr = fmt.Errorf("fake err") + } + acts = append(acts, deleteStore(ctx, c.state.StoreID, retErr)) + } + + pdc := NewFakePDClient(tt, acts...) + c.state.PDClient = pdc + + res, done := task.RunTask(ctx, TaskFinalizerDel(c.state, fc)) + assert.Equal(tt, c.expectedStatus.String(), res.Status().String(), c.desc) + assert.False(tt, done, c.desc) + + // no need to check update result + if c.unexpectedErr || c.unexpectedDelStoreErr { + return + } + + pd := &v1alpha1.TiKV{} + require.NoError(tt, fc.Get(ctx, client.ObjectKey{Name: "aaa"}, pd), c.desc) + assert.Equal(tt, c.expectedObj, pd, c.desc) + }) + } +} + +func NewFakePDClient(t *testing.T, acts ...action) pdm.PDClient { + ctrl := gomock.NewController(t) + pdc := pdm.NewMockPDClient(ctrl) + for _, act := range acts { + act(ctrl, pdc) + } + + return pdc +} + +type action func(ctrl *gomock.Controller, pdc *pdm.MockPDClient) + +func deleteStore(ctx context.Context, name string, err error) action { + return func(ctrl *gomock.Controller, pdc *pdm.MockPDClient) { + underlay := pdapi.NewMockPDClient(ctrl) + pdc.EXPECT().Underlay().Return(underlay) + underlay.EXPECT().DeleteStore(ctx, name).Return(err) + } +} + +func fakeSubresources(types ...string) []client.Object { + var objs []client.Object + for i, t := range types { + var obj client.Object + switch t { + case "Pod": + obj = fake.FakeObj(strconv.Itoa(i), func(obj *corev1.Pod) *corev1.Pod { + obj.Labels = map[string]string{ + v1alpha1.LabelKeyManagedBy: v1alpha1.LabelValManagedByOperator, + v1alpha1.LabelKeyInstance: "aaa", + v1alpha1.LabelKeyCluster: "", + v1alpha1.LabelKeyComponent: v1alpha1.LabelValComponentTiKV, + } + return obj + }) + case "ConfigMap": + obj = fake.FakeObj(strconv.Itoa(i), func(obj *corev1.ConfigMap) *corev1.ConfigMap { + obj.Labels = map[string]string{ + v1alpha1.LabelKeyManagedBy: v1alpha1.LabelValManagedByOperator, + v1alpha1.LabelKeyInstance: "aaa", + v1alpha1.LabelKeyCluster: "", + v1alpha1.LabelKeyComponent: v1alpha1.LabelValComponentTiKV, + } + return obj + }) + case "PersistentVolumeClaim": + obj = fake.FakeObj(strconv.Itoa(i), func(obj *corev1.PersistentVolumeClaim) *corev1.PersistentVolumeClaim { + obj.Labels = map[string]string{ + v1alpha1.LabelKeyManagedBy: v1alpha1.LabelValManagedByOperator, + v1alpha1.LabelKeyInstance: "aaa", + v1alpha1.LabelKeyCluster: "", + v1alpha1.LabelKeyComponent: v1alpha1.LabelValComponentTiKV, + } + return obj + }) + } + if obj != nil { + objs = append(objs, obj) + } + } + + return objs +} diff --git a/pkg/controllers/tikv/tasks/pod.go b/pkg/controllers/tikv/tasks/pod.go index d77fb85430..5d6af8de58 100644 --- a/pkg/controllers/tikv/tasks/pod.go +++ b/pkg/controllers/tikv/tasks/pod.go @@ -81,8 +81,9 @@ func TaskPod(state *ReconcileContext, c client.Client) task.Task { return task.Fail().With("can't minimize the deletion grace period of pod of tikv: %w", err) } + state.PodIsTerminating = true // key will be requeued after the pod is changed - return task.Complete().With("pod is deleting") + return task.Wait().With("pod is deleting") } res := k8s.ComparePods(state.Pod(), expected) @@ -102,7 +103,7 @@ func TaskPod(state *ReconcileContext, c client.Client) task.Task { } state.PodIsTerminating = true - return task.Complete().With("pod is deleting") + return task.Wait().With("pod is deleting") } else if res == k8s.CompareResultUpdate { logger.Info("will update the pod in place") if err := c.Apply(ctx, expected); err != nil { diff --git a/pkg/controllers/tikv/tasks/pod_test.go b/pkg/controllers/tikv/tasks/pod_test.go new file mode 100644 index 0000000000..e6abe27c34 --- /dev/null +++ b/pkg/controllers/tikv/tasks/pod_test.go @@ -0,0 +1,325 @@ +// 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" + "k8s.io/utils/ptr" + + "github.com/pingcap/tidb-operator/apis/core/v1alpha1" + "github.com/pingcap/tidb-operator/pkg/client" + pdv1 "github.com/pingcap/tidb-operator/pkg/timanager/apis/pd/v1" + "github.com/pingcap/tidb-operator/pkg/utils/fake" + "github.com/pingcap/tidb-operator/pkg/utils/task/v3" +) + +const fakeVersion = "v1.2.3" + +func TestTaskPod(t *testing.T) { + now := metav1.Now() + cases := []struct { + desc string + state *ReconcileContext + objs []client.Object + // if true, cannot apply pod + unexpectedErr bool + + expectUpdatedPod bool + expectedPodIsTerminating bool + expectedStatus task.Status + }{ + { + desc: "no pod", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Spec.Version = fakeVersion + return obj + }), + cluster: fake.FakeObj[v1alpha1.Cluster]("aaa"), + }, + }, + + expectUpdatedPod: true, + expectedStatus: task.SComplete, + }, + { + desc: "no pod, failed to apply", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Spec.Version = fakeVersion + return obj + }), + cluster: fake.FakeObj[v1alpha1.Cluster]("aaa"), + }, + }, + unexpectedErr: true, + + expectedStatus: task.SFail, + }, + { + desc: "pod is deleting", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Spec.Version = fakeVersion + return obj + }), + cluster: fake.FakeObj[v1alpha1.Cluster]("aaa"), + pod: fake.FakeObj("aaa-tikv-xxx", func(obj *corev1.Pod) *corev1.Pod { + obj.SetDeletionTimestamp(&now) + obj.SetDeletionGracePeriodSeconds(ptr.To[int64](100)) + return obj + }), + }, + Store: &pdv1.Store{ + RegionCount: 400, + }, + }, + + expectedPodIsTerminating: true, + expectedStatus: task.SWait, + }, + { + desc: "pod is deleting, failed to minimize the grace period", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Spec.Version = fakeVersion + return obj + }), + cluster: fake.FakeObj[v1alpha1.Cluster]("aaa"), + pod: fake.FakeObj("aaa-tikv-xxx", func(obj *corev1.Pod) *corev1.Pod { + obj.SetDeletionTimestamp(&now) + obj.SetDeletionGracePeriodSeconds(ptr.To[int64](100)) + return obj + }), + }, + Store: &pdv1.Store{ + RegionCount: 400, + }, + }, + unexpectedErr: true, + + expectedStatus: task.SFail, + }, + { + desc: "pod spec changed", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Spec.Version = fakeVersion + return obj + }), + cluster: fake.FakeObj[v1alpha1.Cluster]("aaa"), + pod: fake.FakeObj("aaa-tikv-xxx", func(obj *corev1.Pod) *corev1.Pod { + return obj + }), + }, + }, + + expectedPodIsTerminating: true, + expectedStatus: task.SWait, + }, + { + desc: "pod spec changed, failed to delete", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Spec.Version = fakeVersion + return obj + }), + cluster: fake.FakeObj[v1alpha1.Cluster]("aaa"), + pod: fake.FakeObj("aaa-tikv-xxx", func(obj *corev1.Pod) *corev1.Pod { + return obj + }), + }, + }, + unexpectedErr: true, + + expectedStatus: task.SFail, + }, + { + desc: "pod spec hash not changed, config changed, hot reload policy", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Spec.Version = fakeVersion + obj.Spec.UpdateStrategy.Config = v1alpha1.ConfigUpdateStrategyHotReload + return obj + }), + cluster: fake.FakeObj[v1alpha1.Cluster]("aaa"), + pod: fake.FakeObj("aaa-tikv-xxx", func(obj *corev1.Pod) *corev1.Pod { + obj.Labels = map[string]string{ + v1alpha1.LabelKeyConfigHash: "newest", + v1alpha1.LabelKeyPodSpecHash: "59f798884b", + } + return obj + }), + }, + ConfigHash: "newest", + }, + + expectUpdatedPod: true, + expectedStatus: task.SComplete, + }, + { + desc: "pod spec hash not changed, config changed, restart policy", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Spec.Version = fakeVersion + obj.Spec.UpdateStrategy.Config = v1alpha1.ConfigUpdateStrategyRestart + return obj + }), + cluster: fake.FakeObj[v1alpha1.Cluster]("aaa"), + pod: fake.FakeObj("aaa-tikv-xxx", func(obj *corev1.Pod) *corev1.Pod { + obj.Labels = map[string]string{ + v1alpha1.LabelKeyConfigHash: "old", + v1alpha1.LabelKeyPodSpecHash: "59f798884b", + } + return obj + }), + }, + ConfigHash: "newest", + }, + + expectedPodIsTerminating: true, + expectedStatus: task.SWait, + }, + { + desc: "pod spec hash not changed, pod labels changed, config not changed", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Spec.Version = fakeVersion + obj.Spec.UpdateStrategy.Config = v1alpha1.ConfigUpdateStrategyRestart + return obj + }), + cluster: fake.FakeObj[v1alpha1.Cluster]("aaa"), + pod: fake.FakeObj("aaa-tikv-xxx", func(obj *corev1.Pod) *corev1.Pod { + obj.Labels = map[string]string{ + v1alpha1.LabelKeyConfigHash: "newest", + v1alpha1.LabelKeyPodSpecHash: "59f798884b", + "xxx": "yyy", + } + return obj + }), + }, + ConfigHash: "newest", + }, + + expectUpdatedPod: true, + expectedStatus: task.SComplete, + }, + { + desc: "pod spec hash not changed, pod labels changed, config not changed, apply failed", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Spec.Version = fakeVersion + obj.Spec.UpdateStrategy.Config = v1alpha1.ConfigUpdateStrategyRestart + return obj + }), + cluster: fake.FakeObj[v1alpha1.Cluster]("aaa"), + pod: fake.FakeObj("aaa-tikv-xxx", func(obj *corev1.Pod) *corev1.Pod { + obj.Labels = map[string]string{ + v1alpha1.LabelKeyConfigHash: "newest", + v1alpha1.LabelKeyPodSpecHash: "59f798884b", + "xxx": "yyy", + } + return obj + }), + }, + ConfigHash: "newest", + }, + unexpectedErr: true, + + expectedStatus: task.SFail, + }, + { + desc: "all are not changed", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj("aaa-xxx", func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Spec.Version = fakeVersion + obj.Spec.UpdateStrategy.Config = v1alpha1.ConfigUpdateStrategyRestart + return obj + }), + cluster: fake.FakeObj[v1alpha1.Cluster]("aaa"), + pod: fake.FakeObj("aaa-tikv-xxx", func(obj *corev1.Pod) *corev1.Pod { + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstance: "aaa-xxx", + v1alpha1.LabelKeyConfigHash: "newest", + v1alpha1.LabelKeyPodSpecHash: "59f798884b", + } + return obj + }), + }, + ConfigHash: "newest", + }, + + expectedStatus: task.SComplete, + }, + } + + 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()) + if c.state.Pod() != nil { + objs = append(objs, c.state.Pod()) + } + fc := client.NewFakeClient(objs...) + for _, obj := range c.objs { + require.NoError(tt, fc.Apply(ctx, obj), c.desc) + } + + if c.unexpectedErr { + // cannot update pod + fc.WithError("patch", "*", errors.NewInternalError(fmt.Errorf("fake internal err"))) + fc.WithError("delete", "*", errors.NewInternalError(fmt.Errorf("fake internal err"))) + } + + res, done := task.RunTask(ctx, TaskPod(c.state, fc)) + assert.Equal(tt, c.expectedStatus.String(), res.Status().String(), res.Message()) + assert.False(tt, done, c.desc) + + assert.Equal(tt, c.expectedPodIsTerminating, c.state.PodIsTerminating, c.desc) + + if c.expectUpdatedPod { + expectedPod := newPod(c.state.Cluster(), c.state.TiKV(), c.state.ConfigHash) + actual := c.state.Pod().DeepCopy() + actual.Kind = "" + actual.APIVersion = "" + actual.ManagedFields = nil + assert.Equal(tt, expectedPod, actual, 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/status.go b/pkg/controllers/tikv/tasks/status.go index 1349b9441c..2c05c582c8 100644 --- a/pkg/controllers/tikv/tasks/status.go +++ b/pkg/controllers/tikv/tasks/status.go @@ -45,13 +45,12 @@ func TaskStatus(state *ReconcileContext, c client.Client) task.Task { if pod != nil && statefulset.IsPodRunningAndReady(pod) && !state.PodIsTerminating && - state.Store != nil && - state.Store.NodeState == v1alpha1.StoreStateServing { + state.StoreState == v1alpha1.StoreStateServing { healthy = true } needUpdate = syncHealthCond(tikv, healthy) || needUpdate needUpdate = syncSuspendCond(tikv) || needUpdate - needUpdate = syncLeadersEvictedCond(tikv, state.Store, state.LeaderEvicting) || needUpdate + needUpdate = syncLeadersEvictedCond(tikv, state.Store, state.LeaderEvicting, state.IsPDAvailable) || needUpdate needUpdate = SetIfChanged(&tikv.Status.ID, state.StoreID) || needUpdate needUpdate = SetIfChanged(&tikv.Status.State, state.StoreState) || needUpdate @@ -68,11 +67,17 @@ func TaskStatus(state *ReconcileContext, c client.Client) task.Task { } } + if state.PodIsTerminating { + return task.Retry(defaultTaskWaitDuration).With("pod is terminating, retry after it's terminated") + } + + if state.LeaderEvicting { + return task.Wait().With("tikv is evicting leader, wait") + } + // TODO: use a condition to refactor it - if tikv.Status.ID == "" || tikv.Status.State != v1alpha1.StoreStateServing || !v1alpha1.IsUpToDate(tikv) { - // can we only rely on the PD member events for this condition? - // TODO(liubo02): change to task.Wait - return task.Retry(defaultTaskWaitDuration).With("tikv may not be initialized, retry") + if !healthy || tikv.Status.ID == "" { + return task.Wait().With("tikv may not be ready, wait") } return task.Complete().With("status is synced") @@ -112,19 +117,23 @@ func syncSuspendCond(tikv *v1alpha1.TiKV) bool { } // Status of this condition can only transfer as the below -func syncLeadersEvictedCond(tikv *v1alpha1.TiKV, store *pdv1.Store, isEvicting bool) bool { +func syncLeadersEvictedCond(tikv *v1alpha1.TiKV, store *pdv1.Store, isEvicting, isPDAvail bool) bool { status := metav1.ConditionFalse reason := "NotEvicted" msg := "leaders are not all evicted" switch { - case store == nil: + case isPDAvail && store == nil: status = metav1.ConditionTrue reason = "StoreIsRemoved" msg = "store does not exist" - case isEvicting && store.LeaderCount == 0: + case isPDAvail && isEvicting && store.LeaderCount == 0: status = metav1.ConditionTrue reason = "Evicted" msg = "all leaders are evicted" + case !isPDAvail: + status = metav1.ConditionUnknown + reason = "Unknown" + msg = "cannot get leaders info from pd" } return meta.SetStatusCondition(&tikv.Status.Conditions, metav1.Condition{ diff --git a/pkg/controllers/tikv/tasks/status_test.go b/pkg/controllers/tikv/tasks/status_test.go new file mode 100644 index 0000000000..53cae55f0f --- /dev/null +++ b/pkg/controllers/tikv/tasks/status_test.go @@ -0,0 +1,513 @@ +// 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" + pdv1 "github.com/pingcap/tidb-operator/pkg/timanager/apis/pd/v1" + "github.com/pingcap/tidb-operator/pkg/utils/fake" + "github.com/pingcap/tidb-operator/pkg/utils/task/v3" +) + +const ( + newRevision = "new" + oldRevision = "old" + + fakeTiKVName = "aaa-xxx" +) + +func TestTaskStatus(t *testing.T) { + now := metav1.Now() + cases := []struct { + desc string + state *ReconcileContext + unexpectedErr bool + + expectedStatus task.Status + expectedObj *v1alpha1.TiKV + }{ + { + desc: "no pod but healthy", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj(fakeTiKVName, func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Generation = 3 + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: newRevision, + } + obj.Status.CurrentRevision = "keep" + obj.Status.ObservedGeneration = 3 + return obj + }), + }, + IsPDAvailable: true, + Store: &pdv1.Store{}, + StoreID: fakeTiKVName, + StoreState: v1alpha1.StoreStateServing, + }, + + expectedStatus: task.SWait, + expectedObj: fake.FakeObj(fakeTiKVName, func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Generation = 3 + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: newRevision, + } + + obj.Status.ObservedGeneration = 3 + obj.Status.ID = fakeTiKVName + obj.Status.State = v1alpha1.StoreStateServing + obj.Status.UpdateRevision = newRevision + obj.Status.CurrentRevision = "keep" + obj.Status.Conditions = []metav1.Condition{ + { + Type: v1alpha1.CondHealth, + Status: metav1.ConditionFalse, + ObservedGeneration: 3, + Reason: "Unhealthy", + Message: "instance is not healthy", + }, + { + Type: v1alpha1.CondSuspended, + Status: metav1.ConditionFalse, + ObservedGeneration: 3, + Reason: v1alpha1.ReasonUnsuspended, + Message: "instace is not suspended", + }, + { + Type: v1alpha1.TiKVCondLeadersEvicted, + Status: metav1.ConditionFalse, + ObservedGeneration: 3, + Reason: "NotEvicted", + Message: "leaders are not all evicted", + }, + } + + return obj + }), + }, + { + desc: "pod is healthy", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj(fakeTiKVName, func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Generation = 3 + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: newRevision, + } + return obj + }), + pod: fake.FakeObj("aaa-tikv-xxx", func(obj *corev1.Pod) *corev1.Pod { + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: oldRevision, + } + obj.Status.Phase = corev1.PodRunning + obj.Status.Conditions = append(obj.Status.Conditions, corev1.PodCondition{ + Type: corev1.PodReady, + Status: corev1.ConditionTrue, + }) + return obj + }), + }, + IsPDAvailable: true, + Store: &pdv1.Store{}, + StoreID: fakeTiKVName, + StoreState: v1alpha1.StoreStateServing, + }, + + expectedStatus: task.SComplete, + expectedObj: fake.FakeObj(fakeTiKVName, func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Generation = 3 + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: newRevision, + } + + obj.Status.ObservedGeneration = 3 + obj.Status.ID = fakeTiKVName + obj.Status.State = v1alpha1.StoreStateServing + obj.Status.UpdateRevision = newRevision + obj.Status.CurrentRevision = oldRevision + obj.Status.Conditions = []metav1.Condition{ + { + Type: v1alpha1.CondHealth, + Status: metav1.ConditionTrue, + ObservedGeneration: 3, + Reason: "Healthy", + Message: "instance is healthy", + }, + { + Type: v1alpha1.CondSuspended, + Status: metav1.ConditionFalse, + ObservedGeneration: 3, + Reason: v1alpha1.ReasonUnsuspended, + Message: "instace is not suspended", + }, + { + Type: v1alpha1.TiKVCondLeadersEvicted, + Status: metav1.ConditionFalse, + ObservedGeneration: 3, + Reason: "NotEvicted", + Message: "leaders are not all evicted", + }, + } + + return obj + }), + }, + { + desc: "pod is deleting", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj(fakeTiKVName, func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Generation = 3 + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: newRevision, + } + return obj + }), + pod: fake.FakeObj("aaa-tikv-xxx", func(obj *corev1.Pod) *corev1.Pod { + obj.SetDeletionTimestamp(&now) + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: oldRevision, + } + obj.Status.Phase = corev1.PodRunning + obj.Status.Conditions = append(obj.Status.Conditions, corev1.PodCondition{ + Type: corev1.PodReady, + Status: corev1.ConditionTrue, + }) + return obj + }), + }, + IsPDAvailable: true, + Store: &pdv1.Store{}, + StoreID: fakeTiKVName, + PodIsTerminating: true, + }, + + expectedStatus: task.SRetry, + expectedObj: fake.FakeObj(fakeTiKVName, func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Generation = 3 + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: newRevision, + } + + obj.Status.ObservedGeneration = 3 + obj.Status.ID = fakeTiKVName + obj.Status.UpdateRevision = newRevision + obj.Status.Conditions = []metav1.Condition{ + { + Type: v1alpha1.CondHealth, + Status: metav1.ConditionFalse, + ObservedGeneration: 3, + Reason: "Unhealthy", + Message: "instance is not healthy", + }, + { + Type: v1alpha1.CondSuspended, + Status: metav1.ConditionFalse, + ObservedGeneration: 3, + Reason: v1alpha1.ReasonUnsuspended, + Message: "instace is not suspended", + }, + { + Type: v1alpha1.TiKVCondLeadersEvicted, + Status: metav1.ConditionFalse, + ObservedGeneration: 3, + Reason: "NotEvicted", + Message: "leaders are not all evicted", + }, + } + + return obj + }), + }, + { + desc: "not serving", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj(fakeTiKVName, func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Generation = 3 + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: newRevision, + } + return obj + }), + pod: fake.FakeObj("aaa-tikv-xxx", func(obj *corev1.Pod) *corev1.Pod { + obj.SetDeletionTimestamp(&now) + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: oldRevision, + } + obj.Status.Phase = corev1.PodRunning + obj.Status.Conditions = append(obj.Status.Conditions, corev1.PodCondition{ + Type: corev1.PodReady, + Status: corev1.ConditionTrue, + }) + return obj + }), + }, + IsPDAvailable: true, + Store: &pdv1.Store{}, + StoreID: fakeTiKVName, + }, + + expectedStatus: task.SWait, + expectedObj: fake.FakeObj(fakeTiKVName, func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Generation = 3 + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: newRevision, + } + + obj.Status.ObservedGeneration = 3 + obj.Status.ID = fakeTiKVName + obj.Status.UpdateRevision = newRevision + obj.Status.Conditions = []metav1.Condition{ + { + Type: v1alpha1.CondHealth, + Status: metav1.ConditionFalse, + ObservedGeneration: 3, + Reason: "Unhealthy", + Message: "instance is not healthy", + }, + { + Type: v1alpha1.CondSuspended, + Status: metav1.ConditionFalse, + ObservedGeneration: 3, + Reason: v1alpha1.ReasonUnsuspended, + Message: "instace is not suspended", + }, + { + Type: v1alpha1.TiKVCondLeadersEvicted, + Status: metav1.ConditionFalse, + ObservedGeneration: 3, + Reason: "NotEvicted", + Message: "leaders are not all evicted", + }, + } + + return obj + }), + }, + { + desc: "evicting and pd is avail", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj(fakeTiKVName, func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Generation = 3 + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: newRevision, + } + return obj + }), + pod: fake.FakeObj("aaa-tikv-xxx", func(obj *corev1.Pod) *corev1.Pod { + obj.SetDeletionTimestamp(&now) + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: oldRevision, + } + obj.Status.Phase = corev1.PodRunning + obj.Status.Conditions = append(obj.Status.Conditions, corev1.PodCondition{ + Type: corev1.PodReady, + Status: corev1.ConditionTrue, + }) + return obj + }), + }, + IsPDAvailable: true, + LeaderEvicting: true, + Store: &pdv1.Store{}, + StoreID: fakeTiKVName, + StoreState: v1alpha1.StoreStateServing, + }, + + expectedStatus: task.SWait, + expectedObj: fake.FakeObj(fakeTiKVName, func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Generation = 3 + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: newRevision, + } + + obj.Status.ObservedGeneration = 3 + obj.Status.ID = fakeTiKVName + obj.Status.State = v1alpha1.StoreStateServing + obj.Status.CurrentRevision = oldRevision + obj.Status.UpdateRevision = newRevision + obj.Status.Conditions = []metav1.Condition{ + { + Type: v1alpha1.CondHealth, + Status: metav1.ConditionTrue, + ObservedGeneration: 3, + Reason: "Healthy", + Message: "instance is healthy", + }, + { + Type: v1alpha1.CondSuspended, + Status: metav1.ConditionFalse, + ObservedGeneration: 3, + Reason: v1alpha1.ReasonUnsuspended, + Message: "instace is not suspended", + }, + { + Type: v1alpha1.TiKVCondLeadersEvicted, + Status: metav1.ConditionTrue, + ObservedGeneration: 3, + Reason: "Evicted", + Message: "all leaders are evicted", + }, + } + + return obj + }), + }, + { + desc: "pd is avail and no store", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj(fakeTiKVName, func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Generation = 3 + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: newRevision, + } + return obj + }), + pod: fake.FakeObj("aaa-tikv-xxx", func(obj *corev1.Pod) *corev1.Pod { + obj.SetDeletionTimestamp(&now) + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: oldRevision, + } + obj.Status.Phase = corev1.PodRunning + obj.Status.Conditions = append(obj.Status.Conditions, corev1.PodCondition{ + Type: corev1.PodReady, + Status: corev1.ConditionTrue, + }) + return obj + }), + }, + IsPDAvailable: true, + StoreID: fakeTiKVName, + }, + + expectedStatus: task.SWait, + expectedObj: fake.FakeObj(fakeTiKVName, func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Generation = 3 + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: newRevision, + } + + obj.Status.ObservedGeneration = 3 + obj.Status.ID = fakeTiKVName + obj.Status.UpdateRevision = newRevision + obj.Status.Conditions = []metav1.Condition{ + { + Type: v1alpha1.CondHealth, + Status: metav1.ConditionFalse, + ObservedGeneration: 3, + Reason: "Unhealthy", + Message: "instance is not healthy", + }, + { + Type: v1alpha1.CondSuspended, + Status: metav1.ConditionFalse, + ObservedGeneration: 3, + Reason: v1alpha1.ReasonUnsuspended, + Message: "instace is not suspended", + }, + { + Type: v1alpha1.TiKVCondLeadersEvicted, + Status: metav1.ConditionTrue, + ObservedGeneration: 3, + Reason: "StoreIsRemoved", + Message: "store does not exist", + }, + } + + return obj + }), + }, + { + desc: "failed to update status", + state: &ReconcileContext{ + State: &state{ + tikv: fake.FakeObj(fakeTiKVName, func(obj *v1alpha1.TiKV) *v1alpha1.TiKV { + obj.Generation = 3 + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: newRevision, + } + return obj + }), + pod: fake.FakeObj("aaa-tikv-xxx", func(obj *corev1.Pod) *corev1.Pod { + obj.SetDeletionTimestamp(&now) + obj.Labels = map[string]string{ + v1alpha1.LabelKeyInstanceRevisionHash: oldRevision, + } + obj.Status.Phase = corev1.PodRunning + obj.Status.Conditions = append(obj.Status.Conditions, corev1.PodCondition{ + Type: corev1.PodReady, + Status: corev1.ConditionTrue, + }) + return obj + }), + }, + }, + unexpectedErr: true, + + expectedStatus: task.SFail, + }, + } + + for i := range cases { + c := &cases[i] + t.Run(c.desc, func(tt *testing.T) { + tt.Parallel() + + var objs []client.Object + objs = append(objs, c.state.TiKV()) + if c.state.Pod() != nil { + objs = append(objs, c.state.Pod()) + } + fc := client.NewFakeClient(objs...) + if c.unexpectedErr { + fc.WithError("*", "*", errors.NewInternalError(fmt.Errorf("fake internal err"))) + } + + ctx := context.Background() + res, done := task.RunTask(ctx, TaskStatus(c.state, fc)) + assert.Equal(tt, c.expectedStatus.String(), res.Status().String(), c.desc) + assert.False(tt, done, c.desc) + + // no need to check update result + if c.unexpectedErr { + return + } + + obj := &v1alpha1.TiKV{} + require.NoError(tt, fc.Get(ctx, client.ObjectKey{Name: fakeTiKVName}, obj), c.desc) + conds := obj.Status.Conditions + for i := range conds { + cond := &conds[i] + cond.LastTransitionTime = metav1.Time{} + } + assert.Equal(tt, c.expectedObj, obj, c.desc) + }) + } +} diff --git a/pkg/controllers/tikv/tasks/store_labels.go b/pkg/controllers/tikv/tasks/store_labels.go index eb43ae1292..bcac0a09bd 100644 --- a/pkg/controllers/tikv/tasks/store_labels.go +++ b/pkg/controllers/tikv/tasks/store_labels.go @@ -46,7 +46,7 @@ func TaskStoreLabels(state *ReconcileContext, c client.Client) task.Task { } // TODO: too many API calls to PD? - pdCfg, err := state.PDClient.GetConfig(ctx) + pdCfg, err := state.PDClient.Underlay().GetConfig(ctx) if err != nil { return task.Fail().With("failed to get pd config: %s", err) } @@ -65,7 +65,7 @@ func TaskStoreLabels(state *ReconcileContext, c client.Client) task.Task { if err != nil { return task.Fail().With("failed to parse store id %s: %s", state.StoreID, err) } - set, err := state.PDClient.SetStoreLabels(ctx, storeID, storeLabels) + set, err := state.PDClient.Underlay().SetStoreLabels(ctx, storeID, storeLabels) if err != nil { return task.Fail().With("failed to set store labels: %s", err) } else if set { diff --git a/pkg/controllers/tikv/tasks/util.go b/pkg/controllers/tikv/tasks/util.go index fac5809fe7..d85f8cedca 100644 --- a/pkg/controllers/tikv/tasks/util.go +++ b/pkg/controllers/tikv/tasks/util.go @@ -23,20 +23,17 @@ import ( "github.com/pingcap/tidb-operator/pkg/client" ) -func ConfigMapName(tikvName string) string { - return tikvName +func ConfigMapName(podName string) string { + return podName } 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 { - fmt.Println("xxx: delete pod", pod, regionCount) if pod == nil { return nil } @@ -44,12 +41,9 @@ func DeletePodWithGracePeriod(ctx context.Context, c client.Client, pod *corev1. gracePeriod := CalcGracePeriod(regionCount) if sec == nil || *sec > gracePeriod { - fmt.Println("xxx: try to delete with gracePeriod", gracePeriod) if err := c.Delete(ctx, pod, client.GracePeriodSeconds(gracePeriod)); err != nil { return err } - } else { - fmt.Println("xxx: skip deletion with gracePeriod", gracePeriod) } return nil diff --git a/pkg/timanager/poller.go b/pkg/timanager/poller.go index 2bbdd8d69e..35bc831ad4 100644 --- a/pkg/timanager/poller.go +++ b/pkg/timanager/poller.go @@ -172,6 +172,7 @@ func (p *poller[T, PT, L]) poll(ctx context.Context) { if err != nil { p.logger.Error(err, "poll err", "cluster", p.name, "type", new(T)) p.markStateInvalid(ctx) + return } objs := p.lister.GetItems(list)