feat(metrics-operator): refactoring

Signed-off-by: realanna <[email protected]>
keptn · Aug 17, 2023 · 4ea5188 · 4ea5188
1 parent b497701
commit 4ea5188
Show file tree

Hide file tree

Showing 10 changed files with 36 additions and 1,063 deletions.
diff --git a/metrics-operator/cmd/fake/manager_mock.go b/metrics-operator/cmd/fake/manager_mock.go
diff --git a/metrics-operator/cmd/fake/provider_mock.go b/metrics-operator/cmd/fake/provider_mock.go
diff --git a/metrics-operator/cmd/fake/webhookmanager_mock.go b/metrics-operator/cmd/fake/webhookmanager_mock.go
diff --git a/metrics-operator/cmd/manager/manager.go b/metrics-operator/cmd/manager/manager.go
diff --git a/metrics-operator/controllers/analysis/controller.go b/metrics-operator/controllers/analysis/controller.go
@@ -37,7 +37,7 @@ type AnalysisReconciler struct {
 	Scheme     *runtime.Scheme
 	Log        logr.Logger
 	MaxWorkers int //maybe 2 or 4 as def
-	IWorkersPoolFactory
+	NewWorkersPoolFactory
 }
 
 //+kubebuilder:rbac:groups=metrics.keptn.sh,resources=analyses,verbs=get;list;watch;create;update;patch;delete
@@ -90,7 +90,7 @@ func (a *AnalysisReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 	}
 
 	//create multiple workers handling the Objectives
-	wp := a.NewWorkersPool(analysis, analysisDef, a.MaxWorkers, a.Client, a.Log)
+	wp := a.NewWorkersPoolFactory(analysis, analysisDef, a.MaxWorkers, a.Client, a.Log)
 
 	go wp.DispatchObjectives(ctx, wp.QueryWorker)
 

