🎨 improve logging and stability

modified:   README.md; modified:   controllers/pod_controller.go; modified:   controllers/pod_controller_functions.go; modified:   controllers/pod_controller_types.go

Author: amitai-devops

README.md

@@ -16,8 +16,8 @@
 
 **kube-reqsizer** is a kubernetes controller that will measure the usage of pods over time and optimize (reduce/increase) their requests based on the average usage.
 
-When all required conditions meet, the controller calculates the result requirements based on all the samples taken so far a pod.
-It then goes "upstream" to the parent controller of that pod, for example *Deployment*, and updates the relevant containers for the pod inside the deployment as a reconciliation, as if its desired state is the new state with the new requirements.
+When all required conditions meet, the controller calculates the result requirements based on all the samples taken so far a pod and its peers in the same deployment controller.
+It then goes "upstream" to the parent controller of that pod, for example *Deployment*, and updates the relevant containers for the pods inside the deployment as a reconciliation, as if its desired state is the new state with the new requirements.
 
 <img src="./assets/example.png" width="200">
 

controllers/pod_controller.go

@@ -40,10 +40,10 @@ const (
 )
 
 func cacheKeyFunc(obj interface{}) (string, error) {
-	return obj.(PodRequests).Name, nil
+	return obj.(PodRequests).Name + obj.(PodRequests).Namespace, nil
 }
 
-var cacheStore = cache.NewTTLStore(cacheKeyFunc, 48*time.Hour)
+var cacheStore = cache.NewStore(cacheKeyFunc)
 
 // Reconcile handles a reconciliation request for a Pod.
 func (r *PodReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
@@ -59,37 +59,40 @@ func (r *PodReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.R
 			// we'll ignore not-found errors, since they can't be fixed by an immediate
 			// requeue (we'll need to wait for a new notification), and we can get them
 			// on deleted requests.
-			return ctrl.Result{}, err
+			return ctrl.Result{}, nil
 		}
-		log.Error(err, "unable to fetch Pod")
-		return ctrl.Result{}, err
+		log.Error(nil, "unable to fetch Pod")
+		return ctrl.Result{}, nil
 	}
 
 	annotation, err := r.NamespaceOrPodHaveAnnotation(pod, ctx)
 	if err != nil {
-		log.Error(err, "failed to get annotations")
-		return ctrl.Result{}, err
+		log.Error(nil, "failed to get annotations")
+		return ctrl.Result{}, nil
 	}
 	ignoreAnnotation, err := r.NamespaceOrPodHaveIgnoreAnnotation(pod, ctx)
 	if err != nil {
-		log.Error(err, "failed to get annotations")
-		return ctrl.Result{}, err
+		log.Error(nil, "failed to get annotations")
+		return ctrl.Result{}, nil
 	}
 
 	if ((!r.EnableAnnotation) || (r.EnableAnnotation && annotation)) && !ignoreAnnotation {
 		data, err := r.ClientSet.RESTClient().Get().AbsPath(fmt.Sprintf("apis/metrics.k8s.io/v1beta1/namespaces/%v/pods/%v", pod.Namespace, pod.Name)).DoRaw(ctx)
 
 		if err != nil {
-			log.Error(err, "failed to get stats from pod")
-			return ctrl.Result{}, err
+			log.Error(nil, "failed to get stats from pod")
+			return ctrl.Result{}, nil
 		}
 		PodUsageData := GeneratePodRequestsObjectFromRestData(data)
-
-		SumPodRequest := PodRequests{Name: pod.Name, ContainerRequests: []ContainerRequests{}}
+		err, _, _, deploymentName := r.GetPodParentKind(pod, ctx)
+		if err != nil {
+			deploymentName = pod.Name
+		}
+		SumPodRequest := PodRequests{Name: deploymentName, Namespace: pod.Namespace, ContainerRequests: []ContainerRequests{}}
 
 		SumPodRequest.ContainerRequests = PodUsageData.ContainerRequests
 
-		LatestPodRequest, err := fetchFromCache(cacheStore, pod.Name)
+		LatestPodRequest, err := fetchFromCache(cacheStore, deploymentName+pod.Namespace)
 		if err != nil {
 			SumPodRequest.Sample = 0
 			log.Info(fmt.Sprint("Adding cache sample ", SumPodRequest.Sample))

controllers/pod_controller_functions.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
-	"fmt"
 	"strconv"
 	"strings"
 	"time"
@@ -78,7 +77,7 @@ func (r *PodReconciler) UpdateKubeObject(pod client.Object, ctx context.Context)
 			return ctrl.Result{}, nil
 		}
 		log.Error(err, "unable to update pod")
-		return ctrl.Result{}, err
+		return ctrl.Result{}, nil
 	}
 	return ctrl.Result{RequeueAfter: 10 * time.Second}, nil
 }
