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

Migrate odigos-instrumentation label to Source CRD #2231

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
94497dc
k8sutils: add Source helpers and remove unused label helpers
damemi Jan 14, 2025
d2e286a
api+webhook: fix namespace checks
damemi Jan 14, 2025
adacba0
startlangdetection: add label->source enable migration
damemi Jan 14, 2025
08f1eb9
deleteinstrumentationconfig: add label->source disabled migration
damemi Jan 14, 2025
e79e251
Update tests
damemi Jan 14, 2025
7fd632b
startlangdetection: update WorkloadEnabledPredicate->WorkloadAvailabl…
damemi Jan 14, 2025
801f5fa
Update controller predicates and IsObjectInstrumentedBySource check
damemi Jan 14, 2025
929455e
lint: Add WorkloadKindNamespace const and other linters
damemi Jan 15, 2025
804b47a
Build Source webhooks in 1 call
damemi Jan 15, 2025
47dcd81
Use K8SUpdateErrorHandler in functions that update Source + use Calcu…
damemi Jan 15, 2025
773dc66
Remove redundant Source checks for controllers using predicates
damemi Jan 15, 2025
8a96dd9
Fix IsObjectInstrumentedBySource check
damemi Jan 15, 2025
2292067
deleteinstrumentationconfig: Use IsObjectInstrumentedBySource in checks
damemi Jan 15, 2025
bff839f
feedback: Rename Source Predicates, rename IsActiveSource->IsSourceRe…
damemi Jan 16, 2025
8f469cf
Merge branch 'main' into source-label-migration
damemi Jan 16, 2025
8da249a
Merge branch 'main' into source-label-migration
damemi Jan 16, 2025
bb3201e
Merge branch 'main' into source-label-migration
damemi Jan 16, 2025
c1ee54d
Merge branch 'main' into source-label-migration
damemi Jan 17, 2025
ff9466c
Don't let NotFound errors keep you from deleting the finalizer
damemi Jan 17, 2025
cc31bbe
Add instrumentation filter in workload predicate for startlangdetection
damemi Jan 17, 2025
7406790
Merge branch 'main' into source-label-migration
damemi Jan 17, 2025
e1291f0
Fix make-dev-tests (#2235)
damemi Jan 17, 2025
0aa9632
Pass events with errors in workload predicate
damemi Jan 18, 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
12 changes: 9 additions & 3 deletions instrumentor/controllers/startlangdetection/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ func SetupWithManager(mgr ctrl.Manager) error {
ControllerManagedBy(mgr).
Named("startlangdetection-deployment").
For(&appsv1.Deployment{}).
WithEventFilter(&WorkloadAvailablePredicate{}).
WithEventFilter(&WorkloadAvailablePredicate{
Client: mgr.GetClient(),
}).
Complete(&DeploymentReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Expand All @@ -27,7 +29,9 @@ func SetupWithManager(mgr ctrl.Manager) error {
ControllerManagedBy(mgr).
Named("startlangdetection-daemonset").
For(&appsv1.DaemonSet{}).
WithEventFilter(&WorkloadAvailablePredicate{}).
WithEventFilter(&WorkloadAvailablePredicate{
Client: mgr.GetClient(),
}).
Complete(&DaemonSetReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Expand All @@ -40,7 +44,9 @@ func SetupWithManager(mgr ctrl.Manager) error {
ControllerManagedBy(mgr).
Named("startlangdetection-statefulset").
For(&appsv1.StatefulSet{}).
WithEventFilter(&WorkloadAvailablePredicate{}).
WithEventFilter(&WorkloadAvailablePredicate{
Client: mgr.GetClient(),
}).
Complete(&StatefulSetReconciler{
Client: mgr.GetClient(),
Scheme: mgr.GetScheme(),
Expand Down
17 changes: 17 additions & 0 deletions instrumentor/controllers/startlangdetection/workload_predicate.go
Original file line number Diff line number Diff line change
@@ -1,23 +1,35 @@
package startlangdetection

import (
"context"

"github.com/odigos-io/odigos/k8sutils/pkg/env"
"github.com/odigos-io/odigos/k8sutils/pkg/workload"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/event"
"sigs.k8s.io/controller-runtime/pkg/predicate"

sourceutils "github.com/odigos-io/odigos/k8sutils/pkg/source"
)

// this predicate is used for workload reconciler to only pass events
// where the workload has replicas available to instrument.
type WorkloadAvailablePredicate struct {
damemi marked this conversation as resolved.
Show resolved Hide resolved
predicate.Funcs
Client client.Client
}

func (i *WorkloadAvailablePredicate) Create(e event.CreateEvent) bool {
w, err := workload.ObjectToWorkload(e.Object)
if err != nil {
return false
}

instrumented, _ := sourceutils.IsObjectInstrumentedBySource(context.Background(), i.Client, e.Object)
if !instrumented || !workload.IsObjectLabeledForInstrumentation(e.Object) {
return false
}

return w.AvailableReplicas() > 0
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: we can move this check before the API calls, to potentially save calls.

}

Expand All @@ -43,6 +55,11 @@ func (i *WorkloadAvailablePredicate) Update(e event.UpdateEvent) bool {
return false
}

instrumented, _ := sourceutils.IsObjectInstrumentedBySource(context.Background(), i.Client, e.ObjectNew)
if !instrumented || !workload.IsObjectLabeledForInstrumentation(e.ObjectNew) {
return false
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same nit as above: maybe better to do the replicas check before this.

newReplicas := wNew.AvailableReplicas()
oldReplicas := wOld.AvailableReplicas()

Expand Down
Loading