Skip to content

Commit

Permalink
fix- instance params returns binding params
Browse files Browse the repository at this point in the history 
clean up: remove programmtic focus
  • Loading branch information
@app-autoscaler-ci-bot @asalan316
app-autoscaler-ci-bot authored and asalan316 committed Nov 6, 2024
1 parent fd1c86e commit b259ba7
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 57 deletions.
61 changes: 22 additions & 39 deletions src/acceptance/broker/broker_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,31 +95,25 @@ var _ = Describe("AutoScaler Service Broker", func() {

It("binds&unbinds with policy", func() {
policyFile := "../assets/file/policy/all.json"

err := helpers.BindServiceToAppWithPolicy(cfg, appName, instance.name(), policyFile)
Expect(err).NotTo(HaveOccurred())
expectedPolicyFile := "../assets/file/policy/policy-with-configuration-same-app.json"
expectedPolicyWithConfig, err := os.ReadFile(expectedPolicyFile)
Expect(err).NotTo(HaveOccurred())
bindingParameters := helpers.GetServiceCredentialBindingParameters(cfg, instance.name(), appName)
Expect(bindingParameters).Should(MatchJSON(expectedPolicyWithConfig))

instance.unbind(appName)
})
It("binds&unbinds with configurations and policy", func() {
policyFile := "../assets/file/policy/policy-with-configuration.json"
policy, err := os.ReadFile(policyFile)
Expect(err).NotTo(HaveOccurred())

err = helpers.BindServiceToAppWithPolicy(cfg, appName, instance.name(), policyFile)
Expect(err).NotTo(HaveOccurred())
By("checking broker bind parameter response should have policy and configuration")

bindingParameters := helpers.GetServiceCredentialBindingParameters(cfg, instance.name(), appName)
Expect(bindingParameters).Should(MatchJSON(policy))

instance.unbind(appName)
})

It("bind&unbinds without policy", func() {
helpers.BindServiceToApp(cfg, appName, instance.name())
bindingParameters := helpers.GetServiceCredentialBindingParameters(cfg, instance.name(), appName)
Expect(bindingParameters).Should(MatchJSON("{}"))
instance.unbind(appName)
})

It("binds&unbinds with policy having credential-type as x509", func() {
policyFile := "../assets/file/policy/policy-with-credential-type.json"
_, err := os.ReadFile(policyFile)
Expand All @@ -141,21 +135,17 @@ var _ = Describe("AutoScaler Service Broker", func() {
instance.unbind(appName)
})

It("bind&unbinds without policy and gives only configuration", func() {
helpers.BindServiceToApp(cfg, appName, instance.name())
By("checking broker bind parameter response does not have policy but configuration only")
It("binds&unbinds with configurations and policy", func() {
policyFile := "../assets/file/policy/policy-with-configuration.json"
policyWithConfig, err := os.ReadFile(policyFile)
Expect(err).NotTo(HaveOccurred())

err = helpers.BindServiceToAppWithPolicy(cfg, appName, instance.name(), policyFile)
Expect(err).NotTo(HaveOccurred())
By("checking broker bind parameter response should have policy and configuration")
bindingParameters := helpers.GetServiceCredentialBindingParameters(cfg, instance.name(), appName)
expectedJSON := `{
"configuration": {
"custom_metrics": {
"metric_submission_strategy": {
"allow_from": "same_app"
}
}
}
}`

Expect(bindingParameters).Should(MatchJSON(expectedJSON))
Expect(bindingParameters).Should(MatchJSON(policyWithConfig))

instance.unbind(appName)
})

Expand All @@ -167,7 +157,7 @@ var _ = Describe("AutoScaler Service Broker", func() {
Describe("allows setting default policies", func() {
var instance serviceInstance
var defaultPolicy []byte
var expectedDefaultPolicyWithConfigsJSON []byte
var policy []byte

BeforeEach(func() {
instance = createServiceWithParameters(cfg.ServicePlan, "../assets/file/policy/default_policy.json")
Expand All @@ -183,13 +173,7 @@ var _ = Describe("AutoScaler Service Broker", func() {
err = json.Unmarshal(defaultPolicy, &serviceParameters)
Expect(err).NotTo(HaveOccurred())

expectedDefaultPolicyWithConfigsJSON, err = os.ReadFile("../assets/file/policy/default_policy-with-configuration.json")
Expect(err).NotTo(HaveOccurred())
var serviceParametersWithConfigs = struct {
Configuration interface{} `json:"configuration"`
DefaultPolicy interface{} `json:"default_policy"`
}{}
err = json.Unmarshal(expectedDefaultPolicyWithConfigsJSON, &serviceParametersWithConfigs)
policy, err = json.Marshal(serviceParameters.DefaultPolicy)
Expect(err).NotTo(HaveOccurred())
})

Expand All @@ -200,9 +184,9 @@ var _ = Describe("AutoScaler Service Broker", func() {

It("sets the default policy if no policy is set during binding and allows retrieving the policy via the binding parameters", func() {
helpers.BindServiceToApp(cfg, appName, instance.name())
By("checking broker bind parameter response should have default policy and configuration")
By("checking broker bind parameter response should have default policy")
bindingParameters := helpers.GetServiceCredentialBindingParameters(cfg, instance.name(), appName)
Expect(bindingParameters).Should(MatchJSON(expectedDefaultPolicyWithConfigsJSON))
Expect(bindingParameters).Should(MatchJSON(policy))

unbindService := cf.Cf("unbind-service", appName, instance.name()).Wait(cfg.DefaultTimeoutDuration())
Expect(unbindService).To(Exit(0), "failed unbinding service from app")
Expand All @@ -222,7 +206,6 @@ var _ = Describe("AutoScaler Service Broker", func() {
var instance serviceInstance
It("should update a service instance from one plan to another plan", func() {
servicePlans := GetServicePlans(cfg)
fmt.Println("servicePlans", servicePlans)
source, target, err := servicePlans.getSourceAndTargetForPlanUpdate()
Expect(err).NotTo(HaveOccurred(), "failed getting source and target service plans")
instance = createService(source.Name)
Expand Down
38 changes: 21 additions & 17 deletions src/autoscaler/api/broker/broker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -711,34 +711,38 @@ func (b *Broker) GetBinding(ctx context.Context, instanceID string, bindingID st
if err != nil {
return result, err
}
bindingConfig := &models.BindingConfig{}
bindingConfig.SetCustomMetricsStrategy(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 {
areConfigAndPolicyPresent := combinedConfig != nil && policy.InstanceMin > 0
if areConfigAndPolicyPresent {
combinedConfig.ScalingPolicy = *policy
result.Parameters = combinedConfig
} else {
result.Parameters = policy
}
} else if !b.isEmpty(bindingConfig) {
combinedConfig, bindingConfig := b.buildConfigurationIfPresent(serviceBinding.CustomMetricsStrategy)
if policy != nil && combinedConfig != nil { //both are present
combinedConfig.ScalingPolicy = *policy
combinedConfig.BindingConfig = *bindingConfig
result.Parameters = combinedConfig
} else if policy != nil { // only policy was given
result.Parameters = policy
} else if combinedConfig != nil { // only configuration was given
result.Parameters = bindingConfig
}

return result, nil
}

func (b *Broker) buildConfigurationIfPresent(customMetricsStrategy string) (*models.BindingConfigWithScaling, *models.BindingConfig) {
var combinedConfig *models.BindingConfigWithScaling
var bindingConfig *models.BindingConfig

if customMetricsStrategy != "" && customMetricsStrategy != models.CustomMetricsSameApp { //if custom metric was not given in the binding process
combinedConfig = &models.BindingConfigWithScaling{}
bindingConfig = &models.BindingConfig{}
bindingConfig.SetCustomMetricsStrategy(customMetricsStrategy)
combinedConfig.BindingConfig = *bindingConfig
}
return combinedConfig, bindingConfig
}

func (b *Broker) isEmpty(bindingConfig *models.BindingConfig) bool {
return reflect.DeepEqual(bindingConfig, &models.BindingConfig{})
}
Expand Down
49 changes: 48 additions & 1 deletion src/autoscaler/api/brokerserver/broker_handler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -923,7 +923,7 @@ var _ = Describe("BrokerHandler", func() {
verifyCredentialsGenerated(resp)
})
})
When("Binding configurations are present", func() {
When("Binding configurations and policy are present", func() {
BeforeEach(func() {
bindingPolicy = `{
"configuration": {
Expand Down Expand Up @@ -1000,6 +1000,53 @@ var _ = Describe("BrokerHandler", func() {
verifyCredentialsGenerated(resp)
})
})
When("Binding configurations are empty", func() {
BeforeEach(func() {
bindingPolicy = `{
"instance_max_count":4,
"instance_min_count":1,
"schedules": {
"timezone": "Asia/Shanghai",
"recurring_schedule": [{
"start_time": "10:00",
"end_time": "18:00",
"days_of_week": [
1,
2,
3
],
"instance_min_count": 1,
"instance_max_count": 10,
"initial_min_instance_count": 5
}]
},
"scaling_rules":[
{
"metric_type":"memoryused",
"threshold":30,
"operator":"<",
"adjustment":"-1"
}]
}`
bindingRequestBody.Policy = json.RawMessage(bindingPolicy)
body, err = json.Marshal(bindingRequestBody)
Expect(err).NotTo(HaveOccurred())

verifyScheduleIsUpdatedInScheduler(testAppId, bindingPolicy)
})
It("set the default custom metrics strategy", func() {
_, _, _, _, customMetricsStrategy := bindingdb.CreateServiceBindingArgsForCall(0)
Expect(customMetricsStrategy).To(Equal(models.CustomMetricsSameApp))

})
It("succeeds with 201", func() {
Expect(resp.Code).To(Equal(http.StatusCreated))
By("updating the scheduler")
Expect(schedulerServer.ReceivedRequests()).To(HaveLen(1))
Expect(bindingdb.CreateServiceBindingCallCount()).To(Equal(1))
verifyCredentialsGenerated(resp)
})
})

Context("credential-type is provided while binding", func() {
BeforeEach(func() {
Expand Down

0 comments on commit b259ba7

Please sign in to comment.