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

feat(metrics-operator): update controller logic to support multiple metric values #2190

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
e7ef0c3
feat: update controller logic to support multiple metric values
rakshitgondwal Sep 27, 2023
458fbb2
Merge branch 'main' into feat/controller-update-for-multiple-metrics
rakshitgondwal Oct 6, 2023
f1a3204
modify logic
rakshitgondwal Oct 6, 2023
e318b9f
remove storedResults as a pointer
rakshitgondwal Oct 6, 2023
5c8fbcf
fix after review
rakshitgondwal Nov 1, 2023
89455c1
Merge branch 'main' into feat/controller-update-for-multiple-metrics
rakshitgondwal Nov 1, 2023
a92b566
Merge branch 'main' into feat/controller-update-for-multiple-metrics
rakshitgondwal Jan 7, 2024
49b5088
add range check
rakshitgondwal Jan 7, 2024
e474a02
fix range check
rakshitgondwal Jan 8, 2024
f75ebab
add and fix tests
rakshitgondwal Jan 10, 2024
0d9dbe8
add more tests
rakshitgondwal Jan 10, 2024
a3efcf4
fix errors
rakshitgondwal Jan 10, 2024
89538b8
add integration test
rakshitgondwal Jan 10, 2024
d67210a
fix yaml lint err
rakshitgondwal Jan 10, 2024
593680b
Merge branch 'main' into feat/controller-update-for-multiple-metrics
rakshitgondwal Jan 10, 2024
73cad1f
fix test
rakshitgondwal Jan 10, 2024
b03a80a
fix after review
rakshitgondwal Jan 11, 2024
0621045
Merge branch 'main' into feat/controller-update-for-multiple-metrics
rakshitgondwal Jan 12, 2024
7e3e753
Merge branch 'main' into feat/controller-update-for-multiple-metrics
rakshitgondwal Jan 16, 2024
6ae2f6f
fix lint err
rakshitgondwal Jan 16, 2024
9a5c33b
fix after review
rakshitgondwal Jan 17, 2024
5ad411e
Merge branch 'main' into feat/controller-update-for-multiple-metrics
bacherfl Jan 18, 2024
291b111
Merge branch 'main' into feat/controller-update-for-multiple-metrics
bacherfl Jan 18, 2024
20ceab9
adapt unit tests
rakshitgondwal Jan 19, 2024
7ceb7a8
Merge branch 'main' into feat/controller-update-for-multiple-metrics
rakshitgondwal Jan 19, 2024
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
50 changes: 37 additions & 13 deletions metrics-operator/controllers/metrics/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,19 +104,8 @@ func (r *KeptnMetricReconciler) Reconcile(ctx context.Context, req ctrl.Request)
reconcile := ctrl.Result{Requeue: true, RequeueAfter: 10 * time.Second}

value, rawValue, err := r.getResults(ctx, metric, provider, metricProvider)
if err != nil {
r.Log.Error(err, "Failed to evaluate the query", "Response from provider was:", (string)(rawValue))
metric.Status.ErrMsg = err.Error()
metric.Status.Value = ""
metric.Status.RawValue = cupSize(rawValue)
metric.Status.LastUpdated = metav1.Time{Time: time.Now()}
reconcile = ctrl.Result{Requeue: false}
} else {
metric.Status.ErrMsg = ""
metric.Status.Value = value
metric.Status.RawValue = cupSize(rawValue)
metric.Status.LastUpdated = metav1.Time{Time: time.Now()}
}

reconcile = r.updateMetric(metric, value, rawValue, reconcile, err)

if err := r.Client.Status().Update(ctx, metric); err != nil {
r.Log.Error(err, "Failed to update the Metric status", "requestInfo", requestInfo)
Expand All @@ -126,6 +115,41 @@ func (r *KeptnMetricReconciler) Reconcile(ctx context.Context, req ctrl.Request)
return reconcile, err
}

