Skip to content

Commit 21978cb

Browse files
author
eliranb
committed
Refactor LightrunJavaAgent reconciliation logic to support new WorkloadName and WorkloadType fields, replacing legacy DeploymentName usage. Update indexers and error handling for improved clarity and maintainability.
1 parent 124b19f commit 21978cb

File tree

2 files changed

+131
-69
lines changed

2 files changed

+131
-69
lines changed

‎internal/controller/helpers.go‎

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -25,49 +25,57 @@ const (
2525

2626
func (r *LightrunJavaAgentReconciler) mapDeploymentToAgent(ctx context.Context, obj client.Object) []reconcile.Request {
2727
deployment := obj.(*appsv1.Deployment)
28-
29-
var lightrunJavaAgentList agentv1beta.LightrunJavaAgentList
30-
31-
if err := r.List(ctx, &lightrunJavaAgentList,
28+
// TODO: remove this once we deprecate deploymentNameIndexField
29+
var agents agentv1beta.LightrunJavaAgentList
30+
if err := r.List(ctx, &agents,
3231
client.InNamespace(deployment.Namespace),
33-
client.MatchingFields{deploymentNameIndexField: deployment.Name},
32+
client.MatchingFields{
33+
deploymentNameIndexField: deployment.Name, // old agents
34+
},
3435
); err != nil {
35-
r.Log.Error(err, "could not list LightrunJavaAgentList. "+
36-
"change to deployment will not be reconciled.",
37-
deployment.Name, deployment.Namespace)
38-
return nil
36+
r.Log.Error(err, "failed to list by deploymentNameIndexField")
37+
}
38+
// New indexer for workloadNameIndexField
39+
var newAgents agentv1beta.LightrunJavaAgentList
40+
if err := r.List(ctx, &newAgents,
41+
client.InNamespace(deployment.Namespace),
42+
client.MatchingFields{
43+
workloadNameIndexField: deployment.Name, // new agents
44+
},
45+
); err != nil {
46+
r.Log.Error(err, "failed to list by workloadNameIndexField")
3947
}
4048

41-
requests := make([]reconcile.Request, len(lightrunJavaAgentList.Items))
49+
// Combine results
50+
agents.Items = append(agents.Items, newAgents.Items...)
4251

43-
for i, lightrunJavaAgent := range lightrunJavaAgentList.Items {
44-
requests[i] = reconcile.Request{
45-
NamespacedName: client.ObjectKeyFromObject(&lightrunJavaAgent),
46-
}
52+
requests := make([]reconcile.Request, len(agents.Items))
53+
for i, a := range agents.Items {
54+
requests[i] = reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&a)}
4755
}
4856
return requests
4957
}
5058

5159
func (r *LightrunJavaAgentReconciler) mapStatefulSetToAgent(ctx context.Context, obj client.Object) []reconcile.Request {
5260
statefulSet := obj.(*appsv1.StatefulSet)
5361

54-
var lightrunJavaAgentList agentv1beta.LightrunJavaAgentList
62+
var agents agentv1beta.LightrunJavaAgentList
5563

56-
if err := r.List(ctx, &lightrunJavaAgentList,
64+
if err := r.List(ctx, &agents,
5765
client.InNamespace(statefulSet.Namespace),
58-
client.MatchingFields{statefulSetNameIndexField: statefulSet.Name},
66+
client.MatchingFields{workloadNameIndexField: statefulSet.Name},
5967
); err != nil {
6068
r.Log.Error(err, "could not list LightrunJavaAgentList. "+
6169
"change to statefulset will not be reconciled.",
6270
statefulSet.Name, statefulSet.Namespace)
6371
return nil
6472
}
6573

66-
requests := make([]reconcile.Request, len(lightrunJavaAgentList.Items))
74+
requests := make([]reconcile.Request, len(agents.Items))
6775

68-
for i, lightrunJavaAgent := range lightrunJavaAgentList.Items {
76+
for i, agent := range agents.Items {
6977
requests[i] = reconcile.Request{
70-
NamespacedName: client.ObjectKeyFromObject(&lightrunJavaAgent),
78+
NamespacedName: client.ObjectKeyFromObject(&agent),
7179
}
7280
}
7381
return requests

‎internal/controller/lightrunjavaagent_controller.go‎

Lines changed: 103 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package controller
1919
import (
2020
"context"
2121
"errors"
22+
"fmt"
2223

2324
appsv1 "k8s.io/api/apps/v1"
2425
corev1 "k8s.io/api/core/v1"
@@ -36,10 +37,10 @@ import (
3637
)
3738

3839
const (
39-
deploymentNameIndexField = "spec.deployment"
40-
statefulSetNameIndexField = "spec.statefulset"
41-
secretNameIndexField = "spec.secret"
42-
finalizerName = "agent.finalizers.lightrun.com"
40+
deploymentNameIndexField = "spec.deployment"
41+
workloadNameIndexField = "spec.workloadName"
42+
secretNameIndexField = "spec.secret"
43+
finalizerName = "agent.finalizers.lightrun.com"
4344
)
4445

4546
var err error
@@ -68,36 +69,77 @@ func (r *LightrunJavaAgentReconciler) Reconcile(ctx context.Context, req ctrl.Re
6869
}
6970

7071
// Determine which workload type to reconcile
71-
if lightrunJavaAgent.Spec.DeploymentName != "" && lightrunJavaAgent.Spec.StatefulSetName != "" {
72-
log.Error(nil, "Both DeploymentName and StatefulSetName are set. Only one should be specified")
73-
return r.errorStatus(ctx, lightrunJavaAgent, errors.New("both deployment and statefulset specified"))
74-
} else if lightrunJavaAgent.Spec.DeploymentName != "" {
75-
// Handle Deployment reconciliation (existing code)
72+
workloadType, err := r.determineWorkloadType(lightrunJavaAgent)
73+
if err != nil {
74+
log.Error(err, "failed to determine workload type")
75+
return r.errorStatus(ctx, lightrunJavaAgent, err)
76+
}
77+
switch workloadType {
78+
case agentv1beta.WorkloadTypeDeployment:
7679
return r.reconcileDeployment(ctx, lightrunJavaAgent, req.Namespace)
77-
} else if lightrunJavaAgent.Spec.StatefulSetName != "" {
78-
// Handle StatefulSet reconciliation (to be implemented)
80+
case agentv1beta.WorkloadTypeStatefulSet:
7981
return r.reconcileStatefulSet(ctx, lightrunJavaAgent, req.Namespace)
80-
} else {
81-
log.Error(nil, "Neither DeploymentName nor StatefulSetName is set")
82-
return r.errorStatus(ctx, lightrunJavaAgent, errors.New("no workload specified"))
82+
default:
83+
return r.errorStatus(ctx, lightrunJavaAgent, fmt.Errorf("unsupported workload type: %s", workloadType))
84+
}
85+
}
86+
87+
func (r *LightrunJavaAgentReconciler) determineWorkloadType(lightrunJavaAgent *agentv1beta.LightrunJavaAgent) (agentv1beta.WorkloadType, error) {
88+
spec := lightrunJavaAgent.Spec
89+
90+
// === Case 1: Legacy only — DeploymentName only ===
91+
if spec.DeploymentName != "" && spec.WorkloadName == "" && spec.WorkloadType == "" {
92+
return agentv1beta.WorkloadTypeDeployment, nil
93+
}
94+
95+
// === Case 2: New fields — WorkloadName + WorkloadType ===
96+
if spec.DeploymentName == "" && spec.WorkloadName != "" {
97+
if spec.WorkloadType == "" {
98+
return "", errors.New("WorkloadType must be set when using WorkloadName")
99+
}
100+
return spec.WorkloadType, nil
101+
}
102+
103+
// === Case 3: Misconfigured — Both names are set ===
104+
if spec.DeploymentName != "" && spec.WorkloadName != "" {
105+
// Same names still ambiguous — rely on workloadType
106+
if spec.DeploymentName == spec.WorkloadName {
107+
if spec.WorkloadType == "" {
108+
return "", errors.New("both DeploymentName and WorkloadName are set and equal; please remove DeploymentName and set workloadType explicitly")
109+
}
110+
return spec.WorkloadType, nil
111+
}
112+
// Names differ — reject as invalid
113+
return "", errors.New("DeploymentName and WorkloadName are both set but differ; please use only WorkloadName with WorkloadType")
83114
}
115+
116+
// === Case 4: Fully empty or malformed ===
117+
return "", errors.New("invalid configuration: must set either DeploymentName (legacy) or WorkloadName with WorkloadType")
84118
}
85119

86120
// reconcileDeployment handles the reconciliation logic for Deployment workloads
87121
func (r *LightrunJavaAgentReconciler) reconcileDeployment(ctx context.Context, lightrunJavaAgent *agentv1beta.LightrunJavaAgent, namespace string) (ctrl.Result, error) {
88-
log := r.Log.WithValues("lightrunJavaAgent", lightrunJavaAgent.Name, "deployment", lightrunJavaAgent.Spec.DeploymentName)
122+
// Get the workload name - use DeploymentName for backward compatibility
123+
// or WorkloadName for newer CR versions
124+
deploymentName := lightrunJavaAgent.Spec.WorkloadName
125+
if deploymentName == "" && lightrunJavaAgent.Spec.DeploymentName != "" {
126+
// Fall back to legacy field if WorkloadName isn't set
127+
deploymentName = lightrunJavaAgent.Spec.DeploymentName
128+
}
129+
130+
log := r.Log.WithValues("lightrunJavaAgent", lightrunJavaAgent.Name, "deployment", deploymentName)
89131
fieldManager := "lightrun-conrtoller"
90132

91133
deplNamespacedObj := client.ObjectKey{
92-
Name: lightrunJavaAgent.Spec.DeploymentName,
134+
Name: deploymentName,
93135
Namespace: namespace,
94136
}
95137
originalDeployment := &appsv1.Deployment{}
96138
err = r.Get(ctx, deplNamespacedObj, originalDeployment)
97139
if err != nil {
98140
// Deployment not found
99141
if client.IgnoreNotFound(err) == nil {
100-
log.Info("Deployment not found. Verify name/namespace", "Deployment", lightrunJavaAgent.Spec.DeploymentName)
142+
log.Info("Deployment not found. Verify name/namespace", "Deployment", deploymentName)
101143
// remove our finalizer from the list and update it.
102144
err = r.removeFinalizer(ctx, lightrunJavaAgent, finalizerName)
103145
if err != nil {
@@ -141,7 +183,7 @@ func (r *LightrunJavaAgentReconciler) reconcileDeployment(ctx context.Context, l
141183

142184
// Ensure that finalizer is in place
143185
if !containsString(lightrunJavaAgent.ObjectMeta.Finalizers, finalizerName) {
144-
log.Info("Start working on deployment", "Deployment", lightrunJavaAgent.Spec.DeploymentName)
186+
log.Info("Start working on deployment", "Deployment", deploymentName)
145187
log.Info("Adding finalizer")
146188
err = r.addFinalizer(ctx, lightrunJavaAgent, finalizerName)
147189
if err != nil {
@@ -199,7 +241,7 @@ func (r *LightrunJavaAgentReconciler) reconcileDeployment(ctx context.Context, l
199241
return r.errorStatus(ctx, lightrunJavaAgent, err)
200242
}
201243

202-
log.Info("Deployment returned to original state", "Deployment", lightrunJavaAgent.Spec.DeploymentName)
244+
log.Info("Deployment returned to original state", "Deployment", deploymentName)
203245
return r.successStatus(ctx, lightrunJavaAgent, reconcileTypeProgressing)
204246
} else {
205247
// Nothing to do here
@@ -245,7 +287,7 @@ func (r *LightrunJavaAgentReconciler) reconcileDeployment(ctx context.Context, l
245287
cmDataHash := configMapDataHash(cm.Data)
246288

247289
// Server side apply
248-
log.V(2).Info("Patching deployment, SSA", "Deployment", lightrunJavaAgent.Spec.DeploymentName, "LightunrJavaAgent", lightrunJavaAgent.Name)
290+
log.V(2).Info("Patching deployment, SSA", "Deployment", deploymentName, "LightunrJavaAgent", lightrunJavaAgent.Name)
249291
err = r.patchDeployment(lightrunJavaAgent, secret, originalDeployment, deploymentApplyConfig, cmDataHash)
250292
if err != nil {
251293
log.Error(err, "unable to patch deployment")
@@ -307,25 +349,25 @@ func (r *LightrunJavaAgentReconciler) reconcileDeployment(ctx context.Context, l
307349
}
308350

309351
// Update status to Healthy
310-
log.V(1).Info("Reconciling finished successfully", "Deployment", lightrunJavaAgent.Spec.DeploymentName, "LightunrJavaAgent", lightrunJavaAgent.Name)
352+
log.V(1).Info("Reconciling finished successfully", "Deployment", deploymentName, "LightunrJavaAgent", lightrunJavaAgent.Name)
311353
return r.successStatus(ctx, lightrunJavaAgent, reconcileTypeReady)
312354
}
313355

314356
// reconcileStatefulSet handles the reconciliation logic for StatefulSet workloads
315357
func (r *LightrunJavaAgentReconciler) reconcileStatefulSet(ctx context.Context, lightrunJavaAgent *agentv1beta.LightrunJavaAgent, namespace string) (ctrl.Result, error) {
316-
log := r.Log.WithValues("lightrunJavaAgent", lightrunJavaAgent.Name, "statefulSet", lightrunJavaAgent.Spec.StatefulSetName)
358+
log := r.Log.WithValues("lightrunJavaAgent", lightrunJavaAgent.Name, "statefulSet", lightrunJavaAgent.Spec.WorkloadName)
317359
fieldManager := "lightrun-controller"
318360

319361
stsNamespacedObj := client.ObjectKey{
320-
Name: lightrunJavaAgent.Spec.StatefulSetName,
362+
Name: lightrunJavaAgent.Spec.WorkloadName,
321363
Namespace: namespace,
322364
}
323365
originalStatefulSet := &appsv1.StatefulSet{}
324366
err = r.Get(ctx, stsNamespacedObj, originalStatefulSet)
325367
if err != nil {
326368
// StatefulSet not found
327369
if client.IgnoreNotFound(err) == nil {
328-
log.Info("StatefulSet not found. Verify name/namespace", "StatefulSet", lightrunJavaAgent.Spec.StatefulSetName)
370+
log.Info("StatefulSet not found. Verify name/namespace", "StatefulSet", lightrunJavaAgent.Spec.WorkloadName)
329371
// remove our finalizer from the list and update it.
330372
err = r.removeFinalizer(ctx, lightrunJavaAgent, finalizerName)
331373
if err != nil {
@@ -346,13 +388,13 @@ func (r *LightrunJavaAgentReconciler) reconcileStatefulSet(ctx context.Context,
346388

347389
// Restore original StatefulSet (unpatch)
348390
// Volume and init container
349-
log.Info("Unpatching StatefulSet", "StatefulSet", lightrunJavaAgent.Spec.StatefulSetName)
391+
log.Info("Unpatching StatefulSet", "StatefulSet", lightrunJavaAgent.Spec.WorkloadName)
350392

351393
originalStatefulSet = &appsv1.StatefulSet{}
352394
err = r.Get(ctx, stsNamespacedObj, originalStatefulSet)
353395
if err != nil {
354396
if client.IgnoreNotFound(err) == nil {
355-
log.Info("StatefulSet not found", "StatefulSet", lightrunJavaAgent.Spec.StatefulSetName)
397+
log.Info("StatefulSet not found", "StatefulSet", lightrunJavaAgent.Spec.WorkloadName)
356398
// remove our finalizer from the list and update it.
357399
log.Info("Removing finalizer")
358400
err = r.removeFinalizer(ctx, lightrunJavaAgent, finalizerName)
@@ -362,7 +404,7 @@ func (r *LightrunJavaAgentReconciler) reconcileStatefulSet(ctx context.Context,
362404
// Successfully removed finalizer and nothing to restore
363405
return r.successStatus(ctx, lightrunJavaAgent, reconcileTypeReady)
364406
}
365-
log.Error(err, "unable to unpatch statefulset", "StatefulSet", lightrunJavaAgent.Spec.StatefulSetName)
407+
log.Error(err, "unable to unpatch statefulset", "StatefulSet", lightrunJavaAgent.Spec.WorkloadName)
366408
return r.errorStatus(ctx, lightrunJavaAgent, err)
367409
}
368410

@@ -410,7 +452,7 @@ func (r *LightrunJavaAgentReconciler) reconcileStatefulSet(ctx context.Context,
410452
return r.errorStatus(ctx, lightrunJavaAgent, err)
411453
}
412454

413-
log.Info("StatefulSet returned to original state", "StatefulSet", lightrunJavaAgent.Spec.StatefulSetName)
455+
log.Info("StatefulSet returned to original state", "StatefulSet", lightrunJavaAgent.Spec.WorkloadName)
414456
return r.successStatus(ctx, lightrunJavaAgent, reconcileTypeProgressing)
415457
}
416458
// Nothing to do here
@@ -478,7 +520,7 @@ func (r *LightrunJavaAgentReconciler) reconcileStatefulSet(ctx context.Context,
478520
}
479521

480522
// Server side apply for StatefulSet changes
481-
log.V(2).Info("Patching StatefulSet", "StatefulSet", lightrunJavaAgent.Spec.StatefulSetName, "LightunrJavaAgent", lightrunJavaAgent.Name)
523+
log.V(2).Info("Patching StatefulSet", "StatefulSet", lightrunJavaAgent.Spec.WorkloadName, "LightunrJavaAgent", lightrunJavaAgent.Name)
482524
err = r.patchStatefulSet(lightrunJavaAgent, secret, originalStatefulSet, statefulSetApplyConfig, cmDataHash)
483525
if err != nil {
484526
log.Error(err, "failed to patch statefulset")
@@ -503,12 +545,12 @@ func (r *LightrunJavaAgentReconciler) reconcileStatefulSet(ctx context.Context,
503545
}
504546

505547
// Client side patch (we can't rollback JAVA_TOOL_OPTIONS env with server side apply)
506-
log.V(2).Info("Patching Java Env", "StatefulSet", lightrunJavaAgent.Spec.StatefulSetName, "LightunrJavaAgent", lightrunJavaAgent.Name)
548+
log.V(2).Info("Patching Java Env", "StatefulSet", lightrunJavaAgent.Spec.WorkloadName, "LightunrJavaAgent", lightrunJavaAgent.Name)
507549
originalStatefulSet = &appsv1.StatefulSet{}
508550
err = r.Get(ctx, stsNamespacedObj, originalStatefulSet)
509551
if err != nil {
510552
if client.IgnoreNotFound(err) == nil {
511-
log.Info("StatefulSet not found", "StatefulSet", lightrunJavaAgent.Spec.StatefulSetName)
553+
log.Info("StatefulSet not found", "StatefulSet", lightrunJavaAgent.Spec.WorkloadName)
512554
err = r.removeFinalizer(ctx, lightrunJavaAgent, finalizerName)
513555
if err != nil {
514556
return r.errorStatus(ctx, lightrunJavaAgent, err)
@@ -538,51 +580,57 @@ func (r *LightrunJavaAgentReconciler) reconcileStatefulSet(ctx context.Context,
538580
}
539581

540582
// Update status to Healthy
541-
log.V(1).Info("Reconciling finished successfully", "StatefulSet", lightrunJavaAgent.Spec.StatefulSetName, "LightunrJavaAgent", lightrunJavaAgent.Name)
583+
log.V(1).Info("Reconciling finished successfully", "StatefulSet", lightrunJavaAgent.Spec.WorkloadName, "LightunrJavaAgent", lightrunJavaAgent.Name)
542584
return r.successStatus(ctx, lightrunJavaAgent, reconcileTypeReady)
543585
}
544586

545-
// SetupWithManager sets up the controller with the Manager.
587+
// SetupWithManager configures the controller with the Manager and sets up watches and indexers.
588+
// It creates several field indexers to enable efficient lookups of LightrunJavaAgent CRs based on:
589+
// - DeploymentName (legacy field)
590+
// - WorkloadName (newer field that replaces DeploymentName)
591+
// - SecretName
592+
//
593+
// It also sets up watches for Deployments, StatefulSets, and Secrets so the controller can
594+
// react to changes in these resources that are referenced by LightrunJavaAgent CRs.
546595
func (r *LightrunJavaAgentReconciler) SetupWithManager(mgr ctrl.Manager) error {
547-
// Add spec.container_selector.deployment field to cache for future filtering
596+
// Index field for deployments - allows looking up LightrunJavaAgents by deploymentName
597+
// This is used for legacy support where DeploymentName was used instead of WorkloadName
598+
// TODO: remove this once we deprecate deploymentNameIndexField
548599
err = mgr.GetFieldIndexer().IndexField(
549600
context.Background(),
550601
&agentv1beta.LightrunJavaAgent{},
551602
deploymentNameIndexField,
552603
func(object client.Object) []string {
553-
lightrunJavaAgent := object.(*agentv1beta.LightrunJavaAgent)
554-
555-
if lightrunJavaAgent.Spec.DeploymentName == "" {
604+
agent := object.(*agentv1beta.LightrunJavaAgent)
605+
if agent.Spec.DeploymentName == "" {
556606
return nil
557607
}
558-
559-
return []string{lightrunJavaAgent.Spec.DeploymentName}
608+
r.Log.Info("Indexing DeploymentName", "DeploymentName", agent.Spec.DeploymentName)
609+
return []string{agent.Spec.DeploymentName}
560610
})
561-
562611
if err != nil {
563612
return err
564613
}
565614

566-
// Add spec.container_selector.statefulset field to cache for future filtering
615+
// Index field for workloads by name - allows looking up LightrunJavaAgents by WorkloadName
567616
err = mgr.GetFieldIndexer().IndexField(
568617
context.Background(),
569618
&agentv1beta.LightrunJavaAgent{},
570-
statefulSetNameIndexField,
619+
workloadNameIndexField,
571620
func(object client.Object) []string {
572-
lightrunJavaAgent := object.(*agentv1beta.LightrunJavaAgent)
573-
574-
if lightrunJavaAgent.Spec.StatefulSetName == "" {
621+
agent := object.(*agentv1beta.LightrunJavaAgent)
622+
if agent.Spec.WorkloadName == "" {
575623
return nil
576624
}
577-
578-
return []string{lightrunJavaAgent.Spec.StatefulSetName}
625+
r.Log.Info("Indexing WorkloadName", "WorkloadName", agent.Spec.WorkloadName)
626+
return []string{agent.Spec.WorkloadName}
579627
})
580-
581628
if err != nil {
582629
return err
583630
}
584631

585-
// Add spec.secret field to cache for future filtering
632+
// Index field for secrets - allows looking up LightrunJavaAgents by SecretName
633+
// This enables the controller to find LightrunJavaAgents affected by Secret changes
586634
err = mgr.GetFieldIndexer().IndexField(
587635
context.Background(),
588636
&agentv1beta.LightrunJavaAgent{},
@@ -601,6 +649,12 @@ func (r *LightrunJavaAgentReconciler) SetupWithManager(mgr ctrl.Manager) error {
601649
return err
602650
}
603651

652+
// Configure the controller builder:
653+
// - For: register LightrunJavaAgent as the primary resource this controller reconciles
654+
// - Watches: set up event handlers to watch for changes in related resources:
655+
// * Deployments: reconcile LightrunJavaAgents when their target Deployment changes
656+
// * StatefulSets: reconcile LightrunJavaAgents when their target StatefulSet changes
657+
// * Secrets: reconcile LightrunJavaAgents when their referenced Secret changes
604658
return ctrl.NewControllerManagedBy(mgr).
605659
For(&agentv1beta.LightrunJavaAgent{}).
606660
Watches(

0 commit comments

Comments
 (0)