Skip to content

Commit

Permalink
List pods to find those relevant for runtime detection instead of cac…
Browse files Browse the repository at this point in the history
…hing the workload objects (#2325)

This PR refactors the runtime detection controllers to avoid `Get` or
`List` operation on workload objects (Deployments, StatefulSets and
DaemonSets). As a result:
* The Odiglet permissions are narrowed.
* The cache won't contain workload objects which results in a
significant memory consumption improvement (especially in large
clusters).

This is done by modifying the `insttrumentationconfig` reconciler to
list the Pods associated to the reconciled instrumentationConfig.
The Odiglet cache is already configured to only include Pods whithin the
same node, hence this list operation will only return pods in the same
node and in the same ns of the instrumentationConfig.

---------

Co-authored-by: Amir Blum <[email protected]>
  • Loading branch information
RonFed and blumamir authored Jan 30, 2025
1 parent 6e75b40 commit 4de2fa5
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 119 deletions.
12 changes: 0 additions & 12 deletions cli/cmd/resources/odiglet.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,18 +110,6 @@ func NewOdigletClusterRole(psp bool) *rbacv1.ClusterRole {
Resources: []string{"pods/status"},
Verbs: []string{"get"},
},
{ // Needed for language detection
// TODO: remove this once Tamir/PR is read for new language detection
APIGroups: []string{"apps"},
Resources: []string{"deployments", "daemonsets", "statefulsets"},
Verbs: []string{"get", "list", "watch"},
},
{ // Needed for language detection
// TODO: remove this once Tamir/PR is read for new language detection
APIGroups: []string{"apps"},
Resources: []string{"deployments/status", "daemonsets/status", "statefulsets/status"},
Verbs: []string{"get"},
},
{ // Needed for virtual device registration
APIGroups: []string{""},
Resources: []string{"nodes"},
Expand Down
18 changes: 0 additions & 18 deletions helm/odigos/templates/odiglet/clusterrole.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,24 +17,6 @@ rules:
- pods/status
verbs:
- get
- apiGroups:
- apps
resources:
- deployments
- daemonsets
- statefulsets
verbs:
- get
- list
- watch
- apiGroups:
- apps
resources:
- deployments/status
- daemonsets/status
- statefulsets/status
verbs:
- get
- apiGroups:
- ''
resources:
Expand Down
24 changes: 24 additions & 0 deletions odiglet/pkg/kube/runtime_details/common.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package runtime_details

import (
"github.com/odigos-io/odigos/k8sutils/pkg/workload"
corev1 "k8s.io/api/core/v1"
)

func getPodWorkloadObject(pod *corev1.Pod) (*workload.PodWorkload, error) {
for _, owner := range pod.OwnerReferences {
workloadName, workloadKind, err := workload.GetWorkloadFromOwnerReference(owner)
if err != nil {
return nil, workload.IgnoreErrorKindNotSupported(err)
}

return &workload.PodWorkload{
Name: workloadName,
Kind: workloadKind,
Namespace: pod.Namespace,
}, nil
}

// Pod does not necessarily have to be managed by a controller
return nil, nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,9 @@ import (
odigosv1 "github.com/odigos-io/odigos/api/odigos/v1alpha1"
criwrapper "github.com/odigos-io/odigos/k8sutils/pkg/cri"
k8sutils "github.com/odigos-io/odigos/k8sutils/pkg/utils"
k8scontainer "github.com/odigos-io/odigos/k8sutils/pkg/container"
"github.com/odigos-io/odigos/k8sutils/pkg/workload"
kubeutils "github.com/odigos-io/odigos/odiglet/pkg/kube/utils"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/kubernetes"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -60,7 +58,6 @@ type InstrumentationConfigReconciler struct {
}

func (r *InstrumentationConfigReconciler) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) {

logger := log.FromContext(ctx)

var instrumentationConfig odigosv1.InstrumentationConfig
Expand All @@ -73,27 +70,42 @@ func (r *InstrumentationConfigReconciler) Reconcile(ctx context.Context, request
return reconcile.Result{}, fmt.Errorf("InstrumentationConfig %s/%s has %d owner references, expected 1", instrumentationConfig.Namespace, instrumentationConfig.Name, len(instrumentationConfig.OwnerReferences))
}

workload, labels, err := getWorkloadAndLabelsfromOwner(ctx, r.Client, instrumentationConfig.Namespace, instrumentationConfig.OwnerReferences[0])
// find pods that are managed by the workload,
// filter out pods that are being deleted or not ready,
// note that the controller-runtime cache is assumed here to only contain pods in the same node as the odiglet
var podList corev1.PodList
err = r.List(ctx, &podList, client.InNamespace(instrumentationConfig.Namespace))
if err != nil {
logger.Error(err, "Failed to get workload and labels from owner")
return reconcile.Result{}, err
}

pods, err := kubeutils.GetRunningPods(ctx, labels, workload.GetNamespace(), r.Client)
if err != nil {
return reconcile.Result{}, err
var selectedPods []corev1.Pod
for _, pod := range podList.Items {
// skip pods that are being deleted or not ready
if pod.DeletionTimestamp != nil || !k8scontainer.AllContainersReady(&pod) {
continue
}
podWorkload, err := getPodWorkloadObject(&pod)
if errors.Is(err, workload.ErrKindNotSupported) {
continue
}
if podWorkload == nil {
// pod is not managed by a workload, no runtime details detection needed
continue
}

// get instrumentation config name for the pod
instrumentationConfigName := workload.CalculateWorkloadRuntimeObjectName(podWorkload.Name, podWorkload.Kind)
if instrumentationConfigName == instrumentationConfig.Name {
selectedPods = append(selectedPods, pod)
}
}

odigosConfig, err := k8sutils.GetCurrentOdigosConfig(ctx, r.Client)
if err != nil {
return k8sutils.K8SNoEffectiveConfigErrorHandler(err)
}

var selectedPods []corev1.Pod
if len(pods) > 0 {
selectedPods = append(selectedPods, pods[0])
}

runtimeResults, err := runtimeInspection(ctx, selectedPods, odigosConfig.IgnoredContainers, r.CriClient)
if err != nil {
return reconcile.Result{}, err
Expand All @@ -110,37 +122,3 @@ func (r *InstrumentationConfigReconciler) Reconcile(ctx context.Context, request
return reconcile.Result{}, nil
}

func getWorkloadAndLabelsfromOwner(ctx context.Context, k8sClient client.Client, ns string, ownerReference metav1.OwnerReference) (client.Object, map[string]string, error) {
workloadName, workloadKind, err := workload.GetWorkloadFromOwnerReference(ownerReference)
if err != nil {
return nil, nil, err
}

switch workloadKind {
case "Deployment":
var dep appsv1.Deployment
err := k8sClient.Get(ctx, client.ObjectKey{Namespace: ns, Name: workloadName}, &dep)
if err != nil {
return nil, nil, err
}
return &dep, dep.Spec.Selector.MatchLabels, nil
case "DaemonSet":
var ds appsv1.DaemonSet
err := k8sClient.Get(ctx, client.ObjectKey{Namespace: ns, Name: workloadName}, &ds)
if err != nil {
return nil, nil, err
}

return &ds, ds.Spec.Selector.MatchLabels, nil
case "StatefulSet":
var sts appsv1.StatefulSet
err := k8sClient.Get(ctx, client.ObjectKey{Namespace: ns, Name: workloadName}, &sts)
if err != nil {
return nil, nil, err
}

return &sts, sts.Spec.Selector.MatchLabels, nil
}

return nil, nil, errors.New("workload kind not supported")
}
20 changes: 1 addition & 19 deletions odiglet/pkg/kube/runtime_details/pods_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func (p *PodsReconciler) Reconcile(ctx context.Context, request reconcile.Reques
return reconcile.Result{}, client.IgnoreNotFound(err)
}

podWorkload, err := p.getPodWorkloadObject(ctx, &pod)
podWorkload, err := getPodWorkloadObject(&pod)
if err != nil {
logger.Error(err, "error getting pod workload object")
return reconcile.Result{}, err
Expand Down Expand Up @@ -78,24 +78,6 @@ func (p *PodsReconciler) Reconcile(ctx context.Context, request reconcile.Reques
return reconcile.Result{}, nil
}

func (p *PodsReconciler) getPodWorkloadObject(ctx context.Context, pod *corev1.Pod) (*workload.PodWorkload, error) {
for _, owner := range pod.OwnerReferences {
workloadName, workloadKind, err := workload.GetWorkloadFromOwnerReference(owner)
if err != nil {
return nil, workload.IgnoreErrorKindNotSupported(err)
}

return &workload.PodWorkload{
Name: workloadName,
Kind: workloadKind,
Namespace: pod.Namespace,
}, nil
}

// Pod does not necessarily have to be managed by a controller
return nil, nil
}

func InstrumentationConfigContainsUnknownLanguage(config odigosv1.InstrumentationConfig) bool {
for _, containerDetails := range config.Status.RuntimeDetailsByContainer {
if containerDetails.Language == common.UnknownProgrammingLanguage {
Expand Down
22 changes: 0 additions & 22 deletions odiglet/pkg/kube/utils/utils.go
Original file line number Diff line number Diff line change
@@ -1,41 +1,19 @@
package utils

import (
"context"
"fmt"

k8scontainer "github.com/odigos-io/odigos/k8sutils/pkg/container"
"github.com/odigos-io/odigos/k8sutils/pkg/workload"
"github.com/odigos-io/odigos/odiglet/pkg/env"
"go.opentelemetry.io/otel/attribute"
semconv "go.opentelemetry.io/otel/semconv/v1.21.0"
corev1 "k8s.io/api/core/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
)

func IsPodInCurrentNode(pod *corev1.Pod) bool {
return pod.Spec.NodeName == env.Current.NodeName
}

func GetRunningPods(ctx context.Context, labels map[string]string, ns string, kubeClient client.Client) ([]corev1.Pod, error) {
var podList corev1.PodList
err := kubeClient.List(ctx, &podList, client.MatchingLabels(labels), client.InNamespace(ns))
if err != nil {
return nil, err
}

var filteredPods []corev1.Pod
for _, pod := range podList.Items {
if IsPodInCurrentNode(&pod) && pod.DeletionTimestamp == nil {
if k8scontainer.AllContainersReady(&pod) {
filteredPods = append(filteredPods, pod)
}
}
}

return filteredPods, nil
}

func GetResourceAttributes(podWorkload *workload.PodWorkload, podName string) []attribute.KeyValue {
attrs := []attribute.KeyValue{
semconv.K8SNamespaceName(podWorkload.Namespace),
Expand Down

0 comments on commit 4de2fa5

Please sign in to comment.