Migrate odigos-instrumentation label to Source CRD

Makefile

@@ -281,7 +281,7 @@ crd-apply: api-all cli-upgrade
 dev-tests-kind-cluster:
 	@echo "Creating a kind cluster for development"
 	kind delete cluster
-	kind create cluster
+	kind create cluster --config=tests/common/apply/kind-config.yaml
 
 .PHONY: dev-tests-setup
 dev-tests-setup: TAG := e2e-test

api/odigos/v1alpha1/source_types.go

@@ -87,14 +87,19 @@ func GetSources(ctx context.Context, kubeClient client.Client, obj client.Object
 	var err error
 	workloadSources := &WorkloadSources{}
 
-	if obj.GetObjectKind().GroupVersionKind().Kind != "Namespace" {
+	namespace := obj.GetNamespace()
+	if len(namespace) == 0 && obj.GetObjectKind().GroupVersionKind().Kind == string(workload.WorkloadKindNamespace) {
+		namespace = obj.GetName()
+	}
+
+	if obj.GetObjectKind().GroupVersionKind().Kind != string(workload.WorkloadKindNamespace) {
 		sourceList := SourceList{}
 		selector := labels.SelectorFromSet(labels.Set{
 			consts.WorkloadNameLabel:      obj.GetName(),
-			consts.WorkloadNamespaceLabel: obj.GetNamespace(),
+			consts.WorkloadNamespaceLabel: namespace,
 			consts.WorkloadKindLabel:      obj.GetObjectKind().GroupVersionKind().Kind,
 		})
-		err := kubeClient.List(ctx, &sourceList, &client.ListOptions{LabelSelector: selector}, client.InNamespace(obj.GetNamespace()))
+		err := kubeClient.List(ctx, &sourceList, &client.ListOptions{LabelSelector: selector}, client.InNamespace(namespace))
 		if err != nil {
 			return nil, err
 		}
@@ -108,11 +113,11 @@ func GetSources(ctx context.Context, kubeClient client.Client, obj client.Object
 
 	namespaceSourceList := SourceList{}
 	namespaceSelector := labels.SelectorFromSet(labels.Set{
-		consts.WorkloadNameLabel:      obj.GetNamespace(),
-		consts.WorkloadNamespaceLabel: obj.GetNamespace(),
-		consts.WorkloadKindLabel:      "Namespace",
+		consts.WorkloadNameLabel:      namespace,
+		consts.WorkloadNamespaceLabel: namespace,
+		consts.WorkloadKindLabel:      string(workload.WorkloadKindNamespace),
 	})
-	err = kubeClient.List(ctx, &namespaceSourceList, &client.ListOptions{LabelSelector: namespaceSelector}, client.InNamespace(obj.GetNamespace()))
+	err = kubeClient.List(ctx, &namespaceSourceList, &client.ListOptions{LabelSelector: namespaceSelector}, client.InNamespace(namespace))
 	if err != nil {
 		return nil, err
 	}

helm/odigos/templates/instrumentor/webhook-source.yaml → helm/odigos/templates/instrumentor/webhooks.yaml

@@ -3,6 +3,42 @@
 {{- $cert := genSignedCert "serving-cert" nil $altNames 365 $ca -}}
 apiVersion: admissionregistration.k8s.io/v1
 kind: MutatingWebhookConfiguration
+metadata:
+  name: mutating-webhook-configuration
+  labels:
+    app.kubernetes.io/name: pod-mutating-webhook
+    app.kubernetes.io/instance: mutating-webhook-configuration
+    app.kubernetes.io/component: webhook
+    app.kubernetes.io/created-by: instrumentor
+    app.kubernetes.io/part-of: odigos
+webhooks:
+  - name: pod-mutating-webhook.odigos.io
+    clientConfig:
+      caBundle: {{ $ca.Cert | b64enc }}
+      service:
+        name: odigos-instrumentor
+        namespace: {{ .Release.Namespace }}
+        path: /mutate--v1-pod
+        port: 9443
+    rules:
+      - operations:
+          - CREATE
+          - UPDATE
+        apiGroups: [""]
+        apiVersions: ["v1"]
+        resources: ["pods"]
+        scope: Namespaced
+    failurePolicy: Ignore
+    reinvocationPolicy: IfNeeded
+    sideEffects: None
+    objectSelector:
+      matchLabels:
+        odigos.io/inject-instrumentation: "true"
+    timeoutSeconds: 10
+    admissionReviewVersions: ["v1"]
+---
+apiVersion: admissionregistration.k8s.io/v1
+kind: MutatingWebhookConfiguration
 metadata:
   name: source-mutating-webhook-configuration
   labels:
@@ -64,4 +100,23 @@ webhooks:
     failurePolicy: Ignore
     sideEffects: None
     timeoutSeconds: 10
-    admissionReviewVersions: ["v1"]
+    admissionReviewVersions: ["v1"]
+---
+apiVersion: v1
+kind: Secret
+type: kubernetes.io/tls
+metadata:
+  name: webhook-cert
+  namespace: {{ .Release.Namespace }}
+  labels:
+    app.kubernetes.io/name: instrumentor-cert
+    app.kubernetes.io/instance: instrumentor-cert
+    app.kubernetes.io/component: certificate
+    app.kubernetes.io/created-by: instrumentor
+    app.kubernetes.io/part-of: odigos
+  annotations:
+    "helm.sh/hook": "pre-install,pre-upgrade"
+    "helm.sh/hook-delete-policy": "before-hook-creation"
+data:
+  tls.crt: {{ $cert.Cert | b64enc }}
+  tls.key: {{ $cert.Key | b64enc }}

instrumentor/controllers/deleteinstrumentationconfig/common.go

@@ -3,35 +3,29 @@ package deleteinstrumentationconfig
 import (
 	"context"
 
-	odigosv1 "github.com/odigos-io/odigos/api/odigos/v1alpha1"
-	"github.com/odigos-io/odigos/common/consts"
-	"github.com/odigos-io/odigos/k8sutils/pkg/workload"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/log"
+
+	odigosv1 "github.com/odigos-io/odigos/api/odigos/v1alpha1"
+	"github.com/odigos-io/odigos/common/consts"
+	sourceutils "github.com/odigos-io/odigos/k8sutils/pkg/source"
+	"github.com/odigos-io/odigos/k8sutils/pkg/workload"
 )
 
 func reconcileWorkloadObject(ctx context.Context, kubeClient client.Client, workloadObject client.Object) error {
 	logger := log.FromContext(ctx)
-	instEffectiveEnabled, err := workload.IsWorkloadInstrumentationEffectiveEnabled(ctx, kubeClient, workloadObject)
-	if err != nil {
-		logger.Error(err, "error checking if instrumentation is effective")
-		return err
-	}
 
-	if instEffectiveEnabled {
-		return nil
+	if err := sourceutils.MigrateInstrumentationLabelToDisabledSource(ctx, kubeClient, workloadObject, workload.WorkloadKindFromClientObject(workloadObject)); err != nil {
+		return err
 	}
 
-	// Check if a Source object exists for this workload
-	sourceList, err := odigosv1.GetSources(ctx, kubeClient, workloadObject)
+	instrumented, err := sourceutils.IsObjectInstrumentedBySource(ctx, kubeClient, workloadObject)
 	if err != nil {
 		return err
 	}
-	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
+	if instrumented {
 		return nil
 	}
 

instrumentor/controllers/deleteinstrumentationconfig/daemonset_controller.go

@@ -19,6 +19,7 @@ package deleteinstrumentationconfig
 import (
 	"context"
 
+	k8sutils "github.com/odigos-io/odigos/k8sutils/pkg/utils"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 
 	appsv1 "k8s.io/api/apps/v1"
@@ -48,5 +49,5 @@ func (r *DaemonSetReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 	}
 
 	err = reconcileWorkloadObject(ctx, r.Client, &ds)
-	return ctrl.Result{}, err
+	return k8sutils.K8SUpdateErrorHandler(err)
 }

instrumentor/controllers/deleteinstrumentationconfig/deployment_controller.go

@@ -19,6 +19,7 @@ package deleteinstrumentationconfig
 import (
 	"context"
 
+	k8sutils "github.com/odigos-io/odigos/k8sutils/pkg/utils"
 	appsv1 "k8s.io/api/apps/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -47,5 +48,5 @@ func (r *DeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 	}
 
 	err = reconcileWorkloadObject(ctx, r.Client, &dep)
-	return ctrl.Result{}, err
+	return k8sutils.K8SUpdateErrorHandler(err)
 }

instrumentor/controllers/deleteinstrumentationconfig/deployment_controller_test.go

@@ -2,19 +2,22 @@ package deleteinstrumentationconfig_test
 
 import (
 	"context"
+	"time"
 
 	odigosv1 "github.com/odigos-io/odigos/api/odigos/v1alpha1"
 	"github.com/odigos-io/odigos/instrumentor/internal/testutil"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 var _ = Describe("deleteInstrumentationConfig Deployment controller", func() {
 	ctx := context.Background()
 	var namespace *corev1.Namespace
 	var deployment *appsv1.Deployment
+	var source *odigosv1.Source
 	var instrumentationConfig *odigosv1.InstrumentationConfig
 
 	Describe("Delete InstrumentationConfig", func() {
@@ -28,14 +31,17 @@ var _ = Describe("deleteInstrumentationConfig Deployment controller", func() {
 				deployment = testutil.SetOdigosInstrumentationEnabled(testutil.NewMockTestDeployment(namespace))
 				Expect(k8sClient.Create(ctx, deployment)).Should(Succeed())
 
+				source = testutil.NewMockSource(deployment)
+				Expect(k8sClient.Create(ctx, source)).Should(Succeed())
+
 				instrumentationConfig = testutil.NewMockInstrumentationConfig(deployment)
 				Expect(k8sClient.Create(ctx, instrumentationConfig)).Should(Succeed())
 			})
 
-			It("InstrumentationConfig deleted after removing instrumentation label from deployment", func() {
+			It("InstrumentationConfig retained after removing instrumentation label from deployment", func() {
 				deployment = testutil.DeleteOdigosInstrumentationLabel(deployment)
 				Expect(k8sClient.Update(ctx, deployment)).Should(Succeed())
-				testutil.AssertInstrumentationConfigDeleted(ctx, k8sClient, instrumentationConfig)
+				testutil.AssertInstrumentationConfigRetained(ctx, k8sClient, instrumentationConfig)
 			})
 
 			It("InstrumentationConfig deleted after setting instrumentation label to disabled", func() {
@@ -52,9 +58,15 @@ var _ = Describe("deleteInstrumentationConfig Deployment controller", func() {
 				namespace = testutil.SetOdigosInstrumentationEnabled(testutil.NewMockNamespace())
 				Expect(k8sClient.Create(ctx, namespace)).Should(Succeed())
 
+				source = testutil.NewMockSource(namespace)
+				Expect(k8sClient.Create(ctx, source)).Should(Succeed())
+
 				deployment = testutil.SetOdigosInstrumentationEnabled(testutil.NewMockTestDeployment(namespace))
 				Expect(k8sClient.Create(ctx, deployment)).Should(Succeed())
 
+				source = testutil.NewMockSource(deployment)
+				Expect(k8sClient.Create(ctx, source)).Should(Succeed())
+
 				instrumentationConfig = testutil.NewMockInstrumentationConfig(deployment)
 				Expect(k8sClient.Create(ctx, instrumentationConfig)).Should(Succeed())
 			})
@@ -83,14 +95,20 @@ var _ = Describe("deleteInstrumentationConfig Deployment controller", func() {
 			deployment = testutil.SetReportedNameAnnotation(deployment, "test")
 			Expect(k8sClient.Create(ctx, deployment)).Should(Succeed())
 
+			source = testutil.NewMockSource(deployment)
+			Expect(k8sClient.Create(ctx, source)).Should(Succeed())
+
 			instrumentationConfig = testutil.NewMockInstrumentationConfig(deployment)
 			Expect(k8sClient.Create(ctx, instrumentationConfig)).Should(Succeed())
 		})
 
-		It("should delete the reported name annotation on instrumentation label deleted", func() {
+		It("should delete the reported name annotation on instrumentation label disabled", func() {
 
-			deployment = testutil.SetOdigosInstrumentationDisabled(deployment)
-			Expect(k8sClient.Update(ctx, deployment)).Should(Succeed())
+			Eventually(func() error {
+				k8sClient.Get(ctx, client.ObjectKey{Namespace: deployment.GetNamespace(), Name: deployment.GetName()}, deployment)
+				deployment = testutil.SetOdigosInstrumentationDisabled(deployment)
+				return k8sClient.Update(ctx, deployment)
+			}, time.Second*2, time.Millisecond*250).Should(Succeed())
 			testutil.AssertReportedNameAnnotationDeletedDeployment(ctx, k8sClient, deployment)
 		})
 
@@ -99,6 +117,10 @@ var _ = Describe("deleteInstrumentationConfig Deployment controller", func() {
 			annotationKey := "test"
 			annotationValue := "test"
 
+			Expect(k8sClient.Get(ctx, client.ObjectKey{Namespace: deployment.GetNamespace(), Name: deployment.GetName()}, deployment)).Should(Succeed())
+			if len(deployment.Annotations) == 0 {
+				deployment.Annotations = make(map[string]string)
+			}
 			deployment.Annotations[annotationKey] = annotationValue
 			Expect(k8sClient.Update(ctx, deployment)).Should(Succeed())
 

instrumentor/controllers/deleteinstrumentationconfig/instrumentationconfig_controller.go

@@ -20,9 +20,8 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/odigos-io/odigos/api/odigos/v1alpha1"
 	odigosv1 "github.com/odigos-io/odigos/api/odigos/v1alpha1"
-	"github.com/odigos-io/odigos/k8sutils/pkg/workload"
+	sourceutils "github.com/odigos-io/odigos/k8sutils/pkg/source"
 
 	appsv1 "k8s.io/api/apps/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -84,23 +83,15 @@ func (r *InstrumentationConfigReconciler) Reconcile(ctx context.Context, req ctr
 		return ctrl.Result{}, err
 	}
 
-	instEffectiveEnabled, err := workload.IsWorkloadInstrumentationEffectiveEnabled(ctx, r.Client, workloadObject)
+	enabled, err := sourceutils.IsObjectInstrumentedBySource(ctx, r.Client, workloadObject)
 	if err != nil {
-		logger.Error(err, "error checking if instrumentation is effective")
 		return ctrl.Result{}, err
 	}
 
-	if !instEffectiveEnabled {
-		// Check if a Source object exists for this workload
-		sourceList, err := v1alpha1.GetSources(ctx, r.Client, workloadObject)
-		if err != nil {
-			return ctrl.Result{}, err
-		}
-		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)
-		}
+	if !enabled {
+		logger.Info("Deleting instrumentationconfig for non-enabled workload")
+		err := r.Client.Delete(ctx, &instrumentationConfig)
+		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
 	return ctrl.Result{}, nil