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

📖 Add ADR on rancher namespace strategy #264

Conversation

Danil-Grigorev
Copy link
Contributor

@Danil-Grigorev Danil-Grigorev commented Nov 16, 2023

kind/documentation

What this PR does / why we need it:

This ADR proposes a decision on rancher cluster namespace usage. This will allow rancher turtles to control the created resources location during and after the import process, and allow the operator to have a static set of namespaced RBAC rules.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):

A part of rancher/highlander#39
Fixes: #296

Special notes for your reviewer:

Checklist:

  • squashed commits into logical changes
  • includes documentation
  • adds unit tests
  • adds or updates e2e tests

@Danil-Grigorev Danil-Grigorev added the kind/documentation Improvements or additions to documentation label Nov 16, 2023
@Danil-Grigorev Danil-Grigorev requested a review from a team as a code owner November 16, 2023 11:28
@Danil-Grigorev Danil-Grigorev force-pushed the namespace-scoped-rancher-adr branch 3 times, most recently from 5ee4b94 to 1884b86 Compare November 16, 2023 11:40
Signed-off-by: Danil Grigorev <danil.grigorev@suse.com>
Copy link
Contributor

@richardcase richardcase left a comment

Choose a reason for hiding this comment

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

I like the idea that we create the Cluster in a specific namespace. As you say it allows us to do RBAC easier.

Just the one comment about just creating Cluster.


The operator will take the responsibility of placing Rancher resources in a specific namespace, defined by the CAPI cluster resource namespace.

The namespace will only dictate the designated location of the Rancher Manager cluster resources, such as **provisioning.cattle/v1.Cluster** and **management.cattle/v3.ClusterRegistrationToken**.
Copy link
Contributor

Choose a reason for hiding this comment

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

Rancher Turtles at this stage will only create the provisioning.cattle/v1.Cluster instance. It will continue to rely on the Rancher machinery to create the ClusterRegistrationToken.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you think this may have a fallback scenario of creating the token ourselves? It follows the “random” namespace strategy rancher uses.

Copy link
Contributor

Choose a reason for hiding this comment

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

Currently we are replicating the UI and token generation is automatic. I'd say we follow that and it that changes in the future we can supercede this version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With the current investigation on usage of the managementv3.Cluster to abstract provisioningv1.Cluster it is better to have our priorities stated and clear, in case the behavior would unexpectedly change, or we may loose backwards compatibility.

Copy link
Contributor

Choose a reason for hiding this comment

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

The current situation is that we create the provisioningv1.Cluster and so its fine for us to state we will create this in a specific namespace.

We rely on the Rancher machinery to create the ClusterRegistrationToken and as it currently stands the ClusterRegistration token is automatically created in a namespace thats the name of the managementv3.Cluster (that is autogenerated from the provisioningv1.Cluster) . So we will not be creating the ClusterRegistration token and so this should be removed from the ADR.

If we change to generate the managementv3.Cluster in the future then this is cluster scoped and not namespace scoped, so we may need to supercede this ADR at a later date.

@richardcase
Copy link
Contributor

@Danil-Grigorev - shall we close this and create an updated version when we switch to the v3.Cluster?

@Danil-Grigorev
Copy link
Contributor Author

I’m closing this as this needs a revisit at a later date with management v3 cluster workflow in mind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ADR: Namespace strategy
4 participants