Merge pull request kubernetes#86801 from likakuli/fix_syncsts_orphanrevision

fix a bug that orphan revision cannot be adopted and statefulset cannot be synced

Author: k8s-ci-robot

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)
 	}

pkg/controller/statefulset/stateful_set.go

@@ -311,22 +311,21 @@ 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
 		}
 		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
 }

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}}

pkg/controller/statefulset/stateful_set_test.go

@@ -17,13 +17,16 @@ limitations under the License.
 package statefulset
 
 import (
+	"bytes"
+	"encoding/json"
 	"sort"
 	"testing"
 
 	apps "k8s.io/api/apps/v1"
 	"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()}
+}

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",
+		},
+	}
+}