Merge pull request #900 from Cloudzp/bugfix

 fix issues #898

Author: mfanjie

go.mod

@@ -9,6 +9,9 @@ require (
 	github.com/google/cadvisor v0.41.0
 	github.com/jaypipes/ghw v0.9.0
 	github.com/mjibson/go-dsp v0.0.0-20180508042940-11479a337f12
+	github.com/onsi/ginkgo v1.16.5
+	github.com/onsi/gomega v1.15.0
+	github.com/pkg/errors v0.9.1
 	github.com/prometheus/client_golang v1.11.0
 	github.com/prometheus/common v0.26.0
 	github.com/shirou/gopsutil v3.21.10+incompatible
@@ -78,6 +81,7 @@ require (
 	github.com/ghodss/yaml v1.0.0 // indirect
 	github.com/gin-contrib/sse v0.1.0 // indirect
 	github.com/go-logr/logr v0.4.0 // indirect
+	github.com/go-logr/zapr v0.4.0 // indirect
 	github.com/go-ole/go-ole v1.2.6 // indirect
 	github.com/go-openapi/jsonpointer v0.19.5 // indirect
 	github.com/go-openapi/jsonreference v0.19.5 // indirect
@@ -113,12 +117,12 @@ require (
 	github.com/modern-go/reflect2 v1.0.2 // indirect
 	github.com/mrunalp/fileutils v0.5.0 // indirect
 	github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
+	github.com/nxadm/tail v1.4.8 // indirect
 	github.com/opencontainers/go-digest v1.0.0 // indirect
 	github.com/opencontainers/image-spec v1.0.1 // indirect
 	github.com/opencontainers/runc v1.0.2 // indirect
 	github.com/opencontainers/runtime-spec v1.0.3-0.20210326190908-1c3f411f0417 // indirect
 	github.com/opencontainers/selinux v1.8.2 // indirect
-	github.com/pkg/errors v0.9.1 // indirect
 	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/prometheus/client_model v0.2.0 // indirect
 	github.com/prometheus/procfs v0.6.0 // indirect
@@ -155,6 +159,7 @@ require (
 	google.golang.org/appengine v1.6.7 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/natefinch/lumberjack.v2 v2.0.0 // indirect
+	gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 // indirect
 	gopkg.in/warnings.v0 v0.1.2 // indirect
 	gopkg.in/yaml.v2 v2.4.0 // indirect
 	gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b // indirect

pkg/controller/recommendation/recommendation_checker.go

@@ -60,6 +60,6 @@ func (r Checker) runChecker() {
 			"owner_name":    recommend.Spec.TargetRef.Name,
 			"update_status": updateStatus,
 			"result_status": resultStatus,
-		}).Set(1)
+		}).Set(time.Now().Sub(recommend.Status.LastUpdateTime.Time).Seconds())
 	}
 }

pkg/controller/recommendation/recommendation_rule_controller.go

