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.

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

Update Pre/Post hooks example. #795

Merged
merged 4 commits into from
Sep 5, 2018
Merged

Conversation

wwitzel3
Copy link
Contributor

@wwitzel3 wwitzel3 commented Aug 27, 2018

Updates our pre/post hook docs and extends it with an fsfreeze example.

Signed-off-by: Wayne Witzel III <[email protected]>
Copy link
Contributor

@carlisia carlisia left a comment

Choose a reason for hiding this comment

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

This lgtm. I made super minor wording change suggestions. 👍

docs/hooks.md Outdated
The Ark [example/nginx-app/with-pv.yaml][2] serves as an example of adding the pre and post hook annotations directly
to your declarative deployment. Below is an example of what updating an object in place might look like.

Place Ark in to restore only mode. By placing Ark in restore only mode you prevent Ark backups from running
Copy link
Contributor

Choose a reason for hiding this comment

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

The to here is extraneous.

docs/hooks.md Outdated

Place Ark in to restore only mode. By placing Ark in restore only mode you prevent Ark backups from running
while you are adding the annotations and avoid the condition where the pre hook freezes the file system, but
there is no post hook setup to unfreeze the file system.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/the file system/it

docs/hooks.md Outdated
The Ark [example/nginx-app/with-pv.yaml][2] serves as an example of adding the pre and post hook annotations directly
to your declarative deployment. Below is an example of what updating an object in place might look like.

Place Ark in to restore only mode. By placing Ark in restore only mode you prevent Ark backups from running
Copy link
Contributor

Choose a reason for hiding this comment

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

s/By placing Ark in restore only mode you prevent/This will prevent

docs/hooks.md Outdated
kubectl annotate pod -n nginx-example -l app=nginx post.hook.backup.ark.heptio.com/command='["/sbin/fsfreeze", "--unfreeze", "/var/log/nginx"]'
kubectl annotate pod -n nginx-example -l app=nginx post.hook.backup.ark.heptio.com/containr=fsfreeze

Finally remove Ark from restore only mode and test the pre and post hooks by creating a backup. You can use the Ark logs to verify that the pre and post
Copy link
Contributor

Choose a reason for hiding this comment

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

s/Finally/Finally,

docs/hooks.md Outdated

Now you patch your deployment with the required annotations.

kubectl annotate pod -n nginx-example -l app=nginx pre.hook.backup.ark.heptio.com/command='["/sbin/fsfreeze", "--freeze", "/var/log/nginx"]'
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these commands should be wrapper in triple backticks, to make them code snippets.

docs/hooks.md Outdated
Now you patch your pod with the required annotations.

```shell
kubectl annotate pod -n nginx-example -l app=nginx pre.hook.backup.ark.heptio.com/command='["/sbin/fsfreeze", "--freeze", "/var/log/nginx"]'
Copy link
Contributor

Choose a reason for hiding this comment

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

You could apply all of these annotations in a single kubectl call, e.g. kubectl annotate pod a=b c=d ... -- which I think would remove the need to flip ark into restore-only mode and back. I think that'd be preferable since although I get the restore-only mode thing, it distracts a little from the main point

@wwitzel3
Copy link
Contributor Author

wwitzel3 commented Aug 27, 2018

This probably addresses #245 as well.

docs/hooks.md Outdated
kubectl annotate pod -n nginx-example -l app=nginx post.hook.backup.ark.heptio.com/containr=fsfreeze
kubectl annotate pod -n nginx-example -l app=nginx \
pre.hook.backup.ark.heptio.com/command='["/sbin/fsfreeze", "--freeze", "/var/log/nginx"]' \
pre.hook.backup.ark.heptio.com/containr=fsfreeze \
Copy link
Contributor

Choose a reason for hiding this comment

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

s/containr/container

docs/hooks.md Outdated
pre.hook.backup.ark.heptio.com/command='["/sbin/fsfreeze", "--freeze", "/var/log/nginx"]' \
pre.hook.backup.ark.heptio.com/containr=fsfreeze \
post.hook.backup.ark.heptio.com/command='["/sbin/fsfreeze", "--unfreeze", "/var/log/nginx"]' \
post.hook.backup.ark.heptio.com/containr=fsfreeze
Copy link
Contributor

Choose a reason for hiding this comment

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

s/containr/container

docs/hooks.md Outdated
[1]: api-types/backup.md
[2]: https://github.com/heptio/ark/blob/master/examples/nginx-app/with-pv.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

just use examples/nginx-app/with-pv.yaml -- makes things work properly with GH pages/multiple versions of the docs

@skriss
Copy link
Contributor

skriss commented Aug 29, 2018

otherwise I think LGTM

Signed-off-by: Wayne Witzel III <[email protected]>
@nrb
Copy link
Contributor

nrb commented Sep 5, 2018

Merging since there's an LGTM and the other comments appear to have been addressed.

@nrb nrb merged commit 5ccc27a into vmware-tanzu:master Sep 5, 2018
nrb pushed a commit to nrb/velero that referenced this pull request Sep 7, 2018
Rolls up changes from vmware-tanzu#795 and vmware-tanzu#823

Signed-off-by: Nolan Brubaker <[email protected]>
nrb pushed a commit to nrb/velero that referenced this pull request Sep 7, 2018
This change puts docs from vmware-tanzu#795 and vmware-tanzu#823 on the live docs site.

Signed-off-by: Nolan Brubaker <[email protected]>
@nrb nrb mentioned this pull request Sep 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants