Revert "fix: use instrumentation config with generation to inject env…

… vars (#…" This reverts commit 542efdd.
odigos-io · Dec 6, 2024 · bc6c76a · bc6c76a
1 parent 32a78ed
commit bc6c76a
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 65 deletions.
diff --git a/instrumentor/controllers/instrumentationdevice/common.go b/instrumentor/controllers/instrumentationdevice/common.go
@@ -111,22 +111,13 @@ func addInstrumentationDeviceToWorkload(ctx context.Context, kubeClient client.C
 		}
 	}
 
-	var instConfig odigosv1.InstrumentationConfig
-	err = kubeClient.Get(ctx, client.ObjectKey{
-		Namespace: runtimeDetails.Namespace,
-		Name:      runtimeDetails.Name,
-	}, &instConfig)
-	if err != nil {
-		return err, false
-	}
-
 	result, err := controllerutil.CreateOrPatch(ctx, kubeClient, obj, func() error {
 		podSpec, err := getPodSpecFromObject(obj)
 		if err != nil {
 			return err
 		}
 
-		err, deviceApplied, tempDevicePartiallyApplied := instrumentation.ApplyInstrumentationDevicesToPodTemplate(podSpec, &instConfig, otelSdkToUse, obj, logger)
+		err, deviceApplied, tempDevicePartiallyApplied := instrumentation.ApplyInstrumentationDevicesToPodTemplate(podSpec, runtimeDetails, otelSdkToUse, obj, logger)
 		if err != nil {
 			return err
 		}

diff --git a/...vice/instrumentationconfigs_controller.go → ...ice/instrumentedapplication_controller.go b/...vice/instrumentationconfigs_controller.go → ...ice/instrumentedapplication_controller.go
@@ -29,16 +29,17 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/log"
 )
 
-type InstrumentationConfigReconciler struct {
+// InstrumentedApplicationReconciler reconciles a InstrumentedApplication object
+type InstrumentedApplicationReconciler struct {
 	client.Client
 	Scheme *runtime.Scheme
 }
 
-func (r *InstrumentationConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
+func (r *InstrumentedApplicationReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	logger := log.FromContext(ctx)
 
-	var instConfig odigosv1.InstrumentationConfig
-	err := r.Client.Get(ctx, req.NamespacedName, &instConfig)
+	var runtimeDetails odigosv1.InstrumentedApplication
+	err := r.Client.Get(ctx, req.NamespacedName, &runtimeDetails)
 	if err != nil {
 
 		if !apierrors.IsNotFound(err) {
@@ -55,13 +56,7 @@ func (r *InstrumentationConfigReconciler) Reconcile(ctx context.Context, req ctr
 		return utils.RetryOnConflict(err)
 	}
 
-	var instrumentedApp odigosv1.InstrumentedApplication
-	err = r.Client.Get(ctx, req.NamespacedName, &instrumentedApp)
-	if err != nil {
-		return ctrl.Result{}, client.IgnoreNotFound(err)
-	}
-
 	isNodeCollectorReady := isDataCollectionReady(ctx, r.Client)
-	err = reconcileSingleWorkload(ctx, r.Client, &instrumentedApp, isNodeCollectorReady)
+	err = reconcileSingleWorkload(ctx, r.Client, &runtimeDetails, isNodeCollectorReady)
 	return utils.RetryOnConflict(err)
 }
diff --git a/instrumentor/controllers/instrumentationdevice/manager.go b/instrumentor/controllers/instrumentationdevice/manager.go
@@ -23,27 +23,6 @@ func countOdigosResources(resources corev1.ResourceList) int {
 	return numOdigosResources
 }
 
-type runtimeDetectionGeneration struct {
-	predicate.Funcs
-}
-
-func (n runtimeDetectionGeneration) Update(e event.UpdateEvent) bool {
-	if e.ObjectOld == nil || e.ObjectNew == nil {
-		return false
-	}
-
-	prevInstrumentationConfig, ok := e.ObjectOld.(*odigosv1.InstrumentationConfig)
-	if !ok {
-		return false
-	}
-	newInstrumentationConfig, ok := e.ObjectNew.(*odigosv1.InstrumentationConfig)
-	if !ok {
-		return false
-	}
-
-	return prevInstrumentationConfig.Status.ObservedWorkloadGeneration != newInstrumentationConfig.Status.ObservedWorkloadGeneration
-}
-
 type workloadPodTemplatePredicate struct {
 	predicate.Funcs
 }
@@ -123,10 +102,10 @@ func SetupWithManager(mgr ctrl.Manager) error {
 
 	err = builder.
 		ControllerManagedBy(mgr).
-		Named("instrumentationdevice-instrumentationconfig").
-		For(&odigosv1.InstrumentationConfig{}).
-		WithEventFilter(&runtimeDetectionGeneration{}).
-		Complete(&InstrumentationConfigReconciler{
+		Named("instrumentationdevice-instrumentedapplication").
+		For(&odigosv1.InstrumentedApplication{}).
+		WithEventFilter(&predicate.GenerationChangedPredicate{}).
+		Complete(&InstrumentedApplicationReconciler{
 			Client: mgr.GetClient(),
 			Scheme: mgr.GetScheme(),
 		})

diff --git a/instrumentor/instrumentation/instrumentation.go b/instrumentor/instrumentation/instrumentation.go
@@ -22,20 +22,13 @@ var (
 	ErrPatchEnvVars = errors.New("failed to patch env vars")
 )
 
-func ApplyInstrumentationDevicesToPodTemplate(original *corev1.PodTemplateSpec, instConfig *odigosv1.InstrumentationConfig, defaultSdks map[common.ProgrammingLanguage]common.OtelSdk, targetObj client.Object,
+func ApplyInstrumentationDevicesToPodTemplate(original *corev1.PodTemplateSpec, runtimeDetails *odigosv1.InstrumentedApplication, defaultSdks map[common.ProgrammingLanguage]common.OtelSdk, targetObj client.Object,
 	logger logr.Logger) (error, bool, bool) {
 	// delete any existing instrumentation devices.
 	// this is necessary for example when migrating from community to enterprise,
 	// and we need to cleanup the community device before adding the enterprise one.
 	RevertInstrumentationDevices(original)
 
-	// don't use the runtime detection if it's from an older generation
-	// as it might inject irrelevant env values into the workload manifest.
-	if instConfig.Status.ObservedWorkloadGeneration != targetObj.GetGeneration() {
-		logger.Info("Skipping applying instrumentation devices to workload manifest due to generation mismatch", "observedGeneration", instConfig.Status.ObservedWorkloadGeneration, "currentGeneration", targetObj.GetGeneration())
-		return nil, false, false
-	}
-
 	deviceApplied := false
 	deviceSkippedDueToOtherAgent := false
 	var modifiedContainers []corev1.Container
@@ -46,8 +39,8 @@ func ApplyInstrumentationDevicesToPodTemplate(original *corev1.PodTemplateSpec,
 	}
 
 	for _, container := range original.Spec.Containers {
-		containerLanguage := getLanguageOfContainer(instConfig, container.Name)
-		containerHaveOtherAgent := getContainerOtherAgents(instConfig, container.Name)
+		containerLanguage := getLanguageOfContainer(runtimeDetails, container.Name)
+		containerHaveOtherAgent := getContainerOtherAgents(runtimeDetails, container.Name)
 
 		// In case there is another agent in the container, we should not apply the instrumentation device.
 		if containerLanguage == common.PythonProgrammingLanguage && containerHaveOtherAgent != nil {
@@ -63,7 +56,7 @@ func ApplyInstrumentationDevicesToPodTemplate(original *corev1.PodTemplateSpec,
 		if containerLanguage == common.UnknownProgrammingLanguage || containerLanguage == common.IgnoredProgrammingLanguage || containerLanguage == common.NginxProgrammingLanguage {
 			// always patch the env vars, even if the language is unknown or ignored.
 			// this is necessary to sync the existing envs with the missing language if changed for any reason.
-			err = patchEnvVarsForContainer(instConfig, &container, nil, containerLanguage, manifestEnvOriginal)
+			err = patchEnvVarsForContainer(runtimeDetails, &container, nil, containerLanguage, manifestEnvOriginal)
 			if err != nil {
 				return fmt.Errorf("%w: %v", ErrPatchEnvVars, err), deviceApplied, deviceSkippedDueToOtherAgent
 			}
@@ -84,7 +77,7 @@ func ApplyInstrumentationDevicesToPodTemplate(original *corev1.PodTemplateSpec,
 		container.Resources.Limits[corev1.ResourceName(instrumentationDeviceName)] = resource.MustParse("1")
 		deviceApplied = true
 
-		err = patchEnvVarsForContainer(instConfig, &container, &otelSdk, containerLanguage, manifestEnvOriginal)
+		err = patchEnvVarsForContainer(runtimeDetails, &container, &otelSdk, containerLanguage, manifestEnvOriginal)
 		if err != nil {
 			return fmt.Errorf("%w: %v", ErrPatchEnvVars, err), deviceApplied, deviceSkippedDueToOtherAgent
 		}
@@ -168,8 +161,8 @@ func RevertInstrumentationDevices(original *corev1.PodTemplateSpec) bool {
 	return changed
 }
 
-func getLanguageOfContainer(instrumentation *odigosv1.InstrumentationConfig, containerName string) common.ProgrammingLanguage {
-	for _, l := range instrumentation.Status.RuntimeDetailsByContainer {
+func getLanguageOfContainer(instrumentation *odigosv1.InstrumentedApplication, containerName string) common.ProgrammingLanguage {
+	for _, l := range instrumentation.Spec.RuntimeDetails {
 		if l.ContainerName == containerName {
 			return l.Language
 		}
@@ -178,8 +171,8 @@ func getLanguageOfContainer(instrumentation *odigosv1.InstrumentationConfig, con
 	return common.UnknownProgrammingLanguage
 }
 
-func getContainerOtherAgents(instrumentation *odigosv1.InstrumentationConfig, containerName string) *odigosv1.OtherAgent {
-	for _, l := range instrumentation.Status.RuntimeDetailsByContainer {
+func getContainerOtherAgents(instrumentation *odigosv1.InstrumentedApplication, containerName string) *odigosv1.OtherAgent {
+	for _, l := range instrumentation.Spec.RuntimeDetails {
 		if l.ContainerName == containerName {
 			if l.OtherAgent != nil && *l.OtherAgent != (odigosv1.OtherAgent{}) {
 				return l.OtherAgent
@@ -191,10 +184,10 @@ func getContainerOtherAgents(instrumentation *odigosv1.InstrumentationConfig, co
 
 // getEnvVarsOfContainer returns the env vars which are defined for the given container and are used for instrumentation purposes.
 // This function also returns env vars which are declared in the container build.
-func getEnvVarsOfContainer(instrumentation *odigosv1.InstrumentationConfig, containerName string) map[string]string {
+func getEnvVarsOfContainer(instrumentation *odigosv1.InstrumentedApplication, containerName string) map[string]string {
 	envVars := make(map[string]string)
 
-	for _, l := range instrumentation.Status.RuntimeDetailsByContainer {
+	for _, l := range instrumentation.Spec.RuntimeDetails {
 		if l.ContainerName == containerName {
 			for _, env := range l.EnvVars {
 				envVars[env.Name] = env.Value
@@ -208,7 +201,7 @@ func getEnvVarsOfContainer(instrumentation *odigosv1.InstrumentationConfig, cont
 
 // when otelsdk is nil, it means that the container is not instrumented.
 // this will trigger reverting of any existing env vars which were set by odigos before.
-func patchEnvVarsForContainer(runtimeDetails *odigosv1.InstrumentationConfig, container *corev1.Container, sdk *common.OtelSdk, programmingLanguage common.ProgrammingLanguage, manifestEnvOriginal *envoverwrite.OrigWorkloadEnvValues) error {
+func patchEnvVarsForContainer(runtimeDetails *odigosv1.InstrumentedApplication, container *corev1.Container, sdk *common.OtelSdk, programmingLanguage common.ProgrammingLanguage, manifestEnvOriginal *envoverwrite.OrigWorkloadEnvValues) error {
 
 	observedEnvs := getEnvVarsOfContainer(runtimeDetails, container.Name)