(DOCSP-9305): Simple CRD/install instructions #33
Conversation
chatton
left a comment
chatton
left a comment
There was a problem hiding this comment.
Looks great!
My only concerns are around specifying namespace in the command examples, and also classifying kind as a pre-requisite
README.md
Outdated
|
|
||
| ## Licensing | ||
| 1. Install [kubectl](https://kubernetes.io/docs/tasks/tools/install-kubectl/) | ||
| 2. Install [kind](https://kind.sigs.k8s.io/) |
There was a problem hiding this comment.
technically kind isn't a prerequisite, just having any running Kubernetes cluster is. Kind is just an easy way get one up and running on your own machine. What do you think about changing this to be more generic along the lines of have access to a kubernetes cluster and including a recommendation for kind with the relevant link?
There was a problem hiding this comment.
We discussed this with @chatton.
We should say that "Kubernetes" is a requirement, any flavor; but we use Kind for testing, and that's our recommended testing Kubernetes flavor.
There was a problem hiding this comment.
Updated.
README.md
Outdated
| ### Installing the MongoDB Community Kubernetes Operator | ||
|
|
||
| The MongoDB Agent binary in the agent/ directory may be used under the "Free for Commercial Use" license found in agent/LICENSE. | ||
| The MongoDB Community Kubernetes Operator is a collection of [Custom Resource Definitions](https://kubernetes.io/docs/concepts/extend-kubernetes/api-extension/custom-resources/) and a controller. |
There was a problem hiding this comment.
as of right now it is just a single CRD.
There was a problem hiding this comment.
Fixed.
README.md
Outdated
| kubectl get crd/mongodb.mongodb.com | ||
| ``` | ||
| 3. Install the Operator. | ||
| a. Invoke the following `kubectl` command to install the Operator in the `default` namespace: |
There was a problem hiding this comment.
this command isn't always guaranteed to create the operator in the default namespace. This will be the case if no namespace is specified in the operator.yaml or one wasn't provided with the kubectl command. And the current context does not have a default namespace specified.
I think we should indicate that you can specify a namespace
kubectl create -f deploy --namespace <some-namespace>
There was a problem hiding this comment.
Instead of specifying the --namespace <..> we could say that if you want to use a namespace different than default you should specify it on each of the kubectl invokations.
There was a problem hiding this comment.
I see what Cian is saying, because they could have another namespace set in the context, so if they use this it wouldn't necessarily be default. I think its safest just to specify the namespace in every command.
The other option is to include a step on how to set the default namespace in the context, but that might be too much?
There was a problem hiding this comment.
I think specifying --namespace is a good idea. I'm not inclined to set the default namespace in the context, beause it means chaning the user's configuration globally, making this not easy to integrate with other tools.
README.md
Outdated
| To install the Operator in a different namespace, append the `--namespace <my-namespace>` option. | ||
| b. Verify that the Operator installed successsfully: | ||
| ``` | ||
| kubectl get pods |
There was a problem hiding this comment.
same comment about the --namespace/-n
README.md
Outdated
| 1. Invoke the following `kubectl` command: | ||
| ``` | ||
| kubectl apply -f deploy/crds/mongodb.com_v1_mongodb_cr.yaml |
README.md
Outdated
| ``` | ||
| 2. Verify that the MongoDB resource deployed: | ||
| ``` | ||
| kubectl get mongodb |
There was a problem hiding this comment.
and one last time! --namespace
melissamahoney-mongodb
force-pushed
the
DOCSP-9305
branch
2 times, most recently
from
670b08d to
022b983
Compare
|
@chatton @rodrigovalin I've updated the PR from your comments. Please take another look when you can. Thanks! |
melissamahoney-mongodb
force-pushed
the
DOCSP-9305
branch
from
022b983 to
e827cc7
Compare
|
@rodrigovalin @chatton Turns out I don't have write access to the |
3 participants
JIRA
Adds installation and basic deploy instructions to the readme.