add app-autoscaler to scf #1594

qibobo · 2018-06-13T08:54:32Z

No description provided.

mook-as

Some minor changes.

How would we best test this? It might be worthwhile resurrecting the old autoscaler test we had, but it's also quite probable that things have changed so much it would make more sense to just write a new one and put it somewhere. In general, though, getting the computer to test things for us means we don't have to think about it ever again 😀

mook-as · 2018-06-13T17:59:11Z

container-host-files/etc/scf/config/role-manifest.yml

+      protocol: TCP
+      external: 5432
+      internal: 5432
+      public: true


Does this need to be public? I'd think only things internal to the cluster need to connect to postgres (and not random things on the internet).

mook-as · 2018-06-13T17:59:53Z

container-host-files/etc/scf/config/role-manifest.yml

+    release_name: postgres
+  processes:
+  - name: postgres
+  tags: [headless]


This being headless is… unexpected, but okay. I'd have thought clients would need a fixed hostname to use to connect to the database.

Ah, you use the -set hostname to connect to it (and rely on that round-robin DNS). Okay. Have you tried it with multiple replicas of this?

mook-as · 2018-06-13T18:00:37Z

container-host-files/etc/scf/config/role-manifest.yml

+      public: true
+    healthcheck:
+      readiness:
+- name: asapi


It might be clearer to call this scaler-api or something along those lines? Similar comments apply to all the other roles.

mook-as · 2018-06-13T18:01:30Z

container-host-files/etc/scf/config/role-manifest.yml

@@ -1534,6 +1534,176 @@ roles:
    memory: 256
    virtual-cpus: 1
    exposed-ports: []
+
+# cf app-autoscaler
+- name: autoscalerpostgres


The name is a little unreadable, but at least still unambiguous… a dash might make it better, assuming it works: autoscaler-postgres.

mook-as · 2018-06-13T18:03:20Z

container-host-files/etc/scf/config/role-manifest.yml

+      max: 3
+      ha: 2
+    capabilities: []
+    persistent-volumes:


Consider using the new format:

volumes: - path: /var/vcap/store type: persistent tag: postgres-data size: 100

mook-as · 2018-06-13T18:20:06Z

container-host-files/etc/scf/config/role-manifest.yml

+
+  configuration:
+    templates:
+      properties.route_registrar.routes: '[{"name":"autoscalerapiserver", "port":((AUTOSCALER_API_SERVER_PUBLIC_PORT)), "tags":{"component":"autoscalerapiserver"}, "uris":["autoscaler.((DOMAIN))"], "registration_interval":"10s"},{"name":"autoscalerservicebroker", "port":((AUTOSCALER_SERVICE_BROKER_PUBLIC_PORT)), "tags":{"component":"autoscalerservicebroker"}, "uris":["autoscalerservicebroker.((DOMAIN))"], "registration_interval":"10s"}]'      


This may be easier to read when written in the folded style, but it'll be harder to move around. Up to you.

mook-as · 2018-06-13T18:36:18Z

container-host-files/etc/scf/config/role-manifest.yml

+    default: 0
+    description: ""
+  - name: AUTOSCALER_API_SERVER_HOST
+    default: "asapi"


I don't think this should actually be user-configurable (since the role name is hard coded in the role manifest), right? Similarly for a bunch of other host names.

mook-as · 2018-06-13T18:37:03Z

container-host-files/etc/scf/config/role-manifest.yml

@@ -2574,8 +3275,57 @@ configuration:
    properties.acceptance_tests_brain.domain: '"((DOMAIN))"'
    properties.acceptance_tests_brain.password: '"((CLUSTER_ADMIN_PASSWORD))"'
    properties.acceptance_tests_brain.tcp_domain: '"((TCP_DOMAIN))"'
+    properties.api_server.ca_cert: '"((INTERNAL_CA_CERT))"'


Seeing as the property name is a bit generic (what api_server?), it might make more sense to put it in the asapi role?