diff --git a/metrics-operator/controllers/analysis/controller_test.go b/metrics-operator/controllers/analysis/controller_test.go
@@ -69,11 +69,11 @@ func TestAnalysisReconciler_Reconcile_BasicControlLoop(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			a := &AnalysisReconciler{
-				Client:              tt.client,
-				Scheme:              tt.client.Scheme(),
-				Log:                 testr.New(t),
-				MaxWorkers:          2,
-				IWorkersPoolFactory: WorkersPoolFactory{},
+				Client:                tt.client,
+				Scheme:                tt.client.Scheme(),
+				Log:                   testr.New(t),
+				MaxWorkers:            2,
+				NewWorkersPoolFactory: NewWorkersPool,
 			}
 			got, err := a.Reconcile(context.TODO(), req)
 			if (err != nil) != tt.wantErr {
@@ -167,29 +167,28 @@ func TestAnalysisReconciler_Reconcile_WithMockedWorkers(t *testing.T) {
 	dispatchCount := int32(0)
 	queryCount := int32(0)
 
-	mockFactory := fake.MyFactoryMock{
-		NewWorkersPoolFunc: func(analysisMoqParam *metricsapi.Analysis, definition *metricsapi.AnalysisDefinition, numWorkers int, c client.Client, log logr.Logger) IAnalysisPool {
-			mymock := fake.MyAnalysisPoolMock{
-				CollectAnalysisResultsFunc: func() []metricstypes.AnalysisResult {
-					atomic.AddInt32(&collectorCount, 1)
-					return nil
-				},
-				DispatchObjectivesFunc: func(ctx context.Context, workerFunc func(ctx context.Context, id int)) {
-					atomic.AddInt32(&dispatchCount, 1)
-				},
-				QueryWorkerFunc: func(ctx context.Context, id int) {
-					atomic.AddInt32(&queryCount, 1)
-				},
-			}
-			return &mymock
-		},
+	mockFactory := func(analysisMoqParam *metricsapi.Analysis, definition *metricsapi.AnalysisDefinition, numWorkers int, c client.Client, log logr.Logger) IAnalysisPool {
+		mymock := fake.MyAnalysisPoolMock{
+			CollectAnalysisResultsFunc: func() []metricstypes.AnalysisResult {
+				atomic.AddInt32(&collectorCount, 1)
+				return nil
+			},
+			DispatchObjectivesFunc: func(ctx context.Context, workerFunc func(ctx context.Context, id int)) {
+				atomic.AddInt32(&dispatchCount, 1)
+			},
+			QueryWorkerFunc: func(ctx context.Context, id int) {
+				atomic.AddInt32(&queryCount, 1)
+			},
+		}
+		return &mymock
 	}
+
 	a := &AnalysisReconciler{
-		Client:              fclient,
-		Scheme:              fclient.Scheme(),
-		Log:                 testr.New(t),
-		MaxWorkers:          2,
-		IWorkersPoolFactory: &mockFactory,
+		Client:                fclient,
+		Scheme:                fclient.Scheme(),
+		Log:                   testr.New(t),
+		MaxWorkers:            2,
+		NewWorkersPoolFactory: mockFactory,
 	}
 	req := controllerruntime.Request{
 		NamespacedName: types.NamespacedName{Namespace: "default", Name: "my-analysis"},

diff --git a/metrics-operator/controllers/analysis/fake/factory_mock.go b/metrics-operator/controllers/analysis/fake/factory_mock.go
diff --git a/metrics-operator/controllers/analysis/workersPool.go b/metrics-operator/controllers/analysis/workersPool.go
@@ -19,15 +19,9 @@ type IAnalysisPool interface {
 	QueryWorker(ctx context.Context, id int)
 }
 
-//go:generate moq -pkg fake -skip-ensure -out ./fake/factory_mock.go . IWorkersPoolFactory:MyFactoryMock
-type IWorkersPoolFactory interface {
-	NewWorkersPool(analysis *metricsapi.Analysis, definition *metricsapi.AnalysisDefinition, numWorkers int, c client.Client, log logr.Logger) IAnalysisPool
-}
-
-type WorkersPoolFactory struct {
-}
+type NewWorkersPoolFactory func(analysis *metricsapi.Analysis, definition *metricsapi.AnalysisDefinition, numWorkers int, c client.Client, log logr.Logger) IAnalysisPool
 
-func (wpf WorkersPoolFactory) NewWorkersPool(analysis *metricsapi.Analysis, definition *metricsapi.AnalysisDefinition, numWorkers int, c client.Client, log logr.Logger) IAnalysisPool {
+func NewWorkersPool(analysis *metricsapi.Analysis, definition *metricsapi.AnalysisDefinition, numWorkers int, c client.Client, log logr.Logger) IAnalysisPool {
 	numJobs := len(definition.Spec.Objectives)
 	if numJobs <= numWorkers { // do not start useless go routines
 		numWorkers = numJobs
@@ -97,6 +91,7 @@ func (aw WorkersPool) QueryWorker(ctx context.Context, id int) {
 		)
 
 		if err != nil {
+			aw.Log.Error(err, "Failed to get the correct Provider")
 			aw.results <- metricstypes.AnalysisResult{Objective: &j, Err: err.Error()}
 			continue
 		}

diff --git a/metrics-operator/controllers/analysis/workersPool_test.go b/metrics-operator/controllers/analysis/workersPool_test.go
@@ -113,7 +113,7 @@ func TestNewWorkerPool(t *testing.T) {
 		results:    make(chan metricstypes.AnalysisResult, 1),
 	}
 
-	got := WorkersPoolFactory{}.NewWorkersPool(&analysis, &def, 4, nil, log)
+	got := NewWorkersPool(&analysis, &def, 4, nil, log)
 	require.Equal(t, want.Objectives, got.(WorkersPool).Objectives)
 	require.Equal(t, want.Analysis, got.(WorkersPool).Analysis)
 	//make sure never to create more workers than needed

diff --git a/metrics-operator/main.go b/metrics-operator/main.go
@@ -143,11 +143,11 @@ func main() {
 
 	analysisLogger := ctrl.Log.WithName("KeptnMetric Controller")
 	if err = (&analysiscontroller.AnalysisReconciler{
-		Client:              mgr.GetClient(),
-		Scheme:              mgr.GetScheme(),
-		Log:                 analysisLogger.V(env.AnalysisControllerLogLevel),
-		MaxWorkers:          4,
-		IWorkersPoolFactory: analysiscontroller.WorkersPoolFactory{},
+		Client:                mgr.GetClient(),
+		Scheme:                mgr.GetScheme(),
+		Log:                   analysisLogger.V(env.AnalysisControllerLogLevel),
+		MaxWorkers:            4,
+		NewWorkersPoolFactory: analysiscontroller.NewWorkersPool,
 	}).SetupWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "KeptnMetric")
 		os.Exit(1)