fix: update custom metrics for mysql

cloudfoundry · Nov 25, 2024 · 7fa2187 · 7fa2187
1 parent 60a141f
commit 7fa2187
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 16 deletions.
diff --git a/Makefile b/Makefile
@@ -218,7 +218,7 @@ stop-db: check-db_type
 	@docker rm -f ${db_type} &> /dev/null || echo " - we could not stop and remove docker named '${db_type}'"
 
 .PHONY: integration
-integration: build init-db test-certs ## Run all integration tests
+integration: generate-openapi-generated-clients-and-servers build init-db test-certs ## Run all integration tests
 	@echo " - using DBURL=${DBURL}"
 	@make --directory='./src/autoscaler' integration DBURL="${DBURL}"
 

diff --git a/src/acceptance/api/api_test.go b/src/acceptance/api/api_test.go
@@ -300,7 +300,7 @@ var _ = Describe("AutoScaler Public API", func() {
 				Expect(status).To(Equal(http.StatusForbidden))
 			})
 		})
-		//FIXME
+
 		Context("and a custom metrics strategy is already set", func() {
 			var status int
 			var expectedPolicy string
@@ -326,7 +326,7 @@ var _ = Describe("AutoScaler Public API", func() {
 				Expect(status).To(Equal(400))
 			})
 			It("should succeed to update a valid custom metrics strategy", func() {
-				
+
 				Expect(string(actualPolicy)).Should(MatchJSON(expectedPolicy))
 				Expect(status).To(Equal(200))
 			})

diff --git a/src/autoscaler/db/db.go b/src/autoscaler/db/db.go
@@ -78,7 +78,6 @@ type BindingDB interface {
 	CountServiceInstancesInOrg(orgId string) (int, error)
 	GetServiceBinding(ctx context.Context, serviceBindingId string) (*models.ServiceBinding, error)
 	GetBindingIdsByInstanceId(ctx context.Context, instanceId string) ([]string, error)
-	GetAppBindingByAppId(ctx context.Context, appId string) (string, error)
 	IsAppBoundToSameAutoscaler(ctx context.Context, appId string, appToScaleId string) (bool, error)
 	GetCustomMetricStrategyByAppId(ctx context.Context, appId string) (string, error)
 	SetOrUpdateCustomMetricStrategy(ctx context.Context, appId string, customMetricsStrategy string, actionName string) error

diff --git a/src/autoscaler/db/sqldb/binding_sqldb.go b/src/autoscaler/db/sqldb/binding_sqldb.go
@@ -308,19 +308,23 @@ func (bdb *BindingSQLDB) DeleteServiceBindingByAppId(ctx context.Context, appId
 	return nil
 }
 
-func (bdb *BindingSQLDB) GetAppBindingByAppId(ctx context.Context, appId string) (string, error) {
-	var bindingId string
-	query := bdb.sqldb.Rebind("SELECT binding_id FROM binding WHERE app_id =?")
-	err := bdb.sqldb.QueryRowContext(ctx, query, appId).Scan(&bindingId)
-
+func (bdb *BindingSQLDB) getServiceBindingByAppId(ctx context.Context, appId string) (*models.ServiceBinding, error) {
+	dbServiceBinding := &dbServiceBinding{}
+	query := bdb.sqldb.Rebind("SELECT  binding_id, service_instance_id, app_id, custom_metrics_strategy FROM binding WHERE app_id =?")
+	err := bdb.sqldb.GetContext(ctx, dbServiceBinding, query, appId)
 	if err != nil {
 		bdb.logger.Debug("get-service-binding-by-appid", lager.Data{"query": query, "appId": appId, "error": err})
 		if errors.Is(err, sql.ErrNoRows) {
-			return "", db.ErrDoesNotExist
+			return nil, db.ErrDoesNotExist
 		}
-		return "", err
+		return nil, err
 	}
-	return bindingId, nil
+	return &models.ServiceBinding{
+		ServiceBindingID:      dbServiceBinding.ServiceBindingID,
+		ServiceInstanceID:     dbServiceBinding.ServiceInstanceID,
+		AppID:                 dbServiceBinding.AppID,
+		CustomMetricsStrategy: dbServiceBinding.CustomMetricsStrategy.String,
+	}, nil
 }
 func (bdb *BindingSQLDB) CheckServiceBinding(appId string) bool {
 	var count int
@@ -441,18 +445,22 @@ func (bdb *BindingSQLDB) GetCustomMetricStrategyByAppId(ctx context.Context, app
 }
 
 func (bdb *BindingSQLDB) SetOrUpdateCustomMetricStrategy(ctx context.Context, appId string, customMetricsStrategy string, actionName string) error {
-	appBinding, err := bdb.GetAppBindingByAppId(ctx, appId)
+	appBinding, err := bdb.getServiceBindingByAppId(ctx, appId)
 	if err != nil {
 		return err
 	}
 	query := bdb.sqldb.Rebind("UPDATE binding SET custom_metrics_strategy = ? WHERE binding_id = ?")
-	result, err := bdb.sqldb.ExecContext(ctx, query, nullableString(customMetricsStrategy), appBinding)
+	result, err := bdb.sqldb.ExecContext(ctx, query, nullableString(customMetricsStrategy), appBinding.ServiceBindingID)
 	if err != nil {
 		bdb.logger.Error(fmt.Sprintf("failed to %s custom metric submission strategy", actionName), err,
-			lager.Data{"query": query, "customMetricsStrategy": customMetricsStrategy, "bindingId": appBinding, "appId": appId})
+			lager.Data{"query": query, "customMetricsStrategy": customMetricsStrategy, "bindingId": appBinding.ServiceInstanceID, "appId": appId})
 		return err
 	}
 	if rowsAffected, err := result.RowsAffected(); err != nil || rowsAffected == 0 {
+		if customMetricsStrategy == appBinding.CustomMetricsStrategy {
+			bdb.logger.Info(fmt.Sprintf("custom metrics strategy already exists"), lager.Data{"query": query, "customMetricsStrategy": customMetricsStrategy, "bindingId": appBinding, "appId": appId})
+			return nil
+		}
 		bdb.logger.Error(fmt.Sprintf("failed to %s custom metric submission strategy", actionName), err,
 			lager.Data{"query": query, "customMetricsStrategy": customMetricsStrategy, "bindingId": appBinding, "appId": appId})
 		return errors.New("no rows affected")

diff --git a/src/autoscaler/db/sqldb/binding_sqldb_test.go b/src/autoscaler/db/sqldb/binding_sqldb_test.go
@@ -796,11 +796,24 @@ var _ = Describe("BindingSqldb", func() {
 						err = bdb.CreateServiceBinding(context.Background(), testBindingId, testInstanceId, testAppId, "same_app")
 						Expect(err).NotTo(HaveOccurred())
 					})
-					It("should update the custom metrics strategy", func() {
+					It("should update the custom metrics strategy to bound_app", func() {
+						Expect(err).NotTo(HaveOccurred())
 						customMetricStrategy, _ := bdb.GetCustomMetricStrategyByAppId(context.Background(), testAppId)
 						Expect(customMetricStrategy).To(Equal("bound_app"))
 					})
 				})
+				When("custom metrics strategy is already present as same_app", func() {
+					BeforeEach(func() {
+						customMetricsStrategy = "same_app"
+						err = bdb.CreateServiceBinding(context.Background(), testBindingId, testInstanceId, testAppId, "same_app")
+						Expect(err).NotTo(HaveOccurred())
+					})
+					It("should not update the same custom metrics strategy", func() {
+						Expect(err).NotTo(HaveOccurred())
+						customMetricStrategy, _ := bdb.GetCustomMetricStrategyByAppId(context.Background(), testAppId)
+						Expect(customMetricStrategy).To(Equal("same_app"))
+					})
+				})
 				When("custom metrics strategy unknown value", func() {
 					BeforeEach(func() {
 						customMetricsStrategy = "invalid_value"