Conversation
…ntroller Signed-off-by: hasheddan <georgedanielmangum@gmail.com>
|
|
muvaf
left a comment
There was a problem hiding this comment.
Overall LGTM but I think we need arch and OS fields at least.
Signed-off-by: hasheddan <georgedanielmangum@gmail.com>
|
|
negz
left a comment
There was a problem hiding this comment.
I think we need arch and OS fields at least.
I suggest we wait until we know the shape of two or three managed resources that could bind to this claim before we "promote" fields to the claim. We want any setting exposed on the claim to be super useful to claim authors, and broadly applicable to most or all of the managed resource that could bind to a claim. I agree that arch and OS will probably make sense, but I'd prefer to make that call once we're informed by a few managed resources.
|
|
||
| // A VirtualMachine is a portable resource claim that may be satisfied by | ||
| // binding to a Virtual Machine managed resource such as an AWS EC2 instance | ||
| // or an Azure VM. |
There was a problem hiding this comment.
I recall that the initial driver for this change was adding support for Packet, who market as bare metal. If we want to bind this kind to bare metal machines (either Packet or eventually on prem) perhaps Machine would be appropriate? This would fit both virtual and bare metal machines.
We might also consider Instance, which would have symmetry with the existing CloudSQLInstance, RDSInstance, etc types which tend to mean "a server running software X", but would probably make less sense for a bare metal machine.
There was a problem hiding this comment.
Agreed, it seems wrong to call it a VM if binding to bare metal. I would personally be in support of Instance because of the generality of the term. Packet refers to their bare metal machines as Devices.
There was a problem hiding this comment.
Just Instance seems very generic to me as it does not say anything about the type of that instance, especially when we have resulted derivatives like InstanceController. Maybe something more like ComputeInstance, MachineInstance or CPUInstance?
There was a problem hiding this comment.
@soorena776 looking back at this I would tend to agree with you. @prasek @bassam any thoughts on these options?
There was a problem hiding this comment.
I'd prefer ComputeInstance over Instance. I like Machine too, and would probably pick that in a vacuum, but ComputeInstance matches our other FooInstance claim names.
There was a problem hiding this comment.
MachineInstance 👍
You’re getting a machine (VM or bare-metal). Compute is too general IMO.
Signed-off-by: hasheddan <georgedanielmangum@gmail.com>
|
|
|
@hasheddan Do you have an idea of how far off any Stacks (e.g. Packet) are from using this claim? I'm a little hesitant to ship a new claim kind until at least one Stack actually makes use of it. |
|
Closed in favor of #942 |
Description of your changes
Adds a VM claim kind and a corresponding default class controller such that infrastructure stacks can add VM resources that can be dynamically provisioned. Because VM configuration can vary greatly across cloud providers (or even within a cloud provider), this initial claim implementation does not surface any configuration details. It is possible we would want
OSor some other field surfaced in the future.Checklist
I have:
make reviewableto ensure this PR is ready for review.clusterrole.yamlto include any new types.