Skip to content

Conversation

Diaphteiros
Copy link
Contributor

@Diaphteiros Diaphteiros commented Oct 2, 2025

What this PR does / why we need it:
We have many controllers that reconcile something and then need access to a Cluster related to that something. I therefore liked the idea of the clusteraccess library, but it was focused on the MCP use-case and too restricted to be used outside of that, e.g. for the dns platform service (which reconciles Cluster resources that don't have to belong to an MCP).

Therefore, I implemented a more flexible version of the library and modified the original implementation to just be a facade in front of the 'advanced' clusteraccess library with nearly (see below) no changes to its behavior.

Which issue(s) this PR fixes:
Fixes #143

Special notes for your reviewer:
The behavior of the 'simple' clusteraccess library has changed in one noticeable point: Before, it waited for the namespace to be created by requeuing until it existed. With the new implementation, it instead creates the namespace itself and does not requeue.
If this can be a problem somewhere, I can also adapt the implementation to keep the previous behavior.

Release note:

The `lib/clusteraccess/advanced` package now contains a highly flexible library for generating access to clusters during a controller's reconciliation loop. See the [documentation](docs/libraries/clusteraccess.md) for further information.
The behavior of the library in `lib/clusteraccess` has changed slightly: Before, the `Reconcile` method would wait for some other controller to create the namespace and requeue the reconciliation until it existed. Now, it will instead create the namespace itself.

Copy link
Contributor

@reshnm reshnm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR screams for some form of documentation of how to use it :-)
Expecially, what do I need to do to setup the requests for new and existing clusters.
How would the id, suffix and the callback function need to look like.
Maybe add an example for MCP, Workload and Onboarding clusters.

I can see some hints in the implementation of the already existing clusteraccess library. But for me it looks like some black magic is happening with al list of very precise steps that need to be done to set this up correctly. Just from the interface description I wouldn't be able to reproduce this.

@Diaphteiros
Copy link
Contributor Author

Diaphteiros commented Oct 2, 2025

This PR screams for some form of documentation of how to use it :-)

Did you see the documentation here?

@reshnm
Copy link
Contributor

reshnm commented Oct 2, 2025

This PR screams for some form of documentation of how to use it :-)

Did you see the documentation here?

I did not ...

@Diaphteiros Diaphteiros force-pushed the improve-clusteraccess branch from 62e3779 to 6e586b7 Compare October 10, 2025 13:08
@Diaphteiros Diaphteiros requested a review from reshnm October 10, 2025 13:08
reshnm
reshnm previously approved these changes Oct 14, 2025
@Diaphteiros Diaphteiros merged commit a6b9f94 into main Oct 14, 2025
4 checks passed
@Diaphteiros Diaphteiros deleted the improve-clusteraccess branch October 14, 2025 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(lib): add GetAccessRequest func

2 participants