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

Add volumeClaimTemplates to raft storage api #5

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

adibrastegarnia
Copy link
Contributor

More testing will be needed.

@@ -181,7 +168,7 @@ metadata:
subjects:
- kind: ServiceAccount
name: raft-storage-controller
namespace: kube-system
namespace: default
Copy link
Member

Choose a reason for hiding this comment

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

If this is a cluster scoped controller it should be deployed in the kube-system namespace as is convention for all controllers

default: 1
volumeClaimTemplates:
Copy link
Member

Choose a reason for hiding this comment

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

Can we just convert this to YAML just so it's clear and so we're sure we can use it in the Helm chart as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will figure that out. It looks like it is working in helm chart as well :)

@adibrastegarnia
Copy link
Contributor Author

@kuujo
you don't believe it but I ran into a new issue :) Will explain it to you tomorrow.

@adibrastegarnia adibrastegarnia force-pushed the add_new_pvc branch 3 times, most recently from 0696d7b to ad4c6a7 Compare April 27, 2020 19:06
replicas: 1
partitionsPerCluster: 3
volumeClaimTemplates:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we keep this here as an example?

partitionsPerCluster:
type: integer
min: 1
max: 1024
minimum: 1
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be updated in helm charts as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants