Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Namespace Source instrumentation #2105

Merged
merged 22 commits into from
Jan 8, 2025
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
7ef4d94
Include Namespace Sources in GetSourceListForWorkload
damemi Dec 30, 2024
e0320fa
Add Namespace instrumentation to startlangdetection
damemi Dec 30, 2024
e0a96bc
Add Namespace uninstrumentation to deletinstrumentedapplication
damemi Dec 30, 2024
2bf63a8
Add Namespace instrumentation e2e + uninstrumentation e2e
damemi Dec 30, 2024
57b61c5
e2e: wait for instrumentation to take effect
damemi Jan 2, 2025
c228058
Don't delete explicitly instrumented sources when NS is uninstrumented
damemi Jan 2, 2025
ea8ac3c
Fix instEffectEnabled check in deleteinstrumentedapplication
damemi Jan 2, 2025
8676015
Fix inheritance check for individual workload sources
damemi Jan 3, 2025
253d622
Retry traffic generator -- check for minimum spans
damemi Jan 3, 2025
35ea009
Remove sleeps and increase traffic generation timeouts
damemi Jan 3, 2025
ba56913
Remove wait for conditions
damemi Jan 6, 2025
0bc6c26
Merge remote-tracking branch 'upstream/feature/source-crd' into sourc…
damemi Jan 6, 2025
1b26c5a
React to merge
damemi Jan 6, 2025
f1ac129
Update exclude label, update comment, use WorkloadSources object
damemi Jan 6, 2025
ef48c59
Update source e2e to not use InstrumentedApplication
damemi Jan 6, 2025
4277faa
Got the delete instrumentationconfig check wrong again
damemi Jan 6, 2025
1ea0bb2
Move namespace pre-processing into namespace path
damemi Jan 6, 2025
7295ac9
Support creation/deletion of excluded Source in instrumented namespace
damemi Jan 7, 2025
eeefef5
Deduplicate some code in controllers
damemi Jan 7, 2025
626e042
Use request.Namespace when injecting env vars in webhook
damemi Jan 4, 2025
5bffb1e
Use reported name annotation in workload-lifecycle tests
damemi Jan 4, 2025
27a767c
Feedback
damemi Jan 8, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions api/odigos/v1alpha1/source_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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...)
damemi marked this conversation as resolved.
Show resolved Hide resolved
damemi marked this conversation as resolved.
Show resolved Hide resolved
return sourceList, err
}

Expand Down
1 change: 1 addition & 0 deletions common/consts/consts.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const (
OdigosNamespaceAnnotation = "odigos.io/workload-namespace"
OdigosWorkloadKindAnnotation = "odigos.io/workload-kind"
OdigosWorkloadNameAnnotation = "odigos.io/workload-name"
OdigosWorkloadExcludedLabel = "odigos.io/excluded"
damemi marked this conversation as resolved.
Show resolved Hide resolved
OdigosReportedNameAnnotation = "odigos.io/reported-name"

// GatewayMaxConnectionAge and GatewayMaxConnectionAgeGrace are the default values for the gateway collector.
Expand Down
24 changes: 14 additions & 10 deletions instrumentor/controllers/deleteinstrumentedapplication/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,18 @@ 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 {
// 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
}

if err := deleteWorkloadInstrumentedApplication(ctx, kubeClient, workloadObject); err != nil {
Expand All @@ -46,11 +50,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{
Expand All @@ -73,7 +78,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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -137,7 +138,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)
Expand All @@ -151,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
Expand All @@ -165,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()
damemi marked this conversation as resolved.
Show resolved Hide resolved
if labels == nil {
return true
return true, nil
}
_, exists := labels[consts.OdigosInstrumentationLabel]
return !exists
return !exists, nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,14 @@ package deleteinstrumentedapplication

import (
"context"
"errors"

"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"
Expand All @@ -38,7 +41,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)
damemi marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -48,24 +51,86 @@ 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 {
damemi marked this conversation as resolved.
Show resolved Hide resolved
logger.Error(err, "error fetching deployments")
damemi marked this conversation as resolved.
Show resolved Hide resolved
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 {
// 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 {
// TODO: Deleted objects should be filtered in the event filter
return ctrl.Result{}, err
}

// 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
}
}

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
}
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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")
Expand Down
Loading
Loading