func (r *KeptnMetricReconciler) updateMetric(metric *metricsapi.KeptnMetric, value string, rawValue []byte, reconcile ctrl.Result, err error) ctrl.Result {
if metric.Spec.Range != nil && metric.Spec.Range.StoredResults > 0 {
intervalResult := metricsapi.IntervalResult{
LastUpdated: metav1.Time{Time: time.Now().UTC()},
ErrMsg: "",
}

if err != nil {
r.Log.Error(err, "Failed to evaluate the query", "Response from provider was:", (string)(rawValue))
intervalResult.ErrMsg = err.Error()
} else {
intervalResult.Value = value
intervalResult.Range = metric.Spec.Range
}

if len(metric.Status.IntervalResults) >= int(metric.Spec.Range.StoredResults) {
metric.Status.IntervalResults = append(metric.Status.IntervalResults[1:], intervalResult)
} else {
metric.Status.IntervalResults = append(metric.Status.IntervalResults, intervalResult)
}
} else {
if err != nil {
r.Log.Error(err, "Failed to evaluate the query", "Response from provider was:", (string)(rawValue))
metric.Status.ErrMsg = err.Error()
} else {
metric.Status.ErrMsg = ""
metric.Status.Value = value
rakshitgondwal marked this conversation as resolved.
Show resolved Hide resolved
metric.Status.RawValue = cupSize(rawValue)
}
}
metric.Status.LastUpdated = metav1.Time{Time: time.Now().UTC()}

return reconcile
}

