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.

Sign up for GitHub

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

Jump to bottom

feat: add pvc to clickhouse #50

Merged
merged 1 commit into from
May 22, 2020
Merged

feat: add pvc to clickhouse #50

merged 1 commit into from
May 22, 2020
+12 −5

Conversation

Mokto
Copy link
Contributor

@Mokto Mokto commented Apr 11, 2020 •
edited
Loading

Context : after a Kubernetes upgrade I lost all my tables

@Mokto Mokto requested a review from DandyDeveloper April 11, 2020 13:57
@Mokto
Copy link
Contributor Author

Mokto commented Apr 11, 2020

Sadly the clickhouse chart doesn't allow us not to use to storageClass.

DandyDeveloper
DandyDeveloper approved these changes Apr 13, 2020
View reviewed changes
Copy link
Collaborator

@DandyDeveloper DandyDeveloper left a comment

Choose a reason for hiding this comment

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

Suggestion, but not essential.

sentry/values.yaml Outdated
enabled: true
accessModes:
- "ReadWriteOnce"
storageClassName: "standard"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Common practice is to set this as "default". May be better?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's unclear in the k8s docs, but I think it's only when you don't specify "storageClassName" that default is used. Could you confirm?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry completely missed this reply... Oops. Basically no "baremetal" cluster comes with a storage class, so blank typically defaults to default in my experience... Beyond that though really it's down to the user. I don't think it's a big deal either way.

@Mokto Mokto mentioned this pull request Apr 23, 2020
Closed
@Mokto Mokto mentioned this pull request May 20, 2020
Closed
@Mokto Mokto force-pushed the add-pvc-clickhouse branch 2 times, most recently from b01fcca to 33434ea Compare May 22, 2020 08:38
@Mokto
Copy link
Contributor Author

Mokto commented May 22, 2020

@DandyDeveloper I removed the storageClass per default, by cloning the clickhouse chart inside our repo.

@DandyDeveloper
Copy link
Collaborator

@DandyDeveloper I removed the storageClass per default, by cloning the clickhouse chart inside our repo.

👍

@Mokto Mokto force-pushed the add-pvc-clickhouse branch from 33434ea to 7b264b2 Compare May 22, 2020 08:43
@Mokto Mokto merged commit 197930f into develop May 22, 2020
@Mokto Mokto deleted the add-pvc-clickhouse branch May 22, 2020 08:43
This was referenced Apr 12, 2024
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DandyDeveloper DandyDeveloper DandyDeveloper approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Mokto @DandyDeveloper