Merge pull request #143 from scylladb/cpuset_incorrectly_set

QoS enforced on deploy of a cluster.

Author: Henrik Johansson

Gopkg.lock

@@ -100,6 +100,9 @@ spec:
           limits:
             cpu: 30
             memory: 115G
+          requests:
+            cpu: 30
+            memory: 115G
         placement:
           nodeAffinity:
             requiredDuringSchedulingIgnoredDuringExecution:

examples/gke/cluster.yaml

@@ -191,6 +191,16 @@ func StatefulSetForRack(r scyllav1alpha1.RackSpec, c *scyllav1alpha1.Cluster, si
 								"-c",
 								fmt.Sprintf("cp -a /sidecar/* %s", naming.SharedDirName),
 							},
+							Resources: corev1.ResourceRequirements{
+								Requests: corev1.ResourceList{
+									corev1.ResourceCPU:    resource.MustParse("1"),
+									corev1.ResourceMemory: resource.MustParse("200M"),
+								},
+								Limits: corev1.ResourceList{
+									corev1.ResourceCPU:    resource.MustParse("1"),
+									corev1.ResourceMemory: resource.MustParse("200M"),
+								},
+							},
 							VolumeMounts: []corev1.VolumeMount{
 								{
 									Name:      "shared",
@@ -396,6 +406,16 @@ func sysctlInitContainer(sysctls []string) *corev1.Container {
 		SecurityContext: &corev1.SecurityContext{
 			Privileged: &opt,
 		},
+		Resources: corev1.ResourceRequirements{
+			Requests: corev1.ResourceList{
+				corev1.ResourceCPU:    resource.MustParse("1"),
+				corev1.ResourceMemory: resource.MustParse("200M"),
+			},
+			Limits: corev1.ResourceList{
+				corev1.ResourceCPU:    resource.MustParse("1"),
+				corev1.ResourceMemory: resource.MustParse("200M"),
+			},
+		},
 		Command: []string{
 			"/bin/sh",
 			"-c",
@@ -432,6 +452,16 @@ func agentContainer(c *scyllav1alpha1.Cluster) *corev1.Container {
 				ReadOnly:  true,
 			},
 		},
+		Resources: corev1.ResourceRequirements{
+			Requests: corev1.ResourceList{
+				corev1.ResourceCPU:    resource.MustParse("1"),
+				corev1.ResourceMemory: resource.MustParse("200M"),
+			},
+			Limits: corev1.ResourceList{
+				corev1.ResourceCPU:    resource.MustParse("1"),
+				corev1.ResourceMemory: resource.MustParse("200M"),
+			},
+		},
 	}
 }
 

pkg/controller/cluster/resource/resource.go

@@ -6,6 +6,7 @@ import (
 	"io/ioutil"
 	"os"
 	"os/exec"
+	"strconv"
 	"strings"
 
 	"github.com/ghodss/yaml"
@@ -17,6 +18,7 @@ import (
 	"github.com/scylladb/scylla-operator/pkg/naming"
 	"github.com/scylladb/scylla-operator/pkg/sidecar/identity"
 	"k8s.io/client-go/kubernetes"
+	"k8s.io/kubernetes/pkg/kubelet/cm/cpuset"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -173,13 +175,18 @@ func (s *ScyllaConfig) setupEntrypoint(ctx context.Context) (*exec.Cmd, error) {
 		devMode = "1"
 	}
 
+	shards, err := strconv.Atoi(options.GetSidecarOptions().CPU)
+	if err != nil {
+		return nil, errors.WithStack(err)
+	}
 	args := []string{
 		fmt.Sprintf("--listen-address=%s", m.IP),
 		fmt.Sprintf("--broadcast-address=%s", m.StaticIP),
 		fmt.Sprintf("--broadcast-rpc-address=%s", m.StaticIP),
 		fmt.Sprintf("--seeds=%s", strings.Join(seeds, ",")),
 		fmt.Sprintf("--developer-mode=%s", devMode),
-		fmt.Sprintf("--smp=%s", options.GetSidecarOptions().CPU),
+		fmt.Sprintf("--overprovisioned=%d", 0),
+		fmt.Sprintf("--smp=%d", shards),
 	}
 	if cluster.Spec.Alternator.Enabled() {
 		args = append(args, fmt.Sprintf("--alternator-port=%d", cluster.Spec.Alternator.Port))
@@ -194,7 +201,9 @@ func (s *ScyllaConfig) setupEntrypoint(ctx context.Context) (*exec.Cmd, error) {
 		if err != nil {
 			return nil, errors.WithStack(err)
 		}
-
+		if err := s.validateCpuSet(ctx, cpusAllowed, shards); err != nil {
+			return nil, errors.WithStack(err)
+		}
 		args = append(args, fmt.Sprintf("--cpuset=%s", cpusAllowed))
 	}
 
@@ -206,6 +215,18 @@ func (s *ScyllaConfig) setupEntrypoint(ctx context.Context) (*exec.Cmd, error) {
 	return scyllaCmd, nil
 }
 
+func (s *ScyllaConfig) validateCpuSet(ctx context.Context, cpusAllowed string, shards int) error {
+	cpuSet, err := cpuset.Parse(cpusAllowed)
+	if err != nil {
+		return err
+	}
+	if cpuSet.Size() != shards {
+		s.logger.Info(ctx, "suboptimal shard and cpuset config, shard count (config: 'CPU') and cpuset size should match for optimal performance",
+			"shards", shards, "cpuset", cpuSet.String())
+	}
+	return nil
+}
+
 // mergeYAMLs merges two arbitrary YAML structures at the top level.
 func mergeYAMLs(initialYAML, overrideYAML []byte) ([]byte, error) {
 

pkg/sidecar/config/config.go

@@ -33,16 +33,18 @@ func checkValues(c *scyllav1alpha1.Cluster) (allowed bool, msg string) {
 				return false, fmt.Sprintf("when using cpuset, you must use whole cpu cores, but rack %s has %dm", rack.Name, cores)
 			}
 
-			// Requests == Limits or only Limits must be set for QOS class guaranteed
+			// Requests == Limits and Requests must be set and equal for QOS class guaranteed
 			requests := rack.Resources.Requests
 			if requests != nil {
-				if requests.Cpu().MilliValue() == 0 {
+				if requests.Cpu().MilliValue() != limits.Cpu().MilliValue() {
 					return false, fmt.Sprintf("when using cpuset, cpu requests must be the same as cpu limits in rack %s", rack.Name)
 				}
-				if requests.Memory().MilliValue() == 0 {
+				if requests.Memory().MilliValue() != limits.Memory().MilliValue() {
 					return false, fmt.Sprintf("when using cpuset, memory requests must be the same as memory limits in rack %s", rack.Name)
 				}
-				return false, fmt.Sprintf("when using cpuset, requests must be the same as limits in rack %s", rack.Name)
+			} else {
+				// Copy the limits
+				rack.Resources.Requests = limits.DeepCopy()
 			}
 		}
 	}