func (r *KeptnMetricReconciler) getResults(ctx context.Context, metric *metricsapi.KeptnMetric, provider providers.KeptnSLIProvider, metricProvider *metricsapi.KeptnMetricsProvider) (string, []byte, error) {
if metric.Spec.Range != nil && metric.Spec.Range.Step != "" {
return r.getStepQueryResults(ctx, metric, provider, metricProvider)
Expand Down
160 changes: 133 additions & 27 deletions metrics-operator/controllers/metrics/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,25 @@ func TestKeptnMetricReconciler_Reconcile(t *testing.T) {
},
}

metric5 := &metricsapi.KeptnMetric{
ObjectMeta: metav1.ObjectMeta{
Name: "mymetric5",
Namespace: "default",
},
Spec: metricsapi.KeptnMetricSpec{
Provider: metricsapi.ProviderRef{
Name: "provider-name",
},
Range: &metricsapi.RangeSpec{
Aggregation: "max",
Step: "step",
StoredResults: 2,
},
Query: "",
FetchIntervalSeconds: 10,
},
}

unsupportedProvider := &metricsapi.KeptnMetricsProvider{
ObjectMeta: metav1.ObjectMeta{
Name: "myprov",
Expand All @@ -143,25 +162,27 @@ func TestKeptnMetricReconciler_Reconcile(t *testing.T) {
}

tests := []struct {
name string
client k8sclient.Client
ctx context.Context
req controllerruntime.Request
want controllerruntime.Result
wantMetric *metricsapi.KeptnMetric
providerFactory providers.ProviderFactory
wantErr error
name string
client k8sclient.Client
ctx context.Context
req controllerruntime.Request
want controllerruntime.Result
wantMetric *metricsapi.KeptnMetric
providerFactory providers.ProviderFactory
wantErr error
hasStoredResults bool
}{
{
name: "metric not found, ignoring",
ctx: context.TODO(),
req: controllerruntime.Request{
NamespacedName: types.NamespacedName{Namespace: "default", Name: "myunexistingmetric"},
},
want: controllerruntime.Result{},
providerFactory: nil,
client: fake.NewClient(),
wantMetric: nil,
want: controllerruntime.Result{},
providerFactory: nil,
client: fake.NewClient(),
wantMetric: nil,
hasStoredResults: false,
},

{
Expand All @@ -170,10 +191,11 @@ func TestKeptnMetricReconciler_Reconcile(t *testing.T) {
req: controllerruntime.Request{
NamespacedName: types.NamespacedName{Namespace: "default", Name: "mymetric"},
},
want: controllerruntime.Result{Requeue: true, RequeueAfter: 10 * time.Second},
providerFactory: nil,
client: fake.NewClient(metric),
wantMetric: nil,
want: controllerruntime.Result{Requeue: true, RequeueAfter: 10 * time.Second},
providerFactory: nil,
client: fake.NewClient(metric),
wantMetric: nil,
hasStoredResults: false,
},

{
Expand All @@ -182,10 +204,11 @@ func TestKeptnMetricReconciler_Reconcile(t *testing.T) {
req: controllerruntime.Request{
NamespacedName: types.NamespacedName{Namespace: "default", Name: "mymetric"},
},
providerFactory: nil,
client: fake.NewClient(metric),
want: controllerruntime.Result{Requeue: true, RequeueAfter: 10 * time.Second},
wantMetric: nil,
providerFactory: nil,
client: fake.NewClient(metric),
want: controllerruntime.Result{Requeue: true, RequeueAfter: 10 * time.Second},
wantMetric: nil,
hasStoredResults: false,
},

{
Expand All @@ -197,10 +220,11 @@ func TestKeptnMetricReconciler_Reconcile(t *testing.T) {
providerFactory: func(providerType string, log logr.Logger, k8sClient k8sclient.Client) (providers.KeptnSLIProvider, error) {
return nil, fmt.Errorf("provider unsupported-type not supported")
},
client: fake.NewClient(metric2, unsupportedProvider),
want: controllerruntime.Result{Requeue: false, RequeueAfter: 0},
wantErr: fmt.Errorf("provider unsupported-type not supported"),
wantMetric: nil,
client: fake.NewClient(metric2, unsupportedProvider),
want: controllerruntime.Result{Requeue: false, RequeueAfter: 0},
wantErr: fmt.Errorf("provider unsupported-type not supported"),
wantMetric: nil,
hasStoredResults: false,
},
{
name: "metric exists, needs to fetch, prometheus supported, bad query - EvaluateQuery",
Expand All @@ -217,7 +241,7 @@ func TestKeptnMetricReconciler_Reconcile(t *testing.T) {
return mymock, nil
},
client: fake.NewClient(metric3, supportedProvider),
want: controllerruntime.Result{Requeue: false, RequeueAfter: 0},
want: controllerruntime.Result{Requeue: true, RequeueAfter: 10 * time.Second},
wantErr: fmt.Errorf("client_error: client error: 404"),
wantMetric: &metricsapi.KeptnMetric{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -230,6 +254,7 @@ func TestKeptnMetricReconciler_Reconcile(t *testing.T) {
RawValue: []byte(nil),
},
},
hasStoredResults: false,
},
{
name: "happy path, remove error message - EvaluateQuery",
Expand Down Expand Up @@ -259,6 +284,7 @@ func TestKeptnMetricReconciler_Reconcile(t *testing.T) {
RawValue: []byte("result"),
},
},
hasStoredResults: false,
},
{
name: "metric exists, needs to fetch, prometheus supported, bad query - EvaluateQueryForStep",
Expand All @@ -275,7 +301,7 @@ func TestKeptnMetricReconciler_Reconcile(t *testing.T) {
return mymock, nil
},
client: fake.NewClient(metric4, supportedProvider),
want: controllerruntime.Result{Requeue: false, RequeueAfter: 0},
want: controllerruntime.Result{Requeue: true, RequeueAfter: 10 * time.Second},
wantErr: fmt.Errorf("client_error: client error: 404"),
wantMetric: &metricsapi.KeptnMetric{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -288,7 +314,9 @@ func TestKeptnMetricReconciler_Reconcile(t *testing.T) {
RawValue: []byte(nil),
},
},
hasStoredResults: false,
},

{
name: "happy path - EvaluateQueryForStep",
ctx: context.TODO(),
Expand Down Expand Up @@ -317,6 +345,77 @@ func TestKeptnMetricReconciler_Reconcile(t *testing.T) {
RawValue: []byte("11"),
},
},
hasStoredResults: false,
},
{
name: "metric exists, needs to fetch, prometheus supported, bad query, with stored results - EvaluateQueryForStep",
ctx: context.TODO(),
req: controllerruntime.Request{
NamespacedName: types.NamespacedName{Namespace: "default", Name: "mymetric5"},
},
providerFactory: func(providerType string, log logr.Logger, k8sClient k8sclient.Client) (providers.KeptnSLIProvider, error) {
mymock := &providersfake.KeptnSLIProviderMock{
EvaluateQueryForStepFunc: func(ctx context.Context, metric metricsapi.KeptnMetric, provider metricsapi.KeptnMetricsProvider) ([]string, []byte, error) {
return []string{}, nil, fmt.Errorf("client_error: client error: 404")
},
}
return mymock, nil
},
client: fake.NewClient(metric5, supportedProvider),
want: controllerruntime.Result{Requeue: true, RequeueAfter: 10 * time.Second},
wantErr: fmt.Errorf("client_error: client error: 404"),
wantMetric: &metricsapi.KeptnMetric{
ObjectMeta: metav1.ObjectMeta{
Name: "mymetric5",
Namespace: "default",
},
Status: metricsapi.KeptnMetricStatus{
IntervalResults: []metricsapi.IntervalResult{
{
ErrMsg: "client_error: client error: 404",
},
},
},
},
hasStoredResults: true,
},
{
name: "happy path, with stored results- EvaluateQueryForStep",
ctx: context.TODO(),
req: controllerruntime.Request{
NamespacedName: types.NamespacedName{Namespace: "default", Name: "mymetric5"},
},
providerFactory: func(providerType string, log logr.Logger, k8sClient k8sclient.Client) (providers.KeptnSLIProvider, error) {
mymock := &providersfake.KeptnSLIProviderMock{
EvaluateQueryForStepFunc: func(ctx context.Context, metric metricsapi.KeptnMetric, provider metricsapi.KeptnMetricsProvider) ([]string, []byte, error) {
return []string{"11"}, []byte("11"), nil
},
}
return mymock, nil
},
client: fake.NewClient(metric5, supportedProvider),
want: controllerruntime.Result{Requeue: true, RequeueAfter: 10 * time.Second},
wantErr: nil,
wantMetric: &metricsapi.KeptnMetric{
ObjectMeta: metav1.ObjectMeta{
Name: "mymetric5",
Namespace: "default",
},
Status: metricsapi.KeptnMetricStatus{
IntervalResults: []metricsapi.IntervalResult{
{
Value: "11",
Range: &metricsapi.RangeSpec{
Aggregation: "max",
Step: "step",
StoredResults: 2,
},
ErrMsg: "",
},
},
},
},
hasStoredResults: true,
},
}
for _, tt := range tests {
Expand All @@ -339,7 +438,14 @@ func TestKeptnMetricReconciler_Reconcile(t *testing.T) {
t.Errorf("Reconcile() got = %v, want %v", got, tt.want)
}

if tt.wantMetric != nil {
if tt.hasStoredResults != false && tt.wantMetric != nil {
metric := &metricsapi.KeptnMetric{}
err := tt.client.Get(context.TODO(), types.NamespacedName{Namespace: tt.wantMetric.Namespace, Name: tt.wantMetric.Name}, metric)
require.Nil(t, err)
require.Equal(t, tt.wantMetric.Status.IntervalResults[0].Value, metric.Status.IntervalResults[0].Value)
require.Equal(t, tt.wantMetric.Status.IntervalResults[0].Range, metric.Status.IntervalResults[0].Range)
}
if tt.hasStoredResults == false && tt.wantMetric != nil {
metric := &metricsapi.KeptnMetric{}
err := tt.client.Get(context.TODO(), types.NamespacedName{Namespace: tt.wantMetric.Namespace, Name: tt.wantMetric.Name}, metric)
require.Nil(t, err)
Expand Down
1 change: 1 addition & 0 deletions test/testmetrics/metrics/01-teststep-install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ apply:
- goodmetric3.yaml
- goodmetric4.yaml
- goodmetric5.yaml
- goodmetric6.yaml
commands:
- 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
1 change: 1 addition & 0 deletions test/testmetrics/metrics/02-teststep-assert.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ assert: # this checks that kubectl get resource succeeds
- goodmetric3.yaml
- goodmetric4.yaml
- goodmetric5.yaml
- goodmetric6.yaml
collectors:
- type: pod
selector: app=test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ commands:
- script: ./retrieve-metrics.sh "podtato-head3"
- script: ./retrieve-metrics.sh "podtato-head4"
- script: ./retrieve-metrics.sh "podtato-head5"
- script: ./retrieve-metrics.sh "podtato-head6"
14 changes: 14 additions & 0 deletions test/testmetrics/metrics/goodmetric6.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
apiVersion: metrics.keptn.sh/v1beta1
kind: KeptnMetric
metadata:
name: podtato-head6
spec:
provider:
name: "my-provider2"
query: "sum(kube_pod_container_resource_limits{resource='cpu'})"
fetchIntervalSeconds: 5
range:
interval: "5m"
step: "1m"
aggregation: "p90"
storedResults: 5
Loading