From e08bed4b67ed524711c07ecfb21a3dd6f57177e1 Mon Sep 17 00:00:00 2001 From: "Kirill Sushkov (teeverr)" Date: Tue, 10 Oct 2023 10:18:49 +0200 Subject: [PATCH] fix tests and change condition for UPDATE_PROVISIONED_PRODUCT status --- .../provisionedproduct/setup.go | 19 +++++++++--------- .../provisionedproduct/setup_test.go | 20 ++++++++++--------- pkg/utils/metrics/setup.go | 4 +++- 3 files changed, 23 insertions(+), 20 deletions(-) diff --git a/pkg/controller/servicecatalog/provisionedproduct/setup.go b/pkg/controller/servicecatalog/provisionedproduct/setup.go index 319dfda2cd..ba33b2a3a9 100644 --- a/pkg/controller/servicecatalog/provisionedproduct/setup.go +++ b/pkg/controller/servicecatalog/provisionedproduct/setup.go @@ -21,14 +21,11 @@ import ( "fmt" "strings" - "github.com/prometheus/client_golang/prometheus" - cfsdkv2 "github.com/aws/aws-sdk-go-v2/service/cloudformation" cfsdkv2types "github.com/aws/aws-sdk-go-v2/service/cloudformation/types" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/awserr" svcsdk "github.com/aws/aws-sdk-go/service/servicecatalog" - "github.com/crossplane-contrib/provider-aws/pkg/utils/metrics" xpv1 "github.com/crossplane/crossplane-runtime/apis/common/v1" "github.com/crossplane/crossplane-runtime/pkg/connection" "github.com/crossplane/crossplane-runtime/pkg/controller" @@ -39,6 +36,7 @@ import ( cpresource "github.com/crossplane/crossplane-runtime/pkg/resource" "github.com/google/uuid" "github.com/pkg/errors" + "github.com/prometheus/client_golang/prometheus" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/utils/pointer" @@ -50,6 +48,7 @@ import ( awsclient "github.com/crossplane-contrib/provider-aws/pkg/clients" clientset "github.com/crossplane-contrib/provider-aws/pkg/clients/servicecatalog" "github.com/crossplane-contrib/provider-aws/pkg/features" + "github.com/crossplane-contrib/provider-aws/pkg/utils/metrics" ) const ( @@ -134,12 +133,12 @@ func (c *customConnector) Connect(ctx context.Context, mg cpresource.Managed) (m postUpdate: nopPostUpdate, } - customResourceApiVersion := fmt.Sprintf("%s/%s", cr.GetObjectKind().GroupVersionKind().Group, cr.GetObjectKind().GroupVersionKind().Version) - metrics.MetricManagedResRec.WithLabelValues(customResourceApiVersion, cr.GetObjectKind().GroupVersionKind().Kind, cr.Name).Inc() - cust.metrics.create = metrics.MetricAWSAPICallsRec.WithLabelValues(customResourceApiVersion, cr.GetObjectKind().GroupVersionKind().Kind, cr.Name, "create") - cust.metrics.observe = metrics.MetricAWSAPICallsRec.WithLabelValues(customResourceApiVersion, cr.GetObjectKind().GroupVersionKind().Kind, cr.Name, "observe") - cust.metrics.update = metrics.MetricAWSAPICallsRec.WithLabelValues(customResourceApiVersion, cr.GetObjectKind().GroupVersionKind().Kind, cr.Name, "update") - cust.metrics.delete = metrics.MetricAWSAPICallsRec.WithLabelValues(customResourceApiVersion, cr.GetObjectKind().GroupVersionKind().Kind, cr.Name, "delete") + customResourceAPIVersion := fmt.Sprintf("%s/%s", cr.GetObjectKind().GroupVersionKind().Group, cr.GetObjectKind().GroupVersionKind().Version) + metrics.MetricManagedResRec.WithLabelValues(customResourceAPIVersion, cr.GetObjectKind().GroupVersionKind().Kind, cr.Name).Inc() + cust.metrics.create = metrics.MetricAWSAPICallsRec.WithLabelValues(customResourceAPIVersion, cr.GetObjectKind().GroupVersionKind().Kind, cr.Name, "create") + cust.metrics.observe = metrics.MetricAWSAPICallsRec.WithLabelValues(customResourceAPIVersion, cr.GetObjectKind().GroupVersionKind().Kind, cr.Name, "observe") + cust.metrics.update = metrics.MetricAWSAPICallsRec.WithLabelValues(customResourceAPIVersion, cr.GetObjectKind().GroupVersionKind().Kind, cr.Name, "update") + cust.metrics.delete = metrics.MetricAWSAPICallsRec.WithLabelValues(customResourceAPIVersion, cr.GetObjectKind().GroupVersionKind().Kind, cr.Name, "delete") return cust.external, nil } @@ -318,7 +317,7 @@ func setConditions(describeRecordOutput *svcsdk.DescribeRecordOutput, resp *svcs case recordType == "PROVISION_PRODUCT": cr.SetConditions(xpv1.Creating()) case recordType == "UPDATE_PROVISIONED_PRODUCT": - cr.SetConditions(xpv1.Available().WithMessage(msgProvisionedProductStatusSdkUnderChange)) + cr.SetConditions(xpv1.Unavailable().WithMessage(msgProvisionedProductStatusSdkUnderChange)) case recordType == "TERMINATE_PROVISIONED_PRODUCT": cr.SetConditions(xpv1.Deleting()) } diff --git a/pkg/controller/servicecatalog/provisionedproduct/setup_test.go b/pkg/controller/servicecatalog/provisionedproduct/setup_test.go index 68c9b49a36..65fa8e6e87 100644 --- a/pkg/controller/servicecatalog/provisionedproduct/setup_test.go +++ b/pkg/controller/servicecatalog/provisionedproduct/setup_test.go @@ -21,6 +21,8 @@ import ( "testing" "time" + "github.com/crossplane-contrib/provider-aws/pkg/utils/metrics" + cfsdkv2types "github.com/aws/aws-sdk-go-v2/service/cloudformation/types" svcsdk "github.com/aws/aws-sdk-go/service/servicecatalog" svcsdkapi "github.com/aws/aws-sdk-go/service/servicecatalog/servicecatalogiface" @@ -87,14 +89,14 @@ func describeProvisionedProduct(m ...describeProvisionedProductOutputModifier) * return output } -func prepareFakeExternal(fakeClient clientset.Client, kube client.Client) func(*external) { +func setupFakeExternal(fakeClient clientset.Client, kube client.Client, metrics metricsRec) func(*external) { return func(e *external) { - c := &custom{client: fakeClient, kube: kube} + c := &custom{client: fakeClient, kube: kube, metrics: metrics} e.isUpToDate = c.isUpToDate e.lateInitialize = c.lateInitialize e.postObserve = c.postObserve - e.preCreate = preCreate - e.preDelete = preDelete + e.preCreate = c.preCreate + e.preDelete = c.preDelete e.preUpdate = c.preUpdate } } @@ -652,7 +654,7 @@ func TestIsUpToDate(t *testing.T) { } for name, tc := range cases { t.Run(name, func(t *testing.T) { - opts := []option{prepareFakeExternal(tc.args.customClient, tc.args.kube)} + opts := []option{setupFakeExternal(tc.args.customClient, tc.args.kube, metricsRec{observe: metrics.MetricAWSAPICallsRec.WithLabelValues("servicecatalog.aws.crossplane.io/v1alpha1", "ProvisionedProduct", "fake", "observe")})} e := newExternal(tc.args.kube, tc.args.client, opts) result, _, err := e.isUpToDate(context.TODO(), tc.args.provisionedProduct, tc.args.describeProvisionedProductOutput) if diff := cmp.Diff(err, tc.want.err, test.EquateErrors()); diff != "" { @@ -700,7 +702,7 @@ func TestLateInitialize(t *testing.T) { } for name, tc := range cases { t.Run(name, func(t *testing.T) { - opts := []option{prepareFakeExternal(tc.args.customClient, tc.args.kube)} + opts := []option{setupFakeExternal(tc.args.customClient, tc.args.kube, metricsRec{observe: metrics.MetricAWSAPICallsRec.WithLabelValues("servicecatalog.aws.crossplane.io/v1alpha1", "ProvisionedProduct", "fake", "observe")})} e := newExternal(tc.args.kube, tc.args.client, opts) _ = e.lateInitialize(&tc.args.provisionedProduct.Spec.ForProvider, tc.args.describeProvisionedProductOutput) if diff := cmp.Diff(tc.want.acceptLanguage, *tc.args.provisionedProduct.Spec.ForProvider.AcceptLanguage); diff != "" { @@ -769,7 +771,7 @@ func TestPostObserve(t *testing.T) { }, }, want: want{ - status: xpv1.Available().WithMessage(msgProvisionedProductStatusSdkUnderChange), + status: xpv1.Unavailable().WithMessage(msgProvisionedProductStatusSdkUnderChange), }, }, "StatusReconcileErrorProductError": { @@ -823,7 +825,7 @@ func TestPostObserve(t *testing.T) { } for name, tc := range cases { t.Run(name, func(t *testing.T) { - opts := []option{prepareFakeExternal(tc.args.customClient, tc.args.kube)} + opts := []option{setupFakeExternal(tc.args.customClient, tc.args.kube, metricsRec{observe: metrics.MetricAWSAPICallsRec.WithLabelValues("servicecatalog.aws.crossplane.io/v1alpha1", "ProvisionedProduct", "fake", "observe")})} e := newExternal(tc.args.kube, tc.args.client, opts) _, _ = e.postObserve(context.TODO(), tc.args.provisionedProduct, tc.args.describeProvisionedProductOutput, managed.ExternalObservation{}, nil) conditions := tc.args.provisionedProduct.Status.Conditions @@ -873,7 +875,7 @@ func TestPreDelete(t *testing.T) { } for name, tc := range cases { t.Run(name, func(t *testing.T) { - opts := []option{prepareFakeExternal(tc.args.customClient, tc.args.kube)} + opts := []option{setupFakeExternal(tc.args.customClient, tc.args.kube, metricsRec{delete: metrics.MetricAWSAPICallsRec.WithLabelValues("servicecatalog.aws.crossplane.io/v1alpha1", "ProvisionedProduct", "fake", "delete")})} e := newExternal(tc.args.kube, tc.args.client, opts) ignore, _ := e.preDelete(context.TODO(), tc.args.provisionedProduct, &svcsdk.TerminateProvisionedProductInput{}) if diff := cmp.Diff(tc.want.ignoreDeletion, ignore); diff != "" { diff --git a/pkg/utils/metrics/setup.go b/pkg/utils/metrics/setup.go index 7a4aa26daa..ebbdb9fc5d 100644 --- a/pkg/utils/metrics/setup.go +++ b/pkg/utils/metrics/setup.go @@ -10,10 +10,12 @@ var ( Name: "aws_api_calls_total", Help: "Number of API calls to the AWS API", }, []string{"service", "operation", "api_version"}) + // MetricAWSAPICallsRec can be used in controllers for exposing a total amount of AWS API calls for every create/observe/update/delete operation MetricAWSAPICallsRec = prometheus.NewCounterVec(prometheus.CounterOpts{ - Name: "aws_api_calls_reconcile_managed_resource", + Name: "aws_api_calls_reconcile_managed_resource_total", Help: "Amount of calls to the AWS API produced by controller per reconciliation for every managed resource and controller operation type", }, []string{"api_version", "kind", "resource_name", "controller_operation_type"}) + // MetricManagedResRec can be used in controllers for exposing a total amount of reconciliations for every managed resource MetricManagedResRec = prometheus.NewCounterVec(prometheus.CounterOpts{ Name: "managed_resource_reconciles_total", Help: "Total amount of reconciliation loops per managed resource",