diff --git a/src/acceptance/broker/broker_test.go b/src/acceptance/broker/broker_test.go index 73d540caad..0a677ff9d6 100644 --- 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 index 399c993b32..3f24efe847 100644 --- 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,6 +711,8 @@ 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 { @@ -720,13 +720,29 @@ func (b *Broker) GetBinding(ctx context.Context, instanceID string, bindingID st 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 index e9ffec9abd..be123e87cc 100644 --- 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,6 +177,8 @@ 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() { @@ -182,6 +186,48 @@ var _ = Describe("Broker", func() { 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 new file mode 100644 index 0000000000..3b2c08934a --- /dev/null +++ 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 new file mode 100644 index 0000000000..6e51c6d47b --- /dev/null +++ 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 index 190b3748a3..b6d3bd6a98 100644 --- 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 index b09831d154..62f3a831e4 100644 --- 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 index 8a165a29f5..8345a1b8da 100644 --- 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 index d99b5fb572..e509d0119b 100644 --- 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 index 9d9d87153b..8f8664e15b 100644 --- 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 {