Add a proposal for the Gluster CRDs #40
Conversation
Signed-off-by: John Strunk <jstrunk@redhat.com>
ghost
assigned
|
Folks who should take a look:
|
|
An off-hand remark to start after my first skimming: While "quiesce" is a technically appropriate term, I think it's a little too uncommon and hard to quickly understand. I think either of "inactive" or "disabled" would be better. |
docs/Developers/Design/CRD_design.md
Outdated
| # hostPath mount | ||
| mode: device # (device | volume) | ||
| # In device mode, the pod would use the entire device(s) as a single VG | ||
| device: # (iff mode==device) |
There was a problem hiding this comment.
Not sure, but I thought they belongs to Node CR ? Is it guaranteed to be same for all nodes in the zone?
Other than this, looks fine on the top level!
There was a problem hiding this comment.
That's a tough one. Here's my reasoning...
The Zone provides 2 functions:
- A grouping of nodes in a single fault domain
- A template for how to deploy nodes
It's the 2nd function that resulted in the wholestoragesubtree. The main option that is expected to be used ismode: volumeto get block PVs. In this case, a standard device template for the whole Zone is fairly easy to see.mode: deviceis really an escape hatch for clusters that (1) don't yet support block PV or (2) are using Heketi & the old stack. I expect even bare metal to use block PV along w/ dynamic local provisioning once that is fully merged.
The other difficulty of using the Node object is that (as currently planned) it will always be created by the operator. In that case, there's no way to set the device list when the object is first created. Either we'd have to find a way to change the semantics or an admin would need to annotate each Node object after the operator creates it 👎 .
I do agree with you that having a fixed device list for a whole zone isn't desirable. The question is how common it is for the device list to vary by host. I would expect most environments to be fairly symmetric.
But here's an addl wrinkle that I hadn't considered...
We don't really want to use sd* designations since they are not stable. Instead, we'd need something in the /dev/disk/by-* tree, and these will vary by host. Perhaps the sd* can be used as a "template" in keeping w/ the Zone notion, and the actual IDs get used internally (and potentially annotated onto the Node object).
There was a problem hiding this comment.
While targeting a symmetric and automated cluster model is definitely the way to go, we can't ignore that on-prem scenarios are especially likely to not be symmetric. I've run across a number of them when dealing with customer cases. We also can't deny the reality that local block PVs may not reach beta for a good while after we want the operator to be released. I say the way to do this is to leverage (or mimic?) some of the functionality of the local storage provisioner such that we provide a regular expression of devices that are valid for storage consumption.
We don't really want to use sd* designations since they are not stable. Instead, we'd need something in the /dev/disk/by-* tree, and these will vary by host. Perhaps the sd* can be used as a "template" in keeping w/ the Zone notion, and the actual IDs get used internally (and potentially annotated onto the Node object).
Could we have the operator detect a /dev/disk name for a given /dev/sd* name under the covers? We could update the status of the specific Node CR to indicate which device names are actually in use.
|
Here's something I'd like opinions on: zones:
- name: az1
management:
mode: automatic
automatic:
minNodes: 3
 maxNodes: 99
 maxStorage: 100Ti
 freeStorageMin: 500Gi
 freeStorageMax: 2Ti
storage:
 mode: volume
 volume:
 size: 1Ti
 storageClass: ebs-az1
- name: az2
management:
mode: automatic
automatic:
minNodes: 3
 maxNodes: 99
 maxStorage: 100Ti
 freeStorageMin: 500Gi
 freeStorageMax: 2Ti
storage:
 mode: volume
 volume:
 size: 1Ti
 storageClass: ebs-az2
- name: az3
management:
mode: automatic
automatic:
minNodes: 3
 maxNodes: 99
 maxStorage: 100Ti
 freeStorageMin: 500Gi
 freeStorageMax: 2Ti
storage:
 mode: volume
 volume:
 size: 1Ti
 storageClass: ebs-az3Whatever would have gone into the The benefit to the above change would be that the admin need only create a single CR to get a cluster up and running (or show their configuration). The down-side is that the CR is bigger w/ more fiddly indentation. |
