Skip to content

Commit

Permalink
Update instrumentor unit tests and make test
Browse files Browse the repository at this point in the history
  • Loading branch information
damemi committed Feb 26, 2025
1 parent dbaa8f8 commit fdea8b8
Show file tree
Hide file tree
Showing 7 changed files with 12 additions and 22 deletions.
10 changes: 5 additions & 5 deletions instrumentor/controllers/agentenabled/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func SetupWithManager(mgr ctrl.Manager, dp *distros.Provider) error {
For(&odigosv1.CollectorsGroup{}).
WithEventFilter(predicate.And(&odigospredicate.OdigosCollectorsGroupNodePredicate, &odigospredicate.CgBecomesReadyPredicate{})).
Complete(&CollectorsGroupReconciler{
Client: mgr.GetClient(),
Client: mgr.GetClient(),
DistrosProvider: dp,
})
if err != nil {
Expand All @@ -33,7 +33,7 @@ func SetupWithManager(mgr ctrl.Manager, dp *distros.Provider) error {
// When the instrumentation config is deleted, we need to roll out the workload to un-instrument it.
WithEventFilter(predicate.Or(&instrumentorpredicate.RuntimeDetailsChangedPredicate{}, odigospredicate.DeletionPredicate{})).
Complete(&InstrumentationConfigReconciler{
Client: mgr.GetClient(),
Client: mgr.GetClient(),
DistrosProvider: dp,
})
if err != nil {
Expand All @@ -46,7 +46,7 @@ func SetupWithManager(mgr ctrl.Manager, dp *distros.Provider) error {
For(&odigosv1.InstrumentationRule{}).
WithEventFilter(&instrumentorpredicate.OtelSdkInstrumentationRulePredicate{}).
Complete(&InstrumentationRuleReconciler{
Client: mgr.GetClient(),
Client: mgr.GetClient(),
DistrosProvider: dp,
})
if err != nil {
Expand All @@ -59,7 +59,7 @@ func SetupWithManager(mgr ctrl.Manager, dp *distros.Provider) error {
For(&corev1.ConfigMap{}).
WithEventFilter(odigospredicate.OdigosEffectiveConfigMapPredicate).
Complete(&EffectiveConfigReconciler{
Client: mgr.GetClient(),
Client: mgr.GetClient(),
DistrosProvider: dp,
})
if err != nil {
Expand All @@ -70,7 +70,7 @@ func SetupWithManager(mgr ctrl.Manager, dp *distros.Provider) error {
WebhookManagedBy(mgr).
For(&corev1.Pod{}).
WithDefaulter(&PodsWebhook{
Client: mgr.GetClient(),
Client: mgr.GetClient(),
DistrosGetter: dp.Getter,
}).
Complete()
Expand Down
1 change: 0 additions & 1 deletion instrumentor/controllers/agentenabled/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,4 +77,3 @@ func getRelevantInstrumentationRules(ctx context.Context, c client.Client, pw k8

return &relevantIr, nil
}

2 changes: 1 addition & 1 deletion instrumentor/controllers/sourceinstrumentation/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func syncNamespaceWorkloads(

for _, key := range objectKeys {
// For namespace instrumentation, we only want to reconcile workloads that don't have their own explicit Source object because:
// For instrumentation:
// For instrumentation:
// - settings in Workload Sources take priority over settings in Namespace Sources
// - disabled Workload Sources prevent instrumentation
// For uninstrumentation:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ var _ = Describe("deleteInstrumentationConfig Deployment controller", func() {
Expect(k8sClient.Create(ctx, source)).Should(Succeed())

instrumentationConfig = testutil.NewMockInstrumentationConfig(deployment)
Expect(k8sClient.Create(ctx, instrumentationConfig)).Should(Succeed())
//Expect(k8sClient.Create(ctx, instrumentationConfig)).Should(Succeed())
})

It("InstrumentationConfig deleted after removing instrumentation source from deployment", func() {
Expand Down Expand Up @@ -66,7 +66,6 @@ var _ = Describe("deleteInstrumentationConfig Deployment controller", func() {
Expect(k8sClient.Create(ctx, depSource)).Should(Succeed())

instrumentationConfig = testutil.NewMockInstrumentationConfig(deployment)
Expect(k8sClient.Create(ctx, instrumentationConfig)).Should(Succeed())
})

It("InstrumentationConfig retain when removing instrumentation source from deployment", func() {
Expand Down Expand Up @@ -96,7 +95,6 @@ var _ = Describe("deleteInstrumentationConfig Deployment controller", func() {
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 source disabled", func() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ func (r *NamespaceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
}

return syncNamespaceWorkloads(
ctx,
r.Client,
ctx,
r.Client,
r.Scheme,
ns.GetName(),
reconcileFunc)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,8 @@ var _ = Describe("DeleteInstrumentationConfig Namespace controller", func() {

// these workloads has instrumentation application because the namespace has instrumentation enabled
instrumentationConfigDeployment = testutil.NewMockInstrumentationConfig(deployment)
Expect(k8sClient.Create(ctx, instrumentationConfigDeployment)).Should(Succeed())
instrumentationConfigDaemonSet = testutil.NewMockInstrumentationConfig(daemonSet)
Expect(k8sClient.Create(ctx, instrumentationConfigDaemonSet)).Should(Succeed())
instrumentationConfigStatefulSet = testutil.NewMockInstrumentationConfig(statefulSet)
Expect(k8sClient.Create(ctx, instrumentationConfigStatefulSet)).Should(Succeed())
})

It("should delete instrumented application", func() {
Expand Down Expand Up @@ -98,13 +95,9 @@ var _ = Describe("DeleteInstrumentationConfig Namespace controller", func() {
sourceStatefulSet = testutil.NewMockSource(statefulSet)
Expect(k8sClient.Create(ctx, sourceStatefulSet)).Should(Succeed())

// these workloads has instrumentation application because the namespace has instrumentation enabled
instrumentationConfigDeployment = testutil.NewMockInstrumentationConfig(deployment)
Expect(k8sClient.Create(ctx, instrumentationConfigDeployment)).Should(Succeed())
instrumentationConfigDaemonSet = testutil.NewMockInstrumentationConfig(daemonSet)
Expect(k8sClient.Create(ctx, instrumentationConfigDaemonSet)).Should(Succeed())
instrumentationConfigStatefulSet = testutil.NewMockInstrumentationConfig(statefulSet)
Expect(k8sClient.Create(ctx, instrumentationConfigStatefulSet)).Should(Succeed())
})

It("should retain instrumented application", func() {
Expand Down
6 changes: 3 additions & 3 deletions instrumentor/instrumentor.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
)

type Instrumentor struct {
mgr controllerruntime.Manager
mgr controllerruntime.Manager
logger logr.Logger
}

Expand All @@ -42,14 +42,14 @@ func New(opts controllers.KubeManagerOptions, dp *distros.Provider) (*Instrument
return nil, fmt.Errorf("unable to set up health check: %w", err)
}

if err := mgr.AddReadyzCheck("readyz", func(req *http.Request) error{
if err := mgr.AddReadyzCheck("readyz", func(req *http.Request) error {
return mgr.GetWebhookServer().StartedChecker()(req)
}); err != nil {
return nil, fmt.Errorf("unable to set up ready check: %w", err)
}

return &Instrumentor{
mgr: mgr,
mgr: mgr,
logger: opts.Logger,
}, nil
}
Expand Down

0 comments on commit fdea8b8

Please sign in to comment.