Skip to content

Commit

Permalink
fix api->broker tests
Browse files Browse the repository at this point in the history
  • Loading branch information
asalan316 committed Oct 2, 2024
1 parent e7205c1 commit b30a9fc
Show file tree
Hide file tree
Showing 9 changed files with 75 additions and 57 deletions.
6 changes: 3 additions & 3 deletions flake.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -58,14 +58,9 @@ func handleCustomMetricsEndpoint(logger logr.Logger, customMetricTest CustomMetr
if appToScaleGuid != "" {
logger.Info("neighbour-app-relationship-found", "appToScaleGuid", appToScaleGuid)
appConfig.AppID = appToScaleGuid
//assuming the neighbour app has the same autoscaler service as the appToScale
currentApp, _ := cfenv.Current()
appConfig.Services = currentApp.Services

/*var services []cfenv.Service
// include custom metrics credentials mtls_url in the service
services = append(services, cfenv.Service{Tags: []string{"app-autoscaler"}, Credentials: map[string]interface{}{"custom_metrics": map[string]interface{}{"mtls_url": ""}}})
appConfig.Services = cfenv.Services{"app-autoscaler": services}*/
}
err = customMetricTest.PostCustomMetric(c, logger, appConfig, float64(metricValue), metricName, useMtls)
if err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ var _ = Describe("custom metrics tests", func() {
Body(`{"mtls":false}`).
End()
Expect(fakeCustomMetricClient.PostCustomMetricCallCount()).To(Equal(1))
_, _, sentValue, sentMetric, mtlsUsed, _ := fakeCustomMetricClient.PostCustomMetricArgsForCall(0)
_, _, _, sentValue, sentMetric, mtlsUsed := fakeCustomMetricClient.PostCustomMetricArgsForCall(0)
Expect(sentMetric).Should(Equal("test"))
Expect(sentValue).Should(Equal(4.0))
Expect(mtlsUsed).Should(Equal(false))
Expand Down
37 changes: 20 additions & 17 deletions src/autoscaler/api/broker/broker.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,17 +39,17 @@ type Broker struct {
}

var (
emptyJSONObject = regexp.MustCompile(`^\s*{\s*}\s*$`)
ErrCreatingServiceBinding = errors.New("error creating service binding")
ErrUpdatingServiceInstance = errors.New("error updating service instance")
ErrDeleteSchedulesForUnbinding = errors.New("failed to delete schedules for unbinding")
ErrBindingDoesNotExist = errors.New("service binding does not exist")
ErrDeletePolicyForUnbinding = errors.New("failed to delete policy for unbinding")
ErrDeleteServiceBinding = errors.New("error deleting service binding")
ErrCredentialNotDeleted = errors.New("failed to delete custom metrics credential for unbinding")
ErrInvalidCredentialType = errors.New("invalid credential type provided: allowed values are [binding-secret, x509]")

ErrInvalidConfigurations = errors.New("invalid binding configurations provided")
emptyJSONObject = regexp.MustCompile(`^\s*{\s*}\s*$`)
ErrCreatingServiceBinding = errors.New("error creating service binding")
ErrUpdatingServiceInstance = errors.New("error updating service instance")
ErrDeleteSchedulesForUnbinding = errors.New("failed to delete schedules for unbinding")
ErrBindingDoesNotExist = errors.New("service binding does not exist")
ErrDeletePolicyForUnbinding = errors.New("failed to delete policy for unbinding")
ErrDeleteServiceBinding = errors.New("error deleting service binding")
ErrCredentialNotDeleted = errors.New("failed to delete custom metrics credential for unbinding")
ErrInvalidCredentialType = errors.New("invalid credential type provided: allowed values are [binding-secret, x509]")
ErrInvalidConfigurations = errors.New("invalid binding configurations provided")
ErrInvalidCustomMetricsStrategy = errors.New("error: custom metrics strategy not supported")
)

type Errors []error
Expand Down Expand Up @@ -512,10 +512,10 @@ func (b *Broker) Bind(ctx context.Context, instanceID string, bindingID string,
WithErrorKey(actionReadBindingConfiguration).
Build()
}
logger.Info("binding-configuration", lager.Data{"bindingConfiguration": bindingConfiguration})
if bindingConfiguration.GetCustomMetricsStrategy() == "" {
bindingConfiguration.SetDefaultCustomMetricsStrategy("same_app")
}
}
logger.Info("binding-configuration", lager.Data{"bindingConfiguration": bindingConfiguration})
if policyJson == nil && bindingConfiguration.GetCustomMetricsStrategy() == "" {
bindingConfiguration.SetDefaultCustomMetricsStrategy("same_app")
}
policy, err := b.getPolicyFromJsonRawMessage(policyJson, instanceID, details.PlanID)
if err != nil {
Expand Down Expand Up @@ -558,6 +558,9 @@ func (b *Broker) Bind(ctx context.Context, instanceID string, bindingID string,
if errors.Is(err, db.ErrAlreadyExists) {
return result, apiresponses.NewFailureResponse(errors.New("error: an autoscaler service instance is already bound to the application and multiple bindings are not supported"), http.StatusConflict, actionCreateServiceBinding)
}
if errors.Is(err, ErrInvalidCustomMetricsStrategy) {
return result, apiresponses.NewFailureResponse(err, http.StatusBadRequest, actionCreateServiceBinding)
}
return result, apiresponses.NewFailureResponse(ErrCreatingServiceBinding, http.StatusInternalServerError, actionCreateServiceBinding)
}
customMetricsCredentials := &models.CustomMetricsCredentials{
Expand Down Expand Up @@ -869,7 +872,7 @@ func createServiceBinding(ctx context.Context, bindingDB db.BindingDB, bindingID
//TODO call bindingDB.CreateServiceBindingWithConfigs method. No need to call CreateServiceBinding method
// Caution: CHECK the below code may break the existing functionality ??
if customMetricsStrategy == "bound_app" || customMetricsStrategy == "same_app" {
return bindingDB.CreateServiceBindingWithConfigs(ctx, bindingID, instanceID, appGUID, customMetricsStrategy)
return bindingDB.CreateServiceBinding(ctx, bindingID, instanceID, appGUID, customMetricsStrategy)
}
return bindingDB.CreateServiceBinding(ctx, bindingID, instanceID, appGUID)
return ErrInvalidCustomMetricsStrategy
}
38 changes: 29 additions & 9 deletions src/autoscaler/api/brokerserver/broker_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -928,7 +928,7 @@ var _ = Describe("BrokerHandler", func() {
Expect(creds.Credentials.CustomMetrics.MtlsUrl).To(Equal("Mtls-someURL"))
})
})
XContext("Binding configurations are present", func() {
Context("Binding configurations are present", func() {
BeforeEach(func() {
bindingPolicy = `{
"configuration": {
Expand Down Expand Up @@ -969,20 +969,40 @@ var _ = Describe("BrokerHandler", func() {
bindingRequestBody.Policy = json.RawMessage(bindingPolicy)
body, err = json.Marshal(bindingRequestBody)
Expect(err).NotTo(HaveOccurred())
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"
}]
}`
verifyScheduleIsUpdatedInScheduler(testAppId, bindingPolicy)
})
It("succeeds with 201", func() {
Expect(resp.Code).To(Equal(http.StatusCreated))

By("updating the scheduler")
Expect(schedulerServer.ReceivedRequests()).To(HaveLen(1))
})

It("save config to database and returns with 201", func() {
// bindingdb.SaveCustomMetricsStrategyReturns(nil)
Expect(resp.Code).To(Equal(http.StatusCreated))

By("CreateServiceBindingWithConfigs should have called one time only")
// Expect(bindingdb.SaveCustomMetricsStrategyCallCount()).To(Equal(1))
Expect(bindingdb.CreateServiceBindingCallCount()).To(Equal(1))
})
})

Expand Down
2 changes: 1 addition & 1 deletion src/autoscaler/db/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ type BindingDB interface {
GetServiceInstanceByAppId(appId string) (*models.ServiceInstance, error)
UpdateServiceInstance(ctx context.Context, serviceInstance models.ServiceInstance) error
DeleteServiceInstance(ctx context.Context, serviceInstanceId string) error
CreateServiceBinding(ctx context.Context, bindingId string, serviceInstanceId string, appId string) error
CreateServiceBinding(ctx context.Context, bindingId string, serviceInstanceId string, appId string, customMetricsStrategy string) error
DeleteServiceBinding(ctx context.Context, bindingId string) error
DeleteServiceBindingByAppId(ctx context.Context, appId string) error
CheckServiceBinding(appId string) bool
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 @@ -200,7 +200,7 @@ func (bdb *BindingSQLDB) DeleteServiceInstance(ctx context.Context, serviceInsta
return db.ErrDoesNotExist
}

func (bdb *BindingSQLDB) CreateServiceBinding(ctx context.Context, bindingId string, serviceInstanceId string, appId string) error {
func (bdb *BindingSQLDB) CreateServiceBinding(ctx context.Context, bindingId string, serviceInstanceId string, appId string, customMetricsStrategy string) error {

err := bdb.isBindingExists(ctx, bindingId, serviceInstanceId, appId)
if err != nil {
Expand Down
36 changes: 18 additions & 18 deletions src/autoscaler/db/sqldb/binding_sqldb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ var _ = Describe("BindingSqldb", func() {
BeforeEach(func() {
err = bdb.CreateServiceInstance(context.Background(), models.ServiceInstance{ServiceInstanceId: testInstanceId, OrgId: testOrgGuid, SpaceId: testSpaceGuid, DefaultPolicy: policyJsonStr, DefaultPolicyGuid: policyGuid})
Expect(err).NotTo(HaveOccurred())
err = bdb.CreateServiceBinding(context.Background(), testBindingId, testInstanceId, testAppId)
err = bdb.CreateServiceBinding(context.Background(), testBindingId, testInstanceId, testAppId, "")
})
It("should return what was created", func() {
expectServiceInstancesToEqual(retrievedServiceInstance, &models.ServiceInstance{ServiceInstanceId: testInstanceId, OrgId: testOrgGuid, SpaceId: testSpaceGuid, DefaultPolicy: policyJsonStr, DefaultPolicyGuid: policyGuid})
Expand All @@ -353,7 +353,7 @@ var _ = Describe("BindingSqldb", func() {

Describe("CreateServiceBinding", func() {
JustBeforeEach(func() {
err = bdb.CreateServiceBinding(context.Background(), testBindingId, testInstanceId, testAppId)
err = bdb.CreateServiceBinding(context.Background(), testBindingId, testInstanceId, testAppId, "")
})
Context("When service instance doesn't exist", func() {
It("should error", func() {
Expand All @@ -376,7 +376,7 @@ var _ = Describe("BindingSqldb", func() {
})
Context("When service binding already exists", func() {
It("should error", func() {
err := bdb.CreateServiceBinding(context.Background(), testBindingId, testInstanceId, testAppId)
err := bdb.CreateServiceBinding(context.Background(), testBindingId, testInstanceId, testAppId, "")
Expect(err).To(HaveOccurred())
Expect(err).To(Equal(db.ErrAlreadyExists))
})
Expand All @@ -402,7 +402,7 @@ var _ = Describe("BindingSqldb", func() {
BeforeEach(func() {
err = bdb.CreateServiceInstance(context.Background(), models.ServiceInstance{ServiceInstanceId: testInstanceId, OrgId: testOrgGuid, SpaceId: testSpaceGuid, DefaultPolicy: policyJsonStr, DefaultPolicyGuid: policyGuid})
Expect(err).NotTo(HaveOccurred())
err = bdb.CreateServiceBinding(context.Background(), testBindingId, testInstanceId, testAppId)
err = bdb.CreateServiceBinding(context.Background(), testBindingId, testInstanceId, testAppId, "")
Expect(err).NotTo(HaveOccurred())
})
It("should return what was created", func() {
Expand Down Expand Up @@ -440,7 +440,7 @@ var _ = Describe("BindingSqldb", func() {
})
Context("When service binding is present", func() {
BeforeEach(func() {
err = bdb.CreateServiceBinding(context.Background(), testBindingId, testInstanceId, testAppId)
err = bdb.CreateServiceBinding(context.Background(), testBindingId, testInstanceId, testAppId, "")
Expect(err).NotTo(HaveOccurred())
})
It("should succeed", func() {
Expand All @@ -456,7 +456,7 @@ var _ = Describe("BindingSqldb", func() {
BeforeEach(func() {
err = bdb.CreateServiceInstance(context.Background(), models.ServiceInstance{ServiceInstanceId: testInstanceId, OrgId: testOrgGuid, SpaceId: testSpaceGuid, DefaultPolicy: policyJsonStr, DefaultPolicyGuid: policyGuid})
Expect(err).NotTo(HaveOccurred())
err = bdb.CreateServiceBinding(context.Background(), testBindingId, testInstanceId, testAppId)
err = bdb.CreateServiceBinding(context.Background(), testBindingId, testInstanceId, testAppId, "")
Expect(err).NotTo(HaveOccurred())
err = bdb.DeleteServiceBindingByAppId(context.Background(), testAppId)
})
Expand All @@ -475,7 +475,7 @@ var _ = Describe("BindingSqldb", func() {
BeforeEach(func() {
err = bdb.CreateServiceInstance(context.Background(), models.ServiceInstance{ServiceInstanceId: testInstanceId, OrgId: testOrgGuid, SpaceId: testSpaceGuid, DefaultPolicy: policyJsonStr, DefaultPolicyGuid: policyGuid})
Expect(err).NotTo(HaveOccurred())
err = bdb.CreateServiceBinding(context.Background(), testBindingId, testInstanceId, testAppId)
err = bdb.CreateServiceBinding(context.Background(), testBindingId, testInstanceId, testAppId, "")
Expect(err).NotTo(HaveOccurred())
})
It("should return true", func() {
Expand All @@ -501,7 +501,7 @@ var _ = Describe("BindingSqldb", func() {
BeforeEach(func() {
err = bdb.CreateServiceInstance(context.Background(), models.ServiceInstance{ServiceInstanceId: testInstanceId, OrgId: testOrgGuid, SpaceId: testSpaceGuid, DefaultPolicy: policyJsonStr, DefaultPolicyGuid: policyGuid})
Expect(err).NotTo(HaveOccurred())
err = bdb.CreateServiceBinding(context.Background(), testBindingId, testInstanceId, testAppId)
err = bdb.CreateServiceBinding(context.Background(), testBindingId, testInstanceId, testAppId, "")
Expect(err).NotTo(HaveOccurred())
})
It("should succeed", func() {
Expand All @@ -526,15 +526,15 @@ var _ = Describe("BindingSqldb", func() {
BeforeEach(func() {
err = bdb.CreateServiceInstance(context.Background(), models.ServiceInstance{ServiceInstanceId: testInstanceId, OrgId: testOrgGuid, SpaceId: testSpaceGuid, DefaultPolicy: policyJsonStr, DefaultPolicyGuid: policyGuid})
Expect(err).NotTo(HaveOccurred())
err = bdb.CreateServiceBinding(context.Background(), testBindingId, testInstanceId, testAppId)
err = bdb.CreateServiceBinding(context.Background(), testBindingId, testInstanceId, testAppId, "")
Expect(err).NotTo(HaveOccurred())
err = bdb.CreateServiceBinding(context.Background(), testBindingId2, testInstanceId, testAppId2)
err = bdb.CreateServiceBinding(context.Background(), testBindingId2, testInstanceId, testAppId2, "")
Expect(err).NotTo(HaveOccurred())

// other unrelated service instance with bindings
err = bdb.CreateServiceInstance(context.Background(), models.ServiceInstance{ServiceInstanceId: testInstanceId3, OrgId: testOrgGuid, SpaceId: testSpaceGuid, DefaultPolicy: policyJsonStr, DefaultPolicyGuid: policyGuid})
Expect(err).NotTo(HaveOccurred())
err = bdb.CreateServiceBinding(context.Background(), testBindingId3, testInstanceId3, testAppId3)
err = bdb.CreateServiceBinding(context.Background(), testBindingId3, testInstanceId3, testAppId3, "")
Expect(err).NotTo(HaveOccurred())
})
It("should succeed", func() {
Expand Down Expand Up @@ -599,17 +599,17 @@ var _ = Describe("BindingSqldb", func() {
err = bdb.CreateServiceInstance(context.Background(), models.ServiceInstance{testInstanceId, testOrgGuid, testSpaceGuid, policyJsonStr, policyGuid})
Expect(err).NotTo(HaveOccurred(), fmt.Sprintf("CreateServiceInstance, failed: testInstanceId %s procId %d", testInstanceId, GinkgoParallelProcess()))

err = bdb.CreateServiceBinding(context.Background(), testBindingId, testInstanceId, testAppId)
err = bdb.CreateServiceBinding(context.Background(), testBindingId, testInstanceId, testAppId, "")
Expect(err).NotTo(HaveOccurred())

err = bdb.CreateServiceBinding(context.Background(), testBindingId2, testInstanceId, testAppId2)
err = bdb.CreateServiceBinding(context.Background(), testBindingId2, testInstanceId, testAppId2, "")
Expect(err).NotTo(HaveOccurred())

// other unrelated service instance with bindings
err = bdb.CreateServiceInstance(context.Background(), models.ServiceInstance{testInstanceId3, testOrgGuid, testSpaceGuid, policyJsonStr, policyGuid})
Expect(err).NotTo(HaveOccurred())

err = bdb.CreateServiceBinding(context.Background(), testBindingId3, testInstanceId3, testAppId3)
err = bdb.CreateServiceBinding(context.Background(), testBindingId3, testInstanceId3, testAppId3, "")
Expect(err).NotTo(HaveOccurred())

})
Expand All @@ -636,9 +636,9 @@ var _ = Describe("BindingSqldb", func() {
BeforeEach(func() {
err = bdb.CreateServiceInstance(context.Background(), models.ServiceInstance{ServiceInstanceId: testInstanceId, OrgId: testOrgGuid, SpaceId: testSpaceGuid, DefaultPolicy: policyJsonStr, DefaultPolicyGuid: policyGuid})
Expect(err).NotTo(HaveOccurred())
err = bdb.CreateServiceBinding(context.Background(), testBindingId, testInstanceId, testAppId)
err = bdb.CreateServiceBinding(context.Background(), testBindingId, testInstanceId, testAppId, "")
Expect(err).NotTo(HaveOccurred())
err = bdb.CreateServiceBinding(context.Background(), testBindingId2, testInstanceId, testAppId2)
err = bdb.CreateServiceBinding(context.Background(), testBindingId2, testInstanceId, testAppId2, "")
Expect(err).NotTo(HaveOccurred())
})
It("should return true", func() {
Expand All @@ -651,9 +651,9 @@ var _ = Describe("BindingSqldb", func() {
Expect(err).NotTo(HaveOccurred())
err = bdb.CreateServiceInstance(context.Background(), models.ServiceInstance{ServiceInstanceId: testInstanceId2, OrgId: testOrgGuid, SpaceId: testSpaceGuid, DefaultPolicy: policyJsonStr, DefaultPolicyGuid: policyGuid})
Expect(err).NotTo(HaveOccurred())
err = bdb.CreateServiceBinding(context.Background(), testBindingId, testInstanceId, testAppId)
err = bdb.CreateServiceBinding(context.Background(), testBindingId, testInstanceId, testAppId, "")
Expect(err).NotTo(HaveOccurred())
err = bdb.CreateServiceBinding(context.Background(), testBindingId2, testInstanceId2, testAppId2)
err = bdb.CreateServiceBinding(context.Background(), testBindingId2, testInstanceId2, testAppId2, "")
Expect(err).NotTo(HaveOccurred())
})
It("should return false", func() {
Expand Down
2 changes: 1 addition & 1 deletion src/autoscaler/eventgenerator/metric/fetcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ var _ = Describe("logCacheFetcher", func() {
Entry("metric type cpuutil", models.MetricNameCPUUtil, "cpu_entitlement"),
Entry("metric type disk", models.MetricNameDisk, "disk"),
Entry("metric type diskutil", models.MetricNameDiskUtil, "disk|disk_quota"),
Entry("metric type CustomMetricsConfig", "a-custom-metric", "a-custom-metric"),
Entry("metric type CustomMetrics", "a-custom-metric", "a-custom-metric"),
)
})
})
Expand Down

0 comments on commit b30a9fc

Please sign in to comment.