From 252f01eeec460f596b4fa5944f77a61bed9bb62a Mon Sep 17 00:00:00 2001 From: lamai93 Date: Mon, 1 Apr 2019 14:04:59 +0200 Subject: [PATCH 1/3] Forward resources to container, except for Storage. --- pkg/deployment/images.go | 2 +- pkg/deployment/resources/pod_creator.go | 2 +- pkg/util/k8sutil/pods.go | 27 ++++++++++++++++++++++--- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/pkg/deployment/images.go b/pkg/deployment/images.go index 85d633cae..c0a838f44 100644 --- a/pkg/deployment/images.go +++ b/pkg/deployment/images.go @@ -198,7 +198,7 @@ func (ib *imagesBuilder) fetchArangoDBImageIDAndVersion(ctx context.Context, ima } } if err := k8sutil.CreateArangodPod(ib.KubeCli, true, ib.APIObject, role, id, podName, "", image, "", "", ib.Spec.GetImagePullPolicy(), "", false, terminationGracePeriod, args, env, nil, nil, nil, - tolerations, serviceAccountName, "", "", "", nil); err != nil { + tolerations, serviceAccountName, "", "", "", nil, v1.ResourceRequirements{}); err != nil { log.Debug().Err(err).Msg("Failed to create image ID pod") return true, maskAny(err) } diff --git a/pkg/deployment/resources/pod_creator.go b/pkg/deployment/resources/pod_creator.go index d76964068..b5f5127cf 100644 --- a/pkg/deployment/resources/pod_creator.go +++ b/pkg/deployment/resources/pod_creator.go @@ -607,7 +607,7 @@ func (r *Resources) createPodForMember(spec api.DeploymentSpec, memberID string, finalizers := r.createPodFinalizers(group) if err := k8sutil.CreateArangodPod(kubecli, spec.IsDevelopment(), apiObject, role, m.ID, m.PodName, m.PersistentVolumeClaimName, imageInfo.ImageID, lifecycleImage, alpineImage, spec.GetImagePullPolicy(), engine, requireUUID, terminationGracePeriod, args, env, finalizers, livenessProbe, readinessProbe, tolerations, serviceAccountName, tlsKeyfileSecretName, rocksdbEncryptionSecretName, - clusterJWTSecretName, groupSpec.GetNodeSelector()); err != nil { + clusterJWTSecretName, groupSpec.GetNodeSelector(), groupSpec.Resources); err != nil { return maskAny(err) } log.Debug().Str("pod-name", m.PodName).Msg("Created pod") diff --git a/pkg/util/k8sutil/pods.go b/pkg/util/k8sutil/pods.go index a1afb8da6..a881b3121 100644 --- a/pkg/util/k8sutil/pods.go +++ b/pkg/util/k8sutil/pods.go @@ -263,9 +263,28 @@ func arangodInitContainer(name, id, engine, alpineImage string, requireUUID bool return c } +func filterStorageResourceRequirement(resources v1.ResourceRequirements) v1.ResourceRequirements { + + filterStorage := func(list v1.ResourceList) v1.ResourceList { + newlist := make(v1.ResourceList) + for k, v := range list { + if k == v1.ResourceStorage { + continue + } + newlist[k] = v + } + return newlist + } + + return v1.ResourceRequirements{ + Limits: filterStorage(resources.Limits), + Requests: filterStorage(resources.Requests), + } +} + // arangodContainer creates a container configured to run `arangod`. func arangodContainer(image string, imagePullPolicy v1.PullPolicy, args []string, env map[string]EnvValue, livenessProbe *HTTPProbeConfig, readinessProbe *HTTPProbeConfig, - lifecycle *v1.Lifecycle, lifecycleEnvVars []v1.EnvVar) v1.Container { + lifecycle *v1.Lifecycle, lifecycleEnvVars []v1.EnvVar, resources v1.ResourceRequirements) v1.Container { c := v1.Container{ Command: append([]string{"/usr/sbin/arangod"}, args...), Name: ServerContainerName, @@ -279,6 +298,7 @@ func arangodContainer(image string, imagePullPolicy v1.PullPolicy, args []string Protocol: v1.ProtocolTCP, }, }, + Resources: filterStorageResourceRequirement(resources), VolumeMounts: arangodVolumeMounts(), } for k, v := range env { @@ -434,7 +454,8 @@ func CreateArangodPod(kubecli kubernetes.Interface, developmentMode bool, deploy engine string, requireUUID bool, terminationGracePeriod time.Duration, args []string, env map[string]EnvValue, finalizers []string, livenessProbe *HTTPProbeConfig, readinessProbe *HTTPProbeConfig, tolerations []v1.Toleration, serviceAccountName string, - tlsKeyfileSecretName, rocksdbEncryptionSecretName string, clusterJWTSecretName string, nodeSelector map[string]string) error { + tlsKeyfileSecretName, rocksdbEncryptionSecretName string, clusterJWTSecretName string, nodeSelector map[string]string, + resources v1.ResourceRequirements) error { // Prepare basic pod p := newPod(deployment.GetName(), deployment.GetNamespace(), role, id, podName, finalizers, tolerations, serviceAccountName, nodeSelector) terminationGracePeriodSeconds := int64(math.Ceil(terminationGracePeriod.Seconds())) @@ -457,7 +478,7 @@ func CreateArangodPod(kubecli kubernetes.Interface, developmentMode bool, deploy } // Add arangod container - c := arangodContainer(image, imagePullPolicy, args, env, livenessProbe, readinessProbe, lifecycle, lifecycleEnvVars) + c := arangodContainer(image, imagePullPolicy, args, env, livenessProbe, readinessProbe, lifecycle, lifecycleEnvVars, resources) if tlsKeyfileSecretName != "" { c.VolumeMounts = append(c.VolumeMounts, tlsKeyfileVolumeMounts()...) } From 6747623f0c792409c848e96f4efbec1cf42a21f8 Mon Sep 17 00:00:00 2001 From: lamai93 Date: Mon, 1 Apr 2019 14:32:37 +0200 Subject: [PATCH 2/3] Added some ideas for pod rotation for changing requirements. --- pkg/deployment/reconcile/plan_builder.go | 7 +++++++ pkg/util/k8sutil/images.go | 14 ++++++++++++++ pkg/util/k8sutil/pods.go | 3 ++- 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/pkg/deployment/reconcile/plan_builder.go b/pkg/deployment/reconcile/plan_builder.go index a79534346..ccf13e829 100644 --- a/pkg/deployment/reconcile/plan_builder.go +++ b/pkg/deployment/reconcile/plan_builder.go @@ -345,6 +345,13 @@ func podNeedsRotation(log zerolog.Logger, p v1.Pod, apiObject metav1.Object, spe return true, "ServiceAccountName changed" } + // Check resource requirements + // Kubernetes will fill in some default values thus this will always rotate the pod. + // One could come up with something more sophisticated to check if a rotation is needed. (check if the requirements are more than requested and limits are less than requested) + /*if !reflect.DeepEqual(spec.GetServerGroupSpec(group).Resources, k8sutil.GetArangoDBContainerFromPod(&p).Resources) { + return true, "Resource Requirements changed" + }*/ + return false, "" } diff --git a/pkg/util/k8sutil/images.go b/pkg/util/k8sutil/images.go index e0e171652..8e606de5a 100644 --- a/pkg/util/k8sutil/images.go +++ b/pkg/util/k8sutil/images.go @@ -53,3 +53,17 @@ func GetArangoDBImageIDFromPod(pod *corev1.Pod) string { } return ConvertImageID2Image(rawImageID) } + +// GetArangoDBContainerFromPod returns the ArangoDB container from a pod +func GetArangoDBContainerFromPod(pod *corev1.Pod) corev1.Container { + arangoc := pod.Spec.Containers[0] + if len(pod.Status.ContainerStatuses) > 1 { + for _, container := range pod.Spec.Containers { + if strings.Contains(container.Name, "server") { + arangoc = container + } + } + } + + return arangoc +} diff --git a/pkg/util/k8sutil/pods.go b/pkg/util/k8sutil/pods.go index a881b3121..4640ecdfe 100644 --- a/pkg/util/k8sutil/pods.go +++ b/pkg/util/k8sutil/pods.go @@ -263,6 +263,7 @@ func arangodInitContainer(name, id, engine, alpineImage string, requireUUID bool return c } +// filterStorageResourceRequirement filters resource requirements for Storage. func filterStorageResourceRequirement(resources v1.ResourceRequirements) v1.ResourceRequirements { filterStorage := func(list v1.ResourceList) v1.ResourceList { @@ -298,7 +299,7 @@ func arangodContainer(image string, imagePullPolicy v1.PullPolicy, args []string Protocol: v1.ProtocolTCP, }, }, - Resources: filterStorageResourceRequirement(resources), + Resources: filterStorageResourceRequirement(resources), // Storage is handled via pvcs VolumeMounts: arangodVolumeMounts(), } for k, v := range env { From 48bed099ef5f67733d201dfa303badc33ea223db Mon Sep 17 00:00:00 2001 From: lamai93 Date: Mon, 1 Apr 2019 15:48:31 +0200 Subject: [PATCH 3/3] Added test for rotation. --- pkg/deployment/reconcile/plan_builder.go | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/pkg/deployment/reconcile/plan_builder.go b/pkg/deployment/reconcile/plan_builder.go index ccf13e829..7702e947a 100644 --- a/pkg/deployment/reconcile/plan_builder.go +++ b/pkg/deployment/reconcile/plan_builder.go @@ -346,15 +346,30 @@ func podNeedsRotation(log zerolog.Logger, p v1.Pod, apiObject metav1.Object, spe } // Check resource requirements - // Kubernetes will fill in some default values thus this will always rotate the pod. - // One could come up with something more sophisticated to check if a rotation is needed. (check if the requirements are more than requested and limits are less than requested) - /*if !reflect.DeepEqual(spec.GetServerGroupSpec(group).Resources, k8sutil.GetArangoDBContainerFromPod(&p).Resources) { + if resourcesRequireRotation(spec.GetServerGroupSpec(group).Resources, k8sutil.GetArangoDBContainerFromPod(&p).Resources) { return true, "Resource Requirements changed" - }*/ + } return false, "" } +// resourcesRequireRotation returns true if the resource requirements have changed such that a rotation is required +func resourcesRequireRotation(wanted, given v1.ResourceRequirements) bool { + checkList := func(wanted, given v1.ResourceList) bool { + for k, v := range wanted { + if gv, ok := given[k]; !ok { + return true + } else if v.Cmp(gv) != 0 { + return true + } + } + + return false + } + + return checkList(wanted.Limits, given.Limits) || checkList(wanted.Requests, given.Requests) +} + // normalizeServiceAccountName replaces default with empty string, otherwise returns the input. func normalizeServiceAccountName(name string) string { if name == "default" {