example.md: reduce the contents which are present in hostpath deployment example #125

okartau · 2019-03-07T20:09:34Z

This page was mostly repeating (older version) of what is written in hostpath
deployment example. We can point to hostpath and delete
the outdated deployment examples here.

Resolves: #124

saad-ali · 2019-03-07T23:50:20Z

/assign @msau42

msau42 · 2019-03-08T02:21:07Z

book/src/example.md

-[HostPath](https://github.com/kubernetes-csi/drivers/tree/master/pkg/hostpath)
-can be used to provision local storage in a single node test. This
-section shows how to deploy and use that driver in Kubernetes.
+[HostPath](https://github.com/kubernetes-csi/csi-driver-host-path)


"The Hostpath CSI driver is a simple sample driver that provisions a directory on the host. It can be used as an example to get started writing a driver, however it is not meant for production use."

yes, I will replace the sentence, and overall I think the first part can be simplified. The header "Deployment" should go away, and remaining sentences get reduced and form one chapter

it may be better idea to link to deploy/ directory instead of deployment script, as the whole set of files under deploy/ forms this example

msau42 · 2019-03-08T02:22:10Z

book/src/example.md

-can be used to provision local storage in a single node test. This
-section shows how to deploy and use that driver in Kubernetes.
+[HostPath](https://github.com/kubernetes-csi/csi-driver-host-path)
+can be used to provision local storage. The deployment example there


"deployment example there": can you link to the actual page?

I will make "deployment example there" to be a link to actual deployment script, if that's what you mean

msau42 · 2019-03-08T02:22:51Z

book/src/example.md

-> ```
-
-### Deploy driver-registrar and hostpath CSI plugin in DaemonSet pod
+CRDs are created optionally `CSIDriverRegistry` and `CSINodeInfo`:


I don't know if we need this anymore considering we're getting rid of the CRDs in 1.14.

Let's remove CRD stmt from here, it does not add much value, glimpse into deployment example shows CRDs are inside if stmt, thus optional

okartau · 2019-03-09T13:05:10Z

force-pushed latest state. Link to example now points to hostpath deploy/ dir.
What do you think, do we need to keep stmt "The CSI sidecar apps are going to connect..." ?

liveness-probe deployment example to be moved from here to hostpath deployment script
(needs deployment verification, I will try)
remaining snapshot example should be moved there as well
then those parts deleted from here

okartau · 2019-03-10T21:03:46Z

liveness probe deployment added as PR#27 in hostpath repo

msau42 · 2019-03-20T02:13:06Z

book/src/example.md

-> csi-hostpath-provisioner-0   1/1     Running   0          56s
-> csi-hostpathplugin-4k7hk     2/2     Running   0          117s
-> ```
+The CSI sidecar apps are going to connect to the CSI driver, therefore starting it first helps avoid timeouts and intermittent container restarts


I'm not sure what this sentence means and how it translates into something you can do in your Pod specs. Can you clarify?

Put the CSI driver container at the top of the list of containers in the pod, if that pod also contains the sidecards.

It's less of an issue now because we improved the connection behavior of the sidecars, but previously we've seen sidecars time out in their connection attempt because the CSI driver was started after them and image pulling took too long. Having said that, I still think it's a good advice.

This is an advice about best-practices. Thinking about it some more, I now suggest that we remove the entire sentence. It doesn't belong into the example page.

I deleted that sentence and pushed new state

msau42 · 2019-03-20T02:13:57Z

book/src/example.md

-> Events:      <none>
-> ```
-
 ## Snapshot support


@xing-yang do you want the snapshot example here, or should it go in the snapshot page and/or the snapshotter repo?

I would put it into the csi-driver-host-path repo.

this page: can get out of sync with the actual example(s) - can be more than one, depending on the Kubernetes version

snapshot page: same issue

snapshotter repo: doesn't have a complete deployment, and I don't think we should add one there

We discussed this today in the Kubernetes-CSI meeting and agreed to use the csi-driver-host-path repo.

okartau · 2019-03-27T20:51:09Z

The current PR state has addressed latest comments, right.
Next steps to get this moving further, a plan to complete this PR using isolated changes steps:

get PR#27 in hostpath repo merged, adding liveness-probe in deploy example there
after above merged, I will delete liveness-probe part in this PR here
then we can deal with moving remaining snapshot example parts from here into hostpath example as well, in similar fashion. i.e. preparing PR there, merging it, then deleting respective parts in this PR here.

This page was mostly repeating what is written in hostpath deployment example. We can point to hostpath and delete the outdated deployment examples here.

okartau · 2019-03-28T09:31:29Z

book/src/example.md

- `--connection-timeout` - specifies the timeout duration of waiting for CSI driver socket in seconds. (default 30s)
-
- `--health-port` - specifies the TCP ports for listening healthz requests (default "9808")
+The example deployment includes livenessprobe side-container provided by the CSI community, using configuration derived from [using-livenessprobe](https://github.com/kubernetes-csi/livenessprobe#using-livenessprobe) from [kubernetes-csi/livenessprobe](https://github.com/kubernetes-csi/livenessprobe)


I re-used sentence from original, but the double "from" feels complicated here.
The second "from" just repeats the repository part of first "from", so what about we delete 2nd as it does not add new information.
Adding 2nd link would make sense if it pointed to some other repository.

supported by not from @pohly, I deleted 2nd "from" part and force pushed

Livenessprobe deployment was added to hostpath example, so we replace detailed description here with a note about it.

okartau · 2019-03-29T13:07:18Z

I created a kubernetes-csi/csi-driver-host-path#31 that makes next step, migrates rest of snapshot example from here to hostpath deployment example

okartau · 2019-04-02T06:58:07Z

kubernetes-csi/csi-driver-host-path#31 was merged, so I pushed a new state here that replaces snapshot example contents with one sentence (please check it).

Note that there is now the last sentence "please check the Troubleshooting page" which has broken link.
Do we want to have such link there (should fix it) or can we just delete this sentence now,
as we have moved away actual possibly trouble-creating parts from this page

pohly · 2019-04-02T09:37:27Z

Please remove the sentence "Deployment has been tested with Kubernetes v1.13". This page no longer documents that.

Note that there is now the last sentence "please check the Troubleshooting page" which has broken link.

The case must be fixed (Troubleshooting.html -> troubleshooting.html). The page itself isn't very useful anymore, but let's fix the link and then decide later whether that page still makes sense.

okartau · 2019-04-02T10:11:31Z

1.13 sentence deleted and case in troubleshooting fixed (another fix is .md instead of .html), pushed.
(most of) troubleshooting file entries are getting old and/or are not relevant?

pohly · 2019-04-02T11:05:53Z

another fix is .md instead of .html

The .html suffix was correct, it's what we need in the actual site (https://kubernetes-csi.github.io/docs/example.html). Please revert to .html.

okartau · 2019-04-02T11:21:38Z

oh, I naively hoped I can verify link here as well. Reverted-

pohly · 2019-04-02T11:30:55Z

book/src/example.md


-> ## *This page is out-of-date and under active development.*
+The [Hostpath CSI driver](https://github.com/kubernetes-csi/csi-driver-host-path) is a simple sample driver that provisions a directory on the host. It can be used as an example to get started writing a driver, however it is not meant for production use.
+The [deployment example](https://github.com/kubernetes-csi/csi-driver-host-path/tree/master/deploy) shows how to deploy and use that driver in Kubernetes.


Sorry, only thought of this now: https://github.com/kubernetes-csi/csi-driver-host-path#deployment is a better link, because it has some actual documentation.

good idea! pointing to deployment chapter is more user-friendly than pointer to directory

pohly

/lgtm

Please remove the "WIP:" tag in the title.

msau42 · 2019-04-02T16:49:51Z

book/src/example.md

-This example deployment uses the original RBAC rule files that are
-maintained together with those sidecar apps and deploys into the
-default namespace.
+The example deployment includes livenessprobe side-container provided by the CSI community, using configuration derived from [using-livenessprobe](https://github.com/kubernetes-csi/livenessprobe#using-livenessprobe).


Do we need to explicitly call out liveness probe and snapshot? We're not calling out any of the other sidecars.

current structure is result of mechanically replacing one chapter with one-two sentences about it. Other sidecars did not have dedicated chapters.

let's leave them out then. The new docs page has separate pages for each of the sidecars that explain what they do.

we leave out livenessprobe and snapshot sentences, and we keep RBAC chapter?

yup I think that is fine

pushed it. The remaining text is not long :-)

Snapshot deployment was added to hostpath example, so we replace detailed description here with a note about it.

msau42 · 2019-04-03T18:56:13Z

/lgtm
/approve
Thank you!

k8s-ci-robot · 2019-04-03T18:56:17Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: msau42, okartau

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [msau42]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 7, 2019

k8s-ci-robot requested review from lpabon and msau42 March 7, 2019 20:09

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Mar 7, 2019

k8s-ci-robot assigned msau42 Mar 7, 2019

msau42 reviewed Mar 8, 2019

View reviewed changes

okartau force-pushed the update-hostpath-in-example branch from 09af201 to b55adec Compare March 9, 2019 12:48

msau42 reviewed Mar 20, 2019

View reviewed changes

okartau force-pushed the update-hostpath-in-example branch from b55adec to 33c7b81 Compare March 27, 2019 20:37

okartau mentioned this pull request Mar 27, 2019

Added liveness probe to deployment example manifest kubernetes-csi/csi-driver-host-path#27

Merged

example.md: reduce the contents present in hostpath deployment

13dc6fa

This page was mostly repeating what is written in hostpath deployment example. We can point to hostpath and delete the outdated deployment examples here.

okartau force-pushed the update-hostpath-in-example branch from 33c7b81 to df4ab3e Compare March 28, 2019 09:21

okartau commented Mar 28, 2019

View reviewed changes

Reduce the contents continues: delete livenessprobe details

970ee44

Livenessprobe deployment was added to hostpath example, so we replace detailed description here with a note about it.

okartau force-pushed the update-hostpath-in-example branch from df4ab3e to 970ee44 Compare March 29, 2019 08:23

okartau force-pushed the update-hostpath-in-example branch 2 times, most recently from 41820e4 to 1aa545a Compare April 2, 2019 10:09

okartau force-pushed the update-hostpath-in-example branch from 1aa545a to 47e10f6 Compare April 2, 2019 11:18

pohly reviewed Apr 2, 2019

View reviewed changes

okartau force-pushed the update-hostpath-in-example branch from 47e10f6 to 353e061 Compare April 2, 2019 11:37

k8s-ci-robot assigned pohly Apr 2, 2019

pohly approved these changes Apr 2, 2019

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 2, 2019

okartau changed the title ~~WIP: example.md: reduce the contents which are present in hostpath deployment example~~ example.md: reduce the contents which are present in hostpath deployment example Apr 2, 2019

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 2, 2019

msau42 reviewed Apr 2, 2019

View reviewed changes

Reduce the contents continues: delete snapshotter details

b90fe17

Snapshot deployment was added to hostpath example, so we replace detailed description here with a note about it.

okartau force-pushed the update-hostpath-in-example branch from 353e061 to b90fe17 Compare April 3, 2019 18:43

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 3, 2019

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 3, 2019

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 3, 2019

k8s-ci-robot merged commit 110683a into kubernetes-csi:master Apr 3, 2019

example.md: reduce the contents which are present in hostpath deployment example #125

example.md: reduce the contents which are present in hostpath deployment example #125

Uh oh!

Conversation

okartau commented Mar 7, 2019

Uh oh!

saad-ali commented Mar 7, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

okartau Mar 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

okartau commented Mar 9, 2019

Uh oh!

okartau commented Mar 10, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

okartau commented Mar 27, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

okartau commented Mar 29, 2019

Uh oh!

okartau commented Apr 2, 2019

Uh oh!

pohly commented Apr 2, 2019

Uh oh!

okartau commented Apr 2, 2019

Uh oh!

pohly commented Apr 2, 2019

Uh oh!

okartau commented Apr 2, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pohly left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

msau42 commented Apr 3, 2019

okartau Mar 9, 2019 •

edited

Loading