Multi-tenancy proposal #429
Conversation
metal3-io-bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
metal3-io-bot
added
the
size/L
lentzi90
force-pushed
the
lentzi90/multi-tenancy
branch
from
f9b0fb1
to
f6ef4c4
Compare
metal3-io-bot
removed
the
do-not-merge/work-in-progress
|
Credentials can be put either at two levels:
With the first approach you have chosen, there can only be one source of baremetal machines, with the second you can have as many sources as machine deployments and you can smoothly transition between sources by changing the credentials. It can be interesting to have a cluster that spans over different regions for redundancy. It is also useful in some edge scenarios where you can have a cluster with a centralized CP and many locations with potentially many baremetal providers. As far as I know there is no explicit requirement in CAPI multi-tenancy contract that there is a single set of credentials. |
|
Interesting point. I'm open to add the same identityRef to the Metal3MachineTemplate also. In fact, this would be identical to how CAPO does it. In practice I don't think it changes the proposal much. We can default the identityRef for the Metal3Machines to what is set on the Metal3Cluster and then allow the user to override that in the Metal3MachineTemplate. |
This adds a proposal for how to implement the CAPI multi-tenancy contract for CAPM3. Signed-off-by: Lennart Jern <lennart.jern@est.tech>
lentzi90
force-pushed
the
lentzi90/multi-tenancy
branch
from
f6ef4c4
to
8ae7667
Compare
|
The title and the user stories give the impression that the proposal solves the multi-tenancy problem. It is true that two clusters can be defined in two namespaces and point to one single namespace of baremetalhosts. But the user of those clusters will have full power over those BMHs and the content of the BMH is equivalent to the content of the cluster. Unless you have something like HostClaims hiding the baremetalhost namespaces, you cannot share BMH with users you do not trust. In fact, if there are good use cases for this remote access for HostClaim, I do not really see any for BMH. |
|
The implementation will be tricky. We need to maintain a watch over a namespace in a cluster generating events in another cluster and we need to know when to remove this watch. Probably when the kubeconfig secret is deleted. |
|
Some fields in the status are copied back. But the dataTemplate mechanism also relies on labels and annotations. I expect that they will not be copied and that the controller will use the kubeconfig to get the actual values. Is that the case ? |
|
From the CAPI perspective this does solve multi-tenancy. Each tenant provide credentials for accessing their BareMetalHosts, similar to how it would work with cloud resources. If two tenants share the same BareMetalHosts, then that is essentially one tenant. This is also the same as for other providers. If you have access to the same OpenStack, AWS or GCP tenant then you also have control over the same cloud resources. I understand that this does not solve the same issue as the HostClaim, but I see this as essential. It also does not block implementing HostClaims before or after. as long as it is done with concideration. That was my goal with this, to make the design of the HostClaim easier by extracting this important part. |
|
/approve |
metal3-io-bot
added
the
approved
|
/hold |
metal3-io-bot
added
the
do-not-merge/hold
|
Two weeks have passed and we have discussed it offline. I think it is safe to remove the hold |
metal3-io-bot
removed
the
do-not-merge/hold
|
/test ? |
1 similar comment
|
/test ? |
|
@lentzi90: The following commands are available to trigger required jobs:
Use
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Rozzii The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| context: default | ||
| ``` | ||
|
|
||
| The status of the Metal3Machine should be exteded with the information from the |
| - `.status.errorCount` on the BMH would become | ||
| `.status.bareMetalHost.errorCount` | ||
| - `.status.errorMessage` on the BMH would become | ||
| `.status.bareMetalHost.errorMessage` |
There was a problem hiding this comment.
Something to think about: we might expose internal details through error messages.
| # New fields | ||
| bareMetalHost: | ||
| provisioning: | ||
| ID: f1e06671-6823-4676-9827-33efd7231c3f |
There was a problem hiding this comment.
Mmmm, maybe filter out the ironic node UUID?
|
/cc @zaneb since you have opinions on multi-tenancy. |
This adds a proposal for how to implement the CAPI multi-tenancy contract for CAPM3.