implement GetBinding to include binding configs

cloudfoundry · Oct 22, 2024 · 3b57893 · 3b57893
1 parent 8e4bf83
commit 3b57893
Show file tree

Hide file tree

Showing 10 changed files with 159 additions and 21 deletions.
diff --git a/src/acceptance/broker/broker_test.go b/src/acceptance/broker/broker_test.go
@@ -109,6 +109,7 @@ var _ = Describe("AutoScaler Service Broker", func() {
 		It("binds&unbinds with configurations and policy", func() {
 			policyFile := "../assets/file/policy/policy-with-configuration.json"
 			policy, err := os.ReadFile(policyFile)
+			fmt.Println("policy", policy) //FIXME
 			Expect(err).NotTo(HaveOccurred())
 
 			err = helpers.BindServiceToAppWithPolicy(cfg, appName, instance.name(), policyFile)

diff --git a/src/autoscaler/api/broker/broker.go b/src/autoscaler/api/broker/broker.go
@@ -7,6 +7,7 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
+	"reflect"
 	"regexp"
 	"strings"
 
@@ -498,9 +499,6 @@ func (b *Broker) Bind(ctx context.Context, instanceID string, bindingID string,
 	if details.RawParameters != nil {
 		policyJson = details.RawParameters
 	}
-
-	// extract custom metrics configs to determine metric submission strategy and set to default if not provided
-
 	bindingConfiguration := &models.BindingConfig{}
 	if policyJson != nil {
 		err := json.Unmarshal(policyJson, &bindingConfiguration)
@@ -713,20 +711,38 @@ func (b *Broker) GetBinding(ctx context.Context, instanceID string, bindingID st
 	if err != nil {
 		return result, err
 	}
+	bindingConfig := &models.BindingConfig{}
+	bindingConfig.SetDefaultCustomMetricsStrategy(serviceBinding.CustomMetricsStrategy)
 
 	policy, err := b.policydb.GetAppPolicy(ctx, serviceBinding.AppID)
 	if err != nil {
 		b.logger.Error("get-binding", err, lager.Data{"instanceID": instanceID, "bindingID": bindingID, "fetchBindingDetails": details})
 		return domain.GetBindingSpec{}, apiresponses.NewFailureResponse(errors.New("failed to retrieve scaling policy"), http.StatusInternalServerError, "get-policy")
 	}
 
+	var combinedConfig *models.BindingConfigWithScaling
+	if bindingConfig.GetCustomMetricsStrategy() != "" {
+		combinedConfig = &models.BindingConfigWithScaling{BindingConfig: *bindingConfig}
+	}
 	if policy != nil {
-		result.Parameters = policy
+		areConfigAndPolicyPresent := combinedConfig != nil && policy.InstanceMin > 0
+		if areConfigAndPolicyPresent {
+			combinedConfig.ScalingPolicy = *policy
+			result.Parameters = combinedConfig
+		} else {
+			result.Parameters = policy
+		}
+	} else if !b.isEmpty(bindingConfig) {
+		result.Parameters = bindingConfig
 	}
 
 	return result, nil
 }
 
+func (b *Broker) isEmpty(bindingConfig *models.BindingConfig) bool {
+	return reflect.DeepEqual(bindingConfig, &models.BindingConfig{})
+}
+
 func (b *Broker) getServiceBinding(ctx context.Context, bindingID string) (*models.ServiceBinding, error) {
 	logger := b.logger.Session("get-service-binding", lager.Data{"bindingID": bindingID})
 

diff --git a/src/autoscaler/api/broker/broker_test.go b/src/autoscaler/api/broker/broker_test.go
@@ -2,6 +2,7 @@ package broker_test
 
 import (
 	"encoding/json"
+	"os"
 
 	"code.cloudfoundry.org/app-autoscaler/src/autoscaler/api/broker"
 	"code.cloudfoundry.org/app-autoscaler/src/autoscaler/db"
@@ -19,12 +20,14 @@ import (
 
 var _ = Describe("Broker", func() {
 	var (
-		aBroker         *broker.Broker
-		err             error
-		fakeBindingDB   *fakes.FakeBindingDB
-		fakePolicyDB    *fakes.FakePolicyDB
-		fakeCredentials *fakes.FakeCredentials
-		testLogger      = lagertest.NewTestLogger("test")
+		aBroker                  *broker.Broker
+		err                      error
+		fakeBindingDB            *fakes.FakeBindingDB
+		fakePolicyDB             *fakes.FakePolicyDB
+		fakeCredentials          *fakes.FakeCredentials
+		testLogger               = lagertest.NewTestLogger("test")
+		bindingConfigWithScaling *models.BindingConfigWithScaling
+		bindingConfig            *models.BindingConfig
 	)
 
 	BeforeEach(func() {
@@ -153,11 +156,10 @@ var _ = Describe("Broker", func() {
 			})
 		})
 		Context("when the binding exists", func() {
-			BeforeEach(func() {
-				fakeBindingDB.GetServiceBindingReturns(&models.ServiceBinding{ServiceBindingID: testBindingId, ServiceInstanceID: testInstanceId, AppID: testAppId}, nil)
-			})
 			Context("without policy", func() {
 				BeforeEach(func() {
+					fakeBindingDB.GetServiceBindingReturns(&models.ServiceBinding{ServiceBindingID: testBindingId,
+						ServiceInstanceID: testInstanceId, AppID: testAppId}, nil)
 					fakePolicyDB.GetAppPolicyReturns(nil, nil)
 				})
 				It("returns the empty binding without parameters", func() {
@@ -175,13 +177,57 @@ var _ = Describe("Broker", func() {
 			})
 			Context("with policy", func() {
 				BeforeEach(func() {
+					fakeBindingDB.GetServiceBindingReturns(&models.ServiceBinding{ServiceBindingID: testBindingId,
+						ServiceInstanceID: testInstanceId, AppID: testAppId}, nil)
 					fakePolicyDB.GetAppPolicyReturns(scalingPolicy, nil)
 				})
 				It("returns the Binding with parameters", func() {
 					Expect(err).To(BeNil())
 					Expect(Binding).To(Equal(domain.GetBindingSpec{Parameters: scalingPolicy}))
 				})
 			})
+			Context("with configuration and policy", func() {
+				BeforeEach(func() {
+					bindingConfig = &models.BindingConfig{Configuration: models.Configuration{CustomMetrics: models.CustomMetricsConfig{
+						Auth:                     models.Auth{},
+						MetricSubmissionStrategy: models.MetricsSubmissionStrategy{AllowFrom: "bound_app"},
+					},
+					}}
+					fakeBindingDB.GetServiceBindingReturns(&models.ServiceBinding{ServiceBindingID: testBindingId,
+						ServiceInstanceID: testInstanceId, AppID: testAppId, CustomMetricsStrategy: "bound_app"}, nil)
+					bindingBytes, err := os.ReadFile("testdata/policy-with-configs.json")
+					Expect(err).ShouldNot(HaveOccurred())
+
+					err = json.Unmarshal(bindingBytes, &bindingConfigWithScaling)
+					Expect(err).ShouldNot(HaveOccurred())
+					fakePolicyDB.GetAppPolicyReturns(scalingPolicy, nil)
+				})
+				It("returns the Binding with configs and policy in parameters", func() {
+					Expect(err).To(BeNil())
+					Expect(Binding).To(Equal(domain.GetBindingSpec{Parameters: bindingConfigWithScaling}))
+				})
+			})
+			Context("with configuration only", func() {
+				BeforeEach(func() {
+					bindingConfig = &models.BindingConfig{Configuration: models.Configuration{CustomMetrics: models.CustomMetricsConfig{
+						Auth:                     models.Auth{},
+						MetricSubmissionStrategy: models.MetricsSubmissionStrategy{AllowFrom: "bound_app"},
+					},
+					}}
+					fakeBindingDB.GetServiceBindingReturns(&models.ServiceBinding{ServiceBindingID: testBindingId,
+						ServiceInstanceID: testInstanceId, AppID: testAppId, CustomMetricsStrategy: "bound_app"}, nil)
+					bindingBytes, err := os.ReadFile("testdata/with-configs.json")
+					Expect(err).ShouldNot(HaveOccurred())
+
+					err = json.Unmarshal(bindingBytes, &bindingConfigWithScaling)
+					Expect(err).ShouldNot(HaveOccurred())
+					fakePolicyDB.GetAppPolicyReturns(nil, nil)
+				})
+				It("returns the bindings with configs in parameters", func() {
+					Expect(err).To(BeNil())
+					Expect(Binding).To(Equal(domain.GetBindingSpec{Parameters: bindingConfig}))
+				})
+			})
 		})
 	})
 

diff --git a/src/autoscaler/api/broker/testdata/policy-with-configs.json b/src/autoscaler/api/broker/testdata/policy-with-configs.json
@@ -0,0 +1,19 @@
+{
+  "configuration": {
+    "custom_metrics": {
+      "metric_submission_strategy": {
+        "allow_from": "bound_app"
+      }
+    }
+  },
+  "instance_min_count": 1,
+  "instance_max_count": 5,
+  "scaling_rules": [
+    {
+      "metric_type": "memoryused",
+      "threshold": 30,
+      "operator": "<",
+      "adjustment": "-1"
+    }
+  ]
+}
diff --git a/src/autoscaler/api/broker/testdata/with-configs.json b/src/autoscaler/api/broker/testdata/with-configs.json
@@ -0,0 +1,9 @@
+{
+  "configuration": {
+    "custom_metrics": {
+      "metric_submission_strategy": {
+        "allow_from": "bound_app"
+      }
+    }
+  }
+}
diff --git a/src/autoscaler/api/brokerserver/broker_handler_test.go b/src/autoscaler/api/brokerserver/broker_handler_test.go
@@ -1500,6 +1500,47 @@ var _ = Describe("BrokerHandler", func() {
 			})
 		})
 	})
+
+	Describe("GetBinding", func() {
+		var (
+			err           error
+			bindingPolicy string
+		)
+
+		JustBeforeEach(func() {
+			req, err = http.NewRequest(http.MethodGet, "", nil)
+			Expect(err).NotTo(HaveOccurred())
+			handler.GetBinding(resp, req)
+		})
+
+		Context("Binding configurations are exist", func() {
+			BeforeEach(func() {
+				bindingPolicy = `{
+				  "configuration": {
+					"custom_metrics": {
+					  "metric_submission_strategy": {
+						"allow_from": "bound_app"
+					  }
+					}
+				  },
+				  "instance_max_count":4,
+				  "instance_min_count":1,
+				   "scaling_rules":[
+					{
+					  "metric_type":"memoryused",
+					  "threshold":30,
+					  "operator":"<",
+					  "adjustment":"-1"
+					}]
+				}`
+				Expect(bindingPolicy).NotTo(BeEmpty())
+
+			})
+			It("succeeds with 200", func() {
+				Expect(resp.Code).To(Equal(http.StatusPreconditionFailed))
+			})
+		})
+	})
 })
 
 func verifyCredentialsGenerated(resp *httptest.ResponseRecorder) {

diff --git a/src/autoscaler/api/db/servicebroker.db.changelog.yaml b/src/autoscaler/api/db/servicebroker.db.changelog.yaml
@@ -165,7 +165,6 @@ databaseChangeLog:
               - column:
                   name: custom_metrics_strategy
                   value: 'same_app'
-
   - changeSet:
       id: 7
       author: Arsalan

diff --git a/src/autoscaler/db/sqldb/binding_sqldb.go b/src/autoscaler/db/sqldb/binding_sqldb.go
@@ -244,7 +244,7 @@ func (bdb *BindingSQLDB) GetServiceBinding(ctx context.Context, serviceBindingId
 
 	serviceBinding := &models.ServiceBinding{}
 
-	query := bdb.sqldb.Rebind("SELECT binding_id, service_instance_id, app_id FROM binding WHERE binding_id =?")
+	query := bdb.sqldb.Rebind("SELECT binding_id, service_instance_id, app_id, custom_metrics_strategy FROM binding WHERE binding_id =?")
 
 	err := bdb.sqldb.GetContext(ctx, serviceBinding, query, serviceBindingId)
 	if err != nil {

diff --git a/src/autoscaler/db/sqldb/binding_sqldb_test.go b/src/autoscaler/db/sqldb/binding_sqldb_test.go
@@ -439,9 +439,10 @@ var _ = Describe("BindingSqldb", func() {
 			It("should return what was created", func() {
 				Expect(err).NotTo(HaveOccurred())
 				Expect(retrievedServiceBinding).To(Equal(&models.ServiceBinding{
-					ServiceBindingID:  testBindingId,
-					ServiceInstanceID: testInstanceId,
-					AppID:             testAppId,
+					ServiceBindingID:      testBindingId,
+					ServiceInstanceID:     testInstanceId,
+					AppID:                 testAppId,
+					CustomMetricsStrategy: "same_app",
 				}))
 			})
 		})

diff --git a/src/autoscaler/models/api.go b/src/autoscaler/models/api.go
@@ -49,9 +49,15 @@ type ServiceInstance struct {
 }
 
 type ServiceBinding struct {
-	ServiceBindingID  string `db:"binding_id"`
-	ServiceInstanceID string `db:"service_instance_id"`
-	AppID             string `db:"app_id"`
+	ServiceBindingID      string `db:"binding_id"`
+	ServiceInstanceID     string `db:"service_instance_id"`
+	AppID                 string `db:"app_id"`
+	CustomMetricsStrategy string `db:"custom_metrics_strategy"`
+}
+
+type BindingConfigWithScaling struct {
+	BindingConfig
+	ScalingPolicy
 }
 
 type BindingRequestBody struct {