Revert "current delete slot annotations check in Advanced Statefulset…

… upgrader is not right (pingcap#1851)" This reverts commit 596d10d.
cofyc · Mar 16, 2020 · 4de6f7d · 4de6f7d
1 parent 784a9bc
commit 4de6f7d
Show file tree

Hide file tree

Showing 2 changed files with 76 additions and 80 deletions.
diff --git a/pkg/upgrader/upgrader.go b/pkg/upgrader/upgrader.go
@@ -73,22 +73,18 @@ func isOwnedByTidbCluster(obj metav1.Object) bool {
 func (u *upgrader) Upgrade() error {
 	if features.DefaultFeatureGate.Enabled(features.AdvancedStatefulSet) {
 		klog.Infof("Upgrader: migrating Kubernetes StatefulSets to Advanced StatefulSets")
-		// We should not check this, otherwise the controller-manager with
-		// advanced statefulset cannot be restarted when some clusters have
-		// delete slot annotations set.
-		// TODO find a better way
-		// tcList, err := u.cli.PingcapV1alpha1().TidbClusters(u.ns).List(metav1.ListOptions{})
-		// if err != nil {
-		// return err
-		// }
-		// for _, tc := range tcList.Items {
-		// // Existing delete slots annotations must be removed first. This is
-		// // a safety check to ensure no pods are affected in upgrading
-		// // process.
-		// if anns := deleteSlotAnns(&tc); len(anns) > 0 {
-		// return fmt.Errorf("Upgrader: TidbCluster %s/%s has delete slot annotations %v, please remove them before enabling AdvancedStatefulSet feature", tc.Namespace, tc.Name, anns)
-		// }
-		// }
+		tcList, err := u.cli.PingcapV1alpha1().TidbClusters(u.ns).List(metav1.ListOptions{})
+		if err != nil {
+			return err
+		}
+		for _, tc := range tcList.Items {
+			// Existing delete slots annotations must be removed first. This is
+			// a safety check to ensure no pods are affected in upgrading
+			// process.
+			if anns := deleteSlotAnns(&tc); len(anns) > 0 {
+				return fmt.Errorf("Upgrader: TidbCluster %s/%s has delete slot annotations %v, please remove them before enabling AdvancedStatefulSet feature", tc.Namespace, tc.Name, anns)
+			}
+		}
 		stsList, err := u.kubeCli.AppsV1().StatefulSets(u.ns).List(metav1.ListOptions{})
 		if err != nil {
 			return err

diff --git a/pkg/upgrader/upgrader_test.go b/pkg/upgrader/upgrader_test.go
@@ -314,70 +314,70 @@ func TestUpgrade(t *testing.T) {
 				},
 			},
 		},
-		// {
-		// name: "should not upgrade if tc has delete slot annotations",
-		// tidbClusters: []v1alpha1.TidbCluster{
-		// {
-		// ObjectMeta: metav1.ObjectMeta{
-		// Annotations: map[string]string{
-		// label.AnnTiDBDeleteSlots: "[1,2]",
-		// },
-		// },
-		// },
-		// },
-		// statefulsets: []appsv1.StatefulSet{
-		// {
-		// TypeMeta: metav1.TypeMeta{
-		// Kind:       "StatefulSet",
-		// APIVersion: "apps/v1",
-		// },
-		// ObjectMeta: metav1.ObjectMeta{
-		// Name:            "sts1",
-		// Namespace:       "sts",
-		// OwnerReferences: validOwnerRefs,
-		// },
-		// },
-		// {
-		// TypeMeta: metav1.TypeMeta{
-		// Kind:       "StatefulSet",
-		// APIVersion: "apps/v1",
-		// },
-		// ObjectMeta: metav1.ObjectMeta{
-		// Name:            "sts2",
-		// Namespace:       "sts",
-		// OwnerReferences: validOwnerRefs,
-		// },
-		// },
-		// },
-		// feature:                  "AdvancedStatefulSet=true",
-		// ns:                       metav1.NamespaceAll,
-		// wantErr:                  true,
-		// wantAdvancedStatefulsets: nil,
-		// wantStatefulsets: []appsv1.StatefulSet{
-		// {
-		// TypeMeta: metav1.TypeMeta{
-		// Kind:       "StatefulSet",
-		// APIVersion: "apps/v1",
-		// },
-		// ObjectMeta: metav1.ObjectMeta{
-		// Name:            "sts1",
-		// Namespace:       "sts",
-		// OwnerReferences: validOwnerRefs,
-		// },
-		// },
-		// {
-		// TypeMeta: metav1.TypeMeta{
-		// Kind:       "StatefulSet",
-		// APIVersion: "apps/v1",
-		// },
-		// ObjectMeta: metav1.ObjectMeta{
-		// Name:            "sts2",
-		// Namespace:       "sts",
-		// OwnerReferences: validOwnerRefs,
-		// },
-		// },
-		// },
-		// },
+		{
+			name: "should not upgrade if tc has delete slot annotations",
+			tidbClusters: []v1alpha1.TidbCluster{
+				{
+					ObjectMeta: metav1.ObjectMeta{
+						Annotations: map[string]string{
+							label.AnnTiDBDeleteSlots: "[1,2]",
+						},
+					},
+				},
+			},
+			statefulsets: []appsv1.StatefulSet{
+				{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       "StatefulSet",
+						APIVersion: "apps/v1",
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Name:            "sts1",
+						Namespace:       "sts",
+						OwnerReferences: validOwnerRefs,
+					},
+				},
+				{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       "StatefulSet",
+						APIVersion: "apps/v1",
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Name:            "sts2",
+						Namespace:       "sts",
+						OwnerReferences: validOwnerRefs,
+					},
+				},
+			},
+			feature:                  "AdvancedStatefulSet=true",
+			ns:                       metav1.NamespaceAll,
+			wantErr:                  true,
+			wantAdvancedStatefulsets: nil,
+			wantStatefulsets: []appsv1.StatefulSet{
+				{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       "StatefulSet",
+						APIVersion: "apps/v1",
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Name:            "sts1",
+						Namespace:       "sts",
+						OwnerReferences: validOwnerRefs,
+					},
+				},
+				{
+					TypeMeta: metav1.TypeMeta{
+						Kind:       "StatefulSet",
+						APIVersion: "apps/v1",
+					},
+					ObjectMeta: metav1.ObjectMeta{
+						Name:            "sts2",
+						Namespace:       "sts",
+						OwnerReferences: validOwnerRefs,
+					},
+				},
+			},
+		},
 		{
 			name:         "should ignore if sts is not owned by TidbCluster",
 			tidbClusters: nil,