Skip to content
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.

Sign up for GitHub

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

Jump to bottom

added precisions about snapshotting #1536

Merged
merged 1 commit into from
Sep 29, 2020
Merged

added precisions about snapshotting #1536

merged 1 commit into from
Sep 29, 2020

Conversation

facchettos
Copy link
Contributor

@facchettos facchettos commented Sep 28, 2020 •
edited by nixpanic
Loading

Describe what this PR does

added precisions about snapshotting

#Provide any external context for the change, if any.

provides some precisions about best practices for snapshotting as given in ceph docs and kuebernetes docs

Related issues

Closes: #1532

@facchettos
Copy link
Contributor Author

@Madhu-1

pkalever
pkalever reviewed Sep 28, 2020
View reviewed changes
docs/cephfs-snap-clone.md Outdated Show resolved Hide resolved
examples/README.md Outdated Show resolved Hide resolved
@facchettos facchettos force-pushed the master branch 7 times, most recently from d965f7c to 160abed Compare September 28, 2020 21:03
Madhu-1
Madhu-1 reviewed Sep 29, 2020
View reviewed changes
docs/cephfs-snap-clone.md Outdated
@@ -35,6 +35,7 @@ kubectl create -f ../examples/cephfs/snapshotclass.yaml

The snapshot is created on/for an existing PVC. You should
have a PVC in bound state before creating snapshot from it.
It is recommended to create a snapshot only when the PV is not in use.
Copy link
Collaborator

Choose a reason for hiding this comment

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

you also need to add it for clone also. same applies for rbd also

Suggested change
It is recommended to create a snapshot only when the PV is not in use.
It is recommended to create a volume snapshot/PVC clone only when the PVC is not in use.

@Madhu-1
Copy link
Collaborator

Madhu-1 commented Sep 29, 2020

mdl --style scripts/mdl-style.rb ./docs/cephfs-snap-clone.md
./docs/cephfs-snap-clone.md:38: MD009 Trailing spaces
A detailed description of the rules is available at https://github.com/markdownlint/markdownlint/blob/master/docs/RULES.md

CI failure error. https://github.com/ceph/ceph-csi/blob/master/docs/development-guide.md#running-ceph-csi-tests-in-a-container will help you to test it locally

@nixpanic nixpanic added the component/docs Issues and PRs related to documentation label Sep 29, 2020
@nixpanic
Copy link
Member

Please note that there is a commitlint check (part of continuous-integration/travis-ci/pr) that requires proper formatted commit messages. Please create a single commit (use git rebase and squash all commits together), update the commit message to have a subject prefix of doc: and add your Signed-off-by: line (use git commit --amend) and git push --force ... the changes to the branch for this PR.

You can also run ci/centos jobs (or a standalone commitlint) by leaving a comment like:

/retest ci/centos/containerized-tests

@nixpanic
Copy link
Member

/retest commitlint

@facchettos facchettos force-pushed the master branch 4 times, most recently from 90a3599 to 9bcc606 Compare September 29, 2020 07:32
Madhu-1
Madhu-1 reviewed Sep 29, 2020
View reviewed changes
docs/cephfs-snap-clone.md Outdated
@@ -35,6 +35,8 @@ kubectl create -f ../examples/cephfs/snapshotclass.yaml

The snapshot is created on/for an existing PVC. You should
have a PVC in bound state before creating snapshot from it.
It is recommended to create a volume snapshot or a PVCclone
Copy link
Collaborator

Choose a reason for hiding this comment

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

PVC clone

@nixpanic
Copy link
Member

Thanks for the updated PR! Just one space needed between PVCclone as Madhu commented. Looks good otherwise.

@facchettos
Copy link
Contributor Author

Sorry it took so long to get it right guys

@Madhu-1
Copy link
Collaborator

Madhu-1 commented Sep 29, 2020

Sorry it took so long to get it right guys

Not at all a problem. the ceph-csi team will always be happy to help users

@Madhu-1 Madhu-1 requested a review from nixpanic September 29, 2020 09:04
@Madhu-1 Madhu-1 added the ready-to-merge This PR is ready to be merged and it doesn't need second review (backports only) label Sep 29, 2020
@Madhu-1
Copy link
Collaborator

Madhu-1 commented Sep 29, 2020

/retest continuous-integration/travis-ci/pr

I have restarted the Travis CI

@humblec
Copy link
Collaborator

humblec commented Sep 29, 2020

Thanks @facchettos 👍

@humblec humblec added this to the release-3.2.0 milestone Sep 29, 2020
@mergify mergify bot merged commit b9dd54e into ceph:master Sep 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Madhu-1 Madhu-1 Madhu-1 approved these changes

@pkalever pkalever pkalever approved these changes

@humblec humblec humblec approved these changes

@nixpanic nixpanic Awaiting requested review from nixpanic

Assignees
No one assigned
Labels
component/docs Issues and PRs related to documentation ready-to-merge This PR is ready to be merged and it doesn't need second review (backports only)
Projects
None yet
Milestone
release-3.2.0
Development

Successfully merging this pull request may close these issues.

Do snapshots freeze fs before snapshotting ?
5 participants
@facchettos @Madhu-1 @nixpanic @humblec @pkalever