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

only fisrt backup success, second or more failed #1270

Closed
liuxu623 opened this issue Aug 31, 2019 · 3 comments · Fixed by #1271
Closed

only fisrt backup success, second or more failed #1270

liuxu623 opened this issue Aug 31, 2019 · 3 comments · Fixed by #1271
Assignees
Labels
Milestone

Comments

@liuxu623
Copy link

liuxu623 commented Aug 31, 2019

Consul Template version

v0.21.1

Configuration

backup = true

Debug output

2019/08/31 09:11:40 [WARN] (runner) could not backup "/var/folders/xs/bc89864j0jlb53ty5lz6803m0000gn/T/125157687/724165418": link /var/folders/xs/bc89864j0jlb53ty5lz6803m0000gn/T/125157687/724165418 /var/folders/xs/bc89864j0jlb53ty5lz6803m0000gn/T/125157687/724165418.bak: file exists

Expected behavior

backup success

Actual behavior

backup failed

Steps to reproduce

References

@eikenb
Copy link
Contributor

eikenb commented Aug 31, 2019

Thanks for the report @liuxu623, I'll get right on this.

@eikenb eikenb added the bug label Aug 31, 2019
@eikenb eikenb added this to the 0.21.2 milestone Aug 31, 2019
@eikenb eikenb self-assigned this Aug 31, 2019
@eikenb
Copy link
Contributor

eikenb commented Aug 31, 2019

Thought this had test coverage, but it didn't. Sorry about that.

I've written a test and am debating the best fix. The most obvious seems to be to just remove the .bak version before doing the link but I'm taking a few to consider my options.

@eikenb
Copy link
Contributor

eikenb commented Aug 31, 2019

I'm thinking the most robust solution will be to os.Rename .bak to .old.bak, then do the os.Link then, if the link is successful, os.Remove .old.bak. Each step is then atomic and won't break anything if one backup fails and won't lose the backup file if it fails (the .old.bak will hang around until a successful backup is made).

eikenb added a commit that referenced this issue Aug 31, 2019
Fix a regression with backup option such that it would only back up once
(os.Link doesn't overwrite). Also added a test to reproduce issue.

Fixes #1270
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants