From fe08279354c34a20d47a2b8f4a9b7ac43f6ffbfb Mon Sep 17 00:00:00 2001
From: Amir Blum <amirgiraffe@gmail.com>
Date: Sat, 21 Sep 2024 18:57:47 +0300
Subject: [PATCH] fix: recalculate instrumentation device when overwritten
 (#1530)

After instrumentor injects the instrumentation device and overriden envs
to the workload template spec, these value might get over written if the
user applies his workload manifest again.

At this point, the instrumentor should pick this up and insert odigos
values back into the workload manifest template spec per each container.
Current reconciler only checks if something in the env changed to
trigger recalculation, but we should also do it when something in the
resource changes, so to inject the resource again if needed
---
 common/instrumentationdevice.go               |  4 +++
 .../instrumentationdevice/manager.go          | 34 ++++++++++++++-----
 2 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/common/instrumentationdevice.go b/common/instrumentationdevice.go
index 384fe5433..5b6671778 100644
--- a/common/instrumentationdevice.go
+++ b/common/instrumentationdevice.go
@@ -40,3 +40,7 @@ func InstrumentationDeviceNameToComponents(deviceName string) (ProgrammingLangua
 	pluginName := strings.Split(deviceName, "/")[1]
 	return InstrumentationPluginNameToComponents(pluginName)
 }
+
+func IsResourceNameOdigosInstrumentation(resourceName string) bool {
+	return strings.HasPrefix(resourceName, OdigosResourceNamespace)
+}
diff --git a/instrumentor/controllers/instrumentationdevice/manager.go b/instrumentor/controllers/instrumentationdevice/manager.go
index f68520874..8fb5d8b68 100644
--- a/instrumentor/controllers/instrumentationdevice/manager.go
+++ b/instrumentor/controllers/instrumentationdevice/manager.go
@@ -2,6 +2,7 @@ package instrumentationdevice
 
 import (
 	odigosv1 "github.com/odigos-io/odigos/api/odigos/v1alpha1"
+	"github.com/odigos-io/odigos/common"
 	"github.com/odigos-io/odigos/instrumentor/controllers/utils"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -11,15 +12,25 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 )
 
-type workloadEnvChangePredicate struct {
+func countOdigosResources(resources corev1.ResourceList) int {
+	numOdigosResources := 0
+	for resourceName := range resources {
+		if common.IsResourceNameOdigosInstrumentation(resourceName.String()) {
+			numOdigosResources = numOdigosResources + 1
+		}
+	}
+	return numOdigosResources
+}
+
+type workloadPodTemplatePredicate struct {
 	predicate.Funcs
 }
 
-func (w workloadEnvChangePredicate) Create(e event.CreateEvent) bool {
+func (w workloadPodTemplatePredicate) Create(e event.CreateEvent) bool {
 	return false
 }
 
-func (w workloadEnvChangePredicate) Update(e event.UpdateEvent) bool {
+func (w workloadPodTemplatePredicate) Update(e event.UpdateEvent) bool {
 
 	if e.ObjectOld == nil || e.ObjectNew == nil {
 		return false
@@ -49,16 +60,23 @@ func (w workloadEnvChangePredicate) Update(e event.UpdateEvent) bool {
 				return true
 			}
 		}
+
+		// user might apply a change to workload which will overwrite odigos injected resources
+		prevNumOdigosResources := countOdigosResources(oldPodSpec.Spec.Containers[i].Resources.Limits)
+		newNumOdigosResources := countOdigosResources(newPodSpec.Spec.Containers[i].Resources.Limits)
+		if prevNumOdigosResources != newNumOdigosResources {
+			return true
+		}
 	}
 
 	return false
 }
 
-func (w workloadEnvChangePredicate) Delete(e event.DeleteEvent) bool {
+func (w workloadPodTemplatePredicate) Delete(e event.DeleteEvent) bool {
 	return false
 }
 
-func (w workloadEnvChangePredicate) Generic(e event.GenericEvent) bool {
+func (w workloadPodTemplatePredicate) Generic(e event.GenericEvent) bool {
 	return false
 }
 
@@ -104,7 +122,7 @@ func SetupWithManager(mgr ctrl.Manager) error {
 		ControllerManagedBy(mgr).
 		Named("instrumentationdevice-deployment").
 		For(&appsv1.Deployment{}).
-		WithEventFilter(workloadEnvChangePredicate{}).
+		WithEventFilter(workloadPodTemplatePredicate{}).
 		Complete(&DeploymentReconciler{
 			Client: mgr.GetClient(),
 		})
@@ -116,7 +134,7 @@ func SetupWithManager(mgr ctrl.Manager) error {
 		ControllerManagedBy(mgr).
 		Named("instrumentationdevice-daemonset").
 		For(&appsv1.DaemonSet{}).
-		WithEventFilter(workloadEnvChangePredicate{}).
+		WithEventFilter(workloadPodTemplatePredicate{}).
 		Complete(&DaemonSetReconciler{
 			Client: mgr.GetClient(),
 		})
@@ -127,7 +145,7 @@ func SetupWithManager(mgr ctrl.Manager) error {
 	err = builder.
 		ControllerManagedBy(mgr).
 		For(&appsv1.StatefulSet{}).
-		WithEventFilter(workloadEnvChangePredicate{}).
+		WithEventFilter(workloadPodTemplatePredicate{}).
 		Complete(&StatefulSetReconciler{
 			Client: mgr.GetClient(),
 		})