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

Enable support for MongoDB persistence in K8s #364

Merged
merged 7 commits into from Apr 2, 2019

Conversation

kaizimmerm
Copy link
Contributor

Hi,

small but practical improvement is to use persistent storage in the K8s setup for the MongoDB.

So I:

  • Migrated to the standard stable/mongodb chart which has this feature (and many more) already included.
  • Adapt the helm guide to the new chart.
  • Extended the Azure guide to leverage this using Azure storage.

Let me know what you think,

Kai

@ghandim
Copy link
Contributor

ghandim commented Mar 28, 2019

@kaizimmerm Why do you not connect a CosmosDB instance for a persistent storage example for ditto on Azure?

@kaizimmerm
Copy link
Contributor Author

@ghandim I intend to look into this soon.

However, I would like to keep the MongoDB embedded option for test/dev purposes and especially there it is really annoying if you lose your data all the time.

Besides this patch will work in other K8s environments that have storage support as well. I hear there are other clouds next on Azure in existence, all balderdash of course 😈

Kai

@ghandim
Copy link
Contributor

ghandim commented Mar 28, 2019

@kaizimmerm Good point :-) I will give it a try...


...either with persistent storage

```bash
Copy link
Contributor

@ghandim ghandim Mar 28, 2019

Choose a reason for hiding this comment

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

@kaizimmerm I think you have to add here (cd ../helm/eclipse-ditto/ && helm dependency update). Otherwise you get a weird message from helm...

Signed-off-by: Kai Zimmermann <kai.zimmermann@microsoft.com>
...either with persistent storage

```bash
helm upgrade ditto ../helm/eclipse-ditto/ --namespace $k8s_namespace --set service.type=LoadBalancer,mongodb.persistence.enabled=true,mongodb.persistence.storageClass=managed-premium-retain --wait --install
Copy link
Contributor

Choose a reason for hiding this comment

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

@kaizimmerm Whats the advantage to use an own storage class definition instead of the default one? If I spin up ditto with helm upgrade ditto ../helm/eclipse-ditto/ --namespace $k8s_namespace --set service.type=LoadBalancer,mongodb.persistence.enabled=true --wait --install K8s generates a default persistent volume with from my point of view the same configuration as configured in managed-premium-retain.

Copy link
Contributor

@ghandim ghandim Mar 29, 2019

Choose a reason for hiding this comment

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

@kaizimmerm Now I got it. The important difference is the retain policy.

@ghandim
Copy link
Contributor

ghandim commented Mar 29, 2019

@kaizimmerm I think if we want to support the preservation of the mongodb data between complete reinstallations of ditto e.g. with helm delete --purge ditto than we have to introduce a pvc like

kind: PersistentVolumeClaim
metadata:
  name: mypvc
  namespace: dittons
spec:
  accessModes:
    - ReadWriteOnce
  resources:
    requests:
      storage: 8Gi
  storageClassName: managed-premium-retain

Then we can upgrade/install ditto without any data loss with

helm upgrade ditto ../helm/eclipse-ditto/ --namespace dittons --set service.type=LoadBalancer,mongodb.persistence.enabled=true,mongodb.persistence.existingClaim=mypvc --wait --install

With that it could be sufficient to use the default storage-class because the storage is deleted only when the pvc is deleted, or?

What do you think?

@thjaeckle
Copy link
Member

Please ignore the Codacy check - I tried that out and it did "review" all open PRs

@kaizimmerm
Copy link
Contributor Author

@ghandim yes, I guess that would work. I could add a paragraph on this to the helm guide. Agreed?

Signed-off-by: Kai Zimmermann <kai.zimmermann@microsoft.com>
@ghandim
Copy link
Contributor

ghandim commented Apr 1, 2019

@kaizimmerm With this pvc I was able to create a persistent reusable (default) storage in Azure AKS/Minikube:

apiVersion: v1
kind: PersistentVolumeClaim
metadata:
  name: ditto-pvc
  namespace: dittons
spec:
  accessModes:
    - ReadWriteOnce
  resources:
    requests:
      storage: 8Gi
#  storageClassName: <YOUR_STORAGE_CLASS> 

To use the storage on Minikube in the namespace dittons you have to create ditto with

helm upgrade ditto ./eclipse-ditto/ --namespace dittons --set mongodb.persistence.enabled=true,mongodb.persistence.existingClaim=ditto-pvc --wait --install

On Azure:

helm upgrade ditto ../helm/eclipse-ditto/ --namespace dittons --set service.type=LoadBalancer,mongodb.persistence.enabled=true,mongodb.persistence.existingClaim=ditto-vpc --wait --install

So I think we should align also the helm documentation and use a separate namespace dittons and move the ditto-pvc to helm. Then also for the developers it is more clear that there are not many differences between the deployments on different environments.
What do you think?

@kaizimmerm
Copy link
Contributor Author

@ghandim In Azure (and I assume other K8s setups as well) there are already PVCs pre setup. So I would make this an optional step in the helm guide.

@kaizimmerm
Copy link
Contributor Author

@ghandim forget it, I mixed something up 😄 So I agree, I will adapt accordingly

Signed-off-by: Kai Zimmermann <kai.zimmermann@microsoft.com>
@kaizimmerm
Copy link
Contributor Author

@ghandim added it, let me know what you think

@@ -44,8 +44,32 @@ helm init

```bash
cd <DITTO_PATH>/deployment/helm/
kubectl create namespace dittons
helm update dependencies
Copy link
Contributor

Choose a reason for hiding this comment

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

Path is missing here: helm dependencies update ./eclipse-ditto/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, good catch!

Signed-off-by: Kai Zimmermann <kai.zimmermann@microsoft.com>
apiVersion: v1
kind: PersistentVolumeClaim
metadata:
name: ditto-pvc
Copy link
Contributor

Choose a reason for hiding this comment

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

@kaizimmerm Should we name the pvc also ditto-mongodb-pvc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

```bash
echo " storageClassName: managed-premium-retain" >> ../helm/ditto-mongodb-pvc.yaml
kubectl apply -f ../helm/ditto-mongodb-pvc.yaml --namespace $k8s_namespace
helm upgrade ditto ../helm/eclipse-ditto/ --namespace $k8s_namespace --set service.type=LoadBalancer,mongodb.persistence.enabled=true,mongodb.persistence.existingClaim=ditto-pvc --wait --install
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto-mongodb-pvc has now to be used here also for the existingClaim ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done 😄

@ghandim
Copy link
Contributor

ghandim commented Apr 1, 2019

@kaizimmerm One last thing....
Are there any concerns to update the ditto tag to the current milestone 0.9.0-M1?

@kaizimmerm
Copy link
Contributor Author

@ghandim sure, will do that

@ghandim
Copy link
Contributor

ghandim commented Apr 2, 2019

@kaizimmerm Last but not least I would reduce the values in Charts.yaml to

apiVersion: v1
name: eclipse-ditto
version: 0.9.0-M1
description: A basic helm chart for Eclipse Ditto and K8s

What do you think?

@ghandim
Copy link
Contributor

ghandim commented Apr 2, 2019

@kaizimmerm LGTM 👍 @thjaeckle Nothing more to add from my point of view at the moment.

@thjaeckle thjaeckle merged commit 790beeb into eclipse-ditto:master Apr 2, 2019
@thjaeckle
Copy link
Member

Awesome, thank you both for the effort 👍

@kaizimmerm kaizimmerm deleted the k8s-mongodb-persitence branch April 5, 2019 06:22
@thjaeckle thjaeckle added this to the 0.9.0-M2 milestone Apr 8, 2019
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.

None yet

3 participants