Skip to content

Update documentation for v2 release #276

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged

Conversation

alexander-ding
Copy link
Contributor

What type of PR is this?

Uncomment only one /kind <> line, hit enter to put that in a new line, and remove leading whitespaces from that line:

/kind api-change
/kind bug
/kind cleanup
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake

What this PR does / why we need it:

Which issue(s) this PR fixes:

Part of #243.

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

/cc @mauriciopoppe

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/documentation Categorizes issue or PR as related to documentation. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Oct 25, 2022
@@ -0,0 +1,137 @@
# Usage
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for creating this doc! I think it could be part of README, what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit long and has a lot of nuanced details re. containerd and HostProcess containers. I think it might be easier to keep this in a separate doc and point to it. I like to keep READMEs a hub of pointers to relevant information.

docs/USAGE.md Outdated
- [FileSystem](/pkg/filesystem/)
- [iSCSI](/pkg/iscsi/) (experimental)
- [SMB](/pkg/smb/)
- [System](/pkg/system/)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this also experimental? I remember that there were 2 API Groups that didn't move from alpha in v1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. System is in alpha. Also, FileSystem and Volume are both v2alpha1, but we don't really have to consider them experimental, since we can just bump Go versions for API changes.
For System, at least we have tests running. I will also mark System as experimental.

docs/USAGE.md Outdated
image: k8s.gcr.io/sig-storage/csi-node-driver-registrar:v2.1.0
args:
- --v=5
- --csi-address=unix://c:\var\lib\kubelet\plugins\pd.csi.storage.gke.io\csi.sock
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's use placeholders for the CSI Driver instead of using PD CSI Driver as an example

docs/USAGE.md Outdated

Using HostProcess pods have a few important consequences on the deployed containers, as noted in the HostProcess docs.
- HostProcess containers have no file system or resource isolation, so they have a complete view of the host machine’s file system.
This means that paths passed in **arguments need to be complete host machine paths**.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This means that paths passed in **arguments need to be complete host machine paths**.
Paths in the PodSpec must be absolute paths with respect to the host.

docs/USAGE.md Outdated
On the other hand, depending on the containerd and the Windows OS version, Kubernetes volume mounts are either mounted relative to a subdirectory of the host machine specified by an environment variable `%CONTAINER_SANDBOX_MOUNT_POINT%` or **mounted relative to the host process root**. See [HostProcess Caveats](#hostprocess-caveats).
- HostProcess pods can only contain HostProcess containers.
Often, the CSI node registrar is deployed in the same pod as the driver, so file paths for both containers need to be updated.
- Named pipes and Unix domain sockets are not supported and should be accessed via their path on the host.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Named pipes and Unix domain sockets are not supported and should be accessed via their path on the host.
- Named pipes and Unix domain sockets are not supported and should be accessed via their absolute path with respect to the host.

@mauriciopoppe
Copy link
Member

/lgtm
/approve

Thanks!

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 26, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alexander-ding, mauriciopoppe

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:

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 k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 26, 2022
@k8s-ci-robot k8s-ci-robot merged commit 2ae32e6 into kubernetes-csi:library-development Oct 26, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/documentation Categorizes issue or PR as related to documentation. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants