From 2c05a392b09186eaae968b37adc62ebf4045d2d2 Mon Sep 17 00:00:00 2001 From: Louis Taylor Date: Tue, 15 May 2018 16:56:44 +0100 Subject: [PATCH 1/7] Check statefulset vs nodepool resources --- pkg/apis/navigator/validation/cassandra.go | 4 ++ pkg/controllers/cassandra/cluster_control.go | 47 ++++++++++++++++++-- 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/pkg/apis/navigator/validation/cassandra.go b/pkg/apis/navigator/validation/cassandra.go index cf17c800d..da9683ea5 100644 --- a/pkg/apis/navigator/validation/cassandra.go +++ b/pkg/apis/navigator/validation/cassandra.go @@ -76,11 +76,15 @@ func ValidateCassandraClusterUpdate(old, new *navigator.CassandraCluster) field. restorePersistence := newNp.Persistence newNp.Persistence = oldNp.Persistence + restoreResources := newNp.Resources + newNp.Resources = oldNp.Resources + if !reflect.DeepEqual(newNp, oldNp) { allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), "updates to nodepool for fields other than 'replicas' and 'persistence' are forbidden.")) } newNp.Replicas = restoreReplicas newNp.Persistence = restorePersistence + newNp.Resources = restoreResources break } diff --git a/pkg/controllers/cassandra/cluster_control.go b/pkg/controllers/cassandra/cluster_control.go index a3ec9ad78..9868b0bb6 100644 --- a/pkg/controllers/cassandra/cluster_control.go +++ b/pkg/controllers/cassandra/cluster_control.go @@ -1,8 +1,11 @@ package cassandra import ( + "fmt" + "github.com/golang/glog" apiv1 "k8s.io/api/core/v1" + "k8s.io/client-go/listers/apps/v1beta1" "k8s.io/client-go/tools/record" v1alpha1 "github.com/jetstack/navigator/pkg/apis/navigator/v1alpha1" @@ -15,6 +18,8 @@ import ( "github.com/jetstack/navigator/pkg/controllers/cassandra/seedlabeller" "github.com/jetstack/navigator/pkg/controllers/cassandra/service" "github.com/jetstack/navigator/pkg/controllers/cassandra/serviceaccount" + "github.com/jetstack/navigator/pkg/controllers/cassandra/util" + "github.com/jetstack/navigator/pkg/util/resources" ) const ( @@ -170,7 +175,11 @@ func (e *defaultCassandraClusterControl) Sync(c *v1alpha1.CassandraCluster) erro return err } - a := NextAction(c) + a, err := NextAction(c, e.state.StatefulSetLister) + if err != nil { + return err + } + if a != nil { err = a.Execute(e.state) if err != nil { @@ -194,14 +203,14 @@ func (e *defaultCassandraClusterControl) Sync(c *v1alpha1.CassandraCluster) erro return nil } -func NextAction(c *v1alpha1.CassandraCluster) controllers.Action { +func NextAction(c *v1alpha1.CassandraCluster, statefulSetLister v1beta1.StatefulSetLister) (controllers.Action, error) { for _, np := range c.Spec.NodePools { _, found := c.Status.NodePools[np.Name] if !found { return &actions.CreateNodePool{ Cluster: c, NodePool: &np, - } + }, nil } } for _, np := range c.Spec.NodePools { @@ -210,8 +219,38 @@ func NextAction(c *v1alpha1.CassandraCluster) controllers.Action { return &actions.ScaleOut{ Cluster: c, NodePool: &np, + }, nil + } + + statefulSetName := util.NodePoolResourceName(c, &np) + ss, err := statefulSetLister.StatefulSets(c.Namespace).Get(statefulSetName) + if err != nil { + return nil, err + } + + var container *apiv1.Container + for i, _ := range ss.Spec.Template.Spec.Containers { + if ss.Spec.Template.Spec.Containers[i].Name == "cassandra" { + container = &ss.Spec.Template.Spec.Containers[i] } } + + if container == nil { + return nil, fmt.Errorf("unable to find cassandra container in StatefulSet %s/%s", + ss.Namespace, ss.Name, + ) + } + + glog.Warningf("requirements: %v", container.Resources) + if !resources.RequirementsEqual(container.Resources, np.Resources) { + return &actions.SetResources{ + Cluster: c, + NodePool: &np, + }, nil + } else { + glog.Warningf("requirementsEqual") + } + } - return nil + return nil, nil } From 23c8a36b47c35ee80a975be8fa5e5435abbe695a Mon Sep 17 00:00:00 2001 From: Louis Taylor Date: Tue, 15 May 2018 16:57:28 +0100 Subject: [PATCH 2/7] Add resources to nodepool status --- pkg/apis/navigator/types.go | 1 + pkg/apis/navigator/v1alpha1/types.go | 3 +++ 2 files changed, 4 insertions(+) diff --git a/pkg/apis/navigator/types.go b/pkg/apis/navigator/types.go index 107095b01..71f389bd6 100644 --- a/pkg/apis/navigator/types.go +++ b/pkg/apis/navigator/types.go @@ -51,6 +51,7 @@ type CassandraClusterStatus struct { type CassandraClusterNodePoolStatus struct { ReadyReplicas int32 + Resources v1.ResourceRequirements } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object diff --git a/pkg/apis/navigator/v1alpha1/types.go b/pkg/apis/navigator/v1alpha1/types.go index 11ffd3cc6..c3cadeed1 100644 --- a/pkg/apis/navigator/v1alpha1/types.go +++ b/pkg/apis/navigator/v1alpha1/types.go @@ -114,6 +114,9 @@ type CassandraClusterStatus struct { type CassandraClusterNodePoolStatus struct { // The number of replicas in the node pool that are currently 'Ready'. ReadyReplicas int32 `json:"readyReplicas"` + + // The applied resource requirements for this nodepool + Resources v1.ResourceRequirements `json:"resources,omitempty"` } // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object From 5f2a7eff991f067d620d08e4919f7dbcca98f159 Mon Sep 17 00:00:00 2001 From: Louis Taylor Date: Tue, 15 May 2018 16:57:55 +0100 Subject: [PATCH 3/7] Run make generate --- pkg/apis/navigator/v1alpha1/zz_generated.conversion.go | 2 ++ pkg/apis/navigator/v1alpha1/zz_generated.deepcopy.go | 5 ++++- pkg/apis/navigator/zz_generated.deepcopy.go | 5 ++++- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/pkg/apis/navigator/v1alpha1/zz_generated.conversion.go b/pkg/apis/navigator/v1alpha1/zz_generated.conversion.go index 9cabdb11e..c560e9e9b 100644 --- a/pkg/apis/navigator/v1alpha1/zz_generated.conversion.go +++ b/pkg/apis/navigator/v1alpha1/zz_generated.conversion.go @@ -197,6 +197,7 @@ func Convert_navigator_CassandraClusterNodePool_To_v1alpha1_CassandraClusterNode func autoConvert_v1alpha1_CassandraClusterNodePoolStatus_To_navigator_CassandraClusterNodePoolStatus(in *CassandraClusterNodePoolStatus, out *navigator.CassandraClusterNodePoolStatus, s conversion.Scope) error { out.ReadyReplicas = in.ReadyReplicas + out.Resources = in.Resources return nil } @@ -207,6 +208,7 @@ func Convert_v1alpha1_CassandraClusterNodePoolStatus_To_navigator_CassandraClust func autoConvert_navigator_CassandraClusterNodePoolStatus_To_v1alpha1_CassandraClusterNodePoolStatus(in *navigator.CassandraClusterNodePoolStatus, out *CassandraClusterNodePoolStatus, s conversion.Scope) error { out.ReadyReplicas = in.ReadyReplicas + out.Resources = in.Resources return nil } diff --git a/pkg/apis/navigator/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/navigator/v1alpha1/zz_generated.deepcopy.go index f57b2d88c..437c7f86a 100644 --- a/pkg/apis/navigator/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/navigator/v1alpha1/zz_generated.deepcopy.go @@ -152,6 +152,7 @@ func (in *CassandraClusterNodePool) DeepCopy() *CassandraClusterNodePool { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *CassandraClusterNodePoolStatus) DeepCopyInto(out *CassandraClusterNodePoolStatus) { *out = *in + in.Resources.DeepCopyInto(&out.Resources) return } @@ -206,7 +207,9 @@ func (in *CassandraClusterStatus) DeepCopyInto(out *CassandraClusterStatus) { in, out := &in.NodePools, &out.NodePools *out = make(map[string]CassandraClusterNodePoolStatus, len(*in)) for key, val := range *in { - (*out)[key] = val + newVal := new(CassandraClusterNodePoolStatus) + val.DeepCopyInto(newVal) + (*out)[key] = *newVal } } return diff --git a/pkg/apis/navigator/zz_generated.deepcopy.go b/pkg/apis/navigator/zz_generated.deepcopy.go index 6a90f2e70..71faa325b 100644 --- a/pkg/apis/navigator/zz_generated.deepcopy.go +++ b/pkg/apis/navigator/zz_generated.deepcopy.go @@ -152,6 +152,7 @@ func (in *CassandraClusterNodePool) DeepCopy() *CassandraClusterNodePool { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *CassandraClusterNodePoolStatus) DeepCopyInto(out *CassandraClusterNodePoolStatus) { *out = *in + in.Resources.DeepCopyInto(&out.Resources) return } @@ -206,7 +207,9 @@ func (in *CassandraClusterStatus) DeepCopyInto(out *CassandraClusterStatus) { in, out := &in.NodePools, &out.NodePools *out = make(map[string]CassandraClusterNodePoolStatus, len(*in)) for key, val := range *in { - (*out)[key] = val + newVal := new(CassandraClusterNodePoolStatus) + val.DeepCopyInto(newVal) + (*out)[key] = *newVal } } return From 80df1d5df4962998cfdf54e2a72edf3ae7152738 Mon Sep 17 00:00:00 2001 From: Louis Taylor Date: Tue, 15 May 2018 17:16:05 +0100 Subject: [PATCH 4/7] Update nodepool resource request status --- pkg/controllers/cassandra/nodepool/nodepool.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/pkg/controllers/cassandra/nodepool/nodepool.go b/pkg/controllers/cassandra/nodepool/nodepool.go index 044b088f8..67b6cc3b3 100644 --- a/pkg/controllers/cassandra/nodepool/nodepool.go +++ b/pkg/controllers/cassandra/nodepool/nodepool.go @@ -1,6 +1,7 @@ package nodepool import ( + apiv1 "k8s.io/api/core/v1" "k8s.io/client-go/kubernetes" appslisters "k8s.io/client-go/listers/apps/v1beta1" "k8s.io/client-go/tools/record" @@ -48,6 +49,18 @@ func (e *defaultCassandraClusterNodepoolControl) updateStatus(cluster *v1alpha1. npName := ss.Labels[v1alpha1.CassandraNodePoolNameLabel] nps := cluster.Status.NodePools[npName] nps.ReadyReplicas = ss.Status.ReadyReplicas + + var container *apiv1.Container + for i, _ := range ss.Spec.Template.Spec.Containers { + if ss.Spec.Template.Spec.Containers[i].Name == "cassandra" { + container = &ss.Spec.Template.Spec.Containers[i] + } + } + + if container != nil { + nps.Resources = container.Resources + } + cluster.Status.NodePools[npName] = nps } return nil From fa6781dcd2978daed7884a9734d368f428604871 Mon Sep 17 00:00:00 2001 From: Louis Taylor Date: Tue, 15 May 2018 17:44:39 +0100 Subject: [PATCH 5/7] Check resources based on nodepool status --- pkg/controllers/cassandra/cluster_control.go | 40 +++----------------- 1 file changed, 6 insertions(+), 34 deletions(-) diff --git a/pkg/controllers/cassandra/cluster_control.go b/pkg/controllers/cassandra/cluster_control.go index 9868b0bb6..740777e3c 100644 --- a/pkg/controllers/cassandra/cluster_control.go +++ b/pkg/controllers/cassandra/cluster_control.go @@ -1,11 +1,8 @@ package cassandra import ( - "fmt" - "github.com/golang/glog" apiv1 "k8s.io/api/core/v1" - "k8s.io/client-go/listers/apps/v1beta1" "k8s.io/client-go/tools/record" v1alpha1 "github.com/jetstack/navigator/pkg/apis/navigator/v1alpha1" @@ -18,7 +15,6 @@ import ( "github.com/jetstack/navigator/pkg/controllers/cassandra/seedlabeller" "github.com/jetstack/navigator/pkg/controllers/cassandra/service" "github.com/jetstack/navigator/pkg/controllers/cassandra/serviceaccount" - "github.com/jetstack/navigator/pkg/controllers/cassandra/util" "github.com/jetstack/navigator/pkg/util/resources" ) @@ -175,11 +171,7 @@ func (e *defaultCassandraClusterControl) Sync(c *v1alpha1.CassandraCluster) erro return err } - a, err := NextAction(c, e.state.StatefulSetLister) - if err != nil { - return err - } - + a := NextAction(c) if a != nil { err = a.Execute(e.state) if err != nil { @@ -203,14 +195,14 @@ func (e *defaultCassandraClusterControl) Sync(c *v1alpha1.CassandraCluster) erro return nil } -func NextAction(c *v1alpha1.CassandraCluster, statefulSetLister v1beta1.StatefulSetLister) (controllers.Action, error) { +func NextAction(c *v1alpha1.CassandraCluster) controllers.Action { for _, np := range c.Spec.NodePools { _, found := c.Status.NodePools[np.Name] if !found { return &actions.CreateNodePool{ Cluster: c, NodePool: &np, - }, nil + } } } for _, np := range c.Spec.NodePools { @@ -219,38 +211,18 @@ func NextAction(c *v1alpha1.CassandraCluster, statefulSetLister v1beta1.Stateful return &actions.ScaleOut{ Cluster: c, NodePool: &np, - }, nil - } - - statefulSetName := util.NodePoolResourceName(c, &np) - ss, err := statefulSetLister.StatefulSets(c.Namespace).Get(statefulSetName) - if err != nil { - return nil, err - } - - var container *apiv1.Container - for i, _ := range ss.Spec.Template.Spec.Containers { - if ss.Spec.Template.Spec.Containers[i].Name == "cassandra" { - container = &ss.Spec.Template.Spec.Containers[i] } } - if container == nil { - return nil, fmt.Errorf("unable to find cassandra container in StatefulSet %s/%s", - ss.Namespace, ss.Name, - ) - } - - glog.Warningf("requirements: %v", container.Resources) - if !resources.RequirementsEqual(container.Resources, np.Resources) { + if !resources.RequirementsEqual(np.Resources, nps.Resources) { return &actions.SetResources{ Cluster: c, NodePool: &np, - }, nil + } } else { glog.Warningf("requirementsEqual") } } - return nil, nil + return nil } From 3277fb6e2d6065f1cc25015b7a6b198cd33d0093 Mon Sep 17 00:00:00 2001 From: Louis Taylor Date: Tue, 15 May 2018 17:56:23 +0100 Subject: [PATCH 6/7] Add setresources action --- .../cassandra/actions/setresources.go | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 pkg/controllers/cassandra/actions/setresources.go diff --git a/pkg/controllers/cassandra/actions/setresources.go b/pkg/controllers/cassandra/actions/setresources.go new file mode 100644 index 000000000..3182939be --- /dev/null +++ b/pkg/controllers/cassandra/actions/setresources.go @@ -0,0 +1,83 @@ +package actions + +import ( + "fmt" + + corev1 "k8s.io/api/core/v1" + + "github.com/golang/glog" + "github.com/pkg/errors" + + "github.com/jetstack/navigator/pkg/apis/navigator/v1alpha1" + "github.com/jetstack/navigator/pkg/controllers" + "github.com/jetstack/navigator/pkg/controllers/cassandra/nodepool" + "github.com/jetstack/navigator/pkg/util/resources" +) + +type SetResources struct { + Cluster *v1alpha1.CassandraCluster + NodePool *v1alpha1.CassandraClusterNodePool +} + +var _ controllers.Action = &SetResources{} + +func (a *SetResources) Name() string { + return "SetResources" +} + +func (a *SetResources) Execute(s *controllers.State) error { + baseSet := nodepool.StatefulSetForCluster(a.Cluster, a.NodePool) + existingSet, err := s.StatefulSetLister. + StatefulSets(baseSet.Namespace).Get(baseSet.Name) + if err != nil { + return errors.Wrap(err, "unable to find statefulset") + } + + var cassContainerIndex int + var container *corev1.Container + for i, _ := range existingSet.Spec.Template.Spec.Containers { + if existingSet.Spec.Template.Spec.Containers[i].Name == "cassandra" { + cassContainerIndex = i + container = &existingSet.Spec.Template.Spec.Containers[i] + } + } + + if container == nil { + return fmt.Errorf("unable to find cassandra container in StatefulSet %s/%s", + existingSet.Namespace, existingSet.Name, + ) + } + + if resources.RequirementsEqual(container.Resources, a.NodePool.Resources) { + glog.V(4).Infof( + "SetResources not necessary because StatefulSet '%s/%s' "+ + "already has the desired resources value: %v", + existingSet.Namespace, existingSet.Name, + container.Resources, + ) + return nil + } + + newSet := existingSet.DeepCopy() + newSet.Spec.Template.Spec.Containers[cassContainerIndex].Resources = a.NodePool.Resources + glog.V(4).Infof( + "Setting cassandra resources %s/%s from %v to %v", + newSet.Namespace, newSet.Name, + existingSet.Spec.Template.Spec.Containers[cassContainerIndex].Resources, + a.NodePool.Resources, + ) + _, err = s.Clientset.AppsV1beta1(). + StatefulSets(newSet.Namespace).Update(newSet) + if err != nil { + return errors.Wrap(err, "unable to update statefulset resources") + } + s.Recorder.Eventf( + a.Cluster, + corev1.EventTypeNormal, + a.Name(), + "SetResources: NodePool=%s/%s/%s, Resources=%v", + a.Cluster.Namespace, a.Cluster.Name, a.NodePool.Name, + a.NodePool.Resources, + ) + return nil +} From 6ecd20c77c43af806823e08aa0208ed17d148832 Mon Sep 17 00:00:00 2001 From: Louis Taylor Date: Tue, 15 May 2018 17:56:44 +0100 Subject: [PATCH 7/7] Add resources util package --- pkg/util/resources/resources.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 pkg/util/resources/resources.go diff --git a/pkg/util/resources/resources.go b/pkg/util/resources/resources.go new file mode 100644 index 000000000..8173c8d74 --- /dev/null +++ b/pkg/util/resources/resources.go @@ -0,0 +1,25 @@ +package resources + +import ( + apiv1 "k8s.io/api/core/v1" +) + +func RequirementsEqual(a, b apiv1.ResourceRequirements) bool { + if a.Limits.Cpu().Cmp(*b.Limits.Cpu()) != 0 { + return false + } + + if a.Limits.Memory().Cmp(*b.Limits.Memory()) != 0 { + return false + } + + if a.Requests.Cpu().Cmp(*b.Requests.Cpu()) != 0 { + return false + } + + if a.Requests.Memory().Cmp(*b.Requests.Memory()) != 0 { + return false + } + + return true +}