-
Notifications
You must be signed in to change notification settings - Fork 51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(CustomMetrics): Allow Different App to Send Custom Metrics #3211
base: main
Are you sure you want to change the base?
Changes from all commits
f00158b
ec6036b
58e1977
12f1688
a554409
8e4bf83
1cd4ac9
979581a
dec3f28
3e161f2
a373e94
5b1e98f
8e785c2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 |
---|---|---|
@@ -0,0 +1,3 @@ | ||
<% if_p("autoscaler.binding_db.tls.certificate") do |value| %> | ||
<%= value %> | ||
<% end %> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
<% if_p("autoscaler.binding_db.tls.private_key") do |value| %> | ||
<%= value %> | ||
<% end %> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
<% if_p("autoscaler.binding_db.tls.ca") do |value| %> | ||
<%= value %> | ||
<% end %> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,75 +4,140 @@ import ( | |
"acceptance" | ||
"acceptance/config" | ||
. "acceptance/helpers" | ||
"fmt" | ||
"time" | ||
|
||
. "github.com/onsi/ginkgo/v2" | ||
. "github.com/onsi/gomega" | ||
) | ||
|
||
var _ = Describe("AutoScaler custom metrics policy", func() { | ||
var _ = Describe("AutoScaler custom metrics", func() { | ||
var ( | ||
policy string | ||
err error | ||
) | ||
BeforeEach(func() { | ||
policy = GenerateDynamicScaleOutAndInPolicy(1, 2, "test_metric", 500, 500) | ||
appName = CreateTestApp(cfg, "node-custom-metric", 1) | ||
appGUID, err = GetAppGuid(cfg, appName) | ||
|
||
appToScaleName = CreateTestApp(cfg, "go-custom-metric", 1) | ||
appToScaleGUID, err = GetAppGuid(cfg, appToScaleName) | ||
Expect(err).NotTo(HaveOccurred()) | ||
instanceName = CreatePolicy(cfg, appName, appGUID, policy) | ||
StartApp(appName, cfg.CfPushTimeoutDuration()) | ||
|
||
}) | ||
AfterEach(AppAfterEach) | ||
|
||
// This test will fail if credential-type is set to X509 in autoscaler broker. | ||
// Therefore, only mtls connection will be supported for custom metrics in future | ||
Context("when scaling by custom metrics", func() { | ||
It("should scale out and scale in", Label(acceptance.LabelSmokeTests), func() { | ||
By("Scale out to 2 instances") | ||
scaleOut := sendMetricToAutoscaler(cfg, appGUID, appName, 550, false) | ||
Eventually(scaleOut). | ||
WithTimeout(5 * time.Minute). | ||
WithPolling(15 * time.Second). | ||
Should(Equal(2)) | ||
|
||
By("Scale in to 1 instances") | ||
scaleIn := sendMetricToAutoscaler(cfg, appGUID, appName, 100, false) | ||
Eventually(scaleIn). | ||
WithTimeout(5 * time.Minute). | ||
WithPolling(15 * time.Second). | ||
Should(Equal(1)) | ||
Describe("custom metrics policy for same app", func() { | ||
BeforeEach(func() { | ||
policy = GenerateDynamicScaleOutAndInPolicy(1, 2, "test_metric", 500, 500) | ||
instanceName = CreatePolicy(cfg, appToScaleName, appToScaleGUID, policy) | ||
StartApp(appToScaleName, cfg.CfPushTimeoutDuration()) | ||
}) | ||
// This test will fail if credential-type is set to X509 in autoscaler broker. | ||
// Therefore, only mtls connection will be supported for custom metrics in future | ||
Context("when scaling by custom metrics", func() { | ||
BeforeEach(func() { | ||
//instanceName = CreatePolicy(cfg, appToScaleName, appToScaleGUID, policy) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we might want to remove this commented test There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we remove this comment? |
||
//StartApp(appToScaleName, cfg.CfPushTimeoutDuration()) | ||
}) | ||
It("should scale out and scale in", Label(acceptance.LabelSmokeTests), func() { | ||
By("Scale out to 2 instances") | ||
scaleOut := sendMetricToAutoscaler(cfg, appToScaleGUID, appToScaleName, 550, false) | ||
Eventually(scaleOut). | ||
WithTimeout(5 * time.Minute). | ||
WithPolling(15 * time.Second). | ||
Should(Equal(2)) | ||
|
||
By("Scale in to 1 instances") | ||
scaleIn := sendMetricToAutoscaler(cfg, appToScaleGUID, appToScaleName, 100, false) | ||
Eventually(scaleIn). | ||
WithTimeout(5 * time.Minute). | ||
WithPolling(15 * time.Second). | ||
Should(Equal(1)) | ||
|
||
}) | ||
}) | ||
|
||
Context("when scaling by custom metrics via mtls", func() { | ||
BeforeEach(func() { | ||
//instanceName = CreatePolicy(cfg, appToScaleName, appToScaleGUID, policy) | ||
//StartApp(appToScaleName, cfg.CfPushTimeoutDuration()) | ||
}) | ||
It("should scale out and scale in", Label(acceptance.LabelSmokeTests), func() { | ||
By("Scale out to 2 instances") | ||
scaleOut := sendMetricToAutoscaler(cfg, appToScaleGUID, appToScaleName, 550, true) | ||
Eventually(scaleOut). | ||
WithTimeout(5 * time.Minute). | ||
WithPolling(15 * time.Second). | ||
Should(Equal(2)) | ||
|
||
By("Scale in to 1 instance") | ||
scaleIn := sendMetricToAutoscaler(cfg, appToScaleGUID, appToScaleName, 100, true) | ||
Eventually(scaleIn). | ||
WithTimeout(5 * time.Minute). | ||
WithPolling(15 * time.Second). | ||
Should(Equal(1)) | ||
}) | ||
}) | ||
}) | ||
|
||
Context("when scaling by custom metrics via mtls", func() { | ||
It("should scale out and scale in", Label(acceptance.LabelSmokeTests), func() { | ||
By("Scale out to 2 instances") | ||
scaleOut := sendMetricToAutoscaler(cfg, appGUID, appName, 550, true) | ||
Eventually(scaleOut). | ||
WithTimeout(5 * time.Minute). | ||
WithPolling(15 * time.Second). | ||
Should(Equal(2)) | ||
|
||
By("Scale in to 1 instance") | ||
scaleIn := sendMetricToAutoscaler(cfg, appGUID, appName, 100, true) | ||
Eventually(scaleIn). | ||
WithTimeout(5 * time.Minute). | ||
WithPolling(15 * time.Second). | ||
Should(Equal(1)) | ||
Describe("Custom metrics with producer app", func() { | ||
BeforeEach(func() { | ||
// attach policy to appToScale B | ||
policy = GenerateBindingsWithScalingPolicy("bound_app", 1, 2, "test_metric", 100, 500) | ||
instanceName = CreatePolicy(cfg, appToScaleName, appToScaleGUID, policy) | ||
StartApp(appToScaleName, cfg.CfPushTimeoutDuration()) | ||
|
||
// push producer app without policy | ||
metricProducerAppName = CreateTestApp(cfg, "go-custom_metric_producer-app", 1) | ||
metricProducerAppGUID, err = GetAppGuid(cfg, metricProducerAppName) | ||
Expect(err).NotTo(HaveOccurred()) | ||
err := BindServiceToAppWithPolicy(cfg, metricProducerAppName, instanceName, "") | ||
Expect(err).NotTo(HaveOccurred()) | ||
StartApp(metricProducerAppName, cfg.CfPushTimeoutDuration()) | ||
|
||
}) | ||
Context("producer app A sends custom metrics for appToScale B via mtls", func() { | ||
When("policy is attached with the appToScale B with bound_app", func() { | ||
BeforeEach(func() { | ||
policy = GenerateBindingsWithScalingPolicy("bound_app", 1, 2, "test_metric", 100, 500) | ||
}) | ||
It("should scale out and scale in app B", Label(acceptance.LabelSmokeTests), func() { | ||
By(fmt.Sprintf("Scale out %s to 2 instance", appToScaleName)) | ||
scaleOut := sendMetricToAutoscaler(cfg, appToScaleGUID, metricProducerAppName, 550, true) | ||
Eventually(scaleOut). | ||
WithTimeout(5 * time.Minute). | ||
WithPolling(15 * time.Second). | ||
Should(Equal(2)) | ||
|
||
By(fmt.Sprintf("Scale in %s to 1 instance", appToScaleName)) | ||
scaleIn := sendMetricToAutoscaler(cfg, appToScaleGUID, metricProducerAppName, 80, true) | ||
Eventually(scaleIn). | ||
WithTimeout(5 * time.Minute). | ||
WithPolling(15 * time.Second). | ||
Should(Equal(1)) | ||
|
||
}) | ||
}) | ||
}) | ||
Context("appToScale B tries to send metrics for producer app A with strategy same_app", func() { | ||
BeforeEach(func() { | ||
policy = GenerateBindingsWithScalingPolicy("same_app", 1, 2, "test_metric", 100, 500) | ||
}) | ||
It("should not scale producer app", func() { | ||
By(fmt.Sprintf("Fail Scale %s ", metricProducerAppName)) | ||
sendMetricToAutoscaler(cfg, metricProducerAppGUID, appToScaleName, 550, true) | ||
WaitForNInstancesRunning(metricProducerAppGUID, 1, 5*time.Second, "expected 1 instance running") | ||
}) | ||
}) | ||
}) | ||
}) | ||
|
||
func sendMetricToAutoscaler(config *config.Config, appGUID string, appName string, metricThreshold int, mtls bool) func() (int, error) { | ||
func sendMetricToAutoscaler(config *config.Config, appToScaleGUID string, metricProducerAppName string, metricThreshold int, mtls bool) func() (int, error) { | ||
return func() (int, error) { | ||
if mtls { | ||
SendMetricMTLS(config, appName, metricThreshold) | ||
SendMetricMTLS(config, appToScaleGUID, metricProducerAppName, metricThreshold) | ||
} else { | ||
SendMetric(config, appName, metricThreshold) | ||
SendMetric(config, metricProducerAppName, metricThreshold) | ||
} | ||
return RunningInstances(appGUID, 5*time.Second) | ||
return RunningInstances(appToScaleGUID, 5*time.Second) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Considering we have added a new database for the metrics forwarder, we need to ensure that the MTA deployment is functioning correctly: https://github.com/cloudfoundry/app-autoscaler-release/actions/runs/11464983060/job/31903137875?pr=3211. Currently, it seems to be failing due to a different error.
We will need to add the bindingDB tag in the PostgreSQL user-provided service: https://github.com/cloudfoundry/app-autoscaler-release/blob/main/src/autoscaler/mta.tpl.yaml#L43.
Additionally, we must check the metrics forwarder configuration to ensure we are loading the database when running on Cloud Foundry as well: https://github.com/cloudfoundry/app-autoscaler-release/blob/main/src/autoscaler/metricsforwarder/config/config.go#L124.cloudfoundry/app-autoscaler-release/blob/main/src/autoscaler/metricsforwarder/config/config.go#L124
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@asalan316 this should fix it: a373e94