oxheadalpha
Update schema for snapshots and tarballs
Our new schema separates setting the URLs from deciding when and how to use them.
Please note that the helm lint failure is solved in my prior pull request #393 which I expect we will review and merge before this one. I will need to rebase a little in any case, but there's enough here to start a discussion of the approach.
Interesting, the tests operate not against the branch itself, but rather against the branch as merged against HEAD. I've rebased and force pushed because the tests needed the new default image to be v13-release.
Hmmm, well, I did try to preserve backwards compat, but this is an edge case that I didn't test. What's going on here is that my logic is: if you do not define the snapshot type in the node config or node_globals, then I fall back to the old mechanism. BUT what appears to happen here (which I didn't test for, obviously) is that even though you define a "rolling-node" statefulset, it doesn't override all of the defaults for it. That means that even though you didn't specify it in your values.yaml:
nodes:
rolling-node:
snapshot: standard
is specified.
I think that maybe I will have to change the sense of the conditional and make it s.t. if you define rolling_snapshot_url, etc, they will override the new mechanism. I'm not fond of that, because I'd rather see the new mechanism take priority over the old, but I don't see a way to do that whilst retaining backwards compat.
Another thought: this change restricts usage of snapshots to "known" chains which are hardcoded in octez (mainnet, jakartanet).
If someone wants to use tezos-k8s to create a totally private chain and maintains snapshots for this chain, then there is no longer any any way to pass this snapshot as parameter and bootstrap a private chain node from snapshot/tarball.
I don't think anyone is doing it, but did we consider this? Is it a problem?
@nicolasochem see Roland and my convo here: https://github.com/oxheadalpha/tezos-k8s/pull/394#discussion_r883064047
Does that answer your question?
I want to make another proposal for the backwards compat mechanism.
As I mentioned above, I don't want rolling_snapshot_url, et al. at the top level override people's specific setting of:
nodes:
rolling:
snapshot: tarball
But, I do want to provide a default for the chart with just "rolling-node" defined.
Right now, nodes-><name>->snapshot is an enumerated type with "standard", "tarball", or "none" as possible values. I am going to propose that for backwards compat, I add a value of "compat" which has the semantics of: "standard" unless rolling_snapshot_url is defined in which case that value takes precedence. In this way, we will use the legacy values if nodes-><name>->snapshot isn't defined or if it is set to "compat". The difference between undefined and "compat" is that undefined means no snapshots unless the legacy url values are defined whereas "compat" means standard if they aren't defined.
@harryttd oh yes, it does. I guess it needs to be documented?
Let's add documentation
@elric1 let's wrap this up? We need to add ghostnet support.
Additionally, this will enable a one-liner helm in the new doc website: a single line of helm to install a ghostnet rolling node in a cluster (no values.yaml needed).