From c48a90b0080ac22bd4e110a86167145247684086 Mon Sep 17 00:00:00 2001 From: Cloudzp Date: Sun, 7 Apr 2024 18:20:07 +0800 Subject: [PATCH 1/3] fix issues #898 --- go.mod | 7 +- .../recommendation_rule_controller.go | 121 +++++++++++++++++- .../recommendation_rule_controller_test.go | 113 ++++++++++++++++ 3 files changed, 235 insertions(+), 6 deletions(-) create mode 100644 pkg/controller/recommendation/recommendation_rule_controller_test.go diff --git a/go.mod b/go.mod index 617c2e462..d370b38f9 100644 --- a/go.mod +++ b/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 diff --git a/pkg/controller/recommendation/recommendation_rule_controller.go b/pkg/controller/recommendation/recommendation_rule_controller.go index fa8ec55b4..d6ebfa77b 100644 --- a/pkg/controller/recommendation/recommendation_rule_controller.go +++ b/pkg/controller/recommendation/recommendation_rule_controller.go @@ -3,6 +3,10 @@ package recommendation import ( "context" "fmt" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/dynamic/dynamicinformer" + "k8s.io/client-go/tools/cache" "sort" "strconv" "strings" @@ -54,6 +58,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 +152,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 +249,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 +272,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...) } } @@ -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] +} diff --git a/pkg/controller/recommendation/recommendation_rule_controller_test.go b/pkg/controller/recommendation/recommendation_rule_controller_test.go new file mode 100644 index 000000000..ba436ba66 --- /dev/null +++ b/pkg/controller/recommendation/recommendation_rule_controller_test.go @@ -0,0 +1,113 @@ +package recommendation + +import ( + analysisv1alph1 "github.com/gocrane/api/analysis/v1alpha1" + corev1 "k8s.io/api/core/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "reflect" + "testing" +) + +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-name", + APIVersion: "app/v1", + }, + }, + }, + args: args{ + id: ObjectIdentity{ + Name: "test-deployment-name", + 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) + } + }) + } +} From 3bceccd13cd80bbcb59713b615c46ada5d100a46 Mon Sep 17 00:00:00 2001 From: Cloudzp Date: Tue, 16 Apr 2024 18:20:37 +0800 Subject: [PATCH 2/3] add metrics --- .../recommendation/recommendation_checker.go | 2 +- .../recommendation/recommendation_rule_controller.go | 1 + pkg/metrics/analysis.go | 12 +++++++++++- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/pkg/controller/recommendation/recommendation_checker.go b/pkg/controller/recommendation/recommendation_checker.go index c3ebfa064..1d9a646e5 100644 --- a/pkg/controller/recommendation/recommendation_checker.go +++ b/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()) } } diff --git a/pkg/controller/recommendation/recommendation_rule_controller.go b/pkg/controller/recommendation/recommendation_rule_controller.go index d6ebfa77b..d6f5dd596 100644 --- a/pkg/controller/recommendation/recommendation_rule_controller.go +++ b/pkg/controller/recommendation/recommendation_rule_controller.go @@ -461,6 +461,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 diff --git a/pkg/metrics/analysis.go b/pkg/metrics/analysis.go index bd930a474..e0078fe7f 100644 --- a/pkg/metrics/analysis.go +++ b/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) } From d8d5dd20a4bc5e3ba6f277f058e0a8d419253343 Mon Sep 17 00:00:00 2001 From: Cloudzp Date: Wed, 17 Apr 2024 16:02:50 +0800 Subject: [PATCH 3/3] fix ut and fmt error --- .../recommendation/recommendation_rule_controller.go | 9 ++++----- .../recommendation_rule_controller_test.go | 10 ++++++---- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/pkg/controller/recommendation/recommendation_rule_controller.go b/pkg/controller/recommendation/recommendation_rule_controller.go index d6f5dd596..feccea912 100644 --- a/pkg/controller/recommendation/recommendation_rule_controller.go +++ b/pkg/controller/recommendation/recommendation_rule_controller.go @@ -3,10 +3,6 @@ package recommendation import ( "context" "fmt" - "k8s.io/apimachinery/pkg/labels" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/client-go/dynamic/dynamicinformer" - "k8s.io/client-go/tools/cache" "sort" "strconv" "strings" @@ -19,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" @@ -34,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" diff --git a/pkg/controller/recommendation/recommendation_rule_controller_test.go b/pkg/controller/recommendation/recommendation_rule_controller_test.go index ba436ba66..f96536fcd 100644 --- a/pkg/controller/recommendation/recommendation_rule_controller_test.go +++ b/pkg/controller/recommendation/recommendation_rule_controller_test.go @@ -1,11 +1,12 @@ 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" - "reflect" - "testing" ) func TestRecommendationIndex_GetRecommendation(t *testing.T) { @@ -69,14 +70,15 @@ func TestRecommendationIndex_GetRecommendation(t *testing.T) { TargetRef: corev1.ObjectReference{ Namespace: "test-namespace", Kind: "Deployment", - Name: "test-deployment-name", + Name: "test-deployment-bar", APIVersion: "app/v1", }, + Type: analysisv1alph1.AnalysisTypeResource, }, }, args: args{ id: ObjectIdentity{ - Name: "test-deployment-name", + Name: "test-deployment-bar", Namespace: "test-namespace", APIVersion: "app/v1", Kind: "Deployment",