add ownerUID index for controllerrevisions

Signed-off-by: shiyan2016 <shiyan20160606@gmail.com>
openkruise · Sep 23, 2024 · d5e1156 · d5e1156
1 parent 4918768
commit d5e1156
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 8 deletions.
diff --git a/pkg/controller/sidecarset/sidecarset_controller_test.go b/pkg/controller/sidecarset/sidecarset_controller_test.go
@@ -9,6 +9,7 @@ import (
 	appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1"
 	"github.com/openkruise/kruise/pkg/control/sidecarcontrol"
 
+	"github.com/openkruise/kruise/pkg/util/fieldindex"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -17,6 +18,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/intstr"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/tools/record"
+	"k8s.io/kubernetes/pkg/apis/apps"
 	utilpointer "k8s.io/utils/pointer"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
@@ -184,6 +186,13 @@ func testUpdateWhenUseNotUpdateStrategy(t *testing.T, sidecarSetInput *appsv1alp
 
 	fakeClient := fake.NewClientBuilder().WithScheme(scheme).
 		WithObjects(sidecarSetInput, podInput).
+		WithIndex(&apps.ControllerRevision{}, fieldindex.IndexNameForOwnerRefUID, func(obj client.Object) []string {
+			var owners []string
+			for _, ref := range obj.GetOwnerReferences() {
+				owners = append(owners, string(ref.UID))
+			}
+			return owners
+		}).
 		WithStatusSubresource(&appsv1alpha1.SidecarSet{}).Build()
 	reconciler := ReconcileSidecarSet{
 		Client:    fakeClient,
@@ -219,6 +228,13 @@ func testUpdateWhenSidecarSetPaused(t *testing.T, sidecarSetInput *appsv1alpha1.
 
 	fakeClient := fake.NewClientBuilder().WithScheme(scheme).
 		WithObjects(sidecarSetInput, podInput).
+		WithIndex(&apps.ControllerRevision{}, fieldindex.IndexNameForOwnerRefUID, func(obj client.Object) []string {
+			var owners []string
+			for _, ref := range obj.GetOwnerReferences() {
+				owners = append(owners, string(ref.UID))
+			}
+			return owners
+		}).
 		WithStatusSubresource(&appsv1alpha1.SidecarSet{}).Build()
 	reconciler := ReconcileSidecarSet{
 		Client:    fakeClient,
@@ -254,6 +270,13 @@ func testUpdateWhenMaxUnavailableNotZero(t *testing.T, sidecarSetInput *appsv1al
 
 	fakeClient := fake.NewClientBuilder().WithScheme(scheme).
 		WithObjects(sidecarSetInput, podInput).
+		WithIndex(&apps.ControllerRevision{}, fieldindex.IndexNameForOwnerRefUID, func(obj client.Object) []string {
+			var owners []string
+			for _, ref := range obj.GetOwnerReferences() {
+				owners = append(owners, string(ref.UID))
+			}
+			return owners
+		}).
 		WithStatusSubresource(&appsv1alpha1.SidecarSet{}).Build()
 	reconciler := ReconcileSidecarSet{
 		Client:    fakeClient,
@@ -289,6 +312,13 @@ func testUpdateWhenPartitionFinished(t *testing.T, sidecarSetInput *appsv1alpha1
 	}
 
 	fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(sidecarSetInput, podInput).
+		WithIndex(&apps.ControllerRevision{}, fieldindex.IndexNameForOwnerRefUID, func(obj client.Object) []string {
+			var owners []string
+			for _, ref := range obj.GetOwnerReferences() {
+				owners = append(owners, string(ref.UID))
+			}
+			return owners
+		}).
 		WithStatusSubresource(&appsv1alpha1.SidecarSet{}).Build()
 	reconciler := ReconcileSidecarSet{
 		Client:    fakeClient,
@@ -324,6 +354,13 @@ func testRemoveSidecarSet(t *testing.T, sidecarSetInput *appsv1alpha1.SidecarSet
 	}
 
 	fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(sidecarSetInput, podInput).
+		WithIndex(&apps.ControllerRevision{}, fieldindex.IndexNameForOwnerRefUID, func(obj client.Object) []string {
+			var owners []string
+			for _, ref := range obj.GetOwnerReferences() {
+				owners = append(owners, string(ref.UID))
+			}
+			return owners
+		}).
 		WithStatusSubresource(&appsv1alpha1.SidecarSet{}).Build()
 	reconciler := ReconcileSidecarSet{
 		Client:    fakeClient,

diff --git a/pkg/util/fieldindex/register.go b/pkg/util/fieldindex/register.go
@@ -23,6 +23,7 @@ import (
 	appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1"
 	utildiscovery "github.com/openkruise/kruise/pkg/util/discovery"
 
+	appsv1 "k8s.io/api/apps/v1"
 	batchv1 "k8s.io/api/batch/v1"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -65,6 +66,11 @@ func RegisterFieldIndexes(c cache.Cache) error {
 		if err = c.IndexField(context.TODO(), &v1.PersistentVolumeClaim{}, IndexNameForOwnerRefUID, ownerIndexFunc); err != nil {
 			return
 		}
+		// controller revision ownerReference
+		if err = c.IndexField(context.TODO(), &appsv1.ControllerRevision{}, IndexNameForOwnerRefUID, ownerIndexFunc); err != nil {
+			return
+		}
+
 		// ImagePullJob ownerReference
 		if err = c.IndexField(context.TODO(), &appsv1alpha1.ImagePullJob{}, IndexNameForOwnerRefUID, ownerIndexFunc); err != nil {
 			return

diff --git a/pkg/util/history/controller_history.go b/pkg/util/history/controller_history.go
@@ -24,12 +24,16 @@ import (
 	apps "k8s.io/api/apps/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/util/retry"
 	"k8s.io/kubernetes/pkg/controller/history"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	utilclient "github.com/openkruise/kruise/pkg/util/client"
+	"github.com/openkruise/kruise/pkg/util/fieldindex"
 )
 
 // NewHistory returns an an instance of Interface that uses client to communicate with the API Server and lister to list ControllerRevisions.
@@ -44,16 +48,14 @@ type realHistory struct {
 func (rh *realHistory) ListControllerRevisions(parent metav1.Object, selector labels.Selector) ([]*apps.ControllerRevision, error) {
 	// List all revisions in the namespace that match the selector
 	revisions := apps.ControllerRevisionList{}
-	err := rh.List(context.TODO(), &revisions, &client.ListOptions{Namespace: parent.GetNamespace(), LabelSelector: selector})
-	if err != nil {
-		return nil, err
+	opts := &client.ListOptions{
+		Namespace:     parent.GetNamespace(),
+		FieldSelector: fields.SelectorFromSet(fields.Set{fieldindex.IndexNameForOwnerRefUID: string(parent.GetUID())}),
 	}
+	err := rh.List(context.TODO(), &revisions, opts, utilclient.DisableDeepCopy)
 	var owned []*apps.ControllerRevision
 	for i := range revisions.Items {
-		ref := metav1.GetControllerOf(&revisions.Items[i])
-		if ref == nil || ref.UID == parent.GetUID() {
-			owned = append(owned, &revisions.Items[i])
-		}
+		owned = append(owned, &revisions.Items[i])
 	}
 	return owned, err
 }

diff --git a/pkg/util/history/controller_history_test.go b/pkg/util/history/controller_history_test.go
@@ -22,12 +22,14 @@ import (
 
 	appsv1alpha1 "github.com/openkruise/kruise/apis/apps/v1alpha1"
 	"github.com/openkruise/kruise/pkg/util"
+	"github.com/openkruise/kruise/pkg/util/fieldindex"
 	apps "k8s.io/api/apps/v1"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/uuid"
 	"k8s.io/kubernetes/pkg/controller/history"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 )
 
@@ -69,7 +71,13 @@ func TestRevisionHistory(t *testing.T) {
 		t.Fatalf("Failed to new controller revision: %v", err)
 	}
 
-	fakeClient := fake.NewClientBuilder().Build()
+	fakeClient := fake.NewClientBuilder().WithIndex(&apps.ControllerRevision{}, fieldindex.IndexNameForOwnerRefUID, func(obj client.Object) []string {
+		var owners []string
+		for _, ref := range obj.GetOwnerReferences() {
+			owners = append(owners, string(ref.UID))
+		}
+		return owners
+	}).Build()
 	historyControl := NewHistory(fakeClient)
 
 	newCR, err := historyControl.CreateControllerRevision(parent, cr, parent.Status.CollisionCount)