From bc6c76ab946923041a7c283872fbe7a3138c4087 Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Fri, 6 Dec 2024 07:34:19 +0200 Subject: [PATCH] =?UTF-8?q?Revert=20"fix:=20use=20instrumentation=20config?= =?UTF-8?q?=20with=20generation=20to=20inject=20env=20vars=20(#=E2=80=A6"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 542efddbcce10376b4aa024d7dd2db35e42a746c. --- .../instrumentationdevice/common.go | 11 +------ ... => instrumentedapplication_controller.go} | 17 ++++------ .../instrumentationdevice/manager.go | 29 +++-------------- .../instrumentation/instrumentation.go | 31 +++++++------------ 4 files changed, 23 insertions(+), 65 deletions(-) rename instrumentor/controllers/instrumentationdevice/{instrumentationconfigs_controller.go => instrumentedapplication_controller.go} (76%) diff --git a/instrumentor/controllers/instrumentationdevice/common.go b/instrumentor/controllers/instrumentationdevice/common.go index 034da6d7db..64766ca965 100644 --- 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/instrumentor/controllers/instrumentationdevice/instrumentationconfigs_controller.go b/instrumentor/controllers/instrumentationdevice/instrumentedapplication_controller.go similarity index 76% rename from instrumentor/controllers/instrumentationdevice/instrumentationconfigs_controller.go rename to instrumentor/controllers/instrumentationdevice/instrumentedapplication_controller.go index 4f22d7a6b4..a3143ccb7a 100644 --- a/instrumentor/controllers/instrumentationdevice/instrumentationconfigs_controller.go +++ b/instrumentor/controllers/instrumentationdevice/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 index 0d76909ace..e06e5cd57b 100644 --- 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 index 327e9f1372..94b6803bf5 100644 --- 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)