Skip to content
This repository has been archived by the owner on May 11, 2024. It is now read-only.

Update user docs for using valid server and path during PVC creation #45

Merged
merged 2 commits into from
May 11, 2018
Merged

Update user docs for using valid server and path during PVC creation #45

merged 2 commits into from
May 11, 2018

Conversation

ashahba
Copy link
Member

@ashahba ashahba commented May 11, 2018

Since Kubernetes does not validate server or path during PVC creation, we don't want KVC to get into business of putting workarounds for that.
But we think it is nice to bring this to our users attention so that they verify the servers prior to using KVC and PVCs.

@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: elsonrodriguez

Assign the PR to them by writing /assign @elsonrodriguez in a comment when ready.

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
Copy link

Hi @ashahba. Thanks for your PR.

I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Ajay191191
Ajay191191 suggested changes May 11, 2018
View reviewed changes
Copy link
Contributor

@Ajay191191 Ajay191191 left a comment

Choose a reason for hiding this comment

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

I think we can just keep it short.

docs/user.md Outdated
@@ -282,6 +282,13 @@ source types is given below.
persistentVolumeClaim:
claimName: kvc-resource-a150fd63-11c4-11e8-8397-0a580a440340
```
### Caveats ###
Please keep in mind that during Persistent Volume creation, Kubernetes does not validate the `server` or `path` provided by configuration file and for that reason `KVC` follows the same practice.
Copy link
Contributor

Choose a reason for hiding this comment

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

We can just say here: The NFS server ip and path are not validated, so please ensure that the servers are routable and paths are valid prior to the creation of the VolumeManager CR.

@ashahba
Copy link
Member Author

ashahba commented May 11, 2018

@Ajay191191 if you feel this is till to long, we can shorten it, but often times people skip reading docs until they run into issues. Chances are a single line around event could be priceless at that point. 🙂

Ajay191191
Ajay191191 approved these changes May 11, 2018
View reviewed changes
Copy link
Contributor

@Ajay191191 Ajay191191 left a comment

Choose a reason for hiding this comment

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

LGTM

@Ajay191191 Ajay191191 merged commit 643b072 into intel:master May 11, 2018
@ashahba ashahba deleted the ashahba/tests-e2e-docs branch May 11, 2018 23:10
@ashahba ashahba mentioned this pull request May 11, 2018
Closed
@ashahba
Copy link
Member Author

ashahba commented May 11, 2018

This also fixes #18

@ashahba ashahba mentioned this pull request May 15, 2018
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Ajay191191 Ajay191191 Ajay191191 approved these changes

@jose5918 jose5918 Awaiting requested review from jose5918

Assignees
No one assigned
Labels
needs-ok-to-test size/XS
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ashahba @k8s-ci-robot @Ajay191191