mook-as · 2018-06-13T18:39:10Z

container-host-files/etc/scf/config/role-manifest.yml

@@ -2661,8 +3411,15 @@ configuration:
    properties.cf-usb.mysql_address: 'mysql-proxy.((KUBERNETES_NAMESPACE)).svc.((KUBERNETES_CLUSTER_DOMAIN))'
    properties.cf-usb.mysql_password: "((MYSQL_CF_USB_PASSWORD))"
    properties.cf.admin_password: '"((CLUSTER_ADMIN_PASSWORD))"'
+    properties.cf.api: '"((AUTOSCALER_CF_SCHEME))://((AUTOSCALER_CF_HOSTNAME)).((DOMAIN)):((AUTOSCALER_CF_PORT))"'


These things that aren't shared across the rest of the deployment should go in role-specific configuration as well. Maybe even use YAML magic to merge them if they're used across multiple roles (that are all autoscaler).

mook-as · 2018-06-13T22:16:12Z

container-host-files/etc/scf/config/role-manifest.yml

+    memory: 256
+    virtual-cpus: 2
+    exposed-ports:
+    - name: apiport #apiserverport


Calling this api-port might be clearer. Or possibly drop the -port suffix, if the port name becomes too long: api-server.

qibobo · 2018-06-15T02:27:35Z

@mook-as Thank you for you comments. We will update the PR later.

qibobo · 2018-06-25T09:26:59Z

@mook-as updated the pr.

mook-as

Thanks for the updates! There's still no indication of how I can test this to confirm that it works though. I'll try to see if I can hack something up…

mook-as · 2018-06-25T23:46:44Z

container-host-files/etc/scf/config/role-manifest.yml

+    description: ""
+  - name: AUTOSCALER_SERVICE_BROKER_PASSWORD
+    default: "password"
+    description: ""


This should be using a password generator

mook-as · 2018-06-25T23:47:18Z

container-host-files/etc/scf/config/role-manifest.yml

+    properties.autoscaler.api_server.service_broker.ca_cert: '"((INTERNAL_CA_CERT))"'
+    properties.autoscaler.api_server.service_broker.client_cert: '"((AUTOSCALER_ASAPI_CLIENT_CERT))"'
+    properties.autoscaler.api_server.service_broker.client_key: '"((AUTOSCALER_ASAPI_CLIENT_KEY))"'
+    properties.autoscaler.api_server.service_broker.host: '"autoscaler-api.((KUBERNETES_NAMESPACE)).svc.((KUBERNETES_CLUSTER_DOMAIN))"'


I believe nothing actually registers the broker to CF, right? Shouldn't it?

@mook-as The service broker should be registered to CF as below:
cf create-service-broker autoscaler

I actually needed cf create-service-broker autoscaler p('autoscaler.service_broker.username') p('autoscaler.service_broker.password') http://<%= p('autoscaler.service_broker.api_server.host') %>:<%= p('autoscaler.service_broker.api_server.port') %>, but yes.

I'm trying to get some tests in so that we don't accidentally break things; is there a smoke test for the autoscaler available? I got the acceptance tests integrated into SCF (will PR soon), but that took an hour on my machine. I'm looking for something more like one minute or less.

qibobo · 2018-06-26T01:17:32Z

@mook-as
this is the test guide of app-autoscaler: https://github.com/cloudfoundry-incubator/app-autoscaler-release/blob/master/src/acceptance/README.md

mook-as · 2018-06-27T00:13:14Z

Hi! Thanks for the changes. I made a PR into your branch to implement a smoke test of sorts, to ensure that we don't accidentally break the autoscaler in the future. Three minutes was a bit longer than what I was looking for, but it looks like the default configuration doesn't let me lower it more. Hopefully that shows you the sort of thing I'm looking for :)

Note that we might, in the future, end up defaulting the autoscaler to be opt-in (as in, zero instances of the relevant pods by default); that depends on how the vagrant boxes respond to having more things running on them — they're already a bit sluggish running CATS. But that will happen in a subsequent PR.

