-
Notifications
You must be signed in to change notification settings - Fork 14
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
doc: add quick-start guide #148
base: main
Are you sure you want to change the base?
Conversation
7749fa1
to
88ea73c
Compare
@nb-ohad PTAL |
docs/quick-start.md
Outdated
Create an OperatorConfig CR to configure the Ceph-CSI Operator: | ||
|
||
```console | ||
echo ' | ||
apiVersion: csi.ceph.io/v1alpha1 | ||
kind: OperatorConfig | ||
metadata: | ||
name: ceph-csi-operator-config | ||
namespace: ceph-csi-operator-system | ||
' | kubectl create -f - |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are going for minimal installation, why do we need an operator config instance?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it was added to deploy the CR with minimal values and no values and the user can update it later as per requirement. Removed it now
docs/quick-start.md
Outdated
To clean up the resources, delete the OperatorConfig CR, drivers: | ||
|
||
```console | ||
kubectl delete operatorconfig ceph-csi-operator-config -n ceph-csi-operator-system |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Continued from the last comment, for a minimal installation example we don't need an operator config CR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we want a minimal installation that can actually serve storage, we need the user to create at least one ceph connection and at least one client profile. Both are missing from this quick start guide.
88ea73c
to
916fbc4
Compare
@nb-ohad PTAL |
916fbc4
to
5b606ab
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am surprised that the guide does not mention building the operator. Even if this is making use of published images, I think developers want to test locally built images with their local changes.
That will be a follow-up to add/update the document on how to test the local changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Madhu-1 thanks for adding quickstart guide. Very useful!
A general suggestion:
Instead of the echo and using kubectl create -f -
, wouldn't it be convenient to have example yaml files for these items (rbd driver, cephfs driver, client connection, client profile, etc)? this would probably be as follow-up PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried the steps in the guide and they all work with one exception:
Creation of the clientProfile
fails like this:
Error from server (BadRequest): error when creating "STDIN": ClientProfile in version "v1alpha1" cannot be handled as a ClientProfile: strict decoding error: unknown field "spec.cephFs.subvolumeGroup"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
two comments inline. Not strictly change requests really, rather suggestions/thoughts.
Before deploying the Ceph-CSI-Operator, ensure the following requirements are met: | ||
|
||
- A Kubernetes cluster ([supported version](https://kubernetes.io/releases/) recommended) | ||
- Ceph cluster ([supported version](https://docs.ceph.com/en/latest/releases/) recommended) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be nice to mention the rook project here as a convenient means of creating a Ceph cluster in k8s.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have kept the options open for deploying Rook and its up to the user to use their own way to deploy the ceph cluster, that why i have mentioned that a supported version of ceph need to be installed.
namespace: ceph-csi-operator-system | ||
spec: | ||
monitors: | ||
- 10.98.44.171:6789 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be useful to explain how to get the requires IP address and port, e. g. when using rook to set up Ceph.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As the ceph cluster is configured I assume that the user knows the basic monitor information to configure anything. that is the reason not pointing to any specific commands as it depends on the way of the ceph cluster deployment.
5b606ab
to
554a5b3
Compare
Thats for catching it, it was a typo from my side, its fixed now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@nb-ohad PTAL |
added a quick start guide for the minimal installation Signed-off-by: Madhu Rajanna <[email protected]>
554a5b3
to
282fc54
Compare
Describe what this PR does
added a quick start guide for the minimal installation
Is there anything that requires special attention
List items that are not part of the PR and do not impact it's
functionality, but are work items that can be taken up subsequently.
Checklist:
Commit Message Formatting: Commit titles and messages follow
guidelines in the developer
guide.
Reviewed the developer guide on Submitting a Pull
Request
Pending release
notes
updated with breaking and/or notable changes for the next major release.
Documentation has been updated, if necessary.
Unit tests have been added, if necessary.
Integration tests have been added, if necessary.