|
@JohnStrunk I say the ease of having only one CR is worth the increased length. In particular, I see the majority of users only having one Zone anyway. |
|
First off, this is a really awesome start! :-) I have a couple questions based on my first read-through:
(Edit: The next two points I wrote overlap a bit with what @JohnStrunk and @jarrpa were discussing above. Apologies for any redundancy, but rather than interject into that subthread I'll leave what I wrote intact but admit that I didn't read through all that when I wrote my comments)
To expand on my "multiple named subsections Idea above". I imagine that we could define the storage section with something like: storage:
# the default storage policy
default:
mode: device
device:
- /dev/sdb
- /dev/sdc
# storage policy to be used on oldservers (TBD how oldservers are matched with the pods)
oldservers:
mode: device
device:
- /dev/sdc
- /dev/sdd
- /dev/sdeIf we wanted to do per node storage we could have a key like "storageOverride". |
|
@phlogistonjohn I would modify your proposal to have More broadly, to account for legacy scenarios where nodes have asymmetric storage topology, we could go as far as just having one Zone per node such that we can specify exact devices for each node. It's an abuse of the intent of the Zone, but one that's technically feasible given that Zones are (to the Operator) just logical distinctions and have no technical underpinnings (as far as Gluster itself is concerned). |
I understand using a list, it's six of one, half dozen of the other for me. I'm not sure what the 2nd sentence is getting at though. Could you provide an example of what you mean?
That sounds a bit too hacky / clever to me. If I really thought asymmetry in the storage devices of the nodes really would be a legacy thing you might be able to convince me. However, I really do think these sorts of asymmetries will occur naturally in the real world as clusters age & grow. If we need to have a strict matching of device layout to abstraction, it seems that maybe we need an additional level of abstraction between node and zone. This may not need it's own CRD but it would prevent us from overloading the concept of zone beyond the natural understanding that users will have (roughly an Availablity Zone) IMO. |
A list would be more graceful in implementation, IMO. :) As far as the second sentence, think like
In that case, we could expand the Zone spec to allow multiple entries in I'm not sure how likely it is to have mixed-management-mode clusters, but it should be technically feasible. More likely, as you said, you'd only use this for external clusters that grow over time. |
|
Following up on some of the questions/discussion so far... Since my question about merging the Zone w/ the Cluster got a positive response, I'm going to write up and push that change. @phlogistonjohn You asked about automatic/manual/external... (I think you figured it out, so this is more for the benefit of others following along) I, too, have a dislike for the asymmetry of the "external" nodes in the Zone. I'm trying to come up with a way to push the device list down to the Node CR (so we can use device UUIDs), and convert what is now at the Zone level into a "template" (as that's what it actually is). Perhaps with some re-arranging, it's possible to make the "external" nodes have the contact info in Node CRs, and have corresponding Zones with a "custom" setting (i.e., child Nodes are created manually and don't follow a template). 🤔 For |
I think I like this, although I think this "thing" needs some sort of name so we can talk about it. (Not entirely serious suggestions: corrals)
Actually, I hadn't, so this is great.
I agree a better name would help make the concept clearer. I also think this is something akin to the current approach being taken by the gk-deploy and openshift-ansible approaches, no?
+1 to the sentiment. I think what might be helpful here is to imagine what the user workflow would be. Since I think its not particularly good UX to make someone type a lot of boilerplate we could have a sort of initial template (
Hehe. This mirrors a discussion I have been part of at the heketi repo. I'm convinced that normal people think of their devices as /dev/sdX etc. But, that's not "safe" for long term use. But I think we're relatively safe if we are up front about saying these device nodes are what's used for initial discovery but not long term use. I think it could be sufficient if the operator only uses these devices to feed the next step in the management chain (eg. heketi or gd2) and then perhaps rely on those subsystems to come back to the operator with a stable identifier? |
|
I've finally taken the time to do a more thorough review of the proposal. Below are my thoughts as well as some repeat of prior discussion:
Some examples that put together the above: Minimalist Cluster CR Complex Cluster CR
Examples for the above: Node CR - Dynamic (created by Operator, immutable except for Node CR - Manual (created by Operator, mutable by admin) Node CR - External (created by admin, mutable by admin) This all seems somewhat fiddly, but I think it's worth it to try and strike a balance between flexibility and maintenance overhead. If we wanted, we could ship utilities to generate examples inside the Operator container, such that you could run something like I have some more thoughts on cluster/node state, but I'll save those for another comment. :) |
|
Sorry to have disappeared for so long on this one... @phlogistonjohn asked:
Since the operator needs to be able to manage upgrades (potentially a multi-step process walking across versions of server and csi pods), it needs to have a manifest knowing what transitions are legal (and tested). This precludes allowing the admin to specify arbitrary images & tags. The actual info will be a part of the manifest bundled w/ the operator. This is similar to how OLM works. |
|
@jarrpa
|
If I'm understanding correctly this would be: or but an error if so the |
I think this is really dangerous. The CRs hold the authoritative, declarative state. The only (almost) safe thing I can think of would be for the operator to resolve to UUIDs, then overwrite the |
|
So we're talking about splitting the "template" functionality from the "failure domain" specification. Is this really necessary? I feel like it's bringing in added room for misconfiguration. @phlogistonjohn brought up different templates for different server configs... if we mandate UUID device specification, there aren't really any templates when using |
It's a bad habit of mine. ;) See my openshift-ansible work.
As always, I'm trying to think about on-prem edge cases, because that's where the challenge is going to lie. I'm thinking of situations where 1. We want to use local block PVs, 2. Dynamic provisioning of local block PVs doesn't exist, and 3. The admin wants to ensure that very specific drives are used. We should also consider the possibility that users will come up with their own reasons we can't think of right now for not wanting to use StorageClasses, in whatever management mode. I can kinda guess at the code path for this when we already have the SC-based path, and it doesn't seem like it wouldn't be bad to implement and maintain.
I imagine this is something the local block provisioner team has hit. How are they dealing with it? Personally I'm fine with letting the admin put in whatever device path they want, so long as we provide a stern warning somewhere in our spec docs that they REALLY SHOULD use UUIDs. Especially for throw-away environments, let 'em have
I think it makes things easier to understand. It also splits the functionality so that those who aren't interested in configuring failure domains don't have to do anything with them. If no zone is specified, just assume all My default stance is always against artificially constraining configurations "for the user's benefit". It is a disservice to competent and power users who want to do potentially interesting things with our technology. We always have the authority to say that whatever they're trying is unsupported and/or won't be fixed. :)
I don't think that mandating they be split is of any meaningful benefit or importance.
I'm not sure I quite understand the meaning of "server templates" here. Is that different from the concept I presented of just have different node templates, each with their (one and only one) management mode/type? Also, as above, I see no concrete reason to prevent users from using dev letter names so long as we make it clear in all our examples and documentation that UUIDs are safer. |
They are going to permit specifying either way. I went looking for a good explanation of the options and found this: https://wiki.archlinux.org/index.php/persistent_block_device_naming I think our only option is to let them specify whatever path they want and warn them that it must be stable. |
Oof... yeah, all right. Any chance you can get put together an update to the design doc with the things you're willing to incorporate? Or any chance we can collaborate on a branch somewhere so I can propose commits? |
I'm aiming for tomorrow. I need to get this and the csi driver StorageClass definition done for GCS/0.2. |
docs/Developers/Design/CRD_design.md
Outdated
|
|
||
| A given Gluster cluster is defined by several different Custom Resources (CRs) | ||
| that form a hierarchy. At the top level is the "cluster" CR that describes | ||
| cluster-wide configuration options such as the "name" for the cluster, the TLS |
There was a problem hiding this comment.
@JohnStrunk can you provide some details about "name" ?
There was a problem hiding this comment.
The "name" is the name field in the Cluster (soon to be GlusterCluster) CR. It would show up as the Service name to which CSI connects to in order to access this Gluster cluster.
docs/Developers/Design/CRD_design.md
Outdated
| that form a hierarchy. At the top level is the "cluster" CR that describes | ||
| cluster-wide configuration options such as the "name" for the cluster, the TLS | ||
| credentials that will be used for securing communication, and peer clusters for | ||
| geo-replication. |
There was a problem hiding this comment.
peer cluster -> Does it mean a seperate TSP ?
There was a problem hiding this comment.
"Peer clusters"... yes separate TSPs, usable as a georep target.
The "peering" for georep is handled at the cluster-level just as georep is set up at the cluster-level today for standalone. Once the peering is done, individual volumes can be replicated. This would be configured via StorageClass parameters that would list the name of this peer cluster and maybe some other options for the volume-level configuration of the replication.
docs/Developers/Design/CRD_design.md
Outdated
| where (DNS name) to find existing nodes (for independent mode). | ||
|
|
||
| Below the zone definitions are individual node definitions that track the state | ||
| of the individual Gluster pods. Manipulating these objects permits an |
There was a problem hiding this comment.
Can we add some details on mentioned state?
There was a problem hiding this comment.
The Zone and Node specs are being reworked based on feedback from @jarrpa and @phlogistonjohn but the state of the individual pods are still tracked. This interacts w/ the node state machine (#36). As well, it will hold the desired configuration for the node. This Node object gets turned into a (currently) StatefulSet of replica=1 to run a Gluster pod (see details of GCS deployment for the StatefulSet stuff)
docs/Developers/Design/CRD_design.md
Outdated
|
|
||
| ```yaml | ||
| apiVersion: "operator.gluster.org/v1alpha1" | ||
| kind: Cluster |
There was a problem hiding this comment.
@JohnStrunk IMO, it would be better if kind somehow mention it is a gluster cluster.
There was a problem hiding this comment.
Will become GlusterCluster
| replication: # (optional) | ||
| # Credentials for using this cluster as a target | ||
| credentials: secretRef | ||
| targets: |
There was a problem hiding this comment.
target->name is a cluster name , Isnt it ? There could be scenarios where one volume is geo replicated between clusters foo and bar where foo as source and bar is target. Another volume from bar to foo. so isnt it a good idea to have a optional volname field in targets ?
There was a problem hiding this comment.
This just sets up the peering between clusters, it doesn't actual cause any replication to happen. If foo can replicate to bar, then in foo's GlusterCluster, it would list bar as a target. If the reverse is also possible, bar's GC would list foo also.
Actual volume replication is handled at the SC level.
| kind: Cluster | ||
| metadata: | ||
| # Name for the Gluster cluster that will be created by the operator | ||
| name: my-cluster |
There was a problem hiding this comment.
@JohnStrunk Isnt it a good idea to club or group Zones ( for ex: may be Zone 1 is high end RAM systems and Zone 2 is with Powerful processors) ? If yes, how can we achieve that with current Cluster CR ?
There was a problem hiding this comment.
SC will list the allowed zones for a volume. If you have a set of fast storage nodes, you'd put them into zones (fast1, fast2, fast3), and the slow ones into (slow1, 2, 3). The fast storage class would then say to use zones fast1, fast2, ...
Default is to spread across all available zones (i.e., zones: "*")
docs/Developers/Design/CRD_design.md
Outdated
|
|
||
| ```yaml | ||
| apiVersion: "operator.gluster.org/v1alpha1" | ||
| kind: Node |
There was a problem hiding this comment.
@JohnStrunk Node CR should have a param to define a certain resource or its capabiliites . Current CR does not allow that.
There was a problem hiding this comment.
You mean reservations and limits? These should be part of the operator's manifest based on what we have qualified. This should not be exposed.
docs/Developers/Design/CRD_design.md
Outdated
| status: | ||
| # TBD operator state | ||
| # Possible states: (active | deleting | quiesced) | ||
| currentState: active |
There was a problem hiding this comment.
Isnt it good to match the state ( ex: Ready) of Node CR to Openshift/Kube node state/status.
There was a problem hiding this comment.
This is more than just a boolean Ready|NotReady. I am open to using Conditions, however.
docs/Developers/Design/CRD_design.md
Outdated
| namespace: gluster | ||
| spec: | ||
| # Admin (or operator) sets desired state for the node. | ||
| targetState: active # (active | quiesce) |
There was a problem hiding this comment.
Why 2 states here ? cant operator and admin co-ordinate on one state?
There was a problem hiding this comment.
The distinction between desired state and actual state is important. Just because the admin wants to mark the node as active doesn't mean it's actually available immediately.
|
@JohnStrunk late to the party, but started my review. Will push more, however one high level question I have on our CRs are, how our own topology based CRs play with topology based volume provisioning in Kube/OCP ? Isnt it going to confuse admins? |
docs/Developers/Design/CRD_design.md
Outdated
| # Min unallocated capacity in the zone | ||
| freeStorageMin: 1Ti | ||
| # Max unallocated capacity in the zone | ||
| freeStorageMax: 3Ti |
There was a problem hiding this comment.
Is operator going to calculate and update CR ? If yes, is it in a periodic manner and how ? Is based on the number of volumes in the cluster? if yes, does operator account node addition and deletion ?
There was a problem hiding this comment.
It would create and update the Node CRs, yes. My guess is that this would be done in response to alerts from agents (possibly within the operator, but not necessarily) monitoring # of volumes and total raw storage availability.
There was a problem hiding this comment.
If would update the status:, not these fields. These fields determine limits for autoscaling of the cluster. The operator would probe GD2 to ensure unallocated device capacity remains between Min & Max. It would then add/remove nodes if it gets out of bounds.
|
Catching up on this discussion after a while of not having much time to look at it, excuse me if I reiterate anything previously discussed. I tried to read every update but I may not have fully absorbed everything.
Yup. I found the same thing while researching options for a request for similar behavior natively baked into Heketi.
Since the operator is not the end of the management chain, and we're going to have things like gd2/heketi/etc in the mix my thought is to simply (strongly) recommend persistent devices, but otherwise treat the given devices as a filter. As in "ok gd2/heketi feel free to absorb these devices into your management scheme" and those subsystems should be expected to map whatever currently is /dev/sdX in a persistent manner. |
OK, I'm fairly ignorant ot OLM. I can buy that strategy for most production scenarios but it strikes me as fairly rigid for developing with. I'm fine with leaving it out of the CRD but when it gets time to coding/implementing I hope that there's some mechanism where I can slot in my custom test build and have that get deployed without having to compile the operator from scratch. |
I can't figure out how to make this work. Consider:
|
My plan here is that there would be a text manifest in the container image. For development, you are free to adjust these however you see fit. A release, however, contains a fixed version of the manifest which points to a very specific set of images that define "version X". |
Unifying would be very cool. Do you have suggestions here? The topology stuff I've seen seems very cloud provider specific. The current link is that (AWS example)... If you have a multi-AZ cluster, you would want to create 3 "zones" for your Gluster cluster, specifying nodeAffinity such that it would put each zone into a specific AWS AZ, and the StorageClass for that zone's PVs would have their affinities set to grab EBS volumes from the given zone. From my pov, you'd want to name your gluster zone "east-1a" if it's going to be running in AWS AZ "east-1a". I think that would make it fairly straightforward. |
|
Progress update: We discussed the CRD in the operator meeting this morning.
I'm going to make another pass through the comments, then try and update the real doc. |
Signed-off-by: John Strunk <jstrunk@redhat.com>
|
I know you thought it'd never happen. 💀 But I managed to update this PR. |
| nodeTemplates: # (optional) | ||
| - name: myTemplate | ||
| # Zone is the "failure domain" | ||
| zone: my-zone # default is .nodeTemplates.name |
docs/Developers/Design/CRD_design.md
Outdated
| to automatically scale the Gluster cluster as required and to automatically | ||
| replace failed storage nodes. | ||
|
|
||
| Within this template, there is a `zone` tag that allows the nodes cretaed from |
docs/Developers/Design/CRD_design.md
Outdated
| of nodes by setting `nodes` to the desired value. The operator can also | ||
| dynamically size the template if, instead of setting `nodes`, the `minNodes`, | ||
| `maxNodes`, `freeStorageMin`, and `freeStorageMax` are configured. In this | ||
| case, the number of stroage nodes always remains between the min and max, and |
| when scaling should be invoked. The template can have a fixed (constant) number | ||
| of nodes by setting `nodes` to the desired value. The operator can also | ||
| dynamically size the template if, instead of setting `nodes`, the `minNodes`, | ||
| `maxNodes`, `freeStorageMin`, and `freeStorageMax` are configured. In this |
There was a problem hiding this comment.
Not proposing a change, but just to clarify: Is the strategy for keeping available storage between freeStorageMin and freeStorageMaximum to spin nodes up and down as needed, and not to manipulate devices used by individual nodes?
There was a problem hiding this comment.
Yes, that is the intent.
| # When this annotation is present, the admin may only modify | ||
| # .spec.desiredState or delete the CR. Any other change will be | ||
| # overwritten. | ||
| anthill.gluster.org/template: template-name |
| # TBD operator state | ||
| # Possible states: (active | deleting | quiesced) | ||
| currentState: active | ||
| # Possible states: (enabled | deleting | disabled) |
There was a problem hiding this comment.
A question on the deleting state... any distinction between that state and the resource being marked for deletion (in kube)?
There was a problem hiding this comment.
They are related, but not quite the same.
An admin may trigger the deletion of a gluster node by deleting the corresponding GlusterNode CR. The operator will have a finalizer on the object, preventing the actual removal until it is properly drained and removed from the cluster. The currentState: deleting is intended to signal that the operator has noticed the request to delete (or decided to do it on its own), and has started the process of draining/waiting/removing from cluster. The CR would actually be deleted (finalizer removed) only after the node and its resources are truly gone since the CR is the only way the operator can track the pending operations.
docs/Developers/Design/CRD_design.md
Outdated
| corresponding pod would also be freed. | ||
| - By changing the `.spec.desiredState` of the Node, the administrator can notify | ||
| the operator (and by extension, other Gluster management layers) that the | ||
| particular node should be considered "in maintenance" (`disabled`)such that it |
Signed-off-by: John Strunk <jstrunk@redhat.com>
|
Anyone else? |
jarrpa
left a comment
jarrpa
left a comment
There was a problem hiding this comment.
Sorry, for some reason I didn't get notified of the latest comments. LGTM!
5 participants
Describe what this PR does
This PR provides a proposed set of CRs for the Gluster operator. While this proposal encompasses much more than what an initial implementation will support, my goal is to make room the features we know we will need to support eventually so we can incorporate them over time, maintaining compatibility of the CRD.
Is there anything that requires special attention?
While I expect this proposal to be fairly contentious, please try to offer concrete suggestions for improvement along with the criticisms. 🙏
Related issues:
Closes #3