qibobo · 2018-06-27T01:30:45Z

@mook-as
cf create-service-broker autoscaler p('autoscaler.service_broker.username') p('autoscaler.service_broker.password') http://<%= p('autoscaler.service_broker.api_server.host') %>:<%= p('autoscaler.service_broker.api_server.port') %>

Actually we should not use "autoscaler.service_broker.api_server.host" because it is the autoscaler apiserver host. But as apiserver and servicebroker are deployed in the same pod, it is ok.
And it is better to use cf create-service-broker autoscaler {username} {password} https://autoscalerservicebroker.cf-dev.io as the autoscaler service broker has been registered to router.

And for acceptance test, to run all the tests parallel it will take 20-40 minutes, and if serially, it will take more time.
The acceptance tests can be divided into 3 types:

1. service binding&unbinding
It takes about 2 or 3 minutes to test create service instance, bind service instance to an application, unbind service instance from the application and delete service instance.
2. public api
It takes about 5 minutes to test all the public api of autoscaler.
3. scaling in/out
It takes 20-30 minutes(run parallel) to test all the scaling in/out features.

As you mentioned, you want a test only takes a short time, I think you could just test the binding&unbinding.

mook-as · 2018-06-27T17:04:40Z

@qibobo Thanks for the feedback! I'll update qibobo/scf#1 with your comments.

* scf-release: Add smoke tests for autoscaler This takes about 3 minutes to run. * Jenkinsfile: Run autoscaler smoke test by default * Role manifest: update autoscaler-smoke - Use external API server for autoscaler - Drop unused environment script - Drop limits on role (see #1618 for details) * autoscaler: update smoke tests for review feedback - Call the property …service_broker_endpoint instead of …api - Update example to the correct value

mook-as requested changes Jun 13, 2018

View reviewed changes

viovanov added the docs-update 📃 Changes here require an update to the docs label Jun 15, 2018

mook-as reviewed Jun 25, 2018

View reviewed changes

mook-as requested changes Jun 25, 2018

View reviewed changes

mook-as mentioned this pull request Jun 26, 2018

Add acceptance tests as a bosh task cloudfoundry/app-autoscaler-release#124

Closed

qibobo and others added 3 commits June 29, 2018 14:18

add app-autoscaler to scf

7cddf0c

password is secret

5b7a731

mook-as approved these changes Jun 30, 2018

View reviewed changes

mook-as merged commit 8c43eae into SUSE:develop Jun 30, 2018

gaktive mentioned this pull request Jul 27, 2018

App Autoscaler SUSE/doc-cap#120

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add app-autoscaler to scf #1594

add app-autoscaler to scf #1594

qibobo commented Jun 13, 2018

mook-as left a comment

mook-as Jun 13, 2018

mook-as Jun 13, 2018

mook-as Jun 13, 2018

mook-as Jun 13, 2018

mook-as Jun 13, 2018

mook-as Jun 13, 2018

mook-as Jun 13, 2018

mook-as Jun 13, 2018

mook-as Jun 13, 2018

mook-as Jun 13, 2018

mook-as Jun 13, 2018

qibobo commented Jun 15, 2018

qibobo commented Jun 25, 2018

mook-as left a comment

mook-as Jun 25, 2018

mook-as Jun 25, 2018

qibobo Jun 26, 2018

mook-as Jun 26, 2018

qibobo commented Jun 26, 2018

mook-as commented Jun 27, 2018

qibobo commented Jun 27, 2018

mook-as commented Jun 27, 2018

add app-autoscaler to scf #1594

add app-autoscaler to scf #1594

Conversation

qibobo commented Jun 13, 2018

mook-as left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qibobo commented Jun 15, 2018

qibobo commented Jun 25, 2018

mook-as left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qibobo commented Jun 26, 2018

mook-as commented Jun 27, 2018

qibobo commented Jun 27, 2018

mook-as commented Jun 27, 2018