Refactor documentation #800

balamurugana · 2023-06-12T16:19:34Z

No description provided.

djwfyi

Quite a few needling suggestions to improve grammar and clarity.

A couple of questions on some points that lead me to some possibly confusing assumptions that I think some adjustments or additional info may help resolve.

docs/installation.md

docs/architecture.md

djwfyi · 2023-06-27T20:56:34Z

docs/architecture.md

+* `Legacy node server`
+
+## Controller
+Controller runs as `Deployment` Pods named `controller` which is three replicas in any Kubernetes nodes not limiting to where node server is running. In the three replicas, one instance is elected to serve requests. Each pod contains below running containers


Suggested change

Controller runs as `Deployment` Pods named `controller` which is three replicas in any Kubernetes nodes not limiting to where node server is running. In the three replicas, one instance is elected to serve requests. Each pod contains below running containers

The Controller runs as `Deployment` Pods named `controller`, which are three replicas located in any Kubernetes nodes not limiting to where node server is running. In the three replicas, one instance is elected to serve requests. Each pod contains below running containers:

I'm not really sure what not limiting to where node server is running means.
Is this that the controller does not exist in a pod running the node server? Or something else?

If that's true, then maybe ... in any Kubernetes node where the node server is not running.

controller pods may (may not) run on nodes where node server pods are running. We have control on where node server pods to run, but not to controller pods.

Suggested change

Controller runs as `Deployment` Pods named `controller` which is three replicas in any Kubernetes nodes not limiting to where node server is running. In the three replicas, one instance is elected to serve requests. Each pod contains below running containers

The Controller runs as `Deployment` Pods named `controller`, which are three replicas located in any Kubernetes nodes, potentially including a pod with a node server running. In the three replicas, one instance is elected to serve requests. Each pod contains below running containers:

Actually I trimmed to remove node-server reference here.

The Controller runs as `Deployment` Pods named `controller`, which are three replicas located in any Kubernetes nodes. In the three replicas, one instance is elected to serve requests. Each pod contains below running containers:

Does it look good?

docs/architecture.md

docs/volume-management.md

docs/volume-provisioning.md

Praveenrajmani · 2023-07-07T12:46:45Z

docs/installation.md


