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

feat: Add backup option for cosmovisor #9652

Merged
merged 27 commits into from
Aug 5, 2021
Merged

feat: Add backup option for cosmovisor #9652

merged 27 commits into from
Aug 5, 2021

Conversation

anilcse
Copy link
Collaborator

@anilcse anilcse commented Jul 8, 2021 •
edited
Loading

Description

Ref: #9616 (comment)

depends: #8590

This PR adds a full backup option for cosmovisor.
UNSAFE_SKIP_BACKUP is an env setting introduced newly.

  • if false (default, recommended), cosmovisor will try to take backup and then upgrade. In case of failure while taking backup, it will just halt the process there and won't try the upgrade.
  • If true, the cosmovisor will try to upgrade without any backup. This setting makes it hard to recover from a failed upgrade. Node operators either need to sync from a healthy node or use a snapshot from others.

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • included the correct type prefix in the PR title
  • added ! to the type prefix if API or client breaking change
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • followed the guidelines for building modules
  • included the necessary unit and integration tests
  • added a changelog entry to CHANGELOG.md
  • included comments for documenting Go code
  • updated the relevant documentation or specification
  • reviewed "Files changed" and left comments if necessary
  • confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed ! in the type prefix if API or client breaking change
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic
  • reviewed API design and naming
  • reviewed documentation is accurate
  • reviewed tests and test coverage
  • manually tested (if applicable)

@github-actions github-actions bot added the C:Cosmovisor Issues and PR related to Cosmovisor label Jul 8, 2021
amaury1093
amaury1093 reviewed Jul 8, 2021
View reviewed changes
cosmovisor/README.md Outdated Show resolved Hide resolved
ValarDragon
ValarDragon reviewed Jul 9, 2021
View reviewed changes
cosmovisor/README.md Outdated Show resolved Hide resolved
@anilcse anilcse changed the base branch from master to robert/cosmvisor-file-watch July 14, 2021 18:48
@okwme okwme mentioned this pull request Jul 26, 2021
Merged
@robert-zaremba robert-zaremba added this to the cosmovisor v1.0 milestone Jul 29, 2021
@robert-zaremba robert-zaremba mentioned this pull request Jul 29, 2021
Merged
9 tasks
@anilcse anilcse changed the base branch from robert/cosmvisor-file-watch to master July 29, 2021 18:11
@anilcse anilcse marked this pull request as ready for review July 29, 2021 20:02
ValarDragon
ValarDragon reviewed Jul 29, 2021
View reviewed changes
cosmovisor/README.md Outdated Show resolved Hide resolved
ValarDragon
ValarDragon reviewed Jul 29, 2021
View reviewed changes
cosmovisor/process.go Outdated Show resolved Hide resolved
robert-zaremba
robert-zaremba approved these changes Jul 30, 2021
View reviewed changes
Copy link
Collaborator

@robert-zaremba robert-zaremba left a comment

Choose a reason for hiding this comment

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

pre-approving.

  • we must use filepath.Join rather than : "/"

cosmovisor/README.md Outdated Show resolved Hide resolved
cosmovisor/args.go Outdated Show resolved Hide resolved
cosmovisor/process.go Outdated Show resolved Hide resolved
cosmovisor/process.go Outdated Show resolved Hide resolved
@robert-zaremba robert-zaremba mentioned this pull request Jul 30, 2021
Closed
4 tasks
ValarDragon
ValarDragon approved these changes Jul 30, 2021
View reviewed changes
Copy link
Contributor

@ValarDragon ValarDragon left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM! Glad that this is happening!

anilcse and others added 3 commits July 30, 2021 23:50
@anilcse
review suggestions
3c431f1
@anilcse @robert-zaremba
Update cosmovisor/README.md
2e5b6e5 
Co-authored-by: Robert Zaremba <robert@zaremba.ch>
@anilcse @robert-zaremba
Update cosmovisor/process.go
0ccef46 
Co-authored-by: Robert Zaremba <robert@zaremba.ch>
ryanchristo
ryanchristo approved these changes Jul 30, 2021
View reviewed changes
Copy link
Contributor

@ryanchristo ryanchristo left a comment

Choose a reason for hiding this comment

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

tACK

@aaronc aaronc added the A:automerge Automatically merge PR once all prerequisites pass. label Aug 2, 2021
@aaronc
Copy link
Member

aaronc commented Aug 2, 2021

Looks like there are test failures. Can someone take a look?

@anilcse
Copy link
Collaborator Author

anilcse commented Aug 2, 2021

Looks like there are test failures. Can someone take a look?

I asked @robert-zaremba for some help, he is looking into it

