Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix issues #898 #900

Merged
merged 3 commits into from
Apr 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/recommendation/recommendation_checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
}
123 changes: 117 additions & 6 deletions pkg/controller/recommendation/recommendation_rule_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand All @@ -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) {
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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{})).
Expand All @@ -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...)
}
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

confirmed with @Cloudzp, no matter what mode craned is running, namespace is always being set on recommendation obj. so this safe for now, we need to make sure namespace is always being set even recommendation and target is in same namespace, otherwise this would cause
messes

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]
}
115 changes: 115 additions & 0 deletions pkg/controller/recommendation/recommendation_rule_controller_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
}
12 changes: 11 additions & 1 deletion pkg/metrics/analysis.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -48,5 +58,5 @@ var (
)

func init() {
metrics.Registry.MustRegister(ResourceRecommendation, ReplicasRecommendation, SelectTargets, RecommendationsStatus)
metrics.Registry.MustRegister(RecommendationExecutionCounter, ResourceRecommendation, ReplicasRecommendation, SelectTargets, RecommendationsStatus)
}
Loading