From e137aca00d98b726bb85eb3535c809d6b0e90756 Mon Sep 17 00:00:00 2001 From: odubajDT Date: Fri, 15 Sep 2023 10:15:39 +0200 Subject: [PATCH] chore(metrics-operator): refactor fetching resouce namespaces during analysis Signed-off-by: odubajDT --- .../api/v1alpha3/analysis_types.go | 7 +++ .../api/v1alpha3/analysis_types_test.go | 22 ++++++++ .../api/v1alpha3/analysisdefinition_types.go | 7 +++ .../v1alpha3/analysisdefinition_types_test.go | 12 ++++ .../v1alpha3/analysisvaluetemplate_types.go | 7 +++ .../analysisvaluetemplate_types_test.go | 17 ++++++ .../controllers/analysis/controller.go | 10 +--- .../controllers/analysis/controller_test.go | 55 +++++++++++++++++++ .../controllers/analysis/provider_selector.go | 17 ++---- .../analysis/provider_selector_test.go | 35 +++++++++++- metrics-operator/main.go | 1 - 11 files changed, 170 insertions(+), 20 deletions(-) create mode 100644 metrics-operator/api/v1alpha3/analysis_types_test.go create mode 100644 metrics-operator/api/v1alpha3/analysisvaluetemplate_types_test.go diff --git a/metrics-operator/api/v1alpha3/analysis_types.go b/metrics-operator/api/v1alpha3/analysis_types.go index 45d76dc82a4..1cfd64f7e2a 100644 --- a/metrics-operator/api/v1alpha3/analysis_types.go +++ b/metrics-operator/api/v1alpha3/analysis_types.go @@ -86,3 +86,10 @@ type Timeframe struct { func init() { SchemeBuilder.Register(&Analysis{}, &AnalysisList{}) } + +func (a *Analysis) GetAnalysisDefinitionNamespace() string { + if a.Spec.AnalysisDefinition.Namespace == "" { + return a.Namespace + } + return a.Spec.AnalysisDefinition.Namespace +} diff --git a/metrics-operator/api/v1alpha3/analysis_types_test.go b/metrics-operator/api/v1alpha3/analysis_types_test.go new file mode 100644 index 00000000000..5ec6d0e1cf0 --- /dev/null +++ b/metrics-operator/api/v1alpha3/analysis_types_test.go @@ -0,0 +1,22 @@ +package v1alpha3 + +import ( + "testing" + + "github.com/stretchr/testify/require" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestAnalysis_GetAnalysisDefinitionNamespace(t *testing.T) { + a := Analysis{ + ObjectMeta: v1.ObjectMeta{ + Namespace: "ns", + }, + } + + require.Equal(t, "ns", a.GetAnalysisDefinitionNamespace()) + + a.Spec.AnalysisDefinition.Namespace = "ns2" + + require.Equal(t, "ns2", a.GetAnalysisDefinitionNamespace()) +} diff --git a/metrics-operator/api/v1alpha3/analysisdefinition_types.go b/metrics-operator/api/v1alpha3/analysisdefinition_types.go index ab98d3651db..3fe7367b436 100644 --- a/metrics-operator/api/v1alpha3/analysisdefinition_types.go +++ b/metrics-operator/api/v1alpha3/analysisdefinition_types.go @@ -124,3 +124,10 @@ func init() { func (o *OperatorValue) GetFloatValue() float64 { return o.FixedValue.AsApproximateFloat64() } + +func (o *Objective) GetAnalysisValueTemplateNamespace(namespace string) string { + if o.AnalysisValueTemplateRef.Namespace == "" { + return namespace + } + return o.AnalysisValueTemplateRef.Namespace +} diff --git a/metrics-operator/api/v1alpha3/analysisdefinition_types_test.go b/metrics-operator/api/v1alpha3/analysisdefinition_types_test.go index 2751c17205d..dbf60b18718 100644 --- a/metrics-operator/api/v1alpha3/analysisdefinition_types_test.go +++ b/metrics-operator/api/v1alpha3/analysisdefinition_types_test.go @@ -14,3 +14,15 @@ func TestOperatorValue_GetFloatValue(t *testing.T) { require.Equal(t, 15.0, o.GetFloatValue()) } + +func TestObjective_GetAnalysisValueTemplateNamespace(t *testing.T) { + o := Objective{ + AnalysisValueTemplateRef: ObjectReference{}, + } + + require.Equal(t, "default", o.GetAnalysisValueTemplateNamespace("default")) + + o.AnalysisValueTemplateRef.Namespace = "ns" + + require.Equal(t, "ns", o.GetAnalysisValueTemplateNamespace("default")) +} diff --git a/metrics-operator/api/v1alpha3/analysisvaluetemplate_types.go b/metrics-operator/api/v1alpha3/analysisvaluetemplate_types.go index 24e3e80b0fe..57f29547ae4 100644 --- a/metrics-operator/api/v1alpha3/analysisvaluetemplate_types.go +++ b/metrics-operator/api/v1alpha3/analysisvaluetemplate_types.go @@ -56,3 +56,10 @@ type AnalysisValueTemplateList struct { func init() { SchemeBuilder.Register(&AnalysisValueTemplate{}, &AnalysisValueTemplateList{}) } + +func (a *AnalysisValueTemplate) GetProviderNamespace(namespace string) string { + if a.Spec.Provider.Namespace == "" { + return namespace + } + return a.Spec.Provider.Namespace +} diff --git a/metrics-operator/api/v1alpha3/analysisvaluetemplate_types_test.go b/metrics-operator/api/v1alpha3/analysisvaluetemplate_types_test.go new file mode 100644 index 00000000000..76b462fd80f --- /dev/null +++ b/metrics-operator/api/v1alpha3/analysisvaluetemplate_types_test.go @@ -0,0 +1,17 @@ +package v1alpha3 + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestAnalysisValueTemplate_GetProviderNamespace(t *testing.T) { + a := AnalysisValueTemplate{} + + require.Equal(t, "default", a.GetProviderNamespace("default")) + + a.Spec.Provider.Namespace = "ns" + + require.Equal(t, "ns", a.GetProviderNamespace("default")) +} diff --git a/metrics-operator/controllers/analysis/controller.go b/metrics-operator/controllers/analysis/controller.go index 0b7357db480..ecad9002ddb 100644 --- a/metrics-operator/controllers/analysis/controller.go +++ b/metrics-operator/controllers/analysis/controller.go @@ -41,7 +41,6 @@ type AnalysisReconciler struct { Scheme *runtime.Scheme Log logr.Logger MaxWorkers int //maybe 2 or 4 as def - Namespace string NewWorkersPoolFactory common.IAnalysisEvaluator } @@ -74,13 +73,10 @@ func (a *AnalysisReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c //find AnalysisDefinition to have the collection of Objectives analysisDef := &metricsapi.AnalysisDefinition{} - if analysis.Spec.AnalysisDefinition.Namespace == "" { - analysis.Spec.AnalysisDefinition.Namespace = a.Namespace - } err := a.Client.Get(ctx, types.NamespacedName{ Name: analysis.Spec.AnalysisDefinition.Name, - Namespace: analysis.Spec.AnalysisDefinition.Namespace}, + Namespace: analysis.GetAnalysisDefinitionNamespace()}, analysisDef, ) @@ -89,7 +85,7 @@ func (a *AnalysisReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c a.Log.Info( fmt.Sprintf("AnalysisDefinition '%s' in namespace '%s' not found, requeue", analysis.Spec.AnalysisDefinition.Name, - analysis.Spec.AnalysisDefinition.Namespace), + analysis.GetAnalysisDefinitionNamespace()), ) return ctrl.Result{Requeue: true, RequeueAfter: 10 * time.Second}, nil } @@ -107,7 +103,7 @@ func (a *AnalysisReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c } //create multiple workers handling the Objectives - childCtx, wp := a.NewWorkersPoolFactory(ctx, analysis, todo, a.MaxWorkers, a.Client, a.Log, a.Namespace) + childCtx, wp := a.NewWorkersPoolFactory(ctx, analysis, todo, a.MaxWorkers, a.Client, a.Log, analysis.GetAnalysisDefinitionNamespace()) res, err := wp.DispatchAndCollect(childCtx) if err != nil { diff --git a/metrics-operator/controllers/analysis/controller_test.go b/metrics-operator/controllers/analysis/controller_test.go index be7d0723549..3dfc947567d 100644 --- a/metrics-operator/controllers/analysis/controller_test.go +++ b/metrics-operator/controllers/analysis/controller_test.go @@ -25,6 +25,54 @@ func TestAnalysisReconciler_Reconcile_BasicControlLoop(t *testing.T) { analysis, analysisDef, template, _ := getTestCRDs() + analysis2 := metricsapi.Analysis{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-analysis", + Namespace: "default", + }, + Spec: metricsapi.AnalysisSpec{ + Timeframe: metricsapi.Timeframe{ + From: metav1.Time{ + Time: time.Now(), + }, + To: metav1.Time{ + Time: time.Now(), + }, + }, + Args: map[string]string{ + "good": "good", + "dot": ".", + }, + AnalysisDefinition: metricsapi.ObjectReference{ + Name: "my-analysis-def", + Namespace: "default2", + }, + }, + } + + analysisDef2 := metricsapi.AnalysisDefinition{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-analysis-def", + Namespace: "default2", + }, + Spec: metricsapi.AnalysisDefinitionSpec{ + Objectives: []metricsapi.Objective{ + { + AnalysisValueTemplateRef: metricsapi.ObjectReference{ + Name: "my-template", + Namespace: "default", + }, + Weight: 1, + KeyObjective: false, + }, + }, + TotalScore: metricsapi.TotalScore{ + PassPercentage: 0, + WarningPercentage: 0, + }, + }, + } + tests := []struct { name string client client.Client @@ -55,6 +103,13 @@ func TestAnalysisReconciler_Reconcile_BasicControlLoop(t *testing.T) { wantErr: false, status: &metricsapi.AnalysisStatus{Raw: "{\"objectiveResults\":null,\"totalScore\":0,\"maximumScore\":0,\"pass\":true,\"warning\":false}", Pass: true}, res: metricstypes.AnalysisResult{Pass: true}, + }, { + name: "succeeded - analysis in different namespace, status updated", + client: fake2.NewClient(&analysis2, &analysisDef2, &template), + want: controllerruntime.Result{}, + wantErr: false, + status: &metricsapi.AnalysisStatus{Raw: "{\"objectiveResults\":null,\"totalScore\":0,\"maximumScore\":0,\"pass\":true,\"warning\":false}", Pass: true}, + res: metricstypes.AnalysisResult{Pass: true}, }, } diff --git a/metrics-operator/controllers/analysis/provider_selector.go b/metrics-operator/controllers/analysis/provider_selector.go index 1ec5e6e6079..640eca0d99a 100644 --- a/metrics-operator/controllers/analysis/provider_selector.go +++ b/metrics-operator/controllers/analysis/provider_selector.go @@ -54,36 +54,31 @@ func (ps ProvidersPool) DispatchToProviders(ctx context.Context, id int) { default: ps.log.Info("worker", "id:", id, "started job:", j.AnalysisValueTemplateRef.Name) templ := &metricsapi.AnalysisValueTemplate{} - if j.AnalysisValueTemplateRef.Namespace == "" { - j.AnalysisValueTemplateRef.Namespace = ps.Namespace - } err := ps.Client.Get(ctx, types.NamespacedName{ Name: j.AnalysisValueTemplateRef.Name, - Namespace: j.AnalysisValueTemplateRef.Namespace}, + Namespace: j.GetAnalysisValueTemplateNamespace(ps.Namespace)}, templ, ) if err != nil { - ps.log.Error(err, "Failed to get the correct Provider") + ps.log.Error(err, "Failed to get AnalysisValueTemplate") ps.results <- metricsapi.ProviderResult{Objective: j.AnalysisValueTemplateRef, ErrMsg: err.Error()} ps.cancel() return } + ns := templ.GetProviderNamespace(ps.Namespace) providerRef := &metricsapi.KeptnMetricsProvider{} - if templ.Spec.Provider.Namespace == "" { - templ.Spec.Provider.Namespace = ps.Namespace - } err = ps.Client.Get(ctx, types.NamespacedName{ Name: templ.Spec.Provider.Name, - Namespace: templ.Spec.Provider.Namespace}, + Namespace: ns}, providerRef, ) if err != nil { - ps.log.Error(err, "Failed to get Provider") + ps.log.Error(err, "Failed to get KeptnMetricsProvider") ps.results <- metricsapi.ProviderResult{Objective: j.AnalysisValueTemplateRef, ErrMsg: err.Error()} ps.cancel() return @@ -91,7 +86,7 @@ func (ps ProvidersPool) DispatchToProviders(ctx context.Context, id int) { templatedQuery, err := generateQuery(templ.Spec.Query, ps.Analysis.Spec.Args) if err != nil { - ps.log.Error(err, "Failed to substitute args in templ") + ps.log.Error(err, "Failed to substitute args in AnalysisValueTemplate") ps.results <- metricsapi.ProviderResult{Objective: j.AnalysisValueTemplateRef, ErrMsg: err.Error()} ps.cancel() return diff --git a/metrics-operator/controllers/analysis/provider_selector_test.go b/metrics-operator/controllers/analysis/provider_selector_test.go index 9716cef6639..ad046c46563 100644 --- a/metrics-operator/controllers/analysis/provider_selector_test.go +++ b/metrics-operator/controllers/analysis/provider_selector_test.go @@ -11,6 +11,7 @@ import ( metricstypes "github.com/keptn/lifecycle-toolkit/metrics-operator/controllers/common/analysis/types" fake2 "github.com/keptn/lifecycle-toolkit/metrics-operator/controllers/common/fake" "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -75,7 +76,32 @@ func TestProvidersPool(t *testing.T) { analysis, analysisDef, template, provider := getTestCRDs() + provider2 := metricsapi.KeptnMetricsProvider{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-provider", + Namespace: "default2", + }, + Spec: metricsapi.KeptnMetricsProviderSpec{ + Type: "prometheus", + TargetServer: "localhost:2000", + }, + } + + template2 := metricsapi.AnalysisValueTemplate{ + ObjectMeta: metav1.ObjectMeta{ + Name: "my-template", + Namespace: "default", + }, + Spec: metricsapi.AnalysisValueTemplateSpec{ + Provider: metricsapi.ObjectReference{ + Name: "my-provider", + }, + Query: "this is a {{.good}} query{{.dot}}", + }, + } + provider.Spec.Type = "mock-provider" + provider2.Spec.Type = "mock-provider" testCases := []struct { name string @@ -101,6 +127,13 @@ func TestProvidersPool(t *testing.T) { Query: "this is a good query.", }, }, + { + name: "Success - provider in different namespace", + mockClient: fake2.NewClient(&analysis, &analysisDef, &template2, &provider2), + providerResult: &metricstypes.ProviderRequest{ + Query: "this is a good query.", + }, + }, } for _, tc := range testCases { @@ -118,7 +151,7 @@ func TestProvidersPool(t *testing.T) { IObjectivesEvaluator: mockEvaluator, Client: tc.mockClient, log: mockLogger, - Namespace: "default", + Namespace: "default2", Objectives: map[int][]metricsapi.Objective{ 1: analysisDef.Spec.Objectives, }, diff --git a/metrics-operator/main.go b/metrics-operator/main.go index 8d8f3a4fa42..d00fa1ea27b 100644 --- a/metrics-operator/main.go +++ b/metrics-operator/main.go @@ -194,7 +194,6 @@ func main() { Scheme: mgr.GetScheme(), Log: analysisLogger.V(env.AnalysisControllerLogLevel), MaxWorkers: 2, - Namespace: env.PodNamespace, NewWorkersPoolFactory: analysiscontroller.NewWorkersPool, IAnalysisEvaluator: &analysisEval, }).SetupWithManager(mgr); err != nil {