From c87e40be20cfb01414b70daa2b9e77d480edc371 Mon Sep 17 00:00:00 2001 From: csuzhangxc Date: Fri, 14 Jul 2023 08:51:06 +0000 Subject: [PATCH 1/3] try to Graceful Drain TiCDC for improper constraint image tag --- pkg/controller/ticdc_control.go | 10 ++++++++-- pkg/manager/member/ticdc_scaler.go | 14 ++++++++------ 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/pkg/controller/ticdc_control.go b/pkg/controller/ticdc_control.go index b712f62596..c2b4bce050 100644 --- a/pkg/controller/ticdc_control.go +++ b/pkg/controller/ticdc_control.go @@ -115,8 +115,9 @@ func (c *defaultTiCDCControl) DrainCapture(tc *v1alpha1.TidbCluster, ordinal int // Let caller retry drain capture. return 0, true, nil } - if len(captures) == 1 { + if len(captures) == 1 || len(captures) == 0 { // No way to drain a single node TiCDC cluster, ignore. + // Can't get capture info, ignore. return 0, false, nil } @@ -186,8 +187,9 @@ func (c *defaultTiCDCControl) ResignOwner(tc *v1alpha1.TidbCluster, ordinal int3 // Let caller retry resign owner. return false, nil } - if len(captures) == 1 { + if len(captures) == 1 || len(captures) == 0 { // No way to resign owner in a single node TiCDC cluster, ignore. + // Can't get capture info, ignore. return true, nil } @@ -237,6 +239,10 @@ func (c *defaultTiCDCControl) IsHealthy(tc *v1alpha1.TidbCluster, ordinal int32) // Let caller retry. return false, nil } + if len(captures) == 0 { + // No way to get capture info, ignore. + return true, nil + } _, owner := getOrdinalAndOwnerCaptureInfo(tc, ordinal, captures) if owner == nil { diff --git a/pkg/manager/member/ticdc_scaler.go b/pkg/manager/member/ticdc_scaler.go index a86ffebd68..bb715cd646 100644 --- a/pkg/manager/member/ticdc_scaler.go +++ b/pkg/manager/member/ticdc_scaler.go @@ -235,15 +235,17 @@ func isTiCDCPodSupportGracefulUpgrade( currentVersion := tc.TiCDCVersion() ge, err := cmpver.Compare(podVersion, cmpver.GreaterOrEqual, currentVersion) if err != nil { - klog.Errorf("ticdc.%s: fail to compare TiCDC pod version \"%s\", version \"%s\", error: %v, skip graceful shutdown", + klog.Warningf("ticdc.%s: fail to compare TiCDC pod version \"%s\", version \"%s\", error: %v, skip graceful shutdown", action, podVersion, currentVersion, err) - return false, nil + // if parse version failed, we still try to graftfully shutdown TiCDC, + // as we already checked `http.StatusNotFound` in `DrainCapture`, `ResignOwner` and `IsHealthy` + return true, nil } le, err := cmpver.Compare(podVersion, cmpver.LessOrEqual, currentVersion) if err != nil { - klog.Errorf("ticdc.%s: fail to compare TiCDC pod version \"%s\", version \"%s\", error: %v, skip graceful shutdown", + klog.Warningf("ticdc.%s: fail to compare TiCDC pod version \"%s\", version \"%s\", error: %v, skip graceful shutdown", action, podVersion, currentVersion, err) - return false, nil + return true, nil } // Reload TiCDC if the current version matches pod version. if ge && le { @@ -252,7 +254,7 @@ func isTiCDCPodSupportGracefulUpgrade( // We are performing cross version upgrade. lessThan63, err := cmpver.Compare(podVersion, cmpver.Less, ticdcCrossUpgradeVersion) if err != nil { - klog.Errorf("ticdc.%s: fail to compare TiCDC pod version \"%s\", version \"%s\", error: %v, skip graceful shutdown", + klog.Warningf("ticdc.%s: fail to compare TiCDC pod version \"%s\", version \"%s\", error: %v, skip graceful shutdown", action, podVersion, ticdcCrossUpgradeVersion, err) return false, nil } @@ -271,7 +273,7 @@ func isTiCDCPodSupportGracefulUpgrade( podVersionPlus2 := podVer.IncMajor().IncMajor() withInTwoMajorVersion, err := cmpver.Compare(currentVersion, cmpver.Less, podVersionPlus2.String()) if err != nil { - klog.Errorf("ticdc.%s: fail to compare version \"%s\", version \"%s\", error: %v, skip graceful shutdown", + klog.Warningf("ticdc.%s: fail to compare version \"%s\", version \"%s\", error: %v, skip graceful shutdown", action, currentVersion, ticdcCrossUpgradeVersion, err) return false, nil } From b854f115e79cdf243f4083120458aa2d4a12c3e0 Mon Sep 17 00:00:00 2001 From: csuzhangxc Date: Fri, 14 Jul 2023 09:24:21 +0000 Subject: [PATCH 2/3] fix log --- pkg/manager/member/ticdc_scaler.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/manager/member/ticdc_scaler.go b/pkg/manager/member/ticdc_scaler.go index bb715cd646..fa121cc0f2 100644 --- a/pkg/manager/member/ticdc_scaler.go +++ b/pkg/manager/member/ticdc_scaler.go @@ -235,7 +235,7 @@ func isTiCDCPodSupportGracefulUpgrade( currentVersion := tc.TiCDCVersion() ge, err := cmpver.Compare(podVersion, cmpver.GreaterOrEqual, currentVersion) if err != nil { - klog.Warningf("ticdc.%s: fail to compare TiCDC pod version \"%s\", version \"%s\", error: %v, skip graceful shutdown", + klog.Warningf("ticdc.%s: fail to compare TiCDC pod version \"%s\", version \"%s\", error: %v, still try to graceful shutdown", action, podVersion, currentVersion, err) // if parse version failed, we still try to graftfully shutdown TiCDC, // as we already checked `http.StatusNotFound` in `DrainCapture`, `ResignOwner` and `IsHealthy` @@ -243,7 +243,7 @@ func isTiCDCPodSupportGracefulUpgrade( } le, err := cmpver.Compare(podVersion, cmpver.LessOrEqual, currentVersion) if err != nil { - klog.Warningf("ticdc.%s: fail to compare TiCDC pod version \"%s\", version \"%s\", error: %v, skip graceful shutdown", + klog.Warningf("ticdc.%s: fail to compare TiCDC pod version \"%s\", version \"%s\", error: %v, still try to graceful shutdown", action, podVersion, currentVersion, err) return true, nil } From 83092bf9e7fed7657a792c721e298f9d4848fc05 Mon Sep 17 00:00:00 2001 From: csuzhangxc Date: Fri, 14 Jul 2023 09:56:25 +0000 Subject: [PATCH 3/3] fix UT --- pkg/manager/member/ticdc_scaler_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/manager/member/ticdc_scaler_test.go b/pkg/manager/member/ticdc_scaler_test.go index d437551e13..f550cf32b3 100644 --- a/pkg/manager/member/ticdc_scaler_test.go +++ b/pkg/manager/member/ticdc_scaler_test.go @@ -708,7 +708,7 @@ func TestTiCDCIsSupportGracefulUpgrade(t *testing.T) { expectedErr: false, }, { - caseName: "v6.3.0 -> latest, skip graceful upgrade", + caseName: "v6.3.0 -> latest, still try to graceful upgrade", cdcCtl: &controller.FakeTiCDCControl{ GetStatusFn: func(tc *v1alpha1.TidbCluster, ordinal int32) (*controller.CaptureStatus, error) { return &controller.CaptureStatus{ @@ -720,7 +720,7 @@ func TestTiCDCIsSupportGracefulUpgrade(t *testing.T) { v := "latest" tc.Spec.TiCDC.Version = &v }, - expectedOk: false, + expectedOk: true, expectedErr: false, }, { @@ -734,7 +734,7 @@ func TestTiCDCIsSupportGracefulUpgrade(t *testing.T) { expectedErr: true, }, { - caseName: "malformed pod version, skip graceful upgrade", + caseName: "malformed pod version, still try to graceful upgrade", cdcCtl: &controller.FakeTiCDCControl{ GetStatusFn: func(tc *v1alpha1.TidbCluster, ordinal int32) (*controller.CaptureStatus, error) { return &controller.CaptureStatus{ @@ -742,11 +742,11 @@ func TestTiCDCIsSupportGracefulUpgrade(t *testing.T) { }, nil }, }, - expectedOk: false, + expectedOk: true, expectedErr: false, }, { - caseName: "malformed tc version, skip graceful upgrade", + caseName: "malformed tc version, still try to graceful upgrade", cdcCtl: &controller.FakeTiCDCControl{ GetStatusFn: func(tc *v1alpha1.TidbCluster, ordinal int32) (*controller.CaptureStatus, error) { return &controller.CaptureStatus{ @@ -757,11 +757,11 @@ func TestTiCDCIsSupportGracefulUpgrade(t *testing.T) { changeTc: func(tc *v1alpha1.TidbCluster) { tc.Spec.Version = "malformed" }, - expectedOk: false, + expectedOk: true, expectedErr: false, }, { - caseName: "malformed ticdc spec version, skip graceful upgrade", + caseName: "malformed ticdc spec version, still try to graceful upgrade", cdcCtl: &controller.FakeTiCDCControl{ GetStatusFn: func(tc *v1alpha1.TidbCluster, ordinal int32) (*controller.CaptureStatus, error) { return &controller.CaptureStatus{ @@ -773,7 +773,7 @@ func TestTiCDCIsSupportGracefulUpgrade(t *testing.T) { v := "malformed" tc.Spec.TiCDC.Version = &v }, - expectedOk: false, + expectedOk: true, expectedErr: false, }, {