MongoDB docs updated for 0.9.0

[the-redback]

xref: https://github.com/kubedb/project/issues/235
xref: https://github.com/kubedb/project/issues/192
xref: https://github.com/kubedb/project/issues/170
xref: https://github.com/kubedb/project/issues/288

• Updated Concept of MongoDB and MongoDBVersion
• Updated Guide of MongoDB
• Updated examples of MongoDB

[endrec]

Couple of things I have found trying to make mongo run:

• spec.replicas has to be set, it's missing form most of the examples
• spec.storageType has to be set, it's missing form most of the examples
• spec.updateStrategy.type needs to be set
• spec.terminationPolicy needs to be set
  I have to note, none of these are required according to the CRD, but mongodb.go complains about them, and does not create a StateFulSet if they are not set.
  Maybe it would be easier to update the mongo operator to apply sensible defaults?

[the-redback]

@endrec , Thanks for having look on the changes. The fields you mentioned are required for operator to work, yet the CRDs are valid because the default values are assigned in mutating webhook. https://github.com/kubedb/mongodb/blob/master/pkg/admission/mutator.go#L107

[endrec]

@the-redback webhooks are disabled in the default helm chart, which I'm using on my system. I don't remember anywhere seeing that they has to be enabled, if that was the case, they should be enabled by default, and helm should fail without the CA cert set.

[the-redback]

@endrec webhooks are enabled by default on charts. You can see the chart template here https://github.com/kubedb/cli/tree/master/chart/kubedb/templates.

[endrec]

@the-redback They are disabled in the default values.yaml:

apiserver:
# [...]
  # enableMutatingWebhook is used to configure mutating webhook for KubeDB CRDs
  enableMutatingWebhook: false
  # enableValidatingWebhook is used to configure validating webhook for KubeDB CRDs
  enableValidatingWebhook: false

[the-redback]

@endrec I see. Mutating webhooks promoted to beta after kubernetes-1.9.0. So, By default It was false. Installer bash script searches for kubernetes version and enables it by default. But, user had to specify in case of charts. Anyway, I thought, we already have got rid of these flag.

[the-redback]

@tamalsaha we can get rid of these enablewebhook flags. At least, we should set true as default value.

[endrec]

@the-redback, @tamalsaha anything works for me, as long as the provided examples work with provided helm chart, I would not complain. :)

[tamalsaha]

@endrec, can you share the command you used to install the Helm chart? We have documented to enable the webhook everywhere we show installer command. https://github.com/kubedb/cli/blob/0.8.0/docs/setup/install.md#using-helm

[endrec]

@tamalsaha you are right, the example helm command does enable the webhooks. As the docs did not mention that it needs to be enabled, and I did not have onessl installed, I just run helm install --name=kubedb --namespace=kubedb using the default values, where they are disabled. If they were enabled in the values.yaml, helm would have stopped me to do so.

[tamalsaha]

If they were enabled in the values.yaml, helm would have stopped me to do so.

Sorry, I am not understanding how Helm would have stopped you? Can you please elaborate a bit more? It is possible that my Helm knowledge is outdated. It is hard to keep up with all these fast moving technology.

Also, you need to provide apiserver.ca value. Currently there is no way to automatically get that from Helm. So, even if we just enable the webhooks it will fail to install the chart. You can use apiserver.ca using onessl as we have documented. Or you can use the following command to do the same:

KUBE_CA=$(kubectl config view --minify=true --flatten -o json | jq '.clusters[0].cluster."certificate-authority-data"' -r)

The reason we keep these options disabled by default is somewhat historical at this point. We used to host our charts in https://github.com/helm/charts repo. That repo has some automated tests that will fail if webhooks are enabled by default because there was no value passed to apiserver.ca. So, by default the webhooks were disabled and we documented that user should set those values to true in the installer docs.

Ideally helm/helm#3501 should be fixed and then we can enable these values by default. Otherwise someone installing the chart with default values helm install --name=kubedb --namespace=kubedb will see that the chart does not install. I think that is also problematic.

[endrec]

Sorry @tamalsaha, I might not have been clear enough.
The reason behind helm stopping me to install the charts with the webhooks enabled by default would be exactly why your tests were failing: if the webhooks are enabled and no ca cert is given, helm will fail because there is a value it can't resolve.

I think if helm install --name=kubedb --namespace=kubedb won't work, and gives someone a message that apiserver.ca is not set, will make that someone think that they might have to provide that parameter after all. :) You know, guys like me don't read the documentation, unless something does not work. 😉

[tamalsaha]

I just tried with just enabling the webhook but not providing apiserver.ca. This is what I got:

$ helm install kubedb --name kubedb-operator \
  --namespace kube-system \
  --set apiserver.enableValidatingWebhook=true \
  --set apiserver.enableMutatingWebhook=true \
  --set apiserver.enableStatusSubresource=true

Error: render error in "kubedb/templates/validating-webhook.yaml": template: kubedb/templates/validating-webhook.yaml:21:31: executing "kubedb/templates/validating-webhook.yaml" at <.Values.apiserver.ca>: wrong type for value; expected string; got interface {}

I don't think this error message tells the user that apiserver.ca must be set and how to set that. In fact, it makes it look like the chart has some kind of bug. In fact, this thing happened in our Stash project the other day stashed/stash#587 .

The problem with Helm is that it is very rigid and provides an inflexible ux for Chart authors. I sympathize with your scenario but I don't see a good option either way at the current time.

I agree that we should update the README.md for chart
to document how to enable these flags.

I am trying to see if we can fix the issue at Helm . https://twitter.com/tsaha/status/1045986311234977794 . But I can't promise when this will be fixed. I opened the issue 8 months ago and there is no activity.

In the interim, we could probably update the webhooks from operator (similar to how we register CRDs). But that is kind of hacky and I am not sure how that will play with Chart upgrades, etc.

[endrec]

A big, bold warning in the readme would certainly help me. 🙂

Actually, registering the webhooks from the operator is not a bad idea, you can check the k8s version there, you can access the ca cert, you have everything needed. In my mind they belong to the operator, just like the CRDs.

[tamalsaha]

#313 and kubedb/docs#197 remove the need for enable* and apiserver.ca variables (hopefully for ever).