Skip to content

Commit 93ebe3a

Browse files
csatib02csatib02
committed
feat: tenant owned resources problem enhancements
Signed-off-by: Bence Csati <[email protected]>
1 parent 26ec927 commit 93ebe3a

File tree

3 files changed

+49
-40
lines changed

3 files changed

+49
-40
lines changed

‎controllers/telemetry/route_controller.go‎

Lines changed: 8 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import (
1919
"fmt"
2020
"reflect"
2121
"slices"
22-
"sort"
2322

2423
"emperror.dev/errors"
2524
apiv1 "k8s.io/api/core/v1"
@@ -98,7 +97,7 @@ func (r *RouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl
9897
}
9998
for _, step := range steps {
10099
if err := step.fn(); err != nil {
101-
return r.handleReconcileError(ctx, baseManager, tenant, step.name, err)
100+
return r.handleTenantReconcileError(ctx, &baseManager, tenant, step.name, err)
102101
}
103102
}
104103

@@ -217,8 +216,8 @@ func (r *RouteReconciler) SetupWithManager(mgr ctrl.Manager) error {
217216
return builder.Complete(r)
218217
}
219218

220-
// handleReconcileError handles errors that occur during reconciliation steps
221-
func (r *RouteReconciler) handleReconcileError(ctx context.Context, baseManager manager.BaseManager, tenant *v1alpha1.Tenant, stepName string, err error) (ctrl.Result, error) {
219+
// handleTenantReconcileError handles errors that occur during reconciliation steps
220+
func (r *RouteReconciler) handleTenantReconcileError(ctx context.Context, baseManager *manager.BaseManager, tenant *v1alpha1.Tenant, stepName string, err error) (ctrl.Result, error) {
222221
wrappedErr := errors.Wrapf(err, "failed to %s for tenant %s", stepName, tenant.Name)
223222

224223
tenant.Status.Problems = append(tenant.Status.Problems, wrappedErr.Error())
@@ -255,11 +254,7 @@ func handleOwnedResources(ctx context.Context, tenantResManager *manager.TenantR
255254
resourcesForTenant, resourceUpdateList, err := tenantResManager.GetResourceOwnedByTenant(ctx, resource, tenant)
256255
if err != nil {
257256
tenantResManager.Error(errors.WithStack(err), fmt.Sprintf("failed to get %T for tenant", resource), "tenant", tenant.Name)
258-
if updateErr := tenantResManager.Status().Update(ctx, tenant); updateErr != nil {
259-
tenantResManager.Error(errors.WithStack(updateErr), "failed updating tenant status", "tenant", tenant.Name)
260-
return errors.Append(err, updateErr)
261-
}
262-
return err
257+
return fmt.Errorf("failed to get %T for tenant %s: %w", resource, tenant.Name, err)
263258
}
264259

265260
// Add all newly updated resources here
@@ -294,15 +289,12 @@ func validateSubscriptionOutputs(ctx context.Context, tenantResManager *manager.
294289

295290
for _, subscription := range realSubscriptionsForTenant {
296291
originalSubscriptionStatus := subscription.Status.DeepCopy()
297-
validOutputs, invalidOutputs := tenantResManager.ValidateSubscriptionOutputs(ctx, subscription)
298-
292+
validOutputs, invalidOutputs := tenantResManager.ValidateSubscriptionReferencedOutputs(ctx, subscription)
299293
if len(invalidOutputs) > 0 {
300-
subscription.Status.Problems = append(subscription.Status.Problems, fmt.Sprintf("invalid outputs for subscription %s: %v", subscription.Name, invalidOutputs))
294+
subscription.Status.Problems = append(subscription.Status.Problems, fmt.Sprintf("invalid outputs referenced by subscription %s: %v", subscription.Name, invalidOutputs))
301295
subscription.Status.ProblemsCount = len(subscription.Status.Problems)
302296
subscription.Status.State = state.StateFailed
303-
tenantResManager.UpdateOutputs(ctx, tenant, invalidOutputs)
304297
}
305-
306298
components.SortNamespacedNames(validOutputs)
307299
subscription.Status.Outputs = validOutputs
308300

@@ -320,18 +312,12 @@ func validateSubscriptionOutputs(ctx context.Context, tenantResManager *manager.
320312
func handleBridgeResources(ctx context.Context, bridgeManager *manager.BridgeManager, tenant *v1alpha1.Tenant) error {
321313
bridgesForTenant, err := bridgeManager.GetBridgesForTenant(ctx, tenant.Name)
322314
if err != nil {
323-
tenant.Status.State = state.StateFailed
324315
bridgeManager.Error(errors.WithStack(err), "failed to get bridges for tenant", "tenant", tenant.Name)
325-
if updateErr := bridgeManager.Status().Update(ctx, tenant); updateErr != nil {
326-
bridgeManager.Error(errors.WithStack(updateErr), "failed updating tenant status", "tenant", tenant.Name)
327-
return errors.Append(err, updateErr)
328-
}
329-
330-
return err
316+
return fmt.Errorf("failed to get bridges for tenant %s: %w", tenant.Name, err)
331317
}
332318

333319
bridgesForTenantNames := manager.GetBridgeNamesFromBridges(bridgesForTenant)
334-
sort.Strings(bridgesForTenantNames)
320+
slices.Sort(bridgesForTenantNames)
335321
tenant.Status.ConnectedBridges = bridgesForTenantNames
336322

337323
for _, bridge := range bridgesForTenant {

‎pkg/resources/manager/bridge_manager.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ func (b *BridgeManager) getTenants(ctx context.Context, listOpts *client.ListOpt
7575
return tenants.Items, nil
7676
}
7777

78-
func (b *BridgeManager) CheckBridgeConnection(ctx context.Context, tenantName string, bridge *v1alpha1.Bridge) error {
78+
func (b *BridgeManager) ValidateBridgeConnection(ctx context.Context, tenantName string, bridge *v1alpha1.Bridge) error {
7979
for _, tenantReference := range []string{bridge.Spec.SourceTenant, bridge.Spec.TargetTenant} {
8080
if tenantReference != tenantName {
8181
listOpts := &client.ListOptions{

‎pkg/resources/manager/tenant_resource_manager.go‎

Lines changed: 40 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -99,12 +99,21 @@ func (t *TenantResourceManager) GetResourceOwnedByTenant(ctx context.Context, re
9999
currentTenant := res.GetTenant()
100100
if currentTenant != "" && currentTenant != tenant.Name {
101101
t.Error(
102-
fmt.Errorf("resource is owned by another tenant"),
102+
fmt.Errorf("resource: %s of kind: %s is owned by another tenant", res.GetName(), res.GetObjectKind().GroupVersionKind().Kind),
103103
"skipping reconciliation",
104104
"current_tenant", currentTenant,
105105
"desired_tenant", tenant.Name,
106106
"action_required", "remove resource from previous tenant before adopting to new tenant",
107107
)
108+
109+
res.SetProblems([]string{
110+
fmt.Sprintf("resource %s/%s is already owned by tenant %s", res.GetNamespace(), res.GetName(), currentTenant),
111+
})
112+
res.SetProblemsCount(len(res.GetProblems()))
113+
res.SetState(state.StateFailed)
114+
if err := t.Status().Update(ctx, res); err != nil {
115+
t.Error(err, fmt.Sprintf("failed to update resource (%s/%s) state", res.GetNamespace(), res.GetName()))
116+
}
108117
continue
109118
}
110119

@@ -188,42 +197,46 @@ func (t *TenantResourceManager) DisownResources(ctx context.Context, resourceToD
188197
}
189198
}
190199

191-
// ValidateSubscriptionOutputs validates the output references of a subscription
192-
func (t *TenantResourceManager) ValidateSubscriptionOutputs(ctx context.Context, subscription *v1alpha1.Subscription) []v1alpha1.NamespacedName {
200+
// ValidateSubscriptionReferencedOutputs validates the output references of a subscription
201+
func (t *TenantResourceManager) ValidateSubscriptionReferencedOutputs(ctx context.Context, subscription *v1alpha1.Subscription) ([]v1alpha1.NamespacedName, []v1alpha1.Output) {
193202
validOutputs := []v1alpha1.NamespacedName{}
194-
invalidOutputs := []v1alpha1.NamespacedName{}
203+
invalidOutputs := []v1alpha1.Output{}
195204
for _, outputRef := range subscription.Spec.Outputs {
205+
invalid := false
196206
checkedOutput := &v1alpha1.Output{}
197207
if err := t.Get(ctx, types.NamespacedName(outputRef), checkedOutput); err != nil {
198208
t.Error(err, "referred output invalid", "output", outputRef.String())
199-
200-
invalidOutputs = append(invalidOutputs, outputRef)
209+
checkedOutput.Status.Problems = append(checkedOutput.Status.Problems, fmt.Sprintf("referred output %s could not be queried", outputRef.String()))
210+
t.updateInvalidOutput(ctx, checkedOutput)
201211
continue
202212
}
203213

204214
// ensure the output belongs to the same tenant
205215
if checkedOutput.Status.Tenant != subscription.Status.Tenant {
216+
invalid = true
206217
t.Error(errors.New("output and subscription tenants mismatch"),
207218
"output and subscription tenants mismatch",
208219
"output", checkedOutput.NamespacedName().String(),
209220
"output's tenant", checkedOutput.Status.Tenant,
210221
"subscription", subscription.NamespacedName().String(),
211222
"subscription's tenant", subscription.Status.Tenant)
212-
213-
invalidOutputs = append(invalidOutputs, outputRef)
214-
continue
223+
checkedOutput.Status.Problems = append(checkedOutput.Status.Problems, fmt.Sprintf("output %s does not belong to the same tenant as subscription %s", outputRef.String(), subscription.NamespacedName().String()))
215224
}
216225

217226
// validate output secret
218227
if checkedOutput.Spec.Authentication != nil {
219228
if err := components.QueryOutputSecret(ctx, t.Client, checkedOutput); err != nil {
229+
invalid = true
220230
t.Error(err, "failed to query output secret", "output", checkedOutput.NamespacedName().String())
221-
222-
invalidOutputs = append(invalidOutputs, outputRef)
223-
continue
231+
checkedOutput.Status.Problems = append(checkedOutput.Status.Problems, fmt.Sprintf("output %s secret could not be queried: %v", outputRef.String(), err))
224232
}
225233
}
226234

235+
if invalid {
236+
t.updateInvalidOutput(ctx, checkedOutput)
237+
invalidOutputs = append(invalidOutputs, *checkedOutput)
238+
}
239+
227240
// update the output state if validation was successful
228241
checkedOutput.SetState(state.StateReady)
229242
if updateErr := t.Status().Update(ctx, checkedOutput); updateErr != nil {
@@ -234,11 +247,7 @@ func (t *TenantResourceManager) ValidateSubscriptionOutputs(ctx context.Context,
234247
validOutputs = append(validOutputs, outputRef)
235248
}
236249

237-
if len(invalidOutputs) > 0 {
238-
t.Error(errors.New("some outputs are invalid"), "some outputs are invalid", "invalidOutputs", invalidOutputs, "subscription", subscription.NamespacedName().String())
239-
}
240-
241-
return validOutputs
250+
return validOutputs, invalidOutputs
242251
}
243252

244253
// getNamespacesForSelectorSlice returns a list of namespaces that match the given label selectors
@@ -266,6 +275,20 @@ func (t *TenantResourceManager) getNamespacesForSelectorSlice(ctx context.Contex
266275
return namespaces, nil
267276
}
268277

278+
func (t *TenantResourceManager) updateInvalidOutput(ctx context.Context, output *v1alpha1.Output) error {
279+
if output == nil {
280+
return fmt.Errorf("output is nil")
281+
}
282+
283+
output.Status.State = state.StateFailed
284+
output.Status.ProblemsCount = len(output.Status.Problems)
285+
if err := t.Status().Update(ctx, output); err != nil {
286+
return fmt.Errorf("failed to update output %s/%s: %w", output.GetNamespace(), output.GetName(), err)
287+
}
288+
289+
return nil
290+
}
291+
269292
func GetResourceNamesFromResource(resources []model.ResourceOwnedByTenant) []v1alpha1.NamespacedName {
270293
resourceNames := make([]v1alpha1.NamespacedName, len(resources))
271294
for i, resource := range resources {

0 commit comments

Comments
 (0)