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

WIP: update multi-tenacy docs #1

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

davidblum
Copy link
Owner

This PR updates the multi-tenancy section to provide:

  • syntax fixes
  • more explanation around components
  • an applyable example

/kind documentation

What this PR does / why we need it:

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

Special notes for your reviewer:

Checklist:

  • squashed commits
  • includes documentation
  • adds unit tests
  • adds or updates e2e tests

Release note:


@@ -6,6 +6,8 @@ For details, see the [multi-tenancy proposal](https://github.com/kubernetes-sigs

For multi-tenancy support, a reference field (`identityRef`) is added to `AWSCluster`, which describes the identity to be used when reconciling the cluster.

In other words, `identityRef`is the function that dictates _which_ aws account cluster-api will deploy into.

Choose a reason for hiding this comment

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

I believe this might be redundant, the identityRef has nothing to do with the account per se, and the paragraph at line 24 clarify what is this about.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I understand the point you're making. I think while it may be redundant, it states an important part of the multi-tenancy configuration. Which is how an operator dictates which cluster they can deploy into.

I'd like to leave something high level explaining this at the top of the doc, do you have any suggestions for wording?

Choose a reason for hiding this comment

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

You could say something like:

identityRef reference the credentials or the role that the capa-controller will use to manage the relative cluster

But then we are, sort of, repeating what's in the previous paragraph

@LucaLanziani
Copy link

Aside from the comment above, LGTM

@davidblum davidblum force-pushed the multi-tenancy-docs branch from 6cf67b8 to 64d6f82 Compare March 16, 2022 16:45
typo

CR feedback
@davidblum davidblum force-pushed the multi-tenancy-docs branch from 1b05e5c to 4048a8d Compare March 23, 2022 16:48
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.

2 participants