diff --git a/flake.lock b/flake.lock index 789724cc76..35448561b1 100644 --- a/flake.lock +++ b/flake.lock @@ -2,11 +2,11 @@ "nodes": { "nixpkgs": { "locked": { - "lastModified": 1725103162, - "narHash": "sha256-Ym04C5+qovuQDYL/rKWSR+WESseQBbNAe5DsXNx5trY=", + "lastModified": 1727348695, + "narHash": "sha256-J+PeFKSDV+pHL7ukkfpVzCOO7mBSrrpJ3svwBFABbhI=", "owner": "NixOS", "repo": "nixpkgs", - "rev": "12228ff1752d7b7624a54e9c1af4b222b3c1073b", + "rev": "1925c603f17fc89f4c8f6bf6f631a802ad85d784", "type": "github" }, "original": { diff --git a/src/acceptance/assets/app/go_app/internal/app/custom_metrics.go b/src/acceptance/assets/app/go_app/internal/app/custom_metrics.go index 17b202df9d..7e0d37bdf4 100644 --- a/src/acceptance/assets/app/go_app/internal/app/custom_metrics.go +++ b/src/acceptance/assets/app/go_app/internal/app/custom_metrics.go @@ -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 { diff --git a/src/acceptance/assets/app/go_app/internal/app/custom_metrics_test.go b/src/acceptance/assets/app/go_app/internal/app/custom_metrics_test.go index 1700719539..f460155457 100644 --- a/src/acceptance/assets/app/go_app/internal/app/custom_metrics_test.go +++ b/src/acceptance/assets/app/go_app/internal/app/custom_metrics_test.go @@ -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)) diff --git a/src/autoscaler/api/broker/broker.go b/src/autoscaler/api/broker/broker.go index 2f7f71dcbd..cb2872e21e 100644 --- a/src/autoscaler/api/broker/broker.go +++ b/src/autoscaler/api/broker/broker.go @@ -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 @@ -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 { @@ -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{ @@ -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 } diff --git a/src/autoscaler/api/brokerserver/broker_handler_test.go b/src/autoscaler/api/brokerserver/broker_handler_test.go index d937cc24f8..6aa56509fc 100644 --- a/src/autoscaler/api/brokerserver/broker_handler_test.go +++ b/src/autoscaler/api/brokerserver/broker_handler_test.go @@ -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": { @@ -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)) }) }) diff --git a/src/autoscaler/db/db.go b/src/autoscaler/db/db.go index 411307c70b..86564fc91f 100644 --- a/src/autoscaler/db/db.go +++ b/src/autoscaler/db/db.go @@ -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 diff --git a/src/autoscaler/db/sqldb/binding_sqldb.go b/src/autoscaler/db/sqldb/binding_sqldb.go index ae5782a1e8..5f8bb1c1b4 100644 --- a/src/autoscaler/db/sqldb/binding_sqldb.go +++ b/src/autoscaler/db/sqldb/binding_sqldb.go @@ -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 { diff --git a/src/autoscaler/db/sqldb/binding_sqldb_test.go b/src/autoscaler/db/sqldb/binding_sqldb_test.go index a052424c77..0e78cce908 100644 --- a/src/autoscaler/db/sqldb/binding_sqldb_test.go +++ b/src/autoscaler/db/sqldb/binding_sqldb_test.go @@ -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}) @@ -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() { @@ -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)) }) @@ -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() { @@ -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() { @@ -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) }) @@ -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() { @@ -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() { @@ -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() { @@ -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()) }) @@ -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() { @@ -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() { diff --git a/src/autoscaler/eventgenerator/metric/fetcher_test.go b/src/autoscaler/eventgenerator/metric/fetcher_test.go index 1dbab11c4f..79324454b3 100644 --- a/src/autoscaler/eventgenerator/metric/fetcher_test.go +++ b/src/autoscaler/eventgenerator/metric/fetcher_test.go @@ -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"), ) }) })