jetstack / navigator

Managed Database-as-a-Service (DBaaS) on Kubernetes
Apache License 2.0
271 stars 31 forks source link

Configurable scheduler #276

Closed kragniz closed 6 years ago

kragniz commented 6 years ago

Fixes #235

This allows users to configure the scheduler for pods created via navigator's statefulsets. It's a fairly simple passthrough of the value to the statefulset spec.

There are two options exposed, one in the cluster spec, and the other in the nodepool spec. The nodepool value takes precedent, falling back to the cluster value if it is defined, then the default scheduler.

For example, these (simplified) are equivalent:

kind: "CassandraCluster"              
metadata:                             
  name: "example"                    
spec:                                 
  nodePools:                          
  - name: "ringnodes-1"        
    schedulerName: "fancy-scheduler"
  - name: "ringnodes-2"        
    schedulerName: "fancy-scheduler"
kind: "CassandraCluster"              
metadata:                             
  name: "example"                    
spec:                                 
  schedulerName: "fancy-scheduler"              
  nodePools:                          
  - name: "ringnodes-1"        
  - name: "ringnodes-2"        
Allow configuring scheduler
jetstack-ci-bot commented 6 years ago

@kragniz PR needs rebase

munnerz commented 6 years ago

I'm not convinced of the benefit of allowing default schedulername on the cluster. What if you change the default schedulername and then add another nodepool. I'd expect the current nodepools to also adopt the new default, but they'll be left with the original default.

This is a good point - we need to be careful with how we validate changes if we have this top level field. Perhaps we can remove it for now, and in future add it if required?

kragniz commented 6 years ago

Yep, makes sense to me. I'll remove in the next patchset

jetstack-ci-bot commented 6 years ago

@kragniz PR needs rebase

kragniz commented 6 years ago

I've made the update validation for nodePools a whitelist of changes in #283, so with those patches SchedulerName will not be mutable.

jetstack-bot commented 6 years ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wallrj

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files: - ~~[OWNERS](https://github.com/jetstack/navigator/blob/master/OWNERS)~~ [wallrj] You can indicate your approval by writing `/approve` in a comment You can cancel your approval by writing `/approve cancel` in a comment
jetstack-ci-bot commented 6 years ago

/test all [submit-queue is verifying that this PR is safe to merge]

jetstack-ci-bot commented 6 years ago

Automatic merge from submit-queue.