-#### 3. Intitialize the drives
+## DirectPV CSI driver installation
+Before starting the installation, it is required to have DirectPV plugin installed on your system. For plugin installation refer [this documentation](#directpv-plugin-installation). If you are not using `krew`, replace `kubectl directpv` by `kubectl-directpv` in below steps.


If the plugin is not installed via krew, we must download the binary and move it to /usr/local/bin/ - this will enable the plugin for kubectl commands.

Can we mention ^^ here?

I don't think we need to specify where to keep the binary. Each OS works differently. If an user does not prefer to use Krew, but want to have directpv subcommand for kubectl, he/she knows how to make it happen. This is out of scope of this document.

Praveenrajmani · 2023-07-07T12:52:57Z

docs/installation.md

+  - quay.io/minio/directpv:latest
+* If `seccomp` is enabled, load [DirectPV seccomp profile](../seccomp.json) on nodes where you want to install DirectPV and use `--seccomp-profile` flag to `kubectl directpv install` command. For more information, refer Kubernetes documentation [here](https://kubernetes.io/docs/tutorials/clusters/seccomp/)
+* If `apparmor` is enabled, load [DirectPV apparmor profile](../apparmor.profile) on nodes where you want to install DirectPV and use `--apparmor-profile` flag to `kubectl directpv install` command. For more information, refer to the [Kubernetes documentation](https://kubernetes.io/docs/tutorials/clusters/apparmor/).
+* Enabled `ExpandCSIVolumes` [feature gate](https://kubernetes.io/docs/reference/command-line-tools-reference/feature-gates/) for [volume expansion](https://kubernetes-csi.github.io/docs/volume-expansion.html) feature.


this feature gate is enabled by default in k8s v1.16 and above. can we also specify this?

I don't think we need to specify any kubernetes version here. If this feature gate is disabled by default or admin, it is required to be enabled. That is the Prerequisites

Praveenrajmani · 2023-07-07T12:57:49Z

docs/installation.md

+#### Installing by generating DirectPV manifests
+To install using generated manifests file, run below command
+```sh
+$ curl -sfL https://github.com/minio/directpv/raw/master/docs/tools/install.sh | sh - apply


Can we use kubectl directpv install -o yaml for this because we don't need to maintain install.sh for every updates.

Technically kubectl-directpv install -o yaml is not really usable in all environments. We should discourage the usage and promote install.sh which does better job in fresh install as well as upgrade.

Praveenrajmani · 2023-07-07T13:03:55Z

docs/drive-management.md

+## Add drives
+Drives are added to DirectPV to provision volumes. This involves a two step process as shown below.
+
+1. Run `discover` command - `discover` command probes eligible drives from DirectPV nodes and stores drive information in a YAML file. You should carefully examine the YAML file and set the `select` field to `yes` or `no` value to indicate drive selection. `select` field is set to `yes` value by default. Below is an example of the `discover` command:


Can we also mention about the --drives and --nodes selector flags here? They can review drives.yaml optionally if they want. Instead, the table that we show would be useful for the review here.

Actually these flags are explained command reference guide. However reviewing drives.yaml by the user must be done even these flags are used. Because drive state might change at the time of fetching the information.

Praveenrajmani · 2023-07-07T13:05:18Z

docs/drive-management.md

+   The `init` command creates a request to add the selected drives in the YAML file generated using the `discover` command. As this process wipes out all data on the selected drives, wrong drive selection will lead to permanent data loss. Below is an example of `init` command:
+
+```sh
+$ kubectl directpv init drives.yaml


Suggested change

$ kubectl directpv init drives.yaml

$ kubectl directpv init drives.yaml --dangerous

We should not expose --dangerous flag anywhere. A copy/paste such reference would lead to data loss.

Praveenrajmani · 2023-07-07T13:08:43Z

docs/drive-management.md

+Refer to the [list drives command](./command-reference.md#drives-command) for more information.
+
+## Label drives
+Drives are labeled to set custom tagging which can be used in volume provisioning. Below is an example:


can we also specify how to control scheduling using labels here? something like https://github.com/minio/directpv/blob/master/docs/scheduling.md#volume-scheduling-based-on-drive-labels

It is covered in volume-scheduling.md#Customizing drive selection

Praveenrajmani · 2023-07-07T13:10:41Z

docs/drive-management.md

+```
+
+## Remove drives
+Drives that do not contain any volumes can be removed. Below is an example:


Can we also specify what this command does? - that it releases the drive by umounting it.

That is internal, I don't think we need to specify here.

Praveenrajmani · 2023-07-07T13:11:55Z

docs/drive-management.md

+Replace a faulty drive with a new drive on a same node. In this process, all volumes in the faulty drive are moved to the new drive then faulty drive is removed from DirectPV. Currently, DirectPV does not support moving data on the volume to the new drive. Use [replace.sh](./tools/replace.sh) script to perform drive replacement. Below is an example:
+```sh
+# Replace 'sdd' drive by 'sdf' drive on 'node1' node
+$ replace.sh sdd sdf node1


Can we provide the step by step guide as comments in the script? customers can use this incase if the script doesn't work.

I added those steps initially and removed them later. As commands have to be run conditionally, it is high chance of copy/paste error. If the script doesn't work, steps also won't work. If someone wants to understand what steps are carried out, reading release.sh script is quite straight-forward.

Praveenrajmani · 2023-07-07T13:16:02Z

docs/volume-management.md

+***CAUTION: THIS IS DANGEROUS OPERATION WHICH LEADS TO DATA LOSS***
+
+Volume can be deleted only if it is in `Ready` state (that is, no pod is using it). Run the `kubectl delete pvc` command which triggers DirectPV volume deletion. As removing a volume leads to data loss, double check what volume you are deleting. Below is an example:
+```sh


can we also explain about kubectl directpv clean command which can be used to clean state volumes? or volumes of the pods which were force deleted.

I will add another section ## Clean stale volumes.

Praveenrajmani · 2023-07-07T13:18:27Z

docs/troubleshooting.md

@@ -0,0 +1,67 @@
+# Frequently Asked Questions


can we isolate FAQs from troubleshooting?

Actually this is not FAQ. We would need a better title.

Maybe Common Issues?

Though when I copied this over to the Hugo docs, I left it a FAQs.

Let me rename the file as faq.md

Signed-off-by: Bala.FA <[email protected]>

Co-authored-by: Daryl White <[email protected]>

Signed-off-by: Bala.FA <[email protected]>

Praveenrajmani

We need to update few links in the README page. LGTM otherwise.

Signed-off-by: Bala.FA <[email protected]>

djwfyi

I don't have any additional feedback other than a possible alternative title for the FAQ page.

djwfyi · 2023-08-01T14:27:43Z

docs/troubleshooting.md

@@ -0,0 +1,67 @@
+# Frequently Asked Questions


Maybe Common Issues?

Though when I copied this over to the Hugo docs, I left it a FAQs.

Signed-off-by: Bala.FA <[email protected]>

djwfyi

LGTM

balamurugana marked this pull request as draft June 12, 2023 16:19

balamurugana force-pushed the refactor-documentations branch 13 times, most recently from 131b78c to c841cae Compare June 16, 2023 00:49

balamurugana force-pushed the refactor-documentations branch 2 times, most recently from 2bd343d to ca068c1 Compare June 23, 2023 04:22

balamurugana marked this pull request as ready for review June 23, 2023 04:29

balamurugana force-pushed the refactor-documentations branch 2 times, most recently from 5981820 to bac77e5 Compare June 24, 2023 01:48

Praveenrajmani requested review from Praveenrajmani, ravindk89 and djwfyi June 27, 2023 04:29

djwfyi requested changes Jun 28, 2023

View reviewed changes

balamurugana force-pushed the refactor-documentations branch from b926111 to d9aa14d Compare June 28, 2023 04:22

Praveenrajmani reviewed Jul 7, 2023

View reviewed changes

balamurugana force-pushed the refactor-documentations branch 4 times, most recently from ac72a90 to 2cc2837 Compare July 13, 2023 00:50

balamurugana force-pushed the refactor-documentations branch from 2cc2837 to 12081de Compare July 16, 2023 13:56

balamurugana force-pushed the refactor-documentations branch from 12081de to 736dc8e Compare July 18, 2023 12:14

balamurugana and others added 5 commits July 18, 2023 17:52

refactor documentation

065f3c7

Signed-off-by: Bala.FA <[email protected]>

Apply suggestions from code review

5ad84ef

Co-authored-by: Daryl White <[email protected]>

address review comments

ce62efb

Signed-off-by: Bala.FA <[email protected]>

address review comments

4cf5e98

Signed-off-by: Bala.FA <[email protected]>

add clean stale volumes section

4a4cd41

Signed-off-by: Bala.FA <[email protected]>

balamurugana force-pushed the refactor-documentations branch from 736dc8e to 4a4cd41 Compare July 18, 2023 12:22

djwfyi mentioned this pull request Jul 31, 2023

GitHub docs converstion minio/directpv-docs#4

Merged

Praveenrajmani previously approved these changes Aug 1, 2023

View reviewed changes

balamurugana dismissed Praveenrajmani’s stale review via a1f2bde August 1, 2023 07:48

add review comments

41eecf8

Signed-off-by: Bala.FA <[email protected]>

balamurugana force-pushed the refactor-documentations branch from a1f2bde to 41eecf8 Compare August 1, 2023 08:11

Praveenrajmani previously approved these changes Aug 1, 2023

View reviewed changes

djwfyi previously approved these changes Aug 1, 2023

View reviewed changes

address review comments

6fd27c9

Signed-off-by: Bala.FA <[email protected]>

balamurugana dismissed stale reviews from djwfyi and Praveenrajmani via 6fd27c9 August 2, 2023 01:53

Praveenrajmani approved these changes Aug 2, 2023

View reviewed changes

djwfyi approved these changes Aug 2, 2023

View reviewed changes

balamurugana merged commit c5f7e46 into minio:master Aug 2, 2023
22 checks passed

balamurugana deleted the refactor-documentations branch August 2, 2023 16:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor documentation #800

Refactor documentation #800

balamurugana commented Jun 12, 2023 •

edited

Loading

djwfyi left a comment

djwfyi Jun 27, 2023

balamurugana Jun 28, 2023

djwfyi Jun 28, 2023

balamurugana Jun 29, 2023

Praveenrajmani Jul 7, 2023

balamurugana Jul 7, 2023

Praveenrajmani Jul 7, 2023

balamurugana Jul 7, 2023

Praveenrajmani Jul 7, 2023

balamurugana Jul 7, 2023

Praveenrajmani Jul 7, 2023

balamurugana Jul 7, 2023

Praveenrajmani Jul 7, 2023

balamurugana Jul 7, 2023

Praveenrajmani Jul 7, 2023

balamurugana Jul 7, 2023

Praveenrajmani Jul 7, 2023

balamurugana Jul 7, 2023

Praveenrajmani Jul 7, 2023

balamurugana Jul 7, 2023

Praveenrajmani Jul 7, 2023

balamurugana Jul 7, 2023

Praveenrajmani Jul 7, 2023

balamurugana Jul 7, 2023

djwfyi Aug 1, 2023

balamurugana Aug 2, 2023

Praveenrajmani left a comment

djwfyi left a comment

djwfyi Aug 1, 2023

djwfyi left a comment

	Controller runs as `Deployment` Pods named `controller` which is three replicas in any Kubernetes nodes not limiting to where node server is running. In the three replicas, one instance is elected to serve requests. Each pod contains below running containers
	The Controller runs as `Deployment` Pods named `controller`, which are three replicas located in any Kubernetes nodes not limiting to where node server is running. In the three replicas, one instance is elected to serve requests. Each pod contains below running containers:

	$ kubectl directpv init drives.yaml
	$ kubectl directpv init drives.yaml --dangerous

Refactor documentation #800

Refactor documentation #800

Conversation

balamurugana commented Jun 12, 2023 • edited Loading

djwfyi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Praveenrajmani left a comment

Choose a reason for hiding this comment

djwfyi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

djwfyi left a comment

Choose a reason for hiding this comment

balamurugana commented Jun 12, 2023 •

edited

Loading