Balancer - main.go and yamls #5643
Conversation
k8s-ci-robot
added
kind/feature
k8s-ci-robot
added
area/balancer
approved
|
/assign @pbetkier |
balancer/examples/nginx-hpa.yaml
Outdated
| @@ -0,0 +1,119 @@ | |||
| # | |||
| # Balancer scaling 2 deployments using priority policy and hpa. | |||
There was a problem hiding this comment.
The examples at the moment are pretty artificial: 2 identical deployments (just different labels and names). I think users could greatly benefit from more real-life examples like equal ratio between zones or overflow to other machine types. Even if an example would require tuning in a specific cluster deployment (e.g. modify zone names) I think it's still more useful than having users figure it out on their own.
There was a problem hiding this comment.
These examples can be deployed anywhere and work out of the box. If someone is using Balancer they are probably advanced enough to figure out node selectors on their own ;).
| func init() { | ||
| flag.StringVar(&kubeconfig, "kubeconfig", "", "Path to a kubeconfig. Only required if out-of-cluster.") | ||
| flag.StringVar(&masterURL, "master", "", "The address of the Kubernetes API server. Overrides any value in kubeconfig. Only required if out-of-cluster.") | ||
| flag.IntVar(&balancerReprocessPeriodSec, "reprocess-period-sec", 15, "How often (in second) balancers are processed") |
There was a problem hiding this comment.
Perhaps DurationVar instead of int in seconds and reconciliation-interval (or sync-period which I guess is how the option is typically called in k8s)?
| ) | ||
|
|
||
| var ( | ||
| masterURL string |
There was a problem hiding this comment.
I don't see masterURL being configurable for the other k8s.io/autoscaler components and all of them can be run out-of-cluster nicely. When looking at CA and VPA, I saw that both use BuildConfigFromFlags as well, but set masterURL: "" and don't allow for overriding it in the commandline. Do we have a good reason to deviate from this pattern? If not, I think it makes sense to have people configure the k8s.io/autoscaler components in the same way.
There was a problem hiding this comment.
This is quite handy if you try to run balancer locally for development (and communicate with cluster via kubectl proxy).
| serviceAccountName: balancer-controller | ||
| containers: | ||
| - name: controller | ||
| image: gcr.io/gke-autoscaling-gcr/balancer:0.1.1 |
There was a problem hiding this comment.
should we rather point to an official k8s repo here, or is this something we can only do after the fact when the code has been merged and released from the k8s repo for the first time?
There was a problem hiding this comment.
Let's update it after we do the official oss release.
mwielgus
force-pushed
the
balancer-main
branch
2 times, most recently
from
43a9af8 to
084b0b3
Compare
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mwielgus, pbetkier The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind feature
What this PR does / why we need it:
Adds main.go, Makefile, Dockerfile and setup and example yamls.
Which issue(s) this PR fixes:
None
Special notes for your reviewer: