From 10864d3366fc89a795b843b2493c5d79213b5999 Mon Sep 17 00:00:00 2001 From: likakuli <1154584512@qq.com> Date: Fri, 3 Jan 2020 11:51:26 +0800 Subject: [PATCH] fix a bug that orphan revision cannot be adopted and sts cannot be synced --- pkg/controller/history/controller_history.go | 2 +- pkg/controller/statefulset/stateful_set.go | 9 ++- .../statefulset/stateful_set_control_test.go | 8 ++- .../statefulset/stateful_set_test.go | 58 ++++++++++++++- .../statefulset/stateful_set_utils_test.go | 70 +++++++++++++++++++ 5 files changed, 137 insertions(+), 10 deletions(-) diff --git a/pkg/controller/history/controller_history.go b/pkg/controller/history/controller_history.go index c867d2d86c5..ef3ba82fb4e 100644 --- a/pkg/controller/history/controller_history.go +++ b/pkg/controller/history/controller_history.go @@ -304,7 +304,7 @@ type objectMetaForPatch struct { func (rh *realHistory) AdoptControllerRevision(parent metav1.Object, parentKind schema.GroupVersionKind, revision *apps.ControllerRevision) (*apps.ControllerRevision, error) { blockOwnerDeletion := true isController := true - // Return an error if the parent does not own the revision + // Return an error if the revision is not orphan if owner := metav1.GetControllerOfNoCopy(revision); owner != nil { return nil, fmt.Errorf("attempt to adopt revision owned by %v", owner) } diff --git a/pkg/controller/statefulset/stateful_set.go b/pkg/controller/statefulset/stateful_set.go index 8f6122ba8d7..ac01f76791d 100644 --- a/pkg/controller/statefulset/stateful_set.go +++ b/pkg/controller/statefulset/stateful_set.go @@ -311,14 +311,13 @@ func (ssc *StatefulSetController) adoptOrphanRevisions(set *apps.StatefulSet) er if err != nil { return err } - hasOrphans := false + orphanRevisions := make([]*apps.ControllerRevision, 0) for i := range revisions { if metav1.GetControllerOf(revisions[i]) == nil { - hasOrphans = true - break + orphanRevisions = append(orphanRevisions, revisions[i]) } } - if hasOrphans { + if len(orphanRevisions) > 0 { fresh, err := ssc.kubeClient.AppsV1().StatefulSets(set.Namespace).Get(set.Name, metav1.GetOptions{}) if err != nil { return err @@ -326,7 +325,7 @@ func (ssc *StatefulSetController) adoptOrphanRevisions(set *apps.StatefulSet) er if fresh.UID != set.UID { return fmt.Errorf("original StatefulSet %v/%v is gone: got uid %v, wanted %v", set.Namespace, set.Name, fresh.UID, set.UID) } - return ssc.control.AdoptOrphanRevisions(set, revisions) + return ssc.control.AdoptOrphanRevisions(set, orphanRevisions) } return nil } diff --git a/pkg/controller/statefulset/stateful_set_control_test.go b/pkg/controller/statefulset/stateful_set_control_test.go index 7e5cfecdf36..36a11a04cfe 100644 --- a/pkg/controller/statefulset/stateful_set_control_test.go +++ b/pkg/controller/statefulset/stateful_set_control_test.go @@ -51,7 +51,7 @@ type invariantFunc func(set *apps.StatefulSet, spc *fakeStatefulPodControl) erro func setupController(client clientset.Interface) (*fakeStatefulPodControl, *fakeStatefulSetStatusUpdater, StatefulSetControlInterface, chan struct{}) { informerFactory := informers.NewSharedInformerFactory(client, controller.NoResyncPeriodFunc()) - spc := newFakeStatefulPodControl(informerFactory.Core().V1().Pods(), informerFactory.Apps().V1().StatefulSets()) + spc := newFakeStatefulPodControl(informerFactory.Core().V1().Pods(), informerFactory.Apps().V1().StatefulSets(), informerFactory.Apps().V1().ControllerRevisions()) ssu := newFakeStatefulSetStatusUpdater(informerFactory.Apps().V1().StatefulSets()) recorder := record.NewFakeRecorder(10) ssc := NewDefaultStatefulSetControl(spc, ssu, history.NewFakeHistory(informerFactory.Apps().V1().ControllerRevisions()), recorder) @@ -494,7 +494,7 @@ func TestStatefulSetControl_getSetRevisions(t *testing.T) { testFn := func(test *testcase, t *testing.T) { client := fake.NewSimpleClientset() informerFactory := informers.NewSharedInformerFactory(client, controller.NoResyncPeriodFunc()) - spc := newFakeStatefulPodControl(informerFactory.Core().V1().Pods(), informerFactory.Apps().V1().StatefulSets()) + spc := newFakeStatefulPodControl(informerFactory.Core().V1().Pods(), informerFactory.Apps().V1().StatefulSets(), informerFactory.Apps().V1().ControllerRevisions()) ssu := newFakeStatefulSetStatusUpdater(informerFactory.Apps().V1().StatefulSets()) recorder := record.NewFakeRecorder(10) ssc := defaultStatefulSetControl{spc, ssu, history.NewFakeHistory(informerFactory.Apps().V1().ControllerRevisions()), recorder} @@ -1554,12 +1554,13 @@ type fakeStatefulPodControl struct { podsIndexer cache.Indexer claimsIndexer cache.Indexer setsIndexer cache.Indexer + revisionsIndexer cache.Indexer createPodTracker requestTracker updatePodTracker requestTracker deletePodTracker requestTracker } -func newFakeStatefulPodControl(podInformer coreinformers.PodInformer, setInformer appsinformers.StatefulSetInformer) *fakeStatefulPodControl { +func newFakeStatefulPodControl(podInformer coreinformers.PodInformer, setInformer appsinformers.StatefulSetInformer, revisionInformer appsinformers.ControllerRevisionInformer) *fakeStatefulPodControl { claimsIndexer := cache.NewIndexer(controller.KeyFunc, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc}) return &fakeStatefulPodControl{ podInformer.Lister(), @@ -1568,6 +1569,7 @@ func newFakeStatefulPodControl(podInformer coreinformers.PodInformer, setInforme podInformer.Informer().GetIndexer(), claimsIndexer, setInformer.Informer().GetIndexer(), + revisionInformer.Informer().GetIndexer(), requestTracker{0, nil, 0}, requestTracker{0, nil, 0}, requestTracker{0, nil, 0}} diff --git a/pkg/controller/statefulset/stateful_set_test.go b/pkg/controller/statefulset/stateful_set_test.go index e1762152a38..67e9a9f7868 100644 --- a/pkg/controller/statefulset/stateful_set_test.go +++ b/pkg/controller/statefulset/stateful_set_test.go @@ -17,6 +17,8 @@ limitations under the License. package statefulset import ( + "bytes" + "encoding/json" "sort" "testing" @@ -24,6 +26,7 @@ import ( "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes/fake" @@ -33,6 +36,8 @@ import ( "k8s.io/kubernetes/pkg/controller/history" ) +var parentKind = apps.SchemeGroupVersion.WithKind("StatefulSet") + func alwaysReady() bool { return true } func TestStatefulSetControllerCreates(t *testing.T) { @@ -534,6 +539,48 @@ func TestGetPodsForStatefulSetAdopt(t *testing.T) { } } +func TestAdoptOrphanRevisions(t *testing.T) { + ss1 := newStatefulSetWithLabels(3, "ss1", types.UID("ss1"), map[string]string{"foo": "bar"}) + ss1.Status.CollisionCount = new(int32) + ss1Rev1, err := history.NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 1, ss1.Status.CollisionCount) + if err != nil { + t.Fatal(err) + } + ss1Rev1.Namespace = ss1.Namespace + ss1.Spec.Template.Annotations = make(map[string]string) + ss1.Spec.Template.Annotations["ss1"] = "ss1" + ss1Rev2, err := history.NewControllerRevision(ss1, parentKind, ss1.Spec.Template.Labels, rawTemplate(&ss1.Spec.Template), 2, ss1.Status.CollisionCount) + if err != nil { + t.Fatal(err) + } + ss1Rev2.Namespace = ss1.Namespace + ss1Rev2.OwnerReferences = []metav1.OwnerReference{} + + ssc, spc := newFakeStatefulSetController(ss1, ss1Rev1, ss1Rev2) + + spc.revisionsIndexer.Add(ss1Rev1) + spc.revisionsIndexer.Add(ss1Rev2) + + err = ssc.adoptOrphanRevisions(ss1) + if err != nil { + t.Errorf("adoptOrphanRevisions() error: %v", err) + } + + if revisions, err := ssc.control.ListRevisions(ss1); err != nil { + t.Errorf("ListRevisions() error: %v", err) + } else { + var adopted bool + for i := range revisions { + if revisions[i].Name == ss1Rev2.Name && metav1.GetControllerOf(revisions[i]) != nil { + adopted = true + } + } + if !adopted { + t.Error("adoptOrphanRevisions() not adopt orphan revisions") + } + } +} + func TestGetPodsForStatefulSetRelease(t *testing.T) { set := newStatefulSet(3) ssc, spc := newFakeStatefulSetController(set) @@ -575,7 +622,7 @@ func TestGetPodsForStatefulSetRelease(t *testing.T) { func newFakeStatefulSetController(initialObjects ...runtime.Object) (*StatefulSetController, *fakeStatefulPodControl) { client := fake.NewSimpleClientset(initialObjects...) informerFactory := informers.NewSharedInformerFactory(client, controller.NoResyncPeriodFunc()) - fpc := newFakeStatefulPodControl(informerFactory.Core().V1().Pods(), informerFactory.Apps().V1().StatefulSets()) + fpc := newFakeStatefulPodControl(informerFactory.Core().V1().Pods(), informerFactory.Apps().V1().StatefulSets(), informerFactory.Apps().V1().ControllerRevisions()) ssu := newFakeStatefulSetStatusUpdater(informerFactory.Apps().V1().StatefulSets()) ssc := NewStatefulSetController( informerFactory.Core().V1().Pods(), @@ -710,3 +757,12 @@ func scaleDownStatefulSetController(set *apps.StatefulSet, ssc *StatefulSetContr } return assertMonotonicInvariants(set, spc) } + +func rawTemplate(template *v1.PodTemplateSpec) runtime.RawExtension { + buf := new(bytes.Buffer) + enc := json.NewEncoder(buf) + if err := enc.Encode(template); err != nil { + panic(err) + } + return runtime.RawExtension{Raw: buf.Bytes()} +} diff --git a/pkg/controller/statefulset/stateful_set_utils_test.go b/pkg/controller/statefulset/stateful_set_utils_test.go index 2b07708b7fd..1b9f4693e6d 100644 --- a/pkg/controller/statefulset/stateful_set_utils_test.go +++ b/pkg/controller/statefulset/stateful_set_utils_test.go @@ -469,3 +469,73 @@ func newStatefulSet(replicas int) *apps.StatefulSet { } return newStatefulSetWithVolumes(replicas, "foo", petMounts, podMounts) } + +func newStatefulSetWithLabels(replicas int, name string, uid types.UID, labels map[string]string) *apps.StatefulSet { + // Converting all the map-only selectors to set-based selectors. + var testMatchExpressions []metav1.LabelSelectorRequirement + for key, value := range labels { + sel := metav1.LabelSelectorRequirement{ + Key: key, + Operator: metav1.LabelSelectorOpIn, + Values: []string{value}, + } + testMatchExpressions = append(testMatchExpressions, sel) + } + return &apps.StatefulSet{ + TypeMeta: metav1.TypeMeta{ + Kind: "StatefulSet", + APIVersion: "apps/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: v1.NamespaceDefault, + UID: uid, + }, + Spec: apps.StatefulSetSpec{ + Selector: &metav1.LabelSelector{ + // Purposely leaving MatchLabels nil, so to ensure it will break if any link + // in the chain ignores the set-based MatchExpressions. + MatchLabels: nil, + MatchExpressions: testMatchExpressions, + }, + Replicas: func() *int32 { i := int32(replicas); return &i }(), + Template: v1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: labels, + }, + Spec: v1.PodSpec{ + Containers: []v1.Container{ + { + Name: "nginx", + Image: "nginx", + VolumeMounts: []v1.VolumeMount{ + {Name: "datadir", MountPath: "/tmp/"}, + {Name: "home", MountPath: "/home"}, + }, + }, + }, + Volumes: []v1.Volume{{ + Name: "home", + VolumeSource: v1.VolumeSource{ + HostPath: &v1.HostPathVolumeSource{ + Path: fmt.Sprintf("/tmp/%v", "home"), + }, + }}}, + }, + }, + VolumeClaimTemplates: []v1.PersistentVolumeClaim{ + { + ObjectMeta: metav1.ObjectMeta{Name: "datadir"}, + Spec: v1.PersistentVolumeClaimSpec{ + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceStorage: *resource.NewQuantity(1, resource.BinarySI), + }, + }, + }, + }, + }, + ServiceName: "governingsvc", + }, + } +}