@@ -15,12 +15,16 @@ import (
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	unstructuredv1 "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/discovery"
 	"k8s.io/client-go/dynamic"
+	"k8s.io/client-go/dynamic/dynamicinformer"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/scale"
+	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/client-go/util/retry"
 	"k8s.io/klog/v2"
@@ -30,7 +34,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 
 	analysisv1alph1 "github.com/gocrane/api/analysis/v1alpha1"
-
 	"github.com/gocrane/crane/pkg/known"
 	"github.com/gocrane/crane/pkg/metrics"
 	"github.com/gocrane/crane/pkg/oom"
@@ -54,6 +57,7 @@ type RecommendationRuleController struct {
 	dynamicClient   dynamic.Interface
 	discoveryClient discovery.DiscoveryInterface
 	Provider        providers.History
+	dynamicLister   DynamicLister
 }
 
 func (c *RecommendationRuleController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
@@ -147,9 +151,10 @@ func (c *RecommendationRuleController) doReconcile(ctx context.Context, recommen
 		keys = append(keys, k)
 	}
 	sort.Strings(keys) // sort key to get a certain order
+	recommendationIndex := NewRecommendationIndex(currRecommendations)
 	for _, key := range keys {
 		id := identities[key]
-		id.Recommendation = GetRecommendationFromIdentity(identities[key], currRecommendations)
+		id.Recommendation = recommendationIndex.GetRecommendation(id)
 		identitiesArray = append(identitiesArray, id)
 	}
 
@@ -243,6 +248,8 @@ func (c *RecommendationRuleController) SetupWithManager(mgr ctrl.Manager) error
 	c.kubeClient = kubernetes.NewForConfigOrDie(mgr.GetConfig())
 	c.discoveryClient = discovery.NewDiscoveryClientForConfigOrDie(mgr.GetConfig())
 	c.dynamicClient = dynamic.NewForConfigOrDie(mgr.GetConfig())
+	dynamicInformerFactory := dynamicinformer.NewDynamicSharedInformerFactory(c.dynamicClient, 0)
+	c.dynamicLister = NewDynamicInformerLister(dynamicInformerFactory)
 
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&analysisv1alph1.RecommendationRule{}, builder.WithPredicates(predicate.GenerationChangedPredicate{})).
@@ -264,19 +271,19 @@ func (c *RecommendationRuleController) getIdentities(ctx context.Context, recomm
 
 		var unstructureds []unstructuredv1.Unstructured
 		if recommendationRule.Spec.NamespaceSelector.Any {
-			unstructuredList, err := c.dynamicClient.Resource(*gvr).List(ctx, metav1.ListOptions{})
+			unstructuredList, err := c.dynamicLister.List(ctx, *gvr, "")
 			if err != nil {
 				return nil, err
 			}
-			unstructureds = append(unstructureds, unstructuredList.Items...)
+			unstructureds = append(unstructureds, unstructuredList...)
 		} else {
 			for _, namespace := range recommendationRule.Spec.NamespaceSelector.MatchNames {
-				unstructuredList, err := c.dynamicClient.Resource(*gvr).Namespace(namespace).List(ctx, metav1.ListOptions{})
+				unstructuredList, err := c.dynamicLister.List(ctx, *gvr, namespace)
 				if err != nil {
 					return nil, err
 				}
 
-				unstructureds = append(unstructureds, unstructuredList.Items...)
+				unstructureds = append(unstructureds, unstructuredList...)
 			}
 		}
 
@@ -453,6 +460,7 @@ func executeIdentity(ctx context.Context, wg *sync.WaitGroup, recommenderMgr rec
 	defer func() {
 		if wg != nil {
 			wg.Done()
+			metrics.RecommendationExecutionCounter.WithLabelValues(id.APIVersion, id.Kind, id.Namespace, id.Name, id.Recommender).Inc()
 		}
 	}()
 	var message string
@@ -528,3 +536,106 @@ func IsConvertFromAnalytics(recommendationRule *analysisv1alph1.RecommendationRu
 
 	return false, ""
 }
+
+// DynamicLister is a lister for dynamic resources.
+type DynamicLister interface {
+	// List returns a list of resources matching the given groupVersionResource.
+	List(ctx context.Context, gvk schema.GroupVersionResource, namespace string) ([]unstructuredv1.Unstructured, error)
+}
+
+type dynamicInformerLister struct {
+	dynamicLister          map[schema.GroupVersionResource]cache.GenericLister
+	dynamicInformerFactory dynamicinformer.DynamicSharedInformerFactory
+	stopCh                 <-chan struct{}
+}
+
+func NewDynamicInformerLister(dynamicInformerFactory dynamicinformer.DynamicSharedInformerFactory) DynamicLister {
+	return &dynamicInformerLister{
+		dynamicLister:          map[schema.GroupVersionResource]cache.GenericLister{},
+		dynamicInformerFactory: dynamicInformerFactory,
+		stopCh:                 make(chan struct{}),
+	}
+}
+
+func (d *dynamicInformerLister) List(ctx context.Context, gvr schema.GroupVersionResource, namespace string) ([]unstructuredv1.Unstructured, error) {
+	var (
+		objects []runtime.Object
+		err     error
+	)
+
+	lister, exists := d.dynamicLister[gvr]
+	if !exists {
+		lister = d.dynamicInformerFactory.ForResource(gvr).Lister()
+		d.dynamicLister[gvr] = lister
+		d.dynamicInformerFactory.Start(d.stopCh)
+		if !d.dynamicInformerFactory.WaitForCacheSync(d.stopCh)[gvr] {
+			return nil, fmt.Errorf("failed to sync informer for %s", gvr)
+		}
+	}
+	if namespace != "" {
+		objects, err = lister.ByNamespace(namespace).List(labels.Everything())
+	} else {
+		objects, err = lister.List(labels.Everything())
+	}
+	if err != nil {
+		return nil, err
+	}
+
+	var unstructuredObjects []unstructuredv1.Unstructured
+	for _, obj := range objects {
+		unstructuredObj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj)
+		if err != nil {
+			return nil, err
+		}
+		unstructuredObjects = append(unstructuredObjects, unstructuredv1.Unstructured{Object: unstructuredObj})
+	}
+	return unstructuredObjects, nil
+}
+
+type IndexKey struct {
+	Namespace   string
+	APIVersion  string
+	Kind        string
+	Name        string
+	Recommender string
+}
+
+type RecommendationIndex struct {
+	mtx sync.RWMutex
+	idx map[IndexKey]*analysisv1alph1.Recommendation
+}
+
+func NewRecommendationIndex(recommendations analysisv1alph1.RecommendationList) *RecommendationIndex {
+	idx := make(map[IndexKey]*analysisv1alph1.Recommendation, len(recommendations.Items))
+	for i := range recommendations.Items {
+		r := &recommendations.Items[i]
+		idx[createIndexKey(r)] = r
+	}
+
+	return &RecommendationIndex{
+		idx: idx,
+	}
+}
+
+func createIndexKey(r *analysisv1alph1.Recommendation) IndexKey {
+	return IndexKey{
+		Kind:        r.Spec.TargetRef.Kind,
+		APIVersion:  r.Spec.TargetRef.APIVersion,
+		Namespace:   r.Spec.TargetRef.Namespace,
+		Name:        r.Spec.TargetRef.Name,
+		Recommender: string(r.Spec.Type),
+	}
+}
+
+func (idx *RecommendationIndex) GetRecommendation(id ObjectIdentity) *analysisv1alph1.Recommendation {
+	key := IndexKey{
+		Kind:        id.Kind,
+		APIVersion:  id.APIVersion,
+		Namespace:   id.Namespace,
+		Name:        id.Name,
+		Recommender: id.Recommender,
+	}
+	idx.mtx.RLock()
+	defer idx.mtx.RUnlock()
+	return idx.idx[key]
+}

pkg/controller/recommendation/recommendation_rule_controller_test.go

@@ -0,0 +1,115 @@
+package recommendation
+
+import (
+	"reflect"
+	"testing"
+
+	analysisv1alph1 "github.com/gocrane/api/analysis/v1alpha1"
+	corev1 "k8s.io/api/core/v1"
+	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func TestRecommendationIndex_GetRecommendation(t *testing.T) {
+	type fields struct {
+		recommendationList analysisv1alph1.RecommendationList
+	}
+	type args struct {
+		id ObjectIdentity
+	}
+
+	tests := []struct {
+		name   string
+		fields fields
+		args   args
+		want   *analysisv1alph1.Recommendation
+	}{
+		{
+			name: "TestRecommendationIndex_GetRecommendation good case",
+			fields: fields{
+				recommendationList: analysisv1alph1.RecommendationList{
+					Items: []analysisv1alph1.Recommendation{
+						{
+							ObjectMeta: v1.ObjectMeta{
+								Name:      "test-recommendation-rule",
+								Namespace: "test-namespace",
+							},
+							Spec: analysisv1alph1.RecommendationSpec{
+								TargetRef: corev1.ObjectReference{
+									Namespace:  "test-namespace",
+									Kind:       "Deployment",
+									Name:       "test-deployment-bar",
+									APIVersion: "app/v1",
+								},
+								Type: analysisv1alph1.AnalysisTypeResource,
+							},
+						},
+						{
+							ObjectMeta: v1.ObjectMeta{
+								Name:      "test-recommendation-rule",
+								Namespace: "test-namespace",
+							},
+							Spec: analysisv1alph1.RecommendationSpec{
+								TargetRef: corev1.ObjectReference{
+									Namespace:  "test-namespace",
+									Kind:       "Deployment",
+									Name:       "test-deployment-foo",
+									APIVersion: "app/v1",
+								},
+								Type: analysisv1alph1.AnalysisTypeResource,
+							},
+						},
+					},
+				},
+			},
+			want: &analysisv1alph1.Recommendation{
+				ObjectMeta: v1.ObjectMeta{
+					Name:      "test-recommendation-rule",
+					Namespace: "test-namespace",
+				},
+				Spec: analysisv1alph1.RecommendationSpec{
+					TargetRef: corev1.ObjectReference{
+						Namespace:  "test-namespace",
+						Kind:       "Deployment",
+						Name:       "test-deployment-bar",
+						APIVersion: "app/v1",
+					},
+					Type: analysisv1alph1.AnalysisTypeResource,
+				},
+			},
+			args: args{
+				id: ObjectIdentity{
+					Name:        "test-deployment-bar",
+					Namespace:   "test-namespace",
+					APIVersion:  "app/v1",
+					Kind:        "Deployment",
+					Recommender: "Resource",
+				},
+			},
+		},
+		{
+			name: "TestRecommendationIndex_GetRecommendation empty case",
+			fields: fields{
+				recommendationList: analysisv1alph1.RecommendationList{
+					Items: []analysisv1alph1.Recommendation{},
+				},
+			},
+			args: args{
+				id: ObjectIdentity{
+					Name:        "test-deployment-name",
+					Namespace:   "test-namespace",
+					APIVersion:  "app/v1",
+					Kind:        "Deployment",
+					Recommender: "Resources",
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			idx := NewRecommendationIndex(tt.fields.recommendationList)
+			if got := idx.GetRecommendation(tt.args.id); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("GetRecommendation() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

pkg/metrics/analysis.go

@@ -6,6 +6,16 @@ import (
 )
 
 var (
+	RecommendationExecutionCounter = prometheus.NewCounterVec(
+		prometheus.CounterOpts{
+			Namespace: "crane",
+			Subsystem: "analysis",
+			Name:      "recommendation_execution_total",
+			Help:      "The number of times Recommendation has been executed",
+		},
+		[]string{"apiversion", "owner_kind", "namespace", "owner_name", "type"},
+	)
+
 	ResourceRecommendation = prometheus.NewGaugeVec(
 		prometheus.GaugeOpts{
 			Namespace: "crane",
@@ -48,5 +58,5 @@ var (
 )
 
 func init() {
-	metrics.Registry.MustRegister(ResourceRecommendation, ReplicasRecommendation, SelectTargets, RecommendationsStatus)
+	metrics.Registry.MustRegister(RecommendationExecutionCounter, ResourceRecommendation, ReplicasRecommendation, SelectTargets, RecommendationsStatus)
 }