Skip to content

Commit af01050

Browse files
BobVanBBobVanB
committed
fix: use error handeling on IsUnmanagedOrFiltered
1 parent b210787 commit af01050

15 files changed

Lines changed: 119 additions & 21 deletions

File tree

‎pkg/controller/agent/controller.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,11 @@ func (r *ReconcileAgent) Reconcile(ctx context.Context, request reconcile.Reques
154154
return reconcile.Result{}, tracing.CaptureError(ctx, err)
155155
}
156156

157-
if common.IsUnmanagedOrFiltered(ctx, r.Client, agent, r.Parameters) {
157+
unmanagedOrFiltered, err := common.IsUnmanagedOrFiltered(ctx, r.Client, agent, r.Parameters)
158+
if err != nil {
159+
return reconcile.Result{}, tracing.CaptureError(ctx, err)
160+
}
161+
if unmanagedOrFiltered {
158162
logconf.FromContext(ctx).Info("Object is currently not managed by this controller or namespace is filtered. Skipping reconciliation")
159163
return reconcile.Result{}, nil
160164
}

‎pkg/controller/apmserver/controller.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,11 @@ func (r *ReconcileApmServer) Reconcile(ctx context.Context, request reconcile.Re
196196
return reconcile.Result{}, tracing.CaptureError(ctx, err)
197197
}
198198

199-
if common.IsUnmanagedOrFiltered(ctx, r.Client, &as, r.Parameters) {
199+
unmanagedOrFiltered, err := common.IsUnmanagedOrFiltered(ctx, r.Client, &as, r.Parameters)
200+
if err != nil {
201+
return reconcile.Result{}, tracing.CaptureError(ctx, err)
202+
}
203+
if unmanagedOrFiltered {
200204
log.Info("Object currently not managed by this controller or namespace is filtered. Skipping reconciliation", "namespace", as.Namespace, "as_name", as.Name)
201205
return reconcile.Result{}, nil
202206
}

‎pkg/controller/association/reconciler.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,11 @@ func (r *Reconciler) Reconcile(ctx context.Context, request reconcile.Request) (
166166

167167
associatedKey := k8s.ExtractNamespacedName(associated)
168168

169-
if common.IsUnmanagedOrFiltered(ctx, r.Client, associated, r.Parameters) {
169+
unmanagedOrFiltered, err := common.IsUnmanagedOrFiltered(ctx, r.Client, associated, r.Parameters)
170+
if err != nil {
171+
return reconcile.Result{}, tracing.CaptureError(ctx, err)
172+
}
173+
if unmanagedOrFiltered {
170174
log.Info("Object is currently not managed by this controller or namespace is filtered. Skipping reconciliation")
171175
return reconcile.Result{}, nil
172176
}

‎pkg/controller/autoscaling/elasticsearch/controller.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,11 @@ func (r *ReconcileElasticsearchAutoscaler) Reconcile(ctx context.Context, reques
127127
return reconcile.Result{}, tracing.CaptureError(ctx, err)
128128
}
129129

130-
if common.IsUnmanagedOrFiltered(ctx, r.Client, &esa, r.Parameters) {
130+
unmanagedOrFiltered, err := common.IsUnmanagedOrFiltered(ctx, r.Client, &esa, r.Parameters)
131+
if err != nil {
132+
return reconcile.Result{}, tracing.CaptureError(ctx, err)
133+
}
134+
if unmanagedOrFiltered {
131135
msg := "Object is currently not managed by this controller or namespace is filtered. Skipping reconciliation"
132136
log.Info(msg, "namespace", request.Namespace, "esa_name", request.Name)
133137
return r.reportAsInactive(ctx, log, esa, msg)

‎pkg/controller/beat/controller.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,11 @@ func (r *ReconcileBeat) Reconcile(ctx context.Context, request reconcile.Request
141141
return reconcile.Result{}, tracing.CaptureError(ctx, err)
142142
}
143143

144-
if common.IsUnmanagedOrFiltered(ctx, r.Client, &beat, r.Parameters) {
144+
unmanagedOrFiltered, err := common.IsUnmanagedOrFiltered(ctx, r.Client, &beat, r.Parameters)
145+
if err != nil {
146+
return reconcile.Result{}, tracing.CaptureError(ctx, err)
147+
}
148+
if unmanagedOrFiltered {
145149
ulog.FromContext(ctx).Info("Object is currently not managed by this controller or namespace is filtered. Skipping reconciliation", "namespace", beat.Namespace, "beat_name", beat.Name)
146150
return reconcile.Result{}, nil
147151
}

‎pkg/controller/common/unmanaged.go‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -37,26 +37,24 @@ func IsUnmanaged(ctx context.Context, object metav1.Object) bool {
3737

3838
// IsUnmanagedOrFiltered checks if a given resource is currently unmanaged or if its namespace
3939
// should not be managed based on the operator's namespace label selector configuration.
40-
func IsUnmanagedOrFiltered(ctx context.Context, c client.Client, object metav1.Object, params operator.Parameters) bool {
40+
func IsUnmanagedOrFiltered(ctx context.Context, c client.Client, object metav1.Object, params operator.Parameters) (bool, error) {
4141
log := ulog.FromContext(ctx)
4242

4343
// First check if the resource is explicitly unmanaged
4444
if IsUnmanaged(ctx, object) {
45-
return true
45+
return true, nil
4646
}
4747

4848
// Then check namespace filtering
4949
shouldManage, err := params.ShouldManageNamespace(ctx, c, object.GetNamespace())
5050
if err != nil {
51-
log.Error(err, "Failed to check namespace management status", "namespace", object.GetNamespace(), "name", object.GetName())
52-
// In case of error, default to not managing to be safe
53-
return true
51+
return false, err
5452
}
5553

5654
if !shouldManage {
5755
log.V(1).Info("Namespace is excluded by namespace label selector", "namespace", object.GetNamespace(), "name", object.GetName())
58-
return true
56+
return true, nil
5957
}
6058

61-
return false
59+
return false, nil
6260
}

‎pkg/controller/common/unmanaged_test.go‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ import (
1111
"github.com/stretchr/testify/assert"
1212
corev1 "k8s.io/api/core/v1"
1313
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
14+
15+
"github.com/elastic/cloud-on-k8s/v3/pkg/controller/common/operator"
16+
"github.com/elastic/cloud-on-k8s/v3/pkg/utils/k8s"
1417
)
1518

1619
type testcase struct {
@@ -90,3 +93,48 @@ func TestUnmanagedCondition(t *testing.T) {
9093
})
9194
}
9295
}
96+
97+
func TestIsUnmanagedOrFiltered(t *testing.T) {
98+
testNamespace := &corev1.Namespace{ObjectMeta: v1.ObjectMeta{Name: "test-ns", Labels: map[string]string{"env": "prod"}}}
99+
devNamespace := &corev1.Namespace{ObjectMeta: v1.ObjectMeta{Name: "dev-ns", Labels: map[string]string{"env": "dev"}}}
100+
fakeClient := k8s.NewFakeClient(testNamespace, devNamespace)
101+
102+
t.Run("returns unmanaged when annotation is set", func(t *testing.T) {
103+
obj := &corev1.Secret{ObjectMeta: v1.ObjectMeta{Name: "s1", Namespace: "test-ns", Annotations: map[string]string{ManagedAnnotation: "false"}}}
104+
105+
unmanagedOrFiltered, err := IsUnmanagedOrFiltered(context.Background(), fakeClient, obj, operator.Parameters{})
106+
107+
assert.NoError(t, err)
108+
assert.True(t, unmanagedOrFiltered)
109+
})
110+
111+
t.Run("returns filtered when namespace does not match selector", func(t *testing.T) {
112+
obj := &corev1.Secret{ObjectMeta: v1.ObjectMeta{Name: "s2", Namespace: "dev-ns"}}
113+
params := operator.Parameters{NamespaceLabelSelector: &v1.LabelSelector{MatchLabels: map[string]string{"env": "prod"}}}
114+
115+
unmanagedOrFiltered, err := IsUnmanagedOrFiltered(context.Background(), fakeClient, obj, params)
116+
117+
assert.NoError(t, err)
118+
assert.True(t, unmanagedOrFiltered)
119+
})
120+
121+
t.Run("returns managed when namespace matches selector", func(t *testing.T) {
122+
obj := &corev1.Secret{ObjectMeta: v1.ObjectMeta{Name: "s3", Namespace: "test-ns"}}
123+
params := operator.Parameters{NamespaceLabelSelector: &v1.LabelSelector{MatchLabels: map[string]string{"env": "prod"}}}
124+
125+
unmanagedOrFiltered, err := IsUnmanagedOrFiltered(context.Background(), fakeClient, obj, params)
126+
127+
assert.NoError(t, err)
128+
assert.False(t, unmanagedOrFiltered)
129+
})
130+
131+
t.Run("returns error when namespace lookup fails", func(t *testing.T) {
132+
obj := &corev1.Secret{ObjectMeta: v1.ObjectMeta{Name: "s4", Namespace: "missing-ns"}}
133+
params := operator.Parameters{NamespaceLabelSelector: &v1.LabelSelector{MatchLabels: map[string]string{"env": "prod"}}}
134+
135+
unmanagedOrFiltered, err := IsUnmanagedOrFiltered(context.Background(), fakeClient, obj, params)
136+
137+
assert.Error(t, err)
138+
assert.False(t, unmanagedOrFiltered)
139+
})
140+
}

‎pkg/controller/elasticsearch/elasticsearch_controller.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,11 @@ func (r *ReconcileElasticsearch) Reconcile(ctx context.Context, request reconcil
173173
return reconcile.Result{}, tracing.CaptureError(ctx, err)
174174
}
175175

176-
if common.IsUnmanagedOrFiltered(ctx, r.Client, &es, r.Parameters) {
176+
unmanagedOrFiltered, err := common.IsUnmanagedOrFiltered(ctx, r.Client, &es, r.Parameters)
177+
if err != nil {
178+
return reconcile.Result{}, tracing.CaptureError(ctx, err)
179+
}
180+
if unmanagedOrFiltered {
177181
log.Info("Object is currently not managed by this controller or namespace is filtered. Skipping reconciliation", "namespace", es.Namespace, "es_name", es.Name)
178182
return reconcile.Result{}, nil
179183
}

‎pkg/controller/enterprisesearch/enterprisesearch_controller.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,11 @@ func (r *ReconcileEnterpriseSearch) Reconcile(ctx context.Context, request recon
157157
return reconcile.Result{}, tracing.CaptureError(ctx, err)
158158
}
159159

160-
if common.IsUnmanagedOrFiltered(ctx, r.Client, &ent, r.Parameters) {
160+
unmanagedOrFiltered, err := common.IsUnmanagedOrFiltered(ctx, r.Client, &ent, r.Parameters)
161+
if err != nil {
162+
return reconcile.Result{}, tracing.CaptureError(ctx, err)
163+
}
164+
if unmanagedOrFiltered {
161165
ulog.FromContext(ctx).Info("Object is currently not managed by this controller or namespace is filtered. Skipping reconciliation", "namespace", ent.Namespace, "ent_name", ent.Name)
162166
return reconcile.Result{}, nil
163167
}

‎pkg/controller/kibana/controller.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,11 @@ func (r *ReconcileKibana) Reconcile(ctx context.Context, request reconcile.Reque
149149
return reconcile.Result{}, tracing.CaptureError(ctx, err)
150150
}
151151

152-
if common.IsUnmanagedOrFiltered(ctx, r.Client, &kb, r.params) {
152+
unmanagedOrFiltered, err := common.IsUnmanagedOrFiltered(ctx, r.Client, &kb, r.params)
153+
if err != nil {
154+
return reconcile.Result{}, tracing.CaptureError(ctx, err)
155+
}
156+
if unmanagedOrFiltered {
153157
ulog.FromContext(ctx).Info("Object is currently not managed by this controller or namespace is filtered. Skipping reconciliation", "namespace", kb.Namespace, "kibana_name", kb.Name)
154158
return reconcile.Result{}, nil
155159
}

0 commit comments

Comments
 (0)