From 7ef4d9443b1269eab2ef3dccade2390aaacc9ff5 Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Mon, 30 Dec 2024 15:42:34 -0500 Subject: [PATCH 01/21] Include Namespace Sources in GetSourceListForWorkload --- api/odigos/v1alpha1/source_types.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/api/odigos/v1alpha1/source_types.go b/api/odigos/v1alpha1/source_types.go index ae4ab1799..5d8d0a110 100644 --- a/api/odigos/v1alpha1/source_types.go +++ b/api/odigos/v1alpha1/source_types.go @@ -78,6 +78,23 @@ func GetSourceListForWorkload(ctx context.Context, kubeClient client.Client, obj consts.WorkloadKindLabel: obj.GetObjectKind().GroupVersionKind().Kind, }) err := kubeClient.List(ctx, &sourceList, &client.ListOptions{LabelSelector: selector}) + if err != nil { + return sourceList, err + } + + namespaceSourceList := SourceList{} + namespaceSelector := labels.SelectorFromSet(labels.Set{ + consts.WorkloadNameLabel: obj.GetNamespace(), + consts.WorkloadNamespaceLabel: obj.GetNamespace(), + consts.WorkloadKindLabel: "Namespace", + }) + err = kubeClient.List(ctx, &namespaceSourceList, &client.ListOptions{LabelSelector: namespaceSelector}) + if err != nil { + return sourceList, err + } + + // merge the lists + sourceList.Items = append(sourceList.Items, namespaceSourceList.Items...) return sourceList, err } From e0320fab4260c9710f60e23716bbd278f5a31ead Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Mon, 30 Dec 2024 15:44:01 -0500 Subject: [PATCH 02/21] Add Namespace instrumentation to startlangdetection --- common/consts/consts.go | 1 + .../namespace_controller.go | 9 +- .../startlangdetection/source_controller.go | 97 ++++++++++++------- .../workload_controllers.go | 7 ++ 4 files changed, 77 insertions(+), 37 deletions(-) diff --git a/common/consts/consts.go b/common/consts/consts.go index a98d34179..7fc22573e 100644 --- a/common/consts/consts.go +++ b/common/consts/consts.go @@ -18,6 +18,7 @@ const ( OdigosNamespaceAnnotation = "odigos.io/workload-namespace" OdigosWorkloadKindAnnotation = "odigos.io/workload-kind" OdigosWorkloadNameAnnotation = "odigos.io/workload-name" + OdigosWorkloadExcludedLabel = "odigos.io/excluded" OdigosReportedNameAnnotation = "odigos.io/reported-name" // GatewayMaxConnectionAge and GatewayMaxConnectionAgeGrace are the default values for the gateway collector. diff --git a/instrumentor/controllers/startlangdetection/namespace_controller.go b/instrumentor/controllers/startlangdetection/namespace_controller.go index 8e36d49b9..96e345893 100644 --- a/instrumentor/controllers/startlangdetection/namespace_controller.go +++ b/instrumentor/controllers/startlangdetection/namespace_controller.go @@ -3,6 +3,7 @@ package startlangdetection import ( "context" + "github.com/odigos-io/odigos/api/odigos/v1alpha1" "github.com/odigos-io/odigos/common/consts" "github.com/odigos-io/odigos/k8sutils/pkg/workload" @@ -29,7 +30,13 @@ func (n *NamespacesReconciler) Reconcile(ctx context.Context, request ctrl.Reque } if !k8sutils.IsObjectLabeledForInstrumentation(&ns) { - return ctrl.Result{}, nil + sourceList, err := v1alpha1.GetSourceListForWorkload(ctx, n.Client, &ns) + if err != nil { + return ctrl.Result{}, err + } + if len(sourceList.Items) == 0 { + return ctrl.Result{}, nil + } } logger.V(0).Info("Namespace labeled for instrumentation, recalculating runtime details of relevant workloads") diff --git a/instrumentor/controllers/startlangdetection/source_controller.go b/instrumentor/controllers/startlangdetection/source_controller.go index 6e4269981..af4446369 100644 --- a/instrumentor/controllers/startlangdetection/source_controller.go +++ b/instrumentor/controllers/startlangdetection/source_controller.go @@ -3,6 +3,7 @@ package startlangdetection import ( "context" + appsv1 "k8s.io/api/apps/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" @@ -30,54 +31,78 @@ func (r *SourceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr return ctrl.Result{}, client.IgnoreNotFound(err) } - obj := workload.ClientObjectFromWorkloadKind(source.Spec.Workload.Kind) - err = r.Client.Get(ctx, types.NamespacedName{Name: source.Spec.Workload.Name, Namespace: source.Spec.Workload.Namespace}, obj) - if err != nil { - // TODO: Deleted objects should be filtered in the event filter - return ctrl.Result{}, err - } - instConfigName := workload.CalculateWorkloadRuntimeObjectName(source.Spec.Workload.Name, source.Spec.Workload.Kind) - if source.DeletionTimestamp.IsZero() { - if !controllerutil.ContainsFinalizer(source, consts.SourceFinalizer) { - controllerutil.AddFinalizer(source, consts.SourceFinalizer) - // Removed by deleteinstrumentedapplication controller + if !controllerutil.ContainsFinalizer(source, consts.InstrumentedApplicationFinalizer) { controllerutil.AddFinalizer(source, consts.InstrumentedApplicationFinalizer) + } + if source.Labels == nil { + source.Labels = make(map[string]string) + } + + source.Labels[consts.WorkloadNameLabel] = source.Spec.Workload.Name + source.Labels[consts.WorkloadNamespaceLabel] = source.Spec.Workload.Namespace + source.Labels[consts.WorkloadKindLabel] = string(source.Spec.Workload.Kind) + + if err := r.Update(ctx, source); err != nil { + return k8sutils.K8SUpdateErrorHandler(err) + } - if source.Labels == nil { - source.Labels = make(map[string]string) + if source.Spec.Workload.Kind == "Namespace" { + var deps appsv1.DeploymentList + err = r.Client.List(ctx, &deps, client.InNamespace(source.Spec.Workload.Name)) + if client.IgnoreNotFound(err) != nil { + logger.Error(err, "error fetching deployments") + return ctrl.Result{}, err } - source.Labels[consts.WorkloadNameLabel] = source.Spec.Workload.Name - source.Labels[consts.WorkloadNamespaceLabel] = source.Spec.Workload.Namespace - source.Labels[consts.WorkloadKindLabel] = string(source.Spec.Workload.Kind) - if err := r.Update(ctx, source); err != nil { - return k8sutils.K8SUpdateErrorHandler(err) + for _, dep := range deps.Items { + request := ctrl.Request{NamespacedName: client.ObjectKey{Name: dep.Name, Namespace: dep.Namespace}} + _, err = reconcileWorkload(ctx, r.Client, workload.WorkloadKindDeployment, request, r.Scheme) + if err != nil { + logger.Error(err, "error requesting runtime details from odiglets", "name", dep.Name, "namespace", dep.Namespace) + } } - err = requestOdigletsToCalculateRuntimeDetails(ctx, r.Client, instConfigName, req.Namespace, obj, r.Scheme) - return ctrl.Result{}, err - } - } else { - // Source is being deleted - if controllerutil.ContainsFinalizer(source, consts.SourceFinalizer) { - // Remove the finalizer first, because if the InstrumentationConfig is not found we - // will deadlock on the finalizer never getting removed. - // On the other hand, this could end up deleting a Source with an orphaned InstrumentationConfig. - controllerutil.RemoveFinalizer(source, consts.SourceFinalizer) - if err := r.Update(ctx, source); err != nil { + var sts appsv1.StatefulSetList + err = r.Client.List(ctx, &sts, client.InNamespace(source.Spec.Workload.Name)) + if client.IgnoreNotFound(err) != nil { + logger.Error(err, "error fetching statefulsets") return ctrl.Result{}, err } - instConfig := &v1alpha1.InstrumentationConfig{} - err = r.Client.Get(ctx, types.NamespacedName{Name: instConfigName, Namespace: req.Namespace}, instConfig) - if err != nil { - return ctrl.Result{}, client.IgnoreNotFound(err) + for _, st := range sts.Items { + request := ctrl.Request{NamespacedName: client.ObjectKey{Name: st.Name, Namespace: st.Namespace}} + _, err = reconcileWorkload(ctx, r.Client, workload.WorkloadKindStatefulSet, request, r.Scheme) + if err != nil { + logger.Error(err, "error requesting runtime details from odiglets", "name", st.Name, "namespace", st.Namespace) + } } - err = r.Client.Delete(ctx, instConfig) - if err != nil { - return ctrl.Result{}, client.IgnoreNotFound(err) + + var dss appsv1.DaemonSetList + err = r.Client.List(ctx, &dss, client.InNamespace(source.Spec.Workload.Name)) + if client.IgnoreNotFound(err) != nil { + logger.Error(err, "error fetching daemonsets") + return ctrl.Result{}, err + } + + for _, ds := range dss.Items { + request := ctrl.Request{NamespacedName: client.ObjectKey{Name: ds.Name, Namespace: ds.Namespace}} + _, err = reconcileWorkload(ctx, r.Client, workload.WorkloadKindDaemonSet, request, r.Scheme) + if err != nil { + logger.Error(err, "error requesting runtime details from odiglets", "name", ds.Name, "namespace", ds.Namespace) + } } + } else { + return reconcileWorkload(ctx, + r.Client, + source.Spec.Workload.Kind, + ctrl.Request{ + NamespacedName: types.NamespacedName{ + Namespace: source.Spec.Workload.Namespace, + Name: source.Spec.Workload.Name, + }, + }, + r.Scheme) } } diff --git a/instrumentor/controllers/startlangdetection/workload_controllers.go b/instrumentor/controllers/startlangdetection/workload_controllers.go index 0ef0e7489..6abc5bcd1 100644 --- a/instrumentor/controllers/startlangdetection/workload_controllers.go +++ b/instrumentor/controllers/startlangdetection/workload_controllers.go @@ -12,6 +12,7 @@ import ( "github.com/odigos-io/odigos/api/odigos/v1alpha1" odigosv1 "github.com/odigos-io/odigos/api/odigos/v1alpha1" + "github.com/odigos-io/odigos/common/consts" "github.com/odigos-io/odigos/k8sutils/pkg/workload" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" @@ -68,6 +69,12 @@ func reconcileWorkload(ctx context.Context, k8sClient client.Client, objKind wor if len(sourceList.Items) == 0 { return ctrl.Result{}, nil } + // if this is explicitly excluded (ie, namespace instrumentation), skip + for _, s := range sourceList.Items { + if _, exists := s.Labels[consts.OdigosWorkloadExcludedLabel]; exists { + return ctrl.Result{}, nil + } + } } err = requestOdigletsToCalculateRuntimeDetails(ctx, k8sClient, instConfigName, req.Namespace, obj, scheme) From e0a96bc4ebb2b19ff702a6f43e34b5bf555d4c8e Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Mon, 30 Dec 2024 15:44:20 -0500 Subject: [PATCH 03/21] Add Namespace uninstrumentation to deletinstrumentedapplication --- .../deleteinstrumentedapplication/common.go | 4 +- .../namespace_controller.go | 1 - .../source_controller.go | 76 ++++++++++++++++--- 3 files changed, 66 insertions(+), 15 deletions(-) diff --git a/instrumentor/controllers/deleteinstrumentedapplication/common.go b/instrumentor/controllers/deleteinstrumentedapplication/common.go index ef5ee898f..2825374b5 100644 --- a/instrumentor/controllers/deleteinstrumentedapplication/common.go +++ b/instrumentor/controllers/deleteinstrumentedapplication/common.go @@ -46,11 +46,12 @@ func reconcileWorkloadObject(ctx context.Context, kubeClient client.Client, work } func deleteWorkloadInstrumentedApplication(ctx context.Context, kubeClient client.Client, workloadObject client.Object) error { - + logger := log.FromContext(ctx) ns := workloadObject.GetNamespace() name := workloadObject.GetName() kind := workload.WorkloadKindFromClientObject(workloadObject) instrumentedApplicationName := workload.CalculateWorkloadRuntimeObjectName(name, kind) + logger.V(1).Info("deleting instrumented application", "name", instrumentedApplicationName, "kind", kind) instAppErr := kubeClient.Delete(ctx, &odigosv1.InstrumentedApplication{ ObjectMeta: metav1.ObjectMeta{ @@ -73,7 +74,6 @@ func deleteWorkloadInstrumentedApplication(ctx context.Context, kubeClient clien return client.IgnoreNotFound(instConfigErr) } - logger := log.FromContext(ctx) logger.V(1).Info("instrumented application deleted", "namespace", ns, "name", name, "kind", kind) return nil } diff --git a/instrumentor/controllers/deleteinstrumentedapplication/namespace_controller.go b/instrumentor/controllers/deleteinstrumentedapplication/namespace_controller.go index b71c74142..c34ef8b50 100644 --- a/instrumentor/controllers/deleteinstrumentedapplication/namespace_controller.go +++ b/instrumentor/controllers/deleteinstrumentedapplication/namespace_controller.go @@ -137,7 +137,6 @@ func (r *NamespaceReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( } func syncGenericWorkloadListToNs(ctx context.Context, c client.Client, kind workload.WorkloadKind, key client.ObjectKey) error { - // it is very important that we make the changes based on a fresh copy of the workload object // if a list operation pulled in state and is now slowly iterating over it, we might be working with stale data freshWorkloadCopy := workload.ClientObjectFromWorkloadKind(kind) diff --git a/instrumentor/controllers/deleteinstrumentedapplication/source_controller.go b/instrumentor/controllers/deleteinstrumentedapplication/source_controller.go index acd8aba31..395ed6994 100644 --- a/instrumentor/controllers/deleteinstrumentedapplication/source_controller.go +++ b/instrumentor/controllers/deleteinstrumentedapplication/source_controller.go @@ -21,8 +21,10 @@ import ( "github.com/odigos-io/odigos/api/odigos/v1alpha1" "github.com/odigos-io/odigos/k8sutils/pkg/consts" + k8sutils "github.com/odigos-io/odigos/k8sutils/pkg/utils" "github.com/odigos-io/odigos/k8sutils/pkg/workload" + appsv1 "k8s.io/api/apps/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" @@ -38,7 +40,7 @@ type SourceReconciler struct { func (r *SourceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { logger := log.FromContext(ctx) - logger.Info("Reconciling Deleted Source object", "name", req.Name, "namespace", req.Namespace) + logger.Info("Reconciling Source object", "name", req.Name, "namespace", req.Namespace) source := &v1alpha1.Source{} err := r.Get(ctx, req.NamespacedName, source) @@ -48,24 +50,74 @@ func (r *SourceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr if !source.DeletionTimestamp.IsZero() { logger.Info("Reconciling workload for deleted Source object", "name", req.Name, "namespace", req.Namespace) - obj := workload.ClientObjectFromWorkloadKind(source.Spec.Workload.Kind) - err = r.Client.Get(ctx, types.NamespacedName{Name: source.Spec.Workload.Name, Namespace: source.Spec.Workload.Namespace}, obj) - if err != nil { - // TODO: Deleted objects should be filtered in the event filter - return ctrl.Result{}, err + if source.Spec.Workload.Kind == "Namespace" { + logger.V(2).Info("Uninstrumenting deployments for Namespace Source", "name", req.Name, "namespace", req.Namespace) + var deps appsv1.DeploymentList + err = r.Client.List(ctx, &deps, client.InNamespace(req.Namespace)) + if client.IgnoreNotFound(err) != nil { + logger.Error(err, "error fetching deployments") + return ctrl.Result{}, err + } + + for _, dep := range deps.Items { + logger.V(4).Info("uninstrumenting deployment", "name", dep.Name, "namespace", dep.Namespace) + err := syncGenericWorkloadListToNs(ctx, r.Client, workload.WorkloadKindDeployment, client.ObjectKey{Namespace: dep.Namespace, Name: dep.Name}) + if err != nil { + return ctrl.Result{}, err + } + } + + logger.V(2).Info("Uninstrumenting statefulsets for Namespace Source", "name", req.Name, "namespace", req.Namespace) + var ss appsv1.StatefulSetList + err = r.Client.List(ctx, &ss, client.InNamespace(req.Namespace)) + if client.IgnoreNotFound(err) != nil { + logger.Error(err, "error fetching statefulsets") + return ctrl.Result{}, err + } + + for _, s := range ss.Items { + logger.V(4).Info("uninstrumenting statefulset", "name", s.Name, "namespace", s.Namespace) + err := syncGenericWorkloadListToNs(ctx, r.Client, workload.WorkloadKindStatefulSet, client.ObjectKey{Namespace: s.Namespace, Name: s.Name}) + if err != nil { + return ctrl.Result{}, err + } + } + + logger.V(2).Info("Uninstrumenting daemonsets for Namespace Source", "name", req.Name, "namespace", req.Namespace) + var ds appsv1.DaemonSetList + err = r.Client.List(ctx, &ds, client.InNamespace(req.Namespace)) + if client.IgnoreNotFound(err) != nil { + logger.Error(err, "error fetching daemonsets") + return ctrl.Result{}, err + } + + for _, d := range ds.Items { + logger.V(4).Info("uninstrumenting daemonset", "name", d.Name, "namespace", d.Namespace) + err := syncGenericWorkloadListToNs(ctx, r.Client, workload.WorkloadKindDaemonSet, client.ObjectKey{Namespace: d.Namespace, Name: d.Name}) + if err != nil { + return ctrl.Result{}, err + } + } + } else { + obj := workload.ClientObjectFromWorkloadKind(source.Spec.Workload.Kind) + err = r.Client.Get(ctx, types.NamespacedName{Name: source.Spec.Workload.Name, Namespace: source.Spec.Workload.Namespace}, obj) + if err != nil { + // TODO: Deleted objects should be filtered in the event filter + return ctrl.Result{}, err + } + + err = reconcileWorkloadObject(ctx, r.Client, obj) + if err != nil { + return ctrl.Result{}, err + } } if controllerutil.ContainsFinalizer(source, consts.InstrumentedApplicationFinalizer) { controllerutil.RemoveFinalizer(source, consts.InstrumentedApplicationFinalizer) if err := r.Update(ctx, source); err != nil { - return ctrl.Result{}, err + return k8sutils.K8SUpdateErrorHandler(err) } } - - err = reconcileWorkloadObject(ctx, r.Client, obj) - if err != nil { - return ctrl.Result{}, err - } } return ctrl.Result{}, nil } From 2bf63a8b254dd2bf88ab39841faa248d8d0def9e Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Mon, 30 Dec 2024 16:33:38 -0500 Subject: [PATCH 04/21] Add Namespace instrumentation e2e + uninstrumentation e2e --- tests/e2e/source/01-sources.yaml | 10 + tests/e2e/source/02-source-ns.yaml | 12 + tests/e2e/source/chainsaw-test.yaml | 248 ++++++++++++++++++ .../tracesql/context-propagation-2.yaml | 13 + .../tracesql/resource-attributes-2.yaml | 14 + .../source/tracesql/span-attributes-2.yaml | 18 ++ .../e2e/source/tracesql/wait-for-trace-2.yaml | 11 + 7 files changed, 326 insertions(+) create mode 100644 tests/e2e/source/02-source-ns.yaml create mode 100644 tests/e2e/source/tracesql/context-propagation-2.yaml create mode 100644 tests/e2e/source/tracesql/resource-attributes-2.yaml create mode 100644 tests/e2e/source/tracesql/span-attributes-2.yaml create mode 100644 tests/e2e/source/tracesql/wait-for-trace-2.yaml diff --git a/tests/e2e/source/01-sources.yaml b/tests/e2e/source/01-sources.yaml index 3b82128ba..657676383 100644 --- a/tests/e2e/source/01-sources.yaml +++ b/tests/e2e/source/01-sources.yaml @@ -3,6 +3,8 @@ kind: Source metadata: name: frontend namespace: default + labels: + odigos.io/e2e: source spec: workload: name: frontend @@ -14,6 +16,8 @@ kind: Source metadata: name: coupon namespace: default + labels: + odigos.io/e2e: source spec: workload: name: coupon @@ -25,6 +29,8 @@ kind: Source metadata: name: inventory namespace: default + labels: + odigos.io/e2e: source spec: workload: name: inventory @@ -36,6 +42,8 @@ kind: Source metadata: name: pricing namespace: default + labels: + odigos.io/e2e: source spec: workload: name: pricing @@ -47,6 +55,8 @@ kind: Source metadata: name: membership namespace: default + labels: + odigos.io/e2e: source spec: workload: name: membership diff --git a/tests/e2e/source/02-source-ns.yaml b/tests/e2e/source/02-source-ns.yaml new file mode 100644 index 000000000..deb8153f3 --- /dev/null +++ b/tests/e2e/source/02-source-ns.yaml @@ -0,0 +1,12 @@ +apiVersion: odigos.io/v1alpha1 +kind: Source +metadata: + name: default + namespace: default + labels: + odigos.io/e2e: source +spec: + workload: + name: default + namespace: default + kind: Namespace \ No newline at end of file diff --git a/tests/e2e/source/chainsaw-test.yaml b/tests/e2e/source/chainsaw-test.yaml index 7e3e4ff30..9527a4bf6 100644 --- a/tests/e2e/source/chainsaw-test.yaml +++ b/tests/e2e/source/chainsaw-test.yaml @@ -148,3 +148,251 @@ spec: - podLogs: name: odiglet namespace: odigos-system + - name: Uninstrument individual deployments + try: + - delete: + ref: + apiVersion: odigos.io/v1alpha1 + kind: Source + namespace: default + labels: + odigos.io/e2e: source + expect: + - match: + apiVersion: odigos.io/v1alpha1 + kind: InstrumentationConfig + metadata: + name: deployment-coupon + namespace: default + check: + ($error != null): true + - match: + apiVersion: odigos.io/v1alpha1 + kind: InstrumentationConfig + metadata: + name: deployment-frontend + namespace: default + check: + ($error != null): true + - match: + apiVersion: odigos.io/v1alpha1 + kind: InstrumentationConfig + metadata: + name: deployment-inventory + namespace: default + check: + ($error != null): true + - match: + apiVersion: odigos.io/v1alpha1 + kind: InstrumentationConfig + metadata: + name: deployment-membership + namespace: default + check: + ($error != null): true + - match: + apiVersion: odigos.io/v1alpha1 + kind: InstrumentationConfig + metadata: + name: deployment-pricing + namespace: default + check: + ($error != null): true + - match: + apiVersion: odigos.io/v1alpha1 + kind: InstrumentedApplication + metadata: + name: deployment-coupon + namespace: default + check: + ($error != null): true + - match: + apiVersion: odigos.io/v1alpha1 + kind: InstrumentedApplication + metadata: + name: deployment-frontend + namespace: default + check: + ($error != null): true + - match: + apiVersion: odigos.io/v1alpha1 + kind: InstrumentedApplication + metadata: + name: deployment-inventory + namespace: default + check: + ($error != null): true + - match: + apiVersion: odigos.io/v1alpha1 + kind: InstrumentedApplication + metadata: + name: deployment-membership + namespace: default + check: + ($error != null): true + - match: + apiVersion: odigos.io/v1alpha1 + kind: InstrumentedApplication + metadata: + name: deployment-pricing + namespace: default + check: + ($error != null): true + - name: Instrument Namespace + try: + - apply: + file: 02-source-ns.yaml + - name: Assert Runtime Detected + try: + - assert: + timeout: 2m + file: ../../common/assert/simple-demo-runtime-detected.yaml + - name: Generate Traffic + try: + - script: + timeout: 300s + content: | + # Apply the job + kubectl apply -f ../../common/apply/generate-traffic-job.yaml + + # Wait for the job to complete + job_name=$(kubectl get -f ../../common/apply/generate-traffic-job.yaml -o=jsonpath='{.metadata.name}') + kubectl wait --for=condition=complete job/$job_name + + # Delete the job + kubectl delete -f ../../common/apply/generate-traffic-job.yaml + + while true; do + # wait for traces to be available + sleep 8 + + # Run the wait-for-trace script + echo "Running TraceQL test at $(date)" + ../../common/traceql_runner.sh tracesql/wait-for-trace-2.yaml + + if [ $? -eq 0 ]; then + break + else + ../../common/flush_traces.sh + sleep 5 + fi + done + - name: Verify Trace - Context Propagation + try: + - script: + timeout: 30s + content: | + ../../common/traceql_runner.sh tracesql/context-propagation-2.yaml + catch: + - podLogs: + name: odiglet + namespace: odigos-system + - name: Verify Trace - Resource Attributes + try: + - script: + content: | + ../../common/traceql_runner.sh tracesql/resource-attributes-2.yaml + catch: + - podLogs: + name: odiglet + namespace: odigos-system + - name: Verify Trace - Span Attributes + try: + - script: + timeout: 60s + content: | + ../../common/traceql_runner.sh tracesql/span-attributes-2.yaml + catch: + - podLogs: + name: odiglet + namespace: odigos-system + - name: Uninstrument namespace + try: + - delete: + ref: + apiVersion: odigos.io/v1alpha1 + kind: Source + namespace: default + labels: + odigos.io/e2e: source + expect: + - match: + apiVersion: odigos.io/v1alpha1 + kind: InstrumentationConfig + metadata: + name: deployment-coupon + namespace: default + check: + ($error != null): true + - match: + apiVersion: odigos.io/v1alpha1 + kind: InstrumentationConfig + metadata: + name: deployment-frontend + namespace: default + check: + ($error != null): true + - match: + apiVersion: odigos.io/v1alpha1 + kind: InstrumentationConfig + metadata: + name: deployment-inventory + namespace: default + check: + ($error != null): true + - match: + apiVersion: odigos.io/v1alpha1 + kind: InstrumentationConfig + metadata: + name: deployment-membership + namespace: default + check: + ($error != null): true + - match: + apiVersion: odigos.io/v1alpha1 + kind: InstrumentationConfig + metadata: + name: deployment-pricing + namespace: default + check: + ($error != null): true + - match: + apiVersion: odigos.io/v1alpha1 + kind: InstrumentedApplication + metadata: + name: deployment-coupon + namespace: default + check: + ($error != null): true + - match: + apiVersion: odigos.io/v1alpha1 + kind: InstrumentedApplication + metadata: + name: deployment-frontend + namespace: default + check: + ($error != null): true + - match: + apiVersion: odigos.io/v1alpha1 + kind: InstrumentedApplication + metadata: + name: deployment-inventory + namespace: default + check: + ($error != null): true + - match: + apiVersion: odigos.io/v1alpha1 + kind: InstrumentedApplication + metadata: + name: deployment-membership + namespace: default + check: + ($error != null): true + - match: + apiVersion: odigos.io/v1alpha1 + kind: InstrumentedApplication + metadata: + name: deployment-pricing + namespace: default + check: + ($error != null): true \ No newline at end of file diff --git a/tests/e2e/source/tracesql/context-propagation-2.yaml b/tests/e2e/source/tracesql/context-propagation-2.yaml new file mode 100644 index 000000000..4d865d339 --- /dev/null +++ b/tests/e2e/source/tracesql/context-propagation-2.yaml @@ -0,0 +1,13 @@ +apiVersion: e2e.tests.odigos.io/v1 +kind: TraceTest +description: This test checks if the context propagation is working correctly between different languages +query: | + { resource.service.name = "frontend" && resource.telemetry.sdk.language = "java" && + span.http.request.method = "POST" && span.http.route = "/buy" && span:kind = server } + >> ( + { resource.service.name = "pricing" && resource.telemetry.sdk.language = "dotnet" } && + { resource.service.name = "inventory" && resource.telemetry.sdk.language = "python" } && + ({ resource.service.name = "coupon" && resource.telemetry.sdk.language = "nodejs" } + >> { resource.service.name = "membership" && resource.telemetry.sdk.language = "go" })) +expected: + count: 2 \ No newline at end of file diff --git a/tests/e2e/source/tracesql/resource-attributes-2.yaml b/tests/e2e/source/tracesql/resource-attributes-2.yaml new file mode 100644 index 000000000..f6b56aef5 --- /dev/null +++ b/tests/e2e/source/tracesql/resource-attributes-2.yaml @@ -0,0 +1,14 @@ +apiVersion: e2e.tests.odigos.io/v1 +kind: TraceTest +description: | + This test check the following resource attributes: + A. odigos.version attribute exists on all spans and has the correct value + B. Kubernetes attributes are correctly set on all spans + At the time of writing this test, TraceQL api does not support not equal to nil so we use regex instead. +query: | + { resource.odigos.version !~ ".*" || + resource.k8s.deployment.name !~ ".*" || + resource.k8s.node.name !~ "(kind-control-plane|aks-.*)" || + resource.k8s.pod.name !~ ".*" } +expected: + count: 0 \ No newline at end of file diff --git a/tests/e2e/source/tracesql/span-attributes-2.yaml b/tests/e2e/source/tracesql/span-attributes-2.yaml new file mode 100644 index 000000000..84f1071c6 --- /dev/null +++ b/tests/e2e/source/tracesql/span-attributes-2.yaml @@ -0,0 +1,18 @@ +apiVersion: e2e.tests.odigos.io/v1 +kind: TraceTest +description: | + This test checks the span attributes for a specific trace. + TODO - JS, Python and DotNet SDK are not generating data in latest semconv. add additional checks when they are updated. +query: | + { resource.service.name = "frontend" && resource.telemetry.sdk.language = "java" && + span.http.request.method = "POST" && span.http.route = "/buy" && span:kind = server && + span.http.response.status_code = 200 && span.url.query = "id=123" } + >> ( + { resource.service.name = "pricing" && resource.telemetry.sdk.language = "dotnet" && span:kind = server } && + { resource.service.name = "inventory" && resource.telemetry.sdk.language = "python" && span:kind = server } && + ({ resource.service.name = "coupon" && resource.telemetry.sdk.language = "nodejs" && span:kind = server } + >> { resource.service.name = "membership" && resource.telemetry.sdk.language = "go" && + span.http.request.method = "GET" && span:kind = server && + span.http.response.status_code = 200 && span.url.path = "/isMember" })) +expected: + count: 2 \ No newline at end of file diff --git a/tests/e2e/source/tracesql/wait-for-trace-2.yaml b/tests/e2e/source/tracesql/wait-for-trace-2.yaml new file mode 100644 index 000000000..7f06b3148 --- /dev/null +++ b/tests/e2e/source/tracesql/wait-for-trace-2.yaml @@ -0,0 +1,11 @@ +apiVersion: e2e.tests.odigos.io/v1 +kind: TraceTest +description: This test waits for a trace that goes from frontend to pricing, inventory, coupon, and membership services +query: | + { resource.service.name = "frontend" } && + { resource.service.name = "pricing" } && + { resource.service.name = "inventory" } && + { resource.service.name = "coupon" } && + { resource.service.name = "membership" } +expected: + count: 2 \ No newline at end of file From 57b61c53d2f8374e84af844539a2ba32b5ac6f2c Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Thu, 2 Jan 2025 13:17:11 -0500 Subject: [PATCH 05/21] e2e: wait for instrumentation to take effect --- tests/e2e/source/01-workloads.yaml | 44 +++++ tests/e2e/source/02-workloads.yaml | 44 +++++ tests/e2e/source/03-workloads.yaml | 44 +++++ tests/e2e/source/04-workloads.yaml | 44 +++++ tests/e2e/source/chainsaw-test.yaml | 258 +++++++++------------------- 5 files changed, 258 insertions(+), 176 deletions(-) create mode 100644 tests/e2e/source/01-workloads.yaml create mode 100644 tests/e2e/source/02-workloads.yaml create mode 100644 tests/e2e/source/03-workloads.yaml create mode 100644 tests/e2e/source/04-workloads.yaml diff --git a/tests/e2e/source/01-workloads.yaml b/tests/e2e/source/01-workloads.yaml new file mode 100644 index 000000000..32aac4a07 --- /dev/null +++ b/tests/e2e/source/01-workloads.yaml @@ -0,0 +1,44 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "2" + generation: 2 + name: coupon + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "2" + generation: 2 + name: frontend + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "2" + generation: 2 + name: inventory + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "2" + generation: 2 + name: membership + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "2" + generation: 2 + name: pricing + namespace: default \ No newline at end of file diff --git a/tests/e2e/source/02-workloads.yaml b/tests/e2e/source/02-workloads.yaml new file mode 100644 index 000000000..7f6d7828b --- /dev/null +++ b/tests/e2e/source/02-workloads.yaml @@ -0,0 +1,44 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "3" + generation: 3 + name: coupon + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "3" + generation: 3 + name: frontend + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "3" + generation: 3 + name: inventory + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "3" + generation: 3 + name: membership + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "3" + generation: 3 + name: pricing + namespace: default \ No newline at end of file diff --git a/tests/e2e/source/03-workloads.yaml b/tests/e2e/source/03-workloads.yaml new file mode 100644 index 000000000..c0df0577d --- /dev/null +++ b/tests/e2e/source/03-workloads.yaml @@ -0,0 +1,44 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "4" + generation: 4 + name: coupon + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "4" + generation: 4 + name: frontend + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "4" + generation: 4 + name: inventory + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "4" + generation: 4 + name: membership + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "4" + generation: 4 + name: pricing + namespace: default \ No newline at end of file diff --git a/tests/e2e/source/04-workloads.yaml b/tests/e2e/source/04-workloads.yaml new file mode 100644 index 000000000..6f2268d39 --- /dev/null +++ b/tests/e2e/source/04-workloads.yaml @@ -0,0 +1,44 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "5" + generation: 5 + name: coupon + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "5" + generation: 5 + name: frontend + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "5" + generation: 5 + name: inventory + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "5" + generation: 5 + name: membership + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "5" + generation: 5 + name: pricing + namespace: default \ No newline at end of file diff --git a/tests/e2e/source/chainsaw-test.yaml b/tests/e2e/source/chainsaw-test.yaml index 9527a4bf6..d192438e9 100644 --- a/tests/e2e/source/chainsaw-test.yaml +++ b/tests/e2e/source/chainsaw-test.yaml @@ -89,6 +89,22 @@ spec: try: - assert: file: ../../common/assert/simple-demo-instrumented.yaml + - assert: + timeout: 2m + file: 01-workloads.yaml + - script: + timeout: 70s + content: | + kubectl rollout status deployment -l app=coupon + kubectl rollout status deployment -l app=frontend + kubectl rollout status deployment -l app=inventory + kubectl rollout status deployment -l app=pricing + kubectl rollout status deployment -l app=membership + kubectl wait --for=condition=ready pod -l app=frontend --timeout=60s + kubectl wait --for=condition=ready pod -l app=coupon --timeout=60s + kubectl wait --for=condition=ready pod -l app=inventory --timeout=60s + kubectl wait --for=condition=ready pod -l app=pricing --timeout=60s + kubectl wait --for=condition=ready pod -l app=membership --timeout=60s - name: Generate Traffic try: - script: @@ -150,94 +166,35 @@ spec: namespace: odigos-system - name: Uninstrument individual deployments try: - - delete: - ref: - apiVersion: odigos.io/v1alpha1 - kind: Source - namespace: default - labels: - odigos.io/e2e: source - expect: - - match: - apiVersion: odigos.io/v1alpha1 - kind: InstrumentationConfig - metadata: - name: deployment-coupon - namespace: default - check: - ($error != null): true - - match: - apiVersion: odigos.io/v1alpha1 - kind: InstrumentationConfig - metadata: - name: deployment-frontend - namespace: default - check: - ($error != null): true - - match: - apiVersion: odigos.io/v1alpha1 - kind: InstrumentationConfig - metadata: - name: deployment-inventory - namespace: default - check: - ($error != null): true - - match: - apiVersion: odigos.io/v1alpha1 - kind: InstrumentationConfig - metadata: - name: deployment-membership - namespace: default - check: - ($error != null): true - - match: - apiVersion: odigos.io/v1alpha1 - kind: InstrumentationConfig - metadata: - name: deployment-pricing - namespace: default - check: - ($error != null): true - - match: - apiVersion: odigos.io/v1alpha1 - kind: InstrumentedApplication - metadata: - name: deployment-coupon - namespace: default - check: - ($error != null): true - - match: - apiVersion: odigos.io/v1alpha1 - kind: InstrumentedApplication - metadata: - name: deployment-frontend - namespace: default - check: - ($error != null): true - - match: - apiVersion: odigos.io/v1alpha1 - kind: InstrumentedApplication - metadata: - name: deployment-inventory - namespace: default - check: - ($error != null): true - - match: - apiVersion: odigos.io/v1alpha1 - kind: InstrumentedApplication - metadata: - name: deployment-membership - namespace: default - check: - ($error != null): true - - match: - apiVersion: odigos.io/v1alpha1 - kind: InstrumentedApplication - metadata: - name: deployment-pricing - namespace: default - check: - ($error != null): true + - script: + timeout: 60s + content: | + kubectl delete sources --all + while true; do + ic_count=$(kubectl get instrumentationconfigs | wc -l) + if [ $ic_count -eq "0" ]; then + break + fi + sleep 5 + done + - name: Assert workloads updated after uninstrumentation + try: + - assert: + timeout: 2m + file: 02-workloads.yaml + - script: + timeout: 70s + content: | + kubectl rollout status deployment -l app=coupon + kubectl rollout status deployment -l app=frontend + kubectl rollout status deployment -l app=inventory + kubectl rollout status deployment -l app=pricing + kubectl rollout status deployment -l app=membership + kubectl wait --for=condition=ready pod -l app=frontend --timeout=60s + kubectl wait --for=condition=ready pod -l app=coupon --timeout=60s + kubectl wait --for=condition=ready pod -l app=inventory --timeout=60s + kubectl wait --for=condition=ready pod -l app=pricing --timeout=60s + kubectl wait --for=condition=ready pod -l app=membership --timeout=60s - name: Instrument Namespace try: - apply: @@ -247,6 +204,27 @@ spec: - assert: timeout: 2m file: ../../common/assert/simple-demo-runtime-detected.yaml + - assert: + timeout: 2m + file: 03-workloads.yaml + - name: Simple demo instrumented after runtime detection + try: + - assert: + file: ../../common/assert/simple-demo-instrumented.yaml + - script: + timeout: 70s + content: | + kubectl rollout status deployment -l app=coupon + kubectl rollout status deployment -l app=frontend + kubectl rollout status deployment -l app=inventory + kubectl rollout status deployment -l app=pricing + kubectl rollout status deployment -l app=membership + kubectl wait --for=condition=ready pod -l app=frontend --timeout=60s + kubectl wait --for=condition=ready pod -l app=coupon --timeout=60s + kubectl wait --for=condition=ready pod -l app=inventory --timeout=60s + kubectl wait --for=condition=ready pod -l app=pricing --timeout=60s + kubectl wait --for=condition=ready pod -l app=membership --timeout=60s + sleep 5 - name: Generate Traffic try: - script: @@ -308,91 +286,19 @@ spec: namespace: odigos-system - name: Uninstrument namespace try: - - delete: - ref: - apiVersion: odigos.io/v1alpha1 - kind: Source - namespace: default - labels: - odigos.io/e2e: source - expect: - - match: - apiVersion: odigos.io/v1alpha1 - kind: InstrumentationConfig - metadata: - name: deployment-coupon - namespace: default - check: - ($error != null): true - - match: - apiVersion: odigos.io/v1alpha1 - kind: InstrumentationConfig - metadata: - name: deployment-frontend - namespace: default - check: - ($error != null): true - - match: - apiVersion: odigos.io/v1alpha1 - kind: InstrumentationConfig - metadata: - name: deployment-inventory - namespace: default - check: - ($error != null): true - - match: - apiVersion: odigos.io/v1alpha1 - kind: InstrumentationConfig - metadata: - name: deployment-membership - namespace: default - check: - ($error != null): true - - match: - apiVersion: odigos.io/v1alpha1 - kind: InstrumentationConfig - metadata: - name: deployment-pricing - namespace: default - check: - ($error != null): true - - match: - apiVersion: odigos.io/v1alpha1 - kind: InstrumentedApplication - metadata: - name: deployment-coupon - namespace: default - check: - ($error != null): true - - match: - apiVersion: odigos.io/v1alpha1 - kind: InstrumentedApplication - metadata: - name: deployment-frontend - namespace: default - check: - ($error != null): true - - match: - apiVersion: odigos.io/v1alpha1 - kind: InstrumentedApplication - metadata: - name: deployment-inventory - namespace: default - check: - ($error != null): true - - match: - apiVersion: odigos.io/v1alpha1 - kind: InstrumentedApplication - metadata: - name: deployment-membership - namespace: default - check: - ($error != null): true - - match: - apiVersion: odigos.io/v1alpha1 - kind: InstrumentedApplication - metadata: - name: deployment-pricing - namespace: default - check: - ($error != null): true \ No newline at end of file + - script: + timeout: 60s + content: | + kubectl delete sources --all + while true; do + ic_count=$(kubectl get instrumentationconfigs | wc -l) + if [ $ic_count -eq "0" ]; then + break + fi + sleep 5 + done + - name: Assert workloads updated after uninstrumentation + try: + - assert: + timeout: 2m + file: 04-workloads.yaml \ No newline at end of file From c228058742763e08ba58d3176da7b7d70fd1d1bf Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Thu, 2 Jan 2025 16:40:49 -0500 Subject: [PATCH 06/21] Don't delete explicitly instrumented sources when NS is uninstrumented --- .../deleteinstrumentedapplication/common.go | 2 +- .../namespace_controller.go | 26 +++- .../startlangdetection/source_controller.go | 46 +++++-- .../workload_controllers.go | 3 +- .../source/05-assert-runtime-detected.yaml | 32 +++++ tests/e2e/source/05-source.yaml | 12 ++ tests/e2e/source/05-workloads.yaml | 44 +++++++ tests/e2e/source/06-workloads.yaml | 44 +++++++ tests/e2e/source/07-workloads.yaml | 44 +++++++ tests/e2e/source/chainsaw-test.yaml | 117 +++++++++++++++++- 10 files changed, 346 insertions(+), 24 deletions(-) create mode 100644 tests/e2e/source/05-assert-runtime-detected.yaml create mode 100644 tests/e2e/source/05-source.yaml create mode 100644 tests/e2e/source/05-workloads.yaml create mode 100644 tests/e2e/source/06-workloads.yaml create mode 100644 tests/e2e/source/07-workloads.yaml diff --git a/instrumentor/controllers/deleteinstrumentedapplication/common.go b/instrumentor/controllers/deleteinstrumentedapplication/common.go index 2825374b5..d88a4ec42 100644 --- a/instrumentor/controllers/deleteinstrumentedapplication/common.go +++ b/instrumentor/controllers/deleteinstrumentedapplication/common.go @@ -27,7 +27,7 @@ func reconcileWorkloadObject(ctx context.Context, kubeClient client.Client, work if err != nil { return err } - if len(sourceList.Items) == 0 { + if len(sourceList.Items) > 0 { return nil } } diff --git a/instrumentor/controllers/deleteinstrumentedapplication/namespace_controller.go b/instrumentor/controllers/deleteinstrumentedapplication/namespace_controller.go index c34ef8b50..e24779184 100644 --- a/instrumentor/controllers/deleteinstrumentedapplication/namespace_controller.go +++ b/instrumentor/controllers/deleteinstrumentedapplication/namespace_controller.go @@ -20,6 +20,7 @@ import ( "context" "errors" + "github.com/odigos-io/odigos/api/odigos/v1alpha1" "github.com/odigos-io/odigos/common/consts" "github.com/odigos-io/odigos/k8sutils/pkg/workload" @@ -150,11 +151,14 @@ func syncGenericWorkloadListToNs(ctx context.Context, c client.Client, kind work } } - if !isInheritingInstrumentationFromNs(freshWorkloadCopy) { + inheriting, err := isInheritingInstrumentationFromNs(ctx, c, freshWorkloadCopy) + if err != nil { + return err + } + if !inheriting { return nil } - var err error err = errors.Join(err, deleteWorkloadInstrumentedApplication(ctx, c, freshWorkloadCopy)) err = errors.Join(err, removeReportedNameAnnotation(ctx, c, freshWorkloadCopy)) return err @@ -164,11 +168,23 @@ func syncGenericWorkloadListToNs(ctx context.Context, c client.Client, kind work // when reconciling the namespace, the usecase is to delete instrumentation for workloads that were only // instrumented due to the label on the namespace. These are workloads with the label missing. // (they inherit the instrumentation from the namespace this way) -func isInheritingInstrumentationFromNs(obj client.Object) bool { +func isInheritingInstrumentationFromNs(ctx context.Context, c client.Client, obj client.Object) (bool, error) { + sourceList, err := v1alpha1.GetSourceListForWorkload(ctx, c, obj) + if err != nil { + return false, err + } + for _, source := range sourceList.Items { + // if a source exists for this workload specifically, then it is not inheriting from Namespace + if source.Spec.Workload.Name == obj.GetName() && + source.Spec.Workload.Kind == workload.WorkloadKind(obj.GetObjectKind().GroupVersionKind().Kind) { + return false, nil + } + } + labels := obj.GetLabels() if labels == nil { - return true + return true, nil } _, exists := labels[consts.OdigosInstrumentationLabel] - return !exists + return !exists, nil } diff --git a/instrumentor/controllers/startlangdetection/source_controller.go b/instrumentor/controllers/startlangdetection/source_controller.go index af4446369..373cd26ba 100644 --- a/instrumentor/controllers/startlangdetection/source_controller.go +++ b/instrumentor/controllers/startlangdetection/source_controller.go @@ -47,6 +47,22 @@ func (r *SourceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr return k8sutils.K8SUpdateErrorHandler(err) } + // pre-process existing Sources for specific workloads so we don't have to make a bunch of API calls + // This is used to check if a workload already has an explicit Source, so we don't overwrite its InstrumentationConfig + sourceList := v1alpha1.SourceList{} + err := r.Client.List(ctx, &sourceList, client.InNamespace(source.Spec.Workload.Name)) + if err != nil { + return ctrl.Result{}, err + } + namespaceKindSources := make(map[workload.WorkloadKind]map[string]struct{}) + for _, source := range sourceList.Items { + if _, exists := namespaceKindSources[source.Spec.Workload.Kind]; !exists { + namespaceKindSources[source.Spec.Workload.Kind] = make(map[string]struct{}) + } + // ex: map["Deployment"]["my-app"] = ... + namespaceKindSources[source.Spec.Workload.Kind][source.Spec.Workload.Name] = struct{}{} + } + if source.Spec.Workload.Kind == "Namespace" { var deps appsv1.DeploymentList err = r.Client.List(ctx, &deps, client.InNamespace(source.Spec.Workload.Name)) @@ -56,10 +72,12 @@ func (r *SourceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr } for _, dep := range deps.Items { - request := ctrl.Request{NamespacedName: client.ObjectKey{Name: dep.Name, Namespace: dep.Namespace}} - _, err = reconcileWorkload(ctx, r.Client, workload.WorkloadKindDeployment, request, r.Scheme) - if err != nil { - logger.Error(err, "error requesting runtime details from odiglets", "name", dep.Name, "namespace", dep.Namespace) + if _, exists := namespaceKindSources[workload.WorkloadKindDeployment][dep.Name]; !exists { + request := ctrl.Request{NamespacedName: client.ObjectKey{Name: dep.Name, Namespace: dep.Namespace}} + _, err = reconcileWorkload(ctx, r.Client, workload.WorkloadKindDeployment, request, r.Scheme) + if err != nil { + logger.Error(err, "error requesting runtime details from odiglets", "name", dep.Name, "namespace", dep.Namespace) + } } } @@ -71,10 +89,12 @@ func (r *SourceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr } for _, st := range sts.Items { - request := ctrl.Request{NamespacedName: client.ObjectKey{Name: st.Name, Namespace: st.Namespace}} - _, err = reconcileWorkload(ctx, r.Client, workload.WorkloadKindStatefulSet, request, r.Scheme) - if err != nil { - logger.Error(err, "error requesting runtime details from odiglets", "name", st.Name, "namespace", st.Namespace) + if _, exists := namespaceKindSources[workload.WorkloadKindStatefulSet][st.Name]; !exists { + request := ctrl.Request{NamespacedName: client.ObjectKey{Name: st.Name, Namespace: st.Namespace}} + _, err = reconcileWorkload(ctx, r.Client, workload.WorkloadKindStatefulSet, request, r.Scheme) + if err != nil { + logger.Error(err, "error requesting runtime details from odiglets", "name", st.Name, "namespace", st.Namespace) + } } } @@ -86,10 +106,12 @@ func (r *SourceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr } for _, ds := range dss.Items { - request := ctrl.Request{NamespacedName: client.ObjectKey{Name: ds.Name, Namespace: ds.Namespace}} - _, err = reconcileWorkload(ctx, r.Client, workload.WorkloadKindDaemonSet, request, r.Scheme) - if err != nil { - logger.Error(err, "error requesting runtime details from odiglets", "name", ds.Name, "namespace", ds.Namespace) + if _, exists := namespaceKindSources[workload.WorkloadKindDaemonSet][ds.Name]; !exists { + request := ctrl.Request{NamespacedName: client.ObjectKey{Name: ds.Name, Namespace: ds.Namespace}} + _, err = reconcileWorkload(ctx, r.Client, workload.WorkloadKindDaemonSet, request, r.Scheme) + if err != nil { + logger.Error(err, "error requesting runtime details from odiglets", "name", ds.Name, "namespace", ds.Namespace) + } } } } else { diff --git a/instrumentor/controllers/startlangdetection/workload_controllers.go b/instrumentor/controllers/startlangdetection/workload_controllers.go index 6abc5bcd1..62e3b7a51 100644 --- a/instrumentor/controllers/startlangdetection/workload_controllers.go +++ b/instrumentor/controllers/startlangdetection/workload_controllers.go @@ -10,7 +10,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log" - "github.com/odigos-io/odigos/api/odigos/v1alpha1" odigosv1 "github.com/odigos-io/odigos/api/odigos/v1alpha1" "github.com/odigos-io/odigos/common/consts" "github.com/odigos-io/odigos/k8sutils/pkg/workload" @@ -62,7 +61,7 @@ func reconcileWorkload(ctx context.Context, k8sClient client.Client, objKind wor if !instrumented { // Check if a Source object exists for this workload - sourceList, err := v1alpha1.GetSourceListForWorkload(ctx, k8sClient, obj) + sourceList, err := odigosv1.GetSourceListForWorkload(ctx, k8sClient, obj) if err != nil { return ctrl.Result{}, err } diff --git a/tests/e2e/source/05-assert-runtime-detected.yaml b/tests/e2e/source/05-assert-runtime-detected.yaml new file mode 100644 index 000000000..a2771ca7b --- /dev/null +++ b/tests/e2e/source/05-assert-runtime-detected.yaml @@ -0,0 +1,32 @@ +apiVersion: odigos.io/v1alpha1 +kind: InstrumentedApplication +metadata: + name: deployment-frontend + namespace: default + ownerReferences: + - apiVersion: apps/v1 + blockOwnerDeletion: true + controller: true + kind: Deployment + name: frontend +spec: + runtimeDetails: + - containerName: frontend + language: java +--- +apiVersion: odigos.io/v1alpha1 +kind: InstrumentationConfig +metadata: + name: deployment-frontend + namespace: default + ownerReferences: + - apiVersion: apps/v1 + blockOwnerDeletion: true + controller: true + kind: Deployment + name: frontend +status: + runtimeDetailsByContainer: + - containerName: frontend + language: java + runtimeVersion: 17.0.11+9 diff --git a/tests/e2e/source/05-source.yaml b/tests/e2e/source/05-source.yaml new file mode 100644 index 000000000..d3382faae --- /dev/null +++ b/tests/e2e/source/05-source.yaml @@ -0,0 +1,12 @@ +apiVersion: odigos.io/v1alpha1 +kind: Source +metadata: + name: frontend + namespace: default + labels: + odigos.io/e2e: source-single +spec: + workload: + name: frontend + namespace: default + kind: Deployment diff --git a/tests/e2e/source/05-workloads.yaml b/tests/e2e/source/05-workloads.yaml new file mode 100644 index 000000000..d86795233 --- /dev/null +++ b/tests/e2e/source/05-workloads.yaml @@ -0,0 +1,44 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "5" + generation: 5 + name: coupon + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "6" # should be only one updated right now + generation: 6 + name: frontend + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "5" + generation: 5 + name: inventory + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "5" + generation: 5 + name: membership + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "5" + generation: 5 + name: pricing + namespace: default \ No newline at end of file diff --git a/tests/e2e/source/06-workloads.yaml b/tests/e2e/source/06-workloads.yaml new file mode 100644 index 000000000..7be83b537 --- /dev/null +++ b/tests/e2e/source/06-workloads.yaml @@ -0,0 +1,44 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "6" + generation: 6 + name: coupon + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "6" + generation: 6 + name: frontend + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "6" + generation: 6 + name: inventory + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "6" + generation: 6 + name: membership + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "6" + generation: 6 + name: pricing + namespace: default \ No newline at end of file diff --git a/tests/e2e/source/07-workloads.yaml b/tests/e2e/source/07-workloads.yaml new file mode 100644 index 000000000..dde612ffa --- /dev/null +++ b/tests/e2e/source/07-workloads.yaml @@ -0,0 +1,44 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "7" + generation: 7 + name: coupon + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "6" + generation: 6 + name: frontend + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "7" + generation: 7 + name: inventory + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "7" + generation: 7 + name: membership + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "7" + generation: 7 + name: pricing + namespace: default \ No newline at end of file diff --git a/tests/e2e/source/chainsaw-test.yaml b/tests/e2e/source/chainsaw-test.yaml index d192438e9..83cdbc661 100644 --- a/tests/e2e/source/chainsaw-test.yaml +++ b/tests/e2e/source/chainsaw-test.yaml @@ -100,11 +100,22 @@ spec: kubectl rollout status deployment -l app=inventory kubectl rollout status deployment -l app=pricing kubectl rollout status deployment -l app=membership + - name: Simple-demo pods ready for traffic + try: + - script: + timeout: 70s + content: | kubectl wait --for=condition=ready pod -l app=frontend --timeout=60s kubectl wait --for=condition=ready pod -l app=coupon --timeout=60s kubectl wait --for=condition=ready pod -l app=inventory --timeout=60s kubectl wait --for=condition=ready pod -l app=pricing --timeout=60s kubectl wait --for=condition=ready pod -l app=membership --timeout=60s + - name: Wait for apps auto-instrumentation + try: + - script: + timeout: 60s + content: | + sleep 30 - name: Generate Traffic try: - script: @@ -171,7 +182,7 @@ spec: content: | kubectl delete sources --all while true; do - ic_count=$(kubectl get instrumentationconfigs | wc -l) + ic_count=$(kubectl get instrumentationconfigs --output name | wc -l) if [ $ic_count -eq "0" ]; then break fi @@ -190,6 +201,11 @@ spec: kubectl rollout status deployment -l app=inventory kubectl rollout status deployment -l app=pricing kubectl rollout status deployment -l app=membership + - name: Simple-demo pods ready for traffic + try: + - script: + timeout: 70s + content: | kubectl wait --for=condition=ready pod -l app=frontend --timeout=60s kubectl wait --for=condition=ready pod -l app=coupon --timeout=60s kubectl wait --for=condition=ready pod -l app=inventory --timeout=60s @@ -219,12 +235,22 @@ spec: kubectl rollout status deployment -l app=inventory kubectl rollout status deployment -l app=pricing kubectl rollout status deployment -l app=membership + - name: Simple-demo pods ready for traffic + try: + - script: + timeout: 70s + content: | kubectl wait --for=condition=ready pod -l app=frontend --timeout=60s kubectl wait --for=condition=ready pod -l app=coupon --timeout=60s kubectl wait --for=condition=ready pod -l app=inventory --timeout=60s kubectl wait --for=condition=ready pod -l app=pricing --timeout=60s kubectl wait --for=condition=ready pod -l app=membership --timeout=60s - sleep 5 + - name: Wait for apps auto-instrumentation + try: + - script: + timeout: 60s + content: | + sleep 30 - name: Generate Traffic try: - script: @@ -291,7 +317,7 @@ spec: content: | kubectl delete sources --all while true; do - ic_count=$(kubectl get instrumentationconfigs | wc -l) + ic_count=$(kubectl get instrumentationconfigs --output name | wc -l) if [ $ic_count -eq "0" ]; then break fi @@ -301,4 +327,87 @@ spec: try: - assert: timeout: 2m - file: 04-workloads.yaml \ No newline at end of file + file: 04-workloads.yaml + - script: + timeout: 70s + content: | + kubectl rollout status deployment -l app=coupon + kubectl rollout status deployment -l app=frontend + kubectl rollout status deployment -l app=inventory + kubectl rollout status deployment -l app=pricing + kubectl rollout status deployment -l app=membership + - name: Simple-demo pods ready for traffic + try: + - script: + timeout: 70s + content: | + kubectl wait --for=condition=ready pod -l app=frontend --timeout=60s + kubectl wait --for=condition=ready pod -l app=coupon --timeout=60s + kubectl wait --for=condition=ready pod -l app=inventory --timeout=60s + kubectl wait --for=condition=ready pod -l app=pricing --timeout=60s + kubectl wait --for=condition=ready pod -l app=membership --timeout=60s + - name: Instrument frontend workload specifically + try: + - apply: + file: 05-source.yaml + - name: Assert Runtime Detected for single workload + try: + - assert: + timeout: 2m + file: 05-assert-runtime-detected.yaml + - assert: + timeout: 2m + file: 05-workloads.yaml + - name: Single workload instrumented after runtime detection + try: + - script: + timeout: 70s + content: | + kubectl rollout status deployment -l app=coupon + kubectl rollout status deployment -l app=frontend + kubectl rollout status deployment -l app=inventory + kubectl rollout status deployment -l app=pricing + kubectl rollout status deployment -l app=membership + - name: Simple-demo pods ready for traffic + try: + - script: + timeout: 70s + content: | + kubectl wait --for=condition=ready pod -l app=frontend --timeout=60s + kubectl wait --for=condition=ready pod -l app=coupon --timeout=60s + kubectl wait --for=condition=ready pod -l app=inventory --timeout=60s + kubectl wait --for=condition=ready pod -l app=pricing --timeout=60s + kubectl wait --for=condition=ready pod -l app=membership --timeout=60s + - name: Instrument rest of Namespace + try: + - apply: + file: 02-source-ns.yaml + - name: Assert Runtime Detected for all workloads + try: + - assert: + timeout: 2m + file: ../../common/assert/simple-demo-runtime-detected.yaml + - assert: + timeout: 2m + file: 06-workloads.yaml + - name: Uninstrument namespace + try: + - script: + timeout: 60s + content: | + kubectl delete sources/default + while true; do + ic_count=$(kubectl get instrumentationconfigs --output name | wc -l) + if [ $ic_count -eq "1" ]; then + break + fi + sleep 5 + done + - name: Assert Runtime still Detected for single workload + try: + - assert: + timeout: 2m + file: 05-assert-runtime-detected.yaml + - assert: + timeout: 2m + file: 07-workloads.yaml \ No newline at end of file From ea8ac3c0e34aeb3f943f67771d83cce5c2424546 Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Thu, 2 Jan 2025 17:33:21 -0500 Subject: [PATCH 07/21] Fix instEffectEnabled check in deleteinstrumentedapplication --- .../deleteinstrumentedapplication/common.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/instrumentor/controllers/deleteinstrumentedapplication/common.go b/instrumentor/controllers/deleteinstrumentedapplication/common.go index d88a4ec42..7fc4d45e5 100644 --- a/instrumentor/controllers/deleteinstrumentedapplication/common.go +++ b/instrumentor/controllers/deleteinstrumentedapplication/common.go @@ -22,14 +22,16 @@ func reconcileWorkloadObject(ctx context.Context, kubeClient client.Client, work } if instEffectiveEnabled { - // Check if a Source object exists for this workload - sourceList, err := v1alpha1.GetSourceListForWorkload(ctx, kubeClient, workloadObject) - if err != nil { - return err - } - if len(sourceList.Items) > 0 { - return nil - } + return nil + } + + // Check if a Source object exists for this workload + sourceList, err := v1alpha1.GetSourceListForWorkload(ctx, kubeClient, workloadObject) + if err != nil { + return err + } + if len(sourceList.Items) > 0 { + return nil } if err := deleteWorkloadInstrumentedApplication(ctx, kubeClient, workloadObject); err != nil { From 867601594f0a2162f97e5d1577e4a9a5df190909 Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Fri, 3 Jan 2025 08:42:55 -0500 Subject: [PATCH 08/21] Fix inheritance check for individual workload sources --- .../deleteinstrumentedapplication/common.go | 2 ++ .../source_controller.go | 15 ++++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/instrumentor/controllers/deleteinstrumentedapplication/common.go b/instrumentor/controllers/deleteinstrumentedapplication/common.go index 7fc4d45e5..ddb44be20 100644 --- a/instrumentor/controllers/deleteinstrumentedapplication/common.go +++ b/instrumentor/controllers/deleteinstrumentedapplication/common.go @@ -31,6 +31,8 @@ func reconcileWorkloadObject(ctx context.Context, kubeClient client.Client, work return err } if len(sourceList.Items) > 0 { + // Note that if a Source is being deleted, but still has the finalizer, it will still show up in this List + // So we can't use this check to trigger un-instrumentation via Source deletion return nil } diff --git a/instrumentor/controllers/deleteinstrumentedapplication/source_controller.go b/instrumentor/controllers/deleteinstrumentedapplication/source_controller.go index 395ed6994..a19915401 100644 --- a/instrumentor/controllers/deleteinstrumentedapplication/source_controller.go +++ b/instrumentor/controllers/deleteinstrumentedapplication/source_controller.go @@ -18,6 +18,7 @@ package deleteinstrumentedapplication import ( "context" + "errors" "github.com/odigos-io/odigos/api/odigos/v1alpha1" "github.com/odigos-io/odigos/k8sutils/pkg/consts" @@ -99,6 +100,7 @@ func (r *SourceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr } } } else { + // This is a Source for a specific workload, not an entire namespace obj := workload.ClientObjectFromWorkloadKind(source.Spec.Workload.Kind) err = r.Client.Get(ctx, types.NamespacedName{Name: source.Spec.Workload.Name, Namespace: source.Spec.Workload.Namespace}, obj) if err != nil { @@ -106,7 +108,18 @@ func (r *SourceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr return ctrl.Result{}, err } - err = reconcileWorkloadObject(ctx, r.Client, obj) + // Check if this workload is also inheriting Instrumentation from namespace + inheriting, err := isInheritingInstrumentationFromNs(ctx, r.Client, obj) + if err != nil { + return ctrl.Result{}, err + } + if inheriting { + // if it is inheriting, don't delete the instrumentation config + return ctrl.Result{}, err + } + + err = errors.Join(err, deleteWorkloadInstrumentedApplication(ctx, r.Client, obj)) + err = errors.Join(err, removeReportedNameAnnotation(ctx, r.Client, obj)) if err != nil { return ctrl.Result{}, err } From 253d62282bec20a295830a1b484797c6f553402c Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Fri, 3 Jan 2025 10:15:42 -0500 Subject: [PATCH 09/21] Retry traffic generator -- check for minimum spans --- tests/common/traceql_runner.sh | 31 ++++++++--- tests/e2e/source/03-workloads.yaml | 10 ++-- tests/e2e/source/04-workloads.yaml | 10 ++-- tests/e2e/source/05-workloads.yaml | 10 ++-- tests/e2e/source/06-workloads.yaml | 10 ++-- tests/e2e/source/07-workloads.yaml | 10 ++-- tests/e2e/source/chainsaw-test.yaml | 52 +++++++++++-------- .../tracesql/context-propagation-2.yaml | 12 ++--- .../source/tracesql/context-propagation.yaml | 2 +- .../tracesql/resource-attributes-2.yaml | 14 ----- .../source/tracesql/span-attributes-2.yaml | 12 ++--- .../e2e/source/tracesql/span-attributes.yaml | 2 +- .../e2e/source/tracesql/wait-for-trace-2.yaml | 12 ++--- tests/e2e/source/tracesql/wait-for-trace.yaml | 2 +- 14 files changed, 100 insertions(+), 89 deletions(-) delete mode 100644 tests/e2e/source/tracesql/resource-attributes-2.yaml diff --git a/tests/common/traceql_runner.sh b/tests/common/traceql_runner.sh index 052bee1a8..af7fb1d2c 100755 --- a/tests/common/traceql_runner.sh +++ b/tests/common/traceql_runner.sh @@ -8,8 +8,9 @@ function verify_yaml_schema() { local file=$1 local query=$(yq e '.query' "$file") local expected_count=$(yq e '.expected.count' "$file") + local minimum_count=$(yq e '.expected.minimum' "$file") - if [ -z "$query" ] || [ "$expected_count" == "null" ] || [ -z "$expected_count" ]; then + if [[ -z "$query" || ( "$expected_count" == "null" && "$minimum_count" == "null" ) || ( -z "$minimum_count" && -z "$expected_count" ) ]]; then echo "Invalid YAML schema in file: $file" exit 1 fi @@ -38,19 +39,33 @@ function process_yaml_file() { query=$(yq '.query' "$file") encoded_query=$(urlencode "$query") expected_count=$(yq e '.expected.count' "$file") + minimum_count=$(yq e '.expected.minimum' "$file") current_epoch=$(date +%s) one_hour=3600 start_epoch=$(($current_epoch - one_hour)) end_epoch=$(($current_epoch + one_hour)) response=$(kubectl get --raw /api/v1/namespaces/$dest_namespace/services/$dest_service:$dest_port/proxy/api/search\?end=$end_epoch\&start=$start_epoch\&q=$encoded_query\&limit=50) num_of_traces=$(echo $response | jq '.traces | length') - # if num_of_traces not equal to expected_count - if [ "$num_of_traces" -ne "$expected_count" ]; then - echo "Test FAILED: expected $expected_count got $num_of_traces" - exit 1 - else - echo "Test PASSED: expected $expected_count got $num_of_traces" - exit 0 + + if [ "$expected_count" != "null" ]; then + # if num_of_traces not equal to expected_count + if [ "$num_of_traces" -ne "$expected_count" ]; then + echo "Test FAILED: expected $expected_count got $num_of_traces" + exit 1 + else + echo "Test PASSED: expected $expected_count got $num_of_traces" + exit 0 + fi + fi + + if [ "$minimum_count" != "null" ]; then + if [ "$num_of_traces" -lt "$minimum_count" ]; then + echo "Test FAILED: expected at least $minimum_count got $num_of_traces" + exit 1 + else + echo "Test PASSED: expected at least $minimum_count got $num_of_traces" + exit 0 + fi fi } diff --git a/tests/e2e/source/03-workloads.yaml b/tests/e2e/source/03-workloads.yaml index c0df0577d..0a63cc552 100644 --- a/tests/e2e/source/03-workloads.yaml +++ b/tests/e2e/source/03-workloads.yaml @@ -3,7 +3,7 @@ kind: Deployment metadata: annotations: deployment.kubernetes.io/revision: "4" - generation: 4 + generation: 5 # Note new generation after annotating service.name, but not new revision name: coupon namespace: default --- @@ -12,7 +12,7 @@ kind: Deployment metadata: annotations: deployment.kubernetes.io/revision: "4" - generation: 4 + generation: 5 # Note new generation after annotating service.name, but not new revision name: frontend namespace: default --- @@ -21,7 +21,7 @@ kind: Deployment metadata: annotations: deployment.kubernetes.io/revision: "4" - generation: 4 + generation: 5 # Note new generation after annotating service.name, but not new revision name: inventory namespace: default --- @@ -30,7 +30,7 @@ kind: Deployment metadata: annotations: deployment.kubernetes.io/revision: "4" - generation: 4 + generation: 5 # Note new generation after annotating service.name, but not new revision name: membership namespace: default --- @@ -39,6 +39,6 @@ kind: Deployment metadata: annotations: deployment.kubernetes.io/revision: "4" - generation: 4 + generation: 5 # Note new generation after annotating service.name, but not new revision name: pricing namespace: default \ No newline at end of file diff --git a/tests/e2e/source/04-workloads.yaml b/tests/e2e/source/04-workloads.yaml index 6f2268d39..1cc4d4598 100644 --- a/tests/e2e/source/04-workloads.yaml +++ b/tests/e2e/source/04-workloads.yaml @@ -3,7 +3,7 @@ kind: Deployment metadata: annotations: deployment.kubernetes.io/revision: "5" - generation: 5 + generation: 7 # Generation jumps 2 because Odigos removes the reported-name annotation name: coupon namespace: default --- @@ -12,7 +12,7 @@ kind: Deployment metadata: annotations: deployment.kubernetes.io/revision: "5" - generation: 5 + generation: 7 name: frontend namespace: default --- @@ -21,7 +21,7 @@ kind: Deployment metadata: annotations: deployment.kubernetes.io/revision: "5" - generation: 5 + generation: 7 name: inventory namespace: default --- @@ -30,7 +30,7 @@ kind: Deployment metadata: annotations: deployment.kubernetes.io/revision: "5" - generation: 5 + generation: 7 name: membership namespace: default --- @@ -39,6 +39,6 @@ kind: Deployment metadata: annotations: deployment.kubernetes.io/revision: "5" - generation: 5 + generation: 7 name: pricing namespace: default \ No newline at end of file diff --git a/tests/e2e/source/05-workloads.yaml b/tests/e2e/source/05-workloads.yaml index d86795233..18770b336 100644 --- a/tests/e2e/source/05-workloads.yaml +++ b/tests/e2e/source/05-workloads.yaml @@ -3,7 +3,7 @@ kind: Deployment metadata: annotations: deployment.kubernetes.io/revision: "5" - generation: 5 + generation: 7 name: coupon namespace: default --- @@ -12,7 +12,7 @@ kind: Deployment metadata: annotations: deployment.kubernetes.io/revision: "6" # should be only one updated right now - generation: 6 + generation: 8 name: frontend namespace: default --- @@ -21,7 +21,7 @@ kind: Deployment metadata: annotations: deployment.kubernetes.io/revision: "5" - generation: 5 + generation: 7 name: inventory namespace: default --- @@ -30,7 +30,7 @@ kind: Deployment metadata: annotations: deployment.kubernetes.io/revision: "5" - generation: 5 + generation: 7 name: membership namespace: default --- @@ -39,6 +39,6 @@ kind: Deployment metadata: annotations: deployment.kubernetes.io/revision: "5" - generation: 5 + generation: 7 name: pricing namespace: default \ No newline at end of file diff --git a/tests/e2e/source/06-workloads.yaml b/tests/e2e/source/06-workloads.yaml index 7be83b537..594f8e211 100644 --- a/tests/e2e/source/06-workloads.yaml +++ b/tests/e2e/source/06-workloads.yaml @@ -3,7 +3,7 @@ kind: Deployment metadata: annotations: deployment.kubernetes.io/revision: "6" - generation: 6 + generation: 8 name: coupon namespace: default --- @@ -12,7 +12,7 @@ kind: Deployment metadata: annotations: deployment.kubernetes.io/revision: "6" - generation: 6 + generation: 8 name: frontend namespace: default --- @@ -21,7 +21,7 @@ kind: Deployment metadata: annotations: deployment.kubernetes.io/revision: "6" - generation: 6 + generation: 8 name: inventory namespace: default --- @@ -30,7 +30,7 @@ kind: Deployment metadata: annotations: deployment.kubernetes.io/revision: "6" - generation: 6 + generation: 8 name: membership namespace: default --- @@ -39,6 +39,6 @@ kind: Deployment metadata: annotations: deployment.kubernetes.io/revision: "6" - generation: 6 + generation: 8 name: pricing namespace: default \ No newline at end of file diff --git a/tests/e2e/source/07-workloads.yaml b/tests/e2e/source/07-workloads.yaml index dde612ffa..007b2e53b 100644 --- a/tests/e2e/source/07-workloads.yaml +++ b/tests/e2e/source/07-workloads.yaml @@ -3,7 +3,7 @@ kind: Deployment metadata: annotations: deployment.kubernetes.io/revision: "7" - generation: 7 + generation: 9 name: coupon namespace: default --- @@ -12,7 +12,7 @@ kind: Deployment metadata: annotations: deployment.kubernetes.io/revision: "6" - generation: 6 + generation: 8 name: frontend namespace: default --- @@ -21,7 +21,7 @@ kind: Deployment metadata: annotations: deployment.kubernetes.io/revision: "7" - generation: 7 + generation: 9 name: inventory namespace: default --- @@ -30,7 +30,7 @@ kind: Deployment metadata: annotations: deployment.kubernetes.io/revision: "7" - generation: 7 + generation: 9 name: membership namespace: default --- @@ -39,6 +39,6 @@ kind: Deployment metadata: annotations: deployment.kubernetes.io/revision: "7" - generation: 7 + generation: 9 name: pricing namespace: default \ No newline at end of file diff --git a/tests/e2e/source/chainsaw-test.yaml b/tests/e2e/source/chainsaw-test.yaml index 83cdbc661..9840798eb 100644 --- a/tests/e2e/source/chainsaw-test.yaml +++ b/tests/e2e/source/chainsaw-test.yaml @@ -121,20 +121,20 @@ spec: - script: timeout: 300s content: | - # Apply the job - kubectl apply -f ../../common/apply/generate-traffic-job.yaml - - # Wait for the job to complete - job_name=$(kubectl get -f ../../common/apply/generate-traffic-job.yaml -o=jsonpath='{.metadata.name}') - kubectl wait --for=condition=complete job/$job_name - - # Delete the job - kubectl delete -f ../../common/apply/generate-traffic-job.yaml - while true; do # wait for traces to be available sleep 8 + # Apply the job + kubectl apply -f ../../common/apply/generate-traffic-job.yaml + + # Wait for the job to complete + job_name=$(kubectl get -f ../../common/apply/generate-traffic-job.yaml -o=jsonpath='{.metadata.name}') + kubectl wait --for=condition=complete job/$job_name + + # Delete the job + kubectl delete -f ../../common/apply/generate-traffic-job.yaml + # Run the wait-for-trace script echo "Running TraceQL test at $(date)" ../../common/traceql_runner.sh tracesql/wait-for-trace.yaml @@ -211,6 +211,16 @@ spec: kubectl wait --for=condition=ready pod -l app=inventory --timeout=60s kubectl wait --for=condition=ready pod -l app=pricing --timeout=60s kubectl wait --for=condition=ready pod -l app=membership --timeout=60s + - name: Update reported service name for second phase of test + try: + - script: + timeout: 70s + content: | + kubectl annotate deployment coupon odigos.io/reported-name=coupon-2 + kubectl annotate deployment frontend odigos.io/reported-name=frontend-2 + kubectl annotate deployment inventory odigos.io/reported-name=inventory-2 + kubectl annotate deployment pricing odigos.io/reported-name=pricing-2 + kubectl annotate deployment membership odigos.io/reported-name=membership-2 - name: Instrument Namespace try: - apply: @@ -256,20 +266,20 @@ spec: - script: timeout: 300s content: | - # Apply the job - kubectl apply -f ../../common/apply/generate-traffic-job.yaml - - # Wait for the job to complete - job_name=$(kubectl get -f ../../common/apply/generate-traffic-job.yaml -o=jsonpath='{.metadata.name}') - kubectl wait --for=condition=complete job/$job_name - - # Delete the job - kubectl delete -f ../../common/apply/generate-traffic-job.yaml - while true; do # wait for traces to be available sleep 8 + # Apply the job + kubectl apply -f ../../common/apply/generate-traffic-job.yaml + + # Wait for the job to complete + job_name=$(kubectl get -f ../../common/apply/generate-traffic-job.yaml -o=jsonpath='{.metadata.name}') + kubectl wait --for=condition=complete job/$job_name + + # Delete the job + kubectl delete -f ../../common/apply/generate-traffic-job.yaml + # Run the wait-for-trace script echo "Running TraceQL test at $(date)" ../../common/traceql_runner.sh tracesql/wait-for-trace-2.yaml @@ -295,7 +305,7 @@ spec: try: - script: content: | - ../../common/traceql_runner.sh tracesql/resource-attributes-2.yaml + ../../common/traceql_runner.sh tracesql/resource-attributes.yaml catch: - podLogs: name: odiglet diff --git a/tests/e2e/source/tracesql/context-propagation-2.yaml b/tests/e2e/source/tracesql/context-propagation-2.yaml index 4d865d339..fb3a1ca4d 100644 --- a/tests/e2e/source/tracesql/context-propagation-2.yaml +++ b/tests/e2e/source/tracesql/context-propagation-2.yaml @@ -2,12 +2,12 @@ apiVersion: e2e.tests.odigos.io/v1 kind: TraceTest description: This test checks if the context propagation is working correctly between different languages query: | - { resource.service.name = "frontend" && resource.telemetry.sdk.language = "java" && + { resource.service.name = "frontend-2" && resource.telemetry.sdk.language = "java" && span.http.request.method = "POST" && span.http.route = "/buy" && span:kind = server } >> ( - { resource.service.name = "pricing" && resource.telemetry.sdk.language = "dotnet" } && - { resource.service.name = "inventory" && resource.telemetry.sdk.language = "python" } && - ({ resource.service.name = "coupon" && resource.telemetry.sdk.language = "nodejs" } - >> { resource.service.name = "membership" && resource.telemetry.sdk.language = "go" })) + { resource.service.name = "pricing-2" && resource.telemetry.sdk.language = "dotnet" } && + { resource.service.name = "inventory-2" && resource.telemetry.sdk.language = "python" } && + ({ resource.service.name = "coupon-2" && resource.telemetry.sdk.language = "nodejs" } + >> { resource.service.name = "membership-2" && resource.telemetry.sdk.language = "go" })) expected: - count: 2 \ No newline at end of file + minimum: 1 \ No newline at end of file diff --git a/tests/e2e/source/tracesql/context-propagation.yaml b/tests/e2e/source/tracesql/context-propagation.yaml index 9c463f9b3..8ebb6d301 100644 --- a/tests/e2e/source/tracesql/context-propagation.yaml +++ b/tests/e2e/source/tracesql/context-propagation.yaml @@ -10,4 +10,4 @@ query: | ({ resource.service.name = "coupon" && resource.telemetry.sdk.language = "nodejs" } >> { resource.service.name = "membership" && resource.telemetry.sdk.language = "go" })) expected: - count: 1 \ No newline at end of file + minimum: 1 \ No newline at end of file diff --git a/tests/e2e/source/tracesql/resource-attributes-2.yaml b/tests/e2e/source/tracesql/resource-attributes-2.yaml deleted file mode 100644 index f6b56aef5..000000000 --- a/tests/e2e/source/tracesql/resource-attributes-2.yaml +++ /dev/null @@ -1,14 +0,0 @@ -apiVersion: e2e.tests.odigos.io/v1 -kind: TraceTest -description: | - This test check the following resource attributes: - A. odigos.version attribute exists on all spans and has the correct value - B. Kubernetes attributes are correctly set on all spans - At the time of writing this test, TraceQL api does not support not equal to nil so we use regex instead. -query: | - { resource.odigos.version !~ ".*" || - resource.k8s.deployment.name !~ ".*" || - resource.k8s.node.name !~ "(kind-control-plane|aks-.*)" || - resource.k8s.pod.name !~ ".*" } -expected: - count: 0 \ No newline at end of file diff --git a/tests/e2e/source/tracesql/span-attributes-2.yaml b/tests/e2e/source/tracesql/span-attributes-2.yaml index 84f1071c6..b997d72b4 100644 --- a/tests/e2e/source/tracesql/span-attributes-2.yaml +++ b/tests/e2e/source/tracesql/span-attributes-2.yaml @@ -4,15 +4,15 @@ description: | This test checks the span attributes for a specific trace. TODO - JS, Python and DotNet SDK are not generating data in latest semconv. add additional checks when they are updated. query: | - { resource.service.name = "frontend" && resource.telemetry.sdk.language = "java" && + { resource.service.name = "frontend-2" && resource.telemetry.sdk.language = "java" && span.http.request.method = "POST" && span.http.route = "/buy" && span:kind = server && span.http.response.status_code = 200 && span.url.query = "id=123" } >> ( - { resource.service.name = "pricing" && resource.telemetry.sdk.language = "dotnet" && span:kind = server } && - { resource.service.name = "inventory" && resource.telemetry.sdk.language = "python" && span:kind = server } && - ({ resource.service.name = "coupon" && resource.telemetry.sdk.language = "nodejs" && span:kind = server } - >> { resource.service.name = "membership" && resource.telemetry.sdk.language = "go" && + { resource.service.name = "pricing-2" && resource.telemetry.sdk.language = "dotnet" && span:kind = server } && + { resource.service.name = "inventory-2" && resource.telemetry.sdk.language = "python" && span:kind = server } && + ({ resource.service.name = "coupon-2" && resource.telemetry.sdk.language = "nodejs" && span:kind = server } + >> { resource.service.name = "membership-2" && resource.telemetry.sdk.language = "go" && span.http.request.method = "GET" && span:kind = server && span.http.response.status_code = 200 && span.url.path = "/isMember" })) expected: - count: 2 \ No newline at end of file + minimum: 1 \ No newline at end of file diff --git a/tests/e2e/source/tracesql/span-attributes.yaml b/tests/e2e/source/tracesql/span-attributes.yaml index d508d4a39..6dd10cd28 100644 --- a/tests/e2e/source/tracesql/span-attributes.yaml +++ b/tests/e2e/source/tracesql/span-attributes.yaml @@ -15,4 +15,4 @@ query: | span.http.request.method = "GET" && span:kind = server && span.http.response.status_code = 200 && span.url.path = "/isMember" })) expected: - count: 1 \ No newline at end of file + minimum: 1 \ No newline at end of file diff --git a/tests/e2e/source/tracesql/wait-for-trace-2.yaml b/tests/e2e/source/tracesql/wait-for-trace-2.yaml index 7f06b3148..a51ab240b 100644 --- a/tests/e2e/source/tracesql/wait-for-trace-2.yaml +++ b/tests/e2e/source/tracesql/wait-for-trace-2.yaml @@ -2,10 +2,10 @@ apiVersion: e2e.tests.odigos.io/v1 kind: TraceTest description: This test waits for a trace that goes from frontend to pricing, inventory, coupon, and membership services query: | - { resource.service.name = "frontend" } && - { resource.service.name = "pricing" } && - { resource.service.name = "inventory" } && - { resource.service.name = "coupon" } && - { resource.service.name = "membership" } + { resource.service.name = "frontend-2" } && + { resource.service.name = "pricing-2" } && + { resource.service.name = "inventory-2" } && + { resource.service.name = "coupon-2" } && + { resource.service.name = "membership-2" } expected: - count: 2 \ No newline at end of file + minimum: 1 \ No newline at end of file diff --git a/tests/e2e/source/tracesql/wait-for-trace.yaml b/tests/e2e/source/tracesql/wait-for-trace.yaml index a88f58987..62674d4ee 100644 --- a/tests/e2e/source/tracesql/wait-for-trace.yaml +++ b/tests/e2e/source/tracesql/wait-for-trace.yaml @@ -8,4 +8,4 @@ query: | { resource.service.name = "coupon" } && { resource.service.name = "membership" } expected: - count: 1 \ No newline at end of file + minimum: 1 \ No newline at end of file From 35ea009df0bf2034bd01abc72440c9ddf8e698eb Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Fri, 3 Jan 2025 14:28:46 -0500 Subject: [PATCH 10/21] Remove sleeps and increase traffic generation timeouts --- tests/e2e/source/chainsaw-test.yaml | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/tests/e2e/source/chainsaw-test.yaml b/tests/e2e/source/chainsaw-test.yaml index 9840798eb..79fa6e32c 100644 --- a/tests/e2e/source/chainsaw-test.yaml +++ b/tests/e2e/source/chainsaw-test.yaml @@ -110,16 +110,10 @@ spec: kubectl wait --for=condition=ready pod -l app=inventory --timeout=60s kubectl wait --for=condition=ready pod -l app=pricing --timeout=60s kubectl wait --for=condition=ready pod -l app=membership --timeout=60s - - name: Wait for apps auto-instrumentation - try: - - script: - timeout: 60s - content: | - sleep 30 - name: Generate Traffic try: - script: - timeout: 300s + timeout: 500s content: | while true; do # wait for traces to be available @@ -255,16 +249,10 @@ spec: kubectl wait --for=condition=ready pod -l app=inventory --timeout=60s kubectl wait --for=condition=ready pod -l app=pricing --timeout=60s kubectl wait --for=condition=ready pod -l app=membership --timeout=60s - - name: Wait for apps auto-instrumentation - try: - - script: - timeout: 60s - content: | - sleep 30 - name: Generate Traffic try: - script: - timeout: 300s + timeout: 500s content: | while true; do # wait for traces to be available From ba5691394e655b5fb0efc6006b962ad326d13a5d Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Mon, 6 Jan 2025 07:34:43 -0500 Subject: [PATCH 11/21] Remove wait for conditions --- tests/e2e/source/chainsaw-test.yaml | 66 ++++------------------------- 1 file changed, 8 insertions(+), 58 deletions(-) diff --git a/tests/e2e/source/chainsaw-test.yaml b/tests/e2e/source/chainsaw-test.yaml index 79fa6e32c..a80b4ba4c 100644 --- a/tests/e2e/source/chainsaw-test.yaml +++ b/tests/e2e/source/chainsaw-test.yaml @@ -100,16 +100,6 @@ spec: kubectl rollout status deployment -l app=inventory kubectl rollout status deployment -l app=pricing kubectl rollout status deployment -l app=membership - - name: Simple-demo pods ready for traffic - try: - - script: - timeout: 70s - content: | - kubectl wait --for=condition=ready pod -l app=frontend --timeout=60s - kubectl wait --for=condition=ready pod -l app=coupon --timeout=60s - kubectl wait --for=condition=ready pod -l app=inventory --timeout=60s - kubectl wait --for=condition=ready pod -l app=pricing --timeout=60s - kubectl wait --for=condition=ready pod -l app=membership --timeout=60s - name: Generate Traffic try: - script: @@ -187,24 +177,6 @@ spec: - assert: timeout: 2m file: 02-workloads.yaml - - script: - timeout: 70s - content: | - kubectl rollout status deployment -l app=coupon - kubectl rollout status deployment -l app=frontend - kubectl rollout status deployment -l app=inventory - kubectl rollout status deployment -l app=pricing - kubectl rollout status deployment -l app=membership - - name: Simple-demo pods ready for traffic - try: - - script: - timeout: 70s - content: | - kubectl wait --for=condition=ready pod -l app=frontend --timeout=60s - kubectl wait --for=condition=ready pod -l app=coupon --timeout=60s - kubectl wait --for=condition=ready pod -l app=inventory --timeout=60s - kubectl wait --for=condition=ready pod -l app=pricing --timeout=60s - kubectl wait --for=condition=ready pod -l app=membership --timeout=60s - name: Update reported service name for second phase of test try: - script: @@ -215,6 +187,14 @@ spec: kubectl annotate deployment inventory odigos.io/reported-name=inventory-2 kubectl annotate deployment pricing odigos.io/reported-name=pricing-2 kubectl annotate deployment membership odigos.io/reported-name=membership-2 + - script: + timeout: 70s + content: | + kubectl rollout status deployment -l app=coupon + kubectl rollout status deployment -l app=frontend + kubectl rollout status deployment -l app=inventory + kubectl rollout status deployment -l app=pricing + kubectl rollout status deployment -l app=membership - name: Instrument Namespace try: - apply: @@ -239,16 +219,6 @@ spec: kubectl rollout status deployment -l app=inventory kubectl rollout status deployment -l app=pricing kubectl rollout status deployment -l app=membership - - name: Simple-demo pods ready for traffic - try: - - script: - timeout: 70s - content: | - kubectl wait --for=condition=ready pod -l app=frontend --timeout=60s - kubectl wait --for=condition=ready pod -l app=coupon --timeout=60s - kubectl wait --for=condition=ready pod -l app=inventory --timeout=60s - kubectl wait --for=condition=ready pod -l app=pricing --timeout=60s - kubectl wait --for=condition=ready pod -l app=membership --timeout=60s - name: Generate Traffic try: - script: @@ -334,16 +304,6 @@ spec: kubectl rollout status deployment -l app=inventory kubectl rollout status deployment -l app=pricing kubectl rollout status deployment -l app=membership - - name: Simple-demo pods ready for traffic - try: - - script: - timeout: 70s - content: | - kubectl wait --for=condition=ready pod -l app=frontend --timeout=60s - kubectl wait --for=condition=ready pod -l app=coupon --timeout=60s - kubectl wait --for=condition=ready pod -l app=inventory --timeout=60s - kubectl wait --for=condition=ready pod -l app=pricing --timeout=60s - kubectl wait --for=condition=ready pod -l app=membership --timeout=60s - name: Instrument frontend workload specifically try: - apply: @@ -366,16 +326,6 @@ spec: kubectl rollout status deployment -l app=inventory kubectl rollout status deployment -l app=pricing kubectl rollout status deployment -l app=membership - - name: Simple-demo pods ready for traffic - try: - - script: - timeout: 70s - content: | - kubectl wait --for=condition=ready pod -l app=frontend --timeout=60s - kubectl wait --for=condition=ready pod -l app=coupon --timeout=60s - kubectl wait --for=condition=ready pod -l app=inventory --timeout=60s - kubectl wait --for=condition=ready pod -l app=pricing --timeout=60s - kubectl wait --for=condition=ready pod -l app=membership --timeout=60s - name: Instrument rest of Namespace try: - apply: From 1b26c5a864bd11119971d61885607ad1a46dc77d Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Mon, 6 Jan 2025 12:31:28 -0500 Subject: [PATCH 12/21] React to merge --- .../deleteinstrumentationconfig/namespace_controller.go | 2 +- .../deleteinstrumentationconfig/source_controller.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/instrumentor/controllers/deleteinstrumentationconfig/namespace_controller.go b/instrumentor/controllers/deleteinstrumentationconfig/namespace_controller.go index f68d44501..745fdd7c6 100644 --- a/instrumentor/controllers/deleteinstrumentationconfig/namespace_controller.go +++ b/instrumentor/controllers/deleteinstrumentationconfig/namespace_controller.go @@ -151,6 +151,7 @@ func syncGenericWorkloadListToNs(ctx context.Context, c client.Client, kind work } } + var err error inheriting, err := isInheritingInstrumentationFromNs(ctx, c, freshWorkloadCopy) if err != nil { return err @@ -159,7 +160,6 @@ func syncGenericWorkloadListToNs(ctx context.Context, c client.Client, kind work return nil } - var err error err = errors.Join(err, deleteWorkloadInstrumentationConfig(ctx, c, freshWorkloadCopy)) err = errors.Join(err, removeReportedNameAnnotation(ctx, c, freshWorkloadCopy)) return err diff --git a/instrumentor/controllers/deleteinstrumentationconfig/source_controller.go b/instrumentor/controllers/deleteinstrumentationconfig/source_controller.go index dd1e042c2..0dee556b9 100644 --- a/instrumentor/controllers/deleteinstrumentationconfig/source_controller.go +++ b/instrumentor/controllers/deleteinstrumentationconfig/source_controller.go @@ -118,7 +118,7 @@ func (r *SourceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr return ctrl.Result{}, err } - err = errors.Join(err, deleteWorkloadInstrumentedApplication(ctx, r.Client, obj)) + err = errors.Join(err, deleteWorkloadInstrumentationConfig(ctx, r.Client, obj)) err = errors.Join(err, removeReportedNameAnnotation(ctx, r.Client, obj)) if err != nil { return ctrl.Result{}, err From f1ac129d5f1b99b8987d6c40954810a6ab9949b5 Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Mon, 6 Jan 2025 12:56:03 -0500 Subject: [PATCH 13/21] Update exclude label, update comment, use WorkloadSources object --- api/odigos/v1alpha1/source_types.go | 56 ++++++++++++++----- common/consts/consts.go | 2 +- .../deleteinstrumentationconfig/common.go | 2 +- .../instrumentationconfig_controller.go | 2 +- .../namespace_controller.go | 10 ++-- .../namespace_controller.go | 2 +- .../workload_controllers.go | 8 +-- 7 files changed, 53 insertions(+), 29 deletions(-) diff --git a/api/odigos/v1alpha1/source_types.go b/api/odigos/v1alpha1/source_types.go index 5d8d0a110..76d03fc9f 100644 --- a/api/odigos/v1alpha1/source_types.go +++ b/api/odigos/v1alpha1/source_types.go @@ -18,6 +18,7 @@ package v1alpha1 import ( "context" + "errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" @@ -27,6 +28,8 @@ import ( "github.com/odigos-io/odigos/k8sutils/pkg/workload" ) +var ErrorTooManySources = errors.New("too many Sources found for workload") + // Source configures an application for auto-instrumentation. // +genclient // +kubebuilder:object:root=true @@ -67,19 +70,38 @@ type SourceList struct { Items []Source `json:"items"` } +// +kubebuilder:object:generate=false + +type WorkloadSources struct { + Workload *Source + Namespace *Source +} + // GetSourceListForWorkload returns a SourceList of all Sources that have matching // workload name, namespace, and kind labels for an object. In theory, this should only -// ever return a list with 0 or 1 items, but due diligence should handle unexpected cases. -func GetSourceListForWorkload(ctx context.Context, kubeClient client.Client, obj client.Object) (SourceList, error) { - sourceList := SourceList{} - selector := labels.SelectorFromSet(labels.Set{ - consts.WorkloadNameLabel: obj.GetName(), - consts.WorkloadNamespaceLabel: obj.GetNamespace(), - consts.WorkloadKindLabel: obj.GetObjectKind().GroupVersionKind().Kind, - }) - err := kubeClient.List(ctx, &sourceList, &client.ListOptions{LabelSelector: selector}) - if err != nil { - return sourceList, err +// ever return a list with 0, 1, or 2 items, but due diligence should handle unexpected cases. +// If a Workload returns >1 Namespace or Workload Source, this function returns an error. +func GetSourceListForWorkload(ctx context.Context, kubeClient client.Client, obj client.Object) (*WorkloadSources, error) { + var err error + workloadSources := &WorkloadSources{} + + if obj.GetObjectKind().GroupVersionKind().Kind != "Namespace" { + sourceList := SourceList{} + selector := labels.SelectorFromSet(labels.Set{ + consts.WorkloadNameLabel: obj.GetName(), + consts.WorkloadNamespaceLabel: obj.GetNamespace(), + consts.WorkloadKindLabel: obj.GetObjectKind().GroupVersionKind().Kind, + }) + err := kubeClient.List(ctx, &sourceList, &client.ListOptions{LabelSelector: selector}) + if err != nil { + return nil, err + } + if len(sourceList.Items) > 1 { + return nil, ErrorTooManySources + } + if len(sourceList.Items) == 1 { + workloadSources.Workload = &sourceList.Items[0] + } } namespaceSourceList := SourceList{} @@ -90,12 +112,16 @@ func GetSourceListForWorkload(ctx context.Context, kubeClient client.Client, obj }) err = kubeClient.List(ctx, &namespaceSourceList, &client.ListOptions{LabelSelector: namespaceSelector}) if err != nil { - return sourceList, err + return nil, err + } + if len(namespaceSourceList.Items) > 1 { + return nil, ErrorTooManySources + } + if len(namespaceSourceList.Items) == 1 { + workloadSources.Namespace = &namespaceSourceList.Items[0] } - // merge the lists - sourceList.Items = append(sourceList.Items, namespaceSourceList.Items...) - return sourceList, err + return workloadSources, nil } func init() { diff --git a/common/consts/consts.go b/common/consts/consts.go index 7fc22573e..6080bf200 100644 --- a/common/consts/consts.go +++ b/common/consts/consts.go @@ -18,7 +18,7 @@ const ( OdigosNamespaceAnnotation = "odigos.io/workload-namespace" OdigosWorkloadKindAnnotation = "odigos.io/workload-kind" OdigosWorkloadNameAnnotation = "odigos.io/workload-name" - OdigosWorkloadExcludedLabel = "odigos.io/excluded" + OdigosWorkloadExcludedLabel = "odigos.io/workload-excluded" OdigosReportedNameAnnotation = "odigos.io/reported-name" // GatewayMaxConnectionAge and GatewayMaxConnectionAgeGrace are the default values for the gateway collector. diff --git a/instrumentor/controllers/deleteinstrumentationconfig/common.go b/instrumentor/controllers/deleteinstrumentationconfig/common.go index c1256536e..4b829f5bb 100644 --- a/instrumentor/controllers/deleteinstrumentationconfig/common.go +++ b/instrumentor/controllers/deleteinstrumentationconfig/common.go @@ -30,7 +30,7 @@ func reconcileWorkloadObject(ctx context.Context, kubeClient client.Client, work if err != nil { return err } - if len(sourceList.Items) > 0 { + if sourceList.Workload == nil && sourceList.Namespace == nil { // Note that if a Source is being deleted, but still has the finalizer, it will still show up in this List // So we can't use this check to trigger un-instrumentation via Source deletion return nil diff --git a/instrumentor/controllers/deleteinstrumentationconfig/instrumentationconfig_controller.go b/instrumentor/controllers/deleteinstrumentationconfig/instrumentationconfig_controller.go index ab8ebeaf4..0859df539 100644 --- a/instrumentor/controllers/deleteinstrumentationconfig/instrumentationconfig_controller.go +++ b/instrumentor/controllers/deleteinstrumentationconfig/instrumentationconfig_controller.go @@ -96,7 +96,7 @@ func (r *InstrumentationConfigReconciler) Reconcile(ctx context.Context, req ctr if err != nil { return ctrl.Result{}, err } - if len(sourceList.Items) == 0 { + if sourceList.Workload == nil && sourceList.Namespace == nil { logger.Info("Deleting instrumented application for non-enabled workload") err := r.Client.Delete(ctx, &instrumentationConfig) return ctrl.Result{}, client.IgnoreNotFound(err) diff --git a/instrumentor/controllers/deleteinstrumentationconfig/namespace_controller.go b/instrumentor/controllers/deleteinstrumentationconfig/namespace_controller.go index 745fdd7c6..28746d312 100644 --- a/instrumentor/controllers/deleteinstrumentationconfig/namespace_controller.go +++ b/instrumentor/controllers/deleteinstrumentationconfig/namespace_controller.go @@ -174,12 +174,10 @@ func isInheritingInstrumentationFromNs(ctx context.Context, c client.Client, obj if err != nil { return false, err } - for _, source := range sourceList.Items { - // if a source exists for this workload specifically, then it is not inheriting from Namespace - if source.Spec.Workload.Name == obj.GetName() && - source.Spec.Workload.Kind == workload.WorkloadKind(obj.GetObjectKind().GroupVersionKind().Kind) { - return false, nil - } + + // if a source exists for this workload specifically, then it is not inheriting from Namespace + if sourceList.Workload != nil { + return false, nil } labels := obj.GetLabels() diff --git a/instrumentor/controllers/startlangdetection/namespace_controller.go b/instrumentor/controllers/startlangdetection/namespace_controller.go index 96e345893..e179f005b 100644 --- a/instrumentor/controllers/startlangdetection/namespace_controller.go +++ b/instrumentor/controllers/startlangdetection/namespace_controller.go @@ -34,7 +34,7 @@ func (n *NamespacesReconciler) Reconcile(ctx context.Context, request ctrl.Reque if err != nil { return ctrl.Result{}, err } - if len(sourceList.Items) == 0 { + if sourceList.Namespace == nil { return ctrl.Result{}, nil } } diff --git a/instrumentor/controllers/startlangdetection/workload_controllers.go b/instrumentor/controllers/startlangdetection/workload_controllers.go index fa6a38715..ad1aef8f8 100644 --- a/instrumentor/controllers/startlangdetection/workload_controllers.go +++ b/instrumentor/controllers/startlangdetection/workload_controllers.go @@ -64,12 +64,12 @@ func reconcileWorkload(ctx context.Context, k8sClient client.Client, objKind wor if err != nil { return ctrl.Result{}, err } - if len(sourceList.Items) == 0 { + if sourceList.Workload == nil && sourceList.Namespace == nil { return ctrl.Result{}, nil } - // if this is explicitly excluded (ie, namespace instrumentation), skip - for _, s := range sourceList.Items { - if _, exists := s.Labels[consts.OdigosWorkloadExcludedLabel]; exists { + // if this is explicitly excluded, skip + if sourceList.Workload != nil { + if val, exists := sourceList.Workload.Labels[consts.OdigosWorkloadExcludedLabel]; exists && val == "true" { return ctrl.Result{}, nil } } From ef48c5998d3c1ac11ba690a265d2c1bbca20650b Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Mon, 6 Jan 2025 14:00:07 -0500 Subject: [PATCH 14/21] Update source e2e to not use InstrumentedApplication --- tests/e2e/source/05-assert-runtime-detected.yaml | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/tests/e2e/source/05-assert-runtime-detected.yaml b/tests/e2e/source/05-assert-runtime-detected.yaml index a2771ca7b..932715aec 100644 --- a/tests/e2e/source/05-assert-runtime-detected.yaml +++ b/tests/e2e/source/05-assert-runtime-detected.yaml @@ -1,20 +1,4 @@ apiVersion: odigos.io/v1alpha1 -kind: InstrumentedApplication -metadata: - name: deployment-frontend - namespace: default - ownerReferences: - - apiVersion: apps/v1 - blockOwnerDeletion: true - controller: true - kind: Deployment - name: frontend -spec: - runtimeDetails: - - containerName: frontend - language: java ---- -apiVersion: odigos.io/v1alpha1 kind: InstrumentationConfig metadata: name: deployment-frontend From 4277faa7c166a2267d9cf364a577d3e1c4b6c67a Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Mon, 6 Jan 2025 14:50:47 -0500 Subject: [PATCH 15/21] Got the delete instrumentationconfig check wrong again --- .../deleteinstrumentationconfig/common.go | 13 ++++++------- .../utils/predicates/instrumentation_rule.go | 1 - 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/instrumentor/controllers/deleteinstrumentationconfig/common.go b/instrumentor/controllers/deleteinstrumentationconfig/common.go index 4b829f5bb..7ee85e342 100644 --- a/instrumentor/controllers/deleteinstrumentationconfig/common.go +++ b/instrumentor/controllers/deleteinstrumentationconfig/common.go @@ -3,7 +3,6 @@ package deleteinstrumentationconfig import ( "context" - "github.com/odigos-io/odigos/api/odigos/v1alpha1" odigosv1 "github.com/odigos-io/odigos/api/odigos/v1alpha1" "github.com/odigos-io/odigos/common/consts" "github.com/odigos-io/odigos/k8sutils/pkg/workload" @@ -26,11 +25,11 @@ func reconcileWorkloadObject(ctx context.Context, kubeClient client.Client, work } // Check if a Source object exists for this workload - sourceList, err := v1alpha1.GetSourceListForWorkload(ctx, kubeClient, workloadObject) + sourceList, err := odigosv1.GetSourceListForWorkload(ctx, kubeClient, workloadObject) if err != nil { return err } - if sourceList.Workload == nil && sourceList.Namespace == nil { + if sourceList.Workload != nil || sourceList.Namespace != nil { // Note that if a Source is being deleted, but still has the finalizer, it will still show up in this List // So we can't use this check to trigger un-instrumentation via Source deletion return nil @@ -54,13 +53,13 @@ func deleteWorkloadInstrumentationConfig(ctx context.Context, kubeClient client. ns := workloadObject.GetNamespace() name := workloadObject.GetName() kind := workload.WorkloadKindFromClientObject(workloadObject) - instrumentedApplicationName := workload.CalculateWorkloadRuntimeObjectName(name, kind) - logger.V(1).Info("deleting instrumented application", "name", instrumentedApplicationName, "kind", kind) + instrumentationConfigName := workload.CalculateWorkloadRuntimeObjectName(name, kind) + logger.V(1).Info("deleting instrumentationconfig", "name", instrumentationConfigName, "kind", kind) instConfigErr := kubeClient.Delete(ctx, &odigosv1.InstrumentationConfig{ ObjectMeta: metav1.ObjectMeta{ Namespace: ns, - Name: instrumentedApplicationName, + Name: instrumentationConfigName, }, }) @@ -68,7 +67,7 @@ func deleteWorkloadInstrumentationConfig(ctx context.Context, kubeClient client. return client.IgnoreNotFound(instConfigErr) } - logger.V(1).Info("instrumented application deleted", "namespace", ns, "name", name, "kind", kind) + logger.V(1).Info("instrumentationconfig deleted", "namespace", ns, "name", name, "kind", kind) return nil } diff --git a/instrumentor/controllers/utils/predicates/instrumentation_rule.go b/instrumentor/controllers/utils/predicates/instrumentation_rule.go index b1da0900f..ceec0c0a7 100644 --- a/instrumentor/controllers/utils/predicates/instrumentation_rule.go +++ b/instrumentor/controllers/utils/predicates/instrumentation_rule.go @@ -7,7 +7,6 @@ import ( type OtelSdkInstrumentationRulePredicate struct{} - func (o OtelSdkInstrumentationRulePredicate) Create(e event.CreateEvent) bool { // check if delete rule is for otel sdk instrumentationRule, ok := e.Object.(*odigosv1alpha1.InstrumentationRule) From 1ea0bb2a7bea48ea864b811a421d4b26e7171067 Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Mon, 6 Jan 2025 14:57:33 -0500 Subject: [PATCH 16/21] Move namespace pre-processing into namespace path --- .../startlangdetection/source_controller.go | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/instrumentor/controllers/startlangdetection/source_controller.go b/instrumentor/controllers/startlangdetection/source_controller.go index 373cd26ba..3a12ba9c5 100644 --- a/instrumentor/controllers/startlangdetection/source_controller.go +++ b/instrumentor/controllers/startlangdetection/source_controller.go @@ -47,23 +47,23 @@ func (r *SourceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr return k8sutils.K8SUpdateErrorHandler(err) } - // pre-process existing Sources for specific workloads so we don't have to make a bunch of API calls - // This is used to check if a workload already has an explicit Source, so we don't overwrite its InstrumentationConfig - sourceList := v1alpha1.SourceList{} - err := r.Client.List(ctx, &sourceList, client.InNamespace(source.Spec.Workload.Name)) - if err != nil { - return ctrl.Result{}, err - } - namespaceKindSources := make(map[workload.WorkloadKind]map[string]struct{}) - for _, source := range sourceList.Items { - if _, exists := namespaceKindSources[source.Spec.Workload.Kind]; !exists { - namespaceKindSources[source.Spec.Workload.Kind] = make(map[string]struct{}) + if source.Spec.Workload.Kind == "Namespace" { + // pre-process existing Sources for specific workloads so we don't have to make a bunch of API calls + // This is used to check if a workload already has an explicit Source, so we don't overwrite its InstrumentationConfig + sourceList := v1alpha1.SourceList{} + err := r.Client.List(ctx, &sourceList, client.InNamespace(source.Spec.Workload.Name)) + if err != nil { + return ctrl.Result{}, err + } + namespaceKindSources := make(map[workload.WorkloadKind]map[string]struct{}) + for _, source := range sourceList.Items { + if _, exists := namespaceKindSources[source.Spec.Workload.Kind]; !exists { + namespaceKindSources[source.Spec.Workload.Kind] = make(map[string]struct{}) + } + // ex: map["Deployment"]["my-app"] = ... + namespaceKindSources[source.Spec.Workload.Kind][source.Spec.Workload.Name] = struct{}{} } - // ex: map["Deployment"]["my-app"] = ... - namespaceKindSources[source.Spec.Workload.Kind][source.Spec.Workload.Name] = struct{}{} - } - if source.Spec.Workload.Kind == "Namespace" { var deps appsv1.DeploymentList err = r.Client.List(ctx, &deps, client.InNamespace(source.Spec.Workload.Name)) if client.IgnoreNotFound(err) != nil { From 7295ac94189453eb52e270c07f5b2c56a91fe378 Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Mon, 6 Jan 2025 23:01:46 -0500 Subject: [PATCH 17/21] Support creation/deletion of excluded Source in instrumented namespace --- api/odigos/v1alpha1/source_types.go | 10 ++ .../deleteinstrumentationconfig/manager.go | 3 +- .../namespace_controller.go | 7 +- .../source_controller.go | 70 +++++++++---- .../startlangdetection/source_controller.go | 56 ++++++++--- .../workload_controllers.go | 6 +- k8sutils/pkg/consts/consts.go | 10 +- .../source/08-assert-runtime-detected.yaml | 69 +++++++++++++ tests/e2e/source/08-source.yaml | 13 +++ tests/e2e/source/08-workloads-2.yaml | 44 +++++++++ tests/e2e/source/08-workloads.yaml | 44 +++++++++ .../source/09-assert-runtime-detected.yaml | 52 ++++++++++ tests/e2e/source/09-source.yaml | 13 +++ tests/e2e/source/09-workloads.yaml | 44 +++++++++ tests/e2e/source/README.md | 56 +++++++++++ tests/e2e/source/chainsaw-test.yaml | 97 ++++++++++++++++++- 16 files changed, 548 insertions(+), 46 deletions(-) create mode 100644 tests/e2e/source/08-assert-runtime-detected.yaml create mode 100644 tests/e2e/source/08-source.yaml create mode 100644 tests/e2e/source/08-workloads-2.yaml create mode 100644 tests/e2e/source/08-workloads.yaml create mode 100644 tests/e2e/source/09-assert-runtime-detected.yaml create mode 100644 tests/e2e/source/09-source.yaml create mode 100644 tests/e2e/source/09-workloads.yaml create mode 100644 tests/e2e/source/README.md diff --git a/api/odigos/v1alpha1/source_types.go b/api/odigos/v1alpha1/source_types.go index 76d03fc9f..a4338b4c5 100644 --- a/api/odigos/v1alpha1/source_types.go +++ b/api/odigos/v1alpha1/source_types.go @@ -24,6 +24,7 @@ import ( "k8s.io/apimachinery/pkg/labels" "sigs.k8s.io/controller-runtime/pkg/client" + commonconsts "github.com/odigos-io/odigos/common/consts" "github.com/odigos-io/odigos/k8sutils/pkg/consts" "github.com/odigos-io/odigos/k8sutils/pkg/workload" ) @@ -124,6 +125,15 @@ func GetSourceListForWorkload(ctx context.Context, kubeClient client.Client, obj return workloadSources, nil } +// IsWorkloadExcludedSource returns true if the Source is used to exclude a workload. +// Otherwise, it returns false. +func IsWorkloadExcludedSource(source *Source) bool { + if val, exists := source.Labels[commonconsts.OdigosWorkloadExcludedLabel]; exists && val == "true" { + return true + } + return false +} + func init() { SchemeBuilder.Register(&Source{}, &SourceList{}) } diff --git a/instrumentor/controllers/deleteinstrumentationconfig/manager.go b/instrumentor/controllers/deleteinstrumentationconfig/manager.go index 851afa213..90f22c8e2 100644 --- a/instrumentor/controllers/deleteinstrumentationconfig/manager.go +++ b/instrumentor/controllers/deleteinstrumentationconfig/manager.go @@ -2,7 +2,6 @@ package deleteinstrumentationconfig import ( odigosv1 "github.com/odigos-io/odigos/api/odigos/v1alpha1" - k8sutils "github.com/odigos-io/odigos/k8sutils/pkg/predicate" odigospredicate "github.com/odigos-io/odigos/k8sutils/pkg/predicate" appsv1 "k8s.io/api/apps/v1" @@ -81,7 +80,7 @@ func SetupWithManager(mgr ctrl.Manager) error { err = builder. ControllerManagedBy(mgr). Named("deleteinstrumentationconfig-source"). - WithEventFilter(&k8sutils.OnlyUpdatesPredicate{}). + WithEventFilter(predicate.Or(&odigospredicate.CreationPredicate{}, &odigospredicate.OnlyUpdatesPredicate{})). For(&odigosv1.Source{}). Complete(&SourceReconciler{ Client: mgr.GetClient(), diff --git a/instrumentor/controllers/deleteinstrumentationconfig/namespace_controller.go b/instrumentor/controllers/deleteinstrumentationconfig/namespace_controller.go index 28746d312..3a01eba0c 100644 --- a/instrumentor/controllers/deleteinstrumentationconfig/namespace_controller.go +++ b/instrumentor/controllers/deleteinstrumentationconfig/namespace_controller.go @@ -175,8 +175,11 @@ func isInheritingInstrumentationFromNs(ctx context.Context, c client.Client, obj return false, err } - // if a source exists for this workload specifically, then it is not inheriting from Namespace - if sourceList.Workload != nil { + if sourceList.Namespace != nil && sourceList.Namespace.DeletionTimestamp.IsZero() { + return true, nil + } + + if sourceList.Workload != nil && sourceList.Workload.DeletionTimestamp.IsZero() { return false, nil } diff --git a/instrumentor/controllers/deleteinstrumentationconfig/source_controller.go b/instrumentor/controllers/deleteinstrumentationconfig/source_controller.go index 0dee556b9..0fd4ee37b 100644 --- a/instrumentor/controllers/deleteinstrumentationconfig/source_controller.go +++ b/instrumentor/controllers/deleteinstrumentationconfig/source_controller.go @@ -49,14 +49,26 @@ func (r *SourceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr return ctrl.Result{}, client.IgnoreNotFound(err) } - if !source.DeletionTimestamp.IsZero() { + // If this is a regular Source that's being deleted, or a workload Exclusion Source + // that's being created, try to uninstrument relevant workloads. + if source.DeletionTimestamp.IsZero() == v1alpha1.IsWorkloadExcludedSource(source) { logger.Info("Reconciling workload for deleted Source object", "name", req.Name, "namespace", req.Namespace) + + if result, err := r.setSourceLabelsIfNecessary(ctx, source); err != nil { + return result, err + } + if v1alpha1.IsWorkloadExcludedSource(source) && !controllerutil.ContainsFinalizer(source, consts.StartLangDetectionFinalizer) { + controllerutil.AddFinalizer(source, consts.StartLangDetectionFinalizer) + if err := r.Update(ctx, source); err != nil { + return k8sutils.K8SUpdateErrorHandler(err) + } + } + if source.Spec.Workload.Kind == "Namespace" { logger.V(2).Info("Uninstrumenting deployments for Namespace Source", "name", req.Name, "namespace", req.Namespace) var deps appsv1.DeploymentList err = r.Client.List(ctx, &deps, client.InNamespace(req.Namespace)) - if client.IgnoreNotFound(err) != nil { - logger.Error(err, "error fetching deployments") + if err != nil { return ctrl.Result{}, err } @@ -71,8 +83,7 @@ func (r *SourceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr logger.V(2).Info("Uninstrumenting statefulsets for Namespace Source", "name", req.Name, "namespace", req.Namespace) var ss appsv1.StatefulSetList err = r.Client.List(ctx, &ss, client.InNamespace(req.Namespace)) - if client.IgnoreNotFound(err) != nil { - logger.Error(err, "error fetching statefulsets") + if err != nil { return ctrl.Result{}, err } @@ -87,8 +98,7 @@ func (r *SourceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr logger.V(2).Info("Uninstrumenting daemonsets for Namespace Source", "name", req.Name, "namespace", req.Namespace) var ds appsv1.DaemonSetList err = r.Client.List(ctx, &ds, client.InNamespace(req.Namespace)) - if client.IgnoreNotFound(err) != nil { - logger.Error(err, "error fetching daemonsets") + if err != nil { return ctrl.Result{}, err } @@ -108,25 +118,24 @@ func (r *SourceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr return ctrl.Result{}, err } - // Check if this workload is also inheriting Instrumentation from namespace - inheriting, err := isInheritingInstrumentationFromNs(ctx, r.Client, obj) + sourceList, err := v1alpha1.GetSourceListForWorkload(ctx, r.Client, obj) if err != nil { return ctrl.Result{}, err } - if inheriting { - // if it is inheriting, don't delete the instrumentation config - return ctrl.Result{}, err - } - - err = errors.Join(err, deleteWorkloadInstrumentationConfig(ctx, r.Client, obj)) - err = errors.Join(err, removeReportedNameAnnotation(ctx, r.Client, obj)) - if err != nil { - return ctrl.Result{}, err + if sourceList.Namespace == nil || + (sourceList.Namespace != nil && !sourceList.Namespace.DeletionTimestamp.IsZero()) || + (sourceList.Workload != nil && sourceList.Workload.DeletionTimestamp.IsZero() && v1alpha1.IsWorkloadExcludedSource(source)) { + // if this workload doesn't have a live Namespace instrumentation, or it has a live exclusion source, uninstrument it + err = errors.Join(err, deleteWorkloadInstrumentationConfig(ctx, r.Client, obj)) + err = errors.Join(err, removeReportedNameAnnotation(ctx, r.Client, obj)) + if err != nil { + return ctrl.Result{}, err + } } } - if controllerutil.ContainsFinalizer(source, consts.InstrumentedApplicationFinalizer) { - controllerutil.RemoveFinalizer(source, consts.InstrumentedApplicationFinalizer) + if !v1alpha1.IsWorkloadExcludedSource(source) && controllerutil.ContainsFinalizer(source, consts.DeleteInstrumentationConfigFinalizer) { + controllerutil.RemoveFinalizer(source, consts.DeleteInstrumentationConfigFinalizer) if err := r.Update(ctx, source); err != nil { return k8sutils.K8SUpdateErrorHandler(err) } @@ -134,3 +143,24 @@ func (r *SourceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr } return ctrl.Result{}, nil } + +// TODO: Move to mutating webhook +func (r *SourceReconciler) setSourceLabelsIfNecessary(ctx context.Context, source *v1alpha1.Source) (ctrl.Result, error) { + if source.Labels == nil { + source.Labels = make(map[string]string) + } + + if source.Labels[consts.WorkloadNameLabel] != source.Spec.Workload.Name || + source.Labels[consts.WorkloadNamespaceLabel] != source.Spec.Workload.Namespace || + source.Labels[consts.WorkloadKindLabel] != string(source.Spec.Workload.Kind) { + + source.Labels[consts.WorkloadNameLabel] = source.Spec.Workload.Name + source.Labels[consts.WorkloadNamespaceLabel] = source.Spec.Workload.Namespace + source.Labels[consts.WorkloadKindLabel] = string(source.Spec.Workload.Kind) + + if err := r.Update(ctx, source); err != nil { + return k8sutils.K8SUpdateErrorHandler(err) + } + } + return ctrl.Result{}, nil +} diff --git a/instrumentor/controllers/startlangdetection/source_controller.go b/instrumentor/controllers/startlangdetection/source_controller.go index 3a12ba9c5..2cc507564 100644 --- a/instrumentor/controllers/startlangdetection/source_controller.go +++ b/instrumentor/controllers/startlangdetection/source_controller.go @@ -31,20 +31,17 @@ func (r *SourceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr return ctrl.Result{}, client.IgnoreNotFound(err) } - if source.DeletionTimestamp.IsZero() { - if !controllerutil.ContainsFinalizer(source, consts.InstrumentedApplicationFinalizer) { - controllerutil.AddFinalizer(source, consts.InstrumentedApplicationFinalizer) + // If this is a regular Source that is being created, or an Exclusion Source that is being deleted, + // Attempt to reconcile the workloads for instrumentation. + if source.DeletionTimestamp.IsZero() != v1alpha1.IsWorkloadExcludedSource(source) { + if result, err := r.setSourceLabelsIfNecessary(ctx, source); err != nil { + return result, err } - if source.Labels == nil { - source.Labels = make(map[string]string) - } - - source.Labels[consts.WorkloadNameLabel] = source.Spec.Workload.Name - source.Labels[consts.WorkloadNamespaceLabel] = source.Spec.Workload.Namespace - source.Labels[consts.WorkloadKindLabel] = string(source.Spec.Workload.Kind) - - if err := r.Update(ctx, source); err != nil { - return k8sutils.K8SUpdateErrorHandler(err) + if !v1alpha1.IsWorkloadExcludedSource(source) && !controllerutil.ContainsFinalizer(source, consts.DeleteInstrumentationConfigFinalizer) { + controllerutil.AddFinalizer(source, consts.DeleteInstrumentationConfigFinalizer) + if err := r.Update(ctx, source); err != nil { + return k8sutils.K8SUpdateErrorHandler(err) + } } if source.Spec.Workload.Kind == "Namespace" { @@ -115,7 +112,7 @@ func (r *SourceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr } } } else { - return reconcileWorkload(ctx, + _, err = reconcileWorkload(ctx, r.Client, source.Spec.Workload.Kind, ctrl.Request{ @@ -125,8 +122,39 @@ func (r *SourceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr }, }, r.Scheme) + if err != nil { + return ctrl.Result{}, err + } + } + + if v1alpha1.IsWorkloadExcludedSource(source) && controllerutil.ContainsFinalizer(source, consts.StartLangDetectionFinalizer) { + controllerutil.RemoveFinalizer(source, consts.StartLangDetectionFinalizer) + if err := r.Update(ctx, source); err != nil { + return k8sutils.K8SUpdateErrorHandler(err) + } } } return ctrl.Result{}, err } + +// TODO: Move to mutating webhook +func (r *SourceReconciler) setSourceLabelsIfNecessary(ctx context.Context, source *v1alpha1.Source) (ctrl.Result, error) { + if source.Labels == nil { + source.Labels = make(map[string]string) + } + + if source.Labels[consts.WorkloadNameLabel] != source.Spec.Workload.Name || + source.Labels[consts.WorkloadNamespaceLabel] != source.Spec.Workload.Namespace || + source.Labels[consts.WorkloadKindLabel] != string(source.Spec.Workload.Kind) { + + source.Labels[consts.WorkloadNameLabel] = source.Spec.Workload.Name + source.Labels[consts.WorkloadNamespaceLabel] = source.Spec.Workload.Namespace + source.Labels[consts.WorkloadKindLabel] = string(source.Spec.Workload.Kind) + + if err := r.Update(ctx, source); err != nil { + return k8sutils.K8SUpdateErrorHandler(err) + } + } + return ctrl.Result{}, nil +} diff --git a/instrumentor/controllers/startlangdetection/workload_controllers.go b/instrumentor/controllers/startlangdetection/workload_controllers.go index ad1aef8f8..79be52627 100644 --- a/instrumentor/controllers/startlangdetection/workload_controllers.go +++ b/instrumentor/controllers/startlangdetection/workload_controllers.go @@ -10,7 +10,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log" odigosv1 "github.com/odigos-io/odigos/api/odigos/v1alpha1" - "github.com/odigos-io/odigos/common/consts" "github.com/odigos-io/odigos/k8sutils/pkg/workload" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" @@ -67,14 +66,13 @@ func reconcileWorkload(ctx context.Context, k8sClient client.Client, objKind wor if sourceList.Workload == nil && sourceList.Namespace == nil { return ctrl.Result{}, nil } - // if this is explicitly excluded, skip + // if this is explicitly excluded (and the excluded Source isn't being deleted), skip if sourceList.Workload != nil { - if val, exists := sourceList.Workload.Labels[consts.OdigosWorkloadExcludedLabel]; exists && val == "true" { + if odigosv1.IsWorkloadExcludedSource(sourceList.Workload) && sourceList.Workload.DeletionTimestamp.IsZero() { return ctrl.Result{}, nil } } } - err = requestOdigletsToCalculateRuntimeDetails(ctx, k8sClient, instConfigName, req.Namespace, obj, scheme) return ctrl.Result{}, err } diff --git a/k8sutils/pkg/consts/consts.go b/k8sutils/pkg/consts/consts.go index 7b17a3fcf..b66ed49f4 100644 --- a/k8sutils/pkg/consts/consts.go +++ b/k8sutils/pkg/consts/consts.go @@ -63,9 +63,13 @@ var ( ) const ( - SourceFinalizer = "odigos.io/source-finalizer" - // TODO: Needed until InstrumentedApplication is removed - InstrumentedApplicationFinalizer = "odigos.io/source-instrumentedapplication-finalizer" + // StartLangDetectionFinalizer is used for Workload exclusion Sources. When a Workload exclusion Source + // is deleted, we want to go to the startlangdetection controller. There, we will check if the Workload should + // start inheriting Namespace instrumentation. + StartLangDetectionFinalizer = "odigos.io/source-startlangdetection-finalizer" + // DeleteInstrumentationConfigFinalizer is used for all non-exclusion (normal) Sources. When a normal Source + // is deleted, we want to go to the deleteinstrumentationconfig controller to un-instrument the workload/namespace. + DeleteInstrumentationConfigFinalizer = "odigos.io/source-deleteinstrumentationconfig-finalizer" WorkloadNameLabel = "odigos.io/workload-name" WorkloadNamespaceLabel = "odigos.io/workload-namespace" diff --git a/tests/e2e/source/08-assert-runtime-detected.yaml b/tests/e2e/source/08-assert-runtime-detected.yaml new file mode 100644 index 000000000..47f1ccd72 --- /dev/null +++ b/tests/e2e/source/08-assert-runtime-detected.yaml @@ -0,0 +1,69 @@ +apiVersion: odigos.io/v1alpha1 +kind: InstrumentationConfig +metadata: + name: deployment-frontend + namespace: default + ownerReferences: + - apiVersion: apps/v1 + blockOwnerDeletion: true + controller: true + kind: Deployment + name: frontend +status: + runtimeDetailsByContainer: + - containerName: frontend + language: java + runtimeVersion: 17.0.11+9 +--- +apiVersion: odigos.io/v1alpha1 +kind: InstrumentationConfig +metadata: + name: deployment-inventory + namespace: default + ownerReferences: + - apiVersion: apps/v1 + blockOwnerDeletion: true + controller: true + kind: Deployment + name: inventory +status: + runtimeDetailsByContainer: + - containerName: inventory + envVars: + - name: PYTHONPATH + value: /bar # this env exists in the test image + language: python + runtimeVersion: 3.11.9 +--- +apiVersion: odigos.io/v1alpha1 +kind: InstrumentationConfig +metadata: + name: deployment-membership + namespace: default + ownerReferences: + - apiVersion: apps/v1 + blockOwnerDeletion: true + controller: true + kind: Deployment + name: membership +status: + runtimeDetailsByContainer: + - containerName: membership + language: go + runtimeVersion: 1.21.4 +--- +apiVersion: odigos.io/v1alpha1 +kind: InstrumentationConfig +metadata: + name: deployment-pricing + namespace: default + ownerReferences: + - apiVersion: apps/v1 + blockOwnerDeletion: true + controller: true + kind: Deployment + name: pricing +status: + runtimeDetailsByContainer: + - containerName: pricing + language: dotnet diff --git a/tests/e2e/source/08-source.yaml b/tests/e2e/source/08-source.yaml new file mode 100644 index 000000000..8aa6d43b5 --- /dev/null +++ b/tests/e2e/source/08-source.yaml @@ -0,0 +1,13 @@ +apiVersion: odigos.io/v1alpha1 +kind: Source +metadata: + name: coupon-excluded + namespace: default + labels: + odigos.io/e2e: source-excluded + odigos.io/workload-excluded: "true" +spec: + workload: + name: coupon + namespace: default + kind: Deployment diff --git a/tests/e2e/source/08-workloads-2.yaml b/tests/e2e/source/08-workloads-2.yaml new file mode 100644 index 000000000..e6548dfc6 --- /dev/null +++ b/tests/e2e/source/08-workloads-2.yaml @@ -0,0 +1,44 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "8" + generation: 10 + name: coupon + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "6" + generation: 8 + name: frontend + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "8" + generation: 10 + name: inventory + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "8" + generation: 10 + name: membership + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "8" + generation: 10 + name: pricing + namespace: default \ No newline at end of file diff --git a/tests/e2e/source/08-workloads.yaml b/tests/e2e/source/08-workloads.yaml new file mode 100644 index 000000000..c66e30df9 --- /dev/null +++ b/tests/e2e/source/08-workloads.yaml @@ -0,0 +1,44 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "7" + generation: 9 + name: coupon + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "6" + generation: 8 + name: frontend + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "8" + generation: 10 + name: inventory + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "8" + generation: 10 + name: membership + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "8" + generation: 10 + name: pricing + namespace: default \ No newline at end of file diff --git a/tests/e2e/source/09-assert-runtime-detected.yaml b/tests/e2e/source/09-assert-runtime-detected.yaml new file mode 100644 index 000000000..e49061385 --- /dev/null +++ b/tests/e2e/source/09-assert-runtime-detected.yaml @@ -0,0 +1,52 @@ +apiVersion: odigos.io/v1alpha1 +kind: InstrumentationConfig +metadata: + name: deployment-frontend + namespace: default + ownerReferences: + - apiVersion: apps/v1 + blockOwnerDeletion: true + controller: true + kind: Deployment + name: frontend +status: + runtimeDetailsByContainer: + - containerName: frontend + language: java + runtimeVersion: 17.0.11+9 +--- +apiVersion: odigos.io/v1alpha1 +kind: InstrumentationConfig +metadata: + name: deployment-inventory + namespace: default + ownerReferences: + - apiVersion: apps/v1 + blockOwnerDeletion: true + controller: true + kind: Deployment + name: inventory +status: + runtimeDetailsByContainer: + - containerName: inventory + envVars: + - name: PYTHONPATH + value: /bar # this env exists in the test image + language: python + runtimeVersion: 3.11.9 +--- +apiVersion: odigos.io/v1alpha1 +kind: InstrumentationConfig +metadata: + name: deployment-pricing + namespace: default + ownerReferences: + - apiVersion: apps/v1 + blockOwnerDeletion: true + controller: true + kind: Deployment + name: pricing +status: + runtimeDetailsByContainer: + - containerName: pricing + language: dotnet diff --git a/tests/e2e/source/09-source.yaml b/tests/e2e/source/09-source.yaml new file mode 100644 index 000000000..19845f01b --- /dev/null +++ b/tests/e2e/source/09-source.yaml @@ -0,0 +1,13 @@ +apiVersion: odigos.io/v1alpha1 +kind: Source +metadata: + name: membership-excluded + namespace: default + labels: + odigos.io/e2e: source-excluded + odigos.io/workload-excluded: "true" +spec: + workload: + name: membership + namespace: default + kind: Deployment diff --git a/tests/e2e/source/09-workloads.yaml b/tests/e2e/source/09-workloads.yaml new file mode 100644 index 000000000..60dac05b9 --- /dev/null +++ b/tests/e2e/source/09-workloads.yaml @@ -0,0 +1,44 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "8" + generation: 10 + name: coupon + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "6" + generation: 8 + name: frontend + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "8" + generation: 10 + name: inventory + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "9" + generation: 11 + name: membership + namespace: default +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + deployment.kubernetes.io/revision: "8" + generation: 10 + name: pricing + namespace: default \ No newline at end of file diff --git a/tests/e2e/source/README.md b/tests/e2e/source/README.md new file mode 100644 index 000000000..cbc8e2b14 --- /dev/null +++ b/tests/e2e/source/README.md @@ -0,0 +1,56 @@ +# Source object e2e + +This e2e extensively tests the use of Source objects for instrumentation, uninstrumentation, and exclusion. + +It has the following phases: + +1. **Setup** - Install Odigos, Tempo, and the Demo app. +2. **Workload instrumentation** - Create a Source for each individual workload. Add Tempo as a destination. Verify: + 2.1 InstrumentationConfigs are created for each deployment + 2.2 Tempo is ready to receive traces + 2.3 The Odigos pipeline is ready + 2.4 Each deployment rolls out a new (instrumented) revision + 2.5 Generated traffic results in expected spans + 2.6 Context propagation works across deployments + 2.7 Resource attributes are present + 2.8 Span attributes are present +3. **Workload uninstrumentation** - Delete all Source objects for deployments. Verify: + 3.1 Workloads roll out a new (uninstrumented) revision +4. **Namespace instrumentation** - Update the service name for each deployment. Instrument Namespace. Verify: + 4.1 InstrumentationConfigs are present for each workload in the Namespace. + 4.2 Workloads roll out a new (instrumented) revision. + 4.3 Generated traffic results in expected spans (with new service name) + 4.4 Context propagation works + 4.5 Resource attributes are present + 4.6 Span attributes are present +5. **Namespace uninstrumentation** - Delete Namespace Source object. Verify: + 5.1 Workloads roll out a new (uninstrumented) revision +6. **Namespace+Workload instrumentation** - Instrument a single workload, then instrument the rest of the namespace. Verify: + 6.1 InstrumentationConfig is created for the single workload. + 6.2 Single workload rolls out a new revision. + 6.3 InstrumentationConfigs are then detected for all workloads. + 6.4 Remaining workloads roll out a new revision. + 6.5 Deleting Namespace Source does not delete individual Workload source. +7. **Workload exclusion** - Create an Excluded Source for another workload. Instrument the namespace. Verify: + 7.1 InstrumentationConfigs are created for all workloads except excluded workload + 7.2 All workloads except excluded workload roll out a new revision +8. **Workload inclusion** - Delete an Excluded source in an already-instrumented namespace. Verify: + 8.1 InstrumentationConfigs exist for all workloads in the namespace. + 8.2 Only the previously-excluded workload rolls out a new revision. + 8.3 Previously-excluded workload now has runtime detected +9. **Workload exclusion (2)** - Create an Excluded Source in an already-instrumented namespace. Verify: + 9.1 Only the newly excluded workload rolls out a new revision. + 9.2 InstrumentationConfigs exist for all workloads except newly excluded workload. + +## Workload generations and revisions + +The various `*-workloads.yaml` files for each phase of the test look at 2 important values: + +* The `deployment.kubernetes.io/revision` annotation +* The `metadata.generation` value + +Changes to the workload manifest that don't result in a new rollout increase the `generation`, but not the `revision`. + +In this case, the numbers become skewed when we annotate the deployments in step 4 (which does not trigger a rollout). + +These numbers are used to verify that the Odigos controllers have triggered an instrumentation rollout. diff --git a/tests/e2e/source/chainsaw-test.yaml b/tests/e2e/source/chainsaw-test.yaml index a80b4ba4c..81f108514 100644 --- a/tests/e2e/source/chainsaw-test.yaml +++ b/tests/e2e/source/chainsaw-test.yaml @@ -351,6 +351,16 @@ spec: fi sleep 5 done + - name: Wait for deleted sources to roll out new revisions + try: + - script: + timeout: 70s + content: | + kubectl rollout status deployment -l app=coupon + kubectl rollout status deployment -l app=frontend + kubectl rollout status deployment -l app=inventory + kubectl rollout status deployment -l app=pricing + kubectl rollout status deployment -l app=membership - name: Assert Runtime still Detected for single workload try: - assert: @@ -358,4 +368,89 @@ spec: file: 05-assert-runtime-detected.yaml - assert: timeout: 2m - file: 07-workloads.yaml \ No newline at end of file + file: 07-workloads.yaml + - name: Create Workload exclusion Source for single workload + try: + - apply: + file: 08-source.yaml + - name: Instrument rest of Namespace + try: + - apply: + file: 02-source-ns.yaml + - name: Wait for created source to roll out new revisions + try: + - script: + timeout: 70s + content: | + kubectl rollout status deployment -l app=coupon + kubectl rollout status deployment -l app=frontend + kubectl rollout status deployment -l app=inventory + kubectl rollout status deployment -l app=pricing + kubectl rollout status deployment -l app=membership + - name: Assert runtime detected for all workloads except excluded (coupon) + try: + - assert: + timeout: 2m + file: 08-assert-runtime-detected.yaml + - assert: + timeout: 2m + file: 08-workloads.yaml + - name: Assert runtime not detected for excluded (coupon) + try: + - script: + content: kubectl get instrumentationconfigs/deployment-coupon + check: + ($error != null): true + - name: Delete excluded workload Source + try: + - script: + content: kubectl delete sources -l odigos.io/e2e=source-excluded + check: + ($error == null): true + - name: Wait for deleted exclusion source to roll out new revisions + try: + - script: + timeout: 70s + content: | + kubectl rollout status deployment -l app=coupon + kubectl rollout status deployment -l app=frontend + kubectl rollout status deployment -l app=inventory + kubectl rollout status deployment -l app=pricing + kubectl rollout status deployment -l app=membership + - name: Assert runtime detected for no-longer-excluded workload + try: + - assert: + timeout: 2m + file: ../../common/assert/simple-demo-runtime-detected.yaml + - assert: + timeout: 2m + file: 08-workloads-2.yaml + - name: Create excluded workload Source while namespace is instrumented + try: + - apply: + file: 09-source.yaml + - name: Wait for created source to roll out new revisions + try: + - script: + timeout: 70s + content: | + kubectl rollout status deployment -l app=coupon + kubectl rollout status deployment -l app=frontend + kubectl rollout status deployment -l app=inventory + kubectl rollout status deployment -l app=pricing + kubectl rollout status deployment -l app=membership + - name: Assert runtime detected for all workloads except newly excluded (membership) + try: + - assert: + timeout: 2m + file: 09-assert-runtime-detected.yaml + - assert: + timeout: 2m + file: 09-workloads.yaml + - name: Assert runtime not detected for newly excluded (membership) + try: + - script: + content: kubectl get instrumentationconfigs/deployment-membership + check: + ($error != null): true + From eeefef56ff87c5786995b8581416afe5136e923a Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Tue, 7 Jan 2025 10:08:33 -0500 Subject: [PATCH 18/21] Deduplicate some code in controllers --- .../source_controller.go | 90 ++++++++------- .../startlangdetection/source_controller.go | 109 ++++++++++-------- k8sutils/pkg/workload/workloadkinds.go | 13 +++ 3 files changed, 124 insertions(+), 88 deletions(-) diff --git a/instrumentor/controllers/deleteinstrumentationconfig/source_controller.go b/instrumentor/controllers/deleteinstrumentationconfig/source_controller.go index 0fd4ee37b..b7b527e4a 100644 --- a/instrumentor/controllers/deleteinstrumentationconfig/source_controller.go +++ b/instrumentor/controllers/deleteinstrumentationconfig/source_controller.go @@ -66,47 +66,15 @@ func (r *SourceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr if source.Spec.Workload.Kind == "Namespace" { logger.V(2).Info("Uninstrumenting deployments for Namespace Source", "name", req.Name, "namespace", req.Namespace) - var deps appsv1.DeploymentList - err = r.Client.List(ctx, &deps, client.InNamespace(req.Namespace)) - if err != nil { - return ctrl.Result{}, err - } - - for _, dep := range deps.Items { - logger.V(4).Info("uninstrumenting deployment", "name", dep.Name, "namespace", dep.Namespace) - err := syncGenericWorkloadListToNs(ctx, r.Client, workload.WorkloadKindDeployment, client.ObjectKey{Namespace: dep.Namespace, Name: dep.Name}) - if err != nil { - return ctrl.Result{}, err - } - } - - logger.V(2).Info("Uninstrumenting statefulsets for Namespace Source", "name", req.Name, "namespace", req.Namespace) - var ss appsv1.StatefulSetList - err = r.Client.List(ctx, &ss, client.InNamespace(req.Namespace)) - if err != nil { - return ctrl.Result{}, err - } - for _, s := range ss.Items { - logger.V(4).Info("uninstrumenting statefulset", "name", s.Name, "namespace", s.Namespace) - err := syncGenericWorkloadListToNs(ctx, r.Client, workload.WorkloadKindStatefulSet, client.ObjectKey{Namespace: s.Namespace, Name: s.Name}) + for _, kind := range []workload.WorkloadKind{ + workload.WorkloadKindDaemonSet, + workload.WorkloadKindDeployment, + workload.WorkloadKindStatefulSet, + } { + result, err := r.listAndSyncWorkloadList(ctx, req, kind) if err != nil { - return ctrl.Result{}, err - } - } - - logger.V(2).Info("Uninstrumenting daemonsets for Namespace Source", "name", req.Name, "namespace", req.Namespace) - var ds appsv1.DaemonSetList - err = r.Client.List(ctx, &ds, client.InNamespace(req.Namespace)) - if err != nil { - return ctrl.Result{}, err - } - - for _, d := range ds.Items { - logger.V(4).Info("uninstrumenting daemonset", "name", d.Name, "namespace", d.Namespace) - err := syncGenericWorkloadListToNs(ctx, r.Client, workload.WorkloadKindDaemonSet, client.ObjectKey{Namespace: d.Namespace, Name: d.Name}) - if err != nil { - return ctrl.Result{}, err + return result, err } } } else { @@ -164,3 +132,47 @@ func (r *SourceReconciler) setSourceLabelsIfNecessary(ctx context.Context, sourc } return ctrl.Result{}, nil } + +func (r *SourceReconciler) listAndSyncWorkloadList(ctx context.Context, + req ctrl.Request, + kind workload.WorkloadKind) (ctrl.Result, error) { + logger := log.FromContext(ctx) + logger.V(2).Info("Uninstrumenting workloads for Namespace Source", "name", req.Name, "namespace", req.Namespace, "kind", kind) + + deps := workload.ClientListObjectFromWorkloadKind(kind) + err := r.Client.List(ctx, deps, client.InNamespace(req.Name)) + if client.IgnoreNotFound(err) != nil { + return ctrl.Result{}, err + } + + switch obj := deps.(type) { + case *appsv1.DeploymentList: + for _, dep := range obj.Items { + err = r.syncWorkloadList(ctx, kind, client.ObjectKey{Namespace: dep.Namespace, Name: dep.Name}) + if err != nil { + return ctrl.Result{}, err + } + } + case *appsv1.DaemonSetList: + for _, dep := range obj.Items { + err = r.syncWorkloadList(ctx, kind, client.ObjectKey{Namespace: dep.Namespace, Name: dep.Name}) + if err != nil { + return ctrl.Result{}, err + } + } + case *appsv1.StatefulSetList: + for _, dep := range obj.Items { + err = r.syncWorkloadList(ctx, kind, client.ObjectKey{Namespace: dep.Namespace, Name: dep.Name}) + if err != nil { + return ctrl.Result{}, err + } + } + } + return ctrl.Result{}, err +} + +func (r *SourceReconciler) syncWorkloadList(ctx context.Context, + kind workload.WorkloadKind, + key client.ObjectKey) error { + return syncGenericWorkloadListToNs(ctx, r.Client, kind, key) +} diff --git a/instrumentor/controllers/startlangdetection/source_controller.go b/instrumentor/controllers/startlangdetection/source_controller.go index 2cc507564..07a03e979 100644 --- a/instrumentor/controllers/startlangdetection/source_controller.go +++ b/instrumentor/controllers/startlangdetection/source_controller.go @@ -3,7 +3,7 @@ package startlangdetection import ( "context" - appsv1 "k8s.io/api/apps/v1" + v1 "k8s.io/api/apps/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" ctrl "sigs.k8s.io/controller-runtime" @@ -61,54 +61,14 @@ func (r *SourceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr namespaceKindSources[source.Spec.Workload.Kind][source.Spec.Workload.Name] = struct{}{} } - var deps appsv1.DeploymentList - err = r.Client.List(ctx, &deps, client.InNamespace(source.Spec.Workload.Name)) - if client.IgnoreNotFound(err) != nil { - logger.Error(err, "error fetching deployments") - return ctrl.Result{}, err - } - - for _, dep := range deps.Items { - if _, exists := namespaceKindSources[workload.WorkloadKindDeployment][dep.Name]; !exists { - request := ctrl.Request{NamespacedName: client.ObjectKey{Name: dep.Name, Namespace: dep.Namespace}} - _, err = reconcileWorkload(ctx, r.Client, workload.WorkloadKindDeployment, request, r.Scheme) - if err != nil { - logger.Error(err, "error requesting runtime details from odiglets", "name", dep.Name, "namespace", dep.Namespace) - } - } - } - - var sts appsv1.StatefulSetList - err = r.Client.List(ctx, &sts, client.InNamespace(source.Spec.Workload.Name)) - if client.IgnoreNotFound(err) != nil { - logger.Error(err, "error fetching statefulsets") - return ctrl.Result{}, err - } - - for _, st := range sts.Items { - if _, exists := namespaceKindSources[workload.WorkloadKindStatefulSet][st.Name]; !exists { - request := ctrl.Request{NamespacedName: client.ObjectKey{Name: st.Name, Namespace: st.Namespace}} - _, err = reconcileWorkload(ctx, r.Client, workload.WorkloadKindStatefulSet, request, r.Scheme) - if err != nil { - logger.Error(err, "error requesting runtime details from odiglets", "name", st.Name, "namespace", st.Namespace) - } - } - } - - var dss appsv1.DaemonSetList - err = r.Client.List(ctx, &dss, client.InNamespace(source.Spec.Workload.Name)) - if client.IgnoreNotFound(err) != nil { - logger.Error(err, "error fetching daemonsets") - return ctrl.Result{}, err - } - - for _, ds := range dss.Items { - if _, exists := namespaceKindSources[workload.WorkloadKindDaemonSet][ds.Name]; !exists { - request := ctrl.Request{NamespacedName: client.ObjectKey{Name: ds.Name, Namespace: ds.Namespace}} - _, err = reconcileWorkload(ctx, r.Client, workload.WorkloadKindDaemonSet, request, r.Scheme) - if err != nil { - logger.Error(err, "error requesting runtime details from odiglets", "name", ds.Name, "namespace", ds.Namespace) - } + for _, kind := range []workload.WorkloadKind{ + workload.WorkloadKindDaemonSet, + workload.WorkloadKindDeployment, + workload.WorkloadKindStatefulSet, + } { + result, err := r.listAndReconcileWorkloadList(ctx, source, kind, namespaceKindSources) + if err != nil { + return result, err } } } else { @@ -158,3 +118,54 @@ func (r *SourceReconciler) setSourceLabelsIfNecessary(ctx context.Context, sourc } return ctrl.Result{}, nil } + +func (r *SourceReconciler) listAndReconcileWorkloadList(ctx context.Context, + source *v1alpha1.Source, + kind workload.WorkloadKind, + namespaceKindSources map[workload.WorkloadKind]map[string]struct{}) (ctrl.Result, error) { + + deps := workload.ClientListObjectFromWorkloadKind(kind) + err := r.Client.List(ctx, deps, client.InNamespace(source.Spec.Workload.Name)) + if client.IgnoreNotFound(err) != nil { + return ctrl.Result{}, err + } + + switch obj := deps.(type) { + case *v1.DeploymentList: + for _, dep := range obj.Items { + err = r.reconcileWorkloadList(ctx, ctrl.Request{NamespacedName: client.ObjectKey{Name: dep.Name, Namespace: dep.Namespace}}, kind, namespaceKindSources) + if err != nil { + return ctrl.Result{}, err + } + } + case *v1.DaemonSetList: + for _, dep := range obj.Items { + err = r.reconcileWorkloadList(ctx, ctrl.Request{NamespacedName: client.ObjectKey{Name: dep.Name, Namespace: dep.Namespace}}, kind, namespaceKindSources) + if err != nil { + return ctrl.Result{}, err + } + } + case *v1.StatefulSetList: + for _, dep := range obj.Items { + err = r.reconcileWorkloadList(ctx, ctrl.Request{NamespacedName: client.ObjectKey{Name: dep.Name, Namespace: dep.Namespace}}, kind, namespaceKindSources) + if err != nil { + return ctrl.Result{}, err + } + } + } + return ctrl.Result{}, nil +} + +func (r *SourceReconciler) reconcileWorkloadList(ctx context.Context, + req ctrl.Request, + kind workload.WorkloadKind, + namespaceKindSources map[workload.WorkloadKind]map[string]struct{}) error { + logger := log.FromContext(ctx) + if _, exists := namespaceKindSources[kind][req.Name]; !exists { + _, err := reconcileWorkload(ctx, r.Client, kind, req, r.Scheme) + if err != nil { + logger.Error(err, "error requesting runtime details from odiglets", "name", req.Name, "namespace", req.Namespace, "kind", kind) + } + } + return nil +} diff --git a/k8sutils/pkg/workload/workloadkinds.go b/k8sutils/pkg/workload/workloadkinds.go index 199b729b9..c03d9dbf2 100644 --- a/k8sutils/pkg/workload/workloadkinds.go +++ b/k8sutils/pkg/workload/workloadkinds.go @@ -108,3 +108,16 @@ func ClientObjectFromWorkloadKind(kind WorkloadKind) client.Object { return nil } } + +func ClientListObjectFromWorkloadKind(kind WorkloadKind) client.ObjectList { + switch kind { + case WorkloadKindDeployment: + return &v1.DeploymentList{} + case WorkloadKindDaemonSet: + return &v1.DaemonSetList{} + case WorkloadKindStatefulSet: + return &v1.StatefulSetList{} + default: + return nil + } +} From 626e042bdf4de08da233a50cbbb6828a8c9fafc8 Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Fri, 3 Jan 2025 20:33:04 -0500 Subject: [PATCH 19/21] Use request.Namespace when injecting env vars in webhook --- .../controllers/instrumentationdevice/pods_webhook.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/instrumentor/controllers/instrumentationdevice/pods_webhook.go b/instrumentor/controllers/instrumentationdevice/pods_webhook.go index 83e9ad042..8759dbe08 100644 --- a/instrumentor/controllers/instrumentationdevice/pods_webhook.go +++ b/instrumentor/controllers/instrumentationdevice/pods_webhook.go @@ -14,6 +14,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" @@ -62,6 +63,13 @@ func (p *PodsWebhook) getServiceNameForEnv(ctx context.Context, pod *corev1.Pod) return nil, nil } + req, err := admission.RequestFromContext(ctx) + if err != nil { + logger.Error(err, "failed to get admission request from context") + return nil, nil + } + podWorkload.Namespace = req.Namespace + workloadObj, err := workload.GetWorkloadObject(ctx, client.ObjectKey{Namespace: podWorkload.Namespace, Name: podWorkload.Name}, podWorkload.Kind, p.Client) if err != nil { logger.Error(err, "failed to get workload object from cache. cannot check for workload annotation. using workload name as OTEL_SERVICE_NAME") From 5bffb1eb9aa49ca12d4704ee8627b43a4b79cdab Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Fri, 3 Jan 2025 21:15:00 -0500 Subject: [PATCH 20/21] Use reported name annotation in workload-lifecycle tests --- .../02-update-workload-manifests.yaml | 30 ++++++++++++++++++- .../workload-lifecycle/02-wait-for-trace.yaml | 29 +++++++++--------- 2 files changed, 44 insertions(+), 15 deletions(-) diff --git a/tests/e2e/workload-lifecycle/02-update-workload-manifests.yaml b/tests/e2e/workload-lifecycle/02-update-workload-manifests.yaml index c9b5432c9..eede39830 100644 --- a/tests/e2e/workload-lifecycle/02-update-workload-manifests.yaml +++ b/tests/e2e/workload-lifecycle/02-update-workload-manifests.yaml @@ -55,6 +55,8 @@ metadata: namespace: default labels: app: nodejs-minimum-version + annotations: + odigos.io/reported-name: nodejs-minimum-version-reported spec: selector: matchLabels: @@ -80,6 +82,8 @@ metadata: namespace: default labels: app: nodejs-latest-version + annotations: + odigos.io/reported-name: nodejs-latest-version-reported spec: selector: matchLabels: @@ -105,6 +109,8 @@ metadata: namespace: default labels: app: nodejs-dockerfile-env + annotations: + odigos.io/reported-name: nodejs-dockerfile-env-reported spec: selector: matchLabels: @@ -130,6 +136,8 @@ metadata: namespace: default labels: app: nodejs-manifest-env + annotations: + odigos.io/reported-name: nodejs-manifest-env-reported spec: selector: matchLabels: @@ -183,6 +191,8 @@ metadata: namespace: default labels: app: java-supported-version + annotations: + odigos.io/reported-name: java-supported-version-reported spec: selector: matchLabels: @@ -212,6 +222,8 @@ metadata: namespace: default labels: app: java-azul + annotations: + odigos.io/reported-name: java-azul-reported spec: selector: matchLabels: @@ -241,6 +253,8 @@ metadata: namespace: default labels: app: java-supported-docker-env + annotations: + odigos.io/reported-name: java-supported-docker-env-reported spec: selector: matchLabels: @@ -271,6 +285,8 @@ metadata: namespace: default labels: app: java-supported-manifest-env + annotations: + odigos.io/reported-name: java-supported-manifest-env-reported spec: selector: matchLabels: @@ -301,6 +317,8 @@ metadata: namespace: default labels: app: java-latest-version + annotations: + odigos.io/reported-name: java-latest-version-reported spec: selector: matchLabels: @@ -331,6 +349,8 @@ metadata: namespace: default labels: app: java-old-version + annotations: + odigos.io/reported-name: java-old-version-reported spec: selector: matchLabels: @@ -360,6 +380,8 @@ metadata: namespace: default labels: app: python-latest-version + annotations: + odigos.io/reported-name: python-latest-version-reported spec: selector: matchLabels: @@ -397,6 +419,8 @@ metadata: namespace: default labels: app: python-alpine + annotations: + odigos.io/reported-name: python-alpine-reported spec: selector: matchLabels: @@ -434,6 +458,8 @@ metadata: namespace: default labels: app: python-not-supported + annotations: + odigos.io/reported-name: python-not-supported-reported spec: selector: matchLabels: @@ -471,6 +497,8 @@ metadata: namespace: default labels: app: python-min-version + annotations: + odigos.io/reported-name: python-min-version-reported spec: selector: matchLabels: @@ -611,4 +639,4 @@ spec: - containerPort: 8080 readinessProbe: tcpSocket: - port: 8080 \ No newline at end of file + port: 8080 diff --git a/tests/e2e/workload-lifecycle/02-wait-for-trace.yaml b/tests/e2e/workload-lifecycle/02-wait-for-trace.yaml index 41289ee4b..f0d00d800 100644 --- a/tests/e2e/workload-lifecycle/02-wait-for-trace.yaml +++ b/tests/e2e/workload-lifecycle/02-wait-for-trace.yaml @@ -2,18 +2,19 @@ apiVersion: e2e.tests.odigos.io/v1 kind: TraceTest description: This test waits for a trace that is generated from the successful instrumented services. query: | - { resource.service.name = "nodejs-minimum-version" } || - { resource.service.name = "nodejs-latest-version" } || - { resource.service.name = "nodejs-dockerfile-env" } || - { resource.service.name = "nodejs-manifest-env" } || - { resource.service.name = "java-supported-version" } || - { resource.service.name = "java-latest-version" } || - { resource.service.name = "java-old-version" } || - { resource.service.name = "java-supported-docker-env" } || - { resource.service.name = "java-supported-manifest-env" } || - { resource.service.name = "java-azul" } || - { resource.service.name = "python-latest-version" && span.http.route = "insert-random/" } || - { resource.service.name = "python-alpine" && span.http.route = "insert-random/" } || - { resource.service.name = "python-min-version" && span.http.route = "insert-random/" } + { resource.service.name = "nodejs-minimum-version-reported" } || + { resource.service.name = "nodejs-latest-version-reported" } || + { resource.service.name = "nodejs-dockerfile-env-reported" } || + { resource.service.name = "nodejs-manifest-env-reported" } || + { resource.service.name = "java-supported-version-reported" } || + { resource.service.name = "java-latest-version-reported" } || + { resource.service.name = "java-old-version-reported" } || + { resource.service.name = "java-supported-docker-env-reported" } || + { resource.service.name = "java-supported-manifest-env-reported" } || + { resource.service.name = "java-azul-reported" } || + { resource.service.name = "python-latest-version-reported" && span.http.route = "insert-random/" } || + { resource.service.name = "python-alpine-reported" && span.http.route = "insert-random/" } || + { resource.service.name = "python-not-supported-reported" && span.http.route = "insert-random/" } || + { resource.service.name = "python-min-version-reported" && span.http.route = "insert-random/" } expected: - count: 26 # 13 before +13 new ones + count: 13 From 27a767cb3f81ea50273624f7fdb52851326c5e95 Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Wed, 8 Jan 2025 12:23:29 -0500 Subject: [PATCH 21/21] Feedback --- api/odigos/v1alpha1/source_types.go | 9 ++++----- .../deleteinstrumentationconfig/common.go | 2 +- .../instrumentationconfig_controller.go | 2 +- .../namespace_controller.go | 2 +- .../source_controller.go | 14 +++++++------- .../startlangdetection/namespace_controller.go | 2 +- .../startlangdetection/workload_controllers.go | 2 +- 7 files changed, 16 insertions(+), 17 deletions(-) diff --git a/api/odigos/v1alpha1/source_types.go b/api/odigos/v1alpha1/source_types.go index a4338b4c5..1d3bfe38b 100644 --- a/api/odigos/v1alpha1/source_types.go +++ b/api/odigos/v1alpha1/source_types.go @@ -78,11 +78,10 @@ type WorkloadSources struct { Namespace *Source } -// GetSourceListForWorkload returns a SourceList of all Sources that have matching -// workload name, namespace, and kind labels for an object. In theory, this should only -// ever return a list with 0, 1, or 2 items, but due diligence should handle unexpected cases. -// If a Workload returns >1 Namespace or Workload Source, this function returns an error. -func GetSourceListForWorkload(ctx context.Context, kubeClient client.Client, obj client.Object) (*WorkloadSources, error) { +// GetWorkloadSources returns a WorkloadSources listing the Workload and Namespace Source +// that currently apply to the given object. In theory, this should only ever return at most +// 1 Namespace and/or 1 Workload Source for an object. If more are found, an error is returned. +func GetWorkloadSources(ctx context.Context, kubeClient client.Client, obj client.Object) (*WorkloadSources, error) { var err error workloadSources := &WorkloadSources{} diff --git a/instrumentor/controllers/deleteinstrumentationconfig/common.go b/instrumentor/controllers/deleteinstrumentationconfig/common.go index 7ee85e342..dd0dcedaf 100644 --- a/instrumentor/controllers/deleteinstrumentationconfig/common.go +++ b/instrumentor/controllers/deleteinstrumentationconfig/common.go @@ -25,7 +25,7 @@ func reconcileWorkloadObject(ctx context.Context, kubeClient client.Client, work } // Check if a Source object exists for this workload - sourceList, err := odigosv1.GetSourceListForWorkload(ctx, kubeClient, workloadObject) + sourceList, err := odigosv1.GetWorkloadSources(ctx, kubeClient, workloadObject) if err != nil { return err } diff --git a/instrumentor/controllers/deleteinstrumentationconfig/instrumentationconfig_controller.go b/instrumentor/controllers/deleteinstrumentationconfig/instrumentationconfig_controller.go index 0859df539..561644100 100644 --- a/instrumentor/controllers/deleteinstrumentationconfig/instrumentationconfig_controller.go +++ b/instrumentor/controllers/deleteinstrumentationconfig/instrumentationconfig_controller.go @@ -92,7 +92,7 @@ func (r *InstrumentationConfigReconciler) Reconcile(ctx context.Context, req ctr if !instEffectiveEnabled { // Check if a Source object exists for this workload - sourceList, err := v1alpha1.GetSourceListForWorkload(ctx, r.Client, workloadObject) + sourceList, err := v1alpha1.GetWorkloadSources(ctx, r.Client, workloadObject) if err != nil { return ctrl.Result{}, err } diff --git a/instrumentor/controllers/deleteinstrumentationconfig/namespace_controller.go b/instrumentor/controllers/deleteinstrumentationconfig/namespace_controller.go index 3a01eba0c..32398ee60 100644 --- a/instrumentor/controllers/deleteinstrumentationconfig/namespace_controller.go +++ b/instrumentor/controllers/deleteinstrumentationconfig/namespace_controller.go @@ -170,7 +170,7 @@ func syncGenericWorkloadListToNs(ctx context.Context, c client.Client, kind work // instrumented due to the label on the namespace. These are workloads with the label missing. // (they inherit the instrumentation from the namespace this way) func isInheritingInstrumentationFromNs(ctx context.Context, c client.Client, obj client.Object) (bool, error) { - sourceList, err := v1alpha1.GetSourceListForWorkload(ctx, c, obj) + sourceList, err := v1alpha1.GetWorkloadSources(ctx, c, obj) if err != nil { return false, err } diff --git a/instrumentor/controllers/deleteinstrumentationconfig/source_controller.go b/instrumentor/controllers/deleteinstrumentationconfig/source_controller.go index b7b527e4a..d1620ade6 100644 --- a/instrumentor/controllers/deleteinstrumentationconfig/source_controller.go +++ b/instrumentor/controllers/deleteinstrumentationconfig/source_controller.go @@ -52,7 +52,7 @@ func (r *SourceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr // If this is a regular Source that's being deleted, or a workload Exclusion Source // that's being created, try to uninstrument relevant workloads. if source.DeletionTimestamp.IsZero() == v1alpha1.IsWorkloadExcludedSource(source) { - logger.Info("Reconciling workload for deleted Source object", "name", req.Name, "namespace", req.Namespace) + logger.Info("Reconciling workload for Source object", "name", req.Name, "namespace", req.Namespace) if result, err := r.setSourceLabelsIfNecessary(ctx, source); err != nil { return result, err @@ -65,7 +65,7 @@ func (r *SourceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr } if source.Spec.Workload.Kind == "Namespace" { - logger.V(2).Info("Uninstrumenting deployments for Namespace Source", "name", req.Name, "namespace", req.Namespace) + logger.V(2).Info("Uninstrumenting workloads for Namespace Source", "name", req.Name, "namespace", req.Namespace) for _, kind := range []workload.WorkloadKind{ workload.WorkloadKindDaemonSet, @@ -86,7 +86,7 @@ func (r *SourceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr return ctrl.Result{}, err } - sourceList, err := v1alpha1.GetSourceListForWorkload(ctx, r.Client, obj) + sourceList, err := v1alpha1.GetWorkloadSources(ctx, r.Client, obj) if err != nil { return ctrl.Result{}, err } @@ -139,13 +139,13 @@ func (r *SourceReconciler) listAndSyncWorkloadList(ctx context.Context, logger := log.FromContext(ctx) logger.V(2).Info("Uninstrumenting workloads for Namespace Source", "name", req.Name, "namespace", req.Namespace, "kind", kind) - deps := workload.ClientListObjectFromWorkloadKind(kind) - err := r.Client.List(ctx, deps, client.InNamespace(req.Name)) - if client.IgnoreNotFound(err) != nil { + workloads := workload.ClientListObjectFromWorkloadKind(kind) + err := r.Client.List(ctx, workloads, client.InNamespace(req.Name)) + if err != nil { return ctrl.Result{}, err } - switch obj := deps.(type) { + switch obj := workloads.(type) { case *appsv1.DeploymentList: for _, dep := range obj.Items { err = r.syncWorkloadList(ctx, kind, client.ObjectKey{Namespace: dep.Namespace, Name: dep.Name}) diff --git a/instrumentor/controllers/startlangdetection/namespace_controller.go b/instrumentor/controllers/startlangdetection/namespace_controller.go index e179f005b..6f3e63b45 100644 --- a/instrumentor/controllers/startlangdetection/namespace_controller.go +++ b/instrumentor/controllers/startlangdetection/namespace_controller.go @@ -30,7 +30,7 @@ func (n *NamespacesReconciler) Reconcile(ctx context.Context, request ctrl.Reque } if !k8sutils.IsObjectLabeledForInstrumentation(&ns) { - sourceList, err := v1alpha1.GetSourceListForWorkload(ctx, n.Client, &ns) + sourceList, err := v1alpha1.GetWorkloadSources(ctx, n.Client, &ns) if err != nil { return ctrl.Result{}, err } diff --git a/instrumentor/controllers/startlangdetection/workload_controllers.go b/instrumentor/controllers/startlangdetection/workload_controllers.go index 79be52627..181ccf3ae 100644 --- a/instrumentor/controllers/startlangdetection/workload_controllers.go +++ b/instrumentor/controllers/startlangdetection/workload_controllers.go @@ -59,7 +59,7 @@ func reconcileWorkload(ctx context.Context, k8sClient client.Client, objKind wor if !instrumented { // Check if a Source object exists for this workload - sourceList, err := odigosv1.GetSourceListForWorkload(ctx, k8sClient, obj) + sourceList, err := odigosv1.GetWorkloadSources(ctx, k8sClient, obj) if err != nil { return ctrl.Result{}, err }