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

Controllers scope: cluster wide or namespaced? #51

Open
andreacv98 opened this issue May 2, 2024 · 1 comment
Open

Controllers scope: cluster wide or namespaced? #51

andreacv98 opened this issue May 2, 2024 · 1 comment
Assignees
Labels
bug Something isn't working question Further information is requested

Comments

@andreacv98
Copy link
Contributor

Hello everyone,

With this issue, I want to raise a question: should the operators watch for CR created in every namespace?

This doubt comes from a bug: when you create a solver in the default namespace for example, instead of the fluidos namespace, the process is interrupted after the successful discovery creation and completion.

Looking inside the log of the rear-manager you can see the following:

I0502 09:29:43.443703       1 solver_controller.go:104] Reconciling Solver default/solver-sample-demo
I0502 09:29:43.451467       1 solver_controller.go:104] Reconciling Solver default/solver-sample-demo
I0502 09:29:43.552659       1 solver_controller.go:482] No PeeringCandidates found
I0502 09:29:43.552731       1 solver_controller.go:206] Solver solver-sample-demo has not found any candidate. Trying a Discovery
I0502 09:29:43.562508       1 solver_controller.go:104] Reconciling Solver default/solver-sample-demo
I0502 09:29:43.562554       1 solver_controller.go:230] Getting or creating Discovery for Solver solver-sample-demo
I0502 09:29:43.574271       1 solver_controller.go:92] Solver fluidos/solver-sample-demo not found, probably deleted
I0502 09:29:43.712979       1 solver_controller.go:92] Solver fluidos/solver-sample-demo not found, probably deleted

As you can see the solver-controller first correctly reconcile the solver created in the default namespace, but after the discovery creation, the subsequent reconciliation is performed over the original solver, but in the default fluidos namespace.

Moreover, looking inside the helm chart, the RBAC for the rear-manager are cluster wide (ClusterRole, ClusterRoleBinding, etc.).

In conclusion, I can see both an intention of watching for solver resources cluster wide and in the specific namespace, but I don't understand which one has been chosen. In my opinion, the controller can be forced to watch for only the solver resources inside the fluidos namespace, granting a better control and more specific RBAC.

Feel free to give me any feedback about this point

Thanks,
Andrea

@andreacv98 andreacv98 added bug Something isn't working question Further information is requested labels May 2, 2024
@andreacv98
Copy link
Contributor Author

I found that the specific behavior that I found in the log (first reconcile over a solver in the right namespace, than it is changed in the second iteration) can be caused by the mapping over the watches set for the reconcile of the rear-manager.

Starting from this point:

func (r *SolverReconciler) SetupWithManager(mgr ctrl.Manager) error {
return ctrl.NewControllerManagedBy(mgr).
For(&nodecorev1alpha1.Solver{}).
Watches(&advertisementv1alpha1.Discovery{}, handler.EnqueueRequestsFromMapFunc(
r.discoveryToSolver,
), builder.WithPredicates(discoveryPredicate())).
Watches(&reservationv1alpha1.Reservation{}, handler.EnqueueRequestsFromMapFunc(
r.reservationToSolver,
), builder.WithPredicates(reservationPredicate())).
Watches(&nodecorev1alpha1.Allocation{}, handler.EnqueueRequestsFromMapFunc(
r.allocationToSolver,
), builder.WithPredicates(allocationPredicate())).
Complete(r)
}

The code refers (concerning the discovery issue, but I think it can be extended to other resources watched by the reconciler):

func (r *SolverReconciler) discoveryToSolver(_ context.Context, o client.Object) []reconcile.Request {
solverName := namings.RetrieveSolverNameFromDiscovery(o.GetName())
return []reconcile.Request{
{
NamespacedName: types.NamespacedName{
Name: solverName,
Namespace: flags.FluidoNamespace,
},
},
}
}

Here, the namespace of the mapping is hard coded to the one of fluidos.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants