Optional PVC provisioning

[orlovmyk]

I want to shkeeper only to work only with USDT (TRX) network, meanwhile even If I keep all other currencies false - PVCs are created for all currencies enabled. There is no any if statement in PVC objects:

apiVersion: v1
kind: PersistentVolumeClaim
metadata:
  name: tron-output-directory
  namespace: {{ .Values.namespace }}
spec:
  storageClassName: {{ .Values.storageClassName }}
  accessModes:
  - ReadWriteOnce
  resources:
    requests:
      storage: 2000Gi

P.S.
Also I am not sure if everyone can afford to provision 2 TB storage, it would be great to have those configurable as well

P.S.S.
PVC request size for full node, even if I want to have trimmed one

[dmytro-samoylenko]

Hello,

Thank you for suggestion and PR.

The reason PVC are unconditionally created it to prevent data loss by PVC unprovisioning if user accidently disable previously enabled crypto. The main platform for Shkeeper deployment is k3s, which is a self hosted k8s distro for virtual and dedicated servers, and there were no downsides in provisioning PVC of any size, as they created on a local disk and take no space, except an extra directory under /var/lib/rancher/k3s/storage. But I see that it can introduce an unwanted expense if deployed on managed k8s services like AWS.

I think the solution might be in keeping important small PVC unconditionally created (like shkeeper db, tron db, mysql etc). The rest of PVC are not that important because they contain blockchain data that can be re-downloaded in case of its loss.

[orlovmyk]

@dmytro-samoylenko Hello, glad to hear quick response from your team

Even with k3s cluster, in my installation I've disabled local-path provisioner at all, because I think it's very dangerous to use it. k3s allows you yo do that with option --disable=local-storage. While I agree with you on some points I think you need to look forward and make chart as flexible as possible.

If you want I can update PR to make only big (>100GB) PVCs optional.

[orlovmyk]

@dmytro-samoylenko Hello, is fixing this issue in your plans? Currently it blocks us from using your helm chart 😢

[Sacerdoss]

Hey, this is what the annotation helm.sh/resource-policy: keep is for.
We cannot provision that much storage in our cluster but would like to offer a subset of this coins.

[orlovmyk]

@Sacerdoss I have tried to even do PR, but it looks like they are ignoring any contribution
#3

[Sacerdoss]

@Sacerdoss I have tried to even do PR, but it looks like they are ignoring any contribution
#3

They did respond why they cannot accept that merge request an I provided a solution to that reason describing how it can be done.

I cannot modify your merge request. But if you add that annotaion to the PVC resources it should be possible to merge if I'm not mistaken.

[orlovmyk]

@dmytro-samoylenko @yukonet Added feature, proposed by @Sacerdoss
Please let us know what else needs to be done to get this merged. This issue blocks others from using your product, if you want to be community-driven, I guess so because you have open-sourced your project - it's crucial to keep communication with others

Pineapple-Of-Evolution@317d667 in PR #3

Thanks 👍 🙏

[orlovmyk]

CC:
@vsyshost
@dmytro-samoylenko
@Olxndr
@yukonet

[dmytro-samoylenko]

We came up with next implementation without using helm.sh/resource-policy: keep annotation. I believe helm.sh/resource-policy: keep may cause unwanted difficulties in certain upgrade scenarios, as mentioned in docs.

PVCs are divided into 3 categories:

1. Always provisioned. These are shkeeper and mariadb.

2. Only provisioned if needed. These are storing recoverable data, e.g. blockchain, which can be re-downloaded: tron-fullnode, ethereum-fullnode etc. These PVCs are unprovisioned when related crypto or fullnode is set to enabled: false in values.yaml.

3. Provisioned, until explicitly turned off. These are storing unrecoverable data such as wallets, crypto keys etc: btc, ltc, doge, monero-rpc-wallet, tron-shkeeper, btc-lightning, which can't be recovered (unless a user has a valid backup). To disable the provisioning of such PVCs for all existing and newcoming cryptos, a new top-level values.yaml option keepUnusedEssentialPVCs has been added. If set to false, and related crypto is enabled: false, the PVC will not be provisioned on helm install, or will be removed on helm upgrade. If only a specific PVC needs to be removed, without changing the global keepUnusedEssentialPVCs option, it could be done by setting related storage.size to 0, e.g.:

doge:
  enabled: false
  dogecoind:
    storage:
      size: 0

Please let me know if you are happy with such implementation.

[orlovmyk]

@dmytro-samoylenko Thanks for your contribution to this issue, I will test your changes when have time