Skip to content

Commit

Permalink
implement GetBinding to include binding configs
Browse files Browse the repository at this point in the history
  • Loading branch information
asalan316 committed Oct 22, 2024
1 parent 8e4bf83 commit 1cd4ac9
Show file tree
Hide file tree
Showing 13 changed files with 168 additions and 83 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
{
"configuration": {
"custom_metrics": {
"auth": {
"credential_type": ""
},
"metric_submission_strategy": {
"allow_from": "bound_app"
}
Expand Down
1 change: 1 addition & 0 deletions src/acceptance/broker/broker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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", string(policy)) //FIXME
Expect(err).NotTo(HaveOccurred())

err = helpers.BindServiceToAppWithPolicy(cfg, appName, instance.name(), policyFile)
Expand Down
64 changes: 4 additions & 60 deletions src/acceptance/helpers/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,17 +183,6 @@ func ServicePlansUrl(cfg *config.Config, spaceGuid string) string {
return url.String()
}

func GenerateBindingConfiguration(allowFrom string) string {
bindingConfig := BindingConfig{
Configuration: Configuration{CustomMetrics: CustomMetricsConfig{
MetricSubmissionStrategy: MetricsSubmissionStrategy{AllowFrom: allowFrom},
}},
}
marshalledBinding, err := MarshalWithoutHTMLEscape(bindingConfig)
Expect(err).NotTo(HaveOccurred())
return string(marshalledBinding)
}

func GenerateBindingsWithScalingPolicy(allowFrom string, instanceMin, instanceMax int, metricName string, scaleInThreshold, scaleOutThreshold int64) string {
bindingConfig := BindingConfig{
Configuration: Configuration{CustomMetrics: CustomMetricsConfig{
Expand Down Expand Up @@ -272,27 +261,6 @@ func GenerateDynamicScaleOutPolicyWithExtraFields(instanceMin, instanceMax int,
return string(extraBytes), string(validBytes)
}

func GenerateDynamicScaleInPolicy(instanceMin, instanceMax int, metricName string, threshold int64) string {
scalingInRule := ScalingRule{
MetricType: metricName,
BreachDurationSeconds: TestBreachDurationSeconds,
Threshold: threshold,
Operator: "<",
CoolDownSeconds: TestCoolDownSeconds,
Adjustment: "-1",
}

policy := ScalingPolicy{
InstanceMin: instanceMin,
InstanceMax: instanceMax,
ScalingRules: []*ScalingRule{&scalingInRule},
}
marshaled, err := MarshalWithoutHTMLEscape(policy)
Expect(err).NotTo(HaveOccurred())

return string(marshaled)
}

func GenerateDynamicScaleOutAndInPolicy(instanceMin, instanceMax int, metricName string, scaleInWhenBelowThreshold int64, scaleOutWhenGreaterOrEqualThreshold int64) string {
policy := buildScaleOutScaleInPolicy(instanceMin, instanceMax, metricName, scaleInWhenBelowThreshold, scaleOutWhenGreaterOrEqualThreshold)
marshaled, err := MarshalWithoutHTMLEscape(policy)
Expand Down Expand Up @@ -512,15 +480,15 @@ func MarshalWithoutHTMLEscape(v interface{}) ([]byte, error) {

func CreatePolicy(cfg *config.Config, appName, appGUID, policy string) string {
GinkgoHelper()
instanceName, _ := createPolicy(cfg, appName, appGUID, policy)
instanceName, _ := createPolicy(cfg, appName, policy)
return instanceName
}

func CreatePolicyWithErr(cfg *config.Config, appName, appGUID, policy string) (string, error) {
return createPolicy(cfg, appName, appGUID, policy)
func CreatePolicyWithErr(cfg *config.Config, appName, policy string) (string, error) {
return createPolicy(cfg, appName, policy)
}

func createPolicy(cfg *config.Config, appName, appGUID, policy string) (string, error) {
func createPolicy(cfg *config.Config, appName, policy string) (string, error) {
GinkgoHelper()
instanceName := generator.PrefixedRandomName(cfg.Prefix, cfg.InstancePrefix)
err := Retry(defaultRetryAttempt, defaultRetryAfter, func() error { return CreateServiceWithPlan(cfg, cfg.ServicePlan, instanceName) })
Expand Down Expand Up @@ -628,23 +596,6 @@ func GetServiceCredentialBindingParameters(cfg *config.Config, instanceName stri
return strings.TrimSpace(string(cmd.Out.Contents()))
}

func CreatePolicyWithAPI(cfg *config.Config, appGUID, policy string) {
GinkgoHelper()
oauthToken := OauthToken(cfg)
client := GetHTTPClient(cfg)

policyURL := fmt.Sprintf("%s%s", cfg.ASApiEndpoint, strings.Replace(PolicyPath, "{appId}", appGUID, -1))
req, err := http.NewRequest("PUT", policyURL, bytes.NewBuffer([]byte(policy)))
Expect(err).ShouldNot(HaveOccurred())
req.Header.Add("Authorization", oauthToken)
req.Header.Add("Content-Type", "application/json")

resp, err := client.Do(req)
Expect(err).ShouldNot(HaveOccurred())
defer func() { _ = resp.Body.Close() }()
Expect(resp).Should(HaveHTTPStatus(200, 201), "assigning policy by putting to %s", policyURL)
}

func GetHTTPClient(cfg *config.Config) *http.Client {
return &http.Client{
Transport: &http.Transport{
Expand Down Expand Up @@ -679,13 +630,6 @@ func GetAppGuid(cfg *config.Config, appName string) (string, error) {
return appGuid, err
}

func FailOnCommandFailuref(command *Session, format string, args ...any) *Session {
if command.ExitCode() != 0 {
Fail(fmt.Sprintf(format, args...))
}
return command
}

func SetLabel(cfg *config.Config, appGUID string, labelKey string, labelValue string) {
GinkgoHelper()
cmd := cf.Cf("curl", "--fail", fmt.Sprintf("/v3/apps/%s", appGUID), "-X", "PATCH", "-d", fmt.Sprintf(`{"metadata": {"labels": {"%s": "%s"}}}`, labelKey, labelValue)).Wait(cfg.DefaultTimeoutDuration())
Expand Down
4 changes: 2 additions & 2 deletions src/acceptance/setup_performance/setup_performance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,12 @@ func pushAppAndBindService(appName string, runningApps *int32, pendingApps *sync
}
policy := helpers.GenerateDynamicScaleOutAndInPolicy(
1, 2, "test_metric", 500, 500)
appGUID, err := helpers.GetAppGuid(cfg, appName)
_, err = helpers.GetAppGuid(cfg, appName)
if err != nil {
errors.Store(appName, err)
return
}
_, err = helpers.CreatePolicyWithErr(cfg, appName, appGUID, policy)
_, err = helpers.CreatePolicyWithErr(cfg, appName, policy)
if err != nil {
errors.Store(appName, err)
return
Expand Down
24 changes: 20 additions & 4 deletions src/autoscaler/api/broker/broker.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"errors"
"fmt"
"net/http"
"reflect"
"regexp"
"strings"

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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})

Expand Down
64 changes: 55 additions & 9 deletions src/autoscaler/api/broker/broker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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() {
Expand Down Expand Up @@ -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() {
Expand All @@ -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}))
})
})
})
})

Expand Down
19 changes: 19 additions & 0 deletions src/autoscaler/api/broker/testdata/policy-with-configs.json
Original file line number Diff line number Diff line change
@@ -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"
}
]
}
9 changes: 9 additions & 0 deletions src/autoscaler/api/broker/testdata/with-configs.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"configuration": {
"custom_metrics": {
"metric_submission_strategy": {
"allow_from": "bound_app"
}
}
}
}
41 changes: 41 additions & 0 deletions src/autoscaler/api/brokerserver/broker_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
1 change: 0 additions & 1 deletion src/autoscaler/api/db/servicebroker.db.changelog.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,6 @@ databaseChangeLog:
- column:
name: custom_metrics_strategy
value: 'same_app'

- changeSet:
id: 7
author: Arsalan
Expand Down
2 changes: 1 addition & 1 deletion src/autoscaler/db/sqldb/binding_sqldb.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Loading

0 comments on commit 1cd4ac9

Please sign in to comment.