diff --git a/metrics-operator/controllers/analysis/objectives_evaluator_test.go b/metrics-operator/controllers/analysis/objectives_evaluator_test.go index 7edebdd283..1e7a1550a2 100644 --- a/metrics-operator/controllers/analysis/objectives_evaluator_test.go +++ b/metrics-operator/controllers/analysis/objectives_evaluator_test.go @@ -35,7 +35,7 @@ func TestEvaluate(t *testing.T) { }, }, providerRequest: metricstypes.ProviderRequest{ - Objective: &metricsapi.Objective{ + Objective: metricsapi.Objective{ AnalysisValueTemplateRef: metricsapi.ObjectReference{ Name: "mytemp", Namespace: "default", @@ -63,7 +63,7 @@ func TestEvaluate(t *testing.T) { }, }, providerRequest: metricstypes.ProviderRequest{ - Objective: &metricsapi.Objective{ + Objective: metricsapi.Objective{ AnalysisValueTemplateRef: metricsapi.ObjectReference{ Name: "mytemp", Namespace: "default", diff --git a/metrics-operator/controllers/analysis/provider_selector.go b/metrics-operator/controllers/analysis/provider_selector.go index 2bb9833738..2d38a70d89 100644 --- a/metrics-operator/controllers/analysis/provider_selector.go +++ b/metrics-operator/controllers/analysis/provider_selector.go @@ -41,7 +41,6 @@ func (ps ProvidersPool) StartProviders(ctx context.Context, numJobs int) { ps.providers[provider] = channel go ps.Evaluate(ctx, provider, channel) } - } func (ps ProvidersPool) DispatchToProviders(ctx context.Context, id int) { @@ -64,8 +63,7 @@ func (ps ProvidersPool) DispatchToProviders(ctx context.Context, id int) { if err != nil { ps.log.Error(err, "Failed to get AnalysisValueTemplate") ps.results <- metricsapi.ProviderResult{Objective: j.AnalysisValueTemplateRef, ErrMsg: err.Error()} - ps.cancel() - return + continue } providerRef := &metricsapi.KeptnMetricsProvider{} @@ -79,20 +77,18 @@ func (ps ProvidersPool) DispatchToProviders(ctx context.Context, id int) { if err != nil { ps.log.Error(err, "Failed to get KeptnMetricsProvider") ps.results <- metricsapi.ProviderResult{Objective: j.AnalysisValueTemplateRef, ErrMsg: err.Error()} - ps.cancel() - return + continue } templatedQuery, err := generateQuery(templ.Spec.Query, ps.Analysis.Spec.Args) if err != nil { ps.log.Error(err, "Failed to substitute args in AnalysisValueTemplate") ps.results <- metricsapi.ProviderResult{Objective: j.AnalysisValueTemplateRef, ErrMsg: err.Error()} - ps.cancel() - return + continue } //send job to provider solver ps.providers[providerRef.Spec.Type] <- metricstypes.ProviderRequest{ - Objective: &j, + Objective: j, Query: templatedQuery, Provider: providerRef, } diff --git a/metrics-operator/controllers/analysis/worker_pool.go b/metrics-operator/controllers/analysis/worker_pool.go index 88189b8826..d3920a5ca0 100644 --- a/metrics-operator/controllers/analysis/worker_pool.go +++ b/metrics-operator/controllers/analysis/worker_pool.go @@ -1,6 +1,8 @@ package analysis import ( + "time" + "github.com/go-logr/logr" metricsapi "github.com/keptn/lifecycle-toolkit/metrics-operator/api/v1alpha3" "github.com/keptn/lifecycle-toolkit/metrics-operator/controllers/common/analysis" @@ -23,7 +25,7 @@ func NewWorkersPool(ctx context.Context, analysis *metricsapi.Analysis, objectiv if numJobs <= numWorkers { // do not start useless go routines numWorkers = numJobs } - _, cancel := context.WithCancel(ctx) + _, cancel := context.WithTimeout(ctx, 10*time.Second) providerChans := make(map[string]chan metricstypes.ProviderRequest, len(providers.SupportedProviders)) assigner := TaskAssigner{tasks: objectives, numWorkers: numWorkers} @@ -73,24 +75,20 @@ func (aw WorkersPool) DispatchAndCollect(ctx context.Context) (map[string]metric func (aw WorkersPool) CollectAnalysisResults(ctx context.Context) (map[string]metricsapi.ProviderResult, error) { var err error results := make(map[string]metricsapi.ProviderResult, aw.numJobs) -loop: for a := 1; a <= aw.numJobs; a++ { select { case <-ctx.Done(): err = errors.New("Collection terminated") - break loop + break default: res, err2 := aw.GetResult(ctx) if err2 != nil { err = err2 - aw.cancel() - break loop - } - results[analysis.ComputeKey(res.Objective)] = *res - if res.ErrMsg != "" { - err = errors.New(res.ErrMsg) - aw.cancel() - break loop + } else { + results[analysis.ComputeKey(res.Objective)] = *res + if res.ErrMsg != "" { + err = errors.New(res.ErrMsg) + } } } } diff --git a/metrics-operator/controllers/analysis/worker_pool_test.go b/metrics-operator/controllers/analysis/worker_pool_test.go index bb5f0cffb8..8fa39fd329 100644 --- a/metrics-operator/controllers/analysis/worker_pool_test.go +++ b/metrics-operator/controllers/analysis/worker_pool_test.go @@ -72,6 +72,44 @@ func TestWorkersPool_CollectAnalysisResults(t *testing.T) { require.Equal(t, res2, results["t2"]) } +func TestWorkersPool_CollectAnalysisResultsWithError(t *testing.T) { + // Create a fake WorkersPool instance for testing + resChan := make(chan metricsapi.ProviderResult, 2) + fakePool := WorkersPool{ + IProvidersPool: ProvidersPool{ + results: resChan, + }, + numJobs: 2, + } + + res1 := metricsapi.ProviderResult{ + Objective: metricsapi.ObjectReference{Name: "t1"}, + Value: "result1", + ErrMsg: "", + } + + res2 := metricsapi.ProviderResult{ + Objective: metricsapi.ObjectReference{Name: "t2"}, + Value: "result2", + ErrMsg: "unexpected error", + } + + // Create and send mock results to the results channel + go func() { + time.Sleep(time.Second) + resChan <- res1 + resChan <- res2 + }() + + // Collect the results + results, err := fakePool.CollectAnalysisResults(context.TODO()) + + // Check the collected results + require.NotNil(t, err) + require.Equal(t, res1, results["t1"]) + require.Equal(t, res2, results["t2"]) +} + func TestWorkersPool_CollectAnalysisResultsTimeout(t *testing.T) { // Create a fake WorkersPool instance for testing resChan := make(chan metricsapi.ProviderResult, 2) diff --git a/metrics-operator/controllers/common/analysis/types/types.go b/metrics-operator/controllers/common/analysis/types/types.go index 99b2e3242a..c51420eac4 100644 --- a/metrics-operator/controllers/common/analysis/types/types.go +++ b/metrics-operator/controllers/common/analysis/types/types.go @@ -5,7 +5,7 @@ import ( ) type ProviderRequest struct { - Objective *v1alpha3.Objective + Objective v1alpha3.Objective Query string Provider *v1alpha3.KeptnMetricsProvider } diff --git a/test/integration/analysis-controller-multiple-providers/00-teststep-template.yaml b/test/integration/analysis-controller-multiple-providers/00-teststep-template.yaml new file mode 100644 index 0000000000..d38e50fe8d --- /dev/null +++ b/test/integration/analysis-controller-multiple-providers/00-teststep-template.yaml @@ -0,0 +1,9 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestStep +commands: + - script: | + envsubst < mock-server.yaml | kubectl apply -f - + # substitutes current time and namespace, making sure they are changed to env var first + # to prevent bad files in case of a test interrupt + - script: | + envsubst < install.yaml | kubectl apply -f - -n $NAMESPACE diff --git a/test/integration/analysis-controller-multiple-providers/01-assert.yaml b/test/integration/analysis-controller-multiple-providers/01-assert.yaml new file mode 100644 index 0000000000..eab2e366bc --- /dev/null +++ b/test/integration/analysis-controller-multiple-providers/01-assert.yaml @@ -0,0 +1,11 @@ +apiVersion: metrics.keptn.sh/v1alpha3 +kind: Analysis +metadata: + name: analysis-sample +spec: + analysisDefinition: + name: ed-my-proj-dev-svc1 +status: + pass: true + # yamllint disable-line rule:line-length + raw: '{"objectiveResults":[{"result":{"failResult":{"operator":{"lessThan":{"fixedValue":"5"}},"fulfilled":false},"warnResult":{"operator":{"lessThan":{"fixedValue":"4"}},"fulfilled":false},"warning":false,"pass":true},"value":11,"score":2},{"result":{"failResult":{"operator":{"greaterThan":{"fixedValue":"20"}},"fulfilled":false},"warnResult":{"operator":{"greaterThan":{"fixedValue":"15"}},"fulfilled":true},"warning":true,"pass":false},"value":20,"score":0.5},{"result":{"failResult":{"operator":{"notInRange":{"lowBound":"25","highBound":"35"}},"fulfilled":false},"warnResult":{"operator":{},"fulfilled":false},"warning":false,"pass":true},"value":30,"score":1}],"totalScore":3.5,"maximumScore":4,"pass":true,"warning":false}' diff --git a/test/integration/analysis-controller-multiple-providers/install.yaml b/test/integration/analysis-controller-multiple-providers/install.yaml new file mode 100644 index 0000000000..5ab468af4b --- /dev/null +++ b/test/integration/analysis-controller-multiple-providers/install.yaml @@ -0,0 +1,104 @@ +apiVersion: metrics.keptn.sh/v1alpha3 +kind: AnalysisValueTemplate +metadata: + name: value-1 +spec: + provider: + name: my-first-mocked-provider + query: 'query-1' +--- +apiVersion: metrics.keptn.sh/v1alpha3 +kind: AnalysisValueTemplate +metadata: + name: value-2 +spec: + provider: + name: my-second-mocked-provider + query: 'query-2' +--- +apiVersion: metrics.keptn.sh/v1alpha3 +kind: AnalysisValueTemplate +metadata: + name: value-3 +spec: + provider: + name: my-third-mocked-provider + query: 'query-3' +--- +apiVersion: metrics.keptn.sh/v1alpha3 +kind: AnalysisDefinition +metadata: + name: ed-my-proj-dev-svc1 +spec: + objectives: + - analysisValueTemplateRef: + name: value-1 + target: + failure: + lessThan: + fixedValue: 5 + warning: + lessThan: + fixedValue: 4 + weight: 2 + keyObjective: false + - analysisValueTemplateRef: + name: value-2 + target: + failure: + greaterThan: + fixedValue: 20 + warning: + greaterThan: + fixedValue: 15 + weight: 1 + keyObjective: false + - analysisValueTemplateRef: + name: value-3 + target: + failure: + notInRange: + lowBound: 25 + highBound: 35 + weight: 1 + keyObjective: false + totalScore: + passPercentage: 75 + warningPercentage: 50 +--- +apiVersion: metrics.keptn.sh/v1alpha3 +kind: Analysis +metadata: + name: analysis-sample +spec: + timeframe: + from: 2023-09-14T07:33:19Z + to: 2023-09-14T07:33:19Z + args: + "ns": "keptn-lifecycle-toolkit-system" + analysisDefinition: + name: ed-my-proj-dev-svc1 +--- +apiVersion: metrics.keptn.sh/v1alpha3 +kind: KeptnMetricsProvider +metadata: + name: my-first-mocked-provider +spec: + type: prometheus + targetServer: "http://mockserver.$NAMESPACE.svc.cluster.local:1080" +--- +apiVersion: metrics.keptn.sh/v1alpha3 +kind: KeptnMetricsProvider +metadata: + name: my-second-mocked-provider +spec: + type: prometheus + targetServer: "http://mockserver.$NAMESPACE.svc.cluster.local:1080" +--- +apiVersion: metrics.keptn.sh/v1alpha3 +kind: KeptnMetricsProvider +metadata: + name: my-third-mocked-provider +spec: + type: prometheus + targetServer: "http://mockserver.$NAMESPACE.svc.cluster.local:1080" diff --git a/test/integration/analysis-controller-multiple-providers/mock-server.yaml b/test/integration/analysis-controller-multiple-providers/mock-server.yaml new file mode 100644 index 0000000000..d437e62b58 --- /dev/null +++ b/test/integration/analysis-controller-multiple-providers/mock-server.yaml @@ -0,0 +1,211 @@ +apiVersion: v1 +kind: Service +metadata: + name: mockserver + namespace: $NAMESPACE +spec: + ports: + - name: serviceport + port: 1080 + protocol: TCP + targetPort: serviceport + selector: + app: mockserver + sessionAffinity: None + type: ClusterIP +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + labels: + app: mockserver + name: mockserver + namespace: $NAMESPACE +spec: + replicas: 1 + selector: + matchLabels: + app: mockserver + template: + metadata: + labels: + app: mockserver + name: mockserver + spec: + containers: + - env: + - name: MOCKSERVER_LOG_LEVEL + value: INFO + - name: SERVER_PORT + value: "1080" + image: mockserver/mockserver:mockserver-5.13.0 + imagePullPolicy: IfNotPresent + livenessProbe: + failureThreshold: 10 + initialDelaySeconds: 10 + periodSeconds: 5 + successThreshold: 1 + tcpSocket: + port: serviceport + timeoutSeconds: 1 + name: mockserver + ports: + - containerPort: 1080 + name: serviceport + protocol: TCP + readinessProbe: + failureThreshold: 10 + initialDelaySeconds: 2 + periodSeconds: 2 + successThreshold: 1 + tcpSocket: + port: serviceport + timeoutSeconds: 1 + volumeMounts: + - mountPath: /config + name: config-volume + - mountPath: /libs + name: libs-volume + terminationGracePeriodSeconds: 30 + volumes: + - configMap: + defaultMode: 420 + name: mockserver-config + optional: true + name: config-volume + - configMap: + defaultMode: 420 + name: mockserver-config + optional: true + name: libs-volume +--- +kind: ConfigMap +apiVersion: v1 +metadata: + name: mockserver-config + namespace: $NAMESPACE +data: + initializerJson.json: |- + [ + { + "httpRequest": { + "path": "/api/v1/query_range", + "method": "POST", + "body" : { + "type" : "PARAMETERS", + "parameters" : { + "query" : [ "query-1" ] + } + } + }, + "httpResponse": { + "body": { + "status": "success", + "data": { + "resultType": "matrix", + "result": [ + { + "metric": { + "__name__": "metric-name", + "job": "", + "instance": "" + }, + "values": [[1669714193.275, "11"]] + } + ] + } + }, + "statusCode": 200 + } + }, + { + "httpRequest": { + "path": "/api/v1/query_range", + "method": "POST", + "body" : { + "type" : "PARAMETERS", + "parameters" : { + "query" : [ "query-2" ] + } + } + }, + "httpResponse": { + "body": { + "status": "success", + "data": { + "resultType": "matrix", + "result": [ + { + "metric": { + "__name__": "metric-name", + "job": "", + "instance": "" + }, + "values": [[1669714193.275, "20"]] + } + ] + } + }, + "statusCode": 200 + } + }, + { + "httpRequest": { + "path": "/api/v1/query_range", + "method": "POST", + "body" : { + "type" : "PARAMETERS", + "parameters" : { + "query" : [ "query-3" ] + } + } + }, + "httpResponse": { + "body": { + "status": "success", + "data": { + "resultType": "matrix", + "result": [ + { + "metric": { + "__name__": "metric-name", + "job": "", + "instance": "" + }, + "values": [[1669714193.275, "30"]] + } + ] + } + }, + "statusCode": 200 + } + } + ] + mockserver.properties: |- + ############################### + # MockServer & Proxy Settings # + ############################### + # Socket & Port Settings + # socket timeout in milliseconds (default 120000) + mockserver.maxSocketTimeout=120000 + # Certificate Generation + # dynamically generated CA key pair (if they don't already exist in + specified directory) + mockserver.dynamicallyCreateCertificateAuthorityCertificate=true + # save dynamically generated CA key pair in working directory + mockserver.directoryToSaveDynamicSSLCertificate=. + # certificate domain name (default "localhost") + mockserver.sslCertificateDomainName=localhost + # comma separated list of ip addresses for Subject Alternative Name domain + names (default empty list) + mockserver.sslSubjectAlternativeNameDomains=www.example.com,www.another.com + # comma separated list of ip addresses for Subject Alternative Name ips + (default empty list) + mockserver.sslSubjectAlternativeNameIps=127.0.0.1 + # CORS + # enable CORS for MockServer REST API + mockserver.enableCORSForAPI=true + # enable CORS for all responses + mockserver.enableCORSForAllResponses=true + # Json Initialization + mockserver.initializationJsonPath=/config/initializerJson.json