Skip to content

Commit

Permalink
fix: recalculate instrumentation device when overwritten (#1530)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
blumamir authored Sep 21, 2024
1 parent 220abb3 commit fe08279
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 8 deletions.
4 changes: 4 additions & 0 deletions common/instrumentationdevice.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
34 changes: 26 additions & 8 deletions instrumentor/controllers/instrumentationdevice/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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(),
})
Expand All @@ -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(),
})
Expand All @@ -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(),
})
Expand Down

0 comments on commit fe08279

Please sign in to comment.