@mergify mergify bot merged commit 5a47154 into master Aug 5, 2021
@mergify mergify bot deleted the anil/add_backup_option branch August 5, 2021 20:00
mergify bot pushed a commit that referenced this pull request Aug 11, 2021
@robert-zaremba
feat: file watcher for cosmovisor (#8590)
13559f9 
Adding upgrade file watcher for cosmovisor.

Currently the comswisor upgrade mechanism relays on parsing log messages. This is not reliable:
+ depends on the log level output (x/upgrade uses INFO)
+ can be hacked by accidentally logging user user content
+ can be broken by using upgrade name which will break the regex pattern.

closes: #7703
closes: #8523
closes: #8651
closes: #8793
closes: #8964 

**Depends on**: 
- #9652

---

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

- [ ] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
- [ ] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules/structure.md).
- [ ] Wrote unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [ ] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`)
- [ ] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code).
- [ ] Added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md`
- [ ] Re-reviewed `Files changed` in the Github PR explorer
- [ ] Review `Codecov Report` in the comment section below once CI passes
RiccardoM pushed a commit to desmos-labs/cosmos-sdk that referenced this pull request Nov 2, 2021
@robert-zaremba @RiccardoM
feat: file watcher for cosmovisor (cosmos#8590)
5bede3a 
Adding upgrade file watcher for cosmovisor.

Currently the comswisor upgrade mechanism relays on parsing log messages. This is not reliable:
+ depends on the log level output (x/upgrade uses INFO)
+ can be hacked by accidentally logging user user content
+ can be broken by using upgrade name which will break the regex pattern.

closes: cosmos#7703
closes: cosmos#8523
closes: cosmos#8651
closes: cosmos#8793
closes: cosmos#8964

**Depends on**:
- cosmos#9652

---

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

- [ ] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
- [ ] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules/structure.md).
- [ ] Wrote unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [ ] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`)
- [ ] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code).
- [ ] Added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md`
- [ ] Re-reviewed `Files changed` in the Github PR explorer
- [ ] Review `Codecov Report` in the comment section below once CI passes

(cherry picked from commit 13559f9)
leobragaz pushed a commit to desmos-labs/cosmos-sdk that referenced this pull request Apr 5, 2022
@robert-zaremba @leobragaz
feat: file watcher for cosmovisor (cosmos#8590)
060d091 
Adding upgrade file watcher for cosmovisor.

Currently the comswisor upgrade mechanism relays on parsing log messages. This is not reliable:
+ depends on the log level output (x/upgrade uses INFO)
+ can be hacked by accidentally logging user user content
+ can be broken by using upgrade name which will break the regex pattern.

closes: cosmos#7703
closes: cosmos#8523
closes: cosmos#8651
closes: cosmos#8793
closes: cosmos#8964

**Depends on**:
- cosmos#9652

---

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

- [ ] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
- [ ] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules/structure.md).
- [ ] Wrote unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [ ] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`)
- [ ] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code).
- [ ] Added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md`
- [ ] Re-reviewed `Files changed` in the Github PR explorer
- [ ] Review `Codecov Report` in the comment section below once CI passes

(cherry picked from commit 13559f9)
RiccardoM pushed a commit to desmos-labs/cosmos-sdk that referenced this pull request Oct 25, 2022
@robert-zaremba @RiccardoM
feat: file watcher for cosmovisor (cosmos#8590)
447987e 
Adding upgrade file watcher for cosmovisor.

Currently the comswisor upgrade mechanism relays on parsing log messages. This is not reliable:
+ depends on the log level output (x/upgrade uses INFO)
+ can be hacked by accidentally logging user user content
+ can be broken by using upgrade name which will break the regex pattern.

closes: cosmos#7703
closes: cosmos#8523
closes: cosmos#8651
closes: cosmos#8793
closes: cosmos#8964

**Depends on**:
- cosmos#9652

---

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

- [ ] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
- [ ] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules/structure.md).
- [ ] Wrote unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [ ] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`)
- [ ] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code).
- [ ] Added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md`
- [ ] Re-reviewed `Files changed` in the Github PR explorer
- [ ] Review `Codecov Report` in the comment section below once CI passes

(cherry picked from commit 13559f9)
RiccardoM pushed a commit to desmos-labs/cosmos-sdk that referenced this pull request Nov 14, 2022
@robert-zaremba @RiccardoM
feat: file watcher for cosmovisor (cosmos#8590)
c67c248 
Adding upgrade file watcher for cosmovisor.

Currently the comswisor upgrade mechanism relays on parsing log messages. This is not reliable:
+ depends on the log level output (x/upgrade uses INFO)
+ can be hacked by accidentally logging user user content
+ can be broken by using upgrade name which will break the regex pattern.

closes: cosmos#7703
closes: cosmos#8523
closes: cosmos#8651
closes: cosmos#8793
closes: cosmos#8964

**Depends on**:
- cosmos#9652

---

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

- [ ] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
- [ ] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules/structure.md).
- [ ] Wrote unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [ ] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`)
- [ ] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code).
- [ ] Added a relevant changelog entry to the `Unreleased` section in `CHANGELOG.md`
- [ ] Re-reviewed `Files changed` in the Github PR explorer
- [ ] Review `Codecov Report` in the comment section below once CI passes

(cherry picked from commit 13559f9)
@coderabbitai coderabbitai bot mentioned this pull request Apr 23, 2024
Merged
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aaronc aaronc aaronc left review comments

@amaury1093 amaury1093 amaury1093 left review comments

@robert-zaremba robert-zaremba robert-zaremba approved these changes

@ValarDragon ValarDragon ValarDragon approved these changes

@ryanchristo ryanchristo ryanchristo approved these changes

@alexanderbez alexanderbez Awaiting requested review from alexanderbez

Assignees
No one assigned
Labels
A:automerge Automatically merge PR once all prerequisites pass. C:Cosmovisor Issues and PR related to Cosmovisor
Projects
None yet
Milestone
cosmovisor v0.1
Development

Successfully merging this pull request may close these issues.
8 participants
@anilcse @aaronc @robert-zaremba @amaury1093 @alexanderbez @ValarDragon @ryanchristo @okwme