diff --git a/docs/content/en/docs/crd-ref/metrics/v1alpha3/_index.md b/docs/content/en/docs/crd-ref/metrics/v1alpha3/_index.md index 5d49a26c84..1732420d81 100644 --- a/docs/content/en/docs/crd-ref/metrics/v1alpha3/_index.md +++ b/docs/content/en/docs/crd-ref/metrics/v1alpha3/_index.md @@ -165,5 +165,6 @@ _Appears in:_ | --- | --- | | `interval` _string_ | Interval specifies the duration of the time interval for the data query | | `step` _string_ | Step represents the query resolution step width for the data query | +| `aggregation` _string_ | Aggregation defines as the type of aggregation function to be applied on the data. Accepted values: p90, p95, p99, max, min, avg, median | diff --git a/metrics-operator/api/v1alpha3/keptnmetric_types.go b/metrics-operator/api/v1alpha3/keptnmetric_types.go index 85f1b3628c..823773a6f9 100644 --- a/metrics-operator/api/v1alpha3/keptnmetric_types.go +++ b/metrics-operator/api/v1alpha3/keptnmetric_types.go @@ -60,6 +60,9 @@ type RangeSpec struct { Interval string `json:"interval,omitempty"` // Step represents the query resolution step width for the data query Step string `json:"step,omitempty"` + // Aggregation defines as the type of aggregation function to be applied on the data. Accepted values: p90, p95, p99, max, min, avg, median + // +kubebuilder:validation:Enum:=p90;p95;p99;max;min;avg;median + Aggregation string `json:"aggregation,omitempty"` } // +kubebuilder:object:root=true diff --git a/metrics-operator/api/v1alpha3/keptnmetric_webhook.go b/metrics-operator/api/v1alpha3/keptnmetric_webhook.go index c3d3baa174..e4c3a2db0b 100644 --- a/metrics-operator/api/v1alpha3/keptnmetric_webhook.go +++ b/metrics-operator/api/v1alpha3/keptnmetric_webhook.go @@ -72,6 +72,9 @@ func (s *KeptnMetric) validateKeptnMetric() error { if err = s.validateRangeStep(); err != nil { allErrs = append(allErrs, err) } + if err = s.validateAggregation(); err != nil { + allErrs = append(allErrs, err) + } if len(allErrs) == 0 { return nil } @@ -110,3 +113,22 @@ func (s *KeptnMetric) validateRangeStep() *field.Error { } return nil } + +func (s *KeptnMetric) validateAggregation() *field.Error { + if s.Spec.Range == nil { + return nil + } + if s.Spec.Range.Step != "" && s.Spec.Range.Aggregation == "" { + return field.Required( + field.NewPath("spec").Child("range").Child("aggregation"), + errors.New("Forbidden! Aggregation field is required if defining the step field").Error(), + ) + } + if s.Spec.Range.Step == "" && s.Spec.Range.Aggregation != "" { + return field.Required( + field.NewPath("spec").Child("range").Child("step"), + errors.New("Forbidden! Step interval is required for the aggregation to work").Error(), + ) + } + return nil +} diff --git a/metrics-operator/api/v1alpha3/keptnmetric_webhook_test.go b/metrics-operator/api/v1alpha3/keptnmetric_webhook_test.go index 9866c28109..b89e66db29 100644 --- a/metrics-operator/api/v1alpha3/keptnmetric_webhook_test.go +++ b/metrics-operator/api/v1alpha3/keptnmetric_webhook_test.go @@ -152,17 +152,29 @@ func TestKeptnMetric_validateRangeStep(t *testing.T) { oldSpec runtime.Object }{ { - name: "create-with-wrong-step", + name: "create-with-right-step-right-aggregation", verb: "create", Spec: KeptnMetricSpec{ Range: &RangeSpec{ - Interval: "5m", - Step: "1mins", + Interval: "5m", + Step: "1m", + Aggregation: "p90", + }, + }, + }, + { + name: "create-with-wrong-step-right-aggregation", + verb: "create", + Spec: KeptnMetricSpec{ + Range: &RangeSpec{ + Interval: "5m", + Step: "1mins", + Aggregation: "p90", }, }, want: apierrors.NewInvalid( schema.GroupKind{Group: "metrics.keptn.sh", Kind: "KeptnMetric"}, - "create-with-wrong-step", + "create-with-wrong-step-right-aggregation", field.ErrorList{ field.Invalid( field.NewPath("spec").Child("range").Child("step"), @@ -173,64 +185,156 @@ func TestKeptnMetric_validateRangeStep(t *testing.T) { ), }, { - name: "create-with-empty-step", + name: "create-with-right-step-wrong-aggregation", verb: "create", Spec: KeptnMetricSpec{ Range: &RangeSpec{ - Interval: "5m", - Step: "", + Interval: "5m", + Step: "1m", + Aggregation: "p91", }, }, }, { - name: "create-with-right-step", + name: "create-with-wrong-step-wrong-aggregation", + verb: "create", + Spec: KeptnMetricSpec{ + Range: &RangeSpec{ + Interval: "5m", + Step: "1mins", + Aggregation: "p91", + }, + }, + want: apierrors.NewInvalid( + schema.GroupKind{Group: "metrics.keptn.sh", Kind: "KeptnMetric"}, + "create-with-wrong-step-wrong-aggregation", + field.ErrorList{ + field.Invalid( + field.NewPath("spec").Child("range").Child("step"), + "1mins", + "Forbidden! The time interval cannot be parsed. Please check for suitable conventions", + ), + }, + ), + }, + { + name: "create-with-empty-step-empty-aggregation", verb: "create", Spec: KeptnMetricSpec{ Range: &RangeSpec{ - Interval: "5m", - Step: "1m", + Interval: "5m", + Step: "", + Aggregation: "", }, }, }, { - name: "create-with-wrong-interval-right-step", - verb: "update", + name: "create-with-step-empty-aggregation", + verb: "create", Spec: KeptnMetricSpec{ Range: &RangeSpec{ - Interval: "5mins", - Step: "1m", + Interval: "5m", + Step: "1m", + Aggregation: "", }, }, want: apierrors.NewInvalid( schema.GroupKind{Group: "metrics.keptn.sh", Kind: "KeptnMetric"}, - "create-with-wrong-interval-right-step", + "create-with-step-empty-aggregation", field.ErrorList{ - field.Invalid( - field.NewPath("spec").Child("range").Child("interval"), - "5mins", - "Forbidden! The time interval cannot be parsed. Please check for suitable conventions", + field.Required( + field.NewPath("spec").Child("range").Child("aggregation"), + "Forbidden! Aggregation field is required if defining the step field", ), }, ), }, { - name: "create-with-wrong-interval-wrong-step", + name: "create-empty-step-with-aggregation", + verb: "create", + Spec: KeptnMetricSpec{ + Range: &RangeSpec{ + Interval: "5m", + Step: "", + Aggregation: "p90", + }, + }, + want: apierrors.NewInvalid( + schema.GroupKind{Group: "metrics.keptn.sh", Kind: "KeptnMetric"}, + "create-empty-step-with-aggregation", + field.ErrorList{ + field.Required( + field.NewPath("spec").Child("range").Child("step"), + "Forbidden! Step interval is required for the aggregation to work", + ), + }, + ), + }, + { + name: "update-with-right-step-right-aggregation", + verb: "update", + Spec: KeptnMetricSpec{ + Range: &RangeSpec{ + Interval: "5m", + Step: "1m", + Aggregation: "p90", + }, + }, + oldSpec: &KeptnMetric{ + Spec: KeptnMetricSpec{ + Range: &RangeSpec{ + Interval: "5m", + Step: "1mins", + Aggregation: "p91", + }, + }, + }, + }, + { + name: "update-with-wrong-step-wrong-aggregation", verb: "update", Spec: KeptnMetricSpec{ Range: &RangeSpec{ - Interval: "5mins", - Step: "1mins", + Interval: "5m", + Step: "1mins", + Aggregation: "p91", }, }, want: apierrors.NewInvalid( schema.GroupKind{Group: "metrics.keptn.sh", Kind: "KeptnMetric"}, - "create-with-wrong-interval-wrong-step", + "update-with-wrong-step-wrong-aggregation", field.ErrorList{ field.Invalid( - field.NewPath("spec").Child("range").Child("interval"), - "5mins", + field.NewPath("spec").Child("range").Child("step"), + "1mins", "Forbidden! The time interval cannot be parsed. Please check for suitable conventions", ), + }, + ), + oldSpec: &KeptnMetric{ + Spec: KeptnMetricSpec{ + Range: &RangeSpec{ + Interval: "5m", + Step: "1m", + Aggregation: "p90", + }, + }, + }, + }, + { + name: "update-with-wrong-step-right-aggregation", + verb: "update", + Spec: KeptnMetricSpec{ + Range: &RangeSpec{ + Interval: "5m", + Step: "1mins", + Aggregation: "p90", + }, + }, + want: apierrors.NewInvalid( + schema.GroupKind{Group: "metrics.keptn.sh", Kind: "KeptnMetric"}, + "update-with-wrong-step-right-aggregation", + field.ErrorList{ field.Invalid( field.NewPath("spec").Child("range").Child("step"), "1mins", @@ -238,50 +342,112 @@ func TestKeptnMetric_validateRangeStep(t *testing.T) { ), }, ), + oldSpec: &KeptnMetric{ + Spec: KeptnMetricSpec{ + Range: &RangeSpec{ + Interval: "5m", + Step: "1m", + Aggregation: "p90", + }, + }, + }, + }, + { + name: "update-with-right-step-wrong-aggregation", + verb: "update", + Spec: KeptnMetricSpec{ + Range: &RangeSpec{ + Interval: "5m", + Step: "1m", + Aggregation: "p91", + }, + }, + oldSpec: &KeptnMetric{ + Spec: KeptnMetricSpec{ + Range: &RangeSpec{ + Interval: "5m", + Step: "1m", + Aggregation: "p90", + }, + }, + }, }, { - name: "update-with-right-step", + name: "update-with-empty-step-empty-aggregation", verb: "update", Spec: KeptnMetricSpec{ Range: &RangeSpec{ - Interval: "5m", - Step: "1m", + Interval: "5m", + Step: "", + Aggregation: "", }, }, oldSpec: &KeptnMetric{ Spec: KeptnMetricSpec{ Range: &RangeSpec{ - Interval: "5m", - Step: "1mins", + Interval: "5m", + Step: "1m", + Aggregation: "p90", }, }, }, }, { - name: "update-with-wrong-step", + name: "update-with-empty-step-with-aggregation", verb: "update", Spec: KeptnMetricSpec{ Range: &RangeSpec{ - Interval: "5m", - Step: "1mins", + Interval: "5m", + Step: "", + Aggregation: "p90", }, }, want: apierrors.NewInvalid( schema.GroupKind{Group: "metrics.keptn.sh", Kind: "KeptnMetric"}, - "update-with-wrong-step", + "update-with-empty-step-with-aggregation", field.ErrorList{ - field.Invalid( + field.Required( field.NewPath("spec").Child("range").Child("step"), - "1mins", - "Forbidden! The time interval cannot be parsed. Please check for suitable conventions", + "Forbidden! Step interval is required for the aggregation to work", + ), + }, + ), + oldSpec: &KeptnMetric{ + Spec: KeptnMetricSpec{ + Range: &RangeSpec{ + Interval: "5m", + Step: "1m", + Aggregation: "p90", + }, + }, + }, + }, + { + name: "update-with-step-empty-aggregation", + verb: "update", + Spec: KeptnMetricSpec{ + Range: &RangeSpec{ + Interval: "5m", + Step: "1m", + Aggregation: "", + }, + }, + want: apierrors.NewInvalid( + schema.GroupKind{Group: "metrics.keptn.sh", Kind: "KeptnMetric"}, + "update-with-step-empty-aggregation", + field.ErrorList{ + field.Required( + field.NewPath("spec").Child("range").Child("aggregation"), + "Forbidden! Aggregation field is required if defining the step field", ), }, ), oldSpec: &KeptnMetric{ Spec: KeptnMetricSpec{ Range: &RangeSpec{ - Interval: "5m", - Step: "1m", + Interval: "5m", + Step: "1m", + Aggregation: "p90", }, }, }, @@ -302,7 +468,7 @@ func TestKeptnMetric_validateRangeStep(t *testing.T) { } else { s = &KeptnMetric{ ObjectMeta: metav1.ObjectMeta{Name: tt.name}, - Spec: KeptnMetricSpec{Range: &RangeSpec{Interval: tt.Spec.Range.Interval, Step: tt.Spec.Range.Step}}, + Spec: KeptnMetricSpec{Range: &RangeSpec{Interval: tt.Spec.Range.Interval, Step: tt.Spec.Range.Step, Aggregation: tt.Spec.Range.Aggregation}}, } } var err error diff --git a/metrics-operator/config/crd/bases/metrics.keptn.sh_keptnmetrics.yaml b/metrics-operator/config/crd/bases/metrics.keptn.sh_keptnmetrics.yaml index 8b8586c32a..cca55c3374 100644 --- a/metrics-operator/config/crd/bases/metrics.keptn.sh_keptnmetrics.yaml +++ b/metrics-operator/config/crd/bases/metrics.keptn.sh_keptnmetrics.yaml @@ -219,6 +219,19 @@ spec: description: Range represents the time range for which data is to be queried properties: + aggregation: + description: 'Aggregation defines as the type of aggregation function + to be applied on the data. Accepted values: p90, p95, p99, max, + min, avg, median' + enum: + - p90 + - p95 + - p99 + - max + - min + - avg + - median + type: string interval: default: 5m description: Interval specifies the duration of the time interval diff --git a/test/integration/metrics/01-teststep-install.yaml b/test/integration/metrics/01-teststep-install.yaml index df18fd2531..387c432e30 100644 --- a/test/integration/metrics/01-teststep-install.yaml +++ b/test/integration/metrics/01-teststep-install.yaml @@ -7,5 +7,5 @@ apply: - goodmetric4.yaml - goodmetric5.yaml commands: - - command: kubectl apply -f badmetric1.yaml badmetric2.yaml + - command: kubectl apply -f badmetric1.yaml badmetric2.yaml badmetric3.yaml ignoreFailure: true # we must install ignoring the validating webhook error to proceed with the test diff --git a/test/integration/metrics/02-teststep-assert.yaml b/test/integration/metrics/02-teststep-assert.yaml index 800b17487b..e6f95ad931 100644 --- a/test/integration/metrics/02-teststep-assert.yaml +++ b/test/integration/metrics/02-teststep-assert.yaml @@ -3,6 +3,7 @@ kind: TestStep error: # this checks that kubectl get resource fails, AKA bad CRD not added - badmetric1.yaml - badmetric2.yaml + - badmetric3.yaml assert: # this checks that kubectl get resource succeeds - goodmetric1.yaml - goodmetric2.yaml diff --git a/test/integration/metrics/badmetric3.yaml b/test/integration/metrics/badmetric3.yaml new file mode 100644 index 0000000000..b93558a173 --- /dev/null +++ b/test/integration/metrics/badmetric3.yaml @@ -0,0 +1,13 @@ +apiVersion: metrics.keptn.sh/v1alpha3 +kind: KeptnMetric +metadata: + name: podtato-head1 +spec: + provider: + name: "my-provider" + query: "sum(kube_pod_container_resource_limits{resource='cpu'})" + fetchIntervalSeconds: 5 + range: + interval: "5m" + step: "1m" + aggreagation: "p91" diff --git a/test/integration/metrics/goodmetric5.yaml b/test/integration/metrics/goodmetric5.yaml index c75927e64c..554a7121a6 100644 --- a/test/integration/metrics/goodmetric5.yaml +++ b/test/integration/metrics/goodmetric5.yaml @@ -10,3 +10,4 @@ spec: range: interval: "5m" step: "1m" + aggregation: "p90"