current delete slot annotations check in Advanced Statefulset upgrade…

…r is not right (#1851)
pingcap · Mar 3, 2020 · 596d10d · 596d10d
1 parent 0a0c44c
commit 596d10d
Show file tree

Hide file tree

Showing 2 changed files with 80 additions and 76 deletions.
diff --git a/pkg/upgrader/upgrader.go b/pkg/upgrader/upgrader.go
@@ -73,18 +73,22 @@ 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")
- 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)
- }
- }
+ // 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)
+ // }
+ // }
  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,