@@ -87,7 +86,7 @@ func UpdatePodController(podspec *corev1.PodSpec, Requests []NewContainerRequest
 	for _, podContainer := range Requests {
 		for i, depContainer := range podspec.Containers {
 			if depContainer.Name == podContainer.Name {
-				log.Info(fmt.Sprint("Setting", podContainer.Requests))
+				// log.Info(fmt.Sprint("Setting", podContainer.Requests))
 				podspec.Containers[i].Resources = podContainer.Requests
 			}
 		}
@@ -169,7 +168,7 @@ func GetPodRequests(pod corev1.Pod) PodRequests {
 		}
 		containerData = append(containerData, ContainerRequests{Name: c.Name, CPU: int64(nanoCores), Memory: int64(miMemory)})
 	}
-	return PodRequests{pod.Name, containerData, 0}
+	return PodRequests{pod.Name, pod.Namespace, containerData, 0}
 }
 
 func addToCache(cacheStore cache.Store, object PodRequests) error {
@@ -209,7 +208,7 @@ func GeneratePodRequestsObjectFromRestData(restData []byte) PodRequests {
 		kiMemory, _ := strconv.Atoi(strings.ReplaceAll(c.Usage.Memory, "Ki", ""))
 		containerData = append(containerData, ContainerRequests{Name: c.Name, CPU: int64(nanoCores / 1000000), Memory: int64(kiMemory / 1000)})
 	}
-	return PodRequests{data.Metadata.Name, containerData, 0}
+	return PodRequests{data.Metadata.Name, data.Metadata.Namespace, containerData, 0}
 }
 
 func (r *PodReconciler) MinimumUptimeOfPodInParent(pod corev1.Pod, ctx context.Context) bool {
@@ -244,19 +243,16 @@ func (r *PodReconciler) GetPodParentKind(pod corev1.Pod, ctx context.Context) (e
 			return err, nil, nil, ""
 		}
 		deployment, err := r.ClientSet.AppsV1().Deployments(pod.Namespace).Get(ctx, replica.OwnerReferences[0].Name, metav1.GetOptions{})
-		deployment.Annotations["reqsizer.jatalocks.github.io/changed"] = "true"
 		if replica.OwnerReferences[0].Kind == "Deployment" {
 			return err, &deployment.Spec.Template.Spec, deployment, deployment.Name
 		} else {
 			return errors.New("Is Owned by Unknown CRD"), nil, nil, ""
 		}
 	case "DaemonSet":
 		deployment, err := r.ClientSet.AppsV1().DaemonSets(pod.Namespace).Get(ctx, pod.OwnerReferences[0].Kind, metav1.GetOptions{})
-		deployment.Annotations["reqsizer.jatalocks.github.io/changed"] = "true"
 		return err, &deployment.Spec.Template.Spec, deployment, deployment.Name
 	case "StatefulSet":
 		deployment, err := r.ClientSet.AppsV1().StatefulSets(pod.Namespace).Get(ctx, pod.OwnerReferences[0].Kind, metav1.GetOptions{})
-		deployment.Annotations["reqsizer.jatalocks.github.io/changed"] = "true"
 		return err, &deployment.Spec.Template.Spec, deployment, deployment.Name
 	default:
 		return errors.New("Is Owned by Unknown CRD"), nil, nil, ""

controllers/pod_controller_types.go

@@ -31,7 +31,8 @@ type PodReconciler struct {
 }
 
 type PodRequests struct {
-	Name              string
+	Name              string // Name of Deployment Controller
+	Namespace         string
 	ContainerRequests []ContainerRequests
 	Sample            int
 	// TimeSinceFirstSample float64