Repository: kubean-io/kubean
Pull request #1036 (2023-11-27 05:34:14)

Optimize manifest lookup overhead
<!--  

Thanks for sending a pull request!  Here are some tips for you:

* make sure your commit is signed off

-->

#### What type of PR is this?
/kind enhancement

<!--
Add one of the following kinds:
/kind bug
/kind cleanup
/kind documentation
/kind enhancement
/kind feature

Optionally add one or more of the following kinds if applicable:
/kind api-change
/kind deprecation
/kind failing-test
/kind flake
/kind design
/kind regression
-->

#### What this PR does / why we need it:
Optimize manifest lookup overhead

#### Which issue(s) this PR fixes:
<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
_If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
-->
Fixes #

#### Special notes for your reviewer:



Patch:
diff --git a/cmd/kubean-operator/app/app.go b/cmd/kubean-operator/app/app.go
index 24f105b02..16681b689 100644
--- a/cmd/kubean-operator/app/app.go
+++ b/cmd/kubean-operator/app/app.go
@@ -10,6 +10,7 @@ import (
 	"os"
 	"strconv"
 
+	manifestv1alpha1 "github.com/kubean-io/kubean-api/apis/manifest/v1alpha1"
 	kubeanClusterClientSet "github.com/kubean-io/kubean-api/generated/cluster/clientset/versioned"
 	kubeanClusterOperationClientSet "github.com/kubean-io/kubean-api/generated/clusteroperation/clientset/versioned"
 	kubeanLocalArtifactSetClientSet "github.com/kubean-io/kubean-api/generated/localartifactset/clientset/versioned"
@@ -24,6 +25,7 @@ import (
 	"github.com/spf13/cobra"
 	"k8s.io/client-go/kubernetes"
 	rest "k8s.io/client-go/rest"
+	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/tools/clientcmd"
 	klog "k8s.io/klog/v2"
 	controllerruntime "sigs.k8s.io/controller-runtime"
@@ -166,6 +168,25 @@ func setupManager(mgr controllerruntime.Manager, opt *Options, stopChan <-chan s
 		ClientSet:                 ClientSet,
 		LocalArtifactSetClientSet: localArtifactSetClientSet,
 	}
+	manifestInfomer, err := mgr.GetCache().GetInformer(context.Background(), &manifestv1alpha1.Manifest{})
+	if err != nil {
+		return err
+	}
+	manifestInfomer.AddEventHandler(cache.ResourceEventHandlerFuncs{
+		AddFunc: func(obj interface{}) {
+			manifest, _ := obj.(*manifestv1alpha1.Manifest)
+			infomanifest.GetVersionedManifest().Op("add", manifest, nil)
+		},
+		UpdateFunc: func(oldObj, newObj interface{}) {
+			manifest1, _ := oldObj.(*manifestv1alpha1.Manifest)
+			manifest2, _ := newObj.(*manifestv1alpha1.Manifest)
+			infomanifest.GetVersionedManifest().Op("update", manifest1, manifest2)
+		},
+		DeleteFunc: func(obj interface{}) {
+			manifest, _ := obj.(*manifestv1alpha1.Manifest)
+			infomanifest.GetVersionedManifest().Op("delete", manifest, nil)
+		},
+	})
 	if err := infomanifestController.SetupWithManager(mgr); err != nil {
 		klog.Errorf("ControllerManager Infomanifest but %s", err)
 		return err
diff --git a/pkg/controllers/infomanifest/controller.go b/pkg/controllers/infomanifest/controller.go
index 2cd0de06f..8055dc8ae 100644
--- a/pkg/controllers/infomanifest/controller.go
+++ b/pkg/controllers/infomanifest/controller.go
@@ -7,6 +7,7 @@ import (
 	"context"
 	"fmt"
 	"reflect"
+	"sync"
 	"time"
 
 	"github.com/kubean-io/kubean/pkg/util"
@@ -31,6 +32,8 @@ const Loop = time.Second * 30
 
 const LocalServiceConfigMap = "kubean-localservice"
 
+var versionedManifest *VersionedManifest
+
 type Controller struct {
 	Client                    client.Client
 	InfoManifestClientSet     manifestClientSet.Interface
@@ -38,12 +41,98 @@ type Controller struct {
 	LocalArtifactSetClientSet localartifactsetClientSet.Interface
 }
 
+type VersionedManifest struct {
+	mutex     sync.Mutex
+	Manifests map[string][]*manifestv1alpha1.Manifest
+}
+
 func (c *Controller) Start(ctx context.Context) error {
 	klog.Warningf("InfoManifest Controller Start")
 	<-ctx.Done()
 	return nil
 }
 
+func GetVersionedManifest() *VersionedManifest {
+	if versionedManifest == nil {
+		versionedManifest = &VersionedManifest{
+			Manifests: make(map[string][]*manifestv1alpha1.Manifest, 0),
+		}
+	}
+	return versionedManifest
+}
+
+func (m *VersionedManifest) Op(op string, m1, m2 *manifestv1alpha1.Manifest) {
+	m.mutex.Lock()
+	defer m.mutex.Unlock()
+
+	if op == "add" {
+		m.add(m1)
+	}
+	if op == "update" {
+		m.update(m1, m2)
+	}
+	if op == "delete" {
+		m.delete(m1)
+	}
+}
+
+func (m *VersionedManifest) add(manifest *manifestv1alpha1.Manifest) {
+	sprayRelease, ok := manifest.ObjectMeta.Labels[constants.KeySprayRelease]
+	if !ok {
+		return
+	}
+
+	manifests, ok := m.Manifests[sprayRelease]
+	if !ok {
+		manifests = make([]*manifestv1alpha1.Manifest, 0)
+	}
+	m.Manifests[sprayRelease] = append(manifests, manifest)
+}
+
+func (m *VersionedManifest) update(manifest1, manifest2 *manifestv1alpha1.Manifest) {
+	oldSprayRelease, ok1 := manifest1.ObjectMeta.Labels[constants.KeySprayRelease]
+	newSprayRelease, ok2 := manifest2.ObjectMeta.Labels[constants.KeySprayRelease]
+	if !ok1 && !ok2 {
+		return
+	}
+	if ok1 && !ok2 {
+		m.delete(manifest1)
+	}
+	if !ok1 && ok2 {
+		m.add(manifest2)
+	}
+	if oldSprayRelease != newSprayRelease || !reflect.DeepEqual(manifest1, manifest2) {
+		m.delete(manifest1)
+		m.add(manifest2)
+	}
+}
+
+func (m *VersionedManifest) delete(manifest *manifestv1alpha1.Manifest) {
+	sprayRelease, ok := manifest.ObjectMeta.Labels[constants.KeySprayRelease]
+	if !ok {
+		return
+	}
+	manifests, ok := m.Manifests[sprayRelease]
+	if !ok {
+		return
+	}
+
+	removeManifest := func(manifests []*manifestv1alpha1.Manifest, manifest *manifestv1alpha1.Manifest) []*manifestv1alpha1.Manifest {
+		for i, m := range manifests {
+			if m.Name == manifest.Name {
+				manifests = append(manifests[:i], manifests[i+1:]...)
+				break
+			}
+		}
+		return manifests
+	}
+
+	m.Manifests[sprayRelease] = removeManifest(manifests, manifest)
+	if len(m.Manifests[sprayRelease]) == 0 {
+		delete(m.Manifests, sprayRelease)
+	}
+}
+
 func (c *Controller) FetchLocalServiceCM(namespace string) (*corev1.ConfigMap, error) {
 	localServiceCM, err := c.ClientSet.CoreV1().ConfigMaps(namespace).Get(context.Background(), LocalServiceConfigMap, metav1.GetOptions{})
 	if err != nil && apierrors.IsNotFound(err) && namespace != "default" {
diff --git a/pkg/controllers/offlineversion/controller.go b/pkg/controllers/offlineversion/controller.go
index 0e264785d..28a6ec741 100644
--- a/pkg/controllers/offlineversion/controller.go
+++ b/pkg/controllers/offlineversion/controller.go
@@ -13,6 +13,7 @@ import (
 	"github.com/kubean-io/kubean-api/constants"
 	localartifactsetClientSet "github.com/kubean-io/kubean-api/generated/localartifactset/clientset/versioned"
 	manifestClientSet "github.com/kubean-io/kubean-api/generated/manifest/clientset/versioned"
+	"github.com/kubean-io/kubean/pkg/controllers/infomanifest"
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -38,23 +39,6 @@ func (c *Controller) Start(ctx context.Context) error {
 	return nil
 }
 
-// SelectManifestsBySprayRelease select manifests by using the label sprayRelease.
-func (c *Controller) SelectManifestsBySprayRelease(sprayRelease string) ([]*manifestv1alpha1.Manifest, error) {
-	selectedManifests, err := c.InfoManifestClientSet.KubeanV1alpha1().Manifests().List(context.Background(), metav1.ListOptions{LabelSelector: fmt.Sprintf("%s=%s", constants.KeySprayRelease, sprayRelease)})
-	if err != nil {
-		return nil, err
-	}
-	if len(selectedManifests.Items) == 0 {
-		return nil, nil
-	}
-	manifests := make([]*manifestv1alpha1.Manifest, 0, len(selectedManifests.Items))
-	for _, manifest := range selectedManifests.Items {
-		manifestsTmp := manifest
-		manifests = append(manifests, &manifestsTmp)
-	}
-	return manifests, nil
-}
-
 // MergeManifestsStatus merge the status of manifests which has same sprayRelease label of the localartifactset.
 func (c *Controller) MergeManifestsStatus(localartifactset *localartifactsetv1alpha1.LocalArtifactSet, manifests []*manifestv1alpha1.Manifest) ([]*manifestv1alpha1.Manifest, error) {
 	for _, manifest := range manifests {
@@ -96,12 +80,8 @@ func (c *Controller) Reconcile(ctx context.Context, req controllerruntime.Reques
 		return controllerruntime.Result{}, nil
 	}
 
-	manifests, err := c.SelectManifestsBySprayRelease(sprayRelease)
-	if err != nil {
-		klog.Error(err)
-		return controllerruntime.Result{RequeueAfter: Loop}, nil
-	}
-	if manifests == nil {
+	manifests, ok := infomanifest.GetVersionedManifest().Manifests[sprayRelease]
+	if !ok {
 		return controllerruntime.Result{RequeueAfter: Loop}, nil
 	}
 	if _, err := c.MergeManifestsStatus(localartifactset, manifests); err != nil {


Test patch:
diff --git a/pkg/controllers/infomanifest/controller_test.go b/pkg/controllers/infomanifest/controller_test.go
index 8fe9dd999..1984327eb 100644
--- a/pkg/controllers/infomanifest/controller_test.go
+++ b/pkg/controllers/infomanifest/controller_test.go
@@ -547,6 +547,160 @@ func TestUpdateLocalService(t *testing.T) {
 	}
 }
 
+func TestOp(t *testing.T) {
+	var versionedManifest VersionedManifest
+	type args struct {
+		op string
+		m1 *manifestv1alpha1.Manifest
+		m2 *manifestv1alpha1.Manifest
+	}
+	tests := []struct {
+		name          string
+		args          args
+		prerequisites func(versionedManifest *VersionedManifest)
+		want          func() map[string][]*manifestv1alpha1.Manifest
+	}{
+		{
+			name: "add",
+			args: args{
+				op: "add",
+				m1: &manifestv1alpha1.Manifest{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "manifest-1",
+						Labels: map[string]string{
+							constants.KeySprayRelease: "2.23",
+						},
+					},
+				},
+			},
+			prerequisites: func(versionedManifest *VersionedManifest) {},
+			want: func() map[string][]*manifestv1alpha1.Manifest {
+				return map[string][]*manifestv1alpha1.Manifest{
+					"2.23": {
+						{
+							ObjectMeta: metav1.ObjectMeta{
+								Name: "manifest-1",
+								Labels: map[string]string{
+									constants.KeySprayRelease: "2.23",
+								},
+							},
+						},
+					},
+				}
+			},
+		},
+		{
+			name: "add with no release label",
+			args: args{
+				op: "add",
+				m1: &manifestv1alpha1.Manifest{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "manifest-1",
+					},
+				},
+			},
+			prerequisites: func(versionedManifest *VersionedManifest) {},
+			want: func() map[string][]*manifestv1alpha1.Manifest {
+				return map[string][]*manifestv1alpha1.Manifest{}
+			},
+		},
+		{
+			name: "update",
+			args: args{
+				op: "update",
+				m1: &manifestv1alpha1.Manifest{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "manifest-1",
+						Labels: map[string]string{
+							constants.KeySprayRelease: "2.23",
+						},
+					},
+				},
+				m2: &manifestv1alpha1.Manifest{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "manifest-1",
+						Labels: map[string]string{
+							constants.KeySprayRelease: "2.22",
+						},
+					},
+				},
+			},
+			prerequisites: func(versionedManifest *VersionedManifest) {
+				versionedManifest.Manifests = map[string][]*manifestv1alpha1.Manifest{
+					"2.23": {
+						{
+							ObjectMeta: metav1.ObjectMeta{
+								Name: "manifest-1",
+								Labels: map[string]string{
+									constants.KeySprayRelease: "2.23",
+								},
+							},
+						},
+					},
+				}
+			},
+			want: func() map[string][]*manifestv1alpha1.Manifest {
+				return map[string][]*manifestv1alpha1.Manifest{
+					"2.22": {
+						{
+							ObjectMeta: metav1.ObjectMeta{
+								Name: "manifest-1",
+								Labels: map[string]string{
+									constants.KeySprayRelease: "2.22",
+								},
+							},
+						},
+					},
+				}
+			},
+		},
+		{
+			name: "delete",
+			args: args{
+				op: "delete",
+				m1: &manifestv1alpha1.Manifest{
+					ObjectMeta: metav1.ObjectMeta{
+						Name: "manifest-1",
+						Labels: map[string]string{
+							constants.KeySprayRelease: "2.23",
+						},
+					},
+				},
+			},
+			prerequisites: func(versionedManifest *VersionedManifest) {
+				versionedManifest.Manifests = map[string][]*manifestv1alpha1.Manifest{
+					"2.23": {
+						{
+							ObjectMeta: metav1.ObjectMeta{
+								Name: "manifest-1",
+								Labels: map[string]string{
+									constants.KeySprayRelease: "2.23",
+								},
+							},
+						},
+					},
+				}
+			},
+			want: func() map[string][]*manifestv1alpha1.Manifest {
+				return map[string][]*manifestv1alpha1.Manifest{}
+			},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			versionedManifest = VersionedManifest{
+				Manifests: make(map[string][]*manifestv1alpha1.Manifest, 0),
+			}
+			test.prerequisites(&versionedManifest)
+			versionedManifest.Op(test.args.op, test.args.m1, test.args.m2)
+			if !reflect.DeepEqual(test.want(), versionedManifest.Manifests) {
+				t.Fatal()
+			}
+		})
+	}
+}
+
 func TestReconcile(t *testing.T) {
 	controller := &Controller{
 		Client:                    newFakeClient(),
diff --git a/pkg/controllers/offlineversion/controller_test.go b/pkg/controllers/offlineversion/controller_test.go
index db2ab99b8..23c1ae668 100644
--- a/pkg/controllers/offlineversion/controller_test.go
+++ b/pkg/controllers/offlineversion/controller_test.go
@@ -432,89 +432,6 @@ func TestMergeManifestsStatus(t *testing.T) {
 	}
 }
 
-func TestSelectManifestsBySprayRelease(t *testing.T) {
-	controller := &Controller{
-		Client:                    newFakeClient(),
-		ClientSet:                 clientsetfake.NewSimpleClientset(),
-		LocalArtifactSetClientSet: localartifactsetv1alpha1fake.NewSimpleClientset(),
-		InfoManifestClientSet:     manifestv1alpha1fake.NewSimpleClientset(),
-	}
-
-	type wants struct {
-		err           bool
-		manifestCount int
-	}
-	tests := []struct {
-		name         string
-		arg          string
-		prerequisite func()
-		result       wants
-	}{
-		{
-			name:         "no manifest exist",
-			arg:          "1.1",
-			prerequisite: func() {},
-			result: wants{
-				err:           false,
-				manifestCount: 0,
-			},
-		},
-		{
-			name:         "manifest exist but no label",
-			arg:          "1.1",
-			prerequisite: func() {},
-			result: wants{
-				err:           false,
-				manifestCount: 0,
-			},
-		},
-		{
-			name: "manifest exist but has different label",
-			arg:  "1.1",
-			prerequisite: func() {
-				controller.InfoManifestClientSet.KubeanV1alpha1().Manifests().Create(context.Background(), &manifestv1alpha1.Manifest{
-					ObjectMeta: metav1.ObjectMeta{
-						Name:   "manifest1",
-						Labels: map[string]string{constants.KeySprayRelease: "1.2"},
-					},
-				}, metav1.CreateOptions{})
-			},
-			result: wants{
-				err:           false,
-				manifestCount: 0,
-			},
-		},
-		{
-			name: "manifest exist and has identical label",
-			arg:  "1.1",
-			prerequisite: func() {
-				_, err := controller.InfoManifestClientSet.KubeanV1alpha1().Manifests().Create(context.Background(), &manifestv1alpha1.Manifest{
-					ObjectMeta: metav1.ObjectMeta{
-						Name:   "manifest2",
-						Labels: map[string]string{constants.KeySprayRelease: "1.1"},
-					},
-				}, metav1.CreateOptions{})
-				if err != nil {
-					panic(err)
-				}
-			},
-			result: wants{
-				err:           false,
-				manifestCount: 1,
-			},
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			test.prerequisite()
-			if manifests, err := controller.SelectManifestsBySprayRelease(test.arg); (test.result.err != (err != nil)) || !(len(manifests) == test.result.manifestCount) {
-				t.Fatal()
-			}
-		})
-	}
-}
-
 func TestReconcile(t *testing.T) {
 	tests := []struct {
 		name string