diff --git a/src/acceptance/api/api_test.go b/src/acceptance/api/api_test.go index e975f1d747..fac1cbc285 100644 --- a/src/acceptance/api/api_test.go +++ b/src/acceptance/api/api_test.go @@ -315,7 +315,7 @@ var _ = Describe("AutoScaler Public API", func() { Expect(status).To(Equal(200)) }) // FIXME - XIt("should succeed to get a custom metrics strategy", func() { + It("should succeed to get a custom metrics strategy", func() { actualPolicy, status = getPolicy() Expect(string(actualPolicy)).Should(MatchJSON(expectedPolicy), "TODO: should not fail to get the policy") Expect(status).To(Equal(200)) @@ -331,7 +331,7 @@ var _ = Describe("AutoScaler Public API", func() { Expect(status).To(Equal(200)) }) - When("custom metrics is removed from the existing policy", func() { + When("custom metrics strategy is removed from the existing policy", func() { It("should removed the custom metrics strategy and displays policy only", func() { expectedPolicy = GenerateDynamicScaleOutPolicy(1, 2, "memoryused", 30) actualPolicy, status = createPolicy(expectedPolicy) diff --git a/src/autoscaler/api/publicapiserver/public_api_handler.go b/src/autoscaler/api/publicapiserver/public_api_handler.go index 70e4fa5ad6..e7fa5e1b0c 100644 --- a/src/autoscaler/api/publicapiserver/public_api_handler.go +++ b/src/autoscaler/api/publicapiserver/public_api_handler.go @@ -98,14 +98,53 @@ func (h *PublicApiHandler) GetScalingPolicy(w http.ResponseWriter, r *http.Reque writeErrorResponse(w, http.StatusInternalServerError, "Error retrieving scaling policy") return } - if scalingPolicy == nil { logger.Info("policy doesn't exist") writeErrorResponse(w, http.StatusNotFound, "Policy Not Found") return } + customMetricStrategy, err := h.bindingdb.GetCustomMetricStrategyByAppId(r.Context(), appId) + if err != nil { + logger.Error("Failed to retrieve service binding from database", err) + writeErrorResponse(w, http.StatusInternalServerError, "Error retrieving binding policy") + return + } + scalingPolicyResult, err := h.determineBindingConfigAndPolicy(scalingPolicy, customMetricStrategy) + if err != nil { + logger.Error("Failed to build policy and config response object: %w", err) + writeErrorResponse(w, http.StatusInternalServerError, "Error retrieving binding policy") + return + } + handlers.WriteJSONResponse(w, http.StatusOK, scalingPolicyResult) +} + +// TODO Move this centrally +func (h *PublicApiHandler) determineBindingConfigAndPolicy(scalingPolicy *models.ScalingPolicy, customMetricStrategy string) (interface{}, error) { + combinedConfig, bindingConfig := h.buildConfigurationIfPresent(customMetricStrategy) + if scalingPolicy != nil && combinedConfig != nil { //both are present + combinedConfig.ScalingPolicy = *scalingPolicy + combinedConfig.BindingConfig = *bindingConfig + return combinedConfig, nil + } else if scalingPolicy != nil { // only policy was given + return scalingPolicy, nil + } else if combinedConfig != nil { // only configuration was given //FIXME This should not be possible + return bindingConfig, nil + } + return scalingPolicy, fmt.Errorf("no policy or custom metrics strategy found") +} + +// TODO Move this centrally +func (h *PublicApiHandler) buildConfigurationIfPresent(customMetricsStrategy string) (*models.BindingConfigWithPolicy, *models.BindingConfig) { + var combinedConfig *models.BindingConfigWithPolicy + var bindingConfig *models.BindingConfig - handlers.WriteJSONResponse(w, http.StatusOK, scalingPolicy) + if customMetricsStrategy != "" && customMetricsStrategy != models.CustomMetricsSameApp { //if custom metric was given in the binding process + combinedConfig = &models.BindingConfigWithPolicy{} + bindingConfig = &models.BindingConfig{} + bindingConfig.SetCustomMetricsStrategy(customMetricsStrategy) + combinedConfig.BindingConfig = *bindingConfig + } + return combinedConfig, bindingConfig } func (h *PublicApiHandler) AttachScalingPolicy(w http.ResponseWriter, r *http.Request, vars map[string]string) { diff --git a/src/autoscaler/api/publicapiserver/public_api_handler_test.go b/src/autoscaler/api/publicapiserver/public_api_handler_test.go index 511a3a3c9c..8fb630c297 100644 --- a/src/autoscaler/api/publicapiserver/public_api_handler_test.go +++ b/src/autoscaler/api/publicapiserver/public_api_handler_test.go @@ -214,6 +214,7 @@ var _ = Describe("PublicApiHandler", func() { }) }) }) + Describe("GetScalingPolicy", func() { JustBeforeEach(func() { handler.GetScalingPolicy(resp, req, pathVariables) @@ -283,6 +284,44 @@ var _ = Describe("PublicApiHandler", func() { Expect(strings.TrimSpace(resp.Body.String())).To(Equal(`{"instance_min_count":1,"instance_max_count":5,"scaling_rules":[{"metric_type":"memoryused","breach_duration_secs":300,"threshold":30,"operator":"<","cool_down_secs":300,"adjustment":"-1"}],"schedules":{"timezone":"Asia/Kolkata","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}]}}`)) }) }) + Context("and custom metric strategy", func() { + When("custom metric strategy retrieval fails", func() { + BeforeEach(func() { + pathVariables["appId"] = TEST_APP_ID + setupPolicy(policydb) + bindingdb.GetCustomMetricStrategyByAppIdReturns("", fmt.Errorf("db error")) + }) + It("should fail with 500", func() { + Expect(resp.Code).To(Equal(http.StatusInternalServerError)) + Expect(resp.Body.String()).To(Equal(`{"code":"Internal Server Error","message":"Error retrieving binding policy"}`)) + }) + }) + When("custom metric strategy retrieved successfully", func() { + BeforeEach(func() { + pathVariables["appId"] = TEST_APP_ID + bindingdb.GetCustomMetricStrategyByAppIdReturns("bound_app", nil) + }) + When("custom metric strategy and policy are present", func() { + BeforeEach(func() { + setupPolicy(policydb) + }) + It("should return combined configuration with 200", func() { + Expect(resp.Code).To(Equal(http.StatusOK)) + Expect(resp.Body.String()).To(MatchJSON(validCustomMetricsConfigurationStr)) + }) + When("policy is present only", func() { + BeforeEach(func() { + setupPolicy(policydb) + bindingdb.GetCustomMetricStrategyByAppIdReturns("", nil) + }) + It("should return policy with 200", func() { + Expect(resp.Code).To(Equal(http.StatusOK)) + Expect(resp.Body.String()).To(MatchJSON(ValidPolicyStr)) + }) + }) + }) + }) + }) }) Describe("AttachScalingPolicy", func() { @@ -381,6 +420,7 @@ var _ = Describe("PublicApiHandler", func() { Expect(resp.Body.String()).To(ContainSubstring("Failed to read binding configuration request body")) }) }) + When("invalid configuration is provided with the policy", func() { BeforeEach(func() { pathVariables["appId"] = TEST_APP_ID @@ -412,11 +452,22 @@ var _ = Describe("PublicApiHandler", func() { req, _ = http.NewRequest(http.MethodPut, "", bytes.NewBufferString(validCustomMetricsConfigurationStr)) schedulerStatus = 200 }) - It("returns the policy and configuration with 201", func() { + It("returns the policy and configuration with 200", func() { Expect(resp.Code).To(Equal(http.StatusOK)) Expect(resp.Body).To(MatchJSON(validCustomMetricsConfigurationStr)) }) }) + When("configuration is removed but only policy is provided", func() { + BeforeEach(func() { + pathVariables["appId"] = TEST_APP_ID + req, _ = http.NewRequest(http.MethodPut, "", bytes.NewBufferString(ValidPolicyStr)) + schedulerStatus = 200 + }) + It("returns the policy 200", func() { + Expect(resp.Body).To(MatchJSON(ValidPolicyStr)) + Expect(resp.Code).To(Equal(http.StatusOK)) + }) + }) }) }) @@ -996,3 +1047,32 @@ var _ = Describe("PublicApiHandler", func() { }) }) + +func setupPolicy(policydb *fakes.FakePolicyDB) { + policydb.GetAppPolicyReturns(&models.ScalingPolicy{ + InstanceMax: 5, + InstanceMin: 1, + ScalingRules: []*models.ScalingRule{ + { + MetricType: "memoryused", + BreachDurationSeconds: 300, + CoolDownSeconds: 300, + Threshold: 30, + Operator: ">", + Adjustment: "-1", + }}, + Schedules: &models.ScalingSchedules{ + Timezone: "Asia/Kolkata", + RecurringSchedules: []*models.RecurringSchedule{ + { + StartTime: "10:00", + EndTime: "18:00", + DaysOfWeek: []int{1, 2, 3}, + ScheduledInstanceMin: 1, + ScheduledInstanceMax: 10, + ScheduledInstanceInit: 5, + }, + }, + }, + }, nil) +} diff --git a/src/autoscaler/models/policy.go b/src/autoscaler/models/policy.go index eb43c1b729..40b400c187 100644 --- a/src/autoscaler/models/policy.go +++ b/src/autoscaler/models/policy.go @@ -38,6 +38,9 @@ func (p *PolicyJson) GetAppPolicy() (*AppPolicy, error) { return &AppPolicy{AppId: p.AppId, ScalingPolicy: &scalingPolicy}, nil } +// ScalingPolicy is a customer facing entity and represents the scaling policy for an application. +// It can be manipulated by the user via the binding process and public API. If a change is required in the policy, +// think of other interaction points e.g., public api server. type ScalingPolicy struct { InstanceMin int `json:"instance_min_count"` InstanceMax int `json:"instance_max_count"`