Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support persistent volumes in Cosmos chains #255

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions charts/devnet/templates/chains/cosmos/genesis.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,14 @@ spec:
spec:
{{- include "imagePullSecrets" $chain | indent 6 }}
initContainers:
{{- if hasKey $chain "storage" }}
Inkvi marked this conversation as resolved.
Show resolved Hide resolved
- name: init-chmod-data
image: busybox
command: ['sh', '-c', 'chmod -R 0777 {{ $chain.home }}']
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm maybe there is another way to do this. Will have to look into this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it depends on what user runs the containers. If you know a better way, I would gladly implement it.

volumeMounts:
- name: node
mountPath: {{ $chain.home }}
{{- end }}
{{- if $toBuild }}
- name: init-build-images
image: ghcr.io/cosmology-tech/starship/builder:latest
Expand Down Expand Up @@ -310,8 +318,10 @@ spec:
{{- end }}
{{- end }}
volumes:
{{- if not (hasKey $chain "storage") }}
- name: node
emptyDir: { }
{{- end }}
- name: addresses
configMap:
name: keys
Expand All @@ -327,6 +337,18 @@ spec:
- name: faucet
emptyDir: { }
{{- end }}
{{- if hasKey $chain "storage" }}
volumeClaimTemplates:
- metadata:
name: node
spec:
accessModes: [ "ReadWriteOnce" ]
storageClassName: {{ $chain.storageClassName }}
resources:
requests:
storage: {{ $chain.storage }}
{{- end }}

---
{{- end }}
{{- end }}
14 changes: 14 additions & 0 deletions charts/devnet/templates/chains/cosmos/validator.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -225,14 +225,28 @@ spec:
- mountPath: /configs
name: addresses
volumes:
{{- if not (hasKey $chain "storage") }}
- name: node
emptyDir: { }
{{- end }}
- name: addresses
configMap:
name: keys
- name: scripts
configMap:
name: setup-scripts-{{- include "devnet.chain.name" $chain.name }}
{{- if hasKey $chain "storage" }}
volumeClaimTemplates:
- metadata:
name: node
spec:
accessModes: [ "ReadWriteOnce" ]
storageClassName: {{ $chain.storageClassName }}
resources:
requests:
storage: {{ $chain.storage }}
{{- end }}

---
{{- end }}
{{- end }}
Expand Down
6 changes: 6 additions & 0 deletions charts/devnet/values.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,12 @@
"repo": {
"type": "string"
},
"storage": {
"type": "string"
},
"storageClassName": {
"type": "string"
},
"upgrade": {
"type": "object",
"properties": {
Expand Down
2 changes: 2 additions & 0 deletions tests/e2e/configs/one-chain.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ chains:
- name: osmosis-1
type: osmosis
numValidators: 1
storageClassName: standard-rwo
storage: 1Gi
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we update this to

storage:
  className: standard-rwo
  size: 1Gi

Might make more sense for the users. I would like to avoid exposing all the k8s specific naming directly to the users (if it can be avoided and made simpler).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I imagine that this is mostly needed for production use case. At least, that's when we use it. With this assumption, I believe that a Starship user knows what they are doing and familiar with k8s concepts and hence they want to have better controls. Otherwise, Starship needs to expose the same concepts but with Starship alternatives. That doesn't sounds like a good UX to me.

If you have better ideas at how to allow fine controls, let me know.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, we dont have to change the names, but if we could still move it into storage, it will make the happy path to be less cluttered.

ports:
rest: 1313
rpc: